Skip to content

New lint: always_true_conditions #14434

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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 CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -5458,6 +5458,7 @@ Released 2018-09-13
[`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
[`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
[`always_true_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#always_true_conditions
[`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
[`arbitrary_source_item_ordering`]: https://rust-lang.github.io/rust-clippy/master/index.html#arbitrary_source_item_ordering
[`arc_with_non_send_sync`]: https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
61 changes: 61 additions & 0 deletions clippy_lints/src/always_true_conditions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
use clippy_utils::diagnostics::span_lint;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;

declare_clippy_lint! {
/// ### What it does
///
/// ### Why is this bad?
///
/// ### Example
/// ```no_run
/// let foo = "anything";
/// if foo != "thing1" || foo != "thing2" {
/// println!("always executes");
/// }
/// ```
/// Use instead:
/// ```no_run
/// let foo = "anything";
/// if foo != "thing1" && foo != "thing2" {
/// println!("sometimes executes");
/// }
/// ```
#[clippy::version = "1.87.0"]
pub ALWAYS_TRUE_CONDITIONS,
nursery,
"checks for if statement conditions which are always true"
}

declare_lint_pass!(AlwaysTrueConditions => [ALWAYS_TRUE_CONDITIONS]);

fn context_applicable(expr: &Expr<'_>) -> bool {
if let ExprKind::Binary(new_op, new_f, new_l) = expr.kind {
if new_op.node == BinOpKind::Or {
//only continue DOWN if its an or.give me the zuck
let f = context_applicable(new_f);
let l = context_applicable(new_l);
l && f
} else {
new_op.node == BinOpKind::Ne
}
} else {
false
}
}

impl LateLintPass<'_> for AlwaysTrueConditions {
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
if let ExprKind::If(cond, _, _) = e.kind
&& let ExprKind::DropTemps(cond) = cond.kind
&& let ExprKind::Binary(f_op_kind, f_cond, l_cond) = cond.kind
&& let BinOpKind::Or = f_op_kind.node
{
let msg = "expression will always be true, did you mean &&?";
if context_applicable(f_cond) && context_applicable(l_cond) {
span_lint(cx, ALWAYS_TRUE_CONDITIONS, e.span, msg);
}
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
@@ -35,6 +35,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::utils::internal_lints::unsorted_clippy_utils_paths::UNSORTED_CLIPPY_UTILS_PATHS_INFO,
crate::absolute_paths::ABSOLUTE_PATHS_INFO,
crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
crate::always_true_conditions::ALWAYS_TRUE_CONDITIONS_INFO,
crate::approx_const::APPROX_CONSTANT_INFO,
crate::arbitrary_source_item_ordering::ARBITRARY_SOURCE_ITEM_ORDERING_INFO,
crate::arc_with_non_send_sync::ARC_WITH_NON_SEND_SYNC_INFO,
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -75,6 +75,7 @@ pub mod deprecated_lints;
// begin lints modules, do not remove this comment, it’s used in `update_lints`
mod absolute_paths;
mod almost_complete_range;
mod always_true_conditions;
mod approx_const;
mod arbitrary_source_item_ordering;
mod arc_with_non_send_sync;
@@ -984,5 +985,6 @@ 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));
store.register_late_pass(|_| Box::new(always_true_conditions::AlwaysTrueConditions));
// add lints here, do not remove this comment, it's used in `new_lint`
}
36 changes: 36 additions & 0 deletions tests/ui/always_true_conditions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#![warn(clippy::always_true_conditions)]
#[allow(clippy::needless_if)]
fn foo_m(name: &str) {
if name != "Min" || name != "Max" || name != "Middle" {
//~^ always_true_conditions
println!("always prints");
} else {
println!("never prints");
}
if name != "Min" && name != "Max" {
println!("condition satisfied");
} else {
println!("else");
}
}

fn foo_s(name: &str) {
if name != "Min" || name != "Max" {
//~^ always_true_conditions
println!("always prints");
} else {
println!("never prints");
}
if name != "Min" && name != "Max" {
println!("condition satisfied");
} else {
println!("else");
}
}

fn catch_fails(input: &str) {
let b = true;
if b || input != "foo" {
println!("should not fire!");
}
}
27 changes: 27 additions & 0 deletions tests/ui/always_true_conditions.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error: expression will always be true, did you mean &&?
--> tests/ui/always_true_conditions.rs:4:5
|
LL | / if name != "Min" || name != "Max" || name != "Middle" {
LL | |
LL | | println!("always prints");
LL | | } else {
LL | | println!("never prints");
LL | | }
| |_____^
|
= note: `-D clippy::always-true-conditions` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::always_true_conditions)]`

error: expression will always be true, did you mean &&?
--> tests/ui/always_true_conditions.rs:18:5
|
LL | / if name != "Min" || name != "Max" {
LL | |
LL | | println!("always prints");
LL | | } else {
LL | | println!("never prints");
LL | | }
| |_____^

error: aborting due to 2 previous errors