From 644ce5e535f74be304a77dfadb9ff46c743554c7 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Thu, 11 May 2017 15:26:22 +0200 Subject: [PATCH] Address PR reviews --- src/librustc_errors/diagnostic.rs | 11 +++++-- src/librustc_errors/emitter.rs | 50 ++++++++++++++----------------- src/librustc_errors/lib.rs | 28 +++++++++++++---- src/libsyntax/json.rs | 10 +++---- 4 files changed, 59 insertions(+), 40 deletions(-) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index e129e313626..861880aa265 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -9,6 +9,7 @@ // except according to those terms. use CodeSuggestion; +use Substitution; use Level; use RenderSpan; use std::fmt; @@ -205,7 +206,10 @@ pub fn span_help>(&mut self, /// See `diagnostic::CodeSuggestion` for more information. pub fn span_suggestion(&mut self, sp: Span, msg: &str, suggestion: String) -> &mut Self { self.suggestions.push(CodeSuggestion { - substitutes: vec![(sp, vec![suggestion])], + substitution_parts: vec![Substitution { + span: sp, + substitutions: vec![suggestion], + }], msg: msg.to_owned(), }); self @@ -213,7 +217,10 @@ pub fn span_suggestion(&mut self, sp: Span, msg: &str, suggestion: String) -> &m pub fn span_suggestions(&mut self, sp: Span, msg: &str, suggestions: Vec) -> &mut Self { self.suggestions.push(CodeSuggestion { - substitutes: vec![(sp, suggestions)], + substitution_parts: vec![Substitution { + span: sp, + substitutions: suggestions, + }], msg: msg.to_owned(), }); self diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index cd72941146c..d1ec1be47b8 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -35,38 +35,32 @@ fn emit(&mut self, db: &DiagnosticBuilder) { let mut primary_span = db.span.clone(); let mut children = db.children.clone(); - if db.suggestions.len() == 1 { - let sugg = &db.suggestions[0]; - // don't display multispans as labels - if sugg.substitutes.len() == 1 && + if let Some((sugg, rest)) = db.suggestions.split_first() { + if rest.is_empty() && + // don't display multipart suggestions as labels + sugg.substitution_parts.len() == 1 && // don't display multi-suggestions as labels - sugg.substitutes[0].1.len() == 1 && + sugg.substitutions() == 1 && // don't display long messages as labels sugg.msg.split_whitespace().count() < 10 && // don't display multiline suggestions as labels - sugg.substitutes[0].1[0].find('\n').is_none() { - let msg = format!("help: {} `{}`", sugg.msg, sugg.substitutes[0].1[0]); - primary_span.push_span_label(sugg.substitutes[0].0, msg); + sugg.substitution_parts[0].substitutions[0].find('\n').is_none() { + let substitution = &sugg.substitution_parts[0].substitutions[0]; + let msg = format!("help: {} `{}`", sugg.msg, substitution); + primary_span.push_span_label(sugg.substitution_spans().next().unwrap(), msg); } else { - children.push(SubDiagnostic { - level: Level::Help, - message: Vec::new(), - span: MultiSpan::new(), - render_span: Some(Suggestion(sugg.clone())), - }); - } - } else { - // if there are multiple suggestions, print them all in full - // to be consistent. We could try to figure out if we can - // make one (or the first one) inline, but that would give - // undue importance to a semi-random suggestion - for sugg in &db.suggestions { - children.push(SubDiagnostic { - level: Level::Help, - message: Vec::new(), - span: MultiSpan::new(), - render_span: Some(Suggestion(sugg.clone())), - }); + // if there are multiple suggestions, print them all in full + // to be consistent. We could try to figure out if we can + // make one (or the first one) inline, but that would give + // undue importance to a semi-random suggestion + for sugg in &db.suggestions { + children.push(SubDiagnostic { + level: Level::Help, + message: Vec::new(), + span: MultiSpan::new(), + render_span: Some(Suggestion(sugg.clone())), + }); + } } } @@ -1073,7 +1067,7 @@ fn emit_suggestion_default(&mut self, -> io::Result<()> { use std::borrow::Borrow; - let primary_span = suggestion.substitutes[0].0; + let primary_span = suggestion.substitution_spans().next().unwrap(); if let Some(ref cm) = self.cm { let mut buffer = StyledBuffer::new(); diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 82d688d6ba6..e1ec23479ab 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -23,6 +23,7 @@ #![feature(staged_api)] #![feature(range_contains)] #![feature(libc)] +#![feature(conservative_impl_trait)] extern crate term; extern crate libc; @@ -83,10 +84,17 @@ pub struct CodeSuggestion { /// ``` /// vec![(0..7, vec!["a.b", "x.y"])] /// ``` - pub substitutes: Vec<(Span, Vec)>, + pub substitution_parts: Vec, pub msg: String, } +#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)] +/// See the docs on `CodeSuggestion::substitutions` +pub struct Substitution { + pub span: Span, + pub substitutions: Vec, +} + pub trait CodeMapper { fn lookup_char_pos(&self, pos: BytePos) -> Loc; fn span_to_lines(&self, sp: Span) -> FileLinesResult; @@ -96,6 +104,16 @@ pub trait CodeMapper { } impl CodeSuggestion { + /// Returns the number of substitutions + fn substitutions(&self) -> usize { + self.substitution_parts[0].substitutions.len() + } + + /// Returns the number of substitutions + pub fn substitution_spans<'a>(&'a self) -> impl Iterator + 'a { + self.substitution_parts.iter().map(|sub| sub.span) + } + /// Returns the assembled code suggestions. pub fn splice_lines(&self, cm: &CodeMapper) -> Vec { use syntax_pos::{CharPos, Loc, Pos}; @@ -119,13 +137,13 @@ fn push_trailing(buf: &mut String, } } - if self.substitutes.is_empty() { + if self.substitution_parts.is_empty() { return vec![String::new()]; } - let mut primary_spans: Vec<_> = self.substitutes + let mut primary_spans: Vec<_> = self.substitution_parts .iter() - .map(|&(sp, ref sub)| (sp, sub)) + .map(|sub| (sub.span, &sub.substitutions)) .collect(); // Assumption: all spans are in the same file, and all spans @@ -157,7 +175,7 @@ fn push_trailing(buf: &mut String, prev_hi.col = CharPos::from_usize(0); let mut prev_line = fm.get_line(lines.lines[0].line_index); - let mut bufs = vec![String::new(); self.substitutes[0].1.len()]; + let mut bufs = vec![String::new(); self.substitutions()]; for (sp, substitutes) in primary_spans { let cur_lo = cm.lookup_char_pos(sp.lo); diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs index 3d0b0b228a8..06335584c96 100644 --- a/src/libsyntax/json.rs +++ b/src/libsyntax/json.rs @@ -279,12 +279,12 @@ fn from_multispan(msp: &MultiSpan, je: &JsonEmitter) -> Vec { fn from_suggestion(suggestion: &CodeSuggestion, je: &JsonEmitter) -> Vec { - suggestion.substitutes + suggestion.substitution_parts .iter() - .flat_map(|&(span, ref suggestion)| { - suggestion.iter().map(move |suggestion| { + .flat_map(|substitution| { + substitution.substitutions.iter().map(move |suggestion| { let span_label = SpanLabel { - span, + span: substitution.span, is_primary: true, label: None, }; @@ -301,7 +301,7 @@ fn from_render_span(rsp: &RenderSpan, je: &JsonEmitter) -> Vec { RenderSpan::FullSpan(ref msp) => DiagnosticSpan::from_multispan(msp, je), // regular diagnostics don't produce this anymore - // will be removed in a later commit + // FIXME(oli_obk): remove it entirely RenderSpan::Suggestion(_) => unreachable!(), } } -- 2.44.0