Auto merge of #43096 - estebank:ascription-help, r=nikomatsakis

Point at `:` when using it instead of `;`

When triggering type ascription in such a way that we can infer a
statement end was intended, add a suggestion for the change. Always
point out the reason for the expectation of a type is due to type
ascription.

Fix #42057, #41928.

Author: bors

src/librustc_errors/diagnostic.rs

@@ -209,6 +209,22 @@ impl Diagnostic {
         self
     }
 
+    /// Prints out a message with a suggested edit of the code. If the suggestion is presented
+    /// inline it will only show the text message and not the text.
+    ///
+    /// See `diagnostic::CodeSuggestion` for more information.
+    pub fn span_suggestion_short(&mut self, sp: Span, msg: &str, suggestion: String) -> &mut Self {
+        self.suggestions.push(CodeSuggestion {
+            substitution_parts: vec![Substitution {
+                span: sp,
+                substitutions: vec![suggestion],
+            }],
+            msg: msg.to_owned(),
+            show_code_when_inline: false,
+        });
+        self
+    }
+
     /// Prints out a message with a suggested edit of the code.
     ///
     /// In case of short messages and a simple suggestion,
@@ -231,6 +247,7 @@ impl Diagnostic {
                 substitutions: vec![suggestion],
             }],
             msg: msg.to_owned(),
+            show_code_when_inline: true,
         });
         self
     }
@@ -242,6 +259,7 @@ impl Diagnostic {
                 substitutions: suggestions,
             }],
             msg: msg.to_owned(),
+            show_code_when_inline: true,
         });
         self
     }

src/librustc_errors/diagnostic_builder.rs

@@ -146,6 +146,11 @@ impl<'a> DiagnosticBuilder<'a> {
                                                   sp: S,
                                                   msg: &str)
                                                   -> &mut Self);
+    forward!(pub fn span_suggestion_short(&mut self,
+                                          sp: Span,
+                                          msg: &str,
+                                          suggestion: String)
+                                          -> &mut Self);
     forward!(pub fn span_suggestion(&mut self,
                                     sp: Span,
                                     msg: &str,

src/librustc_errors/emitter.rs

@@ -47,8 +47,9 @@ impl Emitter for EmitterWriter {
                // don't display multiline suggestions as labels
                sugg.substitution_parts[0].substitutions[0].find('\n').is_none() {
                 let substitution = &sugg.substitution_parts[0].substitutions[0];
-                let msg = if substitution.len() == 0 {
-                    // This substitution is only removal, don't show it
+                let msg = if substitution.len() == 0 || !sugg.show_code_when_inline {
+                    // This substitution is only removal or we explicitely don't want to show the
+                    // code inline, don't show it
                     format!("help: {}", sugg.msg)
                 } else {
                     format!("help: {}: `{}`", sugg.msg, substitution)

src/librustc_errors/lib.rs

@@ -84,6 +84,7 @@ pub struct CodeSuggestion {
     /// ```
     pub substitution_parts: Vec<Substitution>,
     pub msg: String,
+    pub show_code_when_inline: bool,
 }
 
 #[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)]

src/librustc_resolve/lib.rs

@@ -599,19 +599,24 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> {
         self.resolve_local(local);
     }
     fn visit_ty(&mut self, ty: &'tcx Ty) {
-        if let TyKind::Path(ref qself, ref path) = ty.node {
-            self.smart_resolve_path(ty.id, qself.as_ref(), path, PathSource::Type);
-        } else if let TyKind::ImplicitSelf = ty.node {
-            let self_ty = keywords::SelfType.ident();
-            let def = self.resolve_ident_in_lexical_scope(self_ty, TypeNS, true, ty.span)
-                          .map_or(Def::Err, |d| d.def());
-            self.record_def(ty.id, PathResolution::new(def));
-        } else if let TyKind::Array(ref element, ref length) = ty.node {
-            self.visit_ty(element);
-            self.with_constant_rib(|this| {
-                this.visit_expr(length);
-            });
-            return;
+        match ty.node {
+            TyKind::Path(ref qself, ref path) => {
+                self.smart_resolve_path(ty.id, qself.as_ref(), path, PathSource::Type);
+            }
+            TyKind::ImplicitSelf => {
+                let self_ty = keywords::SelfType.ident();
+                let def = self.resolve_ident_in_lexical_scope(self_ty, TypeNS, true, ty.span)
+                              .map_or(Def::Err, |d| d.def());
+                self.record_def(ty.id, PathResolution::new(def));
+            }
+            TyKind::Array(ref element, ref length) => {
+                self.visit_ty(element);
+                self.with_constant_rib(|this| {
+                    this.visit_expr(length);
+                });
+                return;
+            }
+            _ => (),
         }
         visit::walk_ty(self, ty);
     }
@@ -1221,6 +1226,9 @@ pub struct Resolver<'a> {
     // This table maps struct IDs into struct constructor IDs,
     // it's not used during normal resolution, only for better error reporting.
     struct_constructors: DefIdMap<(Def, ty::Visibility)>,
+
+    // Only used for better errors on `fn(): fn()`
+    current_type_ascription: Vec<Span>,
 }
 
 pub struct ResolverArenas<'a> {
@@ -1411,6 +1419,7 @@ impl<'a> Resolver<'a> {
             struct_constructors: DefIdMap(),
             found_unresolved_macro: false,
             unused_macros: FxHashSet(),
+            current_type_ascription: Vec::new(),
         }
     }
 
@@ -2499,6 +2508,7 @@ impl<'a> Resolver<'a> {
             // Fallback label.
             if !levenshtein_worked {
                 err.span_label(base_span, fallback_label);
+                this.type_ascription_suggestion(&mut err, base_span);
             }
             err
         };
@@ -2554,6 +2564,41 @@ impl<'a> Resolver<'a> {
         resolution
     }
 
+    fn type_ascription_suggestion(&self,
+                                  err: &mut DiagnosticBuilder,
+                                  base_span: Span) {
+        debug!("type_ascription_suggetion {:?}", base_span);
+        let cm = self.session.codemap();
+        debug!("self.current_type_ascription {:?}", self.current_type_ascription);
+        if let Some(sp) = self.current_type_ascription.last() {
+            let mut sp = *sp;
+            loop {  // try to find the `:`, bail on first non-':'/non-whitespace
+                sp = sp.next_point();
+                if let Ok(snippet) = cm.span_to_snippet(sp.to(sp.next_point())) {
+                    debug!("snippet {:?}", snippet);
+                    let line_sp = cm.lookup_char_pos(sp.hi).line;
+                    let line_base_sp = cm.lookup_char_pos(base_span.lo).line;
+                    debug!("{:?} {:?}", line_sp, line_base_sp);
+                    if snippet == ":" {
+                        err.span_label(base_span,
+                                       "expecting a type here because of type ascription");
+                        if line_sp != line_base_sp {
+                            err.span_suggestion_short(sp,
+                                                      "did you mean to use `;` here instead?",
+                                                      ";".to_string());
+                        }
+                        break;
+                    } else if snippet.trim().len() != 0  {
+                        debug!("tried to find type ascription `:` token, couldn't find it");
+                        break;
+                    }
+                } else {
+                    break;
+                }
+            }
+        }
+    }
+
     fn self_type_is_available(&mut self, span: Span) -> bool {
         let binding = self.resolve_ident_in_lexical_scope(keywords::SelfType.ident(),
                                                           TypeNS, false, span);
@@ -3170,7 +3215,11 @@ impl<'a> Resolver<'a> {
                     self.resolve_expr(argument, None);
                 }
             }
-
+            ExprKind::Type(ref type_expr, _) => {
+                self.current_type_ascription.push(type_expr.span);
+                visit::walk_expr(self, expr);
+                self.current_type_ascription.pop();
+            }
             _ => {
                 visit::walk_expr(self, expr);
             }

src/libsyntax/parse/parser.rs

@@ -2798,7 +2798,22 @@ impl<'a> Parser<'a> {
                 lhs = self.parse_assoc_op_cast(lhs, lhs_span, ExprKind::Cast)?;
                 continue
             } else if op == AssocOp::Colon {
-                lhs = self.parse_assoc_op_cast(lhs, lhs_span, ExprKind::Type)?;
+                lhs = match self.parse_assoc_op_cast(lhs, lhs_span, ExprKind::Type) {
+                    Ok(lhs) => lhs,
+                    Err(mut err) => {
+                        err.span_label(self.span,
+                                       "expecting a type here because of type ascription");
+                        let cm = self.sess.codemap();
+                        let cur_pos = cm.lookup_char_pos(self.span.lo);
+                        let op_pos = cm.lookup_char_pos(cur_op_span.hi);
+                        if cur_pos.line != op_pos.line {
+                            err.span_suggestion_short(cur_op_span,
+                                                      "did you mean to use `;` here?",
+                                                      ";".to_string());
+                        }
+                        return Err(err);
+                    }
+                };
                 continue
             } else if op == AssocOp::DotDot || op == AssocOp::DotDotDot {
                 // If we didn’t have to handle `x..`/`x...`, it would be pretty easy to

src/test/ui/issue-22644.stderr

@@ -80,5 +80,5 @@ error: expected type, found `4`
   --> $DIR/issue-22644.rs:38:28
    |
 38 |     println!("{}", a: &mut 4);
-   |                            ^
+   |                            ^ expecting a type here because of type ascription
 

src/test/ui/suggestions/type-ascription-instead-of-statement-end.rs

@@ -0,0 +1,20 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(type_ascription)]
+
+fn main() {
+    println!("test"):
+    0;
+}
+
+fn foo() {
+    println!("test"): 0;
+}

src/test/ui/suggestions/type-ascription-instead-of-statement-end.stderr

@@ -0,0 +1,16 @@
+error: expected type, found `0`
+  --> $DIR/type-ascription-instead-of-statement-end.rs:15:5
+   |
+14 |     println!("test"):
+   |                     - help: did you mean to use `;` here?
+15 |     0;
+   |     ^ expecting a type here because of type ascription
+
+error: expected type, found `0`
+  --> $DIR/type-ascription-instead-of-statement-end.rs:19:23
+   |
+19 |     println!("test"): 0;
+   |                       ^ expecting a type here because of type ascription
+
+error: aborting due to 2 previous errors
+

src/test/ui/suggestions/type-ascription-with-fn-call.rs

@@ -0,0 +1,18 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(type_ascription)]
+
+fn main() {
+    f()  :
+    f();
+}
+
+fn f() {}