diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b62c9a59aa5..60825fd6fa98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5515,6 +5515,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 diff --git a/clippy_lints/src/always_true_conditions.rs b/clippy_lints/src/always_true_conditions.rs new file mode 100644 index 000000000000..0c062b0d4e2b --- /dev/null +++ b/clippy_lints/src/always_true_conditions.rs @@ -0,0 +1,73 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_hir::def::Res; +use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; +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<'_>) -> Option { + if let ExprKind::Binary(new_op, new_f, new_l) = expr.kind { + if new_op.node == BinOpKind::Or { + let f = context_applicable(new_f); + let l = context_applicable(new_l); + if l == f { l } else { None } + } else if new_op.node == BinOpKind::Ne { + find_path(new_f) + } else { + None + } + } else { + None + } +} + +fn find_path(expr: &Expr<'_>) -> Option { + if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind { + Some(path.res) + } else { + None + } +} + +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 &&?"; + let app_f = context_applicable(f_cond); + let app_l = context_applicable(l_cond); + if app_l == app_f && app_f.is_some() && app_l.is_some() { + span_lint(cx, ALWAYS_TRUE_CONDITIONS, e.span, msg); + } + } + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2cccd6ba2702..e63a45d7b49b 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -5,6 +5,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ 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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index bc7fc60827a0..a180b9b93c34 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -74,6 +74,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; @@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static 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(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix)); + store.register_late_pass(move |_| Box::new(always_true_conditions::AlwaysTrueConditions)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/always_true_conditions.rs b/tests/ui/always_true_conditions.rs new file mode 100644 index 000000000000..94667e9563b8 --- /dev/null +++ b/tests/ui/always_true_conditions.rs @@ -0,0 +1,59 @@ +#![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_or_failure(input: &str) { + let b = true; + if b || input != "foo" { + println!("should not fire!"); + } +} + +fn catch_scope_or_failures(input: &str) { + let b = true; + { + if b || input != "foo" { + println!("should not fire!"); + } + } +} + +fn catch_eq_failures() { + let res = "test"; + if res == "foo" || res == "bar" { + println!("should not fire!"); + } +} + +fn catch_diff_var_failure(input: &str) { + let b = "value"; + if b != "bar" || input != "foo" { + println!("should not fire!"); + } +} diff --git a/tests/ui/always_true_conditions.stderr b/tests/ui/always_true_conditions.stderr new file mode 100644 index 000000000000..d435a0781e67 --- /dev/null +++ b/tests/ui/always_true_conditions.stderr @@ -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 +