diff options
| author | Timo <30553356+y21@users.noreply.github.com> | 2025-05-17 10:40:21 +0000 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-05-17 10:40:21 +0000 |
| commit | daeb6a1d180b040ed4f61485cc1294859a287f88 (patch) | |
| tree | f7d913f14aaa45b15ad044bdd270c9ab0628f532 | |
| parent | 9ebfb84fe04e512a17ad6661030a4c6d9dca8d6b (diff) | |
| parent | 77157f59887f8711311ff6049d744abea4eb7d0f (diff) | |
| download | rust-daeb6a1d180b040ed4f61485cc1294859a287f88.tar.gz rust-daeb6a1d180b040ed4f61485cc1294859a287f88.zip | |
Misc changes (#14702)
This mainly fixes `with_leading_whitespace` not always adding the whitespace it can. changelog: None
| -rw-r--r-- | clippy_lints/src/cognitive_complexity.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/copies.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/extra_unused_type_parameters.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/functions/must_use.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/functions/result.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/implicit_hasher.rs | 13 | ||||
| -rw-r--r-- | clippy_lints/src/inherent_to_string.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/lifetimes.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/methods/manual_inspect.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/missing_const_for_fn.rs | 4 | ||||
| -rw-r--r-- | clippy_lints/src/mut_key.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/operators/assign_op_pattern.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/suspicious_trait_impl.rs | 2 | ||||
| -rw-r--r-- | clippy_utils/src/lib.rs | 8 | ||||
| -rw-r--r-- | clippy_utils/src/source.rs | 86 | ||||
| -rw-r--r-- | tests/ui-toml/collapsible_if/collapsible_if.fixed | 4 | ||||
| -rw-r--r-- | tests/ui-toml/collapsible_if/collapsible_if.stderr | 4 | ||||
| -rw-r--r-- | tests/ui/manual_inspect.fixed | 2 | ||||
| -rw-r--r-- | tests/ui/manual_inspect.stderr | 2 |
19 files changed, 90 insertions, 55 deletions
diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index 1d44c7e9c88..aeb53a52cf3 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -104,7 +104,7 @@ impl CognitiveComplexity { FnKind::Closure => { let header_span = body_span.with_hi(decl.output.span().lo()); #[expect(clippy::range_plus_one)] - if let Some(range) = header_span.map_range(cx, |src, range| { + if let Some(range) = header_span.map_range(cx, |_, src, range| { let mut idxs = src.get(range.clone())?.match_indices('|'); Some(range.start + idxs.next()?.0..range.start + idxs.next()?.0 + 1) }) { diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 48bf3dafa6e..2467fc95fd0 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -258,7 +258,7 @@ fn lint_branches_sharing_code<'tcx>( let span = span.with_hi(last_block.span.hi()); // Improve formatting if the inner block has indentation (i.e. normal Rust formatting) let span = span - .map_range(cx, |src, range| { + .map_range(cx, |_, src, range| { (range.start > 4 && src.get(range.start - 4..range.start)? == " ") .then_some(range.start - 4..range.end) }) diff --git a/clippy_lints/src/extra_unused_type_parameters.rs b/clippy_lints/src/extra_unused_type_parameters.rs index 6a217b6182c..c0b0fd88d9e 100644 --- a/clippy_lints/src/extra_unused_type_parameters.rs +++ b/clippy_lints/src/extra_unused_type_parameters.rs @@ -273,7 +273,7 @@ impl<'tcx> LateLintPass<'tcx> for ExtraUnusedTypeParameters { // Only lint on inherent methods, not trait methods. if let ImplItemKind::Fn(.., body_id) = item.kind && !item.generics.params.is_empty() - && trait_ref_of_method(cx, item.owner_id.def_id).is_none() + && trait_ref_of_method(cx, item.owner_id).is_none() && !is_empty_body(cx, body_id) && (!self.avoid_breaking_exported_api || !cx.effective_visibilities.is_exported(item.owner_id.def_id)) && !item.span.in_external_macro(cx.sess().source_map()) diff --git a/clippy_lints/src/functions/must_use.rs b/clippy_lints/src/functions/must_use.rs index c3e0d5e8b69..70655838b6a 100644 --- a/clippy_lints/src/functions/must_use.rs +++ b/clippy_lints/src/functions/must_use.rs @@ -55,7 +55,7 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp let attr = cx.tcx.get_attr(item.owner_id, sym::must_use); if let Some(attr) = attr { check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig); - } else if is_public && !is_proc_macro(attrs) && trait_ref_of_method(cx, item.owner_id.def_id).is_none() { + } else if is_public && !is_proc_macro(attrs) && trait_ref_of_method(cx, item.owner_id).is_none() { check_must_use_candidate( cx, sig.decl, diff --git a/clippy_lints/src/functions/result.rs b/clippy_lints/src/functions/result.rs index 07a92a4ed70..00ce4cfcc52 100644 --- a/clippy_lints/src/functions/result.rs +++ b/clippy_lints/src/functions/result.rs @@ -55,7 +55,7 @@ pub(super) fn check_impl_item<'tcx>( // Don't lint if method is a trait's implementation, we can't do anything about those if let hir::ImplItemKind::Fn(ref sig, _) = item.kind && let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.owner_id.def_id, item.span) - && trait_ref_of_method(cx, item.owner_id.def_id).is_none() + && trait_ref_of_method(cx, item.owner_id).is_none() { if cx.effective_visibilities.is_exported(item.owner_id.def_id) { let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); diff --git a/clippy_lints/src/implicit_hasher.rs b/clippy_lints/src/implicit_hasher.rs index 4c17834c3ad..cab7a9fb709 100644 --- a/clippy_lints/src/implicit_hasher.rs +++ b/clippy_lints/src/implicit_hasher.rs @@ -119,7 +119,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher { } let generics_suggestion_span = impl_.generics.span.substitute_dummy({ - let range = (item.span.lo()..target.span().lo()).map_range(cx, |src, range| { + let range = (item.span.lo()..target.span().lo()).map_range(cx, |_, src, range| { Some(src.get(range.clone())?.find("impl")? + 4..range.end) }); if let Some(range) = range { @@ -165,11 +165,12 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitHasher { continue; } let generics_suggestion_span = generics.span.substitute_dummy({ - let range = (item.span.lo()..body.params[0].pat.span.lo()).map_range(cx, |src, range| { - let (pre, post) = src.get(range.clone())?.split_once("fn")?; - let pos = post.find('(')? + pre.len() + 2; - Some(pos..pos) - }); + let range = + (item.span.lo()..body.params[0].pat.span.lo()).map_range(cx, |_, src, range| { + let (pre, post) = src.get(range.clone())?.split_once("fn")?; + let pos = post.find('(')? + pre.len() + 2; + Some(pos..pos) + }); if let Some(range) = range { range.with_ctxt(item.span.ctxt()) } else { diff --git a/clippy_lints/src/inherent_to_string.rs b/clippy_lints/src/inherent_to_string.rs index 1d582fb0223..7f2e25367a6 100644 --- a/clippy_lints/src/inherent_to_string.rs +++ b/clippy_lints/src/inherent_to_string.rs @@ -106,7 +106,7 @@ impl<'tcx> LateLintPass<'tcx> for InherentToString { // Check if return type is String && is_type_lang_item(cx, return_ty(cx, impl_item.owner_id), LangItem::String) // Filters instances of to_string which are required by a trait - && trait_ref_of_method(cx, impl_item.owner_id.def_id).is_none() + && trait_ref_of_method(cx, impl_item.owner_id).is_none() { show_lint(cx, impl_item); } diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 9a64226b1ed..8fe0c9d60f9 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -159,7 +159,7 @@ impl<'tcx> LateLintPass<'tcx> for Lifetimes { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { if let ImplItemKind::Fn(ref sig, id) = item.kind { - let report_extra_lifetimes = trait_ref_of_method(cx, item.owner_id.def_id).is_none(); + let report_extra_lifetimes = trait_ref_of_method(cx, item.owner_id).is_none(); check_fn_inner( cx, sig, diff --git a/clippy_lints/src/methods/manual_inspect.rs b/clippy_lints/src/methods/manual_inspect.rs index 173ebcb7020..c47879442fc 100644 --- a/clippy_lints/src/methods/manual_inspect.rs +++ b/clippy_lints/src/methods/manual_inspect.rs @@ -101,7 +101,7 @@ pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>, name: UseKind::Return(s) => edits.push((s.with_leading_whitespace(cx).with_ctxt(s.ctxt()), String::new())), UseKind::Borrowed(s) => { #[expect(clippy::range_plus_one)] - let range = s.map_range(cx, |src, range| { + let range = s.map_range(cx, |_, src, range| { let src = src.get(range.clone())?; let trimmed = src.trim_start_matches([' ', '\t', '\n', '\r', '(']); trimmed.starts_with('&').then(|| { diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index f3e24044fb6..a6be7581c9a 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -7,7 +7,7 @@ use rustc_abi::ExternAbi; use rustc_errors::Applicability; use rustc_hir::def_id::CRATE_DEF_ID; use rustc_hir::intravisit::FnKind; -use rustc_hir::{self as hir, Body, Constness, FnDecl, GenericParamKind}; +use rustc_hir::{self as hir, Body, Constness, FnDecl, GenericParamKind, OwnerId}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::impl_lint_pass; @@ -125,7 +125,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { } }, FnKind::Method(_, sig, ..) => { - if already_const(sig.header) || trait_ref_of_method(cx, def_id).is_some() { + if already_const(sig.header) || trait_ref_of_method(cx, OwnerId { def_id }).is_some() { return; } }, diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index a45031ce22b..98a9a98d281 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for MutableKeyType<'tcx> { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'tcx>) { if let hir::ImplItemKind::Fn(ref sig, ..) = item.kind - && trait_ref_of_method(cx, item.owner_id.def_id).is_none() + && trait_ref_of_method(cx, item.owner_id).is_none() { self.check_sig(cx, item.owner_id.def_id, sig.decl); } diff --git a/clippy_lints/src/operators/assign_op_pattern.rs b/clippy_lints/src/operators/assign_op_pattern.rs index 03b907ebdf4..4be42267b14 100644 --- a/clippy_lints/src/operators/assign_op_pattern.rs +++ b/clippy_lints/src/operators/assign_op_pattern.rs @@ -26,7 +26,7 @@ pub(super) fn check<'tcx>( let rty = cx.typeck_results().expr_ty(rhs); if let Some((_, lang_item)) = binop_traits(op.node) && let Some(trait_id) = cx.tcx.lang_items().get(lang_item) - && let parent_fn = cx.tcx.hir_get_parent_item(e.hir_id).def_id + && let parent_fn = cx.tcx.hir_get_parent_item(e.hir_id) && trait_ref_of_method(cx, parent_fn).is_none_or(|t| t.path.res.def_id() != trait_id) && implements_trait(cx, ty, trait_id, &[rty.into()]) { diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 83241f97a99..edb7600b7c0 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -80,7 +80,7 @@ fn check_expr_inner<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, bin && let hir::Node::ImplItem(impl_item) = cx.tcx.hir_node_by_def_id(parent_fn) && let hir::ImplItemKind::Fn(_, body_id) = impl_item.kind && let body = cx.tcx.hir_body(body_id) - && let parent_fn = cx.tcx.hir_get_parent_item(expr.hir_id).def_id + && let parent_fn = cx.tcx.hir_get_parent_item(expr.hir_id) && let Some(trait_ref) = trait_ref_of_method(cx, parent_fn) && let trait_id = trait_ref.path.res.def_id() && ![binop_trait_id, op_assign_trait_id].contains(&trait_id) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index accfb720eef..057b6e62da3 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -523,12 +523,8 @@ pub fn path_def_id<'tcx>(cx: &LateContext<'_>, maybe_path: &impl MaybePath<'tcx> /// } /// } /// ``` -pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'tcx>, def_id: LocalDefId) -> Option<&'tcx TraitRef<'tcx>> { - // Get the implemented trait for the current function - let hir_id = cx.tcx.local_def_id_to_hir_id(def_id); - let parent_impl = cx.tcx.hir_get_parent_item(hir_id); - if parent_impl != hir::CRATE_OWNER_ID - && let Node::Item(item) = cx.tcx.hir_node_by_def_id(parent_impl.def_id) +pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'tcx>, owner: OwnerId) -> Option<&'tcx TraitRef<'tcx>> { + if let Node::Item(item) = cx.tcx.hir_node(cx.tcx.hir_owner_parent(owner)) && let ItemKind::Impl(impl_) = &item.kind { return impl_.of_trait.as_ref(); diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index 8645d5730fe..7f2bf99daff 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -7,13 +7,14 @@ use std::sync::Arc; use rustc_ast::{LitKind, StrStyle}; use rustc_errors::Applicability; use rustc_hir::{BlockCheckMode, Expr, ExprKind, UnsafeSource}; +use rustc_lexer::{LiteralKind, TokenKind, tokenize}; use rustc_lint::{EarlyContext, LateContext}; use rustc_middle::ty::TyCtxt; use rustc_session::Session; use rustc_span::source_map::{SourceMap, original_sp}; use rustc_span::{ - BytePos, DUMMY_SP, FileNameDisplayPreference, Pos, SourceFile, SourceFileAndLine, Span, SpanData, SyntaxContext, - hygiene, + BytePos, DUMMY_SP, FileNameDisplayPreference, Pos, RelativeBytePos, SourceFile, SourceFileAndLine, Span, SpanData, + SyntaxContext, hygiene, }; use std::borrow::Cow; use std::fmt; @@ -137,25 +138,25 @@ pub trait SpanRangeExt: SpanRange { fn map_range( self, cx: &impl HasSession, - f: impl for<'a> FnOnce(&'a str, Range<usize>) -> Option<Range<usize>>, + f: impl for<'a> FnOnce(&'a SourceFile, &'a str, Range<usize>) -> Option<Range<usize>>, ) -> Option<Range<BytePos>> { map_range(cx.sess().source_map(), self.into_range(), f) } #[allow(rustdoc::invalid_rust_codeblocks, reason = "The codeblock is intentionally broken")] - /// Extends the range to include all preceding whitespace characters, unless there - /// are non-whitespace characters left on the same line after `self`. + /// Extends the range to include all preceding whitespace characters. + /// + /// The range will not be expanded if it would cross a line boundary, the line the range would + /// be extended to ends with a line comment and the text after the range contains a + /// non-whitespace character on the same line. e.g. /// - /// This extra condition prevents a problem when removing the '}' in: /// ```ignore - /// ( // There was an opening bracket after the parenthesis, which has been removed - /// // This is a comment - /// }) + /// ( // Some comment + /// foo) /// ``` - /// Removing the whitespaces, including the linefeed, before the '}', would put the - /// closing parenthesis at the end of the `// This is a comment` line, which would - /// make it part of the comment as well. In this case, it is best to keep the span - /// on the '}' alone. + /// + /// When the range points to `foo`, suggesting to remove the range after it's been extended will + /// cause the `)` to be placed inside the line comment as `( // Some comment)`. fn with_leading_whitespace(self, cx: &impl HasSession) -> Range<BytePos> { with_leading_whitespace(cx.sess().source_map(), self.into_range()) } @@ -254,11 +255,11 @@ fn with_source_text_and_range<T>( fn map_range( sm: &SourceMap, sp: Range<BytePos>, - f: impl for<'a> FnOnce(&'a str, Range<usize>) -> Option<Range<usize>>, + f: impl for<'a> FnOnce(&'a SourceFile, &'a str, Range<usize>) -> Option<Range<usize>>, ) -> Option<Range<BytePos>> { if let Some(src) = get_source_range(sm, sp.clone()) && let Some(text) = &src.sf.src - && let Some(range) = f(text, src.range.clone()) + && let Some(range) = f(&src.sf, text, src.range.clone()) { debug_assert!( range.start <= text.len() && range.end <= text.len(), @@ -275,20 +276,57 @@ fn map_range( } } +fn ends_with_line_comment_or_broken(text: &str) -> bool { + let Some(last) = tokenize(text).last() else { + return false; + }; + match last.kind { + // Will give the wrong result on text like `" // "` where the first quote ends a string + // started earlier. The only workaround is to lex the whole file which we don't really want + // to do. + TokenKind::LineComment { .. } | TokenKind::BlockComment { terminated: false, .. } => true, + TokenKind::Literal { kind, .. } => matches!( + kind, + LiteralKind::Byte { terminated: false } + | LiteralKind::ByteStr { terminated: false } + | LiteralKind::CStr { terminated: false } + | LiteralKind::Char { terminated: false } + | LiteralKind::RawByteStr { n_hashes: None } + | LiteralKind::RawCStr { n_hashes: None } + | LiteralKind::RawStr { n_hashes: None } + ), + _ => false, + } +} + +fn with_leading_whitespace_inner(lines: &[RelativeBytePos], src: &str, range: Range<usize>) -> Option<usize> { + debug_assert!(lines.is_empty() || lines[0].to_u32() == 0); + + let start = src.get(..range.start)?.trim_end(); + let next_line = lines.partition_point(|&pos| pos.to_usize() <= start.len()); + if let Some(line_end) = lines.get(next_line) + && line_end.to_usize() <= range.start + && let prev_start = lines.get(next_line - 1).map_or(0, |&x| x.to_usize()) + && ends_with_line_comment_or_broken(&start[prev_start..]) + && let next_line = lines.partition_point(|&pos| pos.to_usize() < range.end) + && let next_start = lines.get(next_line).map_or(src.len(), |&x| x.to_usize()) + && tokenize(src.get(range.end..next_start)?).any(|t| !matches!(t.kind, TokenKind::Whitespace)) + { + Some(range.start) + } else { + Some(start.len()) + } +} + fn with_leading_whitespace(sm: &SourceMap, sp: Range<BytePos>) -> Range<BytePos> { - map_range(sm, sp, |src, range| { - let non_blank_after = src.len() - src.get(range.end..)?.trim_start().len(); - if src.get(range.end..non_blank_after)?.contains(['\r', '\n']) { - Some(src.get(..range.start)?.trim_end().len()..range.end) - } else { - Some(range) - } + map_range(sm, sp.clone(), |sf, src, range| { + Some(with_leading_whitespace_inner(sf.lines(), src, range.clone())?..range.end) }) - .unwrap() + .unwrap_or(sp) } fn trim_start(sm: &SourceMap, sp: Range<BytePos>) -> Range<BytePos> { - map_range(sm, sp.clone(), |src, range| { + map_range(sm, sp.clone(), |_, src, range| { let src = src.get(range.clone())?; Some(range.start + (src.len() - src.trim_start().len())..range.end) }) diff --git a/tests/ui-toml/collapsible_if/collapsible_if.fixed b/tests/ui-toml/collapsible_if/collapsible_if.fixed index 6f5cc47ba6c..f695f9804d5 100644 --- a/tests/ui-toml/collapsible_if/collapsible_if.fixed +++ b/tests/ui-toml/collapsible_if/collapsible_if.fixed @@ -13,7 +13,7 @@ fn main() { //~^^^^^^ collapsible_if // The following tests check for the fix of https://github.com/rust-lang/rust-clippy/issues/798 - if x == "hello" // Inner comment + if x == "hello" // Inner comment && y == "world" { println!("Hello world!"); } @@ -26,7 +26,7 @@ fn main() { } //~^^^^^^ collapsible_if - if x == "hello" /* Inner comment */ + if x == "hello" /* Inner comment */ && y == "world" { println!("Hello world!"); } diff --git a/tests/ui-toml/collapsible_if/collapsible_if.stderr b/tests/ui-toml/collapsible_if/collapsible_if.stderr index 357ce4ad32d..a12c2112f58 100644 --- a/tests/ui-toml/collapsible_if/collapsible_if.stderr +++ b/tests/ui-toml/collapsible_if/collapsible_if.stderr @@ -32,7 +32,7 @@ LL | | } | help: collapse nested if block | -LL ~ if x == "hello" // Inner comment +LL ~ if x == "hello" // Inner comment LL ~ && y == "world" { LL | println!("Hello world!"); LL ~ } @@ -70,7 +70,7 @@ LL | | } | help: collapse nested if block | -LL ~ if x == "hello" /* Inner comment */ +LL ~ if x == "hello" /* Inner comment */ LL ~ && y == "world" { LL | println!("Hello world!"); LL ~ } diff --git a/tests/ui/manual_inspect.fixed b/tests/ui/manual_inspect.fixed index ec87fe217ae..9b768dbad70 100644 --- a/tests/ui/manual_inspect.fixed +++ b/tests/ui/manual_inspect.fixed @@ -107,7 +107,7 @@ fn main() { let _ = || { let _x = x; }; - return ; + return; } println!("test"); }); diff --git a/tests/ui/manual_inspect.stderr b/tests/ui/manual_inspect.stderr index eb98f9f5995..78b085fdfca 100644 --- a/tests/ui/manual_inspect.stderr +++ b/tests/ui/manual_inspect.stderr @@ -98,7 +98,7 @@ LL | if x.is_empty() { LL | let _ = || { LL ~ let _x = x; LL | }; -LL ~ return ; +LL ~ return; LL | } LL ~ println!("test"); | |
