Misc changes (#14702)

y21 · web-flow · commit daeb6a1d180b · 2025-05-17T10:40:21.000Z
This mainly fixes `with_leading_whitespace` not always adding the
whitespace it can.

changelog: None
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -98,7 +98,7 @@ LL |         if x.is_empty() {
 LL |             let _ = || {
 LL ~                 let _x = x;
 LL |             };
-LL ~             return ;
+LL ~             return;
 LL |         }
 LL ~         println!("test");
    |

Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ pub(super) fn check_impl_item<'tcx>(`
`55`	`55`	`// Don't lint if method is a trait's implementation, we can't do anything about those`
`56`	`56`	`if let hir::ImplItemKind::Fn(ref sig, _) = item.kind`
`57`	`57`	`&& let Some((hir_ty, err_ty)) = result_err_ty(cx, sig.decl, item.owner_id.def_id, item.span)`
`58`		`- && trait_ref_of_method(cx, item.owner_id.def_id).is_none()`
	`58`	`+ && trait_ref_of_method(cx, item.owner_id).is_none()`
`59`	`59`	`{`
`60`	`60`	`if cx.effective_visibilities.is_exported(item.owner_id.def_id) {`
`61`	`61`	`let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());`
Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ impl<'tcx> LateLintPass<'tcx> for InherentToString {`
`106`	`106`	`// Check if return type is String`
`107`	`107`	`&& is_type_lang_item(cx, return_ty(cx, impl_item.owner_id), LangItem::String)`
`108`	`108`	`// Filters instances of to_string which are required by a trait`
`109`		`- && trait_ref_of_method(cx, impl_item.owner_id.def_id).is_none()`
	`109`	`+ && trait_ref_of_method(cx, impl_item.owner_id).is_none()`
`110`	`110`	`{`
`111`	`111`	`show_lint(cx, impl_item);`
`112`	`112`	`}`