From 2be94d43019961f855589c1c6e2e27063537d30a Mon Sep 17 00:00:00 2001 From: Ethiraric Date: Wed, 24 Nov 2021 10:20:23 +0100 Subject: [PATCH] Add a lint for duplicated attributes. --- Cargo.lock | 1 + compiler/rustc_builtin_macros/Cargo.toml | 1 + compiler/rustc_builtin_macros/src/cfg_eval.rs | 3 +- compiler/rustc_builtin_macros/src/test.rs | 5 ++- compiler/rustc_builtin_macros/src/util.rs | 35 +++++++++++++++- compiler/rustc_lint_defs/src/builtin.rs | 30 ++++++++++++++ .../ui/attributes/duplicated-attributes.rs | 41 +++++++++++++++++++ .../attributes/duplicated-attributes.stderr | 22 ++++++++++ 8 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/attributes/duplicated-attributes.rs create mode 100644 src/test/ui/attributes/duplicated-attributes.stderr diff --git a/Cargo.lock b/Cargo.lock index 15fdc60c99fc8..c007910e272ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3678,6 +3678,7 @@ dependencies = [ "rustc_expand", "rustc_feature", "rustc_lexer", + "rustc_lint_defs", "rustc_parse", "rustc_parse_format", "rustc_session", diff --git a/compiler/rustc_builtin_macros/Cargo.toml b/compiler/rustc_builtin_macros/Cargo.toml index fd34f947f72c0..9031c3b2ecfbb 100644 --- a/compiler/rustc_builtin_macros/Cargo.toml +++ b/compiler/rustc_builtin_macros/Cargo.toml @@ -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" } diff --git a/compiler/rustc_builtin_macros/src/cfg_eval.rs b/compiler/rustc_builtin_macros/src/cfg_eval.rs index 5933a49ea5803..31086a2acf8cc 100644 --- a/compiler/rustc_builtin_macros/src/cfg_eval.rs +++ b/compiler/rustc_builtin_macros/src/cfg_eval.rs @@ -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; @@ -25,6 +25,7 @@ crate fn expand( annotatable: Annotatable, ) -> Vec { 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)] } diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs index d2629926b51da..c08b141b557ca 100644 --- a/compiler/rustc_builtin_macros/src/test.rs +++ b/compiler/rustc_builtin_macros/src/test.rs @@ -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; @@ -27,6 +27,7 @@ pub fn expand_test_case( anno_item: Annotatable, ) -> Vec { 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![]; @@ -55,6 +56,7 @@ pub fn expand_test( item: Annotatable, ) -> Vec { 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) } @@ -65,6 +67,7 @@ pub fn expand_bench( item: Annotatable, ) -> Vec { 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) } diff --git a/compiler/rustc_builtin_macros/src/util.rs b/compiler/rustc_builtin_macros/src/util.rs index 01ea80c4c8a06..527fe50eff0ce 100644 --- a/compiler/rustc_builtin_macros/src/util.rs +++ b/compiler/rustc_builtin_macros/src/util.rs @@ -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; @@ -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(¶m.attrs), + Annotatable::Param(param) => Some(¶m.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", + ); + } + } +} diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index c9294c68a7dc9..77f143738b4cc 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -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, ] } @@ -3629,3 +3630,32 @@ declare_lint! { reference: "issue #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" +} diff --git a/src/test/ui/attributes/duplicated-attributes.rs b/src/test/ui/attributes/duplicated-attributes.rs new file mode 100644 index 0000000000000..84a5abcf8b4bc --- /dev/null +++ b/src/test/ui/attributes/duplicated-attributes.rs @@ -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() {} diff --git a/src/test/ui/attributes/duplicated-attributes.stderr b/src/test/ui/attributes/duplicated-attributes.stderr new file mode 100644 index 0000000000000..735d950b27c22 --- /dev/null +++ b/src/test/ui/attributes/duplicated-attributes.stderr @@ -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 +