diff options
| author | Esteban Küber <esteban@kuber.com.ar> | 2021-06-21 19:07:19 -0700 |
|---|---|---|
| committer | Esteban Kuber <esteban@kuber.com.ar> | 2021-08-11 09:46:24 +0000 |
| commit | 99f2977031706dfef6730764d359b9e5d0f673b4 (patch) | |
| tree | 16caa7ac1411f6ebe1e56c175e9799a442b59832 /compiler/rustc_errors/src | |
| parent | d488de82f30fd1dcb0220d57498638596622394e (diff) | |
| download | rust-99f2977031706dfef6730764d359b9e5d0f673b4.tar.gz rust-99f2977031706dfef6730764d359b9e5d0f673b4.zip | |
Modify structured suggestion output
* On suggestions that include deletions, use a diff inspired output format * When suggesting addition, use `+` as underline * Color highlight modified span
Diffstat (limited to 'compiler/rustc_errors/src')
| -rw-r--r-- | compiler/rustc_errors/src/emitter.rs | 120 | ||||
| -rw-r--r-- | compiler/rustc_errors/src/lib.rs | 83 | ||||
| -rw-r--r-- | compiler/rustc_errors/src/snippet.rs | 2 |
3 files changed, 180 insertions, 25 deletions
diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 050f3ae5833..07c864c93a1 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -14,7 +14,10 @@ use rustc_span::{MultiSpan, SourceFile, Span}; use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, Style, StyledString}; use crate::styled_buffer::StyledBuffer; -use crate::{CodeSuggestion, Diagnostic, DiagnosticId, Level, SubDiagnostic, SuggestionStyle}; +use crate::{ + CodeSuggestion, Diagnostic, DiagnosticId, Level, SubDiagnostic, SubstitutionHighlight, + SuggestionStyle, +}; use rustc_lint_defs::pluralize; @@ -1590,8 +1593,11 @@ impl EmitterWriter { ); let mut row_num = 2; + draw_col_separator_no_space(&mut buffer, 1, max_line_num_len + 1); let mut notice_capitalization = false; - for (complete, parts, only_capitalization) in suggestions.iter().take(MAX_SUGGESTIONS) { + for (complete, parts, highlights, only_capitalization) in + suggestions.iter().take(MAX_SUGGESTIONS) + { notice_capitalization |= only_capitalization; // Only show underline if the suggestion spans a single line and doesn't cover the // entirety of the code output. If you have multiple replacements in the same line @@ -1599,16 +1605,26 @@ impl EmitterWriter { let show_underline = !(parts.len() == 1 && parts[0].snippet.trim() == complete.trim()) && complete.lines().count() == 1; - let lines = sm + let has_deletion = parts.iter().any(|p| p.is_deletion()); + let is_multiline = complete.lines().count() > 1; + + let show_diff = has_deletion && !is_multiline; + + if show_diff { + row_num += 1; + } + + let file_lines = sm .span_to_lines(parts[0].span) .expect("span_to_lines failed when emitting suggestion"); - assert!(!lines.lines.is_empty() || parts[0].span.is_dummy()); + assert!(!file_lines.lines.is_empty() || parts[0].span.is_dummy()); let line_start = sm.lookup_char_pos(parts[0].span.lo()).line; draw_col_separator_no_space(&mut buffer, 1, max_line_num_len + 1); let mut lines = complete.lines(); - for (line_pos, line) in lines.by_ref().take(MAX_SUGGESTION_HIGHLIGHT_LINES).enumerate() + for (line_pos, (line, parts)) in + lines.by_ref().zip(highlights).take(MAX_SUGGESTION_HIGHLIGHT_LINES).enumerate() { // Print the span column to avoid confusion buffer.puts( @@ -1617,9 +1633,60 @@ impl EmitterWriter { &self.maybe_anonymized(line_start + line_pos), Style::LineNumber, ); + if show_diff { + // Add the line number for both addition and removal to drive the point home. + // + // N - fn foo<A: T>(bar: A) { + // N + fn foo(bar: impl T) { + buffer.puts( + row_num - 1, + 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, + &replace_tabs( + &*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 &parts[..] { + [SubstitutionHighlight { start: 0, end }] if *end == line.len() => { + buffer.puts(row_num, max_line_num_len + 1, "+ ", Style::Addition); + } + [] => { + draw_col_separator(&mut buffer, row_num, max_line_num_len + 1); + } + _ => { + buffer.puts(row_num, max_line_num_len + 1, "~ ", Style::Addition); + } + } + } else { + draw_col_separator(&mut buffer, row_num, max_line_num_len + 1); + } + // print the suggestion - draw_col_separator(&mut buffer, row_num, max_line_num_len + 1); buffer.append(row_num, &replace_tabs(line), Style::NoStyle); + + if is_multiline { + for SubstitutionHighlight { start, end } in parts { + buffer.set_style_range( + row_num, + max_line_num_len + 3 + start, + max_line_num_len + 3 + end, + Style::Addition, + true, + ); + } + } row_num += 1; } @@ -1654,25 +1721,36 @@ impl EmitterWriter { let underline_start = (span_start_pos + start) as isize + offset; let underline_end = (span_start_pos + start + sub_len) as isize + offset; assert!(underline_start >= 0 && underline_end >= 0); + let padding: usize = max_line_num_len + 3; for p in underline_start..underline_end { - buffer.putc( - row_num, - ((max_line_num_len + 3) as isize + p) as usize, - '^', - Style::UnderlinePrimary, + // Colorize addition/replacements with green. + buffer.set_style( + row_num - 1, + (padding as isize + p) as usize, + Style::Addition, + true, ); - } - // underline removals too - if underline_start == underline_end { - for p in underline_start - 1..underline_start + 1 { + if !show_diff { + // If this is a replacement, underline with `^`, if this is an addition + // underline with `+`. buffer.putc( row_num, - ((max_line_num_len + 3) as isize + p) as usize, - '-', - Style::UnderlineSecondary, + (padding as isize + p) as usize, + if part.is_addition(&sm) { '+' } else { '~' }, + Style::Addition, ); } } + if show_diff { + // Colorize removal with red in diff format. + buffer.set_style_range( + row_num - 2, + (padding as isize + span_start_pos as isize) as usize, + (padding as isize + span_end_pos as isize) as usize, + Style::Removal, + true, + ); + } // length of the code after substitution let full_sub_len = part @@ -2129,6 +2207,12 @@ impl<'a> WritableDst<'a> { fn apply_style(&mut self, lvl: Level, style: Style) -> io::Result<()> { let mut spec = ColorSpec::new(); match style { + Style::Addition => { + spec.set_fg(Some(Color::Green)).set_intense(true); + } + Style::Removal => { + spec.set_fg(Some(Color::Red)).set_intense(true); + } Style::LineAndColumn => {} Style::LineNumber => { spec.set_bold(true); diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index fc0924ac5f9..ec29d8016dd 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -160,32 +160,77 @@ pub struct SubstitutionPart { pub snippet: String, } +/// Used to translate between `Span`s and byte positions within a single output line in highlighted +/// code of structured suggestions. +#[derive(Debug, Clone, Copy)] +pub struct SubstitutionHighlight { + start: usize, + end: usize, +} + +impl SubstitutionPart { + pub fn is_addition(&self, sm: &SourceMap) -> bool { + !self.snippet.is_empty() + && sm + .span_to_snippet(self.span) + .map_or(self.span.is_empty(), |snippet| snippet.trim().is_empty()) + } + + pub fn is_deletion(&self) -> bool { + self.snippet.trim().is_empty() + } + + pub fn is_replacement(&self, sm: &SourceMap) -> bool { + !self.snippet.is_empty() + && sm + .span_to_snippet(self.span) + .map_or(!self.span.is_empty(), |snippet| !snippet.trim().is_empty()) + } +} + impl CodeSuggestion { /// Returns the assembled code suggestions, whether they should be shown with an underline /// and whether the substitution only differs in capitalization. - pub fn splice_lines(&self, sm: &SourceMap) -> Vec<(String, Vec<SubstitutionPart>, bool)> { + pub fn splice_lines( + &self, + sm: &SourceMap, + ) -> Vec<(String, Vec<SubstitutionPart>, Vec<Vec<SubstitutionHighlight>>, bool)> { + // For the `Vec<Vec<SubstitutionHighlight>>` value, the first level of the vector + // corresponds to the output snippet's lines, while the second level corresponds to the + // substrings within that line that should be highlighted. + use rustc_span::{CharPos, Pos}; + /// Append to a buffer the remainder of the line of existing source code, and return the + /// count of lines that have been added for accurate highlighting. fn push_trailing( buf: &mut String, line_opt: Option<&Cow<'_, str>>, lo: &Loc, hi_opt: Option<&Loc>, - ) { + ) -> usize { + let mut line_count = 0; let (lo, hi_opt) = (lo.col.to_usize(), hi_opt.map(|hi| hi.col.to_usize())); if let Some(line) = line_opt { if let Some(lo) = line.char_indices().map(|(i, _)| i).nth(lo) { let hi_opt = hi_opt.and_then(|hi| line.char_indices().map(|(i, _)| i).nth(hi)); match hi_opt { - Some(hi) if hi > lo => buf.push_str(&line[lo..hi]), + Some(hi) if hi > lo => { + line_count = line[lo..hi].matches('\n').count(); + buf.push_str(&line[lo..hi]) + } Some(_) => (), - None => buf.push_str(&line[lo..]), + None => { + line_count = line[lo..].matches('\n').count(); + buf.push_str(&line[lo..]) + } } } if hi_opt.is_none() { buf.push('\n'); } } + line_count } assert!(!self.substitutions.is_empty()); @@ -220,6 +265,7 @@ impl CodeSuggestion { return None; } + let mut highlights = vec![]; // To build up the result, we do this for each span: // - push the line segment trailing the previous span // (at the beginning a "phantom" span pointing at the start of the line) @@ -236,17 +282,29 @@ impl CodeSuggestion { lines.lines.get(0).and_then(|line0| sf.get_line(line0.line_index)); let mut buf = String::new(); + let mut line_highlight = vec![]; for part in &substitution.parts { let cur_lo = sm.lookup_char_pos(part.span.lo()); if prev_hi.line == cur_lo.line { - push_trailing(&mut buf, prev_line.as_ref(), &prev_hi, Some(&cur_lo)); + let mut count = + push_trailing(&mut buf, prev_line.as_ref(), &prev_hi, Some(&cur_lo)); + while count > 0 { + highlights.push(std::mem::take(&mut line_highlight)); + count -= 1; + } } else { - push_trailing(&mut buf, prev_line.as_ref(), &prev_hi, None); + highlights.push(std::mem::take(&mut line_highlight)); + let mut count = push_trailing(&mut buf, prev_line.as_ref(), &prev_hi, None); + while count > 0 { + highlights.push(std::mem::take(&mut line_highlight)); + count -= 1; + } // push lines between the previous and current span (if any) for idx in prev_hi.line..(cur_lo.line - 1) { if let Some(line) = sf.get_line(idx) { buf.push_str(line.as_ref()); buf.push('\n'); + highlights.push(std::mem::take(&mut line_highlight)); } } if let Some(cur_line) = sf.get_line(cur_lo.line - 1) { @@ -257,10 +315,21 @@ impl CodeSuggestion { buf.push_str(&cur_line[..end]); } } + // Add a whole line highlight per line in the snippet. + line_highlight.push(SubstitutionHighlight { + start: cur_lo.col.0, + end: cur_lo.col.0 + + part.snippet.split('\n').next().unwrap_or(&part.snippet).len(), + }); + for line in part.snippet.split('\n').skip(1) { + highlights.push(std::mem::take(&mut line_highlight)); + line_highlight.push(SubstitutionHighlight { start: 0, end: line.len() }); + } buf.push_str(&part.snippet); prev_hi = sm.lookup_char_pos(part.span.hi()); prev_line = sf.get_line(prev_hi.line - 1); } + highlights.push(std::mem::take(&mut line_highlight)); let only_capitalization = is_case_difference(sm, &buf, bounding_span); // if the replacement already ends with a newline, don't print the next line if !buf.ends_with('\n') { @@ -270,7 +339,7 @@ impl CodeSuggestion { while buf.ends_with('\n') { buf.pop(); } - Some((buf, substitution.parts, only_capitalization)) + Some((buf, substitution.parts, highlights, only_capitalization)) }) .collect() } diff --git a/compiler/rustc_errors/src/snippet.rs b/compiler/rustc_errors/src/snippet.rs index 3fe02bd0cee..64353461e90 100644 --- a/compiler/rustc_errors/src/snippet.rs +++ b/compiler/rustc_errors/src/snippet.rs @@ -177,4 +177,6 @@ pub enum Style { NoStyle, Level(Level), Highlight, + Addition, + Removal, } |
