From 3654ac68be4f1d8e9c3c4e45f3282dd78dc4bd73 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sat, 7 Jun 2014 23:02:48 -0700 Subject: [PATCH] Add visit_attribute to Visitor, use it for unused_attribute The lint was missing a *lot* of cases previously. --- src/librustc/middle/lang_items.rs | 8 ++-- src/librustc/middle/lint.rs | 23 ++++++----- src/libsyntax/visit.rs | 62 +++++++++++++++++++++------- src/test/compile-fail/unused-attr.rs | 58 ++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 31 deletions(-) create mode 100644 src/test/compile-fail/unused-attr.rs diff --git a/src/librustc/middle/lang_items.rs b/src/librustc/middle/lang_items.rs index 2e00c4d12ffb..748c29cd92c4 100644 --- a/src/librustc/middle/lang_items.rs +++ b/src/librustc/middle/lang_items.rs @@ -187,11 +187,11 @@ impl<'a> LanguageItemCollector<'a> { pub fn extract(attrs: &[ast::Attribute]) -> Option { 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 => {} + _ => {} } } diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index cae8436d6df7..f9ee65a2c499 100644 --- a/src/librustc/middle/lint.rs +++ b/src/librustc/middle/lint.rs @@ -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", @@ -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) { @@ -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, ())); @@ -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> { @@ -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(), diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index efa8c8e66640..906f0c16f396 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -128,6 +128,7 @@ pub trait Visitor { 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>(visitor: &mut V, @@ -142,7 +143,10 @@ pub fn walk_inlined_item>(visitor: &mut V, pub fn walk_crate>(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>(visitor: &mut V, module: &Mod, env: E) { @@ -158,7 +162,7 @@ pub fn walk_mod>(visitor: &mut V, module: &Mod, env: E) pub fn walk_view_item>(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 { @@ -178,6 +182,9 @@ pub fn walk_view_item>(visitor: &mut V, vi: &ViewItem, e } } } + for attr in vi.attrs.iter() { + visitor.visit_attribute(attr, env.clone()); + } } pub fn walk_local>(visitor: &mut V, local: &Local, env: E) { @@ -213,7 +220,7 @@ pub fn walk_item>(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), @@ -221,10 +228,10 @@ pub fn walk_item>(visitor: &mut V, item: &Item, env: E) 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() { @@ -236,11 +243,11 @@ pub fn walk_item>(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, @@ -263,7 +270,7 @@ pub fn walk_item>(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()); @@ -276,7 +283,10 @@ pub fn walk_item>(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()); } } @@ -310,9 +320,12 @@ pub fn walk_variant>(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>(_: &mut V, _: &Ty, _: E) { @@ -469,9 +482,13 @@ pub fn walk_foreign_item>(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()); } } @@ -525,7 +542,10 @@ pub fn walk_method_helper>(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>(visitor: &mut V, @@ -560,7 +580,10 @@ pub fn walk_ty_method>(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>(visitor: &mut V, @@ -596,7 +619,11 @@ pub fn walk_struct_field>(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>(visitor: &mut V, block: &Block, env: E) { @@ -784,5 +811,8 @@ pub fn walk_arm>(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()); + } } diff --git a/src/test/compile-fail/unused-attr.rs b/src/test/compile-fail/unused-attr.rs new file mode 100644 index 000000000000..750dcdf2c5f2 --- /dev/null +++ b/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 or the MIT license +// , 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() {}