From cf0279991d630d51f12a1d4b2933d05eecc4464f Mon Sep 17 00:00:00 2001 From: CastilloDel Date: Sat, 4 Feb 2023 18:52:27 +0100 Subject: [PATCH 1/5] Fix suggestions rendering when the span is multiline --- compiler/rustc_errors/src/emitter.rs | 80 ++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index faeaa54861970..61e1155a79f70 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -2175,30 +2175,69 @@ impl EmitterWriter { 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(bar: A) { // N + fn foo(bar: impl T) { + let number_of_lines = file_lines.lines.len(); + for (index, line_to_remove) in + file_lines.lines.iter().take(number_of_lines - 1).enumerate() + { + buffer.puts( + *row_num - 1, + 0, + &self.maybe_anonymized(line_start + line_pos + index), + 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(line_to_remove.line_index).unwrap(), + ), + Style::NoStyle, + ); + *row_num += 1; + } + let last_line = &file_lines + .file + .get_line(file_lines.lines[number_of_lines - 1].line_index) + .unwrap(); + if last_line != line { + buffer.puts( + *row_num - 1, + 0, + &self.maybe_anonymized(line_start + line_pos + number_of_lines - 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_start + line_pos), + Style::LineNumber, + ); + buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition); + // print the suggestion + buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle); + } else { + *row_num -= 2; + } + } else if is_multiline { buffer.puts( - *row_num - 1, + *row_num, 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); @@ -2210,13 +2249,20 @@ impl EmitterWriter { buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition); } } + // print the suggestion + buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle); } else { + buffer.puts( + *row_num, + 0, + &self.maybe_anonymized(line_start + line_pos), + Style::LineNumber, + ); draw_col_separator(buffer, *row_num, max_line_num_len + 1); + // print the suggestion + buffer.append(*row_num, &normalize_whitespace(line), 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). From 3dd004470d67333dd968fc0fa39c14037c0eeae4 Mon Sep 17 00:00:00 2001 From: CastilloDel Date: Sun, 5 Feb 2023 18:59:31 +0100 Subject: [PATCH 2/5] Clean up and comment EmitterWriter.draw_code_line --- compiler/rustc_errors/src/emitter.rs | 91 ++++++++++------------------ 1 file changed, 32 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 61e1155a79f70..9768526a2f47d 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -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, @@ -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, @@ -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, @@ -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, @@ -2167,49 +2163,44 @@ impl EmitterWriter { buffer: &mut StyledBuffer, row_num: &mut usize, highlight_parts: &Vec, - 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, ) { if let DisplaySuggestion::Diff = show_code_change { - // Add the line number for both addition and removal to drive the point home. - // - // N - fn foo(bar: A) { - // N + fn foo(bar: impl T) { - let number_of_lines = file_lines.lines.len(); - for (index, line_to_remove) in - file_lines.lines.iter().take(number_of_lines - 1).enumerate() - { + // 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_start + line_pos + index), + &self.maybe_anonymized(line_num + index), 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(line_to_remove.line_index).unwrap(), - ), - Style::NoStyle, + 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; } - let last_line = &file_lines - .file - .get_line(file_lines.lines[number_of_lines - 1].line_index) - .unwrap(); - if last_line != line { + // 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_start + line_pos + number_of_lines - 1), + &self.maybe_anonymized(line_num + file_lines.lines.len() - 1), Style::LineNumber, ); buffer.puts(*row_num - 1, max_line_num_len + 1, "- ", Style::Removal); @@ -2219,27 +2210,16 @@ impl EmitterWriter { &normalize_whitespace(last_line), Style::NoStyle, ); - buffer.puts( - *row_num, - 0, - &self.maybe_anonymized(line_start + line_pos), - Style::LineNumber, - ); + buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber); buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition); - // print the suggestion - buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle); + 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_start + line_pos), - Style::LineNumber, - ); + 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); } [] => { @@ -2249,24 +2229,17 @@ impl EmitterWriter { buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition); } } - // print the suggestion - buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle); + buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle); } else { - buffer.puts( - *row_num, - 0, - &self.maybe_anonymized(line_start + line_pos), - Style::LineNumber, - ); + buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber); draw_col_separator(buffer, *row_num, max_line_num_len + 1); - // print the suggestion - buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle); + buffer.append(*row_num, &normalize_whitespace(line_to_add), 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 { From 9cdc07538da4d3e39d3ae29c35546795b524ea29 Mon Sep 17 00:00:00 2001 From: CastilloDel Date: Sun, 5 Feb 2023 17:30:00 +0100 Subject: [PATCH 3/5] Add UI test for issue #92741 --- src/tools/tidy/src/ui_tests.rs | 2 +- tests/ui/issues/issue-92741.rs | 6 ++++++ tests/ui/issues/issue-92741.stderr | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 tests/ui/issues/issue-92741.rs create mode 100644 tests/ui/issues/issue-92741.stderr diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 806e84025c4a2..ba20c77a7d163 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -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 = 2000; fn check_entries(path: &Path, bad: &mut bool) { for dir in Walk::new(&path.join("ui")) { diff --git a/tests/ui/issues/issue-92741.rs b/tests/ui/issues/issue-92741.rs new file mode 100644 index 0000000000000..c42d4338fc105 --- /dev/null +++ b/tests/ui/issues/issue-92741.rs @@ -0,0 +1,6 @@ +fn main() {} +fn foo() -> bool { + & //~ ERROR 3:5: 5:36: mismatched types [E0308] + mut + if true { true } else { false } +} diff --git a/tests/ui/issues/issue-92741.stderr b/tests/ui/issues/issue-92741.stderr new file mode 100644 index 0000000000000..9459757d38fe7 --- /dev/null +++ b/tests/ui/issues/issue-92741.stderr @@ -0,0 +1,19 @@ +error[E0308]: mismatched types + --> $DIR/issue-92741.rs:3: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: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 039f70e926a859669fb046df4bd8115cad850113 Mon Sep 17 00:00:00 2001 From: CastilloDel Date: Mon, 6 Feb 2023 15:30:29 +0100 Subject: [PATCH 4/5] Add more test cases to tests/ui/issues/issue-92741.rs --- tests/ui/issues/issue-92741.rs | 10 ++++++++++ tests/ui/issues/issue-92741.stderr | 32 +++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/ui/issues/issue-92741.rs b/tests/ui/issues/issue-92741.rs index c42d4338fc105..4e18918c1cf09 100644 --- a/tests/ui/issues/issue-92741.rs +++ b/tests/ui/issues/issue-92741.rs @@ -4,3 +4,13 @@ fn foo() -> bool { mut if true { true } else { false } } + +fn bar() -> bool { + & //~ ERROR 9:5: 10:40: mismatched types [E0308] + mut if true { true } else { false } +} + +fn baz() -> bool { + & mut //~ ERROR 14:5: 15:36: mismatched types [E0308] + if true { true } else { false } +} diff --git a/tests/ui/issues/issue-92741.stderr b/tests/ui/issues/issue-92741.stderr index 9459757d38fe7..7716a10cde0cd 100644 --- a/tests/ui/issues/issue-92741.stderr +++ b/tests/ui/issues/issue-92741.stderr @@ -14,6 +14,36 @@ LL - & LL - mut | -error: aborting due to previous error +error[E0308]: mismatched types + --> $DIR/issue-92741.rs:9: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:14: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`. From f0830c0ade39c76142118fc6f8fe7375173af7c7 Mon Sep 17 00:00:00 2001 From: CastilloDel Date: Mon, 6 Feb 2023 15:33:54 +0100 Subject: [PATCH 5/5] Add `run-rustfix` to tests/ui/issues/issue-92741.rs --- src/tools/tidy/src/ui_tests.rs | 2 +- tests/ui/issues/issue-92741.fixed | 13 +++++++++++++ tests/ui/issues/issue-92741.rs | 13 +++++++------ tests/ui/issues/issue-92741.stderr | 18 +++++++++--------- 4 files changed, 30 insertions(+), 16 deletions(-) create mode 100644 tests/ui/issues/issue-92741.fixed diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index ba20c77a7d163..83551a1d820ab 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -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 = 2000; +const ISSUES_ENTRY_LIMIT: usize = 2001; fn check_entries(path: &Path, bad: &mut bool) { for dir in Walk::new(&path.join("ui")) { diff --git a/tests/ui/issues/issue-92741.fixed b/tests/ui/issues/issue-92741.fixed new file mode 100644 index 0000000000000..d07aeb6c029fe --- /dev/null +++ b/tests/ui/issues/issue-92741.fixed @@ -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 } +} diff --git a/tests/ui/issues/issue-92741.rs b/tests/ui/issues/issue-92741.rs index 4e18918c1cf09..413d5bf047853 100644 --- a/tests/ui/issues/issue-92741.rs +++ b/tests/ui/issues/issue-92741.rs @@ -1,16 +1,17 @@ +// run-rustfix fn main() {} -fn foo() -> bool { - & //~ ERROR 3:5: 5:36: mismatched types [E0308] +fn _foo() -> bool { + & //~ ERROR 4:5: 6:36: mismatched types [E0308] mut if true { true } else { false } } -fn bar() -> bool { - & //~ ERROR 9:5: 10:40: mismatched types [E0308] +fn _bar() -> bool { + & //~ ERROR 10:5: 11:40: mismatched types [E0308] mut if true { true } else { false } } -fn baz() -> bool { - & mut //~ ERROR 14:5: 15:36: mismatched types [E0308] +fn _baz() -> bool { + & mut //~ ERROR 15:5: 16:36: mismatched types [E0308] if true { true } else { false } } diff --git a/tests/ui/issues/issue-92741.stderr b/tests/ui/issues/issue-92741.stderr index 7716a10cde0cd..49315e7a8bf6b 100644 --- a/tests/ui/issues/issue-92741.stderr +++ b/tests/ui/issues/issue-92741.stderr @@ -1,8 +1,8 @@ error[E0308]: mismatched types - --> $DIR/issue-92741.rs:3:5 + --> $DIR/issue-92741.rs:4:5 | -LL | fn foo() -> bool { - | ---- expected `bool` because of return type +LL | fn _foo() -> bool { + | ---- expected `bool` because of return type LL | / & LL | | mut LL | | if true { true } else { false } @@ -15,10 +15,10 @@ LL - mut | error[E0308]: mismatched types - --> $DIR/issue-92741.rs:9:5 + --> $DIR/issue-92741.rs:10:5 | -LL | fn bar() -> bool { - | ---- expected `bool` because of return type +LL | fn _bar() -> bool { + | ---- expected `bool` because of return type LL | / & LL | | mut if true { true } else { false } | |_______________________________________^ expected `bool`, found `&mut bool` @@ -31,10 +31,10 @@ LL + if true { true } else { false } | error[E0308]: mismatched types - --> $DIR/issue-92741.rs:14:5 + --> $DIR/issue-92741.rs:15:5 | -LL | fn baz() -> bool { - | ---- expected `bool` because of return type +LL | fn _baz() -> bool { + | ---- expected `bool` because of return type LL | / & mut LL | | if true { true } else { false } | |___________________________________^ expected `bool`, found `&mut bool`