-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add new restriction lint on assert!(!..)
- Loading branch information
Showing
11 changed files
with
157 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of}; | ||
use rustc_errors::Applicability; | ||
use rustc_hir::{Expr, ExprKind, UnOp}; | ||
use rustc_lint::{LateContext, LateLintPass}; | ||
use rustc_session::{declare_lint_pass, declare_tool_lint}; | ||
|
||
declare_clippy_lint! { | ||
/// ### What it does | ||
/// This lint warns about the use of inverted conditions in assert-like macros. | ||
/// | ||
/// ### Why is this bad? | ||
/// It is all too easy to misread the semantics of an assertion when the | ||
/// logic of the condition is reversed. Explicitly comparing to a boolean | ||
/// value is preferable. | ||
/// | ||
/// ### Example | ||
/// ```rust | ||
/// // Bad | ||
/// assert!(!"a".is_empty()); | ||
/// | ||
/// // Good | ||
/// assert_eq!("a".is_empty(), false); | ||
/// | ||
/// // Okay | ||
/// assert_ne!("a".is_empty(), true); | ||
/// ``` | ||
#[clippy::version = "1.58.0"] | ||
pub BOOL_ASSERT_INVERTED, | ||
restriction, | ||
"Asserting on an inverted condition" | ||
} | ||
|
||
declare_lint_pass!(BoolAssertInverted => [BOOL_ASSERT_INVERTED]); | ||
|
||
fn is_inverted(e: &Expr<'_>) -> bool { | ||
matches!(e.kind, ExprKind::Unary(UnOp::Not, _),) && !e.span.from_expansion() | ||
} | ||
|
||
impl<'tcx> LateLintPass<'tcx> for BoolAssertInverted { | ||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | ||
let macros = ["assert", "debug_assert"]; | ||
|
||
for mac in ¯os { | ||
if let Some(span) = is_direct_expn_of(expr.span, mac) { | ||
if let Some(args) = higher::extract_assert_macro_args(expr) { | ||
if let [a, ..] = args[..] { | ||
if !is_inverted(a) { | ||
return; | ||
} | ||
|
||
let eq_mac = format!("{}_eq", mac); | ||
span_lint_and_sugg( | ||
cx, | ||
BOOL_ASSERT_INVERTED, | ||
span, | ||
&format!("used `{}!` with an inverted condition", mac), | ||
"replace it with", | ||
format!("{}!(.., false, ..)", eq_mac), | ||
Applicability::MaybeIncorrect, | ||
); | ||
return; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
#![warn(clippy::bool_assert_inverted)] | ||
|
||
use std::ops::Not; | ||
|
||
macro_rules! a { | ||
() => { | ||
true | ||
}; | ||
} | ||
macro_rules! b { | ||
() => { | ||
true | ||
}; | ||
} | ||
|
||
#[derive(Debug, Clone, Copy)] | ||
struct ImplNotTraitWithBool; | ||
|
||
impl PartialEq<bool> for ImplNotTraitWithBool { | ||
fn eq(&self, other: &bool) -> bool { | ||
false | ||
} | ||
} | ||
|
||
impl Not for ImplNotTraitWithBool { | ||
type Output = bool; | ||
|
||
fn not(self) -> Self::Output { | ||
true | ||
} | ||
} | ||
|
||
fn main() { | ||
let a = ImplNotTraitWithBool; | ||
|
||
assert!(!"a".is_empty()); | ||
assert!("".is_empty()); | ||
assert!(!a); | ||
assert!(a); | ||
|
||
debug_assert!(!"a".is_empty()); | ||
debug_assert!("".is_empty()); | ||
debug_assert!(!a); | ||
debug_assert!(a); | ||
|
||
assert!(!"a".is_empty(), "tadam {}", false); | ||
assert!("".is_empty(), "tadam {}", false); | ||
assert!(!a, "tadam {}", false); | ||
assert!(a, "tadam {}", false); | ||
|
||
debug_assert!(!"a".is_empty(), "tadam {}", false); | ||
debug_assert!("".is_empty(), "tadam {}", false); | ||
debug_assert!(!a, "tadam {}", false); | ||
debug_assert!(a, "tadam {}", false); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
error: used `debug_assert!` with an inverted condition | ||
--> $DIR/bool_assert_inverted.rs:41:5 | ||
| | ||
LL | debug_assert!(!"a".is_empty()); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert_eq!(.., false, ..)` | ||
| | ||
= note: `-D clippy::bool-assert-inverted` implied by `-D warnings` | ||
|
||
error: used `debug_assert!` with an inverted condition | ||
--> $DIR/bool_assert_inverted.rs:43:5 | ||
| | ||
LL | debug_assert!(!a); | ||
| ^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert_eq!(.., false, ..)` | ||
|
||
error: used `debug_assert!` with an inverted condition | ||
--> $DIR/bool_assert_inverted.rs:51:5 | ||
| | ||
LL | debug_assert!(!"a".is_empty(), "tadam {}", false); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert_eq!(.., false, ..)` | ||
|
||
error: used `debug_assert!` with an inverted condition | ||
--> $DIR/bool_assert_inverted.rs:53:5 | ||
| | ||
LL | debug_assert!(!a, "tadam {}", false); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `debug_assert_eq!(.., false, ..)` | ||
|
||
error: aborting due to 4 previous errors | ||
|