From 2daa013290e054151424849e51c0b1f24db7e7f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 22 May 2018 21:05:35 -0700 Subject: [PATCH 1/3] Underline multiple suggested replacements in the same line Follow up to #50943. Fix #50977. --- src/librustc_errors/emitter.rs | 65 ++++++++++++++++++++++++---------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 91075ddcfa422..123b6abab5048 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -1215,19 +1215,20 @@ impl EmitterWriter { let mut row_num = 2; for &(ref complete, ref parts) in suggestions.iter().take(MAX_SUGGESTIONS) { - let show_underline = parts.len() == 1 - && complete.lines().count() == 1 - && parts[0].snippet.trim() != complete.trim(); + // 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 + // of code, show the underline. + let show_underline = !(parts.len() == 1 + && parts[0].snippet.trim() == complete.trim()) + && complete.lines().count() == 1; let lines = cm.span_to_lines(parts[0].span).unwrap(); assert!(!lines.lines.is_empty()); - let span_start_pos = cm.lookup_char_pos(parts[0].span.lo()); - let line_start = span_start_pos.line; + let line_start = cm.lookup_char_pos(parts[0].span.lo()).line; draw_col_separator_no_space(&mut buffer, 1, max_line_num_len + 1); let mut line_pos = 0; - // Only show underline if there's a single suggestion and it is a single line let mut lines = complete.lines(); for line in lines.by_ref().take(MAX_HIGHLIGHT_LINES) { // Print the span column to avoid confusion @@ -1241,22 +1242,50 @@ impl EmitterWriter { line_pos += 1; row_num += 1; } + let mut extra = 0; // Only show an underline in the suggestions if the suggestion is not the // entirety of the code being shown and the displayed code is not multiline. if show_underline { draw_col_separator(&mut buffer, row_num, max_line_num_len + 1); - let start = parts[0].snippet.len() - parts[0].snippet.trim_left().len(); - // account for substitutions containing unicode characters - let sub_len = parts[0].snippet.trim().chars().fold(0, |acc, ch| { - acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(0) - }); - let underline_start = span_start_pos.col_display + start; - let underline_end = span_start_pos.col_display + start + sub_len; - for p in underline_start..underline_end { - buffer.putc(row_num, - max_line_num_len + 3 + p, - '^', - Style::UnderlinePrimary); + for part in parts { + let span_start_pos = cm.lookup_char_pos(part.span.lo()); + let span_end_pos = cm.lookup_char_pos(part.span.hi()); + // length of the code to be substituted + let snippet_len = span_end_pos.col_display - span_start_pos.col_display; + + // Do not underline the leading or trailing spaces. + let start = part.snippet.len() - part.snippet.trim_left().len(); + // account for substitutions containing unicode characters + let sub_len = part.snippet.trim().chars().fold(0, |acc, ch| { + acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(0) + }); + + let underline_start = span_start_pos.col_display + start + extra; + let underline_end = span_start_pos.col_display + start + sub_len + extra; + for p in underline_start..underline_end { + buffer.putc(row_num, + max_line_num_len + 3 + p, + '^', + Style::UnderlinePrimary); + } + // underline removals too + if underline_start == underline_end { + for p in underline_start-1..underline_start+1 { + buffer.putc(row_num, + max_line_num_len + 3 + p, + '-', + Style::UnderlineSecondary); + } + } + + // length of the code after substitution + let full_sub_len = part.snippet.chars().fold(0, |acc, ch| { + acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(0) + }); + + // For multiple substitutions, use the position *after* the previous + // substitutions have happened. + extra += full_sub_len - snippet_len; } row_num += 1; } From 50eefc0d777a70ff8d11cfd358eb0ed36f6e0651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 23 May 2018 18:31:54 -0700 Subject: [PATCH 2/3] Account for negative offsets in suggestions When suggesting code that has a shorter span than the current code, account for this by keeping the offset as a signed value. --- src/librustc_errors/emitter.rs | 35 +++++++++++-------- .../impl-trait/impl-generic-mismatch.stderr | 2 +- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 123b6abab5048..4d1d33e1325b8 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -1242,29 +1242,32 @@ impl EmitterWriter { line_pos += 1; row_num += 1; } - let mut extra = 0; + + // This offset and the ones below need to be signed to account for replacement code + // that is shorter than the original code. + let mut offset: isize = 0; // Only show an underline in the suggestions if the suggestion is not the // entirety of the code being shown and the displayed code is not multiline. if show_underline { draw_col_separator(&mut buffer, row_num, max_line_num_len + 1); for part in parts { - let span_start_pos = cm.lookup_char_pos(part.span.lo()); - let span_end_pos = cm.lookup_char_pos(part.span.hi()); - // length of the code to be substituted - let snippet_len = span_end_pos.col_display - span_start_pos.col_display; - - // Do not underline the leading or trailing spaces. - let start = part.snippet.len() - part.snippet.trim_left().len(); - // account for substitutions containing unicode characters + let span_start_pos = cm.lookup_char_pos(part.span.lo()).col_display; + let span_end_pos = cm.lookup_char_pos(part.span.hi()).col_display; + + // Do not underline the leading... + let start = part.snippet.len() + .saturating_sub(part.snippet.trim_left().len()); + // ...or trailing spaces. Account for substitutions containing unicode + // characters. let sub_len = part.snippet.trim().chars().fold(0, |acc, ch| { acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(0) }); - let underline_start = span_start_pos.col_display + start + extra; - let underline_end = span_start_pos.col_display + start + sub_len + extra; + let underline_start = (span_start_pos + start) as isize + offset; + let underline_end = (span_start_pos + start + sub_len) as isize + offset; for p in underline_start..underline_end { buffer.putc(row_num, - max_line_num_len + 3 + p, + max_line_num_len + 3 + p as usize, '^', Style::UnderlinePrimary); } @@ -1272,7 +1275,7 @@ impl EmitterWriter { if underline_start == underline_end { for p in underline_start-1..underline_start+1 { buffer.putc(row_num, - max_line_num_len + 3 + p, + max_line_num_len + 3 + p as usize, '-', Style::UnderlineSecondary); } @@ -1280,12 +1283,14 @@ impl EmitterWriter { // length of the code after substitution let full_sub_len = part.snippet.chars().fold(0, |acc, ch| { - acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(0) + acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(0) as isize }); + // length of the code to be substituted + let snippet_len = (span_end_pos - span_start_pos) as isize; // For multiple substitutions, use the position *after* the previous // substitutions have happened. - extra += full_sub_len - snippet_len; + offset += full_sub_len - snippet_len; } row_num += 1; } diff --git a/src/test/ui/impl-trait/impl-generic-mismatch.stderr b/src/test/ui/impl-trait/impl-generic-mismatch.stderr index e133d825ba015..d47adee424c08 100644 --- a/src/test/ui/impl-trait/impl-generic-mismatch.stderr +++ b/src/test/ui/impl-trait/impl-generic-mismatch.stderr @@ -9,7 +9,7 @@ LL | fn foo(&self, _: &U) { } help: try removing the generic parameter and using `impl Trait` instead | LL | fn foo(&self, _: &impl Debug) { } - | + | -- ^^^^^^^^^^ error[E0643]: method `bar` has incompatible signature for trait --> $DIR/impl-generic-mismatch.rs:27:23 From f36c643d4ff76f5d96304d31bdde45152e4e602e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 24 May 2018 10:01:13 -0700 Subject: [PATCH 3/3] Fix impl Trait suggestion --- src/librustc_typeck/check/compare_method.rs | 2 +- src/test/ui/impl-trait/impl-generic-mismatch.stderr | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index d185bc6f300aa..808c134e99448 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -777,7 +777,7 @@ fn compare_synthetic_generics<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let new_generics_span = tcx .sess .codemap() - .generate_fn_name_span(impl_m.span)? + .generate_fn_name_span(impl_span)? .shrink_to_hi(); // in case there are generics, just replace them let generics_span = impl_m diff --git a/src/test/ui/impl-trait/impl-generic-mismatch.stderr b/src/test/ui/impl-trait/impl-generic-mismatch.stderr index d47adee424c08..a5f1580b60d2f 100644 --- a/src/test/ui/impl-trait/impl-generic-mismatch.stderr +++ b/src/test/ui/impl-trait/impl-generic-mismatch.stderr @@ -21,12 +21,8 @@ LL | fn bar(&self, _: &impl Debug) { } | ^^^^^^^^^^ expected generic parameter, found `impl Trait` help: try changing the `impl Trait` argument to a generic parameter | -LL | fn bar(&self, _: &U); -LL | } -LL | -LL | impl Bar for () { -LL | fn bar(&self, _: &U) { } - | +LL | fn bar(&self, _: &U) { } + | ^^^^^^^^^^ ^ error[E0643]: method `hash` has incompatible signature for trait --> $DIR/impl-generic-mismatch.rs:38:33