Skip to content

Commit

Permalink
Add a lint for duplicated attributes.
Browse files Browse the repository at this point in the history
  • Loading branch information
Ethiraric committed Dec 15, 2021
1 parent 753e569 commit 2be94d4
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 4 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3678,6 +3678,7 @@ dependencies = [
"rustc_expand",
"rustc_feature",
"rustc_lexer",
"rustc_lint_defs",
"rustc_parse",
"rustc_parse_format",
"rustc_session",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ rustc_data_structures = { path = "../rustc_data_structures" }
rustc_errors = { path = "../rustc_errors" }
rustc_feature = { path = "../rustc_feature" }
rustc_lexer = { path = "../rustc_lexer" }
rustc_lint_defs = { path = "../rustc_lint_defs" }
rustc_parse = { path = "../rustc_parse" }
rustc_target = { path = "../rustc_target" }
rustc_session = { path = "../rustc_session" }
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_builtin_macros/src/cfg_eval.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::util::check_builtin_macro_attribute;
use crate::util::{check_builtin_macro_attribute, warn_on_duplicate_attribute};

use rustc_ast as ast;
use rustc_ast::mut_visit::MutVisitor;
Expand All @@ -25,6 +25,7 @@ crate fn expand(
annotatable: Annotatable,
) -> Vec<Annotatable> {
check_builtin_macro_attribute(ecx, meta_item, sym::cfg_eval);
warn_on_duplicate_attribute(&ecx, &annotatable, sym::cfg_eval);
vec![cfg_eval(ecx.sess, ecx.ecfg.features, annotatable)]
}

Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// The expansion from a test function to the appropriate test struct for libtest
/// Ideally, this code would be in libtest but for efficiency and error messages it lives here.
use crate::util::check_builtin_macro_attribute;
use crate::util::{check_builtin_macro_attribute, warn_on_duplicate_attribute};

use rustc_ast as ast;
use rustc_ast::attr;
Expand All @@ -27,6 +27,7 @@ pub fn expand_test_case(
anno_item: Annotatable,
) -> Vec<Annotatable> {
check_builtin_macro_attribute(ecx, meta_item, sym::test_case);
warn_on_duplicate_attribute(&ecx, &anno_item, sym::test_case);

if !ecx.ecfg.should_test {
return vec![];
Expand Down Expand Up @@ -55,6 +56,7 @@ pub fn expand_test(
item: Annotatable,
) -> Vec<Annotatable> {
check_builtin_macro_attribute(cx, meta_item, sym::test);
warn_on_duplicate_attribute(&cx, &item, sym::test);
expand_test_or_bench(cx, attr_sp, item, false)
}

Expand All @@ -65,6 +67,7 @@ pub fn expand_bench(
item: Annotatable,
) -> Vec<Annotatable> {
check_builtin_macro_attribute(cx, meta_item, sym::bench);
warn_on_duplicate_attribute(&cx, &item, sym::bench);
expand_test_or_bench(cx, attr_sp, item, true)
}

Expand Down
35 changes: 33 additions & 2 deletions compiler/rustc_builtin_macros/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc_ast::MetaItem;
use rustc_expand::base::ExtCtxt;
use rustc_ast::{Attribute, MetaItem};
use rustc_expand::base::{Annotatable, ExtCtxt};
use rustc_feature::AttributeTemplate;
use rustc_lint_defs::builtin::DUPLICATE_MACRO_ATTRIBUTES;
use rustc_parse::validate_attr;
use rustc_span::Symbol;

Expand All @@ -10,3 +11,33 @@ pub fn check_builtin_macro_attribute(ecx: &ExtCtxt<'_>, meta_item: &MetaItem, na
let attr = ecx.attribute(meta_item.clone());
validate_attr::check_builtin_attribute(&ecx.sess.parse_sess, &attr, name, template);
}

/// Emit a warning if the item is annotated with the given attribute. This is used to diagnose when
/// an attribute may have been mistakenly duplicated.
pub fn warn_on_duplicate_attribute(ecx: &ExtCtxt<'_>, item: &Annotatable, name: Symbol) {
let attrs: Option<&[Attribute]> = match item {
Annotatable::Item(item) => Some(&item.attrs),
Annotatable::TraitItem(item) => Some(&item.attrs),
Annotatable::ImplItem(item) => Some(&item.attrs),
Annotatable::ForeignItem(item) => Some(&item.attrs),
Annotatable::Expr(expr) => Some(&expr.attrs),
Annotatable::Arm(arm) => Some(&arm.attrs),
Annotatable::ExprField(field) => Some(&field.attrs),
Annotatable::PatField(field) => Some(&field.attrs),
Annotatable::GenericParam(param) => Some(&param.attrs),
Annotatable::Param(param) => Some(&param.attrs),
Annotatable::FieldDef(def) => Some(&def.attrs),
Annotatable::Variant(variant) => Some(&variant.attrs),
_ => None,
};
if let Some(attrs) = attrs {
if let Some(attr) = ecx.sess.find_by_name(attrs, name) {
ecx.parse_sess().buffer_lint(
DUPLICATE_MACRO_ATTRIBUTES,
attr.span,
ecx.current_expansion.lint_node_id,
"duplicated attribute",
);
}
}
}
30 changes: 30 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3092,6 +3092,7 @@ declare_lint_pass! {
TEXT_DIRECTION_CODEPOINT_IN_COMMENT,
DEREF_INTO_DYN_SUPERTRAIT,
DEPRECATED_CFG_ATTR_CRATE_TYPE_NAME,
DUPLICATE_MACRO_ATTRIBUTES,
]
}

Expand Down Expand Up @@ -3629,3 +3630,32 @@ declare_lint! {
reference: "issue #89460 <https://github.com/rust-lang/rust/issues/89460>",
};
}

declare_lint! {
/// The `duplicate_macro_attributes` lint detects when a `#[test]`-like built-in macro
/// attribute is duplicated on an item. This lint may trigger on `bench`, `cfg_eval`, `test`
/// and `test_case`.
///
/// ### Example
///
/// ```rust,ignore (needs --test)
/// #[test]
/// #[test]
/// fn foo() {}
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// A duplicated attribute may erroneously originate from a copy-paste and the effect of it
/// being duplicated may not be obvious or desireable.
///
/// For instance, doubling the `#[test]` attributes registers the test to be run twice with no
/// change to its environment.
///
/// [issue #90979]: https://github.com/rust-lang/rust/issues/90979
pub DUPLICATE_MACRO_ATTRIBUTES,
Warn,
"duplicated attribute"
}
41 changes: 41 additions & 0 deletions src/test/ui/attributes/duplicated-attributes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Test that, if an item is annotated with a builtin attribute more than once, a warning is
// emitted.
// Tests https://github.com/rust-lang/rust/issues/90979

// check-pass
// compile-flags: --test

#![feature(test)]
#![feature(cfg_eval)]

#[test]
#[test]
//~^ WARNING duplicated attribute
fn f() {}

// The following shouldn't trigger an error. The attribute is not duplicated.
#[test]
fn f2() {}

// The following shouldn't trigger an error either. The second attribute is not #[test].
#[test]
#[inline]
fn f3() {}

extern crate test;
use test::Bencher;

#[bench]
#[bench]
//~^ WARNING duplicated attribute
fn f4(_: &mut Bencher) {}

#[cfg_eval]
#[cfg_eval]
//~^ WARNING duplicated attribute
struct S;

#[cfg_eval]
struct S2;

fn main() {}
22 changes: 22 additions & 0 deletions src/test/ui/attributes/duplicated-attributes.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
warning: duplicated attribute
--> $DIR/duplicated-attributes.rs:12:1
|
LL | #[test]
| ^^^^^^^
|
= note: `#[warn(duplicate_macro_attributes)]` on by default

warning: duplicated attribute
--> $DIR/duplicated-attributes.rs:29:1
|
LL | #[bench]
| ^^^^^^^^

warning: duplicated attribute
--> $DIR/duplicated-attributes.rs:34:1
|
LL | #[cfg_eval]
| ^^^^^^^^^^^

warning: 3 warnings emitted

0 comments on commit 2be94d4

Please sign in to comment.