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)  After rust-lang/rust#145273 ("multi-line" style)  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
This commit is contained in:
commit
99317ef14d
4 changed files with 21 additions and 24 deletions
|
|
@ -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 }],
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue