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.

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

Add a lint to detect unnecessarily qualified names #7706

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 9 additions & 5 deletions src/librustc/middle/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ use syntax::{ast, visit, ast_util};
pub enum lint {
ctypes,
unused_imports,
unnecessary_qualification,
while_true,
path_statement,
implicit_copies,
Expand Down Expand Up @@ -148,6 +149,13 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
default: warn
}),

("unnecessary_qualification",
LintSpec {
lint: unnecessary_qualification,
desc: "detects unnecessarily qualified names",
default: allow
}),

("while_true",
LintSpec {
lint: while_true,
Expand Down Expand Up @@ -557,11 +565,7 @@ fn item_stopping_visitor<E: Copy>(outer: visit::vt<E>) -> visit::vt<E> {
_ => (outer.visit_fn)(fk, fd, b, s, id, (e, v))
}
},
.. **(ty_stopping_visitor(outer))})
}

fn ty_stopping_visitor<E>(v: visit::vt<E>) -> visit::vt<E> {
visit::mk_vt(@visit::Visitor {visit_ty: |_t, (_e, _v)| { },.. **v})
.. **outer})
}

fn lint_while_true() -> visit::vt<@mut Context> {
Expand Down
54 changes: 33 additions & 21 deletions src/librustc/middle/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use metadata::csearch::get_type_name_if_impl;
use metadata::cstore::find_extern_mod_stmt_cnum;
use metadata::decoder::{def_like, dl_def, dl_field, dl_impl};
use middle::lang_items::LanguageItems;
use middle::lint::unused_imports;
use middle::lint::{unnecessary_qualification, unused_imports};
use middle::pat_util::pat_bindings;

use syntax::ast::*;
Expand Down Expand Up @@ -3561,7 +3561,7 @@ impl Resolver {

// Resolve derived traits.
for traits.iter().advance |trt| {
self.resolve_trait_reference(trt, visitor, TraitDerivation);
self.resolve_trait_reference(item.id, trt, visitor, TraitDerivation);
}

for (*methods).iter().advance |method| {
Expand Down Expand Up @@ -3802,27 +3802,29 @@ impl Resolver {
visitor: ResolveVisitor) {
for type_parameters.iter().advance |type_parameter| {
for type_parameter.bounds.iter().advance |bound| {
self.resolve_type_parameter_bound(bound, visitor);
self.resolve_type_parameter_bound(type_parameter.id, bound, visitor);
}
}
}

pub fn resolve_type_parameter_bound(@mut self,
id: node_id,
type_parameter_bound: &TyParamBound,
visitor: ResolveVisitor) {
match *type_parameter_bound {
TraitTyParamBound(ref tref) => {
self.resolve_trait_reference(tref, visitor, TraitBoundingTypeParameter)
self.resolve_trait_reference(id, tref, visitor, TraitBoundingTypeParameter)
}
RegionTyParamBound => {}
}
}

pub fn resolve_trait_reference(@mut self,
id: node_id,
trait_reference: &trait_ref,
visitor: ResolveVisitor,
reference_type: TraitReferenceType) {
match self.resolve_path(&trait_reference.path, TypeNS, true, visitor) {
match self.resolve_path(id, &trait_reference.path, TypeNS, true, visitor) {
None => {
let path_str = self.idents_to_str(trait_reference.path.idents);

Expand Down Expand Up @@ -3930,7 +3932,8 @@ impl Resolver {
let original_trait_refs;
match opt_trait_reference {
&Some(ref trait_reference) => {
self.resolve_trait_reference(trait_reference, visitor, TraitImplementation);
self.resolve_trait_reference(id, trait_reference, visitor,
TraitImplementation);

// Record the current set of trait references.
let mut new_trait_refs = ~[];
Expand Down Expand Up @@ -4142,7 +4145,7 @@ impl Resolver {

match result_def {
None => {
match self.resolve_path(path, TypeNS, true, visitor) {
match self.resolve_path(ty.id, path, TypeNS, true, visitor) {
Some(def) => {
debug!("(resolving type) resolved `%s` to \
type %?",
Expand Down Expand Up @@ -4179,15 +4182,15 @@ impl Resolver {

do bounds.map |bound_vec| {
for bound_vec.iter().advance |bound| {
self.resolve_type_parameter_bound(bound, visitor);
self.resolve_type_parameter_bound(ty.id, bound, visitor);
}
};
}

ty_closure(c) => {
do c.bounds.map |bounds| {
for bounds.iter().advance |bound| {
self.resolve_type_parameter_bound(bound, visitor);
self.resolve_type_parameter_bound(ty.id, bound, visitor);
}
};
visit_ty(ty, ((), visitor));
Expand Down Expand Up @@ -4340,7 +4343,7 @@ impl Resolver {

pat_ident(binding_mode, ref path, _) => {
// This must be an enum variant, struct, or constant.
match self.resolve_path(path, ValueNS, false, visitor) {
match self.resolve_path(pat_id, path, ValueNS, false, visitor) {
Some(def @ def_variant(*)) |
Some(def @ def_struct(*)) => {
self.record_def(pattern.id, def);
Expand Down Expand Up @@ -4373,7 +4376,7 @@ impl Resolver {

pat_enum(ref path, _) => {
// This must be an enum variant, struct or const.
match self.resolve_path(path, ValueNS, false, visitor) {
match self.resolve_path(pat_id, path, ValueNS, false, visitor) {
Some(def @ def_fn(*)) |
Some(def @ def_variant(*)) |
Some(def @ def_struct(*)) |
Expand Down Expand Up @@ -4410,7 +4413,7 @@ impl Resolver {
}

pat_struct(ref path, _, _) => {
match self.resolve_path(path, TypeNS, false, visitor) {
match self.resolve_path(pat_id, path, TypeNS, false, visitor) {
Some(def_ty(class_id))
if self.structs.contains(&class_id) => {
let class_def = def_struct(class_id);
Expand Down Expand Up @@ -4484,6 +4487,7 @@ impl Resolver {
/// If `check_ribs` is true, checks the local definitions first; i.e.
/// doesn't skip straight to the containing module.
pub fn resolve_path(@mut self,
id: node_id,
path: &Path,
namespace: Namespace,
check_ribs: bool,
Expand All @@ -4500,16 +4504,24 @@ impl Resolver {
namespace);
}

let unqualified_def = self.resolve_identifier(
*path.idents.last(), namespace, check_ribs, path.span);

if path.idents.len() > 1 {
return self.resolve_module_relative_path(path,
self.xray_context,
namespace);
let def = self.resolve_module_relative_path(
path, self.xray_context, namespace);
match (def, unqualified_def) {
(Some(d), Some(ud)) if d == ud => {
self.session.add_lint(unnecessary_qualification,
id, path.span,
~"unnecessary qualification");
}
_ => ()
}
return def;
}

return self.resolve_identifier(*path.idents.last(),
namespace,
check_ribs,
path.span);
return unqualified_def;
}

pub fn resolve_identifier(@mut self,
Expand Down Expand Up @@ -4920,7 +4932,7 @@ impl Resolver {
// This is a local path in the value namespace. Walk through
// scopes looking for it.

match self.resolve_path(path, ValueNS, true, visitor) {
match self.resolve_path(expr.id, path, ValueNS, true, visitor) {
Some(def) => {
// Write the result into the def map.
debug!("(resolving expr) resolved `%s`",
Expand Down Expand Up @@ -4987,7 +4999,7 @@ impl Resolver {

expr_struct(ref path, _, _) => {
// Resolve the path to the structure it goes to.
match self.resolve_path(path, TypeNS, false, visitor) {
match self.resolve_path(expr.id, path, TypeNS, false, visitor) {
Some(def_ty(class_id)) | Some(def_struct(class_id))
if self.structs.contains(&class_id) => {
let class_def = def_struct(class_id);
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ pub fn id_visitor<T: Copy>(vfn: @fn(node_id, T)) -> visit::vt<T> {
},

visit_ty: |ty, (t, vt)| {
vfn(ty.id, copy t);
match ty.node {
ty_path(_, _, id) => vfn(id, copy t),
_ => { /* fall through */ }
Expand Down
20 changes: 20 additions & 0 deletions src/test/compile-fail/lint-qualification.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2013 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.

#[deny(unnecessary_qualification)];

mod foo {
pub fn bar() {}
}

fn main() {
use foo::bar;
foo::bar(); //~ ERROR: unnecessary qualification
}