Skip to content

Commit

Permalink
Rollup merge of rust-lang#58296 - estebank:hidden-suggestion, r=oli-obk
Browse files Browse the repository at this point in the history
Hidden suggestion support

Add way to hide suggestion snippet window from cli output to avoid cluttered spans that don't enhance understanding.

r? @pietroalbini CC @zackmdavis
  • Loading branch information
Centril committed Feb 14, 2019
2 parents 4ad3cf2 + 87dd2e1 commit 56e1916
Show file tree
Hide file tree
Showing 16 changed files with 245 additions and 69 deletions.
2 changes: 1 addition & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -277,7 +304,7 @@ impl Diagnostic {
}],
}],
msg: msg.to_owned(),
show_code_when_inline: true,
style: SuggestionStyle::ShowCode,
applicability,
});
self
Expand All @@ -295,7 +322,7 @@ impl Diagnostic {
}],
}).collect(),
msg: msg.to_owned(),
show_code_when_inline: true,
style: SuggestionStyle::ShowCode,
applicability,
});
self
Expand All @@ -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
Expand Down
57 changes: 57 additions & 0 deletions src/librustc_errors/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);

Expand Down
96 changes: 64 additions & 32 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand All @@ -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);
Expand Down Expand Up @@ -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),
_ => ()
}
}
}
}
Expand Down
26 changes: 25 additions & 1 deletion src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,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
Expand All @@ -94,7 +117,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,
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/bad/bad-lint-cap2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/bad/bad-lint-cap3.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 56e1916

Please sign in to comment.