Skip to content

Commit

Permalink
Add visit_attribute to Visitor, use it for unused_attribute
Browse files Browse the repository at this point in the history
The lint was missing a *lot* of cases previously.
  • Loading branch information
sfackler committed Jun 8, 2014
1 parent bbd448a commit 3654ac6
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 31 deletions.
8 changes: 4 additions & 4 deletions src/librustc/middle/lang_items.rs
Expand Up @@ -187,11 +187,11 @@ impl<'a> LanguageItemCollector<'a> {

pub fn extract(attrs: &[ast::Attribute]) -> Option<InternedString> {
for attribute in attrs.iter() {
match attribute.name_str_pair() {
Some((ref key, ref value)) if key.equiv(&("lang")) => {
return Some((*value).clone());
match attribute.value_str() {
Some(ref value) if attribute.check_name("lang") => {
return Some(value.clone());
}
Some(..) | None => {}
_ => {}
}
}

Expand Down
23 changes: 12 additions & 11 deletions src/librustc/middle/lint.rs
Expand Up @@ -1183,7 +1183,7 @@ fn check_attrs_usage(cx: &Context, attrs: &[ast::Attribute]) {
}
}

fn check_unused_attribute(cx: &Context, attrs: &[ast::Attribute]) {
fn check_unused_attribute(cx: &Context, attr: &ast::Attribute) {
static ATTRIBUTE_WHITELIST: &'static [&'static str] = &'static [
// FIXME: #14408 whitelist docs since rustdoc looks at them
"doc",
Expand Down Expand Up @@ -1218,17 +1218,16 @@ fn check_unused_attribute(cx: &Context, attrs: &[ast::Attribute]) {
"stable",
"unstable",
];
for attr in attrs.iter() {
for &name in ATTRIBUTE_WHITELIST.iter() {
if attr.check_name(name) {
break;
}
}

if !attr::is_used(attr) {
cx.span_lint(UnusedAttribute, attr.span, "unused attribute");
for &name in ATTRIBUTE_WHITELIST.iter() {
if attr.check_name(name) {
break;
}
}

if !attr::is_used(attr) {
cx.span_lint(UnusedAttribute, attr.span, "unused attribute");
}
}

fn check_heap_expr(cx: &Context, e: &ast::Expr) {
Expand Down Expand Up @@ -1836,7 +1835,6 @@ impl<'a> Visitor<()> for Context<'a> {
check_heap_item(cx, it);
check_missing_doc_item(cx, it);
check_attrs_usage(cx, it.attrs.as_slice());
check_unused_attribute(cx, it.attrs.as_slice());
check_raw_ptr_deriving(cx, it);

cx.visit_ids(|v| v.visit_item(it, ()));
Expand Down Expand Up @@ -2003,6 +2001,10 @@ impl<'a> Visitor<()> for Context<'a> {

// FIXME(#10894) should continue recursing
fn visit_ty(&mut self, _t: &ast::Ty, _: ()) {}

fn visit_attribute(&mut self, attr: &ast::Attribute, _: ()) {
check_unused_attribute(self, attr);
}
}

impl<'a> IdVisitingOperation for Context<'a> {
Expand Down Expand Up @@ -2054,7 +2056,6 @@ pub fn check_crate(tcx: &ty::ctxt,
check_crate_attrs_usage(cx, krate.attrs.as_slice());
// since the root module isn't visited as an item (because it isn't an item), warn for it
// here.
check_unused_attribute(cx, krate.attrs.as_slice());
check_missing_doc_attrs(cx,
None,
krate.attrs.as_slice(),
Expand Down
62 changes: 46 additions & 16 deletions src/libsyntax/visit.rs
Expand Up @@ -128,6 +128,7 @@ pub trait Visitor<E: Clone> {
fn visit_path(&mut self, path: &Path, _id: ast::NodeId, e: E) {
walk_path(self, path, e)
}
fn visit_attribute(&mut self, _attr: &Attribute, _e: E) {}
}

pub fn walk_inlined_item<E: Clone, V: Visitor<E>>(visitor: &mut V,
Expand All @@ -142,7 +143,10 @@ pub fn walk_inlined_item<E: Clone, V: Visitor<E>>(visitor: &mut V,


pub fn walk_crate<E: Clone, V: Visitor<E>>(visitor: &mut V, krate: &Crate, env: E) {
visitor.visit_mod(&krate.module, krate.span, CRATE_NODE_ID, env)
visitor.visit_mod(&krate.module, krate.span, CRATE_NODE_ID, env.clone());
for attr in krate.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}

pub fn walk_mod<E: Clone, V: Visitor<E>>(visitor: &mut V, module: &Mod, env: E) {
Expand All @@ -158,7 +162,7 @@ pub fn walk_mod<E: Clone, V: Visitor<E>>(visitor: &mut V, module: &Mod, env: E)
pub fn walk_view_item<E: Clone, V: Visitor<E>>(visitor: &mut V, vi: &ViewItem, env: E) {
match vi.node {
ViewItemExternCrate(name, _, _) => {
visitor.visit_ident(vi.span, name, env)
visitor.visit_ident(vi.span, name, env.clone())
}
ViewItemUse(ref vp) => {
match vp.node {
Expand All @@ -178,6 +182,9 @@ pub fn walk_view_item<E: Clone, V: Visitor<E>>(visitor: &mut V, vi: &ViewItem, e
}
}
}
for attr in vi.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}

pub fn walk_local<E: Clone, V: Visitor<E>>(visitor: &mut V, local: &Local, env: E) {
Expand Down Expand Up @@ -213,18 +220,18 @@ pub fn walk_item<E: Clone, V: Visitor<E>>(visitor: &mut V, item: &Item, env: E)
match item.node {
ItemStatic(typ, _, expr) => {
visitor.visit_ty(typ, env.clone());
visitor.visit_expr(expr, env);
visitor.visit_expr(expr, env.clone());
}
ItemFn(declaration, fn_style, abi, ref generics, body) => {
visitor.visit_fn(&FkItemFn(item.ident, generics, fn_style, abi),
declaration,
body,
item.span,
item.id,
env)
env.clone())
}
ItemMod(ref module) => {
visitor.visit_mod(module, item.span, item.id, env)
visitor.visit_mod(module, item.span, item.id, env.clone())
}
ItemForeignMod(ref foreign_module) => {
for view_item in foreign_module.view_items.iter() {
Expand All @@ -236,11 +243,11 @@ pub fn walk_item<E: Clone, V: Visitor<E>>(visitor: &mut V, item: &Item, env: E)
}
ItemTy(typ, ref type_parameters) => {
visitor.visit_ty(typ, env.clone());
visitor.visit_generics(type_parameters, env)
visitor.visit_generics(type_parameters, env.clone())
}
ItemEnum(ref enum_definition, ref type_parameters) => {
visitor.visit_generics(type_parameters, env.clone());
walk_enum_def(visitor, enum_definition, type_parameters, env)
walk_enum_def(visitor, enum_definition, type_parameters, env.clone())
}
ItemImpl(ref type_parameters,
ref trait_reference,
Expand All @@ -263,7 +270,7 @@ pub fn walk_item<E: Clone, V: Visitor<E>>(visitor: &mut V, item: &Item, env: E)
item.ident,
generics,
item.id,
env)
env.clone())
}
ItemTrait(ref generics, _, ref trait_paths, ref methods) => {
visitor.visit_generics(generics, env.clone());
Expand All @@ -276,7 +283,10 @@ pub fn walk_item<E: Clone, V: Visitor<E>>(visitor: &mut V, item: &Item, env: E)
visitor.visit_trait_method(method, env.clone())
}
}
ItemMac(ref macro) => visitor.visit_mac(macro, env),
ItemMac(ref macro) => visitor.visit_mac(macro, env.clone()),
}
for attr in item.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}

Expand Down Expand Up @@ -310,9 +320,12 @@ pub fn walk_variant<E: Clone, V: Visitor<E>>(visitor: &mut V,
}
}
match variant.node.disr_expr {
Some(expr) => visitor.visit_expr(expr, env),
Some(expr) => visitor.visit_expr(expr, env.clone()),
None => ()
}
for attr in variant.node.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}

pub fn skip_ty<E, V: Visitor<E>>(_: &mut V, _: &Ty, _: E) {
Expand Down Expand Up @@ -469,9 +482,13 @@ pub fn walk_foreign_item<E: Clone, V: Visitor<E>>(visitor: &mut V,
match foreign_item.node {
ForeignItemFn(function_declaration, ref generics) => {
walk_fn_decl(visitor, function_declaration, env.clone());
visitor.visit_generics(generics, env)
visitor.visit_generics(generics, env.clone())
}
ForeignItemStatic(typ, _) => visitor.visit_ty(typ, env),
ForeignItemStatic(typ, _) => visitor.visit_ty(typ, env.clone()),
}

for attr in foreign_item.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}

Expand Down Expand Up @@ -525,7 +542,10 @@ pub fn walk_method_helper<E: Clone, V: Visitor<E>>(visitor: &mut V,
method.body,
method.span,
method.id,
env)
env.clone());
for attr in method.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}

pub fn walk_fn<E: Clone, V: Visitor<E>>(visitor: &mut V,
Expand Down Expand Up @@ -560,7 +580,10 @@ pub fn walk_ty_method<E: Clone, V: Visitor<E>>(visitor: &mut V,
visitor.visit_ty(argument_type.ty, env.clone())
}
visitor.visit_generics(&method_type.generics, env.clone());
visitor.visit_ty(method_type.decl.output, env);
visitor.visit_ty(method_type.decl.output, env.clone());
for attr in method_type.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}

pub fn walk_trait_method<E: Clone, V: Visitor<E>>(visitor: &mut V,
Expand Down Expand Up @@ -596,7 +619,11 @@ pub fn walk_struct_field<E: Clone, V: Visitor<E>>(visitor: &mut V,
_ => {}
}

visitor.visit_ty(struct_field.node.ty, env)
visitor.visit_ty(struct_field.node.ty, env.clone());

for attr in struct_field.node.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}

pub fn walk_block<E: Clone, V: Visitor<E>>(visitor: &mut V, block: &Block, env: E) {
Expand Down Expand Up @@ -784,5 +811,8 @@ pub fn walk_arm<E: Clone, V: Visitor<E>>(visitor: &mut V, arm: &Arm, env: E) {
visitor.visit_pat(*pattern, env.clone())
}
walk_expr_opt(visitor, arm.guard, env.clone());
visitor.visit_expr(arm.body, env)
visitor.visit_expr(arm.body, env.clone());
for attr in arm.attrs.iter() {
visitor.visit_attribute(attr, env.clone());
}
}
58 changes: 58 additions & 0 deletions src/test/compile-fail/unused-attr.rs
@@ -0,0 +1,58 @@
// Copyright 2014 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(unused_attribute)]
#![allow(attribute_usage, dead_code, unused_imports)]

#![foo] //~ ERROR unused attribute

#[foo] //~ ERROR unused attribute
extern crate std;

#[foo] //~ ERROR unused attribute
use std::collections;

#[foo] //~ ERROR unused attribute
extern "C" {
#[foo] //~ ERROR unused attribute
fn foo();
}

#[foo] //~ ERROR unused attribute
mod foo {
#[foo] //~ ERROR unused attribute
pub enum Foo {
#[foo] //~ ERROR unused attribute
Bar,
}
}

#[foo] //~ ERROR unused attribute
fn bar(f: foo::Foo) {
match f {
#[foo] //~ ERROR unused attribute
foo::Bar => {}
}
}

#[foo] //~ ERROR unused attribute
struct Foo {
#[foo] //~ ERROR unused attribute
a: int
}

#[foo] //~ ERROR unused attribute
trait Baz {
#[foo] //~ ERROR unused attribute
fn blah();
#[foo] //~ ERROR unused attribute
fn blah2() {}
}

fn main() {}

0 comments on commit 3654ac6

Please sign in to comment.