diff --git a/src/librustc/middle/lang_items.rs b/src/librustc/middle/lang_items.rs index 2e00c4d12ffbf..748c29cd92c42 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 cae8436d6df70..9500f4d7ceeba 100644 --- a/src/librustc/middle/lint.rs +++ b/src/librustc/middle/lint.rs @@ -92,7 +92,6 @@ pub enum Lint { TypeOverflow, UnusedUnsafe, UnsafeBlock, - AttributeUsage, UnusedAttribute, UnknownFeatures, UnknownCrateType, @@ -294,13 +293,6 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[ default: Allow }), - ("attribute_usage", - LintSpec { - lint: AttributeUsage, - desc: "detects bad use of attributes", - default: Warn - }), - ("unused_attribute", LintSpec { lint: UnusedAttribute, @@ -1096,94 +1088,7 @@ fn check_raw_ptr_deriving(cx: &mut Context, item: &ast::Item) { } } -static crate_attrs: &'static [&'static str] = &[ - "crate_type", "feature", "no_start", "no_main", "no_std", "crate_id", - "desc", "comment", "license", "copyright", // not used in rustc now - "no_builtins", -]; - - -static obsolete_attrs: &'static [(&'static str, &'static str)] = &[ - ("abi", "Use `extern \"abi\" fn` instead"), - ("auto_encode", "Use `#[deriving(Encodable)]` instead"), - ("auto_decode", "Use `#[deriving(Decodable)]` instead"), - ("fast_ffi", "Remove it"), - ("fixed_stack_segment", "Remove it"), - ("rust_stack", "Remove it"), -]; - -static other_attrs: &'static [&'static str] = &[ - // item-level - "address_insignificant", // can be crate-level too - "thread_local", // for statics - "allow", "deny", "forbid", "warn", // lint options - "deprecated", "experimental", "unstable", "stable", "locked", "frozen", //item stability - "cfg", "doc", "export_name", "link_section", - "no_mangle", "static_assert", "unsafe_no_drop_flag", "packed", - "simd", "repr", "deriving", "unsafe_destructor", "link", "phase", - "macro_export", "must_use", "automatically_derived", - - //mod-level - "path", "link_name", "link_args", "macro_escape", "no_implicit_prelude", - - // fn-level - "test", "bench", "should_fail", "ignore", "inline", "lang", "main", "start", - "no_split_stack", "cold", "macro_registrar", "linkage", - - // internal attribute: bypass privacy inside items - "!resolve_unexported", -]; - -fn check_crate_attrs_usage(cx: &Context, attrs: &[ast::Attribute]) { - - for attr in attrs.iter() { - let name = attr.node.value.name(); - let mut iter = crate_attrs.iter().chain(other_attrs.iter()); - if !iter.any(|other_attr| { name.equiv(other_attr) }) { - cx.span_lint(AttributeUsage, attr.span, "unknown crate attribute"); - } - if name.equiv(&("link")) { - cx.tcx.sess.span_err(attr.span, - "obsolete crate `link` attribute"); - cx.tcx.sess.note("the link attribute has been superceded by the crate_id \ - attribute, which has the format `#[crate_id = \"name#version\"]`"); - } - } -} - -fn check_attrs_usage(cx: &Context, attrs: &[ast::Attribute]) { - // check if element has crate-level, obsolete, or any unknown attributes. - - for attr in attrs.iter() { - let name = attr.node.value.name(); - for crate_attr in crate_attrs.iter() { - if name.equiv(crate_attr) { - let msg = match attr.node.style { - ast::AttrOuter => "crate-level attribute should be an inner attribute: \ - add an exclamation mark: #![foo]", - ast::AttrInner => "crate-level attribute should be in the root module", - }; - cx.span_lint(AttributeUsage, attr.span, msg); - return; - } - } - - for &(obs_attr, obs_alter) in obsolete_attrs.iter() { - if name.equiv(&obs_attr) { - cx.span_lint(AttributeUsage, attr.span, - format!("obsolete attribute: {:s}", - obs_alter).as_slice()); - return; - } - } - - if !other_attrs.iter().any(|other_attr| { name.equiv(other_attr) }) { - cx.span_lint(AttributeUsage, attr.span, "unknown 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,15 +1123,37 @@ 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; - } + + static CRATE_ATTRS: &'static [&'static str] = &'static [ + "crate_type", + "feature", + "no_start", + "no_main", + "no_std", + "crate_id", + "desc", + "comment", + "license", + "copyright", + "no_builtins", + ]; + + 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"); + if !attr::is_used(attr) { + cx.span_lint(UnusedAttribute, attr.span, "unused attribute"); + if CRATE_ATTRS.contains(&attr.name().get()) { + let msg = match attr.node.style { + ast::AttrOuter => "crate-level attribute should be an inner \ + attribute: add an exclamation mark: #![foo]", + ast::AttrInner => "crate-level attribute should be in the \ + root module", + }; + cx.span_lint(UnusedAttribute, attr.span, msg); } } } @@ -1835,8 +1762,6 @@ impl<'a> Visitor<()> for Context<'a> { check_item_non_uppercase_statics(cx, it); 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, ())); @@ -1847,15 +1772,12 @@ impl<'a> Visitor<()> for Context<'a> { fn visit_foreign_item(&mut self, it: &ast::ForeignItem, _: ()) { self.with_lint_attrs(it.attrs.as_slice(), |cx| { - check_attrs_usage(cx, it.attrs.as_slice()); visit::walk_foreign_item(cx, it, ()); }) } fn visit_view_item(&mut self, i: &ast::ViewItem, _: ()) { self.with_lint_attrs(i.attrs.as_slice(), |cx| { - check_attrs_usage(cx, i.attrs.as_slice()); - cx.visit_ids(|v| v.visit_view_item(i, ())); visit::walk_view_item(cx, i, ()); @@ -1937,7 +1859,6 @@ impl<'a> Visitor<()> for Context<'a> { visit::FkMethod(ident, _, m) => { self.with_lint_attrs(m.attrs.as_slice(), |cx| { check_missing_doc_method(cx, m); - check_attrs_usage(cx, m.attrs.as_slice()); match method_context(cx, m) { PlainImpl => check_snake_case(cx, "method", ident, span), @@ -1962,7 +1883,6 @@ impl<'a> Visitor<()> for Context<'a> { fn visit_ty_method(&mut self, t: &ast::TypeMethod, _: ()) { self.with_lint_attrs(t.attrs.as_slice(), |cx| { check_missing_doc_ty_method(cx, t); - check_attrs_usage(cx, t.attrs.as_slice()); check_snake_case(cx, "trait method", t.ident, t.span); visit::walk_ty_method(cx, t, ()); @@ -1986,7 +1906,6 @@ impl<'a> Visitor<()> for Context<'a> { fn visit_struct_field(&mut self, s: &ast::StructField, _: ()) { self.with_lint_attrs(s.node.attrs.as_slice(), |cx| { check_missing_doc_struct_field(cx, s); - check_attrs_usage(cx, s.node.attrs.as_slice()); visit::walk_struct_field(cx, s, ()); }) @@ -1995,7 +1914,6 @@ impl<'a> Visitor<()> for Context<'a> { fn visit_variant(&mut self, v: &ast::Variant, g: &ast::Generics, _: ()) { self.with_lint_attrs(v.node.attrs.as_slice(), |cx| { check_missing_doc_variant(cx, v); - check_attrs_usage(cx, v.node.attrs.as_slice()); visit::walk_variant(cx, v, g, ()); }) @@ -2003,6 +1921,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> { @@ -2051,10 +1973,8 @@ pub fn check_crate(tcx: &ty::ctxt, visit::walk_crate(v, krate, ()); }); - 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/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 3cb5c663c4ea5..650cd749af69e 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -429,17 +429,11 @@ impl attr::AttrMetaMethods for Attribute { } } fn meta_item_list<'a>(&'a self) -> Option<&'a [@ast::MetaItem]> { None } - fn name_str_pair(&self) -> Option<(InternedString, InternedString)> { - None - } } impl<'a> attr::AttrMetaMethods for &'a Attribute { fn name(&self) -> InternedString { (**self).name() } fn value_str(&self) -> Option { (**self).value_str() } fn meta_item_list<'a>(&'a self) -> Option<&'a [@ast::MetaItem]> { None } - fn name_str_pair(&self) -> Option<(InternedString, InternedString)> { - None - } } #[deriving(Clone, Encodable, Decodable)] diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index bc1da5b629e13..745e29508d218 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -205,7 +205,7 @@ pub fn maketest(s: &str, cratename: Option<&str>, lints: bool) -> String { if lints { prog.push_str(r" #![deny(warnings)] -#![allow(unused_variable, dead_assignment, unused_mut, attribute_usage, dead_code)] +#![allow(unused_variable, dead_assignment, unused_mut, unused_attribute, dead_code)] "); } diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index a8f30c0514b80..6005513af110d 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -53,12 +53,6 @@ pub trait AttrMetaMethods { fn value_str(&self) -> Option; /// Gets a list of inner meta items from a list MetaItem type. fn meta_item_list<'a>(&'a self) -> Option<&'a [@MetaItem]>; - - /** - * If the meta item is a name-value type with a string value then returns - * a tuple containing the name and string value, otherwise `None` - */ - fn name_str_pair(&self) -> Option<(InternedString,InternedString)>; } impl AttrMetaMethods for Attribute { @@ -76,9 +70,6 @@ impl AttrMetaMethods for Attribute { fn meta_item_list<'a>(&'a self) -> Option<&'a [@MetaItem]> { self.node.value.meta_item_list() } - fn name_str_pair(&self) -> Option<(InternedString,InternedString)> { - self.meta().name_str_pair() - } } impl AttrMetaMethods for MetaItem { @@ -108,10 +99,6 @@ impl AttrMetaMethods for MetaItem { _ => None } } - - fn name_str_pair(&self) -> Option<(InternedString,InternedString)> { - self.value_str().map(|s| (self.name(), s)) - } } // Annoying, but required to get test_cfg to work @@ -121,9 +108,6 @@ impl AttrMetaMethods for @MetaItem { fn meta_item_list<'a>(&'a self) -> Option<&'a [@MetaItem]> { (**self).meta_item_list() } - fn name_str_pair(&self) -> Option<(InternedString,InternedString)> { - (**self).name_str_pair() - } } diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index efa8c8e66640f..906f0c16f3964 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/lint-misplaced-attr.rs b/src/test/compile-fail/lint-misplaced-attr.rs index f7db5c97aab11..dea712e976b35 100644 --- a/src/test/compile-fail/lint-misplaced-attr.rs +++ b/src/test/compile-fail/lint-misplaced-attr.rs @@ -11,13 +11,12 @@ // When denying at the crate level, be sure to not get random warnings from the // injected intrinsics by the compiler. -#![deny(attribute_usage)] #![deny(unused_attribute)] mod a { - #![crate_type = "bin"] //~ ERROR: crate-level attribute - //~^ ERROR: unused attribute + #![crate_type = "bin"] //~ ERROR unused attribute + //~^ ERROR should be in the root module } -#[crate_type = "bin"] fn main() {} //~ ERROR: crate-level attribute - //~^ ERROR: unused attribute +#[crate_type = "bin"] fn main() {} //~ ERROR unused attribute + //~^ ERROR should be an inner diff --git a/src/test/compile-fail/lint-obsolete-attr.rs b/src/test/compile-fail/lint-obsolete-attr.rs index 32058737ed302..6b46a0c19bddd 100644 --- a/src/test/compile-fail/lint-obsolete-attr.rs +++ b/src/test/compile-fail/lint-obsolete-attr.rs @@ -11,14 +11,11 @@ // When denying at the crate level, be sure to not get random warnings from the // injected intrinsics by the compiler. -#![deny(attribute_usage)] #![deny(unused_attribute)] #![allow(dead_code)] -#[abi="stdcall"] extern {} //~ ERROR: obsolete attribute - //~^ ERROR: unused attribute +#[abi="stdcall"] extern {} //~ ERROR unused attribute -#[fixed_stack_segment] fn f() {} //~ ERROR: obsolete attribute - //~^ ERROR: unused attribute +#[fixed_stack_segment] fn f() {} //~ ERROR unused attribute fn main() {} diff --git a/src/test/compile-fail/lint-unknown-attr.rs b/src/test/compile-fail/lint-unknown-attr.rs index 32c0722d1ac26..020ed80c0fbba 100644 --- a/src/test/compile-fail/lint-unknown-attr.rs +++ b/src/test/compile-fail/lint-unknown-attr.rs @@ -11,14 +11,10 @@ // When denying at the crate level, be sure to not get random warnings from the // injected intrinsics by the compiler. -#![deny(attribute_usage)] #![deny(unused_attribute)] -#![mutable_doc] //~ ERROR: unknown crate attribute - //~^ ERROR: unused attribute +#![mutable_doc] //~ ERROR unused attribute -#[dance] mod a {} //~ ERROR: unknown attribute - //~^ ERROR: unused attribute +#[dance] mod a {} //~ ERROR unused attribute -#[dance] fn main() {} //~ ERROR: unknown attribute - //~^ ERROR: unused attribute +#[dance] fn main() {} //~ ERROR unused attribute diff --git a/src/test/compile-fail/unused-attr.rs b/src/test/compile-fail/unused-attr.rs new file mode 100644 index 0000000000000..3e1e08c7b58dd --- /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(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() {}