Skip to content

New Lint: Implement unnecessary_paren_in_negated_if lint #14464

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

Closed
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
9 changes: 5 additions & 4 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub mod ctfe; // Very important lint, do not remove (rust#125116)
pub mod declared_lints;
pub mod deprecated_lints;

// begin lints modules, do not remove this comment, its used in `update_lints`
// begin lints modules, do not remove this comment, it's used in `update_lints`
mod absolute_paths;
mod almost_complete_range;
mod approx_const;
Expand Down Expand Up @@ -375,6 +375,7 @@ mod unnecessary_box_returns;
mod unnecessary_literal_bound;
mod unnecessary_map_on_constructor;
mod unnecessary_owned_empty_strings;
mod unnecessary_paren_in_negated_if;
mod unnecessary_self_imports;
mod unnecessary_semicolon;
mod unnecessary_struct_initialization;
Expand Down Expand Up @@ -404,7 +405,7 @@ mod zero_div_zero;
mod zero_repeat_side_effects;
mod zero_sized_map_values;
mod zombie_processes;
// end lints modules, do not remove this comment, its used in `update_lints`
// end lints modules, do not remove this comment, it's used in `update_lints`

use clippy_config::{Conf, get_configuration_metadata, sanitize_explanation};
use clippy_utils::macros::FormatArgsStorage;
Expand Down Expand Up @@ -616,7 +617,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| {
Box::<utils::internal_lints::lint_without_lint_pass::LintWithoutLintPass>::default()
});
store.register_late_pass(|_| Box::<utils::internal_lints::unnecessary_def_path::UnnecessaryDefPath>::default());
store.register_late_pass(|_| Box::new(utils::internal_lints::unnecessary_def_path::UnnecessaryDefPath));
store.register_late_pass(|_| Box::new(utils::internal_lints::outer_expn_data_pass::OuterExpnDataPass));
store.register_late_pass(|_| Box::new(utils::internal_lints::msrv_attr_impl::MsrvAttrImpl));
store.register_late_pass(|_| {
Expand Down Expand Up @@ -984,5 +985,5 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf)));
store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf)));
store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
// add lints here, do not remove this comment, it's used in `new_lint`
store.register_lints(&[&unnecessary_paren_in_negated_if::UNNECESSARY_PAREN_IN_NEGATED_IF]);
}
68 changes: 68 additions & 0 deletions clippy_lints/src/unnecessary_paren_in_negated_if.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;

declare_clippy_lint! {
/// ### What it does
/// Checks for unnecessary parentheses in negated if conditions.
///
/// ### Why is this bad?
/// Unnecessary parentheses make code harder to read and add visual noise.
///
/// ### Example
/// ```rust
/// if !(condition) {
/// // ...
/// }
/// ```
///
/// Use instead:
/// ```rust
/// if !condition {
/// // ...
/// }
/// ```
#[clippy::version = "1.75.0"]
pub UNNECESSARY_PAREN_IN_NEGATED_IF,
style,
"unnecessary parentheses in negated if conditions"
}

declare_lint_pass!(UnnecessaryParenInNegatedIf => [UNNECESSARY_PAREN_IN_NEGATED_IF]);

impl<'tcx> LateLintPass<'tcx> for UnnecessaryParenInNegatedIf {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if let ExprKind::If(cond, ..) = expr.kind {
// Check if the condition is a negation
if let ExprKind::Unary(UnOp::Not, inner) = cond.kind {
// Check if the inner expression is parenthesized by examining the source code
let mut applicability = Applicability::MachineApplicable;
let inner_snippet = snippet_with_applicability(cx, inner.span, "..", &mut applicability);

// If the snippet starts with '(' and ends with ')', it's likely parenthesized
if inner_snippet.starts_with('(') && inner_snippet.ends_with(')') {
// Extract the content inside the parentheses
let content = &inner_snippet[1..inner_snippet.len() - 1];

// Don't lint if the expression is from a macro
if expr.span.from_expansion() {
return;
}

span_lint_and_sugg(
cx,
UNNECESSARY_PAREN_IN_NEGATED_IF,
inner.span,
"unnecessary parentheses in negated if condition",
"try",
format!("!{content}"),
applicability,
);
}
}
}
}
}
27 changes: 27 additions & 0 deletions tests/ui/unnecessary_paren_in_negated_if.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#![warn(clippy::unnecessary_paren_in_negated_if)]

fn main() {
let condition = true;

// These should trigger the lint
if !(condition) {
println!("Negated condition with unnecessary parentheses");
}

if !(condition) {
println!("Negated condition with double unnecessary parentheses");
}

// These should not trigger the lint
if !condition {
println!("Negated condition without parentheses");
}

if !(condition && true) {
println!("Negated condition with necessary parentheses");
}

if !(condition || false) {
println!("Another negated condition with necessary parentheses");
}
}
15 changes: 15 additions & 0 deletions tests/ui/unnecessary_paren_in_negated_if.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: unnecessary parentheses in negated if condition
--> $DIR/unnecessary_paren_in_negated_if.rs:7:8
|
7 | if !(condition) {
| ^^^^^^^^^^^^ help: try: `!condition`
|
= note: `-D clippy::unnecessary-paren-in-negated-if` implied by `-D warnings`

error: unnecessary parentheses in negated if condition
--> $DIR/unnecessary_paren_in_negated_if.rs:11:8
|
11 | if !((condition)) {
| ^^^^^^^^^^^^^ help: try: `!condition`

error: aborting due to 2 previous errors
Loading