From 86f5275e49d9d6dc79dc5b5ce0a440f494a84d61 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 11 Dec 2015 00:00:17 +0100 Subject: [PATCH 1/3] Add note when item accessed from module via `m.i` rather than `m::i`. --- src/librustc_resolve/lib.rs | 69 ++++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index ddada1b513d..b4e2d6beb8b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -179,7 +179,7 @@ pub enum ResolutionError<'a> { /// error E0424: `self` is not available in a static method SelfNotAvailableInStaticMethod, /// error E0425: unresolved name - UnresolvedName(&'a str, &'a str), + UnresolvedName(&'a str, &'a str, UnresolvedNameContext), /// error E0426: use of undeclared label UndeclaredLabel(&'a str), /// error E0427: cannot use `ref` binding mode with ... @@ -202,6 +202,12 @@ pub enum ResolutionError<'a> { AttemptToUseNonConstantValueInConstant, } +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum UnresolvedNameContext { + PathIsMod(ast::NodeId), + Other, +} + fn resolve_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>, span: syntax::codemap::Span, resolution_error: ResolutionError<'b>) { @@ -402,13 +408,46 @@ fn resolve_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>, "`self` is not available in a static method. Maybe a `self` argument is \ missing?"); } - ResolutionError::UnresolvedName(path, name) => { + ResolutionError::UnresolvedName(path, msg, context) => { span_err!(resolver.session, span, E0425, "unresolved name `{}`{}", path, - name); + msg); + + match context { + UnresolvedNameContext::Other => {} // no help available + UnresolvedNameContext::PathIsMod(id) => { + let mut help_msg = String::new(); + let parent_id = resolver.ast_map.get_parent_node(id); + if let Some(hir_map::Node::NodeExpr(e)) = resolver.ast_map.find(parent_id) { + match e.node { + ExprField(_, ident) => { + help_msg = format!("To reference an item from the \ + `{module}` module, use \ + `{module}::{ident}`", + module = &*path, + ident = ident.node); + } + + ExprMethodCall(ident, _, _) => { + help_msg = format!("To call a function from the \ + `{module}` module, use \ + `{module}::{ident}(..)`", + module = &*path, + ident = ident.node); + } + + _ => {} // no help available + } + } + + if !help_msg.is_empty() { + resolver.session.fileline_help(span, &help_msg); + } + } + } } ResolutionError::UndeclaredLabel(name) => { span_err!(resolver.session, @@ -3509,13 +3548,33 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { format!("to call `{}::{}`", path_str, path_name), }; + let mut context = UnresolvedNameContext::Other; if !msg.is_empty() { - msg = format!(". Did you mean {}?", msg) + msg = format!(". Did you mean {}?", msg); + } else { + // we check if this a module and if so, we display a help + // message + let name_path = path.segments.iter() + .map(|seg| seg.identifier.name) + .collect::>(); + let current_module = self.current_module.clone(); + + match self.resolve_module_path(current_module, + &name_path[..], + UseLexicalScope, + expr.span, + PathSearch) { + Success(_) => { + context = UnresolvedNameContext::PathIsMod(expr.id); + }, + _ => {}, + }; } resolve_error(self, expr.span, - ResolutionError::UnresolvedName(&*path_name, &*msg)); + ResolutionError::UnresolvedName( + &*path_name, &*msg, context)); } } } From 694699503a346822ae8453e93c9f327d01fe6f55 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 16 Dec 2015 16:40:19 +0100 Subject: [PATCH 2/3] unit test for new error help. --- .../suggest-path-instead-of-mod-dot-item.rs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 src/test/compile-fail/suggest-path-instead-of-mod-dot-item.rs diff --git a/src/test/compile-fail/suggest-path-instead-of-mod-dot-item.rs b/src/test/compile-fail/suggest-path-instead-of-mod-dot-item.rs new file mode 100644 index 00000000000..ecf17fa84d7 --- /dev/null +++ b/src/test/compile-fail/suggest-path-instead-of-mod-dot-item.rs @@ -0,0 +1,60 @@ +// Copyright 2015 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. + +// Beginners write `mod.item` when they should write `mod::item`. +// This tests that we suggest the latter when we encounter the former. + +pub mod a { + pub const I: i32 = 1; + + pub fn f() -> i32 { 2 } + + pub mod b { + pub const J: i32 = 3; + + pub fn g() -> i32 { 4 } + } +} + +fn h1() -> i32 { + a.I + //~^ ERROR E0425 + //~| HELP To reference an item from the `a` module, use `a::I` +} + +fn h2() -> i32 { + a.g() + //~^ ERROR E0425 + //~| HELP To call a function from the `a` module, use `a::g(..)` +} + +fn h3() -> i32 { + a.b.J + //~^ ERROR E0425 + //~| HELP To reference an item from the `a` module, use `a::b` +} + +fn h4() -> i32 { + a::b.J + //~^ ERROR E0425 + //~| HELP To reference an item from the `a::b` module, use `a::b::J` +} + +fn h5() -> i32 { + a.b.f() + //~^ ERROR E0425 + //~| HELP To reference an item from the `a` module, use `a::b` +} + +fn h6() -> i32 { + a::b.f() + //~^ ERROR E0425 + //~| HELP To call a function from the `a::b` module, use `a::b::f(..)` +} From 04c05c7b01170a7ce3be58a6a2b4a437daf57dd6 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 18 Dec 2015 17:42:46 +0100 Subject: [PATCH 3/3] Added doc comments for new UnresolvedNameContext enum. --- src/librustc_resolve/lib.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index b4e2d6beb8b..3aeb08aa110 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -202,9 +202,18 @@ pub enum ResolutionError<'a> { AttemptToUseNonConstantValueInConstant, } +/// Context of where `ResolutionError::UnresolvedName` arose. #[derive(Clone, PartialEq, Eq, Debug)] pub enum UnresolvedNameContext { + /// `PathIsMod(id)` indicates that a given path, used in + /// expression context, actually resolved to a module rather than + /// a value. The `id` attached to the variant is the node id of + /// the erroneous path expression. PathIsMod(ast::NodeId), + + /// `Other` means we have no extra information about the context + /// of the unresolved name error. (Maybe we could eliminate all + /// such cases; but for now, this is an information-free default.) Other, }