From 7239d7717188f6dc2b380a40335a0c8571be6502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 6 Jul 2017 14:29:55 -0700 Subject: [PATCH 1/3] 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. --- src/libsyntax/parse/parser.rs | 17 +++++++++++++++- ...ype-ascription-instead-of-statement-end.rs | 20 +++++++++++++++++++ ...ascription-instead-of-statement-end.stderr | 16 +++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/suggestions/type-ascription-instead-of-statement-end.rs create mode 100644 src/test/ui/suggestions/type-ascription-instead-of-statement-end.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 74b2ea1df323..1b6c3cf94e4f 100644 --- a/src/libsyntax/parse/parser.rs +++ b/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(cur_op_span, + "did you mean to end the statement here instead?", + ";".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 diff --git a/src/test/ui/suggestions/type-ascription-instead-of-statement-end.rs b/src/test/ui/suggestions/type-ascription-instead-of-statement-end.rs new file mode 100644 index 000000000000..93de55a39e95 --- /dev/null +++ b/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 or the MIT license +// , 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; +} diff --git a/src/test/ui/suggestions/type-ascription-instead-of-statement-end.stderr b/src/test/ui/suggestions/type-ascription-instead-of-statement-end.stderr new file mode 100644 index 000000000000..e4cf78dbb2d9 --- /dev/null +++ b/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 end the statement here instead? `;` +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 + From faf90351b79de04de0176b1e0de07cc5f1730eaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 16 Jul 2017 11:43:24 -0700 Subject: [PATCH 2/3] Add flag to hide code on inline suggestions Now there's a way to add suggestions that hide the suggested code when presented inline, to avoid weird wording when short code snippets are added at the end. --- src/librustc_errors/diagnostic.rs | 18 ++++++++++++++++++ src/librustc_errors/diagnostic_builder.rs | 5 +++++ src/librustc_errors/emitter.rs | 5 +++-- src/librustc_errors/lib.rs | 1 + src/libsyntax/parse/parser.rs | 6 +++--- src/test/ui/issue-22644.stderr | 2 +- ...-ascription-instead-of-statement-end.stderr | 2 +- 7 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index d7c21127474a..2ffa5fc796e1 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/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. /// /// See `diagnostic::CodeSuggestion` for more information. @@ -219,6 +235,7 @@ impl Diagnostic { substitutions: vec![suggestion], }], msg: msg.to_owned(), + show_code_when_inline: true, }); self } @@ -230,6 +247,7 @@ impl Diagnostic { substitutions: suggestions, }], msg: msg.to_owned(), + show_code_when_inline: true, }); self } diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index 0081339a363f..6f6470089d77 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/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, diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 2aea6d125f20..94b3a4396d70 100644 --- a/src/librustc_errors/emitter.rs +++ b/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) diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index c4beb5ebc42d..e873137444d2 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -84,6 +84,7 @@ pub struct CodeSuggestion { /// ``` pub substitution_parts: Vec, pub msg: String, + pub show_code_when_inline: bool, } #[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable)] diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 1b6c3cf94e4f..b3291e9e0b59 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2807,9 +2807,9 @@ impl<'a> Parser<'a> { 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(cur_op_span, - "did you mean to end the statement here instead?", - ";".to_string()); + err.span_suggestion_short(cur_op_span, + "did you mean to use `;` here?", + ";".to_string()); } return Err(err); } diff --git a/src/test/ui/issue-22644.stderr b/src/test/ui/issue-22644.stderr index 144c5ef1ed6a..fd39a27a9f7f 100644 --- a/src/test/ui/issue-22644.stderr +++ b/src/test/ui/issue-22644.stderr @@ -100,5 +100,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 diff --git a/src/test/ui/suggestions/type-ascription-instead-of-statement-end.stderr b/src/test/ui/suggestions/type-ascription-instead-of-statement-end.stderr index e4cf78dbb2d9..550048c7b88f 100644 --- a/src/test/ui/suggestions/type-ascription-instead-of-statement-end.stderr +++ b/src/test/ui/suggestions/type-ascription-instead-of-statement-end.stderr @@ -2,7 +2,7 @@ error: expected type, found `0` --> $DIR/type-ascription-instead-of-statement-end.rs:15:5 | 14 | println!("test"): - | - help: did you mean to end the statement here instead? `;` + | - help: did you mean to use `;` here? 15 | 0; | ^ expecting a type here because of type ascription From e39bcecf79a6951f528b12b47ea671f9b0328bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 18 Jul 2017 11:41:21 -0700 Subject: [PATCH 3/3] Handle type ascription cases with a method call instead of a type --- src/librustc_resolve/lib.rs | 77 +++++++++++++++---- .../type-ascription-with-fn-call.rs | 18 +++++ .../type-ascription-with-fn-call.stderr | 13 ++++ 3 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/suggestions/type-ascription-with-fn-call.rs create mode 100644 src/test/ui/suggestions/type-ascription-with-fn-call.stderr diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 7754cd7366ec..d7316c75b2d2 100644 --- a/src/librustc_resolve/lib.rs +++ b/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, } 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(), } } @@ -2495,6 +2504,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 }; @@ -2550,6 +2560,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); @@ -3166,7 +3211,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); } diff --git a/src/test/ui/suggestions/type-ascription-with-fn-call.rs b/src/test/ui/suggestions/type-ascription-with-fn-call.rs new file mode 100644 index 000000000000..7c10bf98c840 --- /dev/null +++ b/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 or the MIT license +// , 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() {} diff --git a/src/test/ui/suggestions/type-ascription-with-fn-call.stderr b/src/test/ui/suggestions/type-ascription-with-fn-call.stderr new file mode 100644 index 000000000000..93c65c263dd1 --- /dev/null +++ b/src/test/ui/suggestions/type-ascription-with-fn-call.stderr @@ -0,0 +1,13 @@ +error[E0573]: expected type, found function `f` + --> $DIR/type-ascription-with-fn-call.rs:15:5 + | +14 | f() : + | - help: did you mean to use `;` here instead? +15 | f(); + | ^^^ + | | + | not a type + | expecting a type here because of type ascription + +error: aborting due to previous error +