auto merge of #14741 : sfackler/rust/visit-attr, r=alexcrichton

The lint was missing a *lot* of cases previously.

Author: bors

src/librustc/middle/lang_items.rs

@@ -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 => {}
+            _ => {}
         }
     }
 

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,14 +1914,17 @@ 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, ());
         })
     }
 
     // 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(),

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<InternedString> { (**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)]

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)]
 ");
     }
 

src/libsyntax/attr.rs

@@ -53,12 +53,6 @@ pub trait AttrMetaMethods {
     fn value_str(&self) -> Option<InternedString>;
     /// 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()
-    }
 }