Auto merge of rust-lang#137348 - compiler-errors:span-trim, r=estebank

More sophisticated span trimming for suggestions

Previously rust-lang#136958 only cared about prefixes or suffixes. Now it detects more cases where a suggestion is "sandwiched" by unchanged code on the left or the right. Would be cool if we could detect several insertions, like `ACE` going to `ABCDE`, extracting `B` and `D`, but that seems unwieldy.

r? `@estebank`

compiler/rustc_errors/src/emitter.rs

@@ -2216,12 +2216,7 @@ impl HumanEmitter {
             if let DisplaySuggestion::Diff | DisplaySuggestion::Underline | DisplaySuggestion::Add =
                 show_code_change
             {
-                for mut part in parts {
-                    // If this is a replacement of, e.g. `"a"` into `"ab"`, adjust the
-                    // suggestion and snippet to look as if we just suggested to add
-                    // `"b"`, which is typically much easier for the user to understand.
-                    part.trim_trivial_replacements(sm);
-
+                for part in parts {
                     let snippet = if let Ok(snippet) = sm.span_to_snippet(part.span) {
                         snippet
                     } else {

compiler/rustc_errors/src/lib.rs

@@ -71,7 +71,7 @@ use rustc_macros::{Decodable, Encodable};
 pub use rustc_span::ErrorGuaranteed;
 pub use rustc_span::fatal_error::{FatalError, FatalErrorMarker};
 use rustc_span::source_map::SourceMap;
-use rustc_span::{DUMMY_SP, Loc, Span};
+use rustc_span::{BytePos, DUMMY_SP, Loc, Span};
 pub use snippet::Style;
 // Used by external projects such as `rust-gpu`.
 // See https://github.com/rust-lang/rust/pull/115393.
@@ -237,10 +237,9 @@ impl SubstitutionPart {
     /// it with "abx" is, since the "c" character is lost.
     pub fn is_destructive_replacement(&self, sm: &SourceMap) -> bool {
         self.is_replacement(sm)
-            && !sm.span_to_snippet(self.span).is_ok_and(|snippet| {
-                self.snippet.trim_start().starts_with(snippet.trim_start())
-                    || self.snippet.trim_end().ends_with(snippet.trim_end())
-            })
+            && !sm
+                .span_to_snippet(self.span)
+                .is_ok_and(|snippet| as_substr(snippet.trim(), self.snippet.trim()).is_some())
     }
 
     fn replaces_meaningful_content(&self, sm: &SourceMap) -> bool {
@@ -257,16 +256,40 @@ impl SubstitutionPart {
         let Ok(snippet) = sm.span_to_snippet(self.span) else {
             return;
         };
-        if self.snippet.starts_with(&snippet) {
-            self.span = self.span.shrink_to_hi();
-            self.snippet = self.snippet[snippet.len()..].to_string();
-        } else if self.snippet.ends_with(&snippet) {
-            self.span = self.span.shrink_to_lo();
-            self.snippet = self.snippet[..self.snippet.len() - snippet.len()].to_string();
+
+        if let Some((prefix, substr, suffix)) = as_substr(&snippet, &self.snippet) {
+            self.span = Span::new(
+                self.span.lo() + BytePos(prefix as u32),
+                self.span.hi() - BytePos(suffix as u32),
+                self.span.ctxt(),
+                self.span.parent(),
+            );
+            self.snippet = substr.to_string();
         }
     }
 }
 
+/// Given an original string like `AACC`, and a suggestion like `AABBCC`, try to detect
+/// the case where a substring of the suggestion is "sandwiched" in the original, like
+/// `BB` is. Return the length of the prefix, the "trimmed" suggestion, and the length
+/// of the suffix.
+fn as_substr<'a>(original: &'a str, suggestion: &'a str) -> Option<(usize, &'a str, usize)> {
+    let common_prefix = original
+        .chars()
+        .zip(suggestion.chars())
+        .take_while(|(c1, c2)| c1 == c2)
+        .map(|(c, _)| c.len_utf8())
+        .sum();
+    let original = &original[common_prefix..];
+    let suggestion = &suggestion[common_prefix..];
+    if suggestion.ends_with(original) {
+        let common_suffix = original.len();
+        Some((common_prefix, &suggestion[..suggestion.len() - original.len()], common_suffix))
+    } else {
+        None
+    }
+}
+
 impl CodeSuggestion {
     /// Returns the assembled code suggestions, whether they should be shown with an underline
     /// and whether the substitution only differs in capitalization.
@@ -380,7 +403,12 @@ impl CodeSuggestion {
                 // or deleted code in order to point at the correct column *after* substitution.
                 let mut acc = 0;
                 let mut only_capitalization = false;
-                for part in &substitution.parts {
+                for part in &mut substitution.parts {
+                    // If this is a replacement of, e.g. `"a"` into `"ab"`, adjust the
+                    // suggestion and snippet to look as if we just suggested to add
+                    // `"b"`, which is typically much easier for the user to understand.
+                    part.trim_trivial_replacements(sm);
+
                     only_capitalization |= is_case_difference(sm, &part.snippet, part.span);
                     let cur_lo = sm.lookup_char_pos(part.span.lo());
                     if prev_hi.line == cur_lo.line {

src/tools/clippy/tests/ui/async_yields_async.stderr

@@ -14,9 +14,9 @@ LL | |      };
    = help: to override `-D warnings` add `#[allow(clippy::async_yields_async)]`
 help: consider awaiting this value
    |
-LL ~         async {
-LL +             3
-LL +         }.await
+LL |         async {
+LL |             3
+LL ~         }.await
    |
 
 error: an async construct yields a type which is itself awaitable
@@ -46,9 +46,9 @@ LL | |      };
    |
 help: consider awaiting this value
    |
-LL ~         async {
-LL +             3
-LL +         }.await
+LL |         async {
+LL |             3
+LL ~         }.await
    |
 
 error: an async construct yields a type which is itself awaitable

src/tools/clippy/tests/ui/borrow_deref_ref_unfixable.stderr

@@ -13,9 +13,8 @@ LL +         let x: &str = s;
    |
 help: if you would like to deref, try using `&**`
    |
-LL -         let x: &str = &*s;
-LL +         let x: &str = &**s;
-   |
+LL |         let x: &str = &**s;
+   |                         +
 
 error: aborting due to 1 previous error
 

src/tools/clippy/tests/ui/fn_to_numeric_cast_any.stderr

@@ -8,9 +8,8 @@ LL |     let _ = foo as i8;
    = help: to override `-D warnings` add `#[allow(clippy::fn_to_numeric_cast_any)]`
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as i8;
-LL +     let _ = foo() as i8;
-   |
+LL |     let _ = foo() as i8;
+   |                ++
 
 error: casting function pointer `foo` to `i16`
   --> tests/ui/fn_to_numeric_cast_any.rs:26:13
@@ -20,9 +19,8 @@ LL |     let _ = foo as i16;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as i16;
-LL +     let _ = foo() as i16;
-   |
+LL |     let _ = foo() as i16;
+   |                ++
 
 error: casting function pointer `foo` to `i32`
   --> tests/ui/fn_to_numeric_cast_any.rs:28:13
@@ -32,9 +30,8 @@ LL |     let _ = foo as i32;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as i32;
-LL +     let _ = foo() as i32;
-   |
+LL |     let _ = foo() as i32;
+   |                ++
 
 error: casting function pointer `foo` to `i64`
   --> tests/ui/fn_to_numeric_cast_any.rs:30:13
@@ -44,9 +41,8 @@ LL |     let _ = foo as i64;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as i64;
-LL +     let _ = foo() as i64;
-   |
+LL |     let _ = foo() as i64;
+   |                ++
 
 error: casting function pointer `foo` to `i128`
   --> tests/ui/fn_to_numeric_cast_any.rs:32:13
@@ -56,9 +52,8 @@ LL |     let _ = foo as i128;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as i128;
-LL +     let _ = foo() as i128;
-   |
+LL |     let _ = foo() as i128;
+   |                ++
 
 error: casting function pointer `foo` to `isize`
   --> tests/ui/fn_to_numeric_cast_any.rs:34:13
@@ -68,9 +63,8 @@ LL |     let _ = foo as isize;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as isize;
-LL +     let _ = foo() as isize;
-   |
+LL |     let _ = foo() as isize;
+   |                ++
 
 error: casting function pointer `foo` to `u8`
   --> tests/ui/fn_to_numeric_cast_any.rs:37:13
@@ -80,9 +74,8 @@ LL |     let _ = foo as u8;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as u8;
-LL +     let _ = foo() as u8;
-   |
+LL |     let _ = foo() as u8;
+   |                ++
 
 error: casting function pointer `foo` to `u16`
   --> tests/ui/fn_to_numeric_cast_any.rs:39:13
@@ -92,9 +85,8 @@ LL |     let _ = foo as u16;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as u16;
-LL +     let _ = foo() as u16;
-   |
+LL |     let _ = foo() as u16;
+   |                ++
 
 error: casting function pointer `foo` to `u32`
   --> tests/ui/fn_to_numeric_cast_any.rs:41:13
@@ -104,9 +96,8 @@ LL |     let _ = foo as u32;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as u32;
-LL +     let _ = foo() as u32;
-   |
+LL |     let _ = foo() as u32;
+   |                ++
 
 error: casting function pointer `foo` to `u64`
   --> tests/ui/fn_to_numeric_cast_any.rs:43:13
@@ -116,9 +107,8 @@ LL |     let _ = foo as u64;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as u64;
-LL +     let _ = foo() as u64;
-   |
+LL |     let _ = foo() as u64;
+   |                ++
 
 error: casting function pointer `foo` to `u128`
   --> tests/ui/fn_to_numeric_cast_any.rs:45:13
@@ -128,9 +118,8 @@ LL |     let _ = foo as u128;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as u128;
-LL +     let _ = foo() as u128;
-   |
+LL |     let _ = foo() as u128;
+   |                ++
 
 error: casting function pointer `foo` to `usize`
   --> tests/ui/fn_to_numeric_cast_any.rs:47:13
@@ -140,9 +129,8 @@ LL |     let _ = foo as usize;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as usize;
-LL +     let _ = foo() as usize;
-   |
+LL |     let _ = foo() as usize;
+   |                ++
 
 error: casting function pointer `Struct::static_method` to `usize`
   --> tests/ui/fn_to_numeric_cast_any.rs:52:13
@@ -152,9 +140,8 @@ LL |     let _ = Struct::static_method as usize;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = Struct::static_method as usize;
-LL +     let _ = Struct::static_method() as usize;
-   |
+LL |     let _ = Struct::static_method() as usize;
+   |                                  ++
 
 error: casting function pointer `f` to `usize`
   --> tests/ui/fn_to_numeric_cast_any.rs:57:5
@@ -164,9 +151,8 @@ LL |     f as usize
    |
 help: did you mean to invoke the function?
    |
-LL -     f as usize
-LL +     f() as usize
-   |
+LL |     f() as usize
+   |      ++
 
 error: casting function pointer `T::static_method` to `usize`
   --> tests/ui/fn_to_numeric_cast_any.rs:62:5
@@ -176,9 +162,8 @@ LL |     T::static_method as usize
    |
 help: did you mean to invoke the function?
    |
-LL -     T::static_method as usize
-LL +     T::static_method() as usize
-   |
+LL |     T::static_method() as usize
+   |                     ++
 
 error: casting function pointer `(clos as fn(u32) -> u32)` to `usize`
   --> tests/ui/fn_to_numeric_cast_any.rs:69:13
@@ -188,9 +173,8 @@ LL |     let _ = (clos as fn(u32) -> u32) as usize;
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = (clos as fn(u32) -> u32) as usize;
-LL +     let _ = (clos as fn(u32) -> u32)() as usize;
-   |
+LL |     let _ = (clos as fn(u32) -> u32)() as usize;
+   |                                     ++
 
 error: casting function pointer `foo` to `*const ()`
   --> tests/ui/fn_to_numeric_cast_any.rs:74:13
@@ -200,9 +184,8 @@ LL |     let _ = foo as *const ();
    |
 help: did you mean to invoke the function?
    |
-LL -     let _ = foo as *const ();
-LL +     let _ = foo() as *const ();
-   |
+LL |     let _ = foo() as *const ();
+   |                ++
 
 error: aborting due to 17 previous errors
 

src/tools/clippy/tests/ui/implicit_hasher.stderr

@@ -78,9 +78,8 @@ LL | pub fn map(map: &mut HashMap<i32, i32>) {}
    |
 help: add a type parameter for `BuildHasher`
    |
-LL - pub fn map(map: &mut HashMap<i32, i32>) {}
-LL + pub fn map<S: ::std::hash::BuildHasher>(map: &mut HashMap<i32, i32, S>) {}
-   |
+LL | pub fn map<S: ::std::hash::BuildHasher>(map: &mut HashMap<i32, i32, S>) {}
+   |           +++++++++++++++++++++++++++++                           +++
 
 error: parameter of type `HashSet` should be generalized over different hashers
   --> tests/ui/implicit_hasher.rs:70:22
@@ -90,9 +89,8 @@ LL | pub fn set(set: &mut HashSet<i32>) {}
    |
 help: add a type parameter for `BuildHasher`
    |
-LL - pub fn set(set: &mut HashSet<i32>) {}
-LL + pub fn set<S: ::std::hash::BuildHasher>(set: &mut HashSet<i32, S>) {}
-   |
+LL | pub fn set<S: ::std::hash::BuildHasher>(set: &mut HashSet<i32, S>) {}
+   |           +++++++++++++++++++++++++++++                      +++
 
 error: impl for `HashMap` should be generalized over different hashers
   --> tests/ui/implicit_hasher.rs:76:43
@@ -116,9 +114,8 @@ LL | pub async fn election_vote(_data: HashMap<i32, i32>) {}
    |
 help: add a type parameter for `BuildHasher`
    |
-LL - pub async fn election_vote(_data: HashMap<i32, i32>) {}
-LL + pub async fn election_vote<S: ::std::hash::BuildHasher>(_data: HashMap<i32, i32, S>) {}
-   |
+LL | pub async fn election_vote<S: ::std::hash::BuildHasher>(_data: HashMap<i32, i32, S>) {}
+   |                           +++++++++++++++++++++++++++++                        +++
 
 error: aborting due to 9 previous errors