[ty] Add a test suite for upcoming rules detecting always-truthy and always-falsy boolean tests by AlexWaygood · Pull Request #28030 · astral-sh/ruff

AlexWaygood · 2026-08-25T13:09:27Z

Summary

This PR adds a test suite for two rules that I've been working on: redundant-condition and redundant-condition-strict. The proposed behaviour is that redundant-condition will be enabled by default and redundant-condition-strict will be disabled by default. In my implementation currently (which is not part of this PR), redundant-condition has 411 ecosystem hits, while redundant-condition-strict has 2,972.

The two rules both flag a common problem: boolean tests that are accidentally always truthy or always falsy. For example, a common error in Python is to do this:

import random

def f() -> bool:
    return random.choice([True, False])

def g():
    if f:  # oops, forgot the parentheses, so this will always be true!
        ...

Many of the tests here outline exemptions that must be applied to these rules in order to avoid a prohibitive number of false positives, however. For example, if the user has configured their Python version as being 3.14 when checking their code with ty, if sys.version_info >= (3, 14) will always be inferred by ty as evaluating to Literal[True] -- but flagging that condition with a diagnostic wouldn't be helpful behaviour. My implementation therefore exempts any conditions where a subexpression in that condition can be identified as referring to one of four symbols: sys.version_info, sys.platform, os.name or typing.TYPE_CHECKING. It also follows (recursively) the definitions of names and attributes used in the boolean condition through to their original definitions, to examine whether any names or attributes used in the condition were defined in relation to one of those four constants. If so, no diagnostic will be reported on the condition.

Together, these two rules cover all of mypy's truthy-function, truthy-bool and redundant-expr error codes, as well as overlapping partially with mypy's unreachable error code.

AlexWaygood · 2026-08-25T13:12:52Z

+## Tests that include walrus expressions
+
+Walrus expressions can have side effects, so an always-true walrus expression may not always be
+redundant. Examples of this can be found in CPython's scripts, where deliberately true walrus
+expressions are used to continue the boolean-expression chain:
+
+- <https://github.com/python/cpython/blob/f74cdf80a120649e4c353430da8cbd1305c00993/Tools/peg_generator/pegen/grammar_parser.py#L152-L168>
+
+It is arguably always possible to write this kind of code in a clearer, more obvious way, so we
+still emit a diagnostic on code like this, even though it may be deliberate. However, we use the
+`redundant-condition-strict` rule for these patterns, so that the rule that is enabled by default is
+unopinionated:
+
+```py
+def coinflip1() -> bool:
+    return True
+
+def coinflip2() -> bool:
+    return True
+
+foo = ("foo",)
+
+# the always-truthy item is a `tuple[Literal["bar"]]`,
+# so this would normally trigger `redundant-condition`,
+# but the presence of the walrus expression means we use
+# the disabled-by-default error code.
+if coinflip1() and (foo := ("bar",)) and coinflip2():  # TODO: should error
+    ...
+```


This I keep going back and forth on. On the one hand, every walrus-containing condition I saw in the ecosystem report for #27634 was deliberately always-true and being used in a boolean condition for its side effect. So from that sense, it seems clearly true that if there are any walrus expressions in the condition, the rule is much more likely to have false positives, so the that's a good case for flagging the condition with redundant-condition-strict rather than redundant-condition.

What I don't like about that, though, is that it feels like it makes the difference between the two rules much more complicated and harder to explain. And it's not like redundant-condition will be entirely free from false positives. There are many fewer false positives in that rule than in redundant-condition-strict, but it's impossible to get them down to 0 for a rule like this.

I would go further and say that if there's a walrus expression, neither rule should flag it, because it clearly is not redundant; it has an effect. I really would not want our enabled-by-default rule flagging a condition with a walrus in it; that would be a false-positive bug IMO.

I would go further and say that if there's a walrus expression, neither rule should flag it, because it clearly is not redundant; it has an effect.

This I do not agree with. The disabled-by-default rule is targeted towards users who want their type checker to catch as many bugs as possible, and are therefore prepared that they will have to rewrite their code in some situations to adapt to the fact that some idioms are easier for a type checker to understand than other idioms. There is always a way to rewrite a walrus-inside-and expression or walrus-inside-or expression that expresses your intent more clearly. I think there were only 5 or so walrus expressions that showed up in the ecosystem report for #27634, and they were all in the CPython peg_generator project (which itself is generated Python code, not handwritten).

Ok, I won't argue, as long as we don't flag it in the enabled-by-default rule :)

…always-falsy boolean tests

AlexWaygood · 2026-08-26T10:06:25Z

I started a conversation in https://github.com/astral-sh/ruff/pull/28034/files#r3861542184 (a PR higher up the stack) on some possible changes we could make to the split between the rules. It's the most interesting design decision about this PR stack. @MichaReiser, I'd be interested in your take, if you're interested and have time (no worries if not).

sharkdp

This is excellent, thank you. It was very refreshing to read a human-written mdtest suite 😍.

sharkdp · 2026-08-26T10:13:23Z

+class Foo:
+    def __init__(self):
+        self.two_element_tuple: tuple[int, int] = (423, 432)
+        self.at_least_one_element: tuple[int, *tuple[int, ...]] = (42,)
+        self.at_least_two_elements: tuple[int, int, *tuple[int, ...]] = (42, 42)
+        self.no_elements: tuple[()] = ()
+
+    def other_method(self):
+        if self.two_element_tuple:  # TODO: should error
+            pass
+        if self.at_least_one_element:  # TODO: should error
+            pass
+        if self.at_least_two_elements:  # TODO: should error
+            pass
+        if self.no_elements:  # TODO: should error
+            pass
+
+        # TODO: should error
+        assert self.at_least_one_element
+        # TODO: should error
+        assert self.at_least_two_elements


Just curious: why is this implemented in a class?

In an upcoming PR stacked on top of this one, I have it implemented so that we trace back the reason why the attribute is inferred as an always-nonempty tuple, and point back to the original tuple annotation for the instance attribute:

Although... that annotation doesn't seem to be firing properly for this specific test that you're commenting on here, which is interesting! Something for me to look into for that stacked PR...

(a variable explicitly annotated with tuple[int] is almost always a mistake, since single-element tuples are ~useless -- the user almost always meant to annotate the variable with tuple[int, ...])

sharkdp · 2026-08-26T10:17:48Z

+A common error in Python is to accidentally test truthiness of the wrong object; for example
+`if func:` (which is always true) where `if func():` was intended, or `if coroutine():` where
+`if await coroutine():` was intended. By default, ty alerts the user to these errors with the error
+code `redundant-condition`, but only if the inferred type of the object is not assignable to `int`.


The word redundant in the proposed error code might imply something incorrect. It sounds a bit like I could remove the assert condition completely if reduntant-condition fires, but apparently this rule will also apply to always-false conditions.

So maybe something like statically-known-condition?

It sounds a bit like I could remove the assert condition completely if reduntant-condition fires

That is generally the idea...

but apparently this rule will also apply to always-false conditions.

Well, but remember that the strict version of this rule is disabled entirely for assert statements, to allow for defensive assertions and exhaustiveness assertions: https://github.com/astral-sh/ruff/blob/45c8db4c601f023f4d27ac5e0f8868a8f4ddd384/crates/ty_python_semantic/resources/mdtest/redundant_condition.md#defensive-assertions. So you'll only get a diagnostic on an assert statement at all if it's something like assert always_truthy_tuple, assert func or assert value_inferred_as_none, all of which are usually indicative of mistakes in your code.

I'll leave this as-is for now, but happy to reconsider the rule name further down the line!

I was thinking about something like

def f(): # some complex (loop) control flow which should in principle always return from the function assert False, "should not be reachable"

Here, the assertion is not redundant (in case the programmer made a mistake in the complex code before the assertion). But if we would never emit one of the redundant- errors on this assertion, then it's probably fine.

carljm · 2026-08-26T14:02:30Z

Where's my co-author credit? ;)

AlexWaygood added testing Related to testing Ruff itself ty Multi-file analysis & type inference labels Aug 25, 2026

AlexWaygood commented Aug 25, 2026

View reviewed changes

AlexWaygood requested a review from sharkdp August 25, 2026 13:14

AlexWaygood marked this pull request as ready for review August 25, 2026 13:14

AlexWaygood requested a review from a team as a code owner August 25, 2026 13:14

AlexWaygood force-pushed the alex/always-truthy-tests branch from 7a7d536 to 1c3dec1 Compare August 25, 2026 16:11

Base automatically changed from alex/more-autofixes to main August 25, 2026 18:31

[ty] Add a test suite for upcoming rules detecting always-truthy and …

61969d1

…always-falsy boolean tests

AlexWaygood force-pushed the alex/always-truthy-tests branch from 1c3dec1 to 61969d1 Compare August 25, 2026 18:31

sharkdp approved these changes Aug 26, 2026

View reviewed changes

Fix many David comments

45c8db4

AlexWaygood merged commit b52fe1b into main Aug 26, 2026
58 checks passed

AlexWaygood deleted the alex/always-truthy-tests branch August 26, 2026 22:38

Uh oh!

Conversation

AlexWaygood commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented Aug 26, 2026

Uh oh!

sharkdp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carljm commented Aug 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AlexWaygood commented Aug 25, 2026 •

edited

Loading

AlexWaygood Aug 26, 2026 •

edited

Loading