about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2025-09-05 17:31:56 +0000
committerbors <bors@rust-lang.org>2025-09-05 17:31:56 +0000
commit99317ef14d0be42fa4039eea7c5ce50cb4e9aee7 (patch)
tree1f55914225100e3fdb6031057dfc2c588079cf67
parent9cd272dc85320e85a8c83a1a338870de52c005f3 (diff)
parentb307a1146b4af3b5808510e44a13f56f2b0252e9 (diff)
downloadrust-99317ef14d0be42fa4039eea7c5ce50cb4e9aee7.tar.gz
rust-99317ef14d0be42fa4039eea7c5ce50cb4e9aee7.zip
Auto merge of #146121 - Muscraft:filter-suggestion-parts, r=petrochenkov
fix: Filter suggestion parts that match existing code

While testing my changes to make `rustc` use `annotate-snippets`, I encountered a new `clippy` test failure stemming from [two](https://github.com/rust-lang/rust/pull/145273/files#diff-6e8403e31463539666afbc00479cb416dc767a518f562b6e2960630953ee7da2R275-R278) [suggestion](https://github.com/rust-lang/rust/pull/145273/files#diff-6e8403e31463539666afbc00479cb416dc767a518f562b6e2960630953ee7da2R289-R292) output changes in rust-lang/rust#145273. The new output in these two cases feels like a regression as it is not as clear as the old output, and adds unnecessary information.

Before rust-lang/rust#145273 (`Diff` style)
![before](https://github.com/user-attachments/assets/36f33635-cbce-45f1-823d-0cbe6f0cfe46)

After rust-lang/rust#145273 ("multi-line" style)
![after](https://github.com/user-attachments/assets/d4cb00b8-5a42-436e-9329-db84347138f0)

The reason for the change was that a new suggestion part (which matches existing code) was added on a different line than the existing parts, causing the suggestion style to change from `Diff` to "multi-line". Since this new part matches existing code, no code changes show up in the output for it, but it still makes the suggestion style "multi-line" when it doesn't need to be.

To get the old output back, I made it so that suggestion parts that perfectly match existing code get filtered out.

try-job: aarch64-apple
-rw-r--r--compiler/rustc_errors/src/diagnostic.rs5
-rw-r--r--compiler/rustc_errors/src/emitter.rs1
-rw-r--r--compiler/rustc_errors/src/lib.rs27
-rw-r--r--src/tools/clippy/tests/ui/bool_assert_comparison.stderr12
4 files changed, 21 insertions, 24 deletions
diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs
index 96a4ed3218f..ae23ef1e255 100644
--- a/compiler/rustc_errors/src/diagnostic.rs
+++ b/compiler/rustc_errors/src/diagnostic.rs
@@ -945,11 +945,6 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
             None,
             "Span must not be empty and have no suggestion",
         );
-        debug_assert_eq!(
-            parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)),
-            None,
-            "suggestion must not have overlapping parts",
-        );
 
         self.push_suggestion(CodeSuggestion {
             substitutions: vec![Substitution { parts }],
diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs
index b94370e8e9b..93b1e6b7615 100644
--- a/compiler/rustc_errors/src/emitter.rs
+++ b/compiler/rustc_errors/src/emitter.rs
@@ -2354,7 +2354,6 @@ impl HumanEmitter {
                         .sum();
                     let underline_start = (span_start_pos + start) as isize + offset;
                     let underline_end = (span_start_pos + start + sub_len) as isize + offset;
-                    assert!(underline_start >= 0 && underline_end >= 0);
                     let padding: usize = max_line_num_len + 3;
                     for p in underline_start..underline_end {
                         if let DisplaySuggestion::Underline = show_code_change
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index a56e0f3fae1..8869799ce90 100644
--- a/compiler/rustc_errors/src/lib.rs
+++ b/compiler/rustc_errors/src/lib.rs
@@ -381,6 +381,17 @@ impl CodeSuggestion {
                 // Assumption: all spans are in the same file, and all spans
                 // are disjoint. Sort in ascending order.
                 substitution.parts.sort_by_key(|part| part.span.lo());
+                // Verify the assumption that all spans are disjoint
+                assert_eq!(
+                    substitution.parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)),
+                    None,
+                    "all spans must be disjoint",
+                );
+
+                // Account for cases where we are suggesting the same code that's already
+                // there. This shouldn't happen often, but in some cases for multipart
+                // suggestions it's much easier to handle it here than in the origin.
+                substitution.parts.retain(|p| is_different(sm, &p.snippet, p.span));
 
                 // Find the bounding span.
                 let lo = substitution.parts.iter().map(|part| part.span.lo()).min()?;
@@ -470,16 +481,12 @@ impl CodeSuggestion {
                             _ => 1,
                         })
                         .sum();
-                    if !is_different(sm, &part.snippet, part.span) {
-                        // Account for cases where we are suggesting the same code that's already
-                        // there. This shouldn't happen often, but in some cases for multipart
-                        // suggestions it's much easier to handle it here than in the origin.
-                    } else {
-                        line_highlight.push(SubstitutionHighlight {
-                            start: (cur_lo.col.0 as isize + acc) as usize,
-                            end: (cur_lo.col.0 as isize + acc + len) as usize,
-                        });
-                    }
+
+                    line_highlight.push(SubstitutionHighlight {
+                        start: (cur_lo.col.0 as isize + acc) as usize,
+                        end: (cur_lo.col.0 as isize + acc + len) as usize,
+                    });
+
                     buf.push_str(&part.snippet);
                     let cur_hi = sm.lookup_char_pos(part.span.hi());
                     // Account for the difference between the width of the current code and the
diff --git a/src/tools/clippy/tests/ui/bool_assert_comparison.stderr b/src/tools/clippy/tests/ui/bool_assert_comparison.stderr
index f823f08f31d..72aa6303a20 100644
--- a/src/tools/clippy/tests/ui/bool_assert_comparison.stderr
+++ b/src/tools/clippy/tests/ui/bool_assert_comparison.stderr
@@ -272,10 +272,8 @@ LL |     assert_eq!(a!(), true);
    |
 help: replace it with `assert!(..)`
    |
-LL |         true
-...
-LL |
-LL ~     assert!(a!());
+LL -     assert_eq!(a!(), true);
+LL +     assert!(a!());
    |
 
 error: used `assert_eq!` with a literal bool
@@ -286,10 +284,8 @@ LL |     assert_eq!(true, b!());
    |
 help: replace it with `assert!(..)`
    |
-LL |         true
-...
-LL |
-LL ~     assert!(b!());
+LL -     assert_eq!(true, b!());
+LL +     assert!(b!());
    |
 
 error: used `debug_assert_eq!` with a literal bool