Address review comments

This commit is contained in:
León Orell Valerian Liehr 2024-11-10 17:51:37 +01:00
parent 1d78004575
commit acf6344b42
No known key found for this signature in database
GPG Key ID: D17A07215F68E713
3 changed files with 44 additions and 41 deletions

View File

@ -22,7 +22,7 @@
use rustc_lint_defs::pluralize; use rustc_lint_defs::pluralize;
use rustc_span::hygiene::{ExpnKind, MacroKind}; use rustc_span::hygiene::{ExpnKind, MacroKind};
use rustc_span::source_map::SourceMap; use rustc_span::source_map::SourceMap;
use rustc_span::{FileLines, FileName, SourceFile, Span, char_width}; use rustc_span::{FileLines, FileName, SourceFile, Span, char_width, str_width};
use termcolor::{Buffer, BufferWriter, Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; use termcolor::{Buffer, BufferWriter, Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
use tracing::{debug, instrument, trace, warn}; use tracing::{debug, instrument, trace, warn};
@ -113,8 +113,12 @@ fn was_cut_left(&self) -> bool {
fn was_cut_right(&self, line_len: usize) -> bool { fn was_cut_right(&self, line_len: usize) -> bool {
let right = let right =
if self.computed_right == self.span_right || self.computed_right == self.label_right { if self.computed_right == self.span_right || self.computed_right == self.label_right {
// FIXME: This comment refers to the only callsite of this method.
// Rephrase it or refactor it, so it can stand on its own.
// Account for the "..." padding given above. Otherwise we end up with code lines // Account for the "..." padding given above. Otherwise we end up with code lines
// that do fit but end in "..." as if they were trimmed. // that do fit but end in "..." as if they were trimmed.
// FIXME: Don't hard-code this offset. Is this meant to represent
// `2 * str_width(self.margin())`?
self.computed_right - 6 self.computed_right - 6
} else { } else {
self.computed_right self.computed_right
@ -673,6 +677,7 @@ fn draw_line(
// Create the source line we will highlight. // Create the source line we will highlight.
let left = margin.left(line_len); let left = margin.left(line_len);
let right = margin.right(line_len); let right = margin.right(line_len);
// FIXME: The following code looks fishy. See #132860.
// On long lines, we strip the source line, accounting for unicode. // On long lines, we strip the source line, accounting for unicode.
let mut taken = 0; let mut taken = 0;
let code: String = source_string let code: String = source_string
@ -695,7 +700,7 @@ fn draw_line(
buffer.puts(line_offset, code_offset, placeholder, Style::LineNumber); buffer.puts(line_offset, code_offset, placeholder, Style::LineNumber);
} }
if margin.was_cut_right(line_len) { if margin.was_cut_right(line_len) {
let padding: usize = placeholder.chars().map(|ch| char_width(ch)).sum(); let padding = str_width(placeholder);
// We have stripped some code after the rightmost span end, make it clear we did so. // We have stripped some code after the rightmost span end, make it clear we did so.
buffer.puts(line_offset, code_offset + taken - padding, placeholder, Style::LineNumber); buffer.puts(line_offset, code_offset + taken - padding, placeholder, Style::LineNumber);
} }
@ -744,6 +749,7 @@ fn render_source_line(
// Left trim // Left trim
let left = margin.left(source_string.len()); let left = margin.left(source_string.len());
// FIXME: This looks fishy. See #132860.
// Account for unicode characters of width !=0 that were removed. // Account for unicode characters of width !=0 that were removed.
let left = source_string.chars().take(left).map(|ch| char_width(ch)).sum(); let left = source_string.chars().take(left).map(|ch| char_width(ch)).sum();
@ -1068,21 +1074,15 @@ fn render_source_line(
if pos > 1 && (annotation.has_label() || annotation.takes_space()) { if pos > 1 && (annotation.has_label() || annotation.takes_space()) {
for p in line_offset + 1..=line_offset + pos { for p in line_offset + 1..=line_offset + pos {
if let AnnotationType::MultilineLine(_) = annotation.annotation_type {
buffer.putc( buffer.putc(
p, p,
(code_offset + annotation.start_col.display).saturating_sub(left), (code_offset + annotation.start_col.display).saturating_sub(left),
underline.multiline_vertical, match annotation.annotation_type {
AnnotationType::MultilineLine(_) => underline.multiline_vertical,
_ => underline.vertical_text_line,
},
underline.style, underline.style,
); );
} else {
buffer.putc(
p,
(code_offset + annotation.start_col.display).saturating_sub(left),
underline.vertical_text_line,
underline.style,
);
}
} }
if let AnnotationType::MultilineStart(_) = annotation.annotation_type { if let AnnotationType::MultilineStart(_) = annotation.annotation_type {
buffer.putc( buffer.putc(
@ -2134,7 +2134,7 @@ fn emit_suggestion_default(
} }
let placeholder = self.margin(); let placeholder = self.margin();
let padding: usize = placeholder.chars().map(|ch| char_width(ch)).sum(); let padding = str_width(placeholder);
buffer.puts( buffer.puts(
row_num, row_num,
max_line_num_len.saturating_sub(padding), max_line_num_len.saturating_sub(padding),
@ -2224,11 +2224,11 @@ fn emit_suggestion_default(
}; };
// ...or trailing spaces. Account for substitutions containing unicode // ...or trailing spaces. Account for substitutions containing unicode
// characters. // characters.
let sub_len: usize = let sub_len: usize = str_width(if is_whitespace_addition {
if is_whitespace_addition { &part.snippet } else { part.snippet.trim() } &part.snippet
.chars() } else {
.map(|ch| char_width(ch)) part.snippet.trim()
.sum(); });
let offset: isize = offsets let offset: isize = offsets
.iter() .iter()
@ -2266,8 +2266,7 @@ fn emit_suggestion_default(
} }
// length of the code after substitution // length of the code after substitution
let full_sub_len = let full_sub_len = str_width(&part.snippet) as isize;
part.snippet.chars().map(|ch| char_width(ch)).sum::<usize>() as isize;
// length of the code to be substituted // length of the code to be substituted
let snippet_len = span_end_pos as isize - span_start_pos as isize; let snippet_len = span_end_pos as isize - span_start_pos as isize;
@ -2282,7 +2281,7 @@ fn emit_suggestion_default(
// if we elided some lines, add an ellipsis // if we elided some lines, add an ellipsis
if lines.next().is_some() { if lines.next().is_some() {
let placeholder = self.margin(); let placeholder = self.margin();
let padding: usize = placeholder.chars().map(|ch| char_width(ch)).sum(); let padding = str_width(placeholder);
buffer.puts( buffer.puts(
row_num, row_num,
max_line_num_len.saturating_sub(padding), max_line_num_len.saturating_sub(padding),

View File

@ -1772,8 +1772,8 @@ pub fn parse_error_format(
color, color,
)); ));
early_dcx.early_fatal(format!( early_dcx.early_fatal(format!(
"argument for `--error-format` must be `human`, `json` or \ "argument for `--error-format` must be `human`, `human-annotate-rs`, \
`short` (instead was `{arg}`)" `human-unicode`, `json`, `pretty-json` or `short` (instead was `{arg}`)"
)) ))
} }
} }
@ -1826,21 +1826,21 @@ pub fn parse_crate_edition(early_dcx: &EarlyDiagCtxt, matches: &getopts::Matches
fn check_error_format_stability( fn check_error_format_stability(
early_dcx: &EarlyDiagCtxt, early_dcx: &EarlyDiagCtxt,
unstable_opts: &UnstableOptions, unstable_opts: &UnstableOptions,
error_format: ErrorOutputType, format: ErrorOutputType,
) { ) {
if !unstable_opts.unstable_options { if unstable_opts.unstable_options {
if let ErrorOutputType::Json { pretty: true, .. } = error_format { return;
early_dcx.early_fatal("`--error-format=pretty-json` is unstable");
}
if let ErrorOutputType::HumanReadable(HumanReadableErrorType::AnnotateSnippet, _) =
error_format
{
early_dcx.early_fatal("`--error-format=human-annotate-rs` is unstable");
}
if let ErrorOutputType::HumanReadable(HumanReadableErrorType::Unicode, _) = error_format {
early_dcx.early_fatal("`--error-format=human-unicode` is unstable");
}
} }
let format = match format {
ErrorOutputType::Json { pretty: true, .. } => "pretty-json",
ErrorOutputType::HumanReadable(format, _) => match format {
HumanReadableErrorType::AnnotateSnippet => "human-annotate-rs",
HumanReadableErrorType::Unicode => "human-unicode",
_ => return,
},
_ => return,
};
early_dcx.early_fatal(format!("`--error-format={format}` is unstable"))
} }
fn parse_output_types( fn parse_output_types(

View File

@ -2223,6 +2223,10 @@ pub fn char_width(ch: char) -> usize {
} }
} }
pub fn str_width(s: &str) -> usize {
s.chars().map(char_width).sum()
}
/// Normalizes the source code and records the normalizations. /// Normalizes the source code and records the normalizations.
fn normalize_src(src: &mut String) -> Vec<NormalizedPos> { fn normalize_src(src: &mut String) -> Vec<NormalizedPos> {
let mut normalized_pos = vec![]; let mut normalized_pos = vec![];