From 6dd718ca7937fc1a769d65b06396cf64f45f94ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 14 Oct 2019 14:30:59 -0700 Subject: [PATCH] Use heuristics for capitalization warning in suggestions --- src/librustc_errors/emitter.rs | 27 ++++++++++++++----- src/librustc_errors/lib.rs | 6 ++--- .../extern-prelude-from-opaque-fail.stderr | 2 +- .../lint/lint-group-nonstandard-style.stderr | 2 +- ...lint-lowercase-static-const-pattern.stderr | 4 +-- .../ui/lint/lint-non-camel-case-types.stderr | 4 +-- .../lint/lint-non-snake-case-functions.stderr | 2 +- .../ui/lint/lint-non-uppercase-statics.stderr | 2 +- .../ui/lint/lint-uppercase-variables.stderr | 2 +- src/test/ui/resolve/issue-39226.stderr | 2 +- .../inaccessible-test-modules.stderr | 4 +-- .../ui/traits/trait-impl-for-module.stderr | 2 +- src/test/ui/utf8_idents.stderr | 2 +- 13 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 0408445376806..d02201d5321c7 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -13,7 +13,7 @@ use syntax_pos::{SourceFile, Span, MultiSpan}; use crate::{ Level, CodeSuggestion, Diagnostic, SubDiagnostic, - SuggestionStyle, SourceMapperDyn, DiagnosticId, + SuggestionStyle, SourceMapper, SourceMapperDyn, DiagnosticId, }; use crate::Level::Error; use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style}; @@ -239,11 +239,11 @@ pub trait Emitter { format!( "help: {}{}: `{}`", sugg.msg, - if self.source_map().as_ref().map(|sm| substitution.to_lowercase() == sm - .span_to_snippet(sugg.substitutions[0].parts[0].span) - .unwrap() - .to_lowercase()).unwrap_or(false) - { + if self.source_map().map(|sm| is_case_difference( + &**sm, + substitution, + sugg.substitutions[0].parts[0].span, + )).unwrap_or(false) { " (notice the capitalization)" } else { "" @@ -2058,3 +2058,18 @@ impl<'a> Drop for WritableDst<'a> { } } } + +/// Whether the original and suggested code are visually similar enough to warrant extra wording. +pub fn is_case_difference(sm: &dyn SourceMapper, suggested: &str, sp: Span) -> bool { + // FIXME: this should probably be extended to also account for `FO0` → `FOO` and unicode. + let found = sm.span_to_snippet(sp).unwrap(); + let ascii_confusables = &['c', 'f', 'i', 'k', 'o', 's', 'u', 'v', 'w', 'x', 'y', 'z']; + // There are ASCII chars that are confusable (above) and differ in capitalization: + let confusable = found.chars().zip(suggested.chars()).any(|(f, s)| { + (ascii_confusables.contains(&f) || ascii_confusables.contains(&s)) && f != s + }); + confusable && found.to_lowercase() == suggested.to_lowercase() + // FIXME: We sometimes suggest the same thing we already have, which is a + // bug, but be defensive against that here. + && found != suggested +} diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index babaeb7e532ae..63df052a22504 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -13,7 +13,7 @@ pub use emitter::ColorConfig; use Level::*; -use emitter::{Emitter, EmitterWriter}; +use emitter::{Emitter, EmitterWriter, is_case_difference}; use registry::Registry; use rustc_data_structures::sync::{self, Lrc, Lock}; @@ -239,8 +239,7 @@ impl CodeSuggestion { prev_hi = cm.lookup_char_pos(part.span.hi()); prev_line = fm.get_line(prev_hi.line - 1); } - let only_capitalization = buf.clone().to_lowercase() - == cm.span_to_snippet(bounding_span).unwrap().to_lowercase(); + let only_capitalization = is_case_difference(cm, &buf, bounding_span); // if the replacement already ends with a newline, don't print the next line if !buf.ends_with('\n') { push_trailing(&mut buf, prev_line.as_ref(), &prev_hi, None); @@ -250,7 +249,6 @@ impl CodeSuggestion { buf.pop(); } (buf, substitution.parts, only_capitalization) - }).collect() } } diff --git a/src/test/ui/hygiene/extern-prelude-from-opaque-fail.stderr b/src/test/ui/hygiene/extern-prelude-from-opaque-fail.stderr index 1f5e46c133dd1..65133eb1e1873 100644 --- a/src/test/ui/hygiene/extern-prelude-from-opaque-fail.stderr +++ b/src/test/ui/hygiene/extern-prelude-from-opaque-fail.stderr @@ -5,7 +5,7 @@ LL | use my_core; | ^^^^^^^ | | | no `my_core` in the root - | help: a similar name exists in the module (notice the capitalization): `my_core` + | help: a similar name exists in the module: `my_core` error[E0432]: unresolved import `my_core` --> $DIR/extern-prelude-from-opaque-fail.rs:7:13 diff --git a/src/test/ui/lint/lint-group-nonstandard-style.stderr b/src/test/ui/lint/lint-group-nonstandard-style.stderr index 1ecd0f38826af..1cc973d32c2d3 100644 --- a/src/test/ui/lint/lint-group-nonstandard-style.stderr +++ b/src/test/ui/lint/lint-group-nonstandard-style.stderr @@ -41,7 +41,7 @@ error: static variable `bad` should have an upper case name --> $DIR/lint-group-nonstandard-style.rs:14:16 | LL | static bad: isize = 1; - | ^^^ help: convert the identifier to upper case (notice the capitalization): `BAD` + | ^^^ help: convert the identifier to upper case: `BAD` | note: lint level defined here --> $DIR/lint-group-nonstandard-style.rs:10:14 diff --git a/src/test/ui/lint/lint-lowercase-static-const-pattern.stderr b/src/test/ui/lint/lint-lowercase-static-const-pattern.stderr index 5cac1c3d05355..40e5a736a8e56 100644 --- a/src/test/ui/lint/lint-lowercase-static-const-pattern.stderr +++ b/src/test/ui/lint/lint-lowercase-static-const-pattern.stderr @@ -2,7 +2,7 @@ error: constant in pattern `a` should have an upper case name --> $DIR/lint-lowercase-static-const-pattern.rs:11:13 | LL | (0, a) => 0, - | ^ help: convert the identifier to upper case (notice the capitalization): `A` + | ^ help: convert the identifier to upper case: `A` | note: lint level defined here --> $DIR/lint-lowercase-static-const-pattern.rs:4:9 @@ -14,7 +14,7 @@ error: constant in pattern `aha` should have an upper case name --> $DIR/lint-lowercase-static-const-pattern.rs:26:13 | LL | (0, aha) => 0, - | ^^^ help: convert the identifier to upper case (notice the capitalization): `AHA` + | ^^^ help: convert the identifier to upper case: `AHA` error: constant in pattern `not_okay` should have an upper case name --> $DIR/lint-lowercase-static-const-pattern.rs:40:13 diff --git a/src/test/ui/lint/lint-non-camel-case-types.stderr b/src/test/ui/lint/lint-non-camel-case-types.stderr index 02f7db045e5cc..177f8c8fe9b63 100644 --- a/src/test/ui/lint/lint-non-camel-case-types.stderr +++ b/src/test/ui/lint/lint-non-camel-case-types.stderr @@ -38,7 +38,7 @@ error: variant `bar` should have an upper camel case name --> $DIR/lint-non-camel-case-types.rs:22:5 | LL | bar - | ^^^ help: convert the identifier to upper camel case (notice the capitalization): `Bar` + | ^^^ help: convert the identifier to upper camel case: `Bar` error: trait `foo6` should have an upper camel case name --> $DIR/lint-non-camel-case-types.rs:25:7 @@ -50,7 +50,7 @@ error: type parameter `ty` should have an upper camel case name --> $DIR/lint-non-camel-case-types.rs:29:6 | LL | fn f(_: ty) {} - | ^^ help: convert the identifier to upper camel case (notice the capitalization): `Ty` + | ^^ help: convert the identifier to upper camel case: `Ty` error: aborting due to 8 previous errors diff --git a/src/test/ui/lint/lint-non-snake-case-functions.stderr b/src/test/ui/lint/lint-non-snake-case-functions.stderr index 2e7b53b05d16b..244522acf1f80 100644 --- a/src/test/ui/lint/lint-non-snake-case-functions.stderr +++ b/src/test/ui/lint/lint-non-snake-case-functions.stderr @@ -26,7 +26,7 @@ error: method `render_HTML` should have a snake case name --> $DIR/lint-non-snake-case-functions.rs:17:8 | LL | fn render_HTML() {} - | ^^^^^^^^^^^ help: convert the identifier to snake case (notice the capitalization): `render_html` + | ^^^^^^^^^^^ help: convert the identifier to snake case: `render_html` error: trait method `ABC` should have a snake case name --> $DIR/lint-non-snake-case-functions.rs:22:8 diff --git a/src/test/ui/lint/lint-non-uppercase-statics.stderr b/src/test/ui/lint/lint-non-uppercase-statics.stderr index a03bdf52a0a6e..ceb83d08f2777 100644 --- a/src/test/ui/lint/lint-non-uppercase-statics.stderr +++ b/src/test/ui/lint/lint-non-uppercase-statics.stderr @@ -14,7 +14,7 @@ error: static variable `bar` should have an upper case name --> $DIR/lint-non-uppercase-statics.rs:6:12 | LL | static mut bar: isize = 1; - | ^^^ help: convert the identifier to upper case (notice the capitalization): `BAR` + | ^^^ help: convert the identifier to upper case: `BAR` error: aborting due to 2 previous errors diff --git a/src/test/ui/lint/lint-uppercase-variables.stderr b/src/test/ui/lint/lint-uppercase-variables.stderr index cc2be3d0bd9d5..f614d5d71f88c 100644 --- a/src/test/ui/lint/lint-uppercase-variables.stderr +++ b/src/test/ui/lint/lint-uppercase-variables.stderr @@ -39,7 +39,7 @@ error: variable `Test` should have a snake case name --> $DIR/lint-uppercase-variables.rs:18:9 | LL | let Test: usize = 0; - | ^^^^ help: convert the identifier to snake case (notice the capitalization): `test` + | ^^^^ help: convert the identifier to snake case: `test` error: variable `Foo` should have a snake case name --> $DIR/lint-uppercase-variables.rs:22:9 diff --git a/src/test/ui/resolve/issue-39226.stderr b/src/test/ui/resolve/issue-39226.stderr index ad596c25551a7..d9a28e63dce8b 100644 --- a/src/test/ui/resolve/issue-39226.stderr +++ b/src/test/ui/resolve/issue-39226.stderr @@ -8,7 +8,7 @@ LL | handle: Handle | ^^^^^^ | | | did you mean `Handle { /* fields */ }`? - | help: a local variable with a similar name exists (notice the capitalization): `handle` + | help: a local variable with a similar name exists: `handle` error: aborting due to previous error diff --git a/src/test/ui/test-attrs/inaccessible-test-modules.stderr b/src/test/ui/test-attrs/inaccessible-test-modules.stderr index 0b94619f689a7..a94ea1e79bc51 100644 --- a/src/test/ui/test-attrs/inaccessible-test-modules.stderr +++ b/src/test/ui/test-attrs/inaccessible-test-modules.stderr @@ -5,7 +5,7 @@ LL | use main as x; | ----^^^^^ | | | no `main` in the root - | help: a similar name exists in the module (notice the capitalization): `main` + | help: a similar name exists in the module: `main` error[E0432]: unresolved import `test` --> $DIR/inaccessible-test-modules.rs:6:5 @@ -14,7 +14,7 @@ LL | use test as y; | ----^^^^^ | | | no `test` in the root - | help: a similar name exists in the module (notice the capitalization): `test` + | help: a similar name exists in the module: `test` error: aborting due to 2 previous errors diff --git a/src/test/ui/traits/trait-impl-for-module.stderr b/src/test/ui/traits/trait-impl-for-module.stderr index 8b81192aa4701..4a06cd777d49e 100644 --- a/src/test/ui/traits/trait-impl-for-module.stderr +++ b/src/test/ui/traits/trait-impl-for-module.stderr @@ -2,7 +2,7 @@ error[E0573]: expected type, found module `a` --> $DIR/trait-impl-for-module.rs:7:12 | LL | impl A for a { - | ^ help: a trait with a similar name exists (notice the capitalization): `A` + | ^ help: a trait with a similar name exists: `A` error: aborting due to previous error diff --git a/src/test/ui/utf8_idents.stderr b/src/test/ui/utf8_idents.stderr index 869bed61ea0f1..56de63da4f979 100644 --- a/src/test/ui/utf8_idents.stderr +++ b/src/test/ui/utf8_idents.stderr @@ -38,7 +38,7 @@ warning: type parameter `γ` should have an upper camel case name --> $DIR/utf8_idents.rs:3:5 | LL | γ - | ^ help: convert the identifier to upper camel case (notice the capitalization): `Γ` + | ^ help: convert the identifier to upper camel case: `Γ` | = note: `#[warn(non_camel_case_types)]` on by default