Skip to content

Hidden suggestion support #58296

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

Merged
merged 5 commits into from
Feb 14, 2019
Merged
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
2 changes: 1 addition & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
@@ -557,7 +557,7 @@ impl BuiltinLintDiagnostics {
}
BuiltinLintDiagnostics::UnusedImports(message, replaces) => {
if !replaces.is_empty() {
db.multipart_suggestion(
db.tool_only_multipart_suggestion(
&message,
replaces,
Applicability::MachineApplicable,
79 changes: 75 additions & 4 deletions src/librustc_errors/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::CodeSuggestion;
use crate::SuggestionStyle;
use crate::SubstitutionPart;
use crate::Substitution;
use crate::Applicability;
@@ -243,7 +244,33 @@ impl Diagnostic {
.collect(),
}],
msg: msg.to_owned(),
show_code_when_inline: true,
style: SuggestionStyle::ShowCode,
applicability,
});
self
}

/// Prints out a message with for a multipart suggestion without showing the suggested code.
///
/// This is intended to be used for suggestions that are obvious in what the changes need to
/// be from the message, showing the span label inline would be visually unpleasant
/// (marginally overlapping spans or multiline spans) and showing the snippet window wouldn't
/// improve understandability.
pub fn tool_only_multipart_suggestion(
&mut self,
msg: &str,
suggestion: Vec<(Span, String)>,
applicability: Applicability,
) -> &mut Self {
self.suggestions.push(CodeSuggestion {
substitutions: vec![Substitution {
parts: suggestion
.into_iter()
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect(),
}],
msg: msg.to_owned(),
style: SuggestionStyle::CompletelyHidden,
applicability,
});
self
@@ -277,7 +304,7 @@ impl Diagnostic {
}],
}],
msg: msg.to_owned(),
show_code_when_inline: true,
style: SuggestionStyle::ShowCode,
applicability,
});
self
@@ -295,7 +322,7 @@ impl Diagnostic {
}],
}).collect(),
msg: msg.to_owned(),
show_code_when_inline: true,
style: SuggestionStyle::ShowCode,
applicability,
});
self
@@ -316,7 +343,51 @@ impl Diagnostic {
}],
}],
msg: msg.to_owned(),
show_code_when_inline: false,
style: SuggestionStyle::HideCodeInline,
applicability,
});
self
}

/// Prints out a message with for a suggestion without showing the suggested code.
///
/// This is intended to be used for suggestions that are obvious in what the changes need to
/// be from the message, showing the span label inline would be visually unpleasant
/// (marginally overlapping spans or multiline spans) and showing the snippet window wouldn't
/// improve understandability.
pub fn span_suggestion_hidden(
&mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability
) -> &mut Self {
self.suggestions.push(CodeSuggestion {
substitutions: vec![Substitution {
parts: vec![SubstitutionPart {
snippet: suggestion,
span: sp,
}],
}],
msg: msg.to_owned(),
style: SuggestionStyle::HideCodeInline,
applicability,
});
self
}

/// Adds a suggestion to the json output, but otherwise remains silent/undisplayed in the cli.
///
/// This is intended to be used for suggestions that are *very* obvious in what the changes
/// need to be from the message, but we still want other tools to be able to apply them.
pub fn tool_only_span_suggestion(
&mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability
) -> &mut Self {
self.suggestions.push(CodeSuggestion {
substitutions: vec![Substitution {
parts: vec![SubstitutionPart {
snippet: suggestion,
span: sp,
}],
}],
msg: msg.to_owned(),
style: SuggestionStyle::CompletelyHidden,
applicability: applicability,
});
self
57 changes: 57 additions & 0 deletions src/librustc_errors/diagnostic_builder.rs
Original file line number Diff line number Diff line change
@@ -205,6 +205,24 @@ impl<'a> DiagnosticBuilder<'a> {
self
}

pub fn tool_only_multipart_suggestion(
&mut self,
msg: &str,
suggestion: Vec<(Span, String)>,
applicability: Applicability,
) -> &mut Self {
if !self.allow_suggestions {
return self
}
self.diagnostic.tool_only_multipart_suggestion(
msg,
suggestion,
applicability,
);
self
}


pub fn span_suggestion(
&mut self,
sp: Span,
@@ -261,6 +279,45 @@ impl<'a> DiagnosticBuilder<'a> {
);
self
}

pub fn span_suggestion_hidden(
&mut self,
sp: Span,
msg: &str,
suggestion: String,
applicability: Applicability,
) -> &mut Self {
if !self.allow_suggestions {
return self
}
self.diagnostic.span_suggestion_hidden(
sp,
msg,
suggestion,
applicability,
);
self
}

pub fn tool_only_span_suggestion(
&mut self,
sp: Span,
msg: &str,
suggestion: String,
applicability: Applicability,
) -> &mut Self {
if !self.allow_suggestions {
return self
}
self.diagnostic.tool_only_span_suggestion(
sp,
msg,
suggestion,
applicability,
);
self
}

forward!(pub fn set_span<S: Into<MultiSpan>>(&mut self, sp: S) -> &mut Self);
forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self);

96 changes: 64 additions & 32 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,10 @@ use Destination::*;

use syntax_pos::{SourceFile, Span, MultiSpan};

use crate::{Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, SourceMapperDyn, DiagnosticId};
use crate::{
Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic,
SuggestionStyle, SourceMapperDyn, DiagnosticId,
};
use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style};
use crate::styled_buffer::StyledBuffer;

@@ -43,9 +46,14 @@ impl Emitter for EmitterWriter {
// don't display long messages as labels
sugg.msg.split_whitespace().count() < 10 &&
// don't display multiline suggestions as labels
!sugg.substitutions[0].parts[0].snippet.contains('\n') {
!sugg.substitutions[0].parts[0].snippet.contains('\n') &&
// when this style is set we want the suggestion to be a message, not inline
sugg.style != SuggestionStyle::HideCodeAlways &&
// trivial suggestion for tooling's sake, never shown
sugg.style != SuggestionStyle::CompletelyHidden
{
let substitution = &sugg.substitutions[0].parts[0].snippet.trim();
let msg = if substitution.len() == 0 || !sugg.show_code_when_inline {
let msg = if substitution.len() == 0 || sugg.style.hide_inline() {
// This substitution is only removal or we explicitly don't want to show the
// code inline, don't show it
format!("help: {}", sugg.msg)
@@ -942,14 +950,15 @@ impl EmitterWriter {
}
}

fn emit_message_default(&mut self,
msp: &MultiSpan,
msg: &[(String, Style)],
code: &Option<DiagnosticId>,
level: &Level,
max_line_num_len: usize,
is_secondary: bool)
-> io::Result<()> {
fn emit_message_default(
&mut self,
msp: &MultiSpan,
msg: &[(String, Style)],
code: &Option<DiagnosticId>,
level: &Level,
max_line_num_len: usize,
is_secondary: bool,
) -> io::Result<()> {
let mut buffer = StyledBuffer::new();
let header_style = if is_secondary {
Style::HeaderMsg
@@ -1184,11 +1193,12 @@ impl EmitterWriter {

}

fn emit_suggestion_default(&mut self,
suggestion: &CodeSuggestion,
level: &Level,
max_line_num_len: usize)
-> io::Result<()> {
fn emit_suggestion_default(
&mut self,
suggestion: &CodeSuggestion,
level: &Level,
max_line_num_len: usize,
) -> io::Result<()> {
if let Some(ref sm) = self.sm {
let mut buffer = StyledBuffer::new();

@@ -1198,11 +1208,13 @@ impl EmitterWriter {
buffer.append(0, &level_str, Style::Level(level.clone()));
buffer.append(0, ": ", Style::HeaderMsg);
}
self.msg_to_buffer(&mut buffer,
&[(suggestion.msg.to_owned(), Style::NoStyle)],
max_line_num_len,
"suggestion",
Some(Style::HeaderMsg));
self.msg_to_buffer(
&mut buffer,
&[(suggestion.msg.to_owned(), Style::NoStyle)],
max_line_num_len,
"suggestion",
Some(Style::HeaderMsg),
);

// Render the replacements for each suggestion
let suggestions = suggestion.splice_lines(&**sm);
@@ -1340,22 +1352,42 @@ impl EmitterWriter {
if !self.short_message {
for child in children {
let span = child.render_span.as_ref().unwrap_or(&child.span);
match self.emit_message_default(&span,
&child.styled_message(),
&None,
&child.level,
max_line_num_len,
true) {
match self.emit_message_default(
&span,
&child.styled_message(),
&None,
&child.level,
max_line_num_len,
true,
) {
Err(e) => panic!("failed to emit error: {}", e),
_ => ()
}
}
for sugg in suggestions {
match self.emit_suggestion_default(sugg,
&Level::Help,
max_line_num_len) {
Err(e) => panic!("failed to emit error: {}", e),
_ => ()
if sugg.style == SuggestionStyle::CompletelyHidden {
// do not display this suggestion, it is meant only for tools
} else if sugg.style == SuggestionStyle::HideCodeAlways {
match self.emit_message_default(
&MultiSpan::new(),
&[(sugg.msg.to_owned(), Style::HeaderMsg)],
&None,
&Level::Help,
max_line_num_len,
true,
) {
Err(e) => panic!("failed to emit error: {}", e),
_ => ()
}
} else {
match self.emit_suggestion_default(
sugg,
&Level::Help,
max_line_num_len,
) {
Err(e) => panic!("failed to emit error: {}", e),
_ => ()
}
}
}
}
26 changes: 25 additions & 1 deletion src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
@@ -68,6 +68,29 @@ pub enum Applicability {
Unspecified,
}

#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, RustcEncodable, RustcDecodable)]
pub enum SuggestionStyle {
/// Hide the suggested code when displaying this suggestion inline.
HideCodeInline,
/// Always hide the suggested code but display the message.
HideCodeAlways,
/// Do not display this suggestion in the cli output, it is only meant for tools.
CompletelyHidden,
/// Always show the suggested code.
/// This will *not* show the code if the suggestion is inline *and* the suggested code is
/// empty.
ShowCode,
}

impl SuggestionStyle {
fn hide_inline(&self) -> bool {
match *self {
SuggestionStyle::ShowCode => false,
_ => true,
}
}
}

#[derive(Clone, Debug, PartialEq, Hash, RustcEncodable, RustcDecodable)]
pub struct CodeSuggestion {
/// Each substitute can have multiple variants due to multiple
@@ -93,7 +116,8 @@ pub struct CodeSuggestion {
/// ```
pub substitutions: Vec<Substitution>,
pub msg: String,
pub show_code_when_inline: bool,
/// Visual representation of this suggestion.
pub style: SuggestionStyle,
/// Whether or not the suggestion is approximate
///
/// Sometimes we may show suggestions with placeholders,
2 changes: 1 addition & 1 deletion src/test/ui/bad/bad-lint-cap2.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ error: unused import: `std::option`
--> $DIR/bad-lint-cap2.rs:6:5
|
LL | use std::option; //~ ERROR
| ----^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/bad-lint-cap2.rs:4:9
2 changes: 1 addition & 1 deletion src/test/ui/bad/bad-lint-cap3.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ warning: unused import: `std::option`
--> $DIR/bad-lint-cap3.rs:7:5
|
LL | use std::option; //~ WARN
| ----^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/bad-lint-cap3.rs:4:9
2 changes: 1 addition & 1 deletion src/test/ui/imports/unused.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ error: unused import: `super::f`
--> $DIR/unused.rs:7:24
|
LL | pub(super) use super::f; //~ ERROR unused
| ---------------^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^
|
note: lint level defined here
--> $DIR/unused.rs:1:9
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-30730.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ error: unused import: `std::thread`
--> $DIR/issue-30730.rs:3:5
|
LL | use std::thread;
| ----^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/issue-30730.rs:2:9
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ error: unused import: `a::x`
--> $DIR/lint-directives-on-use-items-issue-10534.rs:12:9
|
LL | use a::x; //~ ERROR: unused import
| ----^^^^- help: remove the whole `use` item
| ^^^^
|
note: lint level defined here
--> $DIR/lint-directives-on-use-items-issue-10534.rs:1:9
@@ -14,7 +14,7 @@ error: unused import: `a::y`
--> $DIR/lint-directives-on-use-items-issue-10534.rs:21:9
|
LL | use a::y; //~ ERROR: unused import
| ----^^^^- help: remove the whole `use` item
| ^^^^
|
note: lint level defined here
--> $DIR/lint-directives-on-use-items-issue-10534.rs:20:12
16 changes: 7 additions & 9 deletions src/test/ui/lint/lint-unused-imports.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ error: unused import: `std::fmt::{}`
--> $DIR/lint-unused-imports.rs:8:5
|
LL | use std::fmt::{};
| ----^^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/lint-unused-imports.rs:1:9
@@ -14,39 +14,37 @@ error: unused imports: `None`, `Some`
--> $DIR/lint-unused-imports.rs:12:27
|
LL | use std::option::Option::{Some, None};
| --------------------------^^^^--^^^^-- help: remove the whole `use` item
| ^^^^ ^^^^

error: unused import: `test::A`
--> $DIR/lint-unused-imports.rs:15:5
|
LL | use test::A; //~ ERROR unused import: `test::A`
| ----^^^^^^^- help: remove the whole `use` item
| ^^^^^^^

error: unused import: `bar`
--> $DIR/lint-unused-imports.rs:24:18
|
LL | use test2::{foo, bar}; //~ ERROR unused import: `bar`
| --^^^
| |
| help: remove the unused import
| ^^^

error: unused import: `foo::Square`
--> $DIR/lint-unused-imports.rs:52:13
|
LL | use foo::Square; //~ ERROR unused import: `foo::Square`
| ----^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^

error: unused import: `self::g`
--> $DIR/lint-unused-imports.rs:68:9
|
LL | use self::g; //~ ERROR unused import: `self::g`
| ----^^^^^^^- help: remove the whole `use` item
| ^^^^^^^

error: unused import: `test2::foo`
--> $DIR/lint-unused-imports.rs:77:9
|
LL | use test2::foo; //~ ERROR unused import: `test2::foo`
| ----^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^

error: unused import: `test::B2`
--> $DIR/lint-unused-imports.rs:20:5
6 changes: 3 additions & 3 deletions src/test/ui/lint/lints-in-foreign-macros.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ warning: unused import: `std::string::ToString`
--> $DIR/lints-in-foreign-macros.rs:11:16
|
LL | () => {use std::string::ToString;} //~ WARN: unused import
| ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^^^^^^^^^^^
...
LL | mod a { foo!(); }
| ------- in this macro invocation
@@ -17,13 +17,13 @@ warning: unused import: `std::string::ToString`
--> $DIR/lints-in-foreign-macros.rs:16:18
|
LL | mod c { baz!(use std::string::ToString;); } //~ WARN: unused import
| ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^^^^^^^^^^^

warning: unused import: `std::string::ToString`
--> $DIR/lints-in-foreign-macros.rs:17:19
|
LL | mod d { baz2!(use std::string::ToString;); } //~ WARN: unused import
| ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^^^^^^^^^^^

warning: missing documentation for crate
--> $DIR/lints-in-foreign-macros.rs:4:1
4 changes: 2 additions & 2 deletions src/test/ui/rfc-2166-underscore-imports/basic.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ warning: unused import: `m::Tr1 as _`
--> $DIR/basic.rs:26:9
|
LL | use m::Tr1 as _; //~ WARN unused import
| ----^^^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/basic.rs:4:9
@@ -14,5 +14,5 @@ warning: unused import: `S as _`
--> $DIR/basic.rs:27:9
|
LL | use S as _; //~ WARN unused import
| ----^^^^^^- help: remove the whole `use` item
| ^^^^^^

4 changes: 2 additions & 2 deletions src/test/ui/rfc-2166-underscore-imports/unused-2018.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ error: unused import: `core::any`
--> $DIR/unused-2018.rs:6:9
|
LL | use core::any; //~ ERROR unused import: `core::any`
| ----^^^^^^^^^- help: remove the whole `use` item
| ^^^^^^^^^
|
note: lint level defined here
--> $DIR/unused-2018.rs:3:9
@@ -14,7 +14,7 @@ error: unused import: `core`
--> $DIR/unused-2018.rs:10:9
|
LL | use core; //~ ERROR unused import: `core`
| ----^^^^- help: remove the whole `use` item
| ^^^^

error: aborting due to 2 previous errors

4 changes: 0 additions & 4 deletions src/test/ui/span/multispan-import-lint.stderr
Original file line number Diff line number Diff line change
@@ -10,8 +10,4 @@ note: lint level defined here
LL | #![warn(unused)]
| ^^^^^^
= note: #[warn(unused_imports)] implied by #[warn(unused)]
help: remove the unused imports
|
LL | use std::cmp::{min};
| -- --

8 changes: 3 additions & 5 deletions src/test/ui/use/use-nested-groups-unused-imports.stderr
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ error: unused imports: `*`, `Foo`, `baz::{}`, `foobar::*`
--> $DIR/use-nested-groups-unused-imports.rs:16:11
|
LL | use foo::{Foo, bar::{baz::{}, foobar::*}, *};
| ----------^^^--------^^^^^^^--^^^^^^^^^---^-- help: remove the whole `use` item
| ^^^ ^^^^^^^ ^^^^^^^^^ ^
|
note: lint level defined here
--> $DIR/use-nested-groups-unused-imports.rs:3:9
@@ -14,15 +14,13 @@ error: unused import: `*`
--> $DIR/use-nested-groups-unused-imports.rs:18:24
|
LL | use foo::bar::baz::{*, *};
| --^
| |
| help: remove the unused import
| ^

error: unused import: `foo::{}`
--> $DIR/use-nested-groups-unused-imports.rs:20:5
|
LL | use foo::{};
| ----^^^^^^^- help: remove the whole `use` item
| ^^^^^^^

error: aborting due to 3 previous errors