New lint: always_true_conditions by Dominic-Moser · Pull Request #14434 · rust-lang/rust-clippy

Dominic-Moser · 2025-03-18T20:43:47Z

This lint closes #1593

This is my first lint, please let me know if I messed something up anywhere!

changelog: [always_true_conditions]: add new lint

rustbot · 2025-03-18T20:43:52Z

rustbot has assigned @Alexendoo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

samueltardieu · 2025-03-19T21:18:43Z

The lintcheck results shows 64 hits, which seem to be all false positives.

samueltardieu · 2025-04-06T10:20:48Z

@Dominic-Moser Do you plan to continue working on this lint?
@rustbot author

rustbot · 2025-04-06T10:20:52Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Dominic-Moser · 2025-04-08T19:44:57Z

@rustbot ready, sorry school's been hitting hard

Dominic-Moser · 2025-04-08T19:54:28Z

never mind, I have to to checks to make sure the same variable is being used throughout the whole expression

Dominic-Moser · 2025-04-18T00:26:21Z

@samueltardieu I am so confused right now, I keep trying to rebase and it keeps saying I need to rebase. I am sure this is my fault but i have no clue how to fix this...
On the bright side I do actually think that the code works properly now 🫠

Alexendoo · 2025-04-20T17:33:55Z

Your first commit contains a lot of unrelated changes 9ce8566

What I would do is first reset your branch to the current state of master

git fetch upstream
git reset --hard upstream/master

Then grab the main parts of the PR - the lint and the test file

git checkout b55d82f89c1b29f25044aeaafbbdb7a241c82c3b -- clippy_lints/src/always_true_conditions.rs tests/ui/always_true_conditions.rs

Now you can run cargo dev update_lints, add the lint registration to clippy_lints/src/lib.rs again, and make sure tests pass. Then you can create the commit once more

Dominic-Moser · 2025-05-03T02:39:49Z

@rustbot ready

This lint aims to flag a common mistake where a variable is compared against two primitives using a != operator and mistakenly linked with the || with always evaluate to true. ex. ```a != 1 || a != 2``` This helps flag a common mistake and maintain overall quality of code. changelog: new lint

Alexendoo · 2025-11-02T19:18:59Z

+        if new_op.node == BinOpKind::Or {
+            let f = context_applicable(new_f);
+            let l = context_applicable(new_l);
+            if l == f { l } else { None }


What's the purpose of the Or branch here? If you remove the new_op.node == BinOpKind::Or branch no tests fail

Alexendoo · 2025-11-02T19:22:39Z

+        if let ExprKind::Binary(f_op_kind, f_cond, l_cond) = e.kind
+            && let BinOpKind::Or = f_op_kind.node
+        {
+            let msg = "expression will always be true, did you mean to use &&?";


The "did you mean" can be split out into a help message with https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/diagnostics/fn.span_lint_and_help.html

Could also mention that it might be the wrong variable being compared

Alexendoo · 2025-11-02T19:23:34Z

+            let f_res = context_applicable(f_cond);
+            let l_res = context_applicable(l_cond);


What do f and l stand for here? If there's no good name l/lhs and r/rhs are fairly typical

Alexendoo · 2025-11-02T19:51:04Z

It would be good if you could move the implementation of the lint into clippy_lints/src/operators since it would fit well there

Alexendoo · 2025-11-02T19:52:34Z

+            if f_res.is_some() && (l_res == f_res) {
+                span_lint(cx, ALWAYS_TRUE_CONDITIONS, e.span, msg);
+            }


A case to consider would be a != 1 || a != 1 from e.g. a copy/paste, the lint currently says that it's always true which isn't the case

Dominic-Moser · 2025-11-18T03:45:35Z

+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Flags a relativly common error where users comparing a varible to a primative use || instal of && in conjunction


Will resolve ASAP! 😅 I've had alot of coursework but Ill try to have Alexendoo's suggestions done tonight in conjunction with this.

Knew I shouldnt have cheated on my 8th grade spelling tests...

felix91gr · 2025-11-18T03:54:59Z

+    /// Flags a relativly common error where users comparing a varible to a primative use || instal of && in conjunction
+    /// with !=. This lint was originally meant for simple n != 1 || n != 2 type statements, but the lint will detect
+    /// the primitive and varible in any order for any length, as long as the variable stays the same, and the condition
+    /// is always 1 primitive and 1 varible.
+    ///
+    /// ### Why is this bad?
+    ///
+    ///This is bad because the code will always result in true. If this is intentional a constant can be used in the
+    ///case of a boolean varibale assignment, or code in an if block should just be moved outside with comments
+    ///explaining why.


Suggested change

/// Flags a relativly common error where users comparing a varible to a primative use || instal of && in conjunction

/// with !=. This lint was originally meant for simple n != 1 || n != 2 type statements, but the lint will detect

/// the primitive and varible in any order for any length, as long as the variable stays the same, and the condition

/// is always 1 primitive and 1 varible.

///

/// ### Why is this bad?

///

///This is bad because the code will always result in true. If this is intentional a constant can be used in the

///case of a boolean varibale assignment, or code in an if block should just be moved outside with comments

///explaining why.

/// Flags a relatively common error, where users comparing a variable to a primitive use `||` instead of `&&`, in conjunction

/// with `!=`. This lint was originally meant for simple `n != 1 || n != 2` type expressions, but the lint will now detect

/// the primitive and variable in any order for any length, as long as the variable stays the same, and the conditional

/// is always made of 1 primitive and 1 variable.

///

/// ### Why is this bad?

///

/// This is bad because however complex this expression is, its meaning is the same - `true`, and thus

/// the code can be greatly simplified by replacing it with that value.

Hey, cool lint! (and sorry for the double ping; my suggestion was much shorter 20 minutes ago). I found some typos and some stuff that might not have been very clear. I tried my best to make it easier to read.

The only bit where I'm not sure I understand what is meant is this one:

as long as the variable stays the same

You mean that the variable doesn't change its value? Or do you mean that the variable in question is always the same, like x appearing in multiple parts of the expression, perhaps?

Regardless, I'll see if there's anything else I can help you with. I love the area of symbolic evaluation :)

felix91gr · 2025-11-18T04:17:03Z

This might be bikeshedding, dunno.

First I wanna say that this is a good lint. But I would change the name of the lint to something of a smaller scope.

Aside: finding whether an expression is always true is undecidable, but that's fine here.

Finding whether expressions are always true is exceedingly useful (even if one will never find all of them due to undecidability), and is something that people will definitely want to use.

However, that's a much larger lint, one that probably requires an engine of its own.

Instead of "always true conditions", perhaps something along the lines of "trivial disjunction" could express the scope of this lint better?

(There's probably a better one, since there are other disjunctions that are trivial as well. But perhaps it's a good start?)

Dominic-Moser · 2025-11-18T05:38:59Z

Instead of "always true conditions", perhaps something along the lines of "trivial disjunction" could express the scope of this lint better?

That seems like a much better match! To make it only apply to this small lint would 'trivial_var_primitive_disjunction' be a good name? I cant really differentiate between a space a slash but the meaning is "trivial variable/primitive disjunction."

rustbot · 2026-01-22T19:50:35Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

This lint aims to flag a common mistake where a variable is compared against two primitives using a != operator and mistakenly linked with the || with always evaluate to true. ex. ```a != 1 || a != 2``` This helps flag a common mistake and maintain overall quality of code. changelog: new lint Hopefully final fix. added a variety of checking, but mostly made sure that the lit.node's were NOT equal to avoid the x!=1 || 1!=x. .

rustbot · 2026-03-05T15:13:05Z

☔ The latest upstream changes (possibly #15979) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot assigned Alexendoo Mar 18, 2025

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 18, 2025

rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 6, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Apr 8, 2025