Skip to content

Add visit_attribute to Visitor, use it for unused_attribute #14741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 8, 2014
Merged

Add visit_attribute to Visitor, use it for unused_attribute #14741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3654ac6
Add visit_attribute to Visitor, use it for unused_attribute
sfackler Jun 8, 2014
42a18bd
Remove the attribute_usage lint
sfackler Jun 8, 2014
862cd65
Add back hint for crate level attributes
sfackler Jun 8, 2014
9452cd2
Remove unused name_str_pair method
sfackler Jun 8, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/librustc/middle/lang_items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {}
_ => {}
}
}

Expand Down
148 changes: 34 additions & 114 deletions src/librustc/middle/lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ pub enum Lint {
TypeOverflow,
UnusedUnsafe,
UnsafeBlock,
AttributeUsage,
UnusedAttribute,
UnknownFeatures,
UnknownCrateType,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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, ()));
Expand All @@ -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, ());
Expand Down Expand Up @@ -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),
Expand All @@ -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, ());
Expand All @@ -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, ());
})
Expand All @@ -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> {
Expand Down Expand Up @@ -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(),
Expand Down
6 changes: 0 additions & 6 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
");
}

Expand Down
16 changes: 0 additions & 16 deletions src/libsyntax/attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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()
}
}


Expand Down
Loading