Skip to content

Commit

Permalink
Auto merge of #107671 - CastilloDel:master, r=estebank
Browse files Browse the repository at this point in the history
Fix suggestions rendering when the diff span is multiline

Fixes #92741

cc `@estebank`

I think, I finally fixed. I still want to go back and try to clean up the code a bit. I'm open to suggestions.

Some examples of the new suggestions:

```
help: consider removing the borrow
  |
2 -     &
  |
```
```
help: consider removing the borrow
  |
2 -     &
3 -     mut
  |
```
```
help: consider removing the borrow
  |
2 -     &
3 -     mut if true { true } else { false }
2 +     if true { true } else { false }
  |
```

Should we add a test to ensure this behavior doesn't disappear in the future?
  • Loading branch information
bors committed Feb 7, 2023
2 parents e4dd9ed + f0830c0 commit 5dd0e1b
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 39 deletions.
95 changes: 57 additions & 38 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1882,9 +1882,8 @@ impl EmitterWriter {
&mut buffer,
&mut row_num,
&Vec::new(),
p,
p + line_start,
l,
line_start,
show_code_change,
max_line_num_len,
&file_lines,
Expand All @@ -1907,9 +1906,8 @@ impl EmitterWriter {
&mut buffer,
&mut row_num,
&Vec::new(),
p,
p + line_start,
l,
line_start,
show_code_change,
max_line_num_len,
&file_lines,
Expand All @@ -1925,9 +1923,8 @@ impl EmitterWriter {
&mut buffer,
&mut row_num,
&Vec::new(),
p,
p + line_start,
l,
line_start,
show_code_change,
max_line_num_len,
&file_lines,
Expand All @@ -1941,9 +1938,8 @@ impl EmitterWriter {
&mut buffer,
&mut row_num,
highlight_parts,
line_pos,
line_pos + line_start,
line,
line_start,
show_code_change,
max_line_num_len,
&file_lines,
Expand Down Expand Up @@ -2167,40 +2163,63 @@ impl EmitterWriter {
buffer: &mut StyledBuffer,
row_num: &mut usize,
highlight_parts: &Vec<SubstitutionHighlight>,
line_pos: usize,
line: &str,
line_start: usize,
line_num: usize,
line_to_add: &str,
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);
// We need to print more than one line if the span we need to remove is multiline.
// For more info: https://github.com/rust-lang/rust/issues/92741
let lines_to_remove = file_lines.lines.iter().take(file_lines.lines.len() - 1);
for (index, line_to_remove) in lines_to_remove.enumerate() {
buffer.puts(
*row_num - 1,
0,
&self.maybe_anonymized(line_num + index),
Style::LineNumber,
);
buffer.puts(*row_num - 1, max_line_num_len + 1, "- ", Style::Removal);
let line = normalize_whitespace(
&file_lines.file.get_line(line_to_remove.line_index).unwrap(),
);
buffer.puts(*row_num - 1, max_line_num_len + 3, &line, Style::NoStyle);
*row_num += 1;
}
// If the last line is exactly equal to the line we need to add, we can skip both of them.
// This allows us to avoid output like the following:
// 2 - &
// 2 + if true { true } else { false }
// 3 - if true { true } else { false }
// If those lines aren't equal, we print their diff
let last_line_index = file_lines.lines[file_lines.lines.len() - 1].line_index;
let last_line = &file_lines.file.get_line(last_line_index).unwrap();
if last_line != line_to_add {
buffer.puts(
*row_num - 1,
0,
&self.maybe_anonymized(line_num + file_lines.lines.len() - 1),
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(last_line),
Style::NoStyle,
);
buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
} else {
*row_num -= 2;
}
} else if is_multiline {
buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
match &highlight_parts[..] {
[SubstitutionHighlight { start: 0, end }] if *end == line.len() => {
[SubstitutionHighlight { start: 0, end }] if *end == line_to_add.len() => {
buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition);
}
[] => {
Expand All @@ -2210,17 +2229,17 @@ impl EmitterWriter {
buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition);
}
}
buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
} else {
buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
draw_col_separator(buffer, *row_num, max_line_num_len + 1);
buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
}

// 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
let tabs: usize = line_to_add
.chars()
.take(start)
.map(|ch| match ch {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::path::Path;
const ENTRY_LIMIT: usize = 1000;
// FIXME: The following limits should be reduced eventually.
const ROOT_ENTRY_LIMIT: usize = 939;
const ISSUES_ENTRY_LIMIT: usize = 1998;
const ISSUES_ENTRY_LIMIT: usize = 2001;

fn check_entries(path: &Path, bad: &mut bool) {
for dir in Walk::new(&path.join("ui")) {
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/issues/issue-92741.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// run-rustfix
fn main() {}
fn _foo() -> bool {
if true { true } else { false }
}

fn _bar() -> bool {
if true { true } else { false }
}

fn _baz() -> bool {
if true { true } else { false }
}
17 changes: 17 additions & 0 deletions tests/ui/issues/issue-92741.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-rustfix
fn main() {}
fn _foo() -> bool {
& //~ ERROR 4:5: 6:36: mismatched types [E0308]
mut
if true { true } else { false }
}

fn _bar() -> bool {
& //~ ERROR 10:5: 11:40: mismatched types [E0308]
mut if true { true } else { false }
}

fn _baz() -> bool {
& mut //~ ERROR 15:5: 16:36: mismatched types [E0308]
if true { true } else { false }
}
49 changes: 49 additions & 0 deletions tests/ui/issues/issue-92741.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
error[E0308]: mismatched types
--> $DIR/issue-92741.rs:4:5
|
LL | fn _foo() -> bool {
| ---- expected `bool` because of return type
LL | / &
LL | | mut
LL | | if true { true } else { false }
| |___________________________________^ expected `bool`, found `&mut bool`
|
help: consider removing the borrow
|
LL - &
LL - mut
|

error[E0308]: mismatched types
--> $DIR/issue-92741.rs:10:5
|
LL | fn _bar() -> bool {
| ---- expected `bool` because of return type
LL | / &
LL | | mut if true { true } else { false }
| |_______________________________________^ expected `bool`, found `&mut bool`
|
help: consider removing the borrow
|
LL - &
LL - mut if true { true } else { false }
LL + if true { true } else { false }
|

error[E0308]: mismatched types
--> $DIR/issue-92741.rs:15:5
|
LL | fn _baz() -> bool {
| ---- expected `bool` because of return type
LL | / & mut
LL | | if true { true } else { false }
| |___________________________________^ expected `bool`, found `&mut bool`
|
help: consider removing the borrow
|
LL - & mut
|

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.

0 comments on commit 5dd0e1b

Please sign in to comment.