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

The lint was missing a *lot* of cases previously.
This commit is contained in:
bors 2014-06-08 04:16:53 -07:00
commit 9239bb4960
10 changed files with 153 additions and 175 deletions

View File

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

View File

@ -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;
}
}
if !attr::is_used(attr) {
cx.span_lint(UnusedAttribute, attr.span, "unused attribute");
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 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(),

View File

@ -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)]

View File

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

View File

@ -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()
}
}

View File

@ -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,
@ -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) {
@ -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 {
@ -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) {
@ -213,7 +220,7 @@ 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),
@ -221,10 +228,10 @@ pub fn walk_item<E: Clone, V: Visitor<E>>(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<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,
@ -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());
@ -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());
}
}
@ -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) {
@ -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());
}
}
@ -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,
@ -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,
@ -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) {
@ -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());
}
}

View File

@ -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

View File

@ -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() {}

View File

@ -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

View File

@ -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(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() {}