Skip to content

Commit 99317ef

Browse files
committed
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 #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 #145273 (`Diff` style) ![before](https://github.com/user-attachments/assets/36f33635-cbce-45f1-823d-0cbe6f0cfe46) After #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
2 parents 9cd272d + b307a11 commit 99317ef

File tree

4 files changed

+21
-24
lines changed

4 files changed

+21
-24
lines changed

compiler/rustc_errors/src/diagnostic.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -945,11 +945,6 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
945945
None,
946946
"Span must not be empty and have no suggestion",
947947
);
948-
debug_assert_eq!(
949-
parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)),
950-
None,
951-
"suggestion must not have overlapping parts",
952-
);
953948

954949
self.push_suggestion(CodeSuggestion {
955950
substitutions: vec![Substitution { parts }],

compiler/rustc_errors/src/emitter.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2354,7 +2354,6 @@ impl HumanEmitter {
23542354
.sum();
23552355
let underline_start = (span_start_pos + start) as isize + offset;
23562356
let underline_end = (span_start_pos + start + sub_len) as isize + offset;
2357-
assert!(underline_start >= 0 && underline_end >= 0);
23582357
let padding: usize = max_line_num_len + 3;
23592358
for p in underline_start..underline_end {
23602359
if let DisplaySuggestion::Underline = show_code_change

compiler/rustc_errors/src/lib.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,17 @@ impl CodeSuggestion {
381381
// Assumption: all spans are in the same file, and all spans
382382
// are disjoint. Sort in ascending order.
383383
substitution.parts.sort_by_key(|part| part.span.lo());
384+
// Verify the assumption that all spans are disjoint
385+
assert_eq!(
386+
substitution.parts.array_windows().find(|[a, b]| a.span.overlaps(b.span)),
387+
None,
388+
"all spans must be disjoint",
389+
);
390+
391+
// Account for cases where we are suggesting the same code that's already
392+
// there. This shouldn't happen often, but in some cases for multipart
393+
// suggestions it's much easier to handle it here than in the origin.
394+
substitution.parts.retain(|p| is_different(sm, &p.snippet, p.span));
384395

385396
// Find the bounding span.
386397
let lo = substitution.parts.iter().map(|part| part.span.lo()).min()?;
@@ -470,16 +481,12 @@ impl CodeSuggestion {
470481
_ => 1,
471482
})
472483
.sum();
473-
if !is_different(sm, &part.snippet, part.span) {
474-
// Account for cases where we are suggesting the same code that's already
475-
// there. This shouldn't happen often, but in some cases for multipart
476-
// suggestions it's much easier to handle it here than in the origin.
477-
} else {
478-
line_highlight.push(SubstitutionHighlight {
479-
start: (cur_lo.col.0 as isize + acc) as usize,
480-
end: (cur_lo.col.0 as isize + acc + len) as usize,
481-
});
482-
}
484+
485+
line_highlight.push(SubstitutionHighlight {
486+
start: (cur_lo.col.0 as isize + acc) as usize,
487+
end: (cur_lo.col.0 as isize + acc + len) as usize,
488+
});
489+
483490
buf.push_str(&part.snippet);
484491
let cur_hi = sm.lookup_char_pos(part.span.hi());
485492
// Account for the difference between the width of the current code and the

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,8 @@ LL | assert_eq!(a!(), true);
272272
|
273273
help: replace it with `assert!(..)`
274274
|
275-
LL | true
276-
...
277-
LL |
278-
LL ~ assert!(a!());
275+
LL - assert_eq!(a!(), true);
276+
LL + assert!(a!());
279277
|
280278

281279
error: used `assert_eq!` with a literal bool
@@ -286,10 +284,8 @@ LL | assert_eq!(true, b!());
286284
|
287285
help: replace it with `assert!(..)`
288286
|
289-
LL | true
290-
...
291-
LL |
292-
LL ~ assert!(b!());
287+
LL - assert_eq!(true, b!());
288+
LL + assert!(b!());
293289
|
294290

295291
error: used `debug_assert_eq!` with a literal bool

0 commit comments

Comments
 (0)