Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't suggest using fields in a static method #33615

Merged
merged 1 commit into from
May 25, 2016
Merged

Don't suggest using fields in a static method #33615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 64 additions & 22 deletions src/librustc_resolve/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ use syntax::ast::{Arm, BindingMode, Block, Crate, Expr, ExprKind};
use syntax::ast::{FnDecl, ForeignItem, ForeignItemKind, Generics};
use syntax::ast::{Item, ItemKind, ImplItem, ImplItemKind};
use syntax::ast::{Local, Pat, PatKind, Path};
use syntax::ast::{PathSegment, PathParameters, TraitItemKind, TraitRef, Ty, TyKind};
use syntax::ast::{PathSegment, PathParameters, SelfKind, TraitItemKind, TraitRef, Ty, TyKind};

use std::collections::{HashMap, HashSet};
use std::cell::{Cell, RefCell};
Expand Down Expand Up @@ -148,7 +148,13 @@ enum ResolutionError<'a> {
/// error E0424: `self` is not available in a static method
SelfNotAvailableInStaticMethod,
/// error E0425: unresolved name
UnresolvedName(&'a str, &'a str, UnresolvedNameContext<'a>),
UnresolvedName {
path: &'a str,
message: &'a str,
context: UnresolvedNameContext<'a>,
is_static_method: bool,
is_field: bool
},
/// error E0426: use of undeclared label
UndeclaredLabel(&'a str),
/// error E0427: cannot use `ref` binding mode with ...
Expand Down Expand Up @@ -406,16 +412,21 @@ fn resolve_struct_error<'b, 'a: 'b, 'c>(resolver: &'b Resolver<'a>,
"`self` is not available in a static method. Maybe a `self` \
argument is missing?")
}
ResolutionError::UnresolvedName(path, msg, context) => {
ResolutionError::UnresolvedName { path, message: msg, context, is_static_method,
is_field } => {
let mut err = struct_span_err!(resolver.session,
span,
E0425,
"unresolved name `{}`{}",
path,
msg);

match context {
UnresolvedNameContext::Other => { } // no help available
UnresolvedNameContext::Other => {
if msg.is_empty() && is_static_method && is_field {
err.help("this is an associated function, you don't have access to \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this is only shown if there is a field by that name?

In that case, we should instead say something like "This is a static (non-self) method, and does not have access to fields. If you wished to access self.{fieldname} you should add a self parameter to the method".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, in this case, we should not suggest self.fieldname directly -- I don't think this change makes that happen?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this is only shown if there is a field by that name?

Or a method. So what follows your comment cannot really apply.

Also, in this case, we should not suggest self.fieldname directly -- I don't think this change makes that happen?

It does remove it.

this type's fields or methods");
}
}
UnresolvedNameContext::PathIsMod(parent) => {
err.help(&match parent.map(|parent| &parent.node) {
Some(&ExprKind::Field(_, ident)) => {
Expand Down Expand Up @@ -596,7 +607,7 @@ impl<'a, 'v> Visitor<'v> for Resolver<'a> {
}
FnKind::Method(_, sig, _) => {
self.visit_generics(&sig.generics);
MethodRibKind
MethodRibKind(sig.explicit_self.node == SelfKind::Static)
}
FnKind::Closure => ClosureRibKind(node_id),
};
Expand Down Expand Up @@ -666,7 +677,9 @@ enum RibKind<'a> {
// methods. Allow references to ty params that impl or trait
// binds. Disallow any other upvars (including other ty params that are
// upvars).
MethodRibKind,
//
// The boolean value represents the fact that this method is static or not.
MethodRibKind(bool),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(its good you put the comment here. You might consider the alternatives I outlined above, to make the types self-documenting.)


// We passed through an item scope. Disallow upvars.
ItemRibKind,
Expand Down Expand Up @@ -1095,7 +1108,13 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
Err(false) => {
let path_name = &format!("{}", path);
let error =
ResolutionError::UnresolvedName(path_name, "", UnresolvedNameContext::Other);
ResolutionError::UnresolvedName {
path: path_name,
message: "",
context: UnresolvedNameContext::Other,
is_static_method: false,
is_field: false
};
resolve_error(self, path.span, error);
Def::Err
}
Expand Down Expand Up @@ -1653,7 +1672,9 @@ impl<'a> Resolver<'a> {
let type_parameters =
HasTypeParameters(&sig.generics,
FnSpace,
MethodRibKind);
MethodRibKind(
sig.explicit_self.node ==
SelfKind::Static));
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_trait_item(this, trait_item)
});
Expand Down Expand Up @@ -1772,7 +1793,10 @@ impl<'a> Resolver<'a> {
self.value_ribs.pop();
}

fn resolve_function(&mut self, rib_kind: RibKind<'a>, declaration: &FnDecl, block: &Block) {
fn resolve_function(&mut self,
rib_kind: RibKind<'a>,
declaration: &FnDecl,
block: &Block) {
// Create a value rib for the function.
self.value_ribs.push(Rib::new(rib_kind));

Expand Down Expand Up @@ -1979,7 +2003,9 @@ impl<'a> Resolver<'a> {
let type_parameters =
HasTypeParameters(&sig.generics,
FnSpace,
MethodRibKind);
MethodRibKind(
sig.explicit_self.node ==
SelfKind::Static));
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_impl_item(this, impl_item);
});
Expand Down Expand Up @@ -2673,7 +2699,7 @@ impl<'a> Resolver<'a> {
def = Def::Upvar(node_def_id, node_id, depth, function_id);
seen.insert(node_id, depth);
}
ItemRibKind | MethodRibKind => {
ItemRibKind | MethodRibKind(_) => {
// This was an attempt to access an upvar inside a
// named function item. This is not allowed, so we
// report an error.
Expand All @@ -2695,7 +2721,7 @@ impl<'a> Resolver<'a> {
Def::TyParam(..) | Def::SelfTy(..) => {
for rib in ribs {
match rib.kind {
NormalRibKind | MethodRibKind | ClosureRibKind(..) |
NormalRibKind | MethodRibKind(_) | ClosureRibKind(..) |
ModuleRibKind(..) => {
// Nothing to do. Continue.
}
Expand Down Expand Up @@ -2988,9 +3014,13 @@ impl<'a> Resolver<'a> {
// `resolve_path` already reported the error
} else {
let mut method_scope = false;
let mut is_static = false;
self.value_ribs.iter().rev().all(|rib| {
method_scope = match rib.kind {
MethodRibKind => true,
MethodRibKind(is_static_) => {
is_static = is_static_;
true
}
ItemRibKind | ConstantItemRibKind => false,
_ => return true, // Keep advancing
};
Expand All @@ -3004,22 +3034,29 @@ impl<'a> Resolver<'a> {
ResolutionError::SelfNotAvailableInStaticMethod);
} else {
let last_name = path.segments.last().unwrap().identifier.name;
let mut msg = match self.find_fallback_in_self_type(last_name) {
let (mut msg, is_field) =
match self.find_fallback_in_self_type(last_name) {
NoSuggestion => {
// limit search to 5 to reduce the number
// of stupid suggestions
match self.find_best_match(&path_name) {
(match self.find_best_match(&path_name) {
SuggestionType::Macro(s) => {
format!("the macro `{}`", s)
}
SuggestionType::Function(s) => format!("`{}`", s),
SuggestionType::NotFound => "".to_string(),
}
}, false)
}
Field => {
(if is_static && method_scope {
"".to_string()
} else {
format!("`self.{}`", path_name)
}, true)
}
Field => format!("`self.{}`", path_name),
TraitItem => format!("to call `self.{}`", path_name),
TraitItem => (format!("to call `self.{}`", path_name), false),
TraitMethod(path_str) =>
format!("to call `{}::{}`", path_str, path_name),
(format!("to call `{}::{}`", path_str, path_name), false),
};

let mut context = UnresolvedNameContext::Other;
Expand All @@ -3044,8 +3081,13 @@ impl<'a> Resolver<'a> {

resolve_error(self,
expr.span,
ResolutionError::UnresolvedName(
&path_name, &msg, context));
ResolutionError::UnresolvedName {
path: &path_name,
message: &msg,
context: context,
is_static_method: method_scope && is_static,
is_field: is_field,
});
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/test/compile-fail/issue-2356.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ impl MaybeDog {
impl Groom for cat {
fn shave(other: usize) {
whiskers -= other;
//~^ ERROR: unresolved name `whiskers`. Did you mean `self.whiskers`?
//~^ ERROR: unresolved name `whiskers`
//~| HELP this is an associated function
shave(4);
//~^ ERROR: unresolved name `shave`. Did you mean to call `Groom::shave`?
purr();
Expand Down Expand Up @@ -77,7 +78,8 @@ impl cat {

pub fn grow_older(other:usize) {
whiskers = 4;
//~^ ERROR: unresolved name `whiskers`. Did you mean `self.whiskers`?
//~^ ERROR: unresolved name `whiskers`
//~| HELP this is an associated function
purr_louder();
//~^ ERROR: unresolved name `purr_louder`
}
Expand All @@ -86,5 +88,6 @@ impl cat {
fn main() {
self += 1;
//~^ ERROR: unresolved name `self`
//~| HELP: Module
// it's a bug if this suggests a missing `self` as we're not in a method
}
24 changes: 24 additions & 0 deletions src/test/compile-fail/unresolved_static_type_field.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2016 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.

fn f(_: bool) {}

struct Foo {
cx: bool,
}

impl Foo {
fn bar() {
f(cx); //~ ERROR E0425
//~| HELP this is an associated function
}
}

fn main() {}