auto merge of #10316 : klutzy/rust/attr-lint, r=cmr

This patchset makes warning if crate-level attribute is used at other places, obsolete attributed is used, or unknown attribute is used, since they are usually from mistakes.

Closes #3348

Author: bors

src/etc/extract-tests.py

@@ -63,6 +63,7 @@
 #[ allow(unused_variable) ];\n
 #[ allow(dead_assignment) ];\n
 #[ allow(unused_mut) ];\n
+#[ allow(attribute_usage) ];\n
 #[ feature(macro_rules, globs, struct_variant, managed_boxes) ];\n
 """ + block
             if xfail:

src/librustc/middle/lint.rs

@@ -76,6 +76,7 @@ pub enum lint {
     type_overflow,
     unused_unsafe,
     unsafe_block,
+    attribute_usage,
 
     managed_heap_memory,
     owned_heap_memory,
@@ -244,6 +245,13 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
         default: allow
     }),
 
+    ("attribute_usage",
+     LintSpec {
+        lint: attribute_usage,
+        desc: "detects bad use of attributes",
+        default: warn
+    }),
+
     ("unused_variable",
      LintSpec {
         lint: unused_variable,
@@ -790,6 +798,83 @@ fn check_heap_item(cx: &Context, it: &ast::item) {
     }
 }
 
+static crate_attrs: &'static [&'static str] = &[
+    "crate_type", "link", "feature", "no_uv", "no_main", "no_std",
+    "desc", "comment", "license", "copyright", // not used in rustc now
+];
+
+
+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
+    "allow", "deny", "forbid", "warn", // lint options
+    "deprecated", "experimental", "unstable", "stable", "locked", "frozen", //item stability
+    "crate_map", "cfg", "doc", "export_name", "link_section", "no_freeze",
+    "no_mangle", "no_send", "static_assert", "unsafe_no_drop_flag",
+    "packed", "simd", "repr", "deriving", "unsafe_destructor",
+
+    //mod-level
+    "path", "link_name", "link_args", "nolink", "macro_escape", "no_implicit_prelude",
+
+    // fn-level
+    "test", "bench", "should_fail", "ignore", "inline", "lang", "main", "start",
+    "no_split_stack", "cold",
+
+    // 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(attribute_usage, attr.span, "unknown crate attribute");
+        }
+    }
+}
+
+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 semicolon at end",
+                    ast::AttrInner => "crate-level attribute should be in the root module",
+                };
+                cx.span_lint(attribute_usage, attr.span, msg);
+                return;
+            }
+        }
+
+        for &(obs_attr, obs_alter) in obsolete_attrs.iter() {
+            if name.equiv(&obs_attr) {
+                cx.span_lint(attribute_usage, attr.span,
+                             format!("obsolete attribute: {:s}", obs_alter));
+                return;
+            }
+        }
+
+        if !other_attrs.iter().any(|other_attr| { name.equiv(other_attr) }) {
+            cx.span_lint(attribute_usage, attr.span, "unknown attribute");
+        }
+    }
+}
+
 fn check_heap_expr(cx: &Context, e: &ast::Expr) {
     let ty = ty::expr_ty(cx.tcx, e);
     check_heap_type(cx, e.span, ty);
@@ -1110,6 +1195,7 @@ impl<'self> Visitor<()> for Context<'self> {
             check_item_non_uppercase_statics(cx, it);
             check_heap_item(cx, it);
             check_missing_doc_item(cx, it);
+            check_attrs_usage(cx, it.attrs);
 
             do cx.visit_ids |v| {
                 v.visit_item(it, ());
@@ -1119,6 +1205,20 @@ impl<'self> Visitor<()> for Context<'self> {
         }
     }
 
+    fn visit_foreign_item(&mut self, it: @ast::foreign_item, _: ()) {
+        do self.with_lint_attrs(it.attrs) |cx| {
+            check_attrs_usage(cx, it.attrs);
+            visit::walk_foreign_item(cx, it, ());
+        }
+    }
+
+    fn visit_view_item(&mut self, i: &ast::view_item, _: ()) {
+        do self.with_lint_attrs(i.attrs) |cx| {
+            check_attrs_usage(cx, i.attrs);
+            visit::walk_view_item(cx, i, ());
+        }
+    }
+
     fn visit_pat(&mut self, p: &ast::Pat, _: ()) {
         check_pat_non_uppercase_statics(self, p);
         check_unused_mut_pat(self, p);
@@ -1168,6 +1268,7 @@ impl<'self> Visitor<()> for Context<'self> {
             visit::fk_method(_, _, m) => {
                 do self.with_lint_attrs(m.attrs) |cx| {
                     check_missing_doc_method(cx, m);
+                    check_attrs_usage(cx, m.attrs);
 
                     do cx.visit_ids |v| {
                         v.visit_fn(fk, decl, body, span, id, ());
@@ -1179,9 +1280,11 @@ impl<'self> Visitor<()> for Context<'self> {
         }
     }
 
+
     fn visit_ty_method(&mut self, t: &ast::TypeMethod, _: ()) {
         do self.with_lint_attrs(t.attrs) |cx| {
             check_missing_doc_ty_method(cx, t);
+            check_attrs_usage(cx, t.attrs);
 
             visit::walk_ty_method(cx, t, ());
         }
@@ -1202,6 +1305,7 @@ impl<'self> Visitor<()> for Context<'self> {
     fn visit_struct_field(&mut self, s: @ast::struct_field, _: ()) {
         do self.with_lint_attrs(s.node.attrs) |cx| {
             check_missing_doc_struct_field(cx, s);
+            check_attrs_usage(cx, s.node.attrs);
 
             visit::walk_struct_field(cx, s, ());
         }
@@ -1210,6 +1314,7 @@ impl<'self> Visitor<()> for Context<'self> {
     fn visit_variant(&mut self, v: &ast::variant, g: &ast::Generics, _: ()) {
         do self.with_lint_attrs(v.node.attrs) |cx| {
             check_missing_doc_variant(cx, v);
+            check_attrs_usage(cx, v.node.attrs);
 
             visit::walk_variant(cx, v, g, ());
         }
@@ -1256,6 +1361,9 @@ pub fn check_crate(tcx: ty::ctxt,
             v.visited_outermost = true;
             visit::walk_crate(v, crate, ());
         }
+
+        check_crate_attrs_usage(cx, crate.attrs);
+
         visit::walk_crate(cx, crate, ());
     }
 

src/librustpkg/path_util.rs

@@ -461,7 +461,6 @@ pub fn versionize(p: &Path, v: &Version) -> Path {
 }
 
 #[cfg(target_os = "win32")]
-#[fixed_stack_segment]
 pub fn chmod_read_only(p: &Path) -> bool {
     unsafe {
         do p.with_c_str |src_buf| {
@@ -471,7 +470,6 @@ pub fn chmod_read_only(p: &Path) -> bool {
 }
 
 #[cfg(not(target_os = "win32"))]
-#[fixed_stack_segment]
 pub fn chmod_read_only(p: &Path) -> bool {
     unsafe {
         do p.with_c_str |src_buf| {

src/libstd/cell.rs

@@ -10,8 +10,6 @@
 
 //! Types dealing with dynamic mutability
 
-#[missing_doc];
-
 use prelude::*;
 use cast;
 use util::NonCopyable;

src/libstd/io/native/process.rs

@@ -365,7 +365,6 @@ fn spawn_process_os(prog: &str, args: &[~str],
     use libc::funcs::bsd44::getdtablesize;
 
     mod rustrt {
-        #[abi = "cdecl"]
         extern {
             pub fn rust_unset_sigprocmask();
         }

src/libstd/vec.rs

@@ -2704,7 +2704,7 @@ mod tests {
         assert_eq!(a.init(), &[11]);
     }
 
-    #[init]
+    #[test]
     #[should_fail]
     fn test_init_empty() {
         let a: ~[int] = ~[];
@@ -2719,7 +2719,7 @@ mod tests {
         assert_eq!(a.initn(2), &[11]);
     }
 
-    #[init]
+    #[test]
     #[should_fail]
     fn test_initn_empty() {
         let a: ~[int] = ~[];

src/libsyntax/ext/auto_encode.rs

@@ -143,7 +143,7 @@ pub enum MacResult {
 }
 
 pub enum SyntaxExtension {
-    // #[auto_encode] and such
+    // #[deriving] and such
     ItemDecorator(ItemDecorator),
 
     // Token-tree expanders
@@ -229,12 +229,6 @@ pub fn syntax_expander_table() -> SyntaxEnv {
     syntax_expanders.insert(intern(&"format_args"),
                             builtin_normal_tt_no_ctxt(
                                 ext::format::expand_args));
-    syntax_expanders.insert(
-        intern(&"auto_encode"),
-        @SE(ItemDecorator(ext::auto_encode::expand_auto_encode)));
-    syntax_expanders.insert(
-        intern(&"auto_decode"),
-        @SE(ItemDecorator(ext::auto_encode::expand_auto_decode)));
     syntax_expanders.insert(intern(&"env"),
                             builtin_normal_tt_no_ctxt(
                                     ext::env::expand_env));

src/libsyntax/ext/base.rs

@@ -83,7 +83,6 @@ pub mod ext {
     pub mod concat;
     pub mod concat_idents;
     pub mod log_syntax;
-    pub mod auto_encode;
     pub mod source_util;
 
     pub mod trace_macros;

src/libsyntax/lib.rs

@@ -10,7 +10,6 @@
 
 fn foo(_x: int) { }
 
-#[fixed_stack_segment]
 fn main() {
     let v: u64 = 5;
     let x = foo as extern "C" fn() -> int;

src/test/compile-fail/cast-to-bare-fn.rs

@@ -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.
+
+// When denying at the crate level, be sure to not get random warnings from the
+// injected intrinsics by the compiler.
+
+#[deny(attribute_usage)];
+
+mod a {
+    #[crate_type = "bin"]; //~ ERROR: crate-level attribute
+}
+
+#[crate_type = "bin"] fn main() {} //~ ERROR: crate-level attribute

src/test/compile-fail/lint-misplaced-attr.rs

@@ -8,8 +8,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#[auto_encode] //~ ERROR: `#[auto_encode]` is deprecated
-#[auto_decode] //~ ERROR: `#[auto_decode]` is deprecated
-struct A;
+// When denying at the crate level, be sure to not get random warnings from the
+// injected intrinsics by the compiler.
+
+#[deny(attribute_usage)];
+
+#[abi="stdcall"] extern {} //~ ERROR: obsolete attribute
+
+#[fixed_stack_segment] fn f() {} //~ ERROR: obsolete attribute
 
 fn main() {}

src/test/compile-fail/deprecated-auto-code.rs src/test/compile-fail/lint-obsolete-attr.rs

@@ -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.
+
+// When denying at the crate level, be sure to not get random warnings from the
+// injected intrinsics by the compiler.
+
+#[deny(attribute_usage)];
+
+#[mutable_doc]; //~ ERROR: unknown crate attribute
+
+#[dance] mod a {} //~ ERROR: unknown attribute
+
+#[dance] fn main() {} //~ ERROR: unknown attribute

src/test/compile-fail/lint-unknown-attr.rs

@@ -23,7 +23,6 @@ use std::str;
 struct A;
 struct B;
 
-#[fmt="foo"]
 impl fmt::Signed for A {
     fn fmt(_: &A, f: &mut fmt::Formatter) { f.buf.write("aloha".as_bytes()); }
 }