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.

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

Tweak output for 'add line' suggestion #109786

Merged
merged 2 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 49 additions & 1 deletion compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1832,6 +1832,12 @@ impl EmitterWriter {
}
let show_code_change = if has_deletion && !is_multiline {
DisplaySuggestion::Diff
} else if let [part] = &parts[..]
&& part.snippet.ends_with('\n')
&& part.snippet.trim() == complete.trim()
{
// We are adding a line(s) of code before code that was already there.
DisplaySuggestion::Add
} else if (parts.len() != 1 || parts[0].snippet.trim() != complete.trim())
&& !is_multiline
{
Expand Down Expand Up @@ -1879,14 +1885,23 @@ impl EmitterWriter {
row_num += line_end - line_start;
}
let mut unhighlighted_lines = Vec::new();
let mut last_pos = 0;
let mut is_item_attribute = false;
for (line_pos, (line, highlight_parts)) in lines.by_ref().zip(highlights).enumerate() {
last_pos = line_pos;
debug!(%line_pos, %line, ?highlight_parts);

// Remember lines that are not highlighted to hide them if needed
if highlight_parts.is_empty() {
unhighlighted_lines.push((line_pos, line));
continue;
}
if highlight_parts.len() == 1
&& line.trim().starts_with("#[")
&& line.trim().ends_with(']')
{
is_item_attribute = true;
}

match unhighlighted_lines.len() {
0 => (),
Expand Down Expand Up @@ -1963,13 +1978,41 @@ impl EmitterWriter {
is_multiline,
)
}
if let DisplaySuggestion::Add = show_code_change && is_item_attribute {
// The suggestion adds an entire line of code, ending on a newline, so we'll also
// print the *following* line, to provide context of what we're advicing people to
// do. Otherwise you would only see contextless code that can be confused for
// already existing code, despite the colors and UI elements.
// We special case `#[derive(_)]\n` and other attribute suggestions, because those
// are the ones where context is most useful.
let file_lines = sm
.span_to_lines(span.primary_span().unwrap().shrink_to_hi())
.expect("span_to_lines failed when emitting suggestion");
let line_num = sm.lookup_char_pos(parts[0].span.lo()).line;
if let Some(line) = file_lines.file.get_line(line_num - 1) {
let line = normalize_whitespace(&line);
self.draw_code_line(
&mut buffer,
&mut row_num,
&[],
line_num + last_pos + 1,
&line,
DisplaySuggestion::None,
max_line_num_len,
&file_lines,
is_multiline,
)
}
}

// This offset and the ones below need to be signed to account for replacement code
// that is shorter than the original code.
let mut offsets: Vec<(usize, isize)> = Vec::new();
// Only show an underline in the suggestions if the suggestion is not the
// entirety of the code being shown and the displayed code is not multiline.
if let DisplaySuggestion::Diff | DisplaySuggestion::Underline = show_code_change {
if let DisplaySuggestion::Diff | DisplaySuggestion::Underline | DisplaySuggestion::Add =
show_code_change
{
draw_col_separator_no_space(&mut buffer, row_num, max_line_num_len + 1);
for part in parts {
let span_start_pos = sm.lookup_char_pos(part.span.lo()).col_display;
Expand Down Expand Up @@ -2247,6 +2290,10 @@ impl EmitterWriter {
}
}
buffer.append(*row_num, &normalize_whitespace(line_to_add), Style::NoStyle);
} else if let DisplaySuggestion::Add = show_code_change {
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 {
buffer.puts(*row_num, 0, &self.maybe_anonymized(line_num), Style::LineNumber);
draw_col_separator(buffer, *row_num, max_line_num_len + 1);
Expand Down Expand Up @@ -2281,6 +2328,7 @@ enum DisplaySuggestion {
Underline,
Diff,
None,
Add,
}

impl FileWithAnnotatedLines {
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/tests/ui/crashes/ice-6252.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ LL | _n: PhantomData,
|
help: consider importing one of these items
|
LL | use core::marker::PhantomData;
LL + use core::marker::PhantomData;
estebank marked this conversation as resolved.
Show resolved Hide resolved
|
LL | use serde::__private::PhantomData;
LL + use serde::__private::PhantomData;
|
LL | use std::marker::PhantomData;
LL + use std::marker::PhantomData;
|

error[E0412]: cannot find type `VAL` in this scope
Expand Down
24 changes: 16 additions & 8 deletions src/tools/clippy/tests/ui/derivable_impls.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ LL | | }
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
LL + #[derive(Default)]
LL | struct FooDefault<'a> {
|

error: this `impl` can be derived
Expand All @@ -30,7 +31,8 @@ LL | | }
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
LL + #[derive(Default)]
LL | struct TupleDefault(bool, i32, u64);
|

error: this `impl` can be derived
Expand All @@ -46,7 +48,8 @@ LL | | }
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
LL + #[derive(Default)]
LL | struct StrDefault<'a>(&'a str);
|

error: this `impl` can be derived
Expand All @@ -62,7 +65,8 @@ LL | | }
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
LL + #[derive(Default)]
LL | struct Y(u32);
|

error: this `impl` can be derived
Expand All @@ -78,7 +82,8 @@ LL | | }
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
LL + #[derive(Default)]
LL | struct WithoutSelfCurly {
|

error: this `impl` can be derived
Expand All @@ -94,7 +99,8 @@ LL | | }
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
LL + #[derive(Default)]
LL | struct WithoutSelfParan(bool);
|

error: this `impl` can be derived
Expand All @@ -110,7 +116,8 @@ LL | | }
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
LL + #[derive(Default)]
LL | pub struct RepeatDefault1 {
|

error: this `impl` can be derived
Expand All @@ -126,7 +133,8 @@ LL | | }
= help: remove the manual implementation...
help: ...and instead derive it...
|
LL | #[derive(Default)]
LL + #[derive(Default)]
LL | pub enum SimpleEnum {
|
help: ...and mark the default variant
|
Expand Down
6 changes: 4 additions & 2 deletions tests/ui/array-slice-vec/repeat_empty_ok.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ LL | let headers = [Header{value: &[]}; 128];
= note: the `Copy` trait is required because this value will be copied for each element of the array
help: consider annotating `Header<'_>` with `#[derive(Copy)]`
|
LL | #[derive(Copy)]
LL + #[derive(Copy)]
LL | pub struct Header<'a> {
|

error[E0277]: the trait bound `Header<'_>: Copy` is not satisfied
Expand All @@ -19,7 +20,8 @@ LL | let headers = [Header{value: &[0]}; 128];
= note: the `Copy` trait is required because this value will be copied for each element of the array
help: consider annotating `Header<'_>` with `#[derive(Copy)]`
|
LL | #[derive(Copy)]
LL + #[derive(Copy)]
LL | pub struct Header<'a> {
|

error: aborting due to 2 previous errors
Expand Down
6 changes: 4 additions & 2 deletions tests/ui/associated-types/defaults-suitability.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ LL | type Ty: Clone = NotClone;
| ^^^^^ required by this bound in `Tr::Ty`
help: consider annotating `NotClone` with `#[derive(Clone)]`
|
LL | #[derive(Clone)]
LL + #[derive(Clone)]
LL | struct NotClone;
|

error[E0277]: the trait bound `NotClone: Clone` is not satisfied
Expand All @@ -30,7 +31,8 @@ LL | type Ty = NotClone;
| -- required by a bound in this associated type
help: consider annotating `NotClone` with `#[derive(Clone)]`
|
LL | #[derive(Clone)]
LL + #[derive(Clone)]
LL | struct NotClone;
|

error[E0277]: the trait bound `T: Clone` is not satisfied
Expand Down
18 changes: 12 additions & 6 deletions tests/ui/binop/issue-28837.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ LL | struct A;
| ^^^^^^^^ must implement `PartialEq<_>`
help: consider annotating `A` with `#[derive(PartialEq)]`
|
LL | #[derive(PartialEq)]
LL + #[derive(PartialEq)]
LL | struct A;
|

error[E0369]: binary operation `!=` cannot be applied to type `A`
Expand All @@ -175,7 +176,8 @@ LL | struct A;
| ^^^^^^^^ must implement `PartialEq<_>`
help: consider annotating `A` with `#[derive(PartialEq)]`
|
LL | #[derive(PartialEq)]
LL + #[derive(PartialEq)]
LL | struct A;
|

error[E0369]: binary operation `<` cannot be applied to type `A`
Expand All @@ -193,7 +195,8 @@ LL | struct A;
| ^^^^^^^^ must implement `PartialOrd<_>`
help: consider annotating `A` with `#[derive(PartialEq, PartialOrd)]`
|
LL | #[derive(PartialEq, PartialOrd)]
LL + #[derive(PartialEq, PartialOrd)]
LL | struct A;
|

error[E0369]: binary operation `<=` cannot be applied to type `A`
Expand All @@ -211,7 +214,8 @@ LL | struct A;
| ^^^^^^^^ must implement `PartialOrd<_>`
help: consider annotating `A` with `#[derive(PartialEq, PartialOrd)]`
|
LL | #[derive(PartialEq, PartialOrd)]
LL + #[derive(PartialEq, PartialOrd)]
LL | struct A;
|

error[E0369]: binary operation `>` cannot be applied to type `A`
Expand All @@ -229,7 +233,8 @@ LL | struct A;
| ^^^^^^^^ must implement `PartialOrd<_>`
help: consider annotating `A` with `#[derive(PartialEq, PartialOrd)]`
|
LL | #[derive(PartialEq, PartialOrd)]
LL + #[derive(PartialEq, PartialOrd)]
LL | struct A;
|

error[E0369]: binary operation `>=` cannot be applied to type `A`
Expand All @@ -247,7 +252,8 @@ LL | struct A;
| ^^^^^^^^ must implement `PartialOrd<_>`
help: consider annotating `A` with `#[derive(PartialEq, PartialOrd)]`
|
LL | #[derive(PartialEq, PartialOrd)]
LL + #[derive(PartialEq, PartialOrd)]
LL | struct A;
|

error: aborting due to 15 previous errors
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/box/unit/unique-pinned-nocopy.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ LL | let _j = i.clone();
candidate #1: `Clone`
help: consider annotating `R` with `#[derive(Clone)]`
|
LL | #[derive(Clone)]
LL + #[derive(Clone)]
LL | struct R {
|

error: aborting due to previous error
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/coherence/coherence_inherent.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | s.the_fn();
= help: items from traits can only be used if the trait is in scope
help: the following trait is implemented but not in scope; perhaps add a `use` for it:
|
LL | use Lib::TheTrait;
LL + use Lib::TheTrait;
|

error: aborting due to previous error
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/coherence/coherence_inherent_cc.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ LL | s.the_fn();
= help: items from traits can only be used if the trait is in scope
help: the following trait is implemented but not in scope; perhaps add a `use` for it:
|
LL | use coherence_inherent_cc_lib::TheTrait;
LL + use coherence_inherent_cc_lib::TheTrait;
|

error: aborting due to previous error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ LL | If<{ FRAC <= 32 }>: True,
help: consider enabling this feature
--> $DIR/issue-94287.rs:1:1
|
LL | #![feature(generic_const_exprs)]
LL + #![feature(generic_const_exprs)]
|

error: aborting due to previous error
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/const-generics/issues/issue-82956.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ LL | let mut iter = IntoIter::new(self);
|
help: consider importing one of these items
|
LL | use std::array::IntoIter;
LL + use std::array::IntoIter;
|
LL | use std::collections::binary_heap::IntoIter;
LL + use std::collections::binary_heap::IntoIter;
|
LL | use std::collections::btree_map::IntoIter;
LL + use std::collections::btree_map::IntoIter;
|
LL | use std::collections::btree_set::IntoIter;
LL + use std::collections::btree_set::IntoIter;
|
and 8 other candidates

Expand Down
3 changes: 2 additions & 1 deletion tests/ui/consts/const-blocks/fn-call-in-non-const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ LL | let _: [Option<Bar>; 2] = [no_copy(); 2];
= help: create an inline `const` block, see RFC #2920 <https://github.com/rust-lang/rfcs/pull/2920> for more information
help: consider annotating `Bar` with `#[derive(Copy)]`
|
LL | #[derive(Copy)]
LL + #[derive(Copy)]
LL | struct Bar;
|

error: aborting due to previous error
Expand Down
6 changes: 4 additions & 2 deletions tests/ui/consts/const-blocks/migrate-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ LL | let arr: [Option<Bar>; 2] = [x; 2];
= note: the `Copy` trait is required because this value will be copied for each element of the array
help: consider annotating `Bar` with `#[derive(Copy)]`
|
LL | #[derive(Copy)]
LL + #[derive(Copy)]
LL | struct Bar;
|

error[E0277]: the trait bound `Bar: Copy` is not satisfied
Expand All @@ -21,7 +22,8 @@ LL | let arr: [Option<Bar>; 2] = [x; 2];
= note: the `Copy` trait is required because this value will be copied for each element of the array
help: consider annotating `Bar` with `#[derive(Copy)]`
|
LL | #[derive(Copy)]
LL + #[derive(Copy)]
LL | struct Bar;
|

error: aborting due to 2 previous errors
Expand Down
6 changes: 4 additions & 2 deletions tests/ui/consts/const-blocks/nll-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ LL | let arr: [Option<Bar>; 2] = [x; 2];
= note: the `Copy` trait is required because this value will be copied for each element of the array
help: consider annotating `Bar` with `#[derive(Copy)]`
|
LL | #[derive(Copy)]
LL + #[derive(Copy)]
LL | struct Bar;
|

error[E0277]: the trait bound `Bar: Copy` is not satisfied
Expand All @@ -21,7 +22,8 @@ LL | let arr: [Option<Bar>; 2] = [x; 2];
= note: the `Copy` trait is required because this value will be copied for each element of the array
help: consider annotating `Bar` with `#[derive(Copy)]`
|
LL | #[derive(Copy)]
LL + #[derive(Copy)]
LL | struct Bar;
|

error: aborting due to 2 previous errors
Expand Down
Loading