Skip to content
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.

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

Warn when a #[test]-like built-in attribute macro is present multiple times. #91172

Merged
merged 1 commit into from
Dec 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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"
}
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
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