From 87fded1eddf834f2decbfd05e6fda35cb1ee4747 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 6 Jun 2022 18:04:42 +0400 Subject: [PATCH] Improve suggestions when its parts are far from each other Previously we only show at most 6 lines of suggestions and, if the suggestions are more than 6 lines apart, we've just showed ... at the end. This is probably fine, but quite confusing in my opinion. This commit is an attempt to show ... in places where there is nothing to suggest instead, for example: Before: ```text help: consider enclosing expression in a block | 3 ~ 'l: { match () { () => break 'l, 4 | 5 | 6 | 7 | 8 | ... ``` After: ```text help: consider enclosing expression in a block | 3 ~ 'l: { match () { () => break 'l, 4 | ... 31| 32~ } }; | ``` --- compiler/rustc_errors/src/emitter.rs | 167 ++++++++++++++++----------- 1 file changed, 99 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index e9e7065ec03..269d34ea432 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -656,11 +656,6 @@ impl Emitter for SilentEmitter { } } -/// Maximum number of lines we will print for a multiline suggestion; arbitrary. -/// -/// This should be replaced with a more involved mechanism to output multiline suggestions that -/// more closely mimics the regular diagnostic output, where irrelevant code lines are elided. -pub const MAX_SUGGESTION_HIGHLIGHT_LINES: usize = 6; /// Maximum number of suggestions to be shown /// /// Arbitrary, but taken from trait import suggestion limit @@ -1839,79 +1834,115 @@ impl EmitterWriter { } row_num += line_end - line_start; } - for (line_pos, (line, highlight_parts)) in - lines.by_ref().zip(highlights).take(MAX_SUGGESTION_HIGHLIGHT_LINES).enumerate() - { - // Print the span column to avoid confusion - buffer.puts( - row_num, - 0, - &self.maybe_anonymized(line_start + line_pos), - Style::LineNumber, - ); - if let DisplaySuggestion::Diff = show_code_change { - // Add the line number for both addition and removal to drive the point home. - // - // N - fn foo(bar: A) { - // N + fn foo(bar: impl T) { + let mut unhighlighted_lines = Vec::new(); + for (line_pos, (line, highlight_parts)) in lines.by_ref().zip(highlights).enumerate() { + debug!(%line_pos, %line, ?highlight_parts); + + let print_line = |line_pos: usize, + line: &str, + highlight_parts: &Vec, + buffer: &mut StyledBuffer, + row_num: &mut usize| { + // Print the span column to avoid confusion buffer.puts( - row_num - 1, + *row_num, 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, - &normalize_whitespace( - &*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 &highlight_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); + if let DisplaySuggestion::Diff = show_code_change { + // Add the line number for both addition and removal to drive the point home. + // + // N - fn foo(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, + &normalize_whitespace( + &*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 &highlight_parts[..] { + [SubstitutionHighlight { start: 0, end }] if *end == line.len() => { + buffer.puts(*row_num, max_line_num_len + 1, "+ ", Style::Addition); + } + [] => { + draw_col_separator(buffer, *row_num, max_line_num_len + 1); + } + _ => { + buffer.puts(*row_num, max_line_num_len + 1, "~ ", Style::Addition); + } } + } else { + draw_col_separator(buffer, *row_num, max_line_num_len + 1); } - } else { - draw_col_separator(&mut buffer, row_num, max_line_num_len + 1); + + // print the suggestion + buffer.append(*row_num, &normalize_whitespace(line), Style::NoStyle); + + // Colorize addition/replacements with green. + for &SubstitutionHighlight { start, end } in highlight_parts { + // Account for tabs when highlighting (#87972). + let tabs: usize = line + .chars() + .take(start) + .map(|ch| match ch { + '\t' => 3, + _ => 0, + }) + .sum(); + buffer.set_style_range( + *row_num, + max_line_num_len + 3 + start + tabs, + max_line_num_len + 3 + end + tabs, + Style::Addition, + true, + ); + } + *row_num += 1; + }; + + if highlight_parts.is_empty() { + unhighlighted_lines.push((line_pos, line)); + continue; } - // print the suggestion - buffer.append(row_num, &normalize_whitespace(line), Style::NoStyle); - - // Colorize addition/replacements with green. - for &SubstitutionHighlight { start, end } in highlight_parts { - // Account for tabs when highlighting (#87972). - let tabs: usize = line - .chars() - .take(start) - .map(|ch| match ch { - '\t' => 3, - _ => 0, - }) - .sum(); - buffer.set_style_range( - row_num, - max_line_num_len + 3 + start + tabs, - max_line_num_len + 3 + end + tabs, - Style::Addition, - true, - ); + match unhighlighted_lines.len() { + 0 => (), + // Since we show first line, "..." line and last line, + // There is no reason to hide if there are 3 or less lines + // (because then we just replace a line with ... which is + // not helpful) + n if n <= 3 => unhighlighted_lines.drain(..).for_each(|(p, l)| { + print_line(p, l, &Vec::new(), &mut buffer, &mut row_num) + }), + _ => { + unhighlighted_lines + .drain(..1) + .next() + .map(|(p, l)| print_line(p, l, &Vec::new(), &mut buffer, &mut row_num)); + buffer.puts(row_num, max_line_num_len - 1, "...", Style::LineNumber); + row_num += 1; + unhighlighted_lines + .pop() + .map(|(p, l)| print_line(p, l, &Vec::new(), &mut buffer, &mut row_num)); + } } - row_num += 1; + + print_line(line_pos, line, highlight_parts, &mut buffer, &mut row_num) } // This offset and the ones below need to be signed to account for replacement code