From f7d49a340d63da0bcf9011f7e2f30be2ca88fdf6 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 8 Apr 2024 14:12:32 +0200 Subject: [PATCH] Turn `duplicated_attributes` into a late lint --- .../src/attrs/duplicated_attributes.rs | 8 ++-- clippy_lints/src/attrs/mod.rs | 11 ++---- tests/ui/auxiliary/proc_macro_attr.rs | 14 +++++++ tests/ui/duplicated_attributes.rs | 10 ++++- tests/ui/duplicated_attributes.stderr | 31 ++++----------- tests/ui/unnecessary_clippy_cfg.stderr | 38 ++++++++++++++++++- tests/ui/useless_attribute.fixed | 2 +- tests/ui/useless_attribute.rs | 2 +- 8 files changed, 77 insertions(+), 39 deletions(-) diff --git a/clippy_lints/src/attrs/duplicated_attributes.rs b/clippy_lints/src/attrs/duplicated_attributes.rs index 3a8844d0754..736ee48641d 100644 --- a/clippy_lints/src/attrs/duplicated_attributes.rs +++ b/clippy_lints/src/attrs/duplicated_attributes.rs @@ -2,12 +2,12 @@ use super::DUPLICATED_ATTRIBUTES; use clippy_utils::diagnostics::span_lint_and_then; use rustc_ast::{Attribute, MetaItem}; use rustc_data_structures::fx::FxHashMap; -use rustc_lint::EarlyContext; +use rustc_lint::LateContext; use rustc_span::{sym, Span}; use std::collections::hash_map::Entry; fn emit_if_duplicated( - cx: &EarlyContext<'_>, + cx: &LateContext<'_>, attr: &MetaItem, attr_paths: &mut FxHashMap, complete_path: String, @@ -26,7 +26,7 @@ fn emit_if_duplicated( } fn check_duplicated_attr( - cx: &EarlyContext<'_>, + cx: &LateContext<'_>, attr: &MetaItem, attr_paths: &mut FxHashMap, parent: &mut Vec, @@ -64,7 +64,7 @@ fn check_duplicated_attr( } } -pub fn check(cx: &EarlyContext<'_>, attrs: &[Attribute]) { +pub fn check(cx: &LateContext<'_>, attrs: &[Attribute]) { let mut attr_paths = FxHashMap::default(); for attr in attrs { diff --git a/clippy_lints/src/attrs/mod.rs b/clippy_lints/src/attrs/mod.rs index 684ad7de2f0..8f47bc7653b 100644 --- a/clippy_lints/src/attrs/mod.rs +++ b/clippy_lints/src/attrs/mod.rs @@ -17,7 +17,7 @@ mod useless_attribute; mod utils; use clippy_config::msrvs::Msrv; -use rustc_ast::{Attribute, Crate, MetaItemKind, NestedMetaItem}; +use rustc_ast::{Attribute, MetaItemKind, NestedMetaItem}; use rustc_hir::{ImplItem, Item, ItemKind, TraitItem}; use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, impl_lint_pass}; @@ -534,11 +534,13 @@ declare_lint_pass!(Attributes => [ BLANKET_CLIPPY_RESTRICTION_LINTS, SHOULD_PANIC_WITHOUT_EXPECT, MIXED_ATTRIBUTES_STYLE, + DUPLICATED_ATTRIBUTES, ]); impl<'tcx> LateLintPass<'tcx> for Attributes { fn check_crate(&mut self, cx: &LateContext<'tcx>) { blanket_clippy_restriction_lints::check_command_line(cx); + duplicated_attributes::check(cx, cx.tcx.hir().krate_attrs()); } fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) { @@ -578,6 +580,7 @@ impl<'tcx> LateLintPass<'tcx> for Attributes { _ => {}, } mixed_attributes_style::check(cx, item.span, attrs); + duplicated_attributes::check(cx, attrs); } fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { @@ -606,17 +609,11 @@ impl_lint_pass!(EarlyAttributes => [ MAYBE_MISUSED_CFG, DEPRECATED_CLIPPY_CFG_ATTR, UNNECESSARY_CLIPPY_CFG, - DUPLICATED_ATTRIBUTES, ]); impl EarlyLintPass for EarlyAttributes { - fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &Crate) { - duplicated_attributes::check(cx, &krate.attrs); - } - fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) { empty_line_after::check(cx, item); - duplicated_attributes::check(cx, &item.attrs); } fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) { diff --git a/tests/ui/auxiliary/proc_macro_attr.rs b/tests/ui/auxiliary/proc_macro_attr.rs index a6f3b164c9b..f6fdebaf252 100644 --- a/tests/ui/auxiliary/proc_macro_attr.rs +++ b/tests/ui/auxiliary/proc_macro_attr.rs @@ -176,3 +176,17 @@ pub fn with_empty_docs(_attr: TokenStream, input: TokenStream) -> TokenStream { } .into() } + +#[proc_macro_attribute] +pub fn duplicated_attr(_attr: TokenStream, input: TokenStream) -> TokenStream { + let item = parse_macro_input!(input as syn::Item); + let attrs: Vec = vec![]; + quote! { + #(#attrs)* + #[allow(unused)] + #[allow(unused)] + #[allow(unused)] + #item + } + .into() +} diff --git a/tests/ui/duplicated_attributes.rs b/tests/ui/duplicated_attributes.rs index d051c881f15..d51e7e37beb 100644 --- a/tests/ui/duplicated_attributes.rs +++ b/tests/ui/duplicated_attributes.rs @@ -1,9 +1,14 @@ +//@aux-build:proc_macro_attr.rs + #![warn(clippy::duplicated_attributes)] #![cfg(any(unix, windows))] #![allow(dead_code)] #![allow(dead_code)] //~ ERROR: duplicated attribute #![cfg(any(unix, windows))] // Should not warn! +#[macro_use] +extern crate proc_macro_attr; + #[cfg(any(unix, windows, target_os = "linux"))] #[allow(dead_code)] #[allow(dead_code)] //~ ERROR: duplicated attribute @@ -12,7 +17,10 @@ fn foo() {} #[cfg(unix)] #[cfg(windows)] -#[cfg(unix)] //~ ERROR: duplicated attribute +#[cfg(unix)] // cfgs are not handled fn bar() {} +#[proc_macro_attr::duplicated_attr()] // Should not warn! +fn babar() {} + fn main() {} diff --git a/tests/ui/duplicated_attributes.stderr b/tests/ui/duplicated_attributes.stderr index 9e26ba990ac..0903617a8d1 100644 --- a/tests/ui/duplicated_attributes.stderr +++ b/tests/ui/duplicated_attributes.stderr @@ -1,16 +1,16 @@ error: duplicated attribute - --> tests/ui/duplicated_attributes.rs:4:10 + --> tests/ui/duplicated_attributes.rs:6:10 | LL | #![allow(dead_code)] | ^^^^^^^^^ | note: first defined here - --> tests/ui/duplicated_attributes.rs:3:10 + --> tests/ui/duplicated_attributes.rs:5:10 | LL | #![allow(dead_code)] | ^^^^^^^^^ help: remove this attribute - --> tests/ui/duplicated_attributes.rs:4:10 + --> tests/ui/duplicated_attributes.rs:6:10 | LL | #![allow(dead_code)] | ^^^^^^^^^ @@ -18,38 +18,21 @@ LL | #![allow(dead_code)] = help: to override `-D warnings` add `#[allow(clippy::duplicated_attributes)]` error: duplicated attribute - --> tests/ui/duplicated_attributes.rs:9:9 + --> tests/ui/duplicated_attributes.rs:14:9 | LL | #[allow(dead_code)] | ^^^^^^^^^ | note: first defined here - --> tests/ui/duplicated_attributes.rs:8:9 + --> tests/ui/duplicated_attributes.rs:13:9 | LL | #[allow(dead_code)] | ^^^^^^^^^ help: remove this attribute - --> tests/ui/duplicated_attributes.rs:9:9 + --> tests/ui/duplicated_attributes.rs:14:9 | LL | #[allow(dead_code)] | ^^^^^^^^^ -error: duplicated attribute - --> tests/ui/duplicated_attributes.rs:15:7 - | -LL | #[cfg(unix)] - | ^^^^ - | -note: first defined here - --> tests/ui/duplicated_attributes.rs:13:7 - | -LL | #[cfg(unix)] - | ^^^^ -help: remove this attribute - --> tests/ui/duplicated_attributes.rs:15:7 - | -LL | #[cfg(unix)] - | ^^^^ - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors diff --git a/tests/ui/unnecessary_clippy_cfg.stderr b/tests/ui/unnecessary_clippy_cfg.stderr index fbc05743ca7..3d58c9eb5da 100644 --- a/tests/ui/unnecessary_clippy_cfg.stderr +++ b/tests/ui/unnecessary_clippy_cfg.stderr @@ -57,5 +57,41 @@ error: no need to put clippy lints behind a `clippy` cfg LL | #![cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#![deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg)]` -error: aborting due to 8 previous errors +error: duplicated attribute + --> tests/ui/unnecessary_clippy_cfg.rs:8:26 + | +LL | #![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] + | ^^^^^^^^^ + | +note: first defined here + --> tests/ui/unnecessary_clippy_cfg.rs:6:26 + | +LL | #![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))] + | ^^^^^^^^^ +help: remove this attribute + --> tests/ui/unnecessary_clippy_cfg.rs:8:26 + | +LL | #![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] + | ^^^^^^^^^ + = note: `-D clippy::duplicated-attributes` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::duplicated_attributes)]` + +error: duplicated attribute + --> tests/ui/unnecessary_clippy_cfg.rs:17:25 + | +LL | #[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] + | ^^^^^^^^^ + | +note: first defined here + --> tests/ui/unnecessary_clippy_cfg.rs:15:25 + | +LL | #[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))] + | ^^^^^^^^^ +help: remove this attribute + --> tests/ui/unnecessary_clippy_cfg.rs:17:25 + | +LL | #[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] + | ^^^^^^^^^ + +error: aborting due to 10 previous errors diff --git a/tests/ui/useless_attribute.fixed b/tests/ui/useless_attribute.fixed index d1cdf73d559..81759086f79 100644 --- a/tests/ui/useless_attribute.fixed +++ b/tests/ui/useless_attribute.fixed @@ -1,6 +1,6 @@ //@aux-build:proc_macro_derive.rs -#![allow(unused)] +#![allow(unused, clippy::duplicated_attributes)] #![warn(clippy::useless_attribute)] #![warn(unreachable_pub)] #![feature(rustc_private)] diff --git a/tests/ui/useless_attribute.rs b/tests/ui/useless_attribute.rs index d6aa7fa242c..59a9dcf093b 100644 --- a/tests/ui/useless_attribute.rs +++ b/tests/ui/useless_attribute.rs @@ -1,6 +1,6 @@ //@aux-build:proc_macro_derive.rs -#![allow(unused)] +#![allow(unused, clippy::duplicated_attributes)] #![warn(clippy::useless_attribute)] #![warn(unreachable_pub)] #![feature(rustc_private)]