Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hide irrelevant lines in suggestions to allow for suggestions that are far from each other to be shown #97798

Merged
merged 6 commits into from
Jun 17, 2022
Merged

Hide irrelevant lines in suggestions to allow for suggestions that are far from each other to be shown #97798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
260c5fd
Fix a typo
WaffleLapkin Jun 6, 2022
87fded1
Improve suggestions when its parts are far from each other
WaffleLapkin Jun 6, 2022
a607cff
--bless ui
WaffleLapkin Jun 6, 2022
cf6f821
Try to clean up code...
WaffleLapkin Jun 13, 2022
3c55672
Add back MAX_SUGGESTION_HIGHLIGHT_LINES so clippy is happy & buildable
WaffleLapkin Jun 13, 2022
e502b9a
bless clippy ui tests
WaffleLapkin Jun 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
245 changes: 169 additions & 76 deletions compiler/rustc_errors/src/emitter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use Destination::*;

use rustc_span::source_map::SourceMap;
use rustc_span::{SourceFile, Span};
use rustc_span::{FileLines, SourceFile, Span};

use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString};
use crate::styled_buffer::StyledBuffer;
Expand Down Expand Up @@ -1761,12 +1761,6 @@ impl EmitterWriter {
let has_deletion = parts.iter().any(|p| p.is_deletion());
let is_multiline = complete.lines().count() > 1;

enum DisplaySuggestion {
Underline,
Diff,
None,
}

if let Some(span) = span.primary_span() {
// Compare the primary span of the diagnostic with the span of the suggestion
// being emitted. If they belong to the same file, we don't *need* to show the
Expand Down Expand Up @@ -1839,79 +1833,94 @@ impl EmitterWriter {
}
row_num += line_end - line_start;
}
for (line_pos, (line, highlight_parts)) in
lines.by_ref().zip(highlights).take(MAX_SUGGESTION_HIGHLIGHT_LINES).enumerate()
{
// Print the span column to avoid confusion
buffer.puts(
row_num,
0,
&self.maybe_anonymized(line_start + line_pos),
Style::LineNumber,
);
if let DisplaySuggestion::Diff = show_code_change {
// Add the line number for both addition and removal to drive the point home.
//
// N - fn foo<A: T>(bar: A) {
// N + fn foo(bar: impl T) {
buffer.puts(
row_num - 1,
0,
&self.maybe_anonymized(line_start + line_pos),
Style::LineNumber,
);
buffer.puts(row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
buffer.puts(
row_num - 1,
max_line_num_len + 3,
&normalize_whitespace(
&*file_lines
.file
.get_line(file_lines.lines[line_pos].line_index)
.unwrap(),
),
Style::NoStyle,
);
buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition);
} else if is_multiline {
match &highlight_parts[..] {
[SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition);
}
[] => {
draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
}
_ => {
buffer.puts(row_num, max_line_num_len + 1, "~ ", Style::Addition);
}
}
} else {
draw_col_separator(&mut buffer, row_num, max_line_num_len + 1);
let mut unhighlighted_lines = Vec::new();
for (line_pos, (line, highlight_parts)) in lines.by_ref().zip(highlights).enumerate() {
debug!(%line_pos, %line, ?highlight_parts);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First time I see this, what does % do? 🤯

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It uses Display instead of Debug :D

https://docs.rs/tracing/latest/tracing/#recording-fields


// Remember lines that are not highlighted to hide them if needed
if highlight_parts.is_empty() {
unhighlighted_lines.push((line_pos, line));
continue;
}

// print the suggestion
buffer.append(row_num, &normalize_whitespace(line), Style::NoStyle);
match unhighlighted_lines.len() {
0 => (),
// Since we show first line, "..." line and last line,
// There is no reason to hide if there are 3 or less lines
// (because then we just replace a line with ... which is
// not helpful)
n if n <= 3 => unhighlighted_lines.drain(..).for_each(|(p, l)| {
self.draw_code_line(
&mut buffer,
&mut row_num,
&Vec::new(),
p,
l,
line_start,
show_code_change,
max_line_num_len,
&file_lines,
is_multiline,
)
}),
// Print first unhighlighted line, "..." and last unhighlighted line, like so:
//
// LL | this line was highlighted
// LL | this line is just for context
// ...
// LL | this line is just for context
// LL | this line was highlighted
_ => {
let last_line = unhighlighted_lines.pop();
let first_line = unhighlighted_lines.drain(..).next();

first_line.map(|(p, l)| {
self.draw_code_line(
&mut buffer,
&mut row_num,
&Vec::new(),
p,
l,
line_start,
show_code_change,
max_line_num_len,
&file_lines,
is_multiline,
)
});

// Colorize addition/replacements with green.
for &SubstitutionHighlight { start, end } in highlight_parts {
// Account for tabs when highlighting (#87972).
let tabs: usize = line
.chars()
.take(start)
.map(|ch| match ch {
'\t' => 3,
_ => 0,
})
.sum();
buffer.set_style_range(
row_num,
max_line_num_len + 3 + start + tabs,
max_line_num_len + 3 + end + tabs,
Style::Addition,
true,
);
buffer.puts(row_num, max_line_num_len - 1, "...", Style::LineNumber);
row_num += 1;

last_line.map(|(p, l)| {
self.draw_code_line(
&mut buffer,
&mut row_num,
&Vec::new(),
p,
l,
line_start,
show_code_change,
max_line_num_len,
&file_lines,
is_multiline,
)
});
}
}
row_num += 1;

self.draw_code_line(
&mut buffer,
&mut row_num,
highlight_parts,
line_pos,
line,
line_start,
show_code_change,
max_line_num_len,
&file_lines,
is_multiline,
)
}

// This offset and the ones below need to be signed to account for replacement code
Expand Down Expand Up @@ -2096,6 +2105,90 @@ impl EmitterWriter {
}
}
}

fn draw_code_line(
&self,
buffer: &mut StyledBuffer,
row_num: &mut usize,
highlight_parts: &Vec<SubstitutionHighlight>,
line_pos: usize,
line: &str,
line_start: usize,
show_code_change: DisplaySuggestion,
max_line_num_len: usize,
file_lines: &FileLines,
is_multiline: bool,
) {
// Print the span column to avoid confusion
buffer.puts(*row_num, 0, &self.maybe_anonymized(line_start + line_pos), Style::LineNumber);
if let DisplaySuggestion::Diff = show_code_change {
// Add the line number for both addition and removal to drive the point home.
//
// N - fn foo<A: T>(bar: A) {
// N + fn foo(bar: impl T) {
buffer.puts(
*row_num - 1,
0,
&self.maybe_anonymized(line_start + line_pos),
Style::LineNumber,
);
buffer.puts(*row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
buffer.puts(
*row_num - 1,
max_line_num_len + 3,
&normalize_whitespace(
&*file_lines.file.get_line(file_lines.lines[line_pos].line_index).unwrap(),
),
Style::NoStyle,
);
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
} else if is_multiline {
match &highlight_parts[..] {
[SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
}
[] => {
draw_col_separator(buffer, *row_num, max_line_num_len + 1);
}
_ => {
buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition);
}
}
} else {
draw_col_separator(buffer, *row_num, max_line_num_len + 1);
}

// print the suggestion
buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle);

// Colorize addition/replacements with green.
for &SubstitutionHighlight { start, end } in highlight_parts {
// Account for tabs when highlighting (#87972).
let tabs: usize = line
.chars()
.take(start)
.map(|ch| match ch {
'\t' => 3,
_ => 0,
})
.sum();
buffer.set_style_range(
*row_num,
max_line_num_len + 3 + start + tabs,
max_line_num_len + 3 + end + tabs,
Style::Addition,
true,
);
}
*row_num += 1;
}
}

#[derive(Clone, Copy)]
enum DisplaySuggestion {
Underline,
Diff,
None,
}

impl FileWithAnnotatedLines {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,7 @@ impl HandlerInner {
!this.emitted_diagnostics.insert(diagnostic_hash)
};

// Only emit the diagnostic if we've been asked to deduplicate and
// Only emit the diagnostic if we've been asked to deduplicate or
// haven't already emitted an equivalent diagnostic.
if !(self.flags.deduplicate_diagnostics && already_emitted(self)) {
debug!(?diagnostic);
Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-consts/issue-93835.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ help: you might have meant to write a `struct` literal
|
LL ~ fn e() { SomeStruct {
LL | p:a<p:p<e=6>>
LL |
LL |
LL |
LL |
...
LL |
LL ~ }}
|
help: maybe you meant to write a path separator here
|
LL | p::a<p:p<e=6>>
Expand Down
14 changes: 6 additions & 8 deletions src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ help: add a dummy let to cause `fptr` to be fully captured
|
LL ~ thread::spawn(move || { let _ = &fptr; unsafe {
LL |
LL |
LL |
LL |
LL | *fptr.0 = 20;
...
LL |
LL ~ } });
|

error: changes to closure capture in Rust 2021 will affect which traits the closure implements
--> $DIR/auto_traits.rs:42:19
Expand All @@ -39,12 +38,11 @@ LL | *fptr.0.0 = 20;
help: add a dummy let to cause `fptr` to be fully captured
|
LL ~ thread::spawn(move || { let _ = &fptr; unsafe {
LL |
LL |
LL |
LL |
LL |
...
LL |
LL ~ } });
|

error: changes to closure capture in Rust 2021 will affect drop order and which traits the closure implements
--> $DIR/auto_traits.rs:67:13
Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,11 @@ LL | *fptr2.0 = 20;
help: add a dummy let to cause `fptr1`, `fptr2` to be fully captured
|
LL ~ thread::spawn(move || { let _ = (&fptr1, &fptr2); unsafe {
LL |
LL |
LL |
LL |
LL |
...
LL |
LL ~ } });
|

error: aborting due to 5 previous errors

3 changes: 1 addition & 2 deletions src/test/ui/imports/issue-59764.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,7 @@ help: a macro with this name exists at the root of the crate
|
LL ~ issue_59764::{makro as foobar,
LL |
LL | foobaz,
LL |
...
LL |
LL ~ foo::{baz}
|
Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/issues/issue-22644.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,11 @@ LL | 5);
help: try comparing the cast value
|
LL ~ println!("{}", (a
LL |
LL |
LL | as
LL |
LL |
...
LL |
LL ~ usize)
|

error: `<` is interpreted as a start of generic arguments for `usize`, not a shift
--> $DIR/issue-22644.rs:32:31
Expand Down
3 changes: 1 addition & 2 deletions src/test/ui/let-else/let-else-if.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ help: try placing this code inside a block
|
LL ~ let Some(_) = Some(()) else { if true {
LL |
LL | return;
LL | } else {
...
LL | return;
LL ~ } };
|
Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/parser/recover-labeled-non-block-expr.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,10 @@ help: consider enclosing expression in a block
|
LL ~ let _i = 'label: { match x {
LL | 0 => 42,
LL | 1 if false => break 'label 17,
LL | 1 => {
LL | if true {
LL | break 'label 13
...
LL | _ => 1,
LL ~ } };
|

error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/recover-labeled-non-block-expr.rs:26:24
Expand Down
Loading