Auto merge of #74210 - estebank:type-ascriptomatic, r=petrochenkov

Deduplicate `::` -> `:` typo errors Deduplicate errors caused by the same type ascription typo, including ones suggested during parsing that would get reported again during resolve. Fix #70382.
rust-lang · Aug 2, 2020 · fd4d151 · fd4d151
2 parents 1e99138 + 6ed06b2
commit fd4d151
Show file tree

Hide file tree

Showing 28 changed files with 212 additions and 181 deletions.
diff --git a/src/librustc_parse/parser/diagnostics.rs b/src/librustc_parse/parser/diagnostics.rs
@@ -333,6 +333,7 @@ impl<'a> Parser<'a> {
                         Applicability::MachineApplicable
                     },
                 );
+                self.sess.type_ascription_path_suggestions.borrow_mut().insert(sp);
             } else if op_pos.line != next_pos.line && maybe_expected_semicolon {
                 err.span_suggestion(
                     sp,

diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs
@@ -2241,8 +2241,15 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                     self.resolve_expr(argument, None);
                 }
             }
-            ExprKind::Type(ref type_expr, _) => {
-                self.diagnostic_metadata.current_type_ascription.push(type_expr.span);
+            ExprKind::Type(ref type_expr, ref ty) => {
+                // `ParseSess::type_ascription_path_suggestions` keeps spans of colon tokens in
+                // type ascription. Here we are trying to retrieve the span of the colon token as
+                // well, but only if it's written without spaces `expr:Ty` and therefore confusable
+                // with `expr::Ty`, only in this case it will match the span from
+                // `type_ascription_path_suggestions`.
+                self.diagnostic_metadata
+                    .current_type_ascription
+                    .push(type_expr.span.between(ty.span));
                 visit::walk_expr(self, expr);
                 self.diagnostic_metadata.current_type_ascription.pop();
             }

diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs
@@ -17,7 +17,7 @@ use rustc_hir::PrimTy;
 use rustc_session::config::nightly_options;
 use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{kw, sym, Ident};
-use rustc_span::Span;
+use rustc_span::{BytePos, Span};
 
 use log::debug;
 
@@ -223,13 +223,31 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
         if candidates.is_empty() && is_expected(Res::Def(DefKind::Enum, crate_def_id)) {
             let enum_candidates =
                 self.r.lookup_import_candidates(ident, ns, &self.parent_scope, is_enum_variant);
-            let mut enum_candidates = enum_candidates
-                .iter()
-                .map(|suggestion| import_candidate_to_enum_paths(&suggestion))
-                .collect::<Vec<_>>();
-            enum_candidates.sort();
 
             if !enum_candidates.is_empty() {
+                if let (PathSource::Type, Some(span)) =
+                    (source, self.diagnostic_metadata.current_type_ascription.last())
+                {
+                    if self
+                        .r
+                        .session
+                        .parse_sess
+                        .type_ascription_path_suggestions
+                        .borrow()
+                        .contains(span)
+                    {
+                        // Already reported this issue on the lhs of the type ascription.
+                        err.delay_as_bug();
+                        return (err, candidates);
+                    }
+                }
+
+                let mut enum_candidates = enum_candidates
+                    .iter()
+                    .map(|suggestion| import_candidate_to_enum_paths(&suggestion))
+                    .collect::<Vec<_>>();
+                enum_candidates.sort();
+
                 // Contextualize for E0412 "cannot find type", but don't belabor the point
                 // (that it's a variant) for E0573 "expected type, found variant".
                 let preamble = if res.is_none() {
@@ -484,10 +502,21 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
             match source {
                 PathSource::Expr(Some(
                     parent @ Expr { kind: ExprKind::Field(..) | ExprKind::MethodCall(..), .. },
-                )) => {
-                    path_sep(err, &parent);
-                }
-                PathSource::Expr(None) if followed_by_brace => {
+                )) if path_sep(err, &parent) => {}
+                PathSource::Expr(
+                    None
+                    | Some(Expr {
+                        kind:
+                            ExprKind::Path(..)
+                            | ExprKind::Binary(..)
+                            | ExprKind::Unary(..)
+                            | ExprKind::If(..)
+                            | ExprKind::While(..)
+                            | ExprKind::ForLoop(..)
+                            | ExprKind::Match(..),
+                        ..
+                    }),
+                ) if followed_by_brace => {
                     if let Some(sp) = closing_brace {
                         err.multipart_suggestion(
                             "surround the struct literal with parentheses",
@@ -508,11 +537,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
                         );
                     }
                 }
-                PathSource::Expr(
-                    None | Some(Expr { kind: ExprKind::Call(..) | ExprKind::Path(..), .. }),
-                )
-                | PathSource::TupleStruct(_)
-                | PathSource::Pat => {
+                PathSource::Expr(_) | PathSource::TupleStruct(_) | PathSource::Pat => {
                     let span = match &source {
                         PathSource::Expr(Some(Expr {
                             span, kind: ExprKind::Call(_, _), ..
@@ -593,6 +618,24 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
                 Res::Def(DefKind::Enum, def_id),
                 PathSource::TupleStruct(_) | PathSource::Expr(..),
             ) => {
+                if self
+                    .diagnostic_metadata
+                    .current_type_ascription
+                    .last()
+                    .map(|sp| {
+                        self.r
+                            .session
+                            .parse_sess
+                            .type_ascription_path_suggestions
+                            .borrow()
+                            .contains(&sp)
+                    })
+                    .unwrap_or(false)
+                {
+                    err.delay_as_bug();
+                    // We already suggested changing `:` into `::` during parsing.
+                    return false;
+                }
                 if let Some(variants) = self.collect_enum_variants(def_id) {
                     if !variants.is_empty() {
                         let msg = if variants.len() == 1 {
@@ -609,7 +652,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
                         );
                     }
                 } else {
-                    err.note("did you mean to use one of the enum's variants?");
+                    err.note("you might have meant to use one of the enum's variants");
                 }
             }
             (Res::Def(DefKind::Struct, def_id), _) if ns == ValueNS => {
@@ -829,77 +872,73 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
     fn type_ascription_suggestion(&self, err: &mut DiagnosticBuilder<'_>, base_span: Span) {
         let sm = self.r.session.source_map();
         let base_snippet = sm.span_to_snippet(base_span);
-        if let Some(sp) = self.diagnostic_metadata.current_type_ascription.last() {
-            let mut sp = *sp;
-            loop {
-                // Try to find the `:`; bail on first non-':' / non-whitespace.
-                sp = sm.next_point(sp);
-                if let Ok(snippet) = sm.span_to_snippet(sp.to(sm.next_point(sp))) {
-                    let line_sp = sm.lookup_char_pos(sp.hi()).line;
-                    let line_base_sp = sm.lookup_char_pos(base_span.lo()).line;
-                    if snippet == ":" {
-                        let mut show_label = true;
-                        if line_sp != line_base_sp {
-                            err.span_suggestion_short(
-                                sp,
-                                "did you mean to use `;` here instead?",
-                                ";".to_string(),
+        if let Some(&sp) = self.diagnostic_metadata.current_type_ascription.last() {
+            if let Ok(snippet) = sm.span_to_snippet(sp) {
+                let len = snippet.trim_end().len() as u32;
+                if snippet.trim() == ":" {
+                    let colon_sp =
+                        sp.with_lo(sp.lo() + BytePos(len - 1)).with_hi(sp.lo() + BytePos(len));
+                    let mut show_label = true;
+                    if sm.is_multiline(sp) {
+                        err.span_suggestion_short(
+                            colon_sp,
+                            "maybe you meant to write `;` here",
+                            ";".to_string(),
+                            Applicability::MaybeIncorrect,
+                        );
+                    } else {
+                        let after_colon_sp =
+                            self.get_colon_suggestion_span(colon_sp.shrink_to_hi());
+                        if snippet.len() == 1 {
+                            // `foo:bar`
+                            err.span_suggestion(
+                                colon_sp,
+                                "maybe you meant to write a path separator here",
+                                "::".to_string(),
                                 Applicability::MaybeIncorrect,
                             );
-                        } else {
-                            let colon_sp = self.get_colon_suggestion_span(sp);
-                            let after_colon_sp =
-                                self.get_colon_suggestion_span(colon_sp.shrink_to_hi());
-                            if !sm
-                                .span_to_snippet(after_colon_sp)
-                                .map(|s| s == " ")
-                                .unwrap_or(false)
+                            show_label = false;
+                            if !self
+                                .r
+                                .session
+                                .parse_sess
+                                .type_ascription_path_suggestions
+                                .borrow_mut()
+                                .insert(colon_sp)
                             {
-                                err.span_suggestion(
-                                    colon_sp,
-                                    "maybe you meant to write a path separator here",
-                                    "::".to_string(),
-                                    Applicability::MaybeIncorrect,
-                                );
-                                show_label = false;
+                                err.delay_as_bug();
                             }
-                            if let Ok(base_snippet) = base_snippet {
-                                let mut sp = after_colon_sp;
-                                for _ in 0..100 {
-                                    // Try to find an assignment
-                                    sp = sm.next_point(sp);
-                                    let snippet = sm.span_to_snippet(sp.to(sm.next_point(sp)));
-                                    match snippet {
-                                        Ok(ref x) if x.as_str() == "=" => {
-                                            err.span_suggestion(
-                                                base_span,
-                                                "maybe you meant to write an assignment here",
-                                                format!("let {}", base_snippet),
-                                                Applicability::MaybeIncorrect,
-                                            );
-                                            show_label = false;
-                                            break;
-                                        }
-                                        Ok(ref x) if x.as_str() == "\n" => break,
-                                        Err(_) => break,
-                                        Ok(_) => {}
+                        }
+                        if let Ok(base_snippet) = base_snippet {
+                            let mut sp = after_colon_sp;
+                            for _ in 0..100 {
+                                // Try to find an assignment
+                                sp = sm.next_point(sp);
+                                let snippet = sm.span_to_snippet(sp.to(sm.next_point(sp)));
+                                match snippet {
+                                    Ok(ref x) if x.as_str() == "=" => {
+                                        err.span_suggestion(
+                                            base_span,
+                                            "maybe you meant to write an assignment here",
+                                            format!("let {}", base_snippet),
+                                            Applicability::MaybeIncorrect,
+                                        );
+                                        show_label = false;
+                                        break;
                                     }
+                                    Ok(ref x) if x.as_str() == "\n" => break,
+                                    Err(_) => break,
+                                    Ok(_) => {}
                                 }
                             }
                         }
-                        if show_label {
-                            err.span_label(
-                                base_span,
-                                "expecting a type here because of type ascription",
-                            );
-                        }
-                        break;
-                    } else if !snippet.trim().is_empty() {
-                        debug!("tried to find type ascription `:` token, couldn't find it");
-                        break;
                     }
-                } else {
-                    break;
+                    if show_label {
+                        err.span_label(
+                            base_span,
+                            "expecting a type here because of type ascription",
+                        );
+                    }
                 }
             }
         }

diff --git a/src/librustc_session/parse.rs b/src/librustc_session/parse.rs
@@ -138,6 +138,8 @@ pub struct ParseSess {
     pub reached_eof: Lock<bool>,
     /// Environment variables accessed during the build and their values when they exist.
     pub env_depinfo: Lock<FxHashSet<(Symbol, Option<Symbol>)>>,
+    /// All the type ascriptions expressions that have had a suggestion for likely path typo.
+    pub type_ascription_path_suggestions: Lock<FxHashSet<Span>>,
 }
 
 impl ParseSess {
@@ -164,6 +166,7 @@ impl ParseSess {
             symbol_gallery: SymbolGallery::default(),
             reached_eof: Lock::new(false),
             env_depinfo: Default::default(),
+            type_ascription_path_suggestions: Default::default(),
         }
     }
 

diff --git a/src/test/ui/suggestions/issue-61226.fixed b/src/test/ui/suggestions/issue-61226.fixed
@@ -0,0 +1,6 @@
+// run-rustfix
+struct X {}
+fn main() {
+    let _ = vec![X {}]; //…
+    //~^ ERROR expected value, found struct `X`
+}
diff --git a/src/test/ui/suggestions/issue-61226.rs b/src/test/ui/suggestions/issue-61226.rs
@@ -1,5 +1,6 @@
+// run-rustfix
 struct X {}
 fn main() {
-    vec![X]; //…
+    let _ = vec![X]; //…
     //~^ ERROR expected value, found struct `X`
 }
diff --git a/src/test/ui/suggestions/issue-61226.stderr b/src/test/ui/suggestions/issue-61226.stderr
@@ -1,11 +1,11 @@
 error[E0423]: expected value, found struct `X`
-  --> $DIR/issue-61226.rs:3:10
+  --> $DIR/issue-61226.rs:4:18
    |
 LL | struct X {}
    | ----------- `X` defined here
 LL | fn main() {
-LL |     vec![X]; //…
-   |          ^ help: use struct literal syntax instead: `X {}`
+LL |     let _ = vec![X]; //…
+   |                  ^ help: use struct literal syntax instead: `X {}`
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/suggestions/type-ascription-instead-of-method.fixed b/src/test/ui/suggestions/type-ascription-instead-of-method.fixed
@@ -0,0 +1,5 @@
+// run-rustfix
+fn main() {
+    let _ = Box::new("foo".to_string());
+    //~^ ERROR expected type, found
+}
diff --git a/src/test/ui/suggestions/type-ascription-instead-of-method.rs b/src/test/ui/suggestions/type-ascription-instead-of-method.rs
@@ -1,4 +1,5 @@
+// run-rustfix
 fn main() {
-    Box:new("foo".to_string())
+    let _ = Box:new("foo".to_string());
     //~^ ERROR expected type, found
 }
diff --git a/src/test/ui/suggestions/type-ascription-instead-of-method.stderr b/src/test/ui/suggestions/type-ascription-instead-of-method.stderr
@@ -1,10 +1,10 @@
 error: expected type, found `"foo"`
-  --> $DIR/type-ascription-instead-of-method.rs:2:13
+  --> $DIR/type-ascription-instead-of-method.rs:3:21
    |
-LL |     Box:new("foo".to_string())
-   |        -    ^^^^^ expected type
-   |        |
-   |        help: maybe write a path separator here: `::`
+LL |     let _ = Box:new("foo".to_string());
+   |                -    ^^^^^ expected type
+   |                |
+   |                help: maybe write a path separator here: `::`
    |
    = note: `#![feature(type_ascription)]` lets you annotate an expression with a type: `<expr>: <type>`
 

diff --git a/src/test/ui/suggestions/type-ascription-instead-of-path-2.fixed b/src/test/ui/suggestions/type-ascription-instead-of-path-2.fixed
@@ -0,0 +1,6 @@
+// run-rustfix
+fn main() -> Result<(), ()> {
+    let _ = vec![Ok(2)].into_iter().collect::<Result<Vec<_>,_>>()?;
+    //~^ ERROR expected `::`, found `(`
+    Ok(())
+}
diff --git a/src/test/ui/suggestions/type-ascription-instead-of-path-2.rs b/src/test/ui/suggestions/type-ascription-instead-of-path-2.rs
@@ -1,5 +1,6 @@
+// run-rustfix
 fn main() -> Result<(), ()> {
-    vec![Ok(2)].into_iter().collect:<Result<Vec<_>,_>>()?;
+    let _ = vec![Ok(2)].into_iter().collect:<Result<Vec<_>,_>>()?;
     //~^ ERROR expected `::`, found `(`
     Ok(())
 }
diff --git a/src/test/ui/suggestions/type-ascription-instead-of-path-2.stderr b/src/test/ui/suggestions/type-ascription-instead-of-path-2.stderr
@@ -1,10 +1,10 @@
 error: expected `::`, found `(`
-  --> $DIR/type-ascription-instead-of-path-2.rs:2:55
+  --> $DIR/type-ascription-instead-of-path-2.rs:3:63
    |
-LL |     vec![Ok(2)].into_iter().collect:<Result<Vec<_>,_>>()?;
-   |                                    -                  ^ expected `::`
-   |                                    |
-   |                                    help: maybe write a path separator here: `::`
+LL |     let _ = vec![Ok(2)].into_iter().collect:<Result<Vec<_>,_>>()?;
+   |                                            -                  ^ expected `::`
+   |                                            |
+   |                                            help: maybe write a path separator here: `::`
    |
    = note: `#![feature(type_ascription)]` lets you annotate an expression with a type: `<expr>: <type>`
 

diff --git a/src/test/ui/suggestions/type-ascription-instead-of-variant.fixed b/src/test/ui/suggestions/type-ascription-instead-of-variant.fixed
@@ -0,0 +1,5 @@
+// run-rustfix
+fn main() {
+    let _ = Option::Some("");
+    //~^ ERROR expected type, found
+}
diff --git a/src/test/ui/suggestions/type-ascription-instead-of-variant.rs b/src/test/ui/suggestions/type-ascription-instead-of-variant.rs
@@ -1,3 +1,4 @@
+// run-rustfix
 fn main() {
     let _ = Option:Some("");
     //~^ ERROR expected type, found