diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index 55f5805568..74768281c7 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -8,7 +8,7 @@ Default level: error · Added in 0.0.64 · Related issues · -View source +View source @@ -44,7 +44,7 @@ class Base(ABC): Default level: error · Added in 0.0.13 · Related issues · -View source +View source @@ -90,7 +90,7 @@ class Derived(Base): # error Default level: warn · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -154,7 +154,7 @@ class SubProto(BaseProto, Protocol): Default level: error · Added in 0.0.14 · Related issues · -View source +View source @@ -236,7 +236,7 @@ value = unknown # ty: ignore[unresolved-reference] Default level: error · Added in 0.0.16 · Related issues · -View source +View source @@ -288,7 +288,7 @@ Foo.method() # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -344,7 +344,7 @@ Concrete() # OK Default level: error · Added in 0.0.7 · Related issues · -View source +View source @@ -384,7 +384,7 @@ def f(x: object): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -417,7 +417,7 @@ a = 1 # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -451,7 +451,7 @@ class C(A, B): ... # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -485,7 +485,7 @@ class B(A): ... # error Default level: error · Added in 0.0.1-alpha.29 · Related issues · -View source +View source @@ -536,7 +536,7 @@ type Tree = int | list[Tree] # valid recursive alias Default level: error · Added in 0.0.15 · Related issues · -View source +View source @@ -572,7 +572,7 @@ class Example: Default level: warn · Added in 0.0.1-alpha.16 · Related issues · -View source +View source @@ -611,7 +611,7 @@ old_func() # error: [deprecated] Default level: ignore · Added in 0.0.78 · Related issues · -View source +View source @@ -783,7 +783,7 @@ soundness checks from their type checker, and it may have false positives in som Default level: ignore · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -815,7 +815,7 @@ This rule is currently disabled by default because of the number of false positi Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -846,7 +846,7 @@ class B(A, A): ... # error Default level: error · Added in 0.0.1-alpha.12 · Related issues · -View source +View source @@ -886,7 +886,7 @@ class A: # error Default level: ignore · Added in 0.0.73 · Related issues · -View source +View source @@ -994,7 +994,7 @@ Python code. Default level: error · Added in 0.0.14 · Related issues · -View source +View source @@ -1070,7 +1070,7 @@ def foo() -> "intt\b": ... # error Default level: warn · Added in 0.0.50 · Related issues · -View source +View source @@ -1115,7 +1115,7 @@ def g(value: ~A) -> None: ... # error: [experimental-syntax] Default level: error · Added in 0.0.20 · Related issues · -View source +View source @@ -1149,7 +1149,7 @@ def my_function() -> int: Default level: error · Added in 0.0.15 · Related issues · -View source +View source @@ -1266,7 +1266,7 @@ def test() -> "Literal[5]": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1302,7 +1302,7 @@ class C(A, B): ... # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1331,7 +1331,7 @@ t[3] # error Default level: warn · Added in 0.0.1-alpha.33 · Related issues · -View source +View source @@ -1368,7 +1368,7 @@ class MyClass: ... Default level: error · Added in 0.0.1-alpha.12 · Related issues · -View source +View source @@ -1463,7 +1463,7 @@ will produce instances with an atypical memory layout. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1495,7 +1495,7 @@ func("foo") # error: [invalid-argument-type] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1525,7 +1525,7 @@ a: int = "" # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1606,7 +1606,7 @@ box.value = 1 # okay Default level: error · Added in 0.0.33 · Related issues · -View source +View source @@ -1651,7 +1651,7 @@ class Sub(Base): Default level: error · Added in 0.0.1-alpha.19 · Related issues · -View source +View source @@ -1693,7 +1693,7 @@ asyncio.run(main()) Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1720,7 +1720,7 @@ class A(42): ... # error: [invalid-base] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1749,7 +1749,7 @@ with 1: # error Default level: error · Added in 0.0.12 · Related issues · -View source +View source @@ -1802,7 +1802,7 @@ See: Default level: error · Added in 0.0.13 · Related issues · -View source +View source @@ -1838,7 +1838,7 @@ class A: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1870,7 +1870,7 @@ a: str # error Default level: warn · Added in 0.0.20 · Related issues · -View source +View source @@ -1926,7 +1926,7 @@ class Pet(Enum): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1991,7 +1991,7 @@ This rule corresponds to Ruff's Default level: error · Added in 0.0.1-alpha.28 · Related issues · -View source +View source @@ -2045,7 +2045,7 @@ class D(A): Default level: error · Added in 0.0.1-alpha.35 · Related issues · -View source +View source @@ -2095,7 +2095,7 @@ class NonFrozenChild(FrozenBase): # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2144,7 +2144,7 @@ class D(Generic[U, T]): ... # error Default level: error · Added in 0.0.12 · Related issues · -View source +View source @@ -2239,7 +2239,7 @@ a = 20 / 0 # type: ignore Default level: error · Added in 0.0.83 · Related issues · -View source +View source @@ -2310,7 +2310,7 @@ class LegacyContainer(Generic[T]): Default level: error · Added in 0.0.1-alpha.17 · Related issues · -View source +View source @@ -2357,7 +2357,7 @@ carol = Person(name="Carol", aeg=25) # typo! Default level: warn · Added in 0.0.15 · Related issues · -View source +View source @@ -2418,7 +2418,7 @@ def f(x, y, /): # Python 3.8+ syntax Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2458,7 +2458,7 @@ def f(t: TypeVar("U")): ... # ty: ignore[invalid-type-form] Default level: error · Added in 0.0.18 · Related issues · -View source +View source @@ -2508,7 +2508,7 @@ match object(): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2542,7 +2542,7 @@ class B(metaclass=42): ... # error Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -2659,7 +2659,7 @@ Correct use of `@override` is enforced by ty's [`invalid-explicit-override`](#in Default level: error · Added in 0.0.72 · Related issues · -View source +View source @@ -2697,7 +2697,7 @@ from module import missing # error Default level: error · Added in 0.0.1-alpha.19 · Related issues · -View source +View source @@ -2762,7 +2762,7 @@ TypeError: typing.ClassVar[int] is not valid as type argument Default level: warn · Added in 0.0.31 · Related issues · -View source +View source @@ -2808,7 +2808,7 @@ admin[0] # "Alice" Default level: error · Added in 0.0.1-alpha.27 · Related issues · -View source +View source @@ -2846,7 +2846,7 @@ Baz = NewType("Baz", int | str) # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2903,7 +2903,7 @@ def foo(x: int) -> int: ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2931,7 +2931,7 @@ def f(a: int = ""): ... # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2967,7 +2967,7 @@ P2 = ParamSpec() # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -3019,7 +3019,7 @@ Declare the type variable with `TypeVar("T", covariant=True)` instead. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -3090,7 +3090,7 @@ def g(): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -3122,7 +3122,7 @@ def func() -> int: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -3230,7 +3230,7 @@ class C: ... Default level: error · Added in 0.0.10 · Related issues · -View source +View source @@ -3281,7 +3281,7 @@ class MyClass: Default level: error · Added in 0.0.1-alpha.6 · Related issues · -View source +View source @@ -3327,7 +3327,7 @@ InvalidAlias = TypeAliasType("InvalidAlias", list[T], type_params=(list[T],)) # Default level: error · Added in 0.0.1-alpha.29 · Related issues · -View source +View source @@ -3393,7 +3393,7 @@ Bar[int] # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -3426,7 +3426,7 @@ TYPE_CHECKING = "" # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -3461,7 +3461,7 @@ b: Annotated[int] # error Default level: error · Added in 0.0.1-alpha.11 · Related issues · -View source +View source @@ -3518,7 +3518,7 @@ class C: Default level: error · Added in 0.0.15 · Related issues · -View source +View source @@ -3562,7 +3562,7 @@ def g[U, T: U](): ... # error: [invalid-type-variable-bound] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -3619,7 +3619,7 @@ V = TypeVar("V", list[int], int) # valid constrained Type Default level: error · Added in 0.0.16 · Related issues · -View source +View source @@ -3661,7 +3661,7 @@ U = TypeVar("U", int, str, default=bytes) # error: [invalid-type-variable-defau Default level: error · Added in 0.0.28 · Related issues · -View source +View source @@ -3697,7 +3697,7 @@ class Child(Base): Default level: error · Added in 0.0.14 · Related issues · -View source +View source @@ -3738,7 +3738,7 @@ def f(options: dict[str, object]): Default level: error · Added in 0.0.9 · Related issues · -View source +View source @@ -3772,7 +3772,7 @@ class Foo(TypedDict): Default level: error · Added in 0.0.25 · Related issues · -View source +View source @@ -3806,7 +3806,7 @@ def gen() -> Iterator[int]: Default level: error · Added in 0.0.14 · Related issues · -View source +View source @@ -3872,7 +3872,7 @@ def h(arg2: type): Default level: error · Added in 0.0.15 · Related issues · -View source +View source @@ -3921,7 +3921,7 @@ def g(arg: object): Default level: warn · Added in 0.0.30 · Related issues · -View source +View source @@ -3963,7 +3963,7 @@ Movie = TypedDict("Film", {"title": str}) # error: [mismatched-type-name] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -3994,7 +3994,7 @@ func() # error Default level: ignore · Preview (since 0.0.76) · Related issues · -View source +View source @@ -4080,7 +4080,7 @@ Add `urllib3` to `project.dependencies` if your code imports it directly. Default level: ignore · Added in 0.0.41 · Related issues · -View source +View source @@ -4141,7 +4141,7 @@ class ExplicitChild(Parent): Default level: error · Added in 0.0.75 · Related issues · -View source +View source @@ -4228,7 +4228,7 @@ class Item: Default level: ignore · Added in 0.0.45 · Related issues · -View source +View source @@ -4266,7 +4266,7 @@ def handle(m: re.Match[str]) -> str: Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -4305,7 +4305,7 @@ alice["age"] # KeyError Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -4343,7 +4343,7 @@ func("string") # error: [no-matching-overload] Default level: error · Added in 0.0.30 · Related issues · -View source +View source @@ -4380,7 +4380,7 @@ class Sub(Super): ... # error: [non-callable-init-subclass] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -4471,7 +4471,7 @@ iterating: `async for value in await values(): ...`. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -4499,7 +4499,7 @@ Subscripting an object that does not support it will raise a `TypeError` at runt Default level: error · Added in 0.0.1-alpha.29 · Related issues · -View source +View source @@ -4536,7 +4536,7 @@ class B(A): Default level: error · Added in 0.0.16 · Related issues · -View source +View source @@ -4573,7 +4573,7 @@ class B(A): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -4604,7 +4604,7 @@ f(1, x=2) # error Default level: error · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -4635,7 +4635,7 @@ f(x=1) # error Default level: ignore · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -4673,7 +4673,7 @@ A.c # error Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -4711,7 +4711,7 @@ A()[0] # error Default level: ignore · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -4755,7 +4755,7 @@ from module import a # error Default level: warn · Added in 0.0.23 · Related issues · -View source +View source @@ -4787,7 +4787,7 @@ html.parser # error Default level: ignore · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -4823,7 +4823,7 @@ print(x) # error Default level: warn · Added in 0.0.60 · Related issues · -View source +View source @@ -4898,7 +4898,7 @@ def test() -> "int": Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -4933,7 +4933,7 @@ cast(int, f()) # error Default level: warn · Added in 0.0.79 · Related issues · -View source +View source @@ -5112,13 +5112,21 @@ def find_duplicate_coordinates(coordinates: list[tuple[int, int]]): print(f"Duplicates are {duplicates}") ``` +**See also** + + +- [`truthiness-test-of-callable`](#truthiness-test-of-callable) detects suspicious boolean tests where `Callable`-typed variables + are tested for their truthiness +- [`truthiness-test-of-iterable`](#truthiness-test-of-iterable) detects suspicious boolean tests where `Iterable`-typed variables + are tested for their truthiness + ## `redundant-condition-strict` Default level: ignore · Added in 0.0.79 · Related issues · -View source +View source @@ -5509,13 +5517,21 @@ if DEBUGGING: # error: [redundant-condition-strict] print("Doing debugging stuff...") ``` +**See also** + + +- [`truthiness-test-of-callable`](#truthiness-test-of-callable) detects suspicious boolean tests where `Callable`-typed variables + are tested for their truthiness +- [`truthiness-test-of-iterable`](#truthiness-test-of-iterable) detects suspicious boolean tests where `Iterable`-typed variables + are tested for their truthiness + ## `redundant-final-classvar` Default level: warn · Added in 0.0.18 · Related issues · -View source +View source @@ -5553,7 +5569,7 @@ class C: Default level: error · Added in 0.0.20 · Related issues · -View source +View source @@ -5597,7 +5613,7 @@ class Outer[T]: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -5631,7 +5647,7 @@ static_assert(int(2.0 * 3.0) == 6) # error Default level: warn · Added in 0.0.39 · Related issues · -View source +View source @@ -5683,7 +5699,7 @@ limitation. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -5717,7 +5733,7 @@ class B(A): ... # error Default level: error · Added in 0.0.1-alpha.30 · Related issues · -View source +View source @@ -5757,7 +5773,7 @@ class F(NamedTuple): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -5781,13 +5797,151 @@ def f(): ... f("foo") # error ``` +## `truthiness-test-of-callable` + + +Default level: warn · +Added in 0.0.83 · +Related issues · +View source + + + +**What it does** + + +Detects suspicious truthiness tests of `Callable`-typed values and unions of callable types. + +**Why is this bad?** + + +`Callable`-typed variables are nearly always functions in practice, and functions are always truthy. +If `predicate` is a variable inferred as having a `Callable` type, therefore, a boolean test such as +`if predicate:` is usually not what you want; `if predicate()` (or similar) is usually what was +intended. + +**Examples** + + +```py +from collections.abc import Callable + + +def announce_if_ready(is_ready: Callable[[], bool]): + if is_ready: # error: [truthiness-test-of-callable] + print("Ready") +``` + +You probably meant to call the value instead: + +```py +def announce_if_ready_fixed(is_ready: Callable[[], bool]): + if is_ready(): # no diagnostic + print("Ready") +``` + +**See also** + + +- [`redundant-condition`](#redundant-condition) and [`redundant-condition-strict`](#redundant-condition-strict) detect conditions that can be inferred as + always being truthy or falsy +- [`truthiness-test-of-iterable`](#truthiness-test-of-iterable) detects suspicious boolean tests where `Iterable`-typed variables + are tested for their truthiness + +## `truthiness-test-of-iterable` + + +Default level: warn · +Added in 0.0.83 · +Related issues · +View source + + + +**What it does** + + +Detects boolean conditions where variables typed as `Iterable`, `Iterator`, `Generator` or similar +are tested for their truthiness. + +**Why is this bad?** + + +Testing an `Iterable` object for truthiness strongly suggests that the code expects the object to +evaluate as falsy in a boolean context if it is empty. However, even empty `Iterable` objects can be +truthy if they do not define `__len__` or `__bool__`. The classic example of this is a generator: +unevaluated generators in Python are always truthy, even if they do not yield any elements at +runtime. + +**Examples** + + +```py +from collections.abc import Iterable + + +def process(items: Iterable[int]): + if items: # error: [truthiness-test-of-iterable] + print("Received items") + else: + print("Didn't receive any items") + + +# prints "Received items", even though the passed-in generator is empty! +process(x for x in range(42) if x > 43) +``` + +If `process` in the above example does not need to accept generators, one solution is to rewrite the +annotation to use `Collection` instead of `Iterable`. `Collection` mandates that the object passed +in must define `__len__`, making a truthiness test much more likely to be meaningful: + +```py +from collections.abc import Collection + + +def process(items: Collection[int]): + if items: # no diagnostic + print("Received items") + else: + print("Didn't receive any items") + + +# passing in a generator is now rejected: +# error: [invalid-argument-type] "Expected `Collection[int]`, found `GeneratorType[int, None, None]`" +process(x for x in range(42) if x > 43) +``` + +If the function must also accept generators, another solution can be to collect the iterable into a +tuple or list before testing its length: + +```py +def process(items: Iterable[int]): + collected = tuple(items) + if collected: # no diagnostic + print("Received items") + else: + print("Didn't receive any items") + + +# correctly prints "Didn't receive any items" +process(x for x in range(42) if x > 43) +``` + +**See also** + + +- [`redundant-condition`](#redundant-condition) and [`redundant-condition-strict`](#redundant-condition-strict) detect conditions that can be inferred as + always being truthy or falsy +- [`truthiness-test-of-callable`](#truthiness-test-of-callable) detects suspicious boolean tests where `Callable`-typed variables + are tested for their truthiness + ## `type-assertion-failure` Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -5826,7 +5980,7 @@ def _(x: int): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -5885,7 +6039,7 @@ class A: Default level: error · Added in 0.0.20 · Related issues · -View source +View source @@ -5929,7 +6083,7 @@ class C(Generic[T]): Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -5958,7 +6112,7 @@ reveal_type(1) # revealed: Literal[1] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -5989,7 +6143,7 @@ f(x=1, y=2) # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -6022,7 +6176,7 @@ A().foo # error Default level: warn · Added in 0.0.1-alpha.15 · Related issues · -View source +View source @@ -6097,7 +6251,7 @@ def g(): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -6125,7 +6279,7 @@ import foo # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -6153,7 +6307,7 @@ print(x) # error Default level: ignore · Added in 0.0.73 · Related issues · -View source +View source @@ -6281,7 +6435,7 @@ Python code. Default level: ignore · Added in 0.0.70 · Related issues · -View source +View source @@ -6426,7 +6580,7 @@ Python code. Default level: ignore · Added in 0.0.70 · Related issues · -View source +View source @@ -6573,7 +6727,7 @@ generator boundaries. Default level: warn · Added in 0.0.1-alpha.7 · Related issues · -View source +View source @@ -6619,7 +6773,7 @@ class D(C): ... # error: [unsupported-base] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -6668,7 +6822,7 @@ b1 < b2 < b1 # error Default level: ignore · Added in 0.0.12 · Related issues · -View source +View source @@ -6713,7 +6867,7 @@ def factory(base: type[Base]) -> type: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -6745,7 +6899,7 @@ A() + A() # error Default level: warn · Added in 0.0.21 · Related issues · -View source +View source @@ -6866,7 +7020,7 @@ to `false`. Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -6945,7 +7099,7 @@ def foo(x: int | str) -> int | str: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source diff --git a/crates/ty_python_semantic/mdtest.py.lock b/crates/ty_python_semantic/mdtest.py.lock index c8426b8f5e..6037d82554 100644 --- a/crates/ty_python_semantic/mdtest.py.lock +++ b/crates/ty_python_semantic/mdtest.py.lock @@ -9,10 +9,11 @@ exclude-newer-span = "P7D" [options.exclude-newer-package] astral-dev-toolchain-hyperfine = "2026-08-27T00:00:00Z" astral-dev-toolchain-cargo-insta = "2026-08-27T00:00:00Z" +astral-dev-toolchain-cargo-nextest = "2026-08-27T00:00:00Z" astral-dev-toolchain-cargo-shear = "2026-08-27T00:00:00Z" astral-dev-toolchain-cargo-codspeed = "2026-08-27T00:00:00Z" -astral-dev-toolchain-cargo-nextest = "2026-08-27T00:00:00Z" astral-dev-toolchain-cargo-fuzz = "2026-09-01T00:00:00Z" +astral-dev-toolchain-cargo-deny = "2026-09-15T19:10:00Z" [manifest] requirements = [ diff --git a/crates/ty_python_semantic/resources/lint_docs/redundant-condition-strict.md b/crates/ty_python_semantic/resources/lint_docs/redundant-condition-strict.md index f882254b72..ba404fcb41 100644 --- a/crates/ty_python_semantic/resources/lint_docs/redundant-condition-strict.md +++ b/crates/ty_python_semantic/resources/lint_docs/redundant-condition-strict.md @@ -378,3 +378,10 @@ DEBUGGING = 0 if DEBUGGING: # error: [redundant-condition-strict] print("Doing debugging stuff...") ``` + +## See also + +- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables + are tested for their truthiness +- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables + are tested for their truthiness diff --git a/crates/ty_python_semantic/resources/lint_docs/redundant-condition.md b/crates/ty_python_semantic/resources/lint_docs/redundant-condition.md index e3d0ede976..b4c6fa0d05 100644 --- a/crates/ty_python_semantic/resources/lint_docs/redundant-condition.md +++ b/crates/ty_python_semantic/resources/lint_docs/redundant-condition.md @@ -167,3 +167,10 @@ def find_duplicate_coordinates(coordinates: list[tuple[int, int]]): print(f"Duplicates are {duplicates}") ``` + +## See also + +- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables + are tested for their truthiness +- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables + are tested for their truthiness diff --git a/crates/ty_python_semantic/resources/lint_docs/truthiness-test-of-callable.md b/crates/ty_python_semantic/resources/lint_docs/truthiness-test-of-callable.md new file mode 100644 index 0000000000..df2eadb54d --- /dev/null +++ b/crates/ty_python_semantic/resources/lint_docs/truthiness-test-of-callable.md @@ -0,0 +1,36 @@ +## What it does + +Detects suspicious truthiness tests of `Callable`-typed values and unions of callable types. + +## Why is this bad? + +`Callable`-typed variables are nearly always functions in practice, and functions are always truthy. +If `predicate` is a variable inferred as having a `Callable` type, therefore, a boolean test such as +`if predicate:` is usually not what you want; `if predicate()` (or similar) is usually what was +intended. + +## Examples + +```py +from collections.abc import Callable + + +def announce_if_ready(is_ready: Callable[[], bool]): + if is_ready: # error: [truthiness-test-of-callable] + print("Ready") +``` + +You probably meant to call the value instead: + +```py +def announce_if_ready_fixed(is_ready: Callable[[], bool]): + if is_ready(): # no diagnostic + print("Ready") +``` + +## See also + +- `redundant-condition` and `redundant-condition-strict` detect conditions that can be inferred as + always being truthy or falsy +- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables + are tested for their truthiness diff --git a/crates/ty_python_semantic/resources/lint_docs/truthiness-test-of-iterable.md b/crates/ty_python_semantic/resources/lint_docs/truthiness-test-of-iterable.md new file mode 100644 index 0000000000..63cee4199d --- /dev/null +++ b/crates/ty_python_semantic/resources/lint_docs/truthiness-test-of-iterable.md @@ -0,0 +1,72 @@ +## What it does + +Detects boolean conditions where variables typed as `Iterable`, `Iterator`, `Generator` or similar +are tested for their truthiness. + +## Why is this bad? + +Testing an `Iterable` object for truthiness strongly suggests that the code expects the object to +evaluate as falsy in a boolean context if it is empty. However, even empty `Iterable` objects can be +truthy if they do not define `__len__` or `__bool__`. The classic example of this is a generator: +unevaluated generators in Python are always truthy, even if they do not yield any elements at +runtime. + +## Examples + +```py +from collections.abc import Iterable + + +def process(items: Iterable[int]): + if items: # error: [truthiness-test-of-iterable] + print("Received items") + else: + print("Didn't receive any items") + + +# prints "Received items", even though the passed-in generator is empty! +process(x for x in range(42) if x > 43) +``` + +If `process` in the above example does not need to accept generators, one solution is to rewrite the +annotation to use `Collection` instead of `Iterable`. `Collection` mandates that the object passed +in must define `__len__`, making a truthiness test much more likely to be meaningful: + +```py +from collections.abc import Collection + + +def process(items: Collection[int]): + if items: # no diagnostic + print("Received items") + else: + print("Didn't receive any items") + + +# passing in a generator is now rejected: +# error: [invalid-argument-type] "Expected `Collection[int]`, found `GeneratorType[int, None, None]`" +process(x for x in range(42) if x > 43) +``` + +If the function must also accept generators, another solution can be to collect the iterable into a +tuple or list before testing its length: + +```py +def process(items: Iterable[int]): + collected = tuple(items) + if collected: # no diagnostic + print("Received items") + else: + print("Didn't receive any items") + + +# correctly prints "Didn't receive any items" +process(x for x in range(42) if x > 43) +``` + +## See also + +- `redundant-condition` and `redundant-condition-strict` detect conditions that can be inferred as + always being truthy or falsy +- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables + are tested for their truthiness diff --git a/crates/ty_python_semantic/resources/mdtest/suspicious_conditions.md b/crates/ty_python_semantic/resources/mdtest/suspicious_conditions.md new file mode 100644 index 0000000000..a70b90b2ad --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/suspicious_conditions.md @@ -0,0 +1,1331 @@ +# Suspicious boolean conditions + +This document tests the `truthiness-test-of-callable` and `truthiness-test-of-iterable` rules. The +first warns when a value whose type is `Callable` or a union of callables is tested in a boolean +condition without being called. The second warns when a value typed as `Iterable`, `Iterator`, or +`Generator` is tested for truthiness: a value with one of these types can often be a generator, and +a generator is always truthy even if it is empty. + +Whereas `redundant-condition` and `redundant-condition-strict` rules report conditions that ty +infers as always truthy or always falsy, the rules tested here flag conditions that have ambiguous +truthiness but are nonetheless highly suspicious, and probably indicative of buggy code. + +## Callable values + +Testing a `Callable` does not invoke it. We suggest a call when the value takes no arguments: + +```py +from typing import Callable + +def check(predicate: Callable[[], bool]): + if predicate: # snapshot: truthiness-test-of-callable + pass + if predicate(): # no diagnostic + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:4:8 + | +4 | if predicate: # snapshot: truthiness-test-of-callable + | ^^^^^^^^^ `() -> bool` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `predicate()` + | +3 | def check(predicate: Callable[[], bool]): + - if predicate: # snapshot: truthiness-test-of-callable +4 + if predicate(): # snapshot: truthiness-test-of-callable +5 | pass + | +note: This is an unsafe fix and may change runtime behavior +``` + +When arguments are required, the suggested call leaves them for the user to supply: + +```py +def check_with_argument(predicate: Callable[[int], bool]): + if predicate: # snapshot: truthiness-test-of-callable + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:9:8 + | +9 | if predicate: # snapshot: truthiness-test-of-callable + | ^^^^^^^^^ `(int, /) -> bool` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `predicate(...)` + | +8 | def check_with_argument(predicate: Callable[[int], bool]): + - if predicate: # snapshot: truthiness-test-of-callable +9 + if predicate(...): # snapshot: truthiness-test-of-callable +10 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +## Unions of callable values + +A union of callables still represents callable objects. A union containing `None` may instead be +testing whether the value is present: + +```py +from typing import Callable + +def check_union( + predicate: Callable[[int], bool] | Callable[[str], bool], + optional: Callable[[], bool] | None, +): + if predicate: # error: [truthiness-test-of-callable] + pass + if optional: # no diagnostic + optional() +``` + +If >=1 `Callable` type in a union has parameters, the autofix adding a call is marked as +display-only rather than unsafe: + +```py +def check_union2(predicate: Callable[[], bool] | Callable[[str], bool]): + if predicate: # snapshot: truthiness-test-of-callable + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:12:8 + | +12 | if predicate: # snapshot: truthiness-test-of-callable + | ^^^^^^^^^ `(() -> bool) | ((str, /) -> bool)` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `predicate(...)` + | +11 | def check_union2(predicate: Callable[[], bool] | Callable[[str], bool]): + - if predicate: # snapshot: truthiness-test-of-callable +12 + if predicate(...): # snapshot: truthiness-test-of-callable +13 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +and the same is currently true for callables with the gradual parameter list (`...`), though +arguably an unsafe autofix would be equally fine here: + +```py +# error: [missing-type-argument] "for generic type `Callable`" +def check_gradual(predicate1: Callable[..., bool], predicate2: Callable): + if predicate1: # snapshot: truthiness-test-of-callable + pass + + if predicate2: # snapshot: truthiness-test-of-callable + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:16:8 + | +16 | if predicate1: # snapshot: truthiness-test-of-callable + | ^^^^^^^^^^ `(...) -> bool` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `predicate1(...)` + | +15 | def check_gradual(predicate1: Callable[..., bool], predicate2: Callable): + - if predicate1: # snapshot: truthiness-test-of-callable +16 + if predicate1(...): # snapshot: truthiness-test-of-callable +17 | pass + | +note: This is a display-only fix and is likely to be incorrect + + +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:19:8 + | +19 | if predicate2: # snapshot: truthiness-test-of-callable + | ^^^^^^^^^^ `(...) -> Unknown` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `predicate2(...)` + | +18 | + - if predicate2: # snapshot: truthiness-test-of-callable +19 + if predicate2(...): # snapshot: truthiness-test-of-callable +20 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +## Unions with functions and bound methods + +A union of a `Callable` type and a function-literal function is still suspicious in a boolean test. +The function is always truthy, and testing the selected value does not call it: + +```py +from typing import Callable + +def default() -> str: + return "ready" + +def check(callback: Callable[[], bool], flag: bool): + selected = callback if flag else default + reveal_type(selected) # revealed: (() -> bool) | (def default() -> str) + if selected: # error: [truthiness-test-of-callable] + pass +``` + +Bound methods are also always truthy. The suggested call does not need an argument for `self`, since +the method is already bound to its instance: + +```py +class Handler: + def ready(self) -> str: + return "ready" + +def check_method(callback: Callable[[], bool], handler: Handler, flag: bool): + selected = callback if flag else handler.ready + reveal_type(selected) # revealed: (() -> bool) | (bound method Handler.ready() -> str) + if selected: # snapshot: truthiness-test-of-callable + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:18:8 + | +18 | if selected: # snapshot: truthiness-test-of-callable + | ^^^^^^^^ `(() -> bool) | (bound method Handler.ready() -> str)` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `selected()` + | +17 | reveal_type(selected) # revealed: (() -> bool) | (bound method Handler.ready() -> str) + - if selected: # snapshot: truthiness-test-of-callable +18 + if selected(): # snapshot: truthiness-test-of-callable +19 | pass + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Unions with callable instances + +An always-truthy instance remains suspicious when its `__call__` attribute has several possible +signatures. If any signature requires an argument, the suggested call leaves it for the user to +supply: + +```py +from typing import final, Callable + +@final +class CallbackAlternatives: + __call__: Callable[["CallbackAlternatives"], str] | Callable[["CallbackAlternatives", int], bytes] + +def check_alternatives(callback: Callable[[], bool] | CallbackAlternatives): + if callback: # snapshot: truthiness-test-of-callable + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:8:8 + | +8 | if callback: # snapshot: truthiness-test-of-callable + | ^^^^^^^^ `(() -> bool) | CallbackAlternatives` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `callback(...)` + | +7 | def check_alternatives(callback: Callable[[], bool] | CallbackAlternatives): + - if callback: # snapshot: truthiness-test-of-callable +8 + if callback(...): # snapshot: truthiness-test-of-callable +9 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +## Callable type aliases + +Type aliases do not change whether a callable's truthiness test is suspicious. Both legacy aliases +and PEP 695 aliases receive the same warning as their underlying callable types, including aliases +of unions and aliases used within unions: + +```toml +[environment] +python-version = "3.12" +``` + +```py +from collections.abc import Callable +from typing import TypeAlias + +LegacyCallback: TypeAlias = Callable[[], bool] +type Callback = Callable[[], bool] +type Callbacks = Callback | Callable[[int], bool] + +def check( + legacy: LegacyCallback, + callback: Callback, + callbacks: Callbacks, + mixed: Callbacks | Callable[[str], bool], +): + if legacy: # error: [truthiness-test-of-callable] + pass + if callback: # error: [truthiness-test-of-callable] + pass + if callbacks: # error: [truthiness-test-of-callable] + pass + if mixed: # error: [truthiness-test-of-callable] + pass +``` + +An alias that includes `None` can still be tested to check whether a callback is present: + +```py +type OptionalCallback = Callback | None + +def check_optional(callback: OptionalCallback): + if callback: + callback() +``` + +## Awaitable return values + +Calling a callable that returns a coroutine turns the boolean test from a suspicious one into one +that tests an always-truthy value. That's also probably not what the user intended, so, in an +asynchronous function, we suggest additionally awaiting the result of the call where it's +appropriate: + +```py +from typing import Any, Callable, Coroutine + +async def check(predicate: Callable[[], Coroutine[Any, Any, bool]]): + if predicate: # snapshot: truthiness-test-of-callable + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:4:8 + | +4 | if predicate: # snapshot: truthiness-test-of-callable + | ^^^^^^^^^ `() -> Coroutine[Any, Any, bool]` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call and await this callable? +help: Replace with `await predicate()` + | +3 | async def check(predicate: Callable[[], Coroutine[Any, Any, bool]]): + - if predicate: # snapshot: truthiness-test-of-callable +4 + if await predicate(): # snapshot: truthiness-test-of-callable +5 | pass + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Iterable values + +Testing an `Iterable` for truthiness is a common footgun, since a variable typed as an `Iterable` +could be a generator, in which case it would be truthy even if it were empty: + +```py +from typing import Iterable + +# error: [missing-type-argument] "for generic class `Iterable`" +def check_items(items: Iterable[int], items2: Iterable): + if items: # snapshot: truthiness-test-of-iterable + pass + + if items2: # error: [truthiness-test-of-iterable] + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:5:8 + | +4 | def check_items(items: Iterable[int], items2: Iterable): + | ------------- Inferred as `Iterable[int]` due to this annotation +5 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[int]` as the annotation on line 4 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | +1 + import collections.abc +2 | from typing import Iterable +3 | +4 | # error: [missing-type-argument] "for generic class `Iterable`" + - def check_items(items: Iterable[int], items2: Iterable): +5 + def check_items(items: collections.abc.Collection[int], items2: Iterable): +6 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior +``` + +A `Collection`-typed variable, on the other hand, cannot be a generator, and the `Collection` +protocol can only be inhabited by objects that define `__len__`, so we emit no diagnostic here: + +```py +from typing import Collection + +def check2(items: Collection[int]): + if items: # no diagnostic + pass +``` + +`Iterator` can also describe empty generators. We only suggest replacing an annotation with +`Collection` when it describes an `Iterable`: `Collection` preserves all operations that `Iterable` +provides, but it does not provide the `__next__` method required by `Iterator`. Replacing `Iterator` +with `Collection` would therefore make the call to `next(iterator)` below invalid: + +```py +from typing import Iterator + +# error: [missing-type-argument] "for generic class `Iterator`" +def check_iterators(iterator: Iterator[int], iterator2: Iterator): + if iterator: # snapshot: truthiness-test-of-iterable + next(iterator) + if iterator2: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:19:8 + | +18 | def check_iterators(iterator: Iterator[int], iterator2: Iterator): + | ------------- Inferred as `Iterator[int]` due to this annotation +19 | if iterator: # snapshot: truthiness-test-of-iterable + | ^^^^^^^^ `Iterator[int]` object tested for truthiness +info: `Iterator[int]` objects can be generators, and generators are truthy even when empty +help: Test the length of the iterable instead of its truthiness + | +18 | def check_iterators(iterator: Iterator[int], iterator2: Iterator): + - if iterator: # snapshot: truthiness-test-of-iterable +19 + if len(tuple(iterator)): # snapshot: truthiness-test-of-iterable +20 | next(iterator) + | +note: This is a display-only fix and is likely to be incorrect + + +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:21:8 + | +18 | def check_iterators(iterator: Iterator[int], iterator2: Iterator): + | -------- Inferred as `Iterator[Unknown]` due to this annotation +19 | if iterator: # snapshot: truthiness-test-of-iterable +20 | next(iterator) +21 | if iterator2: # snapshot: truthiness-test-of-iterable + | ^^^^^^^^^ `Iterator[Unknown]` object tested for truthiness +info: `Iterator[Unknown]` objects can be generators, and generators are truthy even when empty +help: Test the length of the iterable instead of its truthiness + | +20 | next(iterator) + - if iterator2: # snapshot: truthiness-test-of-iterable +21 + if len(tuple(iterator2)): # snapshot: truthiness-test-of-iterable +22 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +as can `Generator`: + +```py +from typing import Generator + +# error: [missing-type-argument] "for generic class `Generator`" +def check_generators(generator: Generator[int, None, None], generator2: Generator): + if generator: # snapshot: truthiness-test-of-iterable + pass + if generator2: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:27:8 + | +26 | def check_generators(generator: Generator[int, None, None], generator2: Generator): + | -------------------------- Inferred as `Generator[int, None, None]` due to this annotation +27 | if generator: # snapshot: truthiness-test-of-iterable + | ^^^^^^^^^ `Generator[int, None, None]` object tested for truthiness +info: `Generator[int, None, None]` objects can be generators, and generators are truthy even when empty +help: Test the length of the iterable instead of its truthiness + | +26 | def check_generators(generator: Generator[int, None, None], generator2: Generator): + - if generator: # snapshot: truthiness-test-of-iterable +27 + if len(tuple(generator)): # snapshot: truthiness-test-of-iterable +28 | pass + | +note: This is a display-only fix and is likely to be incorrect + + +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:29:8 + | +26 | def check_generators(generator: Generator[int, None, None], generator2: Generator): + | --------- Inferred as `Generator[Unknown, None, None]` due to this annotation +27 | if generator: # snapshot: truthiness-test-of-iterable +28 | pass +29 | if generator2: # snapshot: truthiness-test-of-iterable + | ^^^^^^^^^^ `Generator[Unknown, None, None]` object tested for truthiness +info: `Generator[Unknown, None, None]` objects can be generators, and generators are truthy even when empty +help: Test the length of the iterable instead of its truthiness + | +28 | pass + - if generator2: # snapshot: truthiness-test-of-iterable +29 + if len(tuple(generator2)): # snapshot: truthiness-test-of-iterable +30 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +So can custom protocols that are subtypes of `Iterable[object]` but supertypes of all +`GeneratorType` specializations. We do not suggest replacing these with `Collection` either, since +that could remove members such as `gi_running` in this example: + +```py +from typing import Protocol, Iterator + +class GeneratorLike(Protocol): + def __iter__(self) -> Iterator[int]: ... + @property + def gi_running(self) -> bool: ... + +def check_custom(x: GeneratorLike): + if x: # snapshot: truthiness-test-of-iterable + print(x.gi_running) +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:39:8 + | +38 | def check_custom(x: GeneratorLike): + | ------------- Inferred as `GeneratorLike` due to this annotation +39 | if x: # snapshot: truthiness-test-of-iterable + | ^ `GeneratorLike` object tested for truthiness +info: `GeneratorLike` objects can be generators, and generators are truthy even when empty +help: Test the length of the iterable instead of its truthiness + | +38 | def check_custom(x: GeneratorLike): + - if x: # snapshot: truthiness-test-of-iterable +39 + if len(tuple(x)): # snapshot: truthiness-test-of-iterable +40 | print(x.gi_running) + | +note: This is a display-only fix and is likely to be incorrect +``` + +and some intersection types: + +```py +from ty_extensions import Intersection, Not + +class Unrelated: ... + +def check_intersection(x: Intersection[GeneratorLike, Not[Unrelated]]): + if x: # error: [truthiness-test-of-iterable] + pass +``` + +and even certain unions: + +```py +from typing import Mapping + +def check_unions(x: Iterable[int] | Mapping[str, str], y: Intersection[GeneratorLike, Not[Unrelated]] | Mapping[str, str]): + # revealed: Iterable[int] | Mapping[str, str] + if reveal_type(x): # error: [truthiness-test-of-iterable] + pass + + # revealed: (GeneratorLike & ~Unrelated) | Mapping[str, str] + if reveal_type(y): # error: [truthiness-test-of-iterable] + pass +``` + +## Annotated iterable attributes + +An attribute's annotation can explain its inferred type, just as a parameter annotation can. In the +below example, we offer a suggestion that points to the annotated assignment that declares the +attribute: + +```py +from collections.abc import Iterable + +class Container: + items: Iterable[int] + +def check(container: Container): + if container.items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:7:8 + | +4 | items: Iterable[int] + | ------------- Inferred as `Iterable[int]` due to this annotation +5 | +6 | def check(container: Container): +7 | if container.items: # snapshot: truthiness-test-of-iterable + | ^^^^^^^^^^^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[int]` as the annotation on line 4 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from collections.abc import Iterable +1 + from collections.abc import Iterable, Collection +2 | +3 | class Container: + - items: Iterable[int] +4 + items: Collection[int] +5 | + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Collection fixes preserve element annotations + +Changing `Iterable` to `Collection` preserves the spelling of the element type. An imported type can +be available only under an alias, and the iterable itself can also be imported under an alias: + +```py +from collections.abc import Iterable as I +from pathlib import Path as P + +def check(items: I[P]): + if items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:5:8 + | +4 | def check(items: I[P]): + | ---- Inferred as `Iterable[Path]` due to this annotation +5 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[Path]` object tested for truthiness +info: `Iterable[Path]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[P]` as the annotation on line 4 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from collections.abc import Iterable as I +1 + from collections.abc import Iterable as I, Collection +2 | from pathlib import Path as P +3 | + - def check(items: I[P]): +4 + def check(items: Collection[P]): +5 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior +``` + +Qualified names are preserved too, so the fix does not require new imports for the element type: + +```py +import datetime +import typing + +def check_qualified(items: typing.Iterable[datetime.date]): + if items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:11:8 + | +10 | def check_qualified(items: typing.Iterable[datetime.date]): + | ------------------------------ Inferred as `Iterable[date]` due to this annotation +11 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[date]` object tested for truthiness +info: `Iterable[date]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[datetime.date]` as the annotation on line 10 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from collections.abc import Iterable as I +1 + from collections.abc import Iterable as I, Collection +2 | from pathlib import Path as P +-------------------------------------------------------------------------------- +9 | + - def check_qualified(items: typing.Iterable[datetime.date]): +10 + def check_qualified(items: Collection[datetime.date]): +11 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Collection fixes preserve forward references + +A quoted annotation remains quoted when its iterable type is replaced. Removing the quotes would +evaluate the reference to `Later` before the class is defined: + +```toml +[environment] +python-version = "3.12" +``` + +```py +from collections.abc import Iterable + +def check(items: "Iterable[Later]"): + if items: # snapshot: truthiness-test-of-iterable + pass + +class Later: ... +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:4:8 + | +3 | def check(items: "Iterable[Later]"): + | ----------------- Inferred as `Iterable[Later]` due to this annotation +4 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[Later]` object tested for truthiness +info: `Iterable[Later]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[Later]` as the annotation on line 3 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from collections.abc import Iterable +1 + from collections.abc import Iterable, Collection +2 | + - def check(items: "Iterable[Later]"): +3 + def check(items: "Collection[Later]"): +4 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior +``` + +The element annotation can also be quoted on its own. The fix preserves these quotes as well: + +```py +def check_element(items: Iterable["Another"]): + if items: # snapshot: truthiness-test-of-iterable + pass + +class Another: ... +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:9:8 + | +8 | def check_element(items: Iterable["Another"]): + | ------------------- Inferred as `Iterable[Another]` due to this annotation +9 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[Another]` object tested for truthiness +info: `Iterable[Another]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection["Another"]` as the annotation on line 8 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from collections.abc import Iterable +1 + from collections.abc import Iterable, Collection +2 | +-------------------------------------------------------------------------------- +7 | class Later: ... + - def check_element(items: Iterable["Another"]): +8 + def check_element(items: Collection["Another"]): +9 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Collection fixes use the annotation's scope + +An import that is visible where an attribute is tested can be shadowed in the class containing its +annotation. The fix uses a qualified name when the class shadows `Collection`: + +```py +from collections.abc import Collection, Iterable + +class Container: + Collection = int + items: Iterable[int] + +def check(container: Container): + if container.items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:8:8 + | +5 | items: Iterable[int] + | ------------- Inferred as `Iterable[int]` due to this annotation +6 | +7 | def check(container: Container): +8 | if container.items: # snapshot: truthiness-test-of-iterable + | ^^^^^^^^^^^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[int]` as the annotation on line 5 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | +1 + import collections.abc +2 | from collections.abc import Collection, Iterable +3 | +4 | class Container: +5 | Collection = int + - items: Iterable[int] +6 + items: collections.abc.Collection[int] +7 | + | +note: This is an unsafe fix and may change runtime behavior +``` + +A method's parameter annotations are also evaluated in the class scope, even though the condition is +evaluated in the method's scope: + +```py +class Checker: + Collection = int + + def check(self, items: Iterable[int]): + if items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:14:12 + | +13 | def check(self, items: Iterable[int]): + | ------------- Inferred as `Iterable[int]` due to this annotation +14 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[int]` as the annotation on line 13 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | +1 + import collections.abc +2 | from collections.abc import Collection, Iterable +-------------------------------------------------------------------------------- +13 | + - def check(self, items: Iterable[int]): +14 + def check(self, items: collections.abc.Collection[int]): +15 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Collection suggestions for wrapped annotations + +Replacing the outer name in an `Annotated` annotation would discard its metadata and produce an +invalid `Collection` specialization. For these annotations, we suggest reworking the annotation +without offering an annotation edit: + +```py +from collections.abc import Iterable +from typing import Annotated + +def check(items: Annotated[Iterable[int], "metadata"]): + if items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:5:8 + | +4 | def check(items: Annotated[Iterable[int], "metadata"]): + | ------------------------------------ Inferred as `Iterable[int]` due to this annotation +5 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Consider reworking the annotation on line 4 to use `collections.abc.Collection` +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | +4 | def check(items: Annotated[Iterable[int], "metadata"]): + - if items: # snapshot: truthiness-test-of-iterable +5 + if len(tuple(items)): # snapshot: truthiness-test-of-iterable +6 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +## Collection fixes before Python 3.9 + +Before Python 3.9, `collections.abc.Collection` cannot be subscripted at runtime. For an eagerly +evaluated annotation, the fix imports `Collection` from `typing`: + +```toml +[environment] +python-version = "3.8" +``` + +```py +from typing import Iterable + +def check(items: Iterable[int]): + if items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:4:8 + | +3 | def check(items: Iterable[int]): + | ------------- Inferred as `Iterable[int]` due to this annotation +4 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Use `typing.Collection[int]` as the annotation on line 3 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from typing import Iterable +1 + from typing import Iterable, Collection +2 | + - def check(items: Iterable[int]): +3 + def check(items: Collection[int]): +4 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior +``` + +The fix also preserves legacy spellings of nested types. Using `list[int]` or `int | str` in an +eagerly evaluated annotation would fail at runtime on Python 3.8: + +```py +from typing import List, Union + +def check_nested(items: Iterable[List[int]]): + if items: # snapshot: truthiness-test-of-iterable + pass + +def check_union(items: Iterable[Union[int, str]]): + if items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:9:8 + | +8 | def check_nested(items: Iterable[List[int]]): + | ------------------- Inferred as `Iterable[list[int]]` due to this annotation +9 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[list[int]]` object tested for truthiness +info: `Iterable[list[int]]` objects can be generators, and generators are truthy even when empty +help: Use `typing.Collection[List[int]]` as the annotation on line 8 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from typing import Iterable +1 + from typing import Iterable, Collection +2 | +-------------------------------------------------------------------------------- +7 | + - def check_nested(items: Iterable[List[int]]): +8 + def check_nested(items: Collection[List[int]]): +9 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior + + +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:13:8 + | +12 | def check_union(items: Iterable[Union[int, str]]): + | ------------------------- Inferred as `Iterable[int | str]` due to this annotation +13 | if items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[int | str]` object tested for truthiness +info: `Iterable[int | str]` objects can be generators, and generators are truthy even when empty +help: Use `typing.Collection[Union[int, str]]` as the annotation on line 12 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from typing import Iterable +1 + from typing import Iterable, Collection +2 | +-------------------------------------------------------------------------------- +11 | + - def check_union(items: Iterable[Union[int, str]]): +12 + def check_union(items: Collection[Union[int, str]]): +13 | if items: # snapshot: truthiness-test-of-iterable + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Iterable annotations in other first-party files + +When an imported variable's annotation comes from another first-party file, the diagnostic points to +that annotation and includes its relative path and line number in the suggestion: + +`items.py`: + +```py +from collections.abc import Iterable + +values: Iterable[int] +``` + +`main.py`: + +```py +from items import values + +if values: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/main.py:3:4 + | +3 | if values: # snapshot: truthiness-test-of-iterable + | ^^^^^^ `Iterable[int]` object tested for truthiness + | + ::: src/items.py:3:9 + | +3 | values: Iterable[int] + | ------------- Inferred as `Iterable[int]` due to this annotation +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Consider using `collections.abc.Collection[int]` in the annotation on line 3 of src/items.py +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | +2 | + - if values: # snapshot: truthiness-test-of-iterable +3 + if len(tuple(values)): # snapshot: truthiness-test-of-iterable +4 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +## Iterable annotations in dependencies + +An annotation in a third-party library also explains the inferred type. The diagnostic suggests +changing the truthiness test, but does not ask the user to edit the library's annotation: + +```toml +[environment] +python = "/.venv" +``` + +`/.venv//items.pyi`: + +```pyi +from collections.abc import Iterable + +values: Iterable[int] +``` + +`main.py`: + +```py +import items + +if items.values: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/main.py:3:4 + | +3 | if items.values: # snapshot: truthiness-test-of-iterable + | ^^^^^^^^^^^^ `Iterable[int]` object tested for truthiness + | + ::: .venv//items.pyi:3:9 + | +3 | values: Iterable[int] + | ------------- Inferred as `Iterable[int]` due to this annotation +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Test the length of the iterable instead of its truthiness + | +2 | + - if items.values: # snapshot: truthiness-test-of-iterable +3 + if len(tuple(items.values)): # snapshot: truthiness-test-of-iterable +4 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +## Iterable values without variable annotations + +A function call can produce an iterable without an annotated variable at the use site. We still +explain why the truthiness test is suspicious and suggest testing its length: + +```py +from collections.abc import Iterable + +def get_items() -> Iterable[int]: + return [] + +if get_items(): # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:6:4 + | +6 | if get_items(): # snapshot: truthiness-test-of-iterable + | ^^^^^^^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Test the length of the iterable instead of its truthiness + | +5 | + - if get_items(): # snapshot: truthiness-test-of-iterable +6 + if len(tuple(get_items())): # snapshot: truthiness-test-of-iterable +7 | pass + | +note: This is a display-only fix and is likely to be incorrect +``` + +## Nested boolean tests + +Each tested operand receives the appropriate diagnostic, even when the whole condition remains +ambiguous: + +```py +from collections.abc import Callable, Iterable + +def check_nested(flag: bool, predicate: Callable[[], bool], items: Iterable[int]): + if flag and predicate: # snapshot: truthiness-test-of-callable + pass + if flag and items: # snapshot: truthiness-test-of-iterable + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:4:17 + | +4 | if flag and predicate: # snapshot: truthiness-test-of-callable + | ^^^^^^^^^ `() -> bool` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `predicate()` + | +3 | def check_nested(flag: bool, predicate: Callable[[], bool], items: Iterable[int]): + - if flag and predicate: # snapshot: truthiness-test-of-callable +4 + if flag and predicate(): # snapshot: truthiness-test-of-callable +5 | pass + | +note: This is an unsafe fix and may change runtime behavior + + +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:6:17 + | +3 | def check_nested(flag: bool, predicate: Callable[[], bool], items: Iterable[int]): + | ------------- Inferred as `Iterable[int]` due to this annotation +4 | if flag and predicate: # snapshot: truthiness-test-of-callable +5 | pass +6 | if flag and items: # snapshot: truthiness-test-of-iterable + | ^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[int]` as the annotation on line 3 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from collections.abc import Callable, Iterable +1 + from collections.abc import Callable, Iterable, Collection +2 | + - def check_nested(flag: bool, predicate: Callable[[], bool], items: Iterable[int]): +3 + def check_nested(flag: bool, predicate: Callable[[], bool], items: Collection[int]): +4 | if flag and predicate: # snapshot: truthiness-test-of-callable + | +note: This is an unsafe fix and may change runtime behavior +``` + +When both operands are suspicious, we report each tested operand without also warning on the +complete `or` expression: + +```py +def check_two_predicates(first: Callable[[], bool], second: Callable[[], bool]): + # snapshot: truthiness-test-of-callable + # snapshot: truthiness-test-of-callable + if first or second: + pass + +def check_two_iterables(left: Iterable[int], right: Iterable[int]): + # error: [truthiness-test-of-iterable] + # error: [truthiness-test-of-iterable] + if left or right: + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:11:8 + | +11 | if first or second: + | ^^^^^ `() -> bool` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `first()` + | +10 | # snapshot: truthiness-test-of-callable + - if first or second: +11 + if first() or second: +12 | pass + | +note: This is an unsafe fix and may change runtime behavior + + +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:11:17 + | +11 | if first or second: + | ^^^^^^ `() -> bool` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `second()` + | +10 | # snapshot: truthiness-test-of-callable + - if first or second: +11 + if first or second(): +12 | pass + | +note: This is an unsafe fix and may change runtime behavior +``` + +When a condition tests a callable and an iterable, both rules report their respective operands: + +```py +def check_mixed(predicate: Callable[[], bool], items: Iterable[int]): + # snapshot: truthiness-test-of-callable + # snapshot: truthiness-test-of-iterable + if predicate and items: + pass +``` + +```snapshot +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:22:8 + | +22 | if predicate and items: + | ^^^^^^^^^ `() -> bool` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `predicate()` + | +21 | # snapshot: truthiness-test-of-iterable + - if predicate and items: +22 + if predicate() and items: +23 | pass + | +note: This is an unsafe fix and may change runtime behavior + + +warning[truthiness-test-of-iterable]: Suspicious boolean test of an `Iterable` + --> src/mdtest_snippet.py:22:22 + | +19 | def check_mixed(predicate: Callable[[], bool], items: Iterable[int]): + | ------------- Inferred as `Iterable[int]` due to this annotation +20 | # snapshot: truthiness-test-of-callable +21 | # snapshot: truthiness-test-of-iterable +22 | if predicate and items: + | ^^^^^ `Iterable[int]` object tested for truthiness +info: `Iterable[int]` objects can be generators, and generators are truthy even when empty +help: Use `collections.abc.Collection[int]` as the annotation on line 19 +info: A `Collection` must define `__len__`, so `Collection` excludes generators +help: Alternatively, test the length of the iterable instead of its truthiness + | + - from collections.abc import Callable, Iterable +1 + from collections.abc import Callable, Iterable, Collection +2 | +-------------------------------------------------------------------------------- +18 | pass + - def check_mixed(predicate: Callable[[], bool], items: Iterable[int]): +19 + def check_mixed(predicate: Callable[[], bool], items: Collection[int]): +20 | # snapshot: truthiness-test-of-callable + | +note: This is an unsafe fix and may change runtime behavior +``` + +## Suspicious operands in fixed conditions + +A `Callable` or `Iterable` expression has ambiguous truthiness, but nesting such an expression +inside `and False` means the outer expression will always be falsy. The operand's suspicious type +and the fixed truthiness of the complete condition are reported as separate findings. The diagnostic +on the complete condition also explains why the body is unreachable: + +```toml +[rules] +redundant-condition-strict = "error" +``` + +```py +from collections.abc import Callable, Iterable + +def check(predicate: Callable[[], bool], items: Iterable[int]): + # snapshot: truthiness-test-of-callable + # snapshot: redundant-condition-strict + if predicate and False: + print("unreachable") + + # error: [truthiness-test-of-iterable] + # error: [redundant-condition-strict] "Condition `items and False` is always false" + if items and False: + pass +``` + +```snapshot +error[redundant-condition-strict]: Condition is always false + --> src/mdtest_snippet.py:6:8 + | +6 | if predicate and False: + | ^^^^^^^^^^^^^^^^^^^ Inferred type is `((() -> bool) & ~AlwaysTruthy) | Literal[False]` +7 | print("unreachable") + | -------------------- This statement is unreachable + + +warning[truthiness-test-of-callable]: Suspicious boolean test of a `Callable` + --> src/mdtest_snippet.py:6:8 + | +6 | if predicate and False: + | ^^^^^^^^^ `() -> bool` object tested for truthiness +info: Callable objects are usually functions, and functions are always truthy +help: Did you mean to call this callable? +help: Replace with `predicate()` + | +5 | # snapshot: redundant-condition-strict + - if predicate and False: +6 + if predicate() and False: +7 | print("unreachable") + | +note: This is an unsafe fix and may change runtime behavior +``` + +The suspicious operand can also be nested inside an expression whose own truthiness is ambiguous. +The outer `and False` still has a fixed outcome: + +```py +def check_nested_fixed(predicate: Callable[[], bool]): + # error: [truthiness-test-of-callable] + # error: [redundant-condition-strict] + if (not predicate) and False: + pass +``` + +A truthiness test inside a tuple will be independent of the test of the tuple itself, so both the +suspicious operand and the always-truthy tuple can be reported: + +```py +def check_tuple(predicate: Callable[[], bool]): + # error: [truthiness-test-of-callable] + # error: [redundant-condition] + if (not predicate,): + pass +``` diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 2a8046720e..3fcb61d80b 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -195,6 +195,8 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&INVALID_LEGACY_POSITIONAL_PARAMETER); registry.register_lint(&REDUNDANT_CONDITION); registry.register_lint(&REDUNDANT_CONDITION_STRICT); + registry.register_lint(&TRUTHINESS_TEST_OF_CALLABLE); + registry.register_lint(&TRUTHINESS_TEST_OF_ITERABLE); // String annotations registry.register_lint(&ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION); @@ -1405,6 +1407,24 @@ declare_lint! { } } +declare_lint! { + #[doc = include_str!("../../resources/lint_docs/truthiness-test-of-callable.md")] + pub(crate) static TRUTHINESS_TEST_OF_CALLABLE = { + summary: "detects truthiness tests of `Callable`-typed objects", + status: LintStatus::stable("0.0.83"), + default_level: Level::Warn, + } +} + +declare_lint! { + #[doc = include_str!("../../resources/lint_docs/truthiness-test-of-iterable.md")] + pub(crate) static TRUTHINESS_TEST_OF_ITERABLE = { + summary: "detects truthiness tests of `Iterable`-typed objects", + status: LintStatus::stable("0.0.83"), + default_level: Level::Warn, + } +} + /// A collection of type check diagnostics. #[derive(Default, Eq, PartialEq, get_size2::GetSize)] pub struct TypeCheckDiagnostics { diff --git a/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs b/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs index e469f73bb6..50459b2e4e 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/diagnostic.rs @@ -4,8 +4,9 @@ use std::borrow::Cow; use ruff_db::{ diagnostic::{Annotation, Diagnostic, Span, SubDiagnostic, SubDiagnosticSeverity}, - parsed::parsed_module, - source::source_text, + files::{FilePath, FileRange}, + parsed::{ParsedModuleRef, parsed_module, parsed_string_annotation}, + source::{line_index, source_text}, }; use ruff_diagnostics::{Applicability, Edit, Fix}; use ruff_python_ast::{ @@ -20,9 +21,11 @@ use ty_module_resolver::{SearchPath, file_to_module}; use ty_python_core::{ Truthiness, ast_ids::HasScopedUseId, - definition::DefinitionKind, + definition::{Definition, DefinitionKind}, place::PlaceExpr, predicate::{Predicate, PredicateNode}, + scope::FileScopeId, + semantic_index, }; use crate::{ @@ -35,6 +38,7 @@ use crate::{ KnownClass, LintDiagnosticGuard, LintDiagnosticGuardBuilder, MemberLookupPolicy, Type, TypeContext, call::bind::CallableDescription, + context::InferContext, diagnostic::typing_module_for_fix, enum_metadata, function::KnownFunction, @@ -96,7 +100,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let RedundantCondition { expression: test, value_type: test_type, - is_truthy, + truthiness, kind, } = condition; @@ -117,7 +121,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> { test_type.display(db, env) )); } else { - let is_truthy = if *is_truthy { "truthy" } else { "falsy" }; + let is_truthy = if truthiness.is_always_true() { + "truthy" + } else { + "falsy" + }; diagnostic.set_primary_annotation_message(format_args!( "Inferred type `{}` is always {is_truthy}", test_type.display(db, env) @@ -127,6 +135,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let describe_condition = |diagnostic: &mut LintDiagnosticGuard| { let source = source_text(db, self.file()); + let is_truthy = if truthiness.is_always_true() { + "true" + } else { + "false" + }; if source.contains_line_break(test.range()) { diagnostic.set_concise_message(format_args!("Condition is always {is_truthy}")); } else { @@ -186,12 +199,233 @@ impl<'db> TypeInferenceBuilder<'db, '_> { // Short-circuit evaluation can determine a condition's truthiness even when its // value type does not. In that case, describe the condition rather than the type. - let describe_as_condition = - test_type.is_subtype_of(db, env, KnownClass::Bool.to_instance(db, env)) - || test_type.bool(db, env) != Truthiness::from(*is_truthy); + let describe_as_condition = !truthiness.is_ambiguous() + && (test_type.is_subtype_of(db, env, KnownClass::Bool.to_instance(db, env)) + || test_type.bool(db, env) != *truthiness); let builder = self.context.report_lint(rule, test)?; - let diagnostic = if *is_truthy { + + let diagnostic = if let ConditionKind::Callable(callables) = kind { + let mut diagnostic = builder.into_diagnostic("Suspicious boolean test of a `Callable`"); + let source = source_text(db, self.file()); + + if source.contains_line_break(test.range()) { + diagnostic.set_concise_message(format_args!( + "Object of type `{}` might always be truthy (did you mean to call it?)", + test_type.display(db, env) + )); + } else if matches!(test, ast::Expr::Name(_) | ast::Expr::Attribute(_)) { + diagnostic.set_concise_message(format_args!( + "Callable `{}` might always be truthy \ + (has type `{}` -- did you mean to call it?)", + &source[test.range()], + test_type.display(db, env) + )); + } else { + diagnostic.set_concise_message(format_args!( + "Expression `{}` of type `{}` might always be truthy \ + (did you mean to call it?)", + &source[test.range()], + test_type.display(db, env) + )); + } + diagnostic.set_primary_annotation_message(format_args!( + "`{}` object tested for truthiness", + test_type.display(db, env) + )); + diagnostic + .info("Callable objects are usually functions, and functions are always truthy"); + + let should_await = self.should_await_call( + test, + callables.iter().flat_map(|callable| { + callable + .signatures(db) + .iter() + .map(|signature| signature.return_ty) + }), + ); + + if should_await { + diagnostic.help("Did you mean to call and await this callable?"); + } else { + diagnostic.help("Did you mean to call this callable?"); + } + + let uncalled_function = UncalledFunction { + has_parameters: callables + .iter() + .any(|callable| callable.signatures(db).has_parameters()), + should_await, + }; + + uncalled_function.suggest_call(&self.context, &mut diagnostic, test); + + diagnostic + } else if kind == &ConditionKind::Iterable { + let mut diagnostic = + builder.into_diagnostic("Suspicious boolean test of an `Iterable`"); + let source = source_text(db, self.file()); + + if source.contains_line_break(test.range()) { + diagnostic.set_concise_message(format_args!( + "Object might be truthy even if its length is 0 (has type `{}`)", + test_type.display(db, env) + )); + } else { + let kind = if test.is_name_expr() { + "Variable" + } else { + "Expression" + }; + diagnostic.set_concise_message(format_args!( + "{kind} `{}` might be truthy even if its length is 0 (has type `{}`)", + &source[test.range()], + test_type.display(db, env) + )); + } + diagnostic.set_primary_annotation_message(format_args!( + "`{}` object tested for truthiness", + test_type.display(db, env) + )); + + let definition_info = + condition_definition_info(db, self.program_file(), test, |expr| { + self.expression_type(expr) + }); + + let mut first_party_annotation = None; + + if let Some(single_definition) = definition_info.single_definition { + let file = single_definition.python_file(db); + let module = parsed_module(db, file).load(db); + + if let Some(annotation) = + self.matching_type_annotation(single_definition, &module, *test_type) + { + let file = single_definition.file(db); + + diagnostic.annotate( + Annotation::secondary(Span::from(file).with_range(annotation.range())) + .message(format_args!( + "Inferred as `{}` due to this annotation", + test_type.display(db, env) + )), + ); + + if test_type + .known_specialization(db, env, KnownClass::Iterable) + .is_some() + && self.is_first_party_definition(single_definition) + { + first_party_annotation = Some(IterableAnnotation { + range: FileRange::new(file, annotation.range()), + scope: semantic_index(db, single_definition.program_file(db)) + .expression_scope_id(annotation), + iterable_ranges: self + .iterable_annotation_ranges(single_definition, annotation), + }); + } + } + } + + diagnostic.info(format_args!( + "`{}` objects can be generators, \ + and generators are truthy even when empty", + test_type.display(db, env) + )); + + if let Some(IterableAnnotation { + range, + scope, + iterable_ranges, + }) = first_party_annotation + { + let line = line_index(db, range.file()).line_index(range.start()); + let collection_module = if env.python_version(db) >= PythonVersion::PY39 { + "collections.abc" + } else { + "typing" + }; + + let annotation_source = source_text(db, range.file()); + let suggestion = iterable_ranges.map(|ranges| &annotation_source[ranges.element]); + + if range.file() == self.file() { + if let Some(suggestion) = suggestion { + diagnostic.help(format_args!( + "Use `{collection_module}.Collection[{suggestion}]` as the annotation \ + on line {line}" + )); + if let Some(ranges) = iterable_ranges + && let Some(action) = self.context.importer().import_for_diagnostic( + ImportRequest::import_from(collection_module, "Collection"), + scope, + range.start(), + ) + { + diagnostic.set_fix(Fix::unsafe_edits( + Edit::range_replacement( + action.symbol_text().to_string(), + ranges.origin, + ), + action.import().cloned(), + )); + } + } else { + diagnostic.help(format_args!( + "Consider reworking the annotation on line {line} \ + to use `{collection_module}.Collection`", + )); + } + } else { + let path = range.file().path(db); + let rendered_path = match path { + FilePath::System(path) => path + .strip_prefix(db.system().current_directory()) + .unwrap_or(path) + .as_str(), + FilePath::Vendored(_) | FilePath::SystemVirtual(_) => path.as_str(), + }; + if let Some(suggestion) = suggestion { + diagnostic.help(format_args!( + "Consider using `{collection_module}.Collection[{suggestion}]` \ + in the annotation on line {line} of {rendered_path}" + )); + } else { + diagnostic.help(format_args!( + "Consider reworking the annotation on line {line} of {rendered_path} \ + to use `{collection_module}.Collection`", + )); + } + } + diagnostic.info( + "A `Collection` must define `__len__`, \ + so `Collection` excludes generators", + ); + diagnostic.help( + "Alternatively, test the length of the iterable \ + instead of its truthiness", + ); + } else { + diagnostic.help("Test the length of the iterable instead of its truthiness"); + } + + if diagnostic.fix().is_none() { + let semantic_model = SemanticModel::new(db, self.program_file()); + let test_ref = ast::AnyNodeRef::from(*test); + if semantic_model.definitely_has_builtin_binding("len", test_ref) + && semantic_model.definitely_has_builtin_binding("tuple", test_ref) + { + diagnostic.set_fix(Fix::display_only_edits( + Edit::insertion("len(tuple(".to_string(), test.start()), + [Edit::insertion("))".to_string(), test.end())], + )); + } + } + + diagnostic + } else if truthiness.is_always_true() { let add_always_truthy_concise_message = |diagnostic: &mut LintDiagnosticGuard| { if should_quote_test_expression() && let source = source_text(db, self.file()) @@ -242,31 +476,17 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let mut diagnostic = builder.into_diagnostic(format_args!("{function} is always truthy")); - // Add a suggestion and fix that they might have meant to call (and possibly - // also await) this function. - // - // It's true that calling the function might not actually fix this diagnostic - // if the function returns something that is always truthy. They still probably - // meant to call the function, though, so it's still a useful suggestion/fix! - - // A coroutine return type establishes that calling and awaiting the function - // is appropriate. `Any`, `Unknown`, and `Never` do not establish this, even - // though they are assignable to `CoroutineType`. - // Use the top materialization so the unspecified generic arguments do not - // prevent concrete coroutine types from being subtypes. - let coroutine = KnownClass::CoroutineType - .to_instance(db, env) - .top_materialization(db, env); - - let is_awaitable_coro_function = self.can_await_here(test) - && function.signature().iter().all(|signature| { - !signature.return_ty.is_equivalent_to(db, env, Type::Never) - && signature.return_ty.is_subtype_of(db, env, coroutine) - }); + let should_await = self.should_await_call( + test, + function + .signature() + .iter() + .map(|signature| signature.return_ty), + ); let kind = function.kind(); - if is_awaitable_coro_function { + if should_await { diagnostic.set_primary_annotation_message(format_args!( "Did you mean to `await` and call this {kind}?", )); @@ -276,35 +496,12 @@ impl<'db> TypeInferenceBuilder<'db, '_> { )); } - if matches!(test, ast::Expr::Name(_) | ast::Expr::Attribute(_)) { - let (call, applicability) = if function.signature().has_parameters() { - ("(...)", Applicability::DisplayOnly) - } else { - ("()", Applicability::Unsafe) - }; - let call_edit = Edit::insertion(call.to_string(), test.end()); + let uncalled_function = UncalledFunction { + has_parameters: function.signature().has_parameters(), + should_await, + }; - let fix = if is_awaitable_coro_function { - Fix::applicable_edits( - Edit::insertion("await ".to_string(), test.start()), - [call_edit], - applicability, - ) - } else { - Fix::applicable_edit(call_edit, applicability) - }; - let source = source_text(db, self.file()); - let expression_text = &source[test.range()]; - let prefix = if is_awaitable_coro_function { - "await " - } else { - "" - }; - diagnostic.help(format_args!( - "Replace with `{prefix}{expression_text}{call}`" - )); - diagnostic.set_fix(fix); - } + uncalled_function.suggest_call(&self.context, &mut diagnostic, test); diagnostic } else if let Some(tuple_spec) = test_type.tuple_instance_spec(db, env) @@ -687,6 +884,82 @@ impl<'db> TypeInferenceBuilder<'db, '_> { diagnostic } + /// Find a definition's annotation if it matches the type inferred at the use site. + fn matching_type_annotation<'ast>( + &self, + definition: Definition<'db>, + module: &'ast ParsedModuleRef, + inferred_type: Type<'db>, + ) -> Option<&'ast ast::Expr> { + let db = self.db(); + let annotation = match definition.kind(db) { + DefinitionKind::AnnotatedAssignment(assignment) => assignment.annotation(module), + DefinitionKind::Parameter(parameter) => parameter.annotation(module)?, + _ => return None, + }; + (self.annotation_expression_type(definition, annotation)? == inferred_type) + .then_some(annotation) + } + + fn annotation_expression_type( + &self, + definition: Definition<'db>, + expression: &ast::Expr, + ) -> Option> { + let db = self.db(); + match definition.kind(db) { + DefinitionKind::AnnotatedAssignment(_) => { + infer_definition_types(db, definition).try_expression_type(expression) + } + DefinitionKind::Parameter(_) => { + let scope = definition.scope(db).scope(db).parent()?; + let scope_id = scope.to_scope_id(db, definition.program_file(db)); + infer_scope_types(db, scope_id, TypeContext::default()) + .try_expression_type(expression) + } + _ => None, + } + } + + /// Locate the iterable name and its element annotation so a fix can change only the name. + /// Keeping the rest of the source preserves aliases, forward references, and version-specific + /// syntax. Parsed string annotations retain source offsets, including nested quotes. + fn iterable_annotation_ranges( + &self, + definition: Definition<'db>, + annotation: &ast::Expr, + ) -> Option { + let db = self.db(); + match annotation { + ast::Expr::StringLiteral(string) => { + let source = source_text(db, definition.file(db)); + let parsed = + parsed_string_annotation(&source, string.as_single_part_string()?).ok()?; + self.iterable_annotation_ranges(definition, parsed.expr()) + } + ast::Expr::Subscript(subscript) => { + let known_class = self + .annotation_expression_type(definition, &subscript.value)? + .as_class_literal()? + .known(db)?; + + (known_class == KnownClass::Iterable).then_some(IterableAnnotationRanges { + origin: subscript.value.range(), + element: subscript.slice.range(), + }) + } + _ => None, + } + } + + fn is_first_party_definition(&self, definition: Definition<'db>) -> bool { + let db = self.db(); + definition.file(db) == self.file() + || file_to_module(db, definition.program_file(db).resolver_file(db)) + .and_then(|module| module.search_path(db)) + .is_some_and(SearchPath::is_first_party) + } + fn diagnose_single_length_tuple( &self, length: TupleLength, @@ -727,31 +1000,9 @@ impl<'db> TypeInferenceBuilder<'db, '_> { if let Some(single_definition) = definition_info.single_definition { let file = single_definition.python_file(db); - let program_file = single_definition.program_file(db); let module = parsed_module(db, file).load(db); - let annotation_info = match single_definition.kind(db) { - DefinitionKind::AnnotatedAssignment(assignment) => { - let annotation = assignment.annotation(&module); - infer_definition_types(db, single_definition) - .try_expression_type(annotation) - .map(|annotation_type| (annotation, annotation_type)) - } - DefinitionKind::Parameter(parameter) => { - parameter.annotation(&module).and_then(|annotation| { - let scope = single_definition.scope(db).scope(db).parent()?; - let annotation_type = infer_scope_types( - db, - scope.to_scope_id(db, program_file), - TypeContext::default(), - ) - .try_expression_type(annotation)?; - Some((annotation, annotation_type)) - }) - } - _ => None, - }; - if let Some((annotation, annotation_type)) = annotation_info - && annotation_type == node_type + if let Some(annotation) = + self.matching_type_annotation(single_definition, &module, node_type) { let file = single_definition.file(db); let diagnostic_annotation = @@ -767,19 +1018,13 @@ impl<'db> TypeInferenceBuilder<'db, '_> { .to_string_parts(); if suggested_type.is_valid_syntax { - let resolver_file = single_definition.program_file(db).resolver_file(db); - let annotated_in_first_party_code = file == self.file() - || file_to_module(db, resolver_file) - .and_then(|module| module.search_path(db)) - .is_some_and(SearchPath::is_first_party); - let maybe_star = if annotation.is_starred_expr() { "*" } else { "" }; - let annotation = if annotated_in_first_party_code { + let annotation = if self.is_first_party_definition(single_definition) { diagnostic_annotation().message(format_args!( "Did you mean `{maybe_star}{}`?", suggested_type.label @@ -798,6 +1043,32 @@ impl<'db> TypeInferenceBuilder<'db, '_> { } } + /// Whether every callable return type is known to be awaitable in this context. + fn should_await_call( + &self, + expression: &ast::Expr, + return_types: impl IntoIterator>, + ) -> bool { + if !self.can_await_here(expression) { + return false; + } + + let db = self.db(); + let env = self.program_environment(); + + // Use the top materialization so concrete return types can be subtypes regardless of + // the awaitable's generic arguments. `Any`, `Unknown`, and `Never` do not establish + // that a call returns an awaitable. + let awaitable = KnownClass::Awaitable + .to_instance(db, env) + .top_materialization(db, env); + + return_types.into_iter().all(|return_ty| { + !return_ty.is_equivalent_to(db, env, Type::Never) + && return_ty.is_subtype_of(db, env, awaitable) + }) + } + /// Return a [`TextRange`] spanning from `branch_start` up to and including /// the offset of the first newline character after the start of `first_statement`. /// @@ -1007,7 +1278,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let RedundantCondition { expression: test, value_type: _, - is_truthy: _, + truthiness: _, kind, } = condition; @@ -1070,7 +1341,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { } if !implicit_else_is_unreachable - && *kind == ConditionKind::Boolean + && kind.is_boolean() && let Some(clause) = if_stmt.elif_else_clauses.last() && clause.test.as_ref() == Some(test) && !diagnostic.has_applicable_fix(Applicability::DisplayOnly) @@ -1337,3 +1608,76 @@ fn logical_line_end(source: &str, tokens: &Tokens, offset: TextSize) -> TextSize fn first_nontrivial_statement(suite: &[ast::Stmt]) -> Option<&ast::Stmt> { suite.iter().find(|stmt| !is_trivial_statement(stmt)) } + +#[derive(Debug, Clone, Copy)] +struct UncalledFunction { + has_parameters: bool, + should_await: bool, +} + +impl UncalledFunction { + /// Add a suggestion and fix that they might have meant to call (and possibly + /// also await) this function. + /// + /// It's true that calling the function might not actually fix this diagnostic + /// if the function returns something that is always truthy. They still probably + /// meant to call the function, though, so it's still a useful suggestion/fix! + fn suggest_call(self, context: &InferContext, diagnostic: &mut Diagnostic, test: &ast::Expr) { + if matches!(test, ast::Expr::Name(_) | ast::Expr::Attribute(_)) { + let (call, applicability) = if self.has_parameters { + ("(...)", Applicability::DisplayOnly) + } else { + ("()", Applicability::Unsafe) + }; + let call_edit = Edit::insertion(call.to_string(), test.end()); + let prefix = if self.should_await { "await " } else { "" }; + let fix = if self.should_await { + Fix::applicable_edits( + Edit::insertion(prefix.to_string(), test.start()), + [call_edit], + applicability, + ) + } else { + Fix::applicable_edit(call_edit, applicability) + }; + let source = source_text(context.db(), context.file()); + diagnostic.help(format_args!( + "Replace with `{prefix}{}{call}`", + &source[test.range()] + )); + diagnostic.set_fix(fix); + } + } +} + +/// A first-party `Iterable` annotation that can be replaced with `Collection`. +#[derive(Debug)] +struct IterableAnnotation { + range: FileRange, + /// The scope where the annotation's names are resolved. Import validation uses this scope + /// because names can be shadowed differently at the truthiness test. + scope: FileScopeId, + iterable_ranges: Option, +} + +/// Source ranges for replacing `Iterable` while preserving the element annotation. +/// +/// In this function's stringized parameter annotation, `origin` covers `Iterable` inside the +/// string, and `element` covers `P` inside the string: +/// +/// ```python +/// from collections.abc import Iterable +/// from pathlib import Path as P +/// +/// def check(items: "Iterable[P]"): +/// if items: +/// print("Received paths") +/// ``` +/// +/// The fix adds a `Collection` import and replaces `origin`, producing `"Collection[P]"` while +/// preserving the original quotes and the alias `P`. +#[derive(Debug, Clone, Copy)] +struct IterableAnnotationRanges { + origin: TextRange, + element: TextRange, +} diff --git a/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/exemptions.rs b/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/exemptions.rs index 4e61760ef1..939a615fd9 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/exemptions.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/exemptions.rs @@ -190,7 +190,7 @@ impl RedundantConditionContext { ); is_boolean_or_short_circuit - && if condition.is_truthy { + && if condition.truthiness.is_always_true() { falsy_branch } else { truthy_branch diff --git a/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/mod.rs b/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/mod.rs index dfd1c02a0b..c5f3f07dad 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/mod.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/redundant_conditions/mod.rs @@ -1,5 +1,5 @@ -//! Logic for reporting boolean tests that are unintentionally always truthy or always falsy. -//! These may be reported under either `redundant-condition` or `redundant-condition-strict`. +//! Logic for reporting boolean tests with fixed truthiness, or suspicious tests of values typed as +//! `Callable` or `Iterable`. //! //! This module classifies tests and selects which expressions to report. [`exemptions`] handles //! assertions, defensive branches, and environment checks; [`diagnostic`] builds messages and fixes. @@ -25,11 +25,11 @@ //! //! The whole condition is also always truthy, but reporting the fixed truthiness of the outer //! condition as well the fixed truthiness of its subexpressions would flag the same mistake -//! twice. To choose which expression to report, the redundant-condition checker therefore needs -//! to see the context of the complete condition. We therefore wait until the -//! [`TypeInferenceBuilder`] has inferred the whole condition before checking any subexpression -//! boolean tests within it. See [`TypeInferenceBuilder::check_condition_redundancy`] for how a -//! check of a smaller expression is postponed until that point. +//! twice. To choose which expression to report, we therefore need to see the context of the +//! complete condition when checking subexpressions. To do this, we wait until the +//! [`TypeInferenceBuilder`] has inferred types for the whole condition before checking any +//! subexpression boolean tests within it. See [`TypeInferenceBuilder::check_condition_redundancy`] +//! for how a check of a smaller expression is postponed until that point. //! //! A boolean test can also occur inside an expression that is being passed to a function or //! stored in a variable. For example, `not ready` below tests the truthiness of `ready` while @@ -45,10 +45,16 @@ //! //! consume(not ready) # `redundant-condition` on `ready`. //! ``` +//! +//! Note that while `redundant-condition` on a subexpression can suppress a +//! `redundant-condition-strict` warning on an outer expression, `truthiness-test-of-iterable` +//! can only suppress `truthiness-test-of-iterable` warnings on an outer expression. The same is +//! true for `truthiness-test-of-callable`. mod diagnostic; mod exemptions; +use bitflags::bitflags; use ruff_python_ast::{ self as ast, helpers::any_over_expr, @@ -58,22 +64,28 @@ use ruff_text_size::Ranged; use ty_python_core::{Truthiness, expression::ExpressionContext, predicate::StatementCall}; use crate::{ + Db, lint::LintMetadata, reachability::{analyze_condition_expression, is_non_terminal_call}, types::{ - KnownClass, KnownInstanceType, Type, - diagnostic::{REDUNDANT_CONDITION, REDUNDANT_CONDITION_STRICT}, + CallableTypes, KnownClass, KnownInstanceType, Type, + constraints::ConstraintSetBuilder, + diagnostic::{ + REDUNDANT_CONDITION, REDUNDANT_CONDITION_STRICT, TRUTHINESS_TEST_OF_CALLABLE, + TRUTHINESS_TEST_OF_ITERABLE, + }, infer::TypeInferenceBuilder, + typevar::TypeVarSet, }, }; use self::exemptions::RedundantConditionContext; -/// Classification of a redundant condition. +/// Classification of a boolean test. /// /// This is used to determine which diagnostic rule would apply to the condition. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum ConditionKind { +#[derive(Debug, PartialEq, Eq)] +enum ConditionKind<'db> { /// A condition whose value type is assignable to `int`, including `bool`. /// /// These tests commonly enforce runtime invariants, so they are only flagged by the @@ -127,18 +139,34 @@ enum ConditionKind { /// print("ready") /// ``` Value, + + /// A value that does not have a known truthiness, + /// but is nonetheless suspicious in a boolean context because it is a subtype + /// of `Iterable[object]` and a supertype of `GeneratorType[Never]`. + Iterable, + + /// A value with ambiguous truthiness whose type is `Callable`, or a union in which + /// every member is either a `Callable` type or another callable type known to be + /// always truthy, such as a function or bound method. + Callable(CallableTypes<'db>), } -impl ConditionKind { +impl ConditionKind<'_> { /// Return the rule responsible for reporting this category of redundant condition. - const fn rule(self) -> &'static LintMetadata { + const fn rule(&self) -> &'static LintMetadata { match self { Self::Value => &REDUNDANT_CONDITION, + Self::Iterable => &TRUTHINESS_TEST_OF_ITERABLE, + Self::Callable(_) => &TRUTHINESS_TEST_OF_CALLABLE, Self::Boolean | Self::ShortCircuit | Self::ContainsWalrus => { &REDUNDANT_CONDITION_STRICT } } } + + const fn is_boolean(&self) -> bool { + matches!(self, Self::Boolean) + } } /// An already-inferred boolean test. @@ -254,7 +282,7 @@ impl BooleanTest<'_, '_> { condition: &RedundantCondition<'_, '_>, ) -> Truthiness { if expression.range() == condition.expression.range() { - return Truthiness::from(condition.is_truthy); + return condition.truthiness; } match expression { @@ -291,21 +319,21 @@ impl BooleanTest<'_, '_> { } } -/// A condition with known truthiness, and the rule category needed to report it. +/// A classified boolean test and the rule category needed to report it. /// -/// We may or may not eventually report a diagnostic for this condition! A condition is classified -/// before we examine the context the condition occurs in, so an instance of this struct represents -/// a "diagnostic candidate" rather than a guarantee that a diagnostic will be emitted. +/// A test is classified before its context is considered, so this struct represents a diagnostic +/// candidate rather than a guarantee that a diagnostic will be emitted. /// -/// The value type is retained for diagnostic messages. The `is_truthy` field describes the condition's -/// outcome, which can depend on short-circuit evaluation as well as the value type (see the doc-comment -/// for [`ExpressionContext`] for more details). +/// The value type is retained for diagnostic messages. The `truthiness` field records what can be +/// inferred about the test's outcome, which can depend on short-circuit evaluation as well as the +/// value type (see the doc-comment for [`ExpressionContext`] for more details). It remains +/// ambiguous for suspicious `Callable` and `Iterable` tests. #[derive(Debug)] struct RedundantCondition<'ast, 'db> { expression: &'ast ast::Expr, value_type: Type<'db>, - is_truthy: bool, - kind: ConditionKind, + truthiness: Truthiness, + kind: ConditionKind<'db>, } /// Determination of whether `redundant-condition-strict` diagnostics should be reported @@ -323,40 +351,26 @@ enum BooleanDiagnosticPreference { EnclosingCondition, } -/// The result of checking a boolean test, indicating whether an expression containing that test -/// should also be checked for redundancy. -/// -/// This is returned by [`TypeInferenceBuilder::check_boolean_test`] to prevent duplicate diagnostics -/// on an expression and its subexpressions. A condition can suppress an enclosing diagnostic even -/// when its own diagnostic is disabled, exempt, or ignored. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum ConditionCheckResult { - /// The containing boolean or conditional expression can still be reported. - /// - /// For example, the boolean result of `isinstance()` is not reported as a subexpression when - /// the complete `not` expression has fixed truthiness: - /// - /// ```python - /// def check(value: str): - /// if not isinstance(value, str): # `redundant-condition-strict` on the whole condition. - /// print("unreachable") - /// ``` - CheckEnclosingCondition, +bitflags! { + /// Biset representing the rules on an outer expression that can be suppressed by a diagnostic + /// on an inner subexpression. + #[derive(Clone, Copy, Debug)] + struct ConditionCheckResult: u8 { + /// A fixed-truthiness operand takes precedence over the entire enclosing test. + const SUPPRESS_ALL = 1 << 0; - /// The containing boolean or conditional expression should not be reported. - /// - /// For example, reporting that `ready` is always truthy suppresses a second diagnostic saying - /// that `not ready` is always false. This also applies if the diagnostic on `ready` is ignored - /// or its rule is disabled: - /// - /// ```python - /// def ready() -> bool: - /// return True - /// - /// if not ready: # Only `ready` is reported, under `redundant-condition`. - /// print("unreachable") - /// ``` - SuppressEnclosingCondition, + /// A suspicious `Callable` operand suppresses only an enclosing callable diagnostic. + const SUPPRESS_CALLABLE = 1 << 1; + + /// A suspicious `Iterable` operand suppresses only an enclosing iterable diagnostic. + const SUPPRESS_ITERABLE = 1 << 2; + } +} + +impl ConditionCheckResult { + const fn suppresses(self, enclosing: Self) -> bool { + self.contains(Self::SUPPRESS_ALL) || self.intersects(enclosing) + } } impl<'db> TypeInferenceBuilder<'db, '_> { @@ -368,14 +382,21 @@ impl<'db> TypeInferenceBuilder<'db, '_> { /// - It's a stub file /// - We're inside a string annotation. Its expressions are absent from the semantic index, /// so we cannot group nested tests to avoid duplicate diagnostics. - /// - Neither `redundant-condition` nor `redundant-condition-strict` is enabled - /// in the user's configuration. + /// - None of the relevant rules are enabled in the user's configuration fn should_check_redundant_conditions(&self) -> bool { + static RELEVANT_RULES: &[&LintMetadata] = &[ + &REDUNDANT_CONDITION, + &REDUNDANT_CONDITION_STRICT, + &TRUTHINESS_TEST_OF_CALLABLE, + &TRUTHINESS_TEST_OF_ITERABLE, + ]; + !self.in_string_annotation() && self.db().should_check_file(self.file()) && !self.file().is_stub(self.db()) - && (self.context.is_lint_enabled(&REDUNDANT_CONDITION) - || self.context.is_lint_enabled(&REDUNDANT_CONDITION_STRICT)) + && RELEVANT_RULES + .iter() + .any(|rule| self.context.is_lint_enabled(rule)) } /// Check a condition, for which types have already been inferred, to see if it is redundant. @@ -641,38 +662,78 @@ impl<'db> TypeInferenceBuilder<'db, '_> { /// Classify a condition using its inferred type and truthiness. /// - /// We return `None` if we cannot determine whether the test is always truthy or always falsy. - /// We also return `None` for literal boolean and integer expressions: a condition written as - /// `True`, `False`, `1`, or `0` is almost certainly deliberate. A variable with an inferred - /// literal type is still classified, however, because its truthiness may be less obvious to - /// the author: + /// We return `Some` if the condition is always truthy, always falsy, or otherwise suspicious + /// in a boolean context due to it being a `Callable` or `Iterable` type. Exemptions are made + /// for literal boolean and integer expressions: although these can always be determined to be + /// either always truthy or always falsy, a condition written as `True`, `False`, `1`, or `0` + /// is almost certainly deliberate. A variable with an inferred literal type is still classified, + /// however, because its truthiness may be less obvious to the author: /// /// ```python - /// from typing import Literal + /// from typing import Literal, Callable, Iterable /// - /// def check(flag: bool, known: Literal[True]): - /// if flag: # No classification: the outcome is unknown. + /// def check( + /// flag: bool, + /// known: Literal[True], + /// callback: Callable[[], bool], + /// items: Iterable[int] + /// ): + /// if flag: # No classification: ambiguous truthiness without a suspicious type. /// pass - /// if True: # No classification: a literal boolean expression. + /// + /// if True: # No classification: a literal boolean expression. /// pass - /// if 1: # No classification: a literal integer expression. + /// + /// if 1: # No classification: a literal integer expression. /// pass - /// if known: # `ConditionKind::Boolean`: the value type is boolean. + /// + /// if known: # `ConditionKind::Boolean`: the value type is boolean. + /// pass + /// + /// if callback: # `ConditionKind::Callable`: ambiguous truthiness. + /// pass + /// + /// if items: # `ConditionKind::Iterable`: ambiguous truthiness. /// pass /// ``` fn classify_redundant_condition<'expr>( &self, test: BooleanTest<'expr, 'db>, ) -> Option> { + /// The bottom specialization for `GeneratorType` + static BOTTOM_SPEC: &[Type] = &[Type::Never, Type::object(), Type::Never]; + + /// The top specialization for `Iterable` + static TOP_SPEC: &[Type] = &[Type::object()]; + + /// Fast path for conditions with ambiguous truthiness, + /// so that we do not have to materialize `GeneratorType` specializations + /// for common cases such as `bool`, `TypeIs` and `TypeGuard` conditions, etc. + fn cannot_satisfy_iterable_check<'db>(db: &'db dyn Db, ty: Type<'db>) -> bool { + match ty { + Type::NominalInstance(_) + | Type::TypeGuard(_) + | Type::TypeIs(_) + | Type::Dynamic(_) + | Type::LiteralValue(_) => true, + Type::Intersection(intersection) => intersection + .positive(db) + .iter() + .any(|&item| cannot_satisfy_iterable_check(db, item)), + Type::Union(union) => union + .elements(db) + .iter() + .all(|&item| cannot_satisfy_iterable_check(db, item)), + _ => false, + } + } + let BooleanTest { expression, value_type, truthiness, .. } = test; - if truthiness.is_ambiguous() { - return None; - } if matches!( expression, @@ -691,7 +752,68 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let db = self.db(); let env = self.program_environment(); - let kind = if value_type.is_assignable_to(db, env, KnownClass::Int.to_instance(db, env)) { + let kind = if truthiness.is_ambiguous() { + let expanded = match value_type.resolve_type_alias(db) { + Type::Union(union) if union.has_aliases(db) => union.expand_aliases(db, env), + ty => ty, + }; + let callables = match expanded { + Type::Callable(callable) => Some(CallableTypes::one(callable)), + Type::Union(union) => { + let mut emit_diagnostic = true; + let union_elements = union.elements(db); + let mut callable_elements = Vec::with_capacity(union_elements.len()); + for element in union_elements { + if let Type::Callable(callable) = element { + callable_elements.push(*callable); + } else if element.bool(db, env).is_always_true() + && let Some(callables) = element.try_upcast_to_callable(db, env) + { + callable_elements.extend(&callables); + } else { + emit_diagnostic = false; + break; + } + } + if emit_diagnostic { + Some(CallableTypes::from_elements(callable_elements)) + } else { + None + } + } + _ => None, + }; + + if let Some(callables) = callables { + ConditionKind::Callable(callables) + } else { + if cannot_satisfy_iterable_check(db, value_type) { + return None; + } + + let builder = ConstraintSetBuilder::new(); + + let generator = + KnownClass::GeneratorType.to_specialized_instance(db, env, BOTTOM_SPEC); + + let check = |source: Type<'db>, target: Type<'db>| { + source.when_subtype_of(db, env, target, &builder, TypeVarSet::None) + }; + + if check(generator, value_type) + .and(db, &builder, || { + let iterable = + KnownClass::Iterable.to_specialized_instance(db, env, TOP_SPEC); + check(value_type, iterable) + }) + .is_always_satisfied(db, env) + { + ConditionKind::Iterable + } else { + return None; + } + } + } else if value_type.is_assignable_to(db, env, KnownClass::Int.to_instance(db, env)) { ConditionKind::Boolean } else if value_type.bool(db, env).is_ambiguous() { ConditionKind::ShortCircuit @@ -705,7 +827,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { Some(RedundantCondition { expression, value_type, - is_truthy: truthiness.is_always_true(), + truthiness, kind, }) } @@ -746,9 +868,10 @@ impl<'db> TypeInferenceBuilder<'db, '_> { /// /// Independent tests within subexpressions are included in the same result. /// - /// If the truthiness of the complete condition is fixed, we only report a single diagnostic - /// for the entire condition. If its truthiness is ambiguous, however, subexpressions with - /// fixed truthiness can be reported individually: + /// When an operand and the complete condition both have fixed truthiness, we select one + /// diagnostic for the mistake. Suspicious `Callable` and `Iterable` operands can instead be + /// reported alongside a complete condition with fixed truthiness. If the complete condition + /// has ambiguous truthiness, subexpressions with fixed truthiness can be reported individually: /// /// ```python /// def check(value: int, flag: bool): @@ -775,22 +898,22 @@ impl<'db> TypeInferenceBuilder<'db, '_> { /// Check `test` and the boolean tests contributing to its outcome. /// - /// Diagnostic selection follows two rules: + /// Diagnostic selection follows these rules: /// /// - Within a condition with fixed truthiness, boolean and short-circuit operands are not /// reported. The enclosing condition represents their redundancy, even if its diagnostic /// is exempt or ignored. - /// - Other operands, such as uncalled functions, take precedence over their enclosing - /// conditions because they identify the likely mistake more directly. + /// - Other operands with fixed truthiness, such as uncalled functions, take precedence over + /// their enclosing conditions because they identify the likely mistake more directly. + /// - Suspicious `Callable` and `Iterable` operands suppress only their own rule on enclosing + /// expressions, leaving the other suspicious rule and fixed-truthiness rules eligible. /// /// The first rule is decided before visiting operands, since the whole condition's truthiness - /// is already known. The second is decided after visiting them, using [`ConditionCheckResult`]. + /// is already known. The others are decided after visiting them, using [`ConditionCheckResult`]. /// - /// Return [`ConditionCheckResult::SuppressEnclosingCondition`] if a caller checking a boolean - /// or conditional expression containing `test` should suppress the diagnostic on that larger - /// expression. For example, the code below receives a diagnostic on `ready` because the - /// function object is always truthy. It does not also receive a diagnostic saying that - /// `not ready` is always false: + /// A fixed-truthiness operand can suppress a diagnostic on the enclosing expression. For + /// example, the code below receives a diagnostic on `ready` because the function object is + /// always truthy. It does not also receive a diagnostic saying that `not ready` is always false: /// /// ```python /// def ready() -> bool: @@ -800,11 +923,10 @@ impl<'db> TypeInferenceBuilder<'db, '_> { /// print("unreachable") /// ``` /// - /// Checking `ready` returns [`ConditionCheckResult::SuppressEnclosingCondition`], so the call - /// checking `not ready` knows not to report a diagnostic regarding the larger expression. + /// Checking `ready` returns [`ConditionCheckResult::SUPPRESS_ALL`], so the call checking + /// `not ready` does not report a diagnostic on the larger expression. A suspicious `Callable` + /// or `Iterable` test instead suppresses only the same rule on enclosing expressions. /// - /// Return the same result if the selected diagnostic is disabled or ignored. Disabling the - /// diagnostic on `ready` should not cause a different diagnostic on `not ready` to appear. /// Conditions selected for reporting are appended to `conditions` in traversal order. fn check_boolean_test<'ast>( &self, @@ -819,18 +941,16 @@ impl<'db> TypeInferenceBuilder<'db, '_> { BooleanDiagnosticPreference::EnclosingCondition }; - let mut operand_result = ConditionCheckResult::CheckEnclosingCondition; + let mut operand_result = ConditionCheckResult::empty(); let mut check_operand = |expression: &'ast ast::Expr, context, conditions: &mut Vec<_>| { - if self.check_boolean_test( + let result = self.check_boolean_test( self.boolean_test(expression, test.evaluation), context, operand_preference, conditions, - ) == ConditionCheckResult::SuppressEnclosingCondition - { - operand_result = ConditionCheckResult::SuppressEnclosingCondition; - } + ); + operand_result |= result; }; match test.expression { @@ -873,7 +993,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { ), } - if operand_result == ConditionCheckResult::SuppressEnclosingCondition { + if operand_result.contains(ConditionCheckResult::SUPPRESS_ALL) { return operand_result; } @@ -884,11 +1004,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> { // Boolean tests against `ty_extensions._internal.ConstraintSet` are (hopefully) // only going to occur in the context of our test suite, and it's very annoying // if we report them as redundant. - return ConditionCheckResult::SuppressEnclosingCondition; + return ConditionCheckResult::SUPPRESS_ALL; } let Some(condition) = self.classify_redundant_condition(test) else { - return ConditionCheckResult::CheckEnclosingCondition; + return operand_result; }; if preference == BooleanDiagnosticPreference::EnclosingCondition @@ -897,7 +1017,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { ConditionKind::Boolean | ConditionKind::ShortCircuit ) { - return ConditionCheckResult::CheckEnclosingCondition; + return operand_result; } // An unreachable operand can retain a literal type, as in `False and "yes"`. @@ -909,16 +1029,28 @@ impl<'db> TypeInferenceBuilder<'db, '_> { .context .is_range_reachable(condition.expression.range()) { - return ConditionCheckResult::CheckEnclosingCondition; + return operand_result; + } + + let result = match &condition.kind { + ConditionKind::Callable(_) => ConditionCheckResult::SUPPRESS_CALLABLE, + ConditionKind::Iterable => ConditionCheckResult::SUPPRESS_ITERABLE, + ConditionKind::Boolean + | ConditionKind::ContainsWalrus + | ConditionKind::ShortCircuit + | ConditionKind::Value => ConditionCheckResult::SUPPRESS_ALL, + }; + + if operand_result.suppresses(result) { + return operand_result; } let rule = condition.kind.rule(); - if self.context.is_lint_enabled(rule) && !condition_context.exempts(self, &condition) { conditions.push(condition); } - ConditionCheckResult::SuppressEnclosingCondition + operand_result | result } /// Find and check boolean tests within subexpressions of `test`. This includes tests inside diff --git a/ty.schema.json b/ty.schema.json index 168878aa08..24bfa6aa69 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -1386,7 +1386,7 @@ }, "redundant-condition": { "title": "detects conditions that are always truthy or always falsey", - "description": "## What it does\n\nDetects boolean conditions where the condition can be statically inferred to be always true or\nalways false due to the inferred type of the condition.\n\nThis rule is enabled by default, and is deliberately not comprehensive. In order to avoid false\npositives, it excludes conditions that meet any of these criteria:\n\n- The boolean test is inferred as evaluating to `True` itself, `False` itself, or an exact integer\n such as `1` or `0`.\n- The boolean test can be inferred as always evaluating to `True` and `False`, but this inference is\n due to boolean-test short-circuiting in `if` conditions, `while` conditions or `assert` tests\n rather than the inferred type of the boolean test.\n- The condition uses a walrus operator (`:=`). The assignment's side effect may be intentional, even\n when its result has fixed truthiness.\n\n## Why is this bad?\n\nA boolean condition that is always true or always false usually indicates a mistake in your code,\nand can often lead to incorrect behavior. If an `if` condition is inferred as always false,\nmoreover, ty will infer all code within that `if` branch as being unreachable, and will not report\nany diagnostics on code in that region.\n\n## Examples\n\nA common error that triggers this rule is to forget to call a function, for example:\n\n```py\nimport random\n\n\ndef should_do_action() -> bool:\n return random.choice([True, False])\n\n\n# oops! You forgot the parentheses here... this should have been `if should_do_action()`.\n# Because it's not, this will always be `True`:\nif should_do_action: # error: [redundant-condition]\n print(\"Doing stuff...\")\n```\n\nAnother common mistake is to forget to `await` a coroutine:\n\n```py\nimport random\n\n\nasync def should_do_async_action():\n return random.choice([True, False])\n\n\nasync def main():\n # oops! Forgot the await here... this should have been `if await should_do_async_action()`.\n # Because it's not, this will always be `True`:\n if should_do_async_action(): # error: [redundant-condition]\n print(\"Doing stuff async...\")\n```\n\nOr to forget that `tuple[X]` means \"A tuple with exactly one element\" rather than \"a tuple with an\narbitrary number of elements\" (for which you'd use `tuple[X, ...]`):\n\n```py\n# you almost certainly meant to write `tuple[str, ...]` here rather than `tuple[str]`...\ndef consume_tuples(x: tuple[str]):\n # ...and that means that this later condition is inferred as always being True by ty:\n if x: # error: [redundant-condition]\n print(\"Got a non-empty tuple\")\n```\n\nSome Pythonistas fall into the trap of thinking that a generator expression will be falsy if it has\nzero elements inside it -- but generator expressions are lazy, and so they're always truthy unless\nyou collect them into a tuple:\n\n```py\ndef test_my_data(data: list[int]):\n # this will always be `True`, because the asserted object is a `types.GeneratorType` instance,\n # not a `tuple`! `assert any(item for item in data if item > 42)`\n # is probably what you meant instead.\n assert (item for item in data if item > 42) # error: [redundant-condition]\n```\n\n## Boolean operators used to compute values\n\nThe rule checks `and` and `or` operands when the expression is used as a condition: in an `if`,\n`elif`, `while`, or `assert` test, a conditional expression, a comprehension filter, a match guard,\nor as the operand of `not`. It does not flag `and` or `or` expressions used to compute values --\neven if an operand in an `and` or `or` expression is always truthy, it doesn't necessarily make the\nexpression redundant:\n\n```py\ndef f(): ...\ndef g(): ...\n\n\ndef test(coinflip: bool):\n # could also be written as `func = f if coinflip else g`,\n # but use of an `and` expression for this is common in older codebases.\n func = coinflip and f or g\n\n # `func` will be the `f` function if `coinflip` is `True`,\n # and the `g` function otherwise\n func()\n```\n\nThis also allows calls that are deliberately always falsy but are used for their side effects:\n\n```py\nfrom unittest.mock import patch\n\n\ndef ask_to_continue() -> bool:\n return input(\"Continue? \") == \"yes\"\n\n\ndef test_ask_to_continue():\n prompts = []\n with patch(\n \"builtins.input\",\n side_effect=lambda prompt: prompts.append(prompt) or \"yes\",\n ):\n assert ask_to_continue()\n\n assert prompts == [\"Continue? \"]\n```\n\nBy contrast, `not` always produces a boolean, so we will still emit a diagnostic on the following\nexample -- negating the truthiness of a function object is pointless, since a function object is\nalways truthy:\n\n```py\ndef f(): ...\n\n\nvalue = not f # error: [redundant-condition]\n```\n\n## Known issues and workarounds\n\nThis rule can sometimes trigger on code that is not incorrect, but could be written in a clearer\nway. For example, the rule will flag this code:\n\n```py\ndef find_duplicate_coordinates(coordinates: list[tuple[int, int]]):\n seen: set[tuple[int, int]] = set()\n # error: [redundant-condition] \"Expression `seen.add(coord)` is always falsy (has type `None`)\"\n duplicates = {coord for coord in coordinates if coord in seen or seen.add(coord)}\n print(f\"Duplicates are {duplicates}\")\n```\n\nThe error here is triggered due to `seen.add(coord)` being used in a boolean expression, despite the\nfact that `set.add()` always returns `None`. Here this is deliberate: `set.add()` is being used for\nits side effect.\n\nTo workaround this issue, the above code could be rewritten like this, which may also be easier for\nsome readers to understand:\n\n```py\ndef find_duplicate_coordinates(coordinates: list[tuple[int, int]]):\n seen: set[tuple[int, int]] = set()\n duplicates: set[tuple[int, int]] = set()\n\n for coord in coordinates:\n if coord in seen:\n duplicates.add(coord)\n else:\n seen.add(coord)\n\n print(f\"Duplicates are {duplicates}\")\n```", + "description": "## What it does\n\nDetects boolean conditions where the condition can be statically inferred to be always true or\nalways false due to the inferred type of the condition.\n\nThis rule is enabled by default, and is deliberately not comprehensive. In order to avoid false\npositives, it excludes conditions that meet any of these criteria:\n\n- The boolean test is inferred as evaluating to `True` itself, `False` itself, or an exact integer\n such as `1` or `0`.\n- The boolean test can be inferred as always evaluating to `True` and `False`, but this inference is\n due to boolean-test short-circuiting in `if` conditions, `while` conditions or `assert` tests\n rather than the inferred type of the boolean test.\n- The condition uses a walrus operator (`:=`). The assignment's side effect may be intentional, even\n when its result has fixed truthiness.\n\n## Why is this bad?\n\nA boolean condition that is always true or always false usually indicates a mistake in your code,\nand can often lead to incorrect behavior. If an `if` condition is inferred as always false,\nmoreover, ty will infer all code within that `if` branch as being unreachable, and will not report\nany diagnostics on code in that region.\n\n## Examples\n\nA common error that triggers this rule is to forget to call a function, for example:\n\n```py\nimport random\n\n\ndef should_do_action() -> bool:\n return random.choice([True, False])\n\n\n# oops! You forgot the parentheses here... this should have been `if should_do_action()`.\n# Because it's not, this will always be `True`:\nif should_do_action: # error: [redundant-condition]\n print(\"Doing stuff...\")\n```\n\nAnother common mistake is to forget to `await` a coroutine:\n\n```py\nimport random\n\n\nasync def should_do_async_action():\n return random.choice([True, False])\n\n\nasync def main():\n # oops! Forgot the await here... this should have been `if await should_do_async_action()`.\n # Because it's not, this will always be `True`:\n if should_do_async_action(): # error: [redundant-condition]\n print(\"Doing stuff async...\")\n```\n\nOr to forget that `tuple[X]` means \"A tuple with exactly one element\" rather than \"a tuple with an\narbitrary number of elements\" (for which you'd use `tuple[X, ...]`):\n\n```py\n# you almost certainly meant to write `tuple[str, ...]` here rather than `tuple[str]`...\ndef consume_tuples(x: tuple[str]):\n # ...and that means that this later condition is inferred as always being True by ty:\n if x: # error: [redundant-condition]\n print(\"Got a non-empty tuple\")\n```\n\nSome Pythonistas fall into the trap of thinking that a generator expression will be falsy if it has\nzero elements inside it -- but generator expressions are lazy, and so they're always truthy unless\nyou collect them into a tuple:\n\n```py\ndef test_my_data(data: list[int]):\n # this will always be `True`, because the asserted object is a `types.GeneratorType` instance,\n # not a `tuple`! `assert any(item for item in data if item > 42)`\n # is probably what you meant instead.\n assert (item for item in data if item > 42) # error: [redundant-condition]\n```\n\n## Boolean operators used to compute values\n\nThe rule checks `and` and `or` operands when the expression is used as a condition: in an `if`,\n`elif`, `while`, or `assert` test, a conditional expression, a comprehension filter, a match guard,\nor as the operand of `not`. It does not flag `and` or `or` expressions used to compute values --\neven if an operand in an `and` or `or` expression is always truthy, it doesn't necessarily make the\nexpression redundant:\n\n```py\ndef f(): ...\ndef g(): ...\n\n\ndef test(coinflip: bool):\n # could also be written as `func = f if coinflip else g`,\n # but use of an `and` expression for this is common in older codebases.\n func = coinflip and f or g\n\n # `func` will be the `f` function if `coinflip` is `True`,\n # and the `g` function otherwise\n func()\n```\n\nThis also allows calls that are deliberately always falsy but are used for their side effects:\n\n```py\nfrom unittest.mock import patch\n\n\ndef ask_to_continue() -> bool:\n return input(\"Continue? \") == \"yes\"\n\n\ndef test_ask_to_continue():\n prompts = []\n with patch(\n \"builtins.input\",\n side_effect=lambda prompt: prompts.append(prompt) or \"yes\",\n ):\n assert ask_to_continue()\n\n assert prompts == [\"Continue? \"]\n```\n\nBy contrast, `not` always produces a boolean, so we will still emit a diagnostic on the following\nexample -- negating the truthiness of a function object is pointless, since a function object is\nalways truthy:\n\n```py\ndef f(): ...\n\n\nvalue = not f # error: [redundant-condition]\n```\n\n## Known issues and workarounds\n\nThis rule can sometimes trigger on code that is not incorrect, but could be written in a clearer\nway. For example, the rule will flag this code:\n\n```py\ndef find_duplicate_coordinates(coordinates: list[tuple[int, int]]):\n seen: set[tuple[int, int]] = set()\n # error: [redundant-condition] \"Expression `seen.add(coord)` is always falsy (has type `None`)\"\n duplicates = {coord for coord in coordinates if coord in seen or seen.add(coord)}\n print(f\"Duplicates are {duplicates}\")\n```\n\nThe error here is triggered due to `seen.add(coord)` being used in a boolean expression, despite the\nfact that `set.add()` always returns `None`. Here this is deliberate: `set.add()` is being used for\nits side effect.\n\nTo workaround this issue, the above code could be rewritten like this, which may also be easier for\nsome readers to understand:\n\n```py\ndef find_duplicate_coordinates(coordinates: list[tuple[int, int]]):\n seen: set[tuple[int, int]] = set()\n duplicates: set[tuple[int, int]] = set()\n\n for coord in coordinates:\n if coord in seen:\n duplicates.add(coord)\n else:\n seen.add(coord)\n\n print(f\"Duplicates are {duplicates}\")\n```\n\n## See also\n\n- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables\n are tested for their truthiness\n- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables\n are tested for their truthiness", "default": "warn", "oneOf": [ { @@ -1396,7 +1396,7 @@ }, "redundant-condition-strict": { "title": "detects conditions that are always truthy or always falsey (strict)", - "description": "## What it does\n\nDetects boolean conditions where the condition can be statically inferred to be always true or\nalways false.\n\nThis rule is disabled by default. It exclusively covers cases that its sibling (enabled-by-default)\nrule `redundant-condition` does not cover. These cases often flag real bugs in user code, but also\nhave a significantly higher rate of unavoidable false positives than other cases.\n\nThis rule reports redundant conditions that meet any of these criteria:\n\n- The boolean test is inferred as evaluating to `True` itself, `False` itself, or an exact integer\n such as `1` or `0`.\n- Short-circuit evaluation means the condition can be guaranteed to be always truthy or always falsy\n despite fixed truthiness not being guaranteed by the inferred type of the expression's value\n (see \"Short-circuiting boolean conditions\" below for an example).\n- The condition uses a walrus operator (`:=`). The assignment's side effect may be intentional, even\n when its result has fixed truthiness.\n\n## Why is this bad?\n\nA boolean condition that is always true or always false usually indicates a mistake in your code,\nand can often lead to incorrect behavior. If an `if` condition is inferred as always false,\nmoreover, ty will infer all code within that `if` branch as being unreachable, and will not report\nany diagnostics on code in that region.\n\n## Examples\n\nA common error in Python code is to make the mistake of thinking that indexing into a `bytes` object\nwill get you an object of type `bytes`. But `bytes` work differently to `str`s in Python -- although\na string is a sequence of strings, a bytestring is a sequence of `int`s, so indexing into a `bytes`\nobject gives you an `int`. This rule can catch that error by alerting you to the fact that checking\nwhether a `bytes` object is unequal to an `int` will always evaluate to `True`:\n\n```py\ndef validate_record(data: bytes) -> None:\n if data[0] != b\"\\x1e\": # error: [redundant-condition-strict]\n raise ValueError(\"Invalid record separator\")\n```\n\nAnother common mistake is to assume that annotating `**kwargs` with `dict[str, str]` describes the\ndictionary containing the keyword arguments. In fact, a `**kwargs` annotation describes each\nindividual keyword argument, so this annotation says that every value is itself a dictionary.\nComparing one of those values with a string will therefore always evaluate to `False`:\n\n```py\ndef trace(**kwargs: dict[str, str]) -> None:\n if kwargs.get(\"operation\") == \"task\": # error: [redundant-condition-strict]\n print(\"Tracing task\")\n```\n\n## Short-circuiting boolean conditions\n\nIn some situations, ty can know that a condition will always be true, or it can know that a\ncondition will always be false, even when this is not guaranteed by the inferred type of that\ncondition. This is because of the way that Python short-circuits evaluation of conditions in the\ncontext of `if` tests, `while` tests and `assert` statements.\n\nConsider a class whose comparison method has an `object` return type:\n\n```py\nfrom typing_extensions import reveal_type\n\n\nclass Comparable:\n def __lt__(self, other: int) -> object: ...\n\n\ndef check(value: Comparable):\n reveal_type(value < 1 < 0) # revealed: ~AlwaysTruthy\n\n if value < 1 < 0: # error: [redundant-condition-strict] \"always false\"\n pass\n```\n\nOutside the context of an `if` test, the revealed type of the condition here is `~AlwaysTruthy`: in\nother words, ty knows that this expression is not *always true*, but cannot guarantee that it is\ndefinitely *always false*. It could be an object that is sometimes true and sometimes false -- for\nexample, a `list` (which is falsy when it is empty, and truthy otherwise).\n\nNonetheless, when `value < 1 < 0` is used directly as a condition, ty knows that the condition will\nalways be falsy and the `if` branch will never be taken. Python tests the truthiness of the object\nreturned by `Comparable.__lt__` once: if it is falsy, the condition fails immediately. If it is\ntruthy, Python evaluates `1 < 0`, which is false. There is no second truthiness test of the object\nreturned by `__lt__`.\n\nIf the chained comparison is saved as a variable first, its value can be the object returned by\n`__lt__`, if that object was falsy when first tested. The `if result` statement then tests that\nobject's truthiness again. A user-defined `__bool__` method can return a different result on that\nsecond call, so ty cannot guarantee that the saved value is still falsy, and no diagnostic is\nemitted:\n\n```py\ndef check_saved(value: Comparable):\n result = value < 1 < 0\n if result: # no diagnostic\n pass\n```\n\n## Exemptions\n\nLike `redundant-condition`, this rule checks subexpressions of an `and` or `or` expression only when\nthe outer expression is used as a condition. This is to avoid emitting false-positive diagnostics on\ncode like the following, where the `and` expression is clearly not redundant despite the fact that\nboth `CONSTANT_1` and `CONSTANT_2` are always truthy:\n\n```py\nfrom typing import Final\n\n\nCONSTANT_1: Final = 1\nCONSTANT_2: Final = 2\n\n\ndef do_something(coinflip: bool):\n # could also be written as `constant_to_use = CONSTANT_1 if coinflip else CONSTANT_2`,\n # but use of an `and` expression for this is common in older codebases.\n constant_to_use = coinflip and CONSTANT_1 or CONSTANT_2\n\n # do something with `constant_to_use` now...\n ...\n```\n\nUnlike `and` and `or`, however, `not` explicitly converts its operand to a boolean, so the rule\nchecks `not` expressions in every context.\n\nAnother exemption applied by this rule concerns `assert`-statement tests. A common pattern in Python\ncode is to use defensive `assert`s to enforce behaviour at runtime, even when the asserted condition\ncan be inferred statically to be always true. For example:\n\n```py\ndef add_one(x: int) -> int:\n assert isinstance(x, int) # no diagnostic\n return x + 1\n```\n\nThis kind of defensive behaviour is often reasonable, since the author of a library cannot guarantee\nthat end users of the library will run a type checker on code calling into the library, meaning that\nit's entirely possible at runtime for an object passed into the `x`a parameter above to be a `str`\n(for example) even though the parameter annotation states that only `int`s can ever be passed in.\nThis rule therefore also exempts all assertion tests or subexpressions that evaluate to a subtype of\n`int` or `bool`:\n\n`redundant-condition-strict` can still trigger on `assert` statements in some contexts, however. For\nexample, `redundant-condition-strict` will be emitted on the below example, where the left-hand side\nof the `and` expression is always true and not a subtype of `bool` or `int`, but where the condition\nis nonetheless excluded from the enabled-by-default `redundant-condition` rule due to the use of the\nwalrus operator:\n\n```py\ndef func() -> bool:\n return True\n\n\ndef test_func():\n assert (result := func) and result != func() # error: [redundant-condition-strict]\n```\n\nFor similar reasons to the `assert` exemptions, this rule also exempts always-false `if` or `elif`\nconditions when their bodies end in a defensive check: a `raise`, an assertion that could fail, a\ncall returning `Never`, an `await` to a call returning `Never`, or `return NotImplemented`:\n\n```py\nimport sys\n\n\ndef add_two(x: int) -> int:\n if not isinstance(x, int): # no diagnostic\n raise TypeError(\"need an int!!\")\n return x + 2\n\n\ndef add_three(x: int) -> int:\n if not isinstance(x, int): # no diagnostic\n assert False, \"unreachable\"\n return x + 3\n\n\ndef add_four(x: int) -> int:\n if not isinstance(x, int): # no diagnostic\n sys.exit(1)\n return x + 4\n\n\nclass Foo:\n def __init__(self, data: int):\n self.data = data\n\n def __add__(self, other: \"Foo\"):\n if not isinstance(other, Foo): # no diagnostic\n return NotImplemented\n return Foo(self.data + other.data)\n```\n\nAnd an exemption is applied for always-true `if` or `elif` statements that are followed by branches\nwhich contain defensive checks:\n\n```py\nfrom typing_extensions import assert_never\n\n\ndef parse_data(data: int | str):\n if isinstance(data, int):\n print(\"got an int\")\n elif isinstance(data, str): # Always true, but no diagnostic, since\n # the `else` branch following this branch is always terminal.\n # (`assert_never` returns `Never`, indicating that it always raises an exception)\n print(\"got a str\")\n else:\n assert_never(data)\n\n\ndef parse_data_early_return(data: int | str):\n if isinstance(data, int):\n print(\"got an int\")\n return\n\n # Always true, but no diagnostic, since\n # the suite following this branch is always terminal\n # (every control-flow path following this `if` statement ends in a `raise` statement)\n if isinstance(data, str):\n print(\"got a str\")\n return\n\n raise AssertionError(\"unexpected data\")\n```\n\nAny conditions defined in relation to `sys.version_info`, `sys.platform`, `os.name` or\n`typing.TYPE_CHECKING` are also exempted. The rule recursively follows the definitions of names and\nattributes across module boundaries to determine if a name or attribute was indirectly defined in\nrelation to one of these highly special-cased symbols:\n\n```toml\n[environment]\npython-version = \"3.14\"\npython-platform = \"linux\"\n```\n\n```py\nimport os\nimport sys\nfrom typing import TYPE_CHECKING\n\nif sys.version_info >= (3, 14): # inferred as always true here, but no diagnostic\n pass\n\nif sys.platform == \"win32\": # inferred as always false here, but no diagnostic\n pass\n\nLINE_ENDING = \"\\n\" if os.name == \"posix\" else \"\\r\\n\"\n\nif LINE_ENDING == \"\\n\": # inferred as always true here, but no diagnostic\n pass\n\nif TYPE_CHECKING: # inferred as always true, but no diagnostic\n pass\n```\n\nConditions involving these constants, or conditions involving values defined in relation to these\nconstants, can often be inferred as always-true or always-false by ty. Indeed, these conditions\nusually *will* be always true or always false across a single invocation run of a Python programme.\nNonetheless, Python code is often written so that it can work on multiple different Python versions\nand/or multiple different operating systems, and a condition that is always true on one operating\nsystem might very well be always false on another operating system (for example). Flagging these\nconditions as being always true or always false would only add noise: the aim of the rule is to flag\nconditions that are *unintentionally* always true or always false.\n\nLastly, some conditions involving literal integers and booleans in the AST are also exempted:\nthere's no reason why you'd use a condition like this unless it was intentional.\n\n```py\nif True: # inferred as always true (obviously), but no diagnostic\n pass\n\nif 0:\n pass # inferred as always false, but no diagnostic\n```\n\n## Known issues and workarounds\n\nThis rule can often trigger on code that is not incorrect, but could be written in a clearer way.\nFor example, the rule will flag this code:\n\n```py\nfrom enum import Enum\n\n\nclass YesOrNo(Enum):\n YES = 1\n NO = 0\n\n\ndef say_yes_or_no(what_to_say: YesOrNo):\n if what_to_say == YesOrNo.YES:\n print(\"yes\")\n elif what_to_say == YesOrNo.NO: # error: [redundant-condition-strict]\n print(\"no\")\n```\n\nThis snippet could be written more clearly as this, which would not trigger the rule owing to the\nexemptions described in the section above:\n\n```py\ndef say_yes_or_no(what_to_say: YesOrNo):\n if what_to_say == YesOrNo.YES:\n print(\"yes\")\n else:\n assert what_to_say == YesOrNo.NO\n print(\"no\")\n```\n\nor the snippet could also be rewritten as this, which would also be fine according to the rule's\nheuristics:\n\n```py\nfrom typing_extensions import assert_never\n\n\ndef say_yes_or_no(what_to_say: YesOrNo):\n if what_to_say == YesOrNo.YES:\n print(\"yes\")\n elif what_to_say == YesOrNo.NO:\n print(\"no\")\n else:\n assert_never(what_to_say)\n```\n\nIn a similar vein, this rule can often flag `and` or `or` expressions that have operands which are\ndeliberately always truthy or deliberately always falsy, because the purpose of the operand is to\nhave some side effect occur. For example:\n\n```py\nimport random\nfrom typing import Literal\n\n\ndef want_to_go_fishing() -> bool:\n return random.choice([True, False])\n\n\ndef weather_report() -> Literal[\"rainy\", \"sunny\", \"cloudy\"]:\n return random.choice([\"rainy\", \"sunny\", \"cloudy\"])\n\n\ndef have_fishing_supplies() -> bool:\n return random.choice([True, False])\n\n\ndef main():\n if (\n want_to_go_fishing()\n and (weather := weather_report()) # error: [redundant-condition-strict]\n and have_fishing_supplies()\n ):\n print(f\"The weather is {weather}, let's go fishing\")\n```\n\nThe middle operand in the above `and` expression is always truthy. This might be deliberate, but\neven if it is, the function would arguably be clearer if it were written like this instead:\n\n```py\ndef main():\n if want_to_go_fishing():\n weather = weather_report()\n if have_fishing_supplies():\n print(f\"The weather is {weather}, let's go fishing\")\n```\n\nLastly, the rule cannot reliably distinguish in all cases comparisons that are intentionally always\ntrue/false from those that are unintentionally always true/false. The rule takes care to avoid\nflagging code that uses `if TYPE_CHECKING`, `if sys.version_info < (X, Y)`, `if sys.platform == ...`\nand `if os.name == ...`. But it cannot reliably determine that code like this was written the way it\nwas meant to be:\n\n```py\nDEBUGGING = 0\n\nif DEBUGGING: # error: [redundant-condition-strict]\n print(\"Doing debugging stuff...\")\n```", + "description": "## What it does\n\nDetects boolean conditions where the condition can be statically inferred to be always true or\nalways false.\n\nThis rule is disabled by default. It exclusively covers cases that its sibling (enabled-by-default)\nrule `redundant-condition` does not cover. These cases often flag real bugs in user code, but also\nhave a significantly higher rate of unavoidable false positives than other cases.\n\nThis rule reports redundant conditions that meet any of these criteria:\n\n- The boolean test is inferred as evaluating to `True` itself, `False` itself, or an exact integer\n such as `1` or `0`.\n- Short-circuit evaluation means the condition can be guaranteed to be always truthy or always falsy\n despite fixed truthiness not being guaranteed by the inferred type of the expression's value\n (see \"Short-circuiting boolean conditions\" below for an example).\n- The condition uses a walrus operator (`:=`). The assignment's side effect may be intentional, even\n when its result has fixed truthiness.\n\n## Why is this bad?\n\nA boolean condition that is always true or always false usually indicates a mistake in your code,\nand can often lead to incorrect behavior. If an `if` condition is inferred as always false,\nmoreover, ty will infer all code within that `if` branch as being unreachable, and will not report\nany diagnostics on code in that region.\n\n## Examples\n\nA common error in Python code is to make the mistake of thinking that indexing into a `bytes` object\nwill get you an object of type `bytes`. But `bytes` work differently to `str`s in Python -- although\na string is a sequence of strings, a bytestring is a sequence of `int`s, so indexing into a `bytes`\nobject gives you an `int`. This rule can catch that error by alerting you to the fact that checking\nwhether a `bytes` object is unequal to an `int` will always evaluate to `True`:\n\n```py\ndef validate_record(data: bytes) -> None:\n if data[0] != b\"\\x1e\": # error: [redundant-condition-strict]\n raise ValueError(\"Invalid record separator\")\n```\n\nAnother common mistake is to assume that annotating `**kwargs` with `dict[str, str]` describes the\ndictionary containing the keyword arguments. In fact, a `**kwargs` annotation describes each\nindividual keyword argument, so this annotation says that every value is itself a dictionary.\nComparing one of those values with a string will therefore always evaluate to `False`:\n\n```py\ndef trace(**kwargs: dict[str, str]) -> None:\n if kwargs.get(\"operation\") == \"task\": # error: [redundant-condition-strict]\n print(\"Tracing task\")\n```\n\n## Short-circuiting boolean conditions\n\nIn some situations, ty can know that a condition will always be true, or it can know that a\ncondition will always be false, even when this is not guaranteed by the inferred type of that\ncondition. This is because of the way that Python short-circuits evaluation of conditions in the\ncontext of `if` tests, `while` tests and `assert` statements.\n\nConsider a class whose comparison method has an `object` return type:\n\n```py\nfrom typing_extensions import reveal_type\n\n\nclass Comparable:\n def __lt__(self, other: int) -> object: ...\n\n\ndef check(value: Comparable):\n reveal_type(value < 1 < 0) # revealed: ~AlwaysTruthy\n\n if value < 1 < 0: # error: [redundant-condition-strict] \"always false\"\n pass\n```\n\nOutside the context of an `if` test, the revealed type of the condition here is `~AlwaysTruthy`: in\nother words, ty knows that this expression is not *always true*, but cannot guarantee that it is\ndefinitely *always false*. It could be an object that is sometimes true and sometimes false -- for\nexample, a `list` (which is falsy when it is empty, and truthy otherwise).\n\nNonetheless, when `value < 1 < 0` is used directly as a condition, ty knows that the condition will\nalways be falsy and the `if` branch will never be taken. Python tests the truthiness of the object\nreturned by `Comparable.__lt__` once: if it is falsy, the condition fails immediately. If it is\ntruthy, Python evaluates `1 < 0`, which is false. There is no second truthiness test of the object\nreturned by `__lt__`.\n\nIf the chained comparison is saved as a variable first, its value can be the object returned by\n`__lt__`, if that object was falsy when first tested. The `if result` statement then tests that\nobject's truthiness again. A user-defined `__bool__` method can return a different result on that\nsecond call, so ty cannot guarantee that the saved value is still falsy, and no diagnostic is\nemitted:\n\n```py\ndef check_saved(value: Comparable):\n result = value < 1 < 0\n if result: # no diagnostic\n pass\n```\n\n## Exemptions\n\nLike `redundant-condition`, this rule checks subexpressions of an `and` or `or` expression only when\nthe outer expression is used as a condition. This is to avoid emitting false-positive diagnostics on\ncode like the following, where the `and` expression is clearly not redundant despite the fact that\nboth `CONSTANT_1` and `CONSTANT_2` are always truthy:\n\n```py\nfrom typing import Final\n\n\nCONSTANT_1: Final = 1\nCONSTANT_2: Final = 2\n\n\ndef do_something(coinflip: bool):\n # could also be written as `constant_to_use = CONSTANT_1 if coinflip else CONSTANT_2`,\n # but use of an `and` expression for this is common in older codebases.\n constant_to_use = coinflip and CONSTANT_1 or CONSTANT_2\n\n # do something with `constant_to_use` now...\n ...\n```\n\nUnlike `and` and `or`, however, `not` explicitly converts its operand to a boolean, so the rule\nchecks `not` expressions in every context.\n\nAnother exemption applied by this rule concerns `assert`-statement tests. A common pattern in Python\ncode is to use defensive `assert`s to enforce behaviour at runtime, even when the asserted condition\ncan be inferred statically to be always true. For example:\n\n```py\ndef add_one(x: int) -> int:\n assert isinstance(x, int) # no diagnostic\n return x + 1\n```\n\nThis kind of defensive behaviour is often reasonable, since the author of a library cannot guarantee\nthat end users of the library will run a type checker on code calling into the library, meaning that\nit's entirely possible at runtime for an object passed into the `x`a parameter above to be a `str`\n(for example) even though the parameter annotation states that only `int`s can ever be passed in.\nThis rule therefore also exempts all assertion tests or subexpressions that evaluate to a subtype of\n`int` or `bool`:\n\n`redundant-condition-strict` can still trigger on `assert` statements in some contexts, however. For\nexample, `redundant-condition-strict` will be emitted on the below example, where the left-hand side\nof the `and` expression is always true and not a subtype of `bool` or `int`, but where the condition\nis nonetheless excluded from the enabled-by-default `redundant-condition` rule due to the use of the\nwalrus operator:\n\n```py\ndef func() -> bool:\n return True\n\n\ndef test_func():\n assert (result := func) and result != func() # error: [redundant-condition-strict]\n```\n\nFor similar reasons to the `assert` exemptions, this rule also exempts always-false `if` or `elif`\nconditions when their bodies end in a defensive check: a `raise`, an assertion that could fail, a\ncall returning `Never`, an `await` to a call returning `Never`, or `return NotImplemented`:\n\n```py\nimport sys\n\n\ndef add_two(x: int) -> int:\n if not isinstance(x, int): # no diagnostic\n raise TypeError(\"need an int!!\")\n return x + 2\n\n\ndef add_three(x: int) -> int:\n if not isinstance(x, int): # no diagnostic\n assert False, \"unreachable\"\n return x + 3\n\n\ndef add_four(x: int) -> int:\n if not isinstance(x, int): # no diagnostic\n sys.exit(1)\n return x + 4\n\n\nclass Foo:\n def __init__(self, data: int):\n self.data = data\n\n def __add__(self, other: \"Foo\"):\n if not isinstance(other, Foo): # no diagnostic\n return NotImplemented\n return Foo(self.data + other.data)\n```\n\nAnd an exemption is applied for always-true `if` or `elif` statements that are followed by branches\nwhich contain defensive checks:\n\n```py\nfrom typing_extensions import assert_never\n\n\ndef parse_data(data: int | str):\n if isinstance(data, int):\n print(\"got an int\")\n elif isinstance(data, str): # Always true, but no diagnostic, since\n # the `else` branch following this branch is always terminal.\n # (`assert_never` returns `Never`, indicating that it always raises an exception)\n print(\"got a str\")\n else:\n assert_never(data)\n\n\ndef parse_data_early_return(data: int | str):\n if isinstance(data, int):\n print(\"got an int\")\n return\n\n # Always true, but no diagnostic, since\n # the suite following this branch is always terminal\n # (every control-flow path following this `if` statement ends in a `raise` statement)\n if isinstance(data, str):\n print(\"got a str\")\n return\n\n raise AssertionError(\"unexpected data\")\n```\n\nAny conditions defined in relation to `sys.version_info`, `sys.platform`, `os.name` or\n`typing.TYPE_CHECKING` are also exempted. The rule recursively follows the definitions of names and\nattributes across module boundaries to determine if a name or attribute was indirectly defined in\nrelation to one of these highly special-cased symbols:\n\n```toml\n[environment]\npython-version = \"3.14\"\npython-platform = \"linux\"\n```\n\n```py\nimport os\nimport sys\nfrom typing import TYPE_CHECKING\n\nif sys.version_info >= (3, 14): # inferred as always true here, but no diagnostic\n pass\n\nif sys.platform == \"win32\": # inferred as always false here, but no diagnostic\n pass\n\nLINE_ENDING = \"\\n\" if os.name == \"posix\" else \"\\r\\n\"\n\nif LINE_ENDING == \"\\n\": # inferred as always true here, but no diagnostic\n pass\n\nif TYPE_CHECKING: # inferred as always true, but no diagnostic\n pass\n```\n\nConditions involving these constants, or conditions involving values defined in relation to these\nconstants, can often be inferred as always-true or always-false by ty. Indeed, these conditions\nusually *will* be always true or always false across a single invocation run of a Python programme.\nNonetheless, Python code is often written so that it can work on multiple different Python versions\nand/or multiple different operating systems, and a condition that is always true on one operating\nsystem might very well be always false on another operating system (for example). Flagging these\nconditions as being always true or always false would only add noise: the aim of the rule is to flag\nconditions that are *unintentionally* always true or always false.\n\nLastly, some conditions involving literal integers and booleans in the AST are also exempted:\nthere's no reason why you'd use a condition like this unless it was intentional.\n\n```py\nif True: # inferred as always true (obviously), but no diagnostic\n pass\n\nif 0:\n pass # inferred as always false, but no diagnostic\n```\n\n## Known issues and workarounds\n\nThis rule can often trigger on code that is not incorrect, but could be written in a clearer way.\nFor example, the rule will flag this code:\n\n```py\nfrom enum import Enum\n\n\nclass YesOrNo(Enum):\n YES = 1\n NO = 0\n\n\ndef say_yes_or_no(what_to_say: YesOrNo):\n if what_to_say == YesOrNo.YES:\n print(\"yes\")\n elif what_to_say == YesOrNo.NO: # error: [redundant-condition-strict]\n print(\"no\")\n```\n\nThis snippet could be written more clearly as this, which would not trigger the rule owing to the\nexemptions described in the section above:\n\n```py\ndef say_yes_or_no(what_to_say: YesOrNo):\n if what_to_say == YesOrNo.YES:\n print(\"yes\")\n else:\n assert what_to_say == YesOrNo.NO\n print(\"no\")\n```\n\nor the snippet could also be rewritten as this, which would also be fine according to the rule's\nheuristics:\n\n```py\nfrom typing_extensions import assert_never\n\n\ndef say_yes_or_no(what_to_say: YesOrNo):\n if what_to_say == YesOrNo.YES:\n print(\"yes\")\n elif what_to_say == YesOrNo.NO:\n print(\"no\")\n else:\n assert_never(what_to_say)\n```\n\nIn a similar vein, this rule can often flag `and` or `or` expressions that have operands which are\ndeliberately always truthy or deliberately always falsy, because the purpose of the operand is to\nhave some side effect occur. For example:\n\n```py\nimport random\nfrom typing import Literal\n\n\ndef want_to_go_fishing() -> bool:\n return random.choice([True, False])\n\n\ndef weather_report() -> Literal[\"rainy\", \"sunny\", \"cloudy\"]:\n return random.choice([\"rainy\", \"sunny\", \"cloudy\"])\n\n\ndef have_fishing_supplies() -> bool:\n return random.choice([True, False])\n\n\ndef main():\n if (\n want_to_go_fishing()\n and (weather := weather_report()) # error: [redundant-condition-strict]\n and have_fishing_supplies()\n ):\n print(f\"The weather is {weather}, let's go fishing\")\n```\n\nThe middle operand in the above `and` expression is always truthy. This might be deliberate, but\neven if it is, the function would arguably be clearer if it were written like this instead:\n\n```py\ndef main():\n if want_to_go_fishing():\n weather = weather_report()\n if have_fishing_supplies():\n print(f\"The weather is {weather}, let's go fishing\")\n```\n\nLastly, the rule cannot reliably distinguish in all cases comparisons that are intentionally always\ntrue/false from those that are unintentionally always true/false. The rule takes care to avoid\nflagging code that uses `if TYPE_CHECKING`, `if sys.version_info < (X, Y)`, `if sys.platform == ...`\nand `if os.name == ...`. But it cannot reliably determine that code like this was written the way it\nwas meant to be:\n\n```py\nDEBUGGING = 0\n\nif DEBUGGING: # error: [redundant-condition-strict]\n print(\"Doing debugging stuff...\")\n```\n\n## See also\n\n- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables\n are tested for their truthiness\n- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables\n are tested for their truthiness", "default": "ignore", "oneOf": [ { @@ -1474,6 +1474,26 @@ } ] }, + "truthiness-test-of-callable": { + "title": "detects truthiness tests of `Callable`-typed objects", + "description": "## What it does\n\nDetects suspicious truthiness tests of `Callable`-typed values and unions of callable types.\n\n## Why is this bad?\n\n`Callable`-typed variables are nearly always functions in practice, and functions are always truthy.\nIf `predicate` is a variable inferred as having a `Callable` type, therefore, a boolean test such as\n`if predicate:` is usually not what you want; `if predicate()` (or similar) is usually what was\nintended.\n\n## Examples\n\n```py\nfrom collections.abc import Callable\n\n\ndef announce_if_ready(is_ready: Callable[[], bool]):\n if is_ready: # error: [truthiness-test-of-callable]\n print(\"Ready\")\n```\n\nYou probably meant to call the value instead:\n\n```py\ndef announce_if_ready_fixed(is_ready: Callable[[], bool]):\n if is_ready(): # no diagnostic\n print(\"Ready\")\n```\n\n## See also\n\n- `redundant-condition` and `redundant-condition-strict` detect conditions that can be inferred as\n always being truthy or falsy\n- `truthiness-test-of-iterable` detects suspicious boolean tests where `Iterable`-typed variables\n are tested for their truthiness", + "default": "warn", + "oneOf": [ + { + "$ref": "#/definitions/Level" + } + ] + }, + "truthiness-test-of-iterable": { + "title": "detects truthiness tests of `Iterable`-typed objects", + "description": "## What it does\n\nDetects boolean conditions where variables typed as `Iterable`, `Iterator`, `Generator` or similar\nare tested for their truthiness.\n\n## Why is this bad?\n\nTesting an `Iterable` object for truthiness strongly suggests that the code expects the object to\nevaluate as falsy in a boolean context if it is empty. However, even empty `Iterable` objects can be\ntruthy if they do not define `__len__` or `__bool__`. The classic example of this is a generator:\nunevaluated generators in Python are always truthy, even if they do not yield any elements at\nruntime.\n\n## Examples\n\n```py\nfrom collections.abc import Iterable\n\n\ndef process(items: Iterable[int]):\n if items: # error: [truthiness-test-of-iterable]\n print(\"Received items\")\n else:\n print(\"Didn't receive any items\")\n\n\n# prints \"Received items\", even though the passed-in generator is empty!\nprocess(x for x in range(42) if x > 43)\n```\n\nIf `process` in the above example does not need to accept generators, one solution is to rewrite the\nannotation to use `Collection` instead of `Iterable`. `Collection` mandates that the object passed\nin must define `__len__`, making a truthiness test much more likely to be meaningful:\n\n```py\nfrom collections.abc import Collection\n\n\ndef process(items: Collection[int]):\n if items: # no diagnostic\n print(\"Received items\")\n else:\n print(\"Didn't receive any items\")\n\n\n# passing in a generator is now rejected:\n# error: [invalid-argument-type] \"Expected `Collection[int]`, found `GeneratorType[int, None, None]`\"\nprocess(x for x in range(42) if x > 43)\n```\n\nIf the function must also accept generators, another solution can be to collect the iterable into a\ntuple or list before testing its length:\n\n```py\ndef process(items: Iterable[int]):\n collected = tuple(items)\n if collected: # no diagnostic\n print(\"Received items\")\n else:\n print(\"Didn't receive any items\")\n\n\n# correctly prints \"Didn't receive any items\"\nprocess(x for x in range(42) if x > 43)\n```\n\n## See also\n\n- `redundant-condition` and `redundant-condition-strict` detect conditions that can be inferred as\n always being truthy or falsy\n- `truthiness-test-of-callable` detects suspicious boolean tests where `Callable`-typed variables\n are tested for their truthiness", + "default": "warn", + "oneOf": [ + { + "$ref": "#/definitions/Level" + } + ] + }, "type-assertion-failure": { "title": "detects failed type assertions", "description": "## What it does\n\nChecks for `assert_type()` and `assert_never()` calls where the actual type is not the same as the\nasserted type.\n\n## Why is this bad?\n\n`assert_type()` allows confirming the inferred type of a certain value.\n\n## Example\n\n```toml\n[environment]\npython-version = \"3.11\"\n```\n\n```python\nfrom typing import assert_type\n\n\ndef _(x: int):\n assert_type(x, int) # fine\n # Actual type does not match asserted type\n assert_type(x, str) # error\n```",