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

Add allow_attribute lint #10481

Merged
merged 7 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4382,6 +4382,7 @@ Released 2018-09-13
<!-- begin autogenerated links to lint list -->
[`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons
[`alloc_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#alloc_instead_of_core
[`allow_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#allow_attribute
[`allow_attributes_without_reason`]: https://rust-lang.github.io/rust-clippy/master/index.html#allow_attributes_without_reason
[`almost_complete_letter_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_letter_range
[`almost_complete_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range
Expand Down
93 changes: 93 additions & 0 deletions clippy_lints/src/allow_attribute.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
use ast::{AttrStyle, MetaItemKind};
use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet};
use rustc_ast as ast;
use rustc_errors::Applicability;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{symbol::Ident, BytePos};

declare_clippy_lint! {
/// ### What it does
/// Detects uses of the `#[allow]` attribute and suggests to replace it with the new `#[expect]` attribute implemented by `#![feature(lint_reasons)]` ([RFC 2383](https://rust-lang.github.io/rfcs/2383-lint-reasons.html))
/// ### Why is this bad?
/// Using `#[allow]` isn't bad, but `#[expect]` may be preferred as it lints if the code **doesn't** produce a warning.
/// ### Example
/// ```rust,ignore
/// #[allow(unused_mut)]
/// fn foo() -> usize {
/// let mut a = Vec::new();
/// a.len()
///}
/// ```
/// Use instead:
/// ```rust,ignore
/// # #![feature(lint_reasons)]
/// #[expect(unused_mut)]
/// fn foo() -> usize {
/// let mut a = Vec::new();
/// a.len()
/// }
/// ```
#[clippy::version = "1.69.0"]
pub ALLOW_ATTRIBUTE,
restriction,
"`#[allow]` will not trigger if a warning isn't found. `#[expect]` triggers if there are no warnings."
}

pub struct AllowAttribute {
pub lint_reasons_active: bool,
}

impl_lint_pass!(AllowAttribute => [ALLOW_ATTRIBUTE]);

impl LateLintPass<'_> for AllowAttribute {
// Separate each crate's features.
fn check_crate_post(&mut self, _: &LateContext<'_>) {
self.lint_reasons_active = false;
}
fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &ast::Attribute) {
// Check inner attributes

if_chain! {
if let AttrStyle::Inner = attr.style;
if attr.ident()
.unwrap_or(Ident::with_dummy_span(sym!(empty))) // Will not trigger if doesn't have an ident.
.name == sym!(feature);
if let ast::AttrKind::Normal(normal) = &attr.kind;
if let Some(MetaItemKind::List(list)) = normal.item.meta_kind();
if let Some(symbol) = list.get(0);
if symbol.ident().unwrap().name == sym!(lint_reasons);
then {
self.lint_reasons_active = true;
}
}

// Check outer attributes

if_chain! {
if let AttrStyle::Outer = attr.style;
if attr.ident()
.unwrap_or(Ident::with_dummy_span(sym!(empty))) // Will not trigger if doesn't have an ident.
.name == sym!(allow);
if self.lint_reasons_active;
then {
span_lint_and_sugg(
cx,
ALLOW_ATTRIBUTE,
attr.span,
"#[allow] attribute found",
"replace it with",
format!("#[expect{})]", snippet(
cx,
attr.ident().unwrap().span
.with_lo(
attr.ident().unwrap().span.hi() + BytePos(2) // Cut [(
)
.with_hi(
attr.meta().unwrap().span.hi() - BytePos(2) // Cut )]
)
, "...")), Applicability::MachineApplicable);
}
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::utils::internal_lints::produce_ice::PRODUCE_ICE_INFO,
#[cfg(feature = "internal")]
crate::utils::internal_lints::unnecessary_def_path::UNNECESSARY_DEF_PATH_INFO,
crate::allow_attribute::ALLOW_ATTRIBUTE_INFO,
crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
crate::approx_const::APPROX_CONSTANT_INFO,
crate::as_conversions::AS_CONVERSIONS_INFO,
Expand Down
6 changes: 6 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ mod declared_lints;
mod renamed_lints;

// begin lints modules, do not remove this comment, it’s used in `update_lints`
mod allow_attribute;
mod almost_complete_range;
mod approx_const;
mod as_conversions;
Expand Down Expand Up @@ -933,6 +934,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(missing_assert_message::MissingAssertMessage));
store.register_early_pass(|| Box::new(redundant_async_block::RedundantAsyncBlock));
store.register_late_pass(|_| Box::new(let_with_type_underscore::UnderscoreTyped));
store.register_late_pass(|_| {
Box::new(allow_attribute::AllowAttribute {
lint_reasons_active: false,
})
});
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
18 changes: 18 additions & 0 deletions tests/ui/allow_attribute.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// run-rustfix
#![allow(unused)]
#![warn(clippy::allow_attribute)]
#![feature(lint_reasons)]

fn main() {}

// Using clippy::needless_borrow just as a placeholder, it isn't relevant.

// Should lint
#[expect(dead_code)]
struct T1;

struct T2; // Should not lint
#[deny(clippy::needless_borrow)] // Should not lint
struct T3;
#[warn(clippy::needless_borrow)] // Should not lint
struct T4;
18 changes: 18 additions & 0 deletions tests/ui/allow_attribute.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// run-rustfix
#![allow(unused)]
#![warn(clippy::allow_attribute)]
#![feature(lint_reasons)]

fn main() {}

// Using clippy::needless_borrow just as a placeholder, it isn't relevant.

// Should lint
#[allow(dead_code)]
struct T1;

struct T2; // Should not lint
#[deny(clippy::needless_borrow)] // Should not lint
struct T3;
#[warn(clippy::needless_borrow)] // Should not lint
struct T4;
10 changes: 10 additions & 0 deletions tests/ui/allow_attribute.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: #[allow] attribute found
--> $DIR/allow_attribute.rs:11:1
|
LL | #[allow(dead_code)]
| ^^^^^^^^^^^^^^^^^^^ help: replace it with: `#[expect(dead_code)]`
|
= note: `-D clippy::allow-attribute` implied by `-D warnings`

error: aborting due to previous error