diff --git a/crates/red_knot_python_semantic/resources/mdtest/binary/instances.md b/crates/red_knot_python_semantic/resources/mdtest/binary/instances.md index 42af9383f7..5fad860267 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/binary/instances.md +++ b/crates/red_knot_python_semantic/resources/mdtest/binary/instances.md @@ -351,6 +351,20 @@ class Y(Foo): ... reveal_type(X() + Y()) # revealed: int ``` +## Operations involving types with invalid `__bool__` methods + + + +```py +class NotBoolable: + __bool__ = 3 + +a = NotBoolable() + +# error: [unsupported-bool-conversion] +10 and a and True +``` + ## Unsupported ### Dunder as instance attribute diff --git a/crates/red_knot_python_semantic/resources/mdtest/call/builtins.md b/crates/red_knot_python_semantic/resources/mdtest/call/builtins.md new file mode 100644 index 0000000000..e04227c6a6 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/call/builtins.md @@ -0,0 +1,14 @@ +# Calling builtins + +## `bool` with incorrect arguments + +```py +class NotBool: + __bool__ = None + +# TODO: We should emit an `invalid-argument` error here for `2` because `bool` only takes one argument. +bool(1, 2) + +# TODO: We should emit an `unsupported-bool-conversion` error here because the argument doesn't implement `__bool__` correctly. +bool(NotBool()) +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/comparison/instances/membership_test.md b/crates/red_knot_python_semantic/resources/mdtest/comparison/instances/membership_test.md index 90dc173474..4b1617a979 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/comparison/instances/membership_test.md +++ b/crates/red_knot_python_semantic/resources/mdtest/comparison/instances/membership_test.md @@ -160,3 +160,45 @@ reveal_type(42 in A()) # revealed: bool # error: [unsupported-operator] "Operator `in` is not supported for types `str` and `A`, in comparing `Literal["hello"]` with `A`" reveal_type("hello" in A()) # revealed: bool ``` + +## Return type that doesn't implement `__bool__` correctly + +`in` and `not in` operations will fail at runtime if the object on the right-hand side of the +operation has a `__contains__` method that returns a type which is not convertible to `bool`. This +is because of the way these operations are handled by the Python interpreter at runtime. If we +assume that `y` is an object that has a `__contains__` method, the Python expression `x in y` +desugars to a `contains(y, x)` call, where `contains` looks something like this: + +```ignore +def contains(y, x): + return bool(type(y).__contains__(y, x)) +``` + +where the `bool()` conversion itself implicitly calls `__bool__` under the hood. + +TODO: Ideally the message would explain to the user what's wrong. E.g, + +```ignore +error: [operator] cannot use `in` operator on object of type `WithContains` + note: This is because the `in` operator implicitly calls `WithContains.__contains__`, but `WithContains.__contains__` is invalidly defined + note: `WithContains.__contains__` is invalidly defined because it returns an instance of `NotBoolable`, which cannot be evaluated in a boolean context + note: `NotBoolable` cannot be evaluated in a boolean context because its `__bool__` attribute is not callable +``` + +It may also be more appropriate to use `unsupported-operator` as the error code. + + + +```py +class NotBoolable: + __bool__ = 3 + +class WithContains: + def __contains__(self, item) -> NotBoolable: + return NotBoolable() + +# error: [unsupported-bool-conversion] +10 in WithContains() +# error: [unsupported-bool-conversion] +10 not in WithContains() +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/comparison/instances/rich_comparison.md b/crates/red_knot_python_semantic/resources/mdtest/comparison/instances/rich_comparison.md index 9f5475072a..3ba6b001d6 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/comparison/instances/rich_comparison.md +++ b/crates/red_knot_python_semantic/resources/mdtest/comparison/instances/rich_comparison.md @@ -345,3 +345,29 @@ def f(x: bool, y: int): reveal_type(4.2 < x) # revealed: bool reveal_type(x < 4.2) # revealed: bool ``` + +## Chained comparisons with objects that don't implement `__bool__` correctly + + + +Python implicitly calls `bool` on the comparison result of preceding elements (but not for the last +element) of a chained comparison. + +```py +class NotBoolable: + __bool__ = 3 + +class Comparable: + def __lt__(self, item) -> NotBoolable: + return NotBoolable() + + def __gt__(self, item) -> NotBoolable: + return NotBoolable() + +# error: [unsupported-bool-conversion] +10 < Comparable() < 20 +# error: [unsupported-bool-conversion] +10 < Comparable() < Comparable() + +Comparable() < Comparable() # fine +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md b/crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md index 425b438e5d..ff3f684a5d 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md +++ b/crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md @@ -334,3 +334,61 @@ reveal_type(a is not c) # revealed: Literal[True] For tuples like `tuple[int, ...]`, `tuple[Any, ...]` // TODO + +## Chained comparisons with elements that incorrectly implement `__bool__` + + + +For an operation `A() < A()` to succeed at runtime, the `A.__lt__` method does not necessarily need +to return an object that is convertible to a `bool`. However, the return type _does_ need to be +convertible to a `bool` for the operation `A() < A() < A()` (a _chained_ comparison) to succeed. +This is because `A() < A() < A()` desugars to something like this, which involves several implicit +conversions to `bool`: + +```ignore +def compute_chained_comparison(): + a1 = A() + a2 = A() + first_comparison = a1 < a2 + return first_comparison and (a2 < A()) +``` + +```py +class NotBoolable: + __bool__ = 5 + +class Comparable: + def __lt__(self, other) -> NotBoolable: + return NotBoolable() + + def __gt__(self, other) -> NotBoolable: + return NotBoolable() + +a = (1, Comparable()) +b = (1, Comparable()) + +# error: [unsupported-bool-conversion] +a < b < b + +a < b # fine +``` + +## Equality with elements that incorrectly implement `__bool__` + + + +Python does not generally attempt to coerce the result of `==` and `!=` operations between two +arbitrary objects to a `bool`, but a comparison of tuples will fail if the result of comparing any +pair of elements at equivalent positions cannot be converted to a `bool`: + +```py +class A: + def __eq__(self, other) -> NotBoolable: + return NotBoolable() + +class NotBoolable: + __bool__ = None + +# error: [unsupported-bool-conversion] +(A(),) == (A(),) +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/conditional/if_expression.md b/crates/red_knot_python_semantic/resources/mdtest/conditional/if_expression.md index d9ef65b8f3..47696c0658 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/conditional/if_expression.md +++ b/crates/red_knot_python_semantic/resources/mdtest/conditional/if_expression.md @@ -35,3 +35,13 @@ def _(flag: bool): x = 1 if flag else None reveal_type(x) # revealed: Literal[1] | None ``` + +## Condition with object that implements `__bool__` incorrectly + +```py +class NotBoolable: + __bool__ = 3 + +# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable" +3 if NotBoolable() else 4 +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md b/crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md index 539fdf2a48..fff1018427 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md +++ b/crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md @@ -147,3 +147,17 @@ def _(flag: bool): reveal_type(y) # revealed: Literal[0, 1] ``` + +## Condition with object that implements `__bool__` incorrectly + +```py +class NotBoolable: + __bool__ = 3 + +# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable" +if NotBoolable(): + ... +# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable" +elif NotBoolable(): + ... +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/conditional/match.md b/crates/red_knot_python_semantic/resources/mdtest/conditional/match.md index 2b92c6c734..3fe4956b34 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/conditional/match.md +++ b/crates/red_knot_python_semantic/resources/mdtest/conditional/match.md @@ -43,3 +43,21 @@ def _(target: int): reveal_type(y) # revealed: Literal[2, 3, 4] ``` + +## Guard with object that implements `__bool__` incorrectly + +```py +class NotBoolable: + __bool__ = 3 + +def _(target: int, flag: NotBoolable): + y = 1 + match target: + # error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable" + case 1 if flag: + y = 2 + case 2: + y = 3 + + reveal_type(y) # revealed: Literal[1, 2, 3] +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/expression/assert.md b/crates/red_knot_python_semantic/resources/mdtest/expression/assert.md new file mode 100644 index 0000000000..f7e1715246 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/expression/assert.md @@ -0,0 +1,9 @@ +## Condition with object that implements `__bool__` incorrectly + +```py +class NotBoolable: + __bool__ = 3 + +# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable" +assert NotBoolable() +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/expression/boolean.md b/crates/red_knot_python_semantic/resources/mdtest/expression/boolean.md index 22fea2c5c4..4016733720 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/expression/boolean.md +++ b/crates/red_knot_python_semantic/resources/mdtest/expression/boolean.md @@ -101,3 +101,55 @@ reveal_type(bool([])) # revealed: bool reveal_type(bool({})) # revealed: bool reveal_type(bool(set())) # revealed: bool ``` + +## `__bool__` returning `NoReturn` + +```py +from typing import NoReturn + +class NotBoolable: + def __bool__(self) -> NoReturn: + raise NotImplementedError("This object can't be converted to a boolean") + +# TODO: This should emit an error that `NotBoolable` can't be converted to a bool but it currently doesn't +# because `Never` is assignable to `bool`. This probably requires dead code analysis to fix. +if NotBoolable(): + ... +``` + +## Not callable `__bool__` + +```py +class NotBoolable: + __bool__ = None + +# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable" +if NotBoolable(): + ... +``` + +## Not-boolable union + +```py +def test(cond: bool): + class NotBoolable: + __bool__ = None if cond else 3 + + # error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; it incorrectly implements `__bool__`" + if NotBoolable(): + ... +``` + +## Union with some variants implementing `__bool__` incorrectly + +```py +def test(cond: bool): + class NotBoolable: + __bool__: int + + a = 10 if cond else NotBoolable() + + # error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `Literal[10] | NotBoolable`; its `__bool__` method isn't callable" + if a: + ... +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/loops/while_loop.md b/crates/red_knot_python_semantic/resources/mdtest/loops/while_loop.md index e30297f905..c3da62e064 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/loops/while_loop.md +++ b/crates/red_knot_python_semantic/resources/mdtest/loops/while_loop.md @@ -116,3 +116,14 @@ def _(flag: bool, flag2: bool): # error: [possibly-unresolved-reference] y ``` + +## Condition with object that implements `__bool__` incorrectly + +```py +class NotBoolable: + __bool__ = 3 + +# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable" +while NotBoolable(): + ... +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/truthiness.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/truthiness.md index 03884b95e5..5ad241beaa 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/truthiness.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/truthiness.md @@ -266,7 +266,7 @@ def _( if af: reveal_type(af) # revealed: type[AmbiguousClass] & ~AlwaysFalsy - # TODO: Emit a diagnostic (`d` is not valid in boolean context) + # error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `MetaDeferred`; the return type of its bool method (`MetaAmbiguous`) isn't assignable to `bool" if d: # TODO: Should be `Unknown` reveal_type(d) # revealed: type[DeferredClass] & ~AlwaysFalsy diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/instances.md_-_Binary_operations_on_instances_-_Operations_involving_types_with_invalid_`__bool__`_methods.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/instances.md_-_Binary_operations_on_instances_-_Operations_involving_types_with_invalid_`__bool__`_methods.snap new file mode 100644 index 0000000000..cd7e6f94b9 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/instances.md_-_Binary_operations_on_instances_-_Operations_involving_types_with_invalid_`__bool__`_methods.snap @@ -0,0 +1,35 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: instances.md - Binary operations on instances - Operations involving types with invalid `__bool__` methods +mdtest path: crates/red_knot_python_semantic/resources/mdtest/binary/instances.md +--- + +# Python source files + +## mdtest_snippet.py + +``` +1 | class NotBoolable: +2 | __bool__ = 3 +3 | +4 | a = NotBoolable() +5 | +6 | # error: [unsupported-bool-conversion] +7 | 10 and a and True +``` + +# Diagnostics + +``` +error: lint:unsupported-bool-conversion + --> /src/mdtest_snippet.py:7:8 + | +6 | # error: [unsupported-bool-conversion] +7 | 10 and a and True + | ^ Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/membership_test.md_-_Comparison___Membership_Test_-_Return_type_that_doesn't_implement_`__bool__`_correctly.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/membership_test.md_-_Comparison___Membership_Test_-_Return_type_that_doesn't_implement_`__bool__`_correctly.snap new file mode 100644 index 0000000000..d714fd2c18 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/membership_test.md_-_Comparison___Membership_Test_-_Return_type_that_doesn't_implement_`__bool__`_correctly.snap @@ -0,0 +1,53 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: membership_test.md - Comparison: Membership Test - Return type that doesn't implement `__bool__` correctly +mdtest path: crates/red_knot_python_semantic/resources/mdtest/comparison/instances/membership_test.md +--- + +# Python source files + +## mdtest_snippet.py + +``` + 1 | class NotBoolable: + 2 | __bool__ = 3 + 3 | + 4 | class WithContains: + 5 | def __contains__(self, item) -> NotBoolable: + 6 | return NotBoolable() + 7 | + 8 | # error: [unsupported-bool-conversion] + 9 | 10 in WithContains() +10 | # error: [unsupported-bool-conversion] +11 | 10 not in WithContains() +``` + +# Diagnostics + +``` +error: lint:unsupported-bool-conversion + --> /src/mdtest_snippet.py:9:1 + | + 8 | # error: [unsupported-bool-conversion] + 9 | 10 in WithContains() + | ^^^^^^^^^^^^^^^^^^^^ Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable +10 | # error: [unsupported-bool-conversion] +11 | 10 not in WithContains() + | + +``` + +``` +error: lint:unsupported-bool-conversion + --> /src/mdtest_snippet.py:11:1 + | + 9 | 10 in WithContains() +10 | # error: [unsupported-bool-conversion] +11 | 10 not in WithContains() + | ^^^^^^^^^^^^^^^^^^^^^^^^ Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/not.md_-_Unary_not_-_Object_that_implements_`__bool__`_incorrectly.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/not.md_-_Unary_not_-_Object_that_implements_`__bool__`_incorrectly.snap new file mode 100644 index 0000000000..8471ca5c59 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/not.md_-_Unary_not_-_Object_that_implements_`__bool__`_incorrectly.snap @@ -0,0 +1,33 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: not.md - Unary not - Object that implements `__bool__` incorrectly +mdtest path: crates/red_knot_python_semantic/resources/mdtest/unary/not.md +--- + +# Python source files + +## mdtest_snippet.py + +``` +1 | class NotBoolable: +2 | __bool__ = 3 +3 | +4 | # error: [unsupported-bool-conversion] +5 | not NotBoolable() +``` + +# Diagnostics + +``` +error: lint:unsupported-bool-conversion + --> /src/mdtest_snippet.py:5:1 + | +4 | # error: [unsupported-bool-conversion] +5 | not NotBoolable() + | ^^^^^^^^^^^^^^^^^ Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/rich_comparison.md_-_Comparison___Rich_Comparison_-_Chained_comparisons_with_objects_that_don't_implement_`__bool__`_correctly.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/rich_comparison.md_-_Comparison___Rich_Comparison_-_Chained_comparisons_with_objects_that_don't_implement_`__bool__`_correctly.snap new file mode 100644 index 0000000000..87779b02dc --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/rich_comparison.md_-_Comparison___Rich_Comparison_-_Chained_comparisons_with_objects_that_don't_implement_`__bool__`_correctly.snap @@ -0,0 +1,60 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: rich_comparison.md - Comparison: Rich Comparison - Chained comparisons with objects that don't implement `__bool__` correctly +mdtest path: crates/red_knot_python_semantic/resources/mdtest/comparison/instances/rich_comparison.md +--- + +# Python source files + +## mdtest_snippet.py + +``` + 1 | class NotBoolable: + 2 | __bool__ = 3 + 3 | + 4 | class Comparable: + 5 | def __lt__(self, item) -> NotBoolable: + 6 | return NotBoolable() + 7 | + 8 | def __gt__(self, item) -> NotBoolable: + 9 | return NotBoolable() +10 | +11 | # error: [unsupported-bool-conversion] +12 | 10 < Comparable() < 20 +13 | # error: [unsupported-bool-conversion] +14 | 10 < Comparable() < Comparable() +15 | +16 | Comparable() < Comparable() # fine +``` + +# Diagnostics + +``` +error: lint:unsupported-bool-conversion + --> /src/mdtest_snippet.py:12:1 + | +11 | # error: [unsupported-bool-conversion] +12 | 10 < Comparable() < 20 + | ^^^^^^^^^^^^^^^^^ Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable +13 | # error: [unsupported-bool-conversion] +14 | 10 < Comparable() < Comparable() + | + +``` + +``` +error: lint:unsupported-bool-conversion + --> /src/mdtest_snippet.py:14:1 + | +12 | 10 < Comparable() < 20 +13 | # error: [unsupported-bool-conversion] +14 | 10 < Comparable() < Comparable() + | ^^^^^^^^^^^^^^^^^ Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable +15 | +16 | Comparable() < Comparable() # fine + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/tuples.md_-_Comparison___Tuples_-_Chained_comparisons_with_elements_that_incorrectly_implement_`__bool__`.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/tuples.md_-_Comparison___Tuples_-_Chained_comparisons_with_elements_that_incorrectly_implement_`__bool__`.snap new file mode 100644 index 0000000000..f0694a0fda --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/tuples.md_-_Comparison___Tuples_-_Chained_comparisons_with_elements_that_incorrectly_implement_`__bool__`.snap @@ -0,0 +1,47 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: tuples.md - Comparison: Tuples - Chained comparisons with elements that incorrectly implement `__bool__` +mdtest path: crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md +--- + +# Python source files + +## mdtest_snippet.py + +``` + 1 | class NotBoolable: + 2 | __bool__ = 5 + 3 | + 4 | class Comparable: + 5 | def __lt__(self, other) -> NotBoolable: + 6 | return NotBoolable() + 7 | + 8 | def __gt__(self, other) -> NotBoolable: + 9 | return NotBoolable() +10 | +11 | a = (1, Comparable()) +12 | b = (1, Comparable()) +13 | +14 | # error: [unsupported-bool-conversion] +15 | a < b < b +16 | +17 | a < b # fine +``` + +# Diagnostics + +``` +error: lint:unsupported-bool-conversion + --> /src/mdtest_snippet.py:15:1 + | +14 | # error: [unsupported-bool-conversion] +15 | a < b < b + | ^^^^^ Boolean conversion is unsupported for type `NotBoolable | Literal[False]`; its `__bool__` method isn't callable +16 | +17 | a < b # fine + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/snapshots/tuples.md_-_Comparison___Tuples_-_Equality_with_elements_that_incorrectly_implement_`__bool__`.snap b/crates/red_knot_python_semantic/resources/mdtest/snapshots/tuples.md_-_Comparison___Tuples_-_Equality_with_elements_that_incorrectly_implement_`__bool__`.snap new file mode 100644 index 0000000000..386af00456 --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/snapshots/tuples.md_-_Comparison___Tuples_-_Equality_with_elements_that_incorrectly_implement_`__bool__`.snap @@ -0,0 +1,37 @@ +--- +source: crates/red_knot_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: tuples.md - Comparison: Tuples - Equality with elements that incorrectly implement `__bool__` +mdtest path: crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.md +--- + +# Python source files + +## mdtest_snippet.py + +``` +1 | class A: +2 | def __eq__(self, other) -> NotBoolable: +3 | return NotBoolable() +4 | +5 | class NotBoolable: +6 | __bool__ = None +7 | +8 | # error: [unsupported-bool-conversion] +9 | (A(),) == (A(),) +``` + +# Diagnostics + +``` +error: lint:unsupported-bool-conversion + --> /src/mdtest_snippet.py:9:1 + | +8 | # error: [unsupported-bool-conversion] +9 | (A(),) == (A(),) + | ^^^^^^^^^^^^^^^^ Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable + | + +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/type_api.md b/crates/red_knot_python_semantic/resources/mdtest/type_api.md index 3ae113cf2e..922ccaca06 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/type_api.md +++ b/crates/red_knot_python_semantic/resources/mdtest/type_api.md @@ -223,7 +223,7 @@ class InvalidBoolDunder: def __bool__(self) -> int: return 1 -# error: "Static assertion error: argument of type `InvalidBoolDunder` has an ambiguous static truthiness" +# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `InvalidBoolDunder`; the return type of its bool method (`int`) isn't assignable to `bool" static_assert(InvalidBoolDunder()) ``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/unary/not.md b/crates/red_knot_python_semantic/resources/mdtest/unary/not.md index e53d29fb10..b3b75678f9 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/unary/not.md +++ b/crates/red_knot_python_semantic/resources/mdtest/unary/not.md @@ -183,12 +183,11 @@ class WithBothLenAndBool2: # revealed: Literal[False] reveal_type(not WithBothLenAndBool2()) -# TODO: raise diagnostic when __bool__ method is not valid: [unsupported-operator] "Method __bool__ for type `MethodBoolInvalid` should return `bool`, returned type `int`" -# https://docs.python.org/3/reference/datamodel.html#object.__bool__ class MethodBoolInvalid: def __bool__(self) -> int: return 0 +# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `MethodBoolInvalid`; the return type of its bool method (`int`) isn't assignable to `bool" # revealed: bool reveal_type(not MethodBoolInvalid()) @@ -204,3 +203,15 @@ class PossiblyUnboundBool: # revealed: bool reveal_type(not PossiblyUnboundBool()) ``` + +## Object that implements `__bool__` incorrectly + + + +```py +class NotBoolable: + __bool__ = 3 + +# error: [unsupported-bool-conversion] +not NotBoolable() +``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 5e3a5db004..b493975d45 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -9,6 +9,7 @@ use itertools::Itertools; use ruff_db::files::File; use ruff_python_ast as ast; use ruff_python_ast::PythonVersion; +use ruff_text_size::{Ranged, TextRange}; use type_ordering::union_elements_ordering; pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder}; @@ -36,9 +37,9 @@ use crate::symbol::{ imported_symbol, known_module_symbol, symbol, symbol_from_bindings, symbol_from_declarations, Boundness, LookupError, LookupResult, Symbol, SymbolAndQualifiers, }; -use crate::types::call::{bind_call, CallArguments, CallBinding, CallOutcome}; +use crate::types::call::{bind_call, CallArguments, CallBinding, CallOutcome, UnionCallError}; use crate::types::class_base::ClassBase; -use crate::types::diagnostic::INVALID_TYPE_FORM; +use crate::types::diagnostic::{INVALID_TYPE_FORM, UNSUPPORTED_BOOL_CONVERSION}; use crate::types::infer::infer_unpack_types; use crate::types::mro::{Mro, MroError, MroIterator}; pub(crate) use crate::types::narrow::narrowing_constraint; @@ -1681,27 +1682,66 @@ impl<'db> Type<'db> { } } + /// Resolves the boolean value of the type and falls back to [`Truthiness::Ambiguous`] if the type doesn't implement `__bool__` correctly. + /// + /// This method should only be used outside type checking or when evaluating if a type + /// is truthy or falsy in a context where Python doesn't make an implicit `bool` call. + /// Use [`try_bool`](Self::try_bool) for type checking or implicit `bool` calls. + pub(crate) fn bool(&self, db: &'db dyn Db) -> Truthiness { + self.try_bool_impl(db, true) + .unwrap_or_else(|err| err.fallback_truthiness()) + } + /// Resolves the boolean value of a type. /// /// This is used to determine the value that would be returned /// when `bool(x)` is called on an object `x`. - pub(crate) fn bool(&self, db: &'db dyn Db) -> Truthiness { - match self { + /// + /// Returns an error if the type doesn't implement `__bool__` correctly. + pub(crate) fn try_bool(&self, db: &'db dyn Db) -> Result> { + self.try_bool_impl(db, false) + } + + /// Resolves the boolean value of a type. + /// + /// Setting `allow_short_circuit` to `true` allows the implementation to + /// early return if the bool value of any union variant is `Truthiness::Ambiguous`. + /// Early returning shows a 1-2% perf improvement on our benchmarks because + /// `bool` (which doesn't care about errors) is used heavily when evaluating statically known branches. + /// + /// An alternative to this flag is to implement a trait similar to Rust's `Try` trait. + /// The advantage of that is that it would allow collecting the errors as well. However, + /// it is significantly more complex and duplicating the logic into `bool` without the error + /// handling didn't show any significant performance difference to when using the `allow_short_circuit` flag. + #[inline] + fn try_bool_impl( + &self, + db: &'db dyn Db, + allow_short_circuit: bool, + ) -> Result> { + let truthiness = match self { Type::Dynamic(_) | Type::Never => Truthiness::Ambiguous, Type::FunctionLiteral(_) => Truthiness::AlwaysTrue, Type::Callable(_) => Truthiness::AlwaysTrue, Type::ModuleLiteral(_) => Truthiness::AlwaysTrue, Type::ClassLiteral(ClassLiteralType { class }) => { - class.metaclass(db).to_instance(db).bool(db) + return class + .metaclass(db) + .to_instance(db) + .try_bool_impl(db, allow_short_circuit); } - Type::SubclassOf(subclass_of_ty) => subclass_of_ty - .subclass_of() - .into_class() - .map(|class| Type::class_literal(class).bool(db)) - .unwrap_or(Truthiness::Ambiguous), + Type::SubclassOf(subclass_of_ty) => match subclass_of_ty.subclass_of() { + ClassBase::Dynamic(_) => Truthiness::Ambiguous, + ClassBase::Class(class) => { + return class + .metaclass(db) + .to_instance(db) + .try_bool_impl(db, allow_short_circuit); + } + }, Type::AlwaysTruthy => Truthiness::AlwaysTrue, Type::AlwaysFalsy => Truthiness::AlwaysFalse, - Type::Instance(InstanceType { class }) => { + instance_ty @ Type::Instance(InstanceType { class }) => { if class.is_known(db, KnownClass::Bool) { Truthiness::Ambiguous } else if class.is_known(db, KnownClass::NoneType) { @@ -1711,32 +1751,119 @@ impl<'db> Type<'db> { // runtime there is a fallback to `__len__`, since `__bool__` takes precedence // and a subclass could add a `__bool__` method. - if let Ok(Type::BooleanLiteral(bool_val)) = self - .try_call_dunder(db, "__bool__", &CallArguments::none()) - .map(|outcome| outcome.return_type(db)) - { - bool_val.into() - } else { - // TODO diagnostic if not assignable to bool - Truthiness::Ambiguous + let type_to_truthiness = |ty| { + if let Type::BooleanLiteral(bool_val) = ty { + Truthiness::from(bool_val) + } else { + Truthiness::Ambiguous + } + }; + + match self.try_call_dunder(db, "__bool__", &CallArguments::none()) { + ref result @ (Ok(ref outcome) + | Err(CallDunderError::PossiblyUnbound(ref outcome))) => { + let return_type = outcome.return_type(db); + + // The type has a `__bool__` method, but it doesn't return a boolean. + if !return_type.is_assignable_to(db, KnownClass::Bool.to_instance(db)) { + return Err(BoolError::IncorrectReturnType { + return_type: outcome.return_type(db), + not_boolable_type: *instance_ty, + }); + } + + if result.is_ok() { + type_to_truthiness(return_type) + } else { + // Don't trust possibly unbound `__bool__` method. + Truthiness::Ambiguous + } + } + Err(CallDunderError::MethodNotAvailable) => Truthiness::Ambiguous, + Err(CallDunderError::Call(err)) => { + let err = match err { + // Unwrap call errors where only a single variant isn't callable. + // E.g. in the case of `Unknown & T` + // TODO: Improve handling of unions. While this improves messages overall, + // it still results in loosing information. Or should the information + // be recomputed when rendering the diagnostic? + CallError::Union(union_error) => { + if let Type::Union(_) = union_error.called_ty { + if union_error.errors.len() == 1 { + union_error.errors.into_vec().pop().unwrap() + } else { + CallError::Union(union_error) + } + } else { + CallError::Union(union_error) + } + } + err => err, + }; + + match err { + CallError::BindingError { binding } => { + return Err(BoolError::IncorrectArguments { + truthiness: type_to_truthiness(binding.return_type()), + not_boolable_type: *instance_ty, + }); + } + CallError::NotCallable { .. } => { + return Err(BoolError::NotCallable { + not_boolable_type: *instance_ty, + }); + } + + CallError::PossiblyUnboundDunderCall { .. } + | CallError::Union(..) => { + return Err(BoolError::Other { + not_boolable_type: *self, + }) + } + } + } } } } Type::KnownInstance(known_instance) => known_instance.bool(), Type::Union(union) => { - let union_elements = union.elements(db); - let first_element_truthiness = union_elements[0].bool(db); - if first_element_truthiness.is_ambiguous() { - return Truthiness::Ambiguous; + let mut truthiness = None; + let mut all_not_callable = true; + let mut has_errors = false; + + for element in union.elements(db) { + let element_truthiness = match element.try_bool_impl(db, allow_short_circuit) { + Ok(truthiness) => truthiness, + Err(err) => { + has_errors = true; + all_not_callable &= matches!(err, BoolError::NotCallable { .. }); + err.fallback_truthiness() + } + }; + + truthiness.get_or_insert(element_truthiness); + + if Some(element_truthiness) != truthiness { + truthiness = Some(Truthiness::Ambiguous); + + if allow_short_circuit { + return Ok(Truthiness::Ambiguous); + } + } } - if !union_elements - .iter() - .skip(1) - .all(|element| element.bool(db) == first_element_truthiness) - { - return Truthiness::Ambiguous; + + if has_errors { + if all_not_callable { + return Err(BoolError::NotCallable { + not_boolable_type: *self, + }); + } + return Err(BoolError::Union { + union: *union, + truthiness: truthiness.unwrap_or(Truthiness::Ambiguous), + }); } - first_element_truthiness + truthiness.unwrap_or(Truthiness::Ambiguous) } Type::Intersection(_) => { // TODO @@ -1749,7 +1876,9 @@ impl<'db> Type<'db> { Type::BytesLiteral(bytes) => Truthiness::from(!bytes.value(db).is_empty()), Type::SliceLiteral(_) => Truthiness::AlwaysTrue, Type::Tuple(items) => Truthiness::from(!items.elements(db).is_empty()), - } + }; + + Ok(truthiness) } /// Return the type of `len()` on a type if it is known more precisely than `int`, @@ -2081,6 +2210,9 @@ impl<'db> Type<'db> { Type::ClassLiteral(ClassLiteralType { class }) => { Ok(CallOutcome::Single(CallBinding::from_return_type( match class.known(db) { + // TODO: We should check the call signature and error if the bool call doesn't have the + // right signature and return a binding error. + // If the class is the builtin-bool class (for example `bool(1)`), we try to // return the specific truthiness value of the input arg, `Literal[True]` for // the example above. @@ -2112,15 +2244,15 @@ impl<'db> Type<'db> { not_callable_ty: self, } } - CallDunderError::Call(CallError::Union { + CallDunderError::Call(CallError::Union(UnionCallError { called_ty: _, bindings, errors, - }) => CallError::Union { + })) => CallError::Union(UnionCallError { called_ty: self, bindings, errors, - }, + }), CallDunderError::Call(error) => error, // Turn "possibly unbound object of type `Literal['__call__']`" // into "`X` not callable (possibly unbound `__call__` method)" @@ -2195,6 +2327,9 @@ impl<'db> Type<'db> { } /// Look up a dunder method on the meta type of `self` and call it. + /// + /// Returns an `Err` if the dunder method can't be called, + /// or the given arguments are not valid. fn try_call_dunder( self, db: &'db dyn Db, @@ -2213,6 +2348,15 @@ impl<'db> Type<'db> { } } + /// Returns the element type when iterating over `self`. + /// + /// This method should only be used outside of type checking because it omits any errors. + /// For type checking, use [`try_iterate`](Self::try_iterate) instead. + fn iterate(self, db: &'db dyn Db) -> Type<'db> { + self.try_iterate(db) + .unwrap_or_else(|err| err.fallback_element_type()) + } + /// Given the type of an object that is iterated over in some way, /// return the type of objects that are yielded by that iteration. /// @@ -2221,11 +2365,9 @@ impl<'db> Type<'db> { /// for y in x: /// pass /// ``` - fn iterate(self, db: &'db dyn Db) -> IterationOutcome<'db> { + fn try_iterate(self, db: &'db dyn Db) -> Result, IterateError<'db>> { if let Type::Tuple(tuple_type) = self { - return IterationOutcome::Iterable { - element_ty: UnionType::from_elements(db, tuple_type.elements(db)), - }; + return Ok(UnionType::from_elements(db, tuple_type.elements(db))); } let dunder_iter_result = self.try_call_dunder(db, "__iter__", &CallArguments::none()); @@ -2239,33 +2381,31 @@ impl<'db> Type<'db> { dunder_iter_result, Err(CallDunderError::PossiblyUnbound { .. }) ) { - IterationOutcome::PossiblyUnboundDunderIter { + Err(IterateError::PossiblyUnbound { iterable_ty: self, element_ty: outcome.return_type(db), - } + }) } else { - IterationOutcome::Iterable { - element_ty: outcome.return_type(db), - } + Ok(outcome.return_type(db)) } } Err(CallDunderError::PossiblyUnbound(outcome)) => { - IterationOutcome::PossiblyUnboundDunderIter { + Err(IterateError::PossiblyUnbound { iterable_ty: self, element_ty: outcome.return_type(db), - } + }) } - Err(_) => IterationOutcome::NotIterable { + Err(_) => Err(IterateError::NotIterable { not_iterable_ty: self, - }, + }), }; } // If `__iter__` exists but can't be called or doesn't have the expected signature, // return not iterable over falling back to `__getitem__`. Err(CallDunderError::Call(_)) => { - return IterationOutcome::NotIterable { + return Err(IterateError::NotIterable { not_iterable_ty: self, - } + }) } Err(CallDunderError::MethodNotAvailable) => { // No `__iter__` attribute, try `__getitem__` next. @@ -2283,18 +2423,14 @@ impl<'db> Type<'db> { "__getitem__", &CallArguments::positional([KnownClass::Int.to_instance(db)]), ) { - Ok(outcome) => IterationOutcome::Iterable { + Ok(outcome) => Ok(outcome.return_type(db)), + Err(CallDunderError::PossiblyUnbound(outcome)) => Err(IterateError::PossiblyUnbound { + iterable_ty: self, element_ty: outcome.return_type(db), - }, - Err(CallDunderError::PossiblyUnbound(outcome)) => { - IterationOutcome::PossiblyUnboundDunderIter { - iterable_ty: self, - element_ty: outcome.return_type(db), - } - } - Err(_) => IterationOutcome::NotIterable { + }), + Err(_) => Err(IterateError::NotIterable { not_iterable_ty: self, - }, + }), } } @@ -3469,47 +3605,182 @@ pub enum TypeVarBoundOrConstraints<'db> { Constraints(TupleType<'db>), } +/// Error returned if a type isn't iterable. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum IterationOutcome<'db> { - Iterable { - element_ty: Type<'db>, - }, - NotIterable { - not_iterable_ty: Type<'db>, - }, - PossiblyUnboundDunderIter { +enum IterateError<'db> { + /// The type isn't iterable because it doesn't implement the new-style or old-style iteration protocol + /// + /// The new-style iteration protocol requires a type being iterated over to have an `__iter__` + /// method that returns something with a `__next__` method. The old-style iteration + /// protocol requires a type being iterated over to have a `__getitem__` method that accepts + /// a positive-integer argument. + NotIterable { not_iterable_ty: Type<'db> }, + + /// The type is iterable but the methods aren't always bound. + PossiblyUnbound { iterable_ty: Type<'db>, element_ty: Type<'db>, }, } -impl<'db> IterationOutcome<'db> { - fn unwrap_with_diagnostic( - self, - context: &InferContext<'db>, - iterable_node: ast::AnyNodeRef, - ) -> Type<'db> { +impl<'db> IterateError<'db> { + /// Reports the diagnostic for this error. + fn report_diagnostic(&self, context: &InferContext<'db>, iterable_node: ast::AnyNodeRef) { match self { - Self::Iterable { element_ty } => element_ty, Self::NotIterable { not_iterable_ty } => { - report_not_iterable(context, iterable_node, not_iterable_ty); - Type::unknown() + report_not_iterable(context, iterable_node, *not_iterable_ty); } - Self::PossiblyUnboundDunderIter { + Self::PossiblyUnbound { iterable_ty, - element_ty, + element_ty: _, } => { - report_not_iterable_possibly_unbound(context, iterable_node, iterable_ty); - element_ty + report_not_iterable_possibly_unbound(context, iterable_node, *iterable_ty); } } } - fn unwrap_without_diagnostic(self) -> Type<'db> { + /// Returns the element type if it is known, or `None` if the type is never iterable. + fn element_type(&self) -> Option> { match self { - Self::Iterable { element_ty } => element_ty, - Self::NotIterable { .. } => Type::unknown(), - Self::PossiblyUnboundDunderIter { element_ty, .. } => element_ty, + IterateError::NotIterable { .. } => None, + IterateError::PossiblyUnbound { element_ty, .. } => Some(*element_ty), + } + } + + /// Returns the element type if it is known, or `Type::unknown()` if it is not. + fn fallback_element_type(&self) -> Type<'db> { + self.element_type().unwrap_or(Type::unknown()) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum BoolError<'db> { + /// The type has a `__bool__` attribute but it can't be called. + NotCallable { not_boolable_type: Type<'db> }, + + /// The type has a callable `__bool__` attribute, but it isn't callable + /// with the given arguments. + IncorrectArguments { + not_boolable_type: Type<'db>, + truthiness: Truthiness, + }, + + /// The type has a `__bool__` method, is callable with the given arguments, + /// but the return type isn't assignable to `bool`. + IncorrectReturnType { + not_boolable_type: Type<'db>, + return_type: Type<'db>, + }, + + /// A union type doesn't implement `__bool__` correctly. + Union { + union: UnionType<'db>, + truthiness: Truthiness, + }, + + /// Any other reason why the type can't be converted to a bool. + /// E.g. because calling `__bool__` returns in a union type and not all variants support `__bool__` or + /// because `__bool__` points to a type that has a possibly unbound `__call__` method. + Other { not_boolable_type: Type<'db> }, +} + +impl<'db> BoolError<'db> { + pub(super) fn fallback_truthiness(&self) -> Truthiness { + match self { + BoolError::NotCallable { .. } + | BoolError::IncorrectReturnType { .. } + | BoolError::Other { .. } => Truthiness::Ambiguous, + BoolError::IncorrectArguments { truthiness, .. } + | BoolError::Union { truthiness, .. } => *truthiness, + } + } + + fn not_boolable_type(&self) -> Type<'db> { + match self { + BoolError::NotCallable { + not_boolable_type, .. + } + | BoolError::IncorrectArguments { + not_boolable_type, .. + } + | BoolError::Other { not_boolable_type } + | BoolError::IncorrectReturnType { + not_boolable_type, .. + } => *not_boolable_type, + BoolError::Union { union, .. } => Type::Union(*union), + } + } + + pub(super) fn report_diagnostic(&self, context: &InferContext, condition: impl Ranged) { + self.report_diagnostic_impl(context, condition.range()); + } + + fn report_diagnostic_impl(&self, context: &InferContext, condition: TextRange) { + match self { + Self::IncorrectArguments { + not_boolable_type, .. + } => { + context.report_lint( + &UNSUPPORTED_BOOL_CONVERSION, + condition, + format_args!( + "Boolean conversion is unsupported for type `{}`; it incorrectly implements `__bool__`", + not_boolable_type.display(context.db()) + ), + ); + } + Self::IncorrectReturnType { + not_boolable_type, + return_type, + } => { + context.report_lint( + &UNSUPPORTED_BOOL_CONVERSION, + condition, + format_args!( + "Boolean conversion is unsupported for type `{not_boolable}`; the return type of its bool method (`{return_type}`) isn't assignable to `bool", + not_boolable = not_boolable_type.display(context.db()), + return_type = return_type.display(context.db()) + ), + ); + } + Self::NotCallable { not_boolable_type } => { + context.report_lint( + &UNSUPPORTED_BOOL_CONVERSION, + condition, + format_args!( + "Boolean conversion is unsupported for type `{}`; its `__bool__` method isn't callable", + not_boolable_type.display(context.db()) + ), + ); + } + Self::Union { union, .. } => { + let first_error = union + .elements(context.db()) + .iter() + .find_map(|element| element.try_bool(context.db()).err()) + .unwrap(); + + context.report_lint( + &UNSUPPORTED_BOOL_CONVERSION, + condition, + format_args!( + "Boolean conversion is unsupported for union `{}` because `{}` doesn't implement `__bool__` correctly", + Type::Union(*union).display(context.db()), + first_error.not_boolable_type().display(context.db()), + ), + ); + } + + Self::Other { not_boolable_type } => { + context.report_lint( + &UNSUPPORTED_BOOL_CONVERSION, + condition, + format_args!( + "Boolean conversion is unsupported for type `{}`; it incorrectly implements `__bool__`", + not_boolable_type.display(context.db()) + ), + ); + } } } } @@ -4159,11 +4430,11 @@ impl<'db> Class<'db> { kind: MetaclassErrorKind::NotCallable(not_callable_ty), }), - Err(CallError::Union { + Err(CallError::Union(UnionCallError { called_ty, errors, bindings, - }) => { + })) => { let mut partly_not_callable = false; let return_ty = errors @@ -4389,10 +4660,9 @@ impl<'db> Class<'db> { // // for self.name in : - // TODO: Potential diagnostics resulting from the iterable are currently not reported. - let iterable_ty = infer_expression_type(db, *iterable); - let inferred_ty = iterable_ty.iterate(db).unwrap_without_diagnostic(); + // TODO: Potential diagnostics resulting from the iterable are currently not reported. + let inferred_ty = iterable_ty.iterate(db); union_of_inferred_types = union_of_inferred_types.add(inferred_ty); } diff --git a/crates/red_knot_python_semantic/src/types/call.rs b/crates/red_knot_python_semantic/src/types/call.rs index ad91c33ab9..8c45b7e154 100644 --- a/crates/red_knot_python_semantic/src/types/call.rs +++ b/crates/red_knot_python_semantic/src/types/call.rs @@ -57,11 +57,11 @@ impl<'db> CallOutcome<'db> { not_callable_ty: Type::Union(union), }) } else { - Err(CallError::Union { + Err(CallError::Union(UnionCallError { errors: errors.into(), bindings: bindings.into(), called_ty: Type::Union(union), - }) + })) } } @@ -96,16 +96,7 @@ pub(super) enum CallError<'db> { /// can't be called with the given arguments. /// /// A union where all variants are not callable is represented as a `NotCallable` error. - Union { - /// The variants that can't be called with the given arguments. - errors: Box<[CallError<'db>]>, - - /// The bindings for the callable variants (that have no binding errors). - bindings: Box<[CallBinding<'db>]>, - - /// The union type that we tried calling. - called_ty: Type<'db>, - }, + Union(UnionCallError<'db>), /// The type has a `__call__` method but it isn't always bound. PossiblyUnboundDunderCall { @@ -126,9 +117,9 @@ impl<'db> CallError<'db> { CallError::NotCallable { .. } => None, // If some variants are callable, and some are not, return the union of the return types of the callable variants // combined with `Type::Unknown` - CallError::Union { - errors, bindings, .. - } => Some(UnionType::from_elements( + CallError::Union(UnionCallError { + bindings, errors, .. + }) => Some(UnionType::from_elements( db, bindings .iter() @@ -158,7 +149,7 @@ impl<'db> CallError<'db> { Self::NotCallable { not_callable_ty, .. } => *not_callable_ty, - Self::Union { called_ty, .. } => *called_ty, + Self::Union(UnionCallError { called_ty, .. }) => *called_ty, Self::PossiblyUnboundDunderCall { called_type, .. } => *called_type, Self::BindingError { binding } => binding.callable_type(), } @@ -169,6 +160,18 @@ impl<'db> CallError<'db> { } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct UnionCallError<'db> { + /// The variants that can't be called with the given arguments. + pub(super) errors: Box<[CallError<'db>]>, + + /// The bindings for the callable variants (that have no binding errors). + pub(super) bindings: Box<[CallBinding<'db>]>, + + /// The union type that we tried calling. + pub(super) called_ty: Type<'db>, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum CallDunderError<'db> { /// The dunder attribute exists but it can't be called with the given arguments. diff --git a/crates/red_knot_python_semantic/src/types/diagnostic.rs b/crates/red_knot_python_semantic/src/types/diagnostic.rs index 63c0fc2b16..29538ac353 100644 --- a/crates/red_knot_python_semantic/src/types/diagnostic.rs +++ b/crates/red_knot_python_semantic/src/types/diagnostic.rs @@ -44,6 +44,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) { registry.register_lint(&MISSING_ARGUMENT); registry.register_lint(&NON_SUBSCRIPTABLE); registry.register_lint(&NOT_ITERABLE); + registry.register_lint(&UNSUPPORTED_BOOL_CONVERSION); registry.register_lint(&PARAMETER_ALREADY_ASSIGNED); registry.register_lint(&POSSIBLY_UNBOUND_ATTRIBUTE); registry.register_lint(&POSSIBLY_UNBOUND_IMPORT); @@ -490,6 +491,37 @@ declare_lint! { } } +declare_lint! { + /// ## What it does + /// Checks for bool conversions where the object doesn't correctly implement `__bool__`. + /// + /// ## Why is this bad? + /// If an exception is raised when you attempt to evaluate the truthiness of an object, + /// using the object in a boolean context will fail at runtime. + /// + /// ## Examples + /// + /// ```python + /// class NotBoolable: + /// __bool__ = None + /// + /// b1 = NotBoolable() + /// b2 = NotBoolable() + /// + /// if b1: # exception raised here + /// pass + /// + /// b1 and b2 # exception raised here + /// not b1 # exception raised here + /// b1 < b2 < b1 # exception raised here + /// ``` + pub(crate) static UNSUPPORTED_BOOL_CONVERSION = { + summary: "detects boolean conversion where the object incorrectly implements `__bool__`", + status: LintStatus::preview("1.0.0"), + default_level: Level::Error, + } +} + declare_lint! { /// ## What it does /// Checks for calls which provide more than one argument for a single parameter. diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index a49379be34..5265597f1c 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -33,7 +33,7 @@ use ruff_db::diagnostic::{DiagnosticId, Severity}; use ruff_db::files::File; use ruff_db::parsed::parsed_module; use ruff_python_ast::{self as ast, AnyNodeRef, ExprContext}; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use rustc_hash::{FxHashMap, FxHashSet}; use salsa; use salsa::plumbing::AsId; @@ -54,7 +54,7 @@ use crate::symbol::{ module_type_implicit_global_symbol, symbol, symbol_from_bindings, symbol_from_declarations, typing_extensions_symbol, LookupError, }; -use crate::types::call::{Argument, CallArguments}; +use crate::types::call::{Argument, CallArguments, UnionCallError}; use crate::types::diagnostic::{ report_invalid_arguments_to_annotated, report_invalid_assignment, report_invalid_attribute_assignment, report_unresolved_module, TypeCheckDiagnostics, @@ -69,9 +69,9 @@ use crate::types::mro::MroErrorKind; use crate::types::unpacker::{UnpackResult, Unpacker}; use crate::types::{ todo_type, Boundness, Class, ClassLiteralType, DynamicType, FunctionType, InstanceType, - IntersectionBuilder, IntersectionType, IterationOutcome, KnownClass, KnownFunction, - KnownInstanceType, MetaclassCandidate, MetaclassErrorKind, SliceLiteralType, SubclassOfType, - Symbol, SymbolAndQualifiers, Truthiness, TupleType, Type, TypeAliasType, TypeAndQualifiers, + IntersectionBuilder, IntersectionType, KnownClass, KnownFunction, KnownInstanceType, + MetaclassCandidate, MetaclassErrorKind, SliceLiteralType, SubclassOfType, Symbol, + SymbolAndQualifiers, Truthiness, TupleType, Type, TypeAliasType, TypeAndQualifiers, TypeArrayDisplay, TypeQualifiers, TypeVarBoundOrConstraints, TypeVarInstance, UnionBuilder, UnionType, }; @@ -1481,7 +1481,12 @@ impl<'db> TypeInferenceBuilder<'db> { elif_else_clauses, } = if_statement; - self.infer_standalone_expression(test); + let test_ty = self.infer_standalone_expression(test); + + if let Err(err) = test_ty.try_bool(self.db()) { + err.report_diagnostic(&self.context, &**test); + } + self.infer_body(body); for clause in elif_else_clauses { @@ -1492,7 +1497,11 @@ impl<'db> TypeInferenceBuilder<'db> { } = clause; if let Some(test) = &test { - self.infer_standalone_expression(test); + let test_ty = self.infer_standalone_expression(test); + + if let Err(err) = test_ty.try_bool(self.db()) { + err.report_diagnostic(&self.context, test); + } } self.infer_body(body); @@ -1888,9 +1897,15 @@ impl<'db> TypeInferenceBuilder<'db> { guard, } = case; self.infer_match_pattern(pattern); - guard - .as_deref() - .map(|guard| self.infer_standalone_expression(guard)); + + if let Some(guard) = guard.as_deref() { + let guard_ty = self.infer_standalone_expression(guard); + + if let Err(err) = guard_ty.try_bool(self.db()) { + err.report_diagnostic(&self.context, guard); + } + } + self.infer_body(body); } } @@ -2359,7 +2374,10 @@ impl<'db> TypeInferenceBuilder<'db> { } = for_statement; self.infer_target(target, iter, |db, iter_ty| { - iter_ty.iterate(db).unwrap_without_diagnostic() + // TODO: `infer_for_statement_definition` reports a diagnostic if `iter_ty` isn't iterable + // but only if the target is a name. We should report a diagnostic here if the target isn't a name: + // `for a.x in not_iterable: ... + iter_ty.iterate(db) }); self.infer_body(body); @@ -2388,9 +2406,10 @@ impl<'db> TypeInferenceBuilder<'db> { let name_ast_id = name.scoped_expression_id(self.db(), self.scope()); unpacked.expression_type(name_ast_id) } - TargetKind::Name => iterable_ty - .iterate(self.db()) - .unwrap_with_diagnostic(&self.context, iterable.into()), + TargetKind::Name => iterable_ty.try_iterate(self.db()).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, iterable.into()); + err.fallback_element_type() + }), } }; @@ -2406,7 +2425,12 @@ impl<'db> TypeInferenceBuilder<'db> { orelse, } = while_statement; - self.infer_standalone_expression(test); + let test_ty = self.infer_standalone_expression(test); + + if let Err(err) = test_ty.try_bool(self.db()) { + err.report_diagnostic(&self.context, &**test); + } + self.infer_body(body); self.infer_body(orelse); } @@ -2488,7 +2512,12 @@ impl<'db> TypeInferenceBuilder<'db> { msg, } = assert; - self.infer_expression(test); + let test_ty = self.infer_expression(test); + + if let Err(err) = test_ty.try_bool(self.db()) { + err.report_diagnostic(&self.context, &**test); + } + self.infer_optional_expression(msg.as_deref()); } @@ -3172,9 +3201,10 @@ impl<'db> TypeInferenceBuilder<'db> { // TODO: async iterables/iterators! -- Alex todo_type!("async iterables/iterators") } else { - iterable_ty - .iterate(self.db()) - .unwrap_with_diagnostic(&self.context, iterable.into()) + iterable_ty.try_iterate(self.db()).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, iterable.into()); + err.fallback_element_type() + }) }; self.types.expressions.insert( @@ -3230,7 +3260,10 @@ impl<'db> TypeInferenceBuilder<'db> { let body_ty = self.infer_expression(body); let orelse_ty = self.infer_expression(orelse); - match test_ty.bool(self.db()) { + match test_ty.try_bool(self.db()).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, &**test); + err.fallback_truthiness() + }) { Truthiness::AlwaysTrue => body_ty, Truthiness::AlwaysFalse => orelse_ty, Truthiness::Ambiguous => UnionType::from_elements(self.db(), [body_ty, orelse_ty]), @@ -3323,7 +3356,26 @@ impl<'db> TypeInferenceBuilder<'db> { } KnownFunction::StaticAssert => { if let Some((parameter_ty, message)) = binding.two_parameter_types() { - let truthiness = parameter_ty.bool(self.db()); + let truthiness = match parameter_ty.try_bool(self.db()) { + Ok(truthiness) => truthiness, + Err(err) => { + let condition = arguments + .find_argument("condition", 0) + .map(|argument| match argument { + ruff_python_ast::ArgOrKeyword::Arg(expr) => { + ast::AnyNodeRef::from(expr) + } + ruff_python_ast::ArgOrKeyword::Keyword(keyword) => { + ast::AnyNodeRef::from(keyword) + } + }) + .unwrap_or(ast::AnyNodeRef::from(call_expression)); + + err.report_diagnostic(&self.context, condition); + + continue; + } + }; if !truthiness.is_always_true() { if let Some(message) = @@ -3389,14 +3441,8 @@ impl<'db> TypeInferenceBuilder<'db> { ); } - CallError::Union { - called_ty: _, - bindings: _, - errors, - } => { - // TODO: Remove the `Vec::from` call once we use the Rust 2024 edition - // which adds `Box<[T]>::into_iter` - if let Some(first) = Vec::from(errors).into_iter().next() { + CallError::Union(UnionCallError { errors, .. }) => { + if let Some(first) = IntoIterator::into_iter(errors).next() { report_call_error(context, first, call_expression); } else { debug_assert!( @@ -3438,9 +3484,10 @@ impl<'db> TypeInferenceBuilder<'db> { } = starred; let iterable_ty = self.infer_expression(value); - iterable_ty - .iterate(self.db()) - .unwrap_with_diagnostic(&self.context, value.as_ref().into()); + iterable_ty.try_iterate(self.db()).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, value.as_ref().into()); + err.fallback_element_type() + }); // TODO todo_type!("starred expression") @@ -3456,9 +3503,10 @@ impl<'db> TypeInferenceBuilder<'db> { let ast::ExprYieldFrom { range: _, value } = yield_from; let iterable_ty = self.infer_expression(value); - iterable_ty - .iterate(self.db()) - .unwrap_with_diagnostic(&self.context, value.as_ref().into()); + iterable_ty.try_iterate(self.db()).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, value.as_ref().into()); + err.fallback_element_type() + }); // TODO get type from `ReturnType` of generator todo_type!("Generic `typing.Generator` type") @@ -3754,7 +3802,14 @@ impl<'db> TypeInferenceBuilder<'db> { Type::IntLiteral(!i64::from(bool)) } - (ast::UnaryOp::Not, ty) => ty.bool(self.db()).negate().into_type(self.db()), + (ast::UnaryOp::Not, ty) => ty + .try_bool(self.db()) + .unwrap_or_else(|err| { + err.report_diagnostic(&self.context, unary); + err.fallback_truthiness() + }) + .negate() + .into_type(self.db()), ( op @ (ast::UnaryOp::UAdd | ast::UnaryOp::USub | ast::UnaryOp::Invert), Type::FunctionLiteral(_) @@ -4099,11 +4154,13 @@ impl<'db> TypeInferenceBuilder<'db> { *op, values.iter().enumerate(), |builder, (index, value)| { - if index == values.len() - 1 { + let ty = if index == values.len() - 1 { builder.infer_expression(value) } else { builder.infer_standalone_expression(value) - } + }; + + (ty, value.range()) }, ) } @@ -4120,7 +4177,7 @@ impl<'db> TypeInferenceBuilder<'db> { ) -> Type<'db> where Iterator: IntoIterator, - F: Fn(&mut Self, Item) -> Type<'db>, + F: Fn(&mut Self, Item) -> (Type<'db>, TextRange), { let mut done = false; let db = self.db(); @@ -4128,37 +4185,48 @@ impl<'db> TypeInferenceBuilder<'db> { let elements = operations .into_iter() .with_position() - .map(|(position, ty)| { - let ty = infer_ty(self, ty); - - if done { - return Type::Never; - } + .map(|(position, item)| { + let (ty, range) = infer_ty(self, item); let is_last = matches!( position, itertools::Position::Last | itertools::Position::Only ); - match (ty.bool(db), is_last, op) { - (Truthiness::AlwaysTrue, false, ast::BoolOp::And) => Type::Never, - (Truthiness::AlwaysFalse, false, ast::BoolOp::Or) => Type::Never, - - (Truthiness::AlwaysFalse, _, ast::BoolOp::And) - | (Truthiness::AlwaysTrue, _, ast::BoolOp::Or) => { - done = true; + if is_last { + if done { + Type::Never + } else { ty } + } else { + let truthiness = ty.try_bool(self.db()).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, range); + err.fallback_truthiness() + }); - (Truthiness::Ambiguous, false, _) => IntersectionBuilder::new(db) - .add_positive(ty) - .add_negative(match op { - ast::BoolOp::And => Type::AlwaysTruthy, - ast::BoolOp::Or => Type::AlwaysFalsy, - }) - .build(), + if done { + return Type::Never; + }; - (_, true, _) => ty, + match (truthiness, op) { + (Truthiness::AlwaysTrue, ast::BoolOp::And) => Type::Never, + (Truthiness::AlwaysFalse, ast::BoolOp::Or) => Type::Never, + + (Truthiness::AlwaysFalse, ast::BoolOp::And) + | (Truthiness::AlwaysTrue, ast::BoolOp::Or) => { + done = true; + ty + } + + (Truthiness::Ambiguous, _) => IntersectionBuilder::new(db) + .add_positive(ty) + .add_negative(match op { + ast::BoolOp::And => Type::AlwaysTruthy, + ast::BoolOp::Or => Type::AlwaysFalsy, + }) + .build(), + } } }); @@ -4174,9 +4242,6 @@ impl<'db> TypeInferenceBuilder<'db> { } = compare; self.infer_expression(left); - for right in comparators { - self.infer_expression(right); - } // https://docs.python.org/3/reference/expressions.html#comparisons // > Formally, if `a, b, c, …, y, z` are expressions and `op1, op2, …, opN` are comparison @@ -4193,15 +4258,17 @@ impl<'db> TypeInferenceBuilder<'db> { .zip(ops), |builder, ((left, right), op)| { let left_ty = builder.expression_type(left); - let right_ty = builder.expression_type(right); + let right_ty = builder.infer_expression(right); - builder - .infer_binary_type_comparison(left_ty, *op, right_ty) + let range = TextRange::new(left.start(), right.end()); + + let ty = builder + .infer_binary_type_comparison(left_ty, *op, right_ty, range) .unwrap_or_else(|error| { // Handle unsupported operators (diagnostic, `bool`/`Unknown` outcome) builder.context.report_lint( &UNSUPPORTED_OPERATOR, - AnyNodeRef::ExprCompare(compare), + range, format_args!( "Operator `{}` is not supported for types `{}` and `{}`{}", error.op, @@ -4228,7 +4295,9 @@ impl<'db> TypeInferenceBuilder<'db> { // Other operators can return arbitrary types _ => Type::unknown(), } - }) + }); + + (ty, range) }, ) } @@ -4239,14 +4308,19 @@ impl<'db> TypeInferenceBuilder<'db> { op: ast::CmpOp, other: Type<'db>, intersection_on: IntersectionOn, + range: TextRange, ) -> Result, CompareUnsupportedError<'db>> { // If a comparison yields a definitive true/false answer on a (positive) part // of an intersection type, it will also yield a definitive answer on the full // intersection type, which is even more specific. for pos in intersection.positive(self.db()) { let result = match intersection_on { - IntersectionOn::Left => self.infer_binary_type_comparison(*pos, op, other)?, - IntersectionOn::Right => self.infer_binary_type_comparison(other, op, *pos)?, + IntersectionOn::Left => { + self.infer_binary_type_comparison(*pos, op, other, range)? + } + IntersectionOn::Right => { + self.infer_binary_type_comparison(other, op, *pos, range)? + } }; if let Type::BooleanLiteral(b) = result { return Ok(Type::BooleanLiteral(b)); @@ -4257,8 +4331,12 @@ impl<'db> TypeInferenceBuilder<'db> { // special cases that allow us to narrow down the result type of the comparison. for neg in intersection.negative(self.db()) { let result = match intersection_on { - IntersectionOn::Left => self.infer_binary_type_comparison(*neg, op, other).ok(), - IntersectionOn::Right => self.infer_binary_type_comparison(other, op, *neg).ok(), + IntersectionOn::Left => self + .infer_binary_type_comparison(*neg, op, other, range) + .ok(), + IntersectionOn::Right => self + .infer_binary_type_comparison(other, op, *neg, range) + .ok(), }; match (op, result) { @@ -4319,8 +4397,12 @@ impl<'db> TypeInferenceBuilder<'db> { let mut builder = IntersectionBuilder::new(self.db()); for pos in intersection.positive(self.db()) { let result = match intersection_on { - IntersectionOn::Left => self.infer_binary_type_comparison(*pos, op, other)?, - IntersectionOn::Right => self.infer_binary_type_comparison(other, op, *pos)?, + IntersectionOn::Left => { + self.infer_binary_type_comparison(*pos, op, other, range)? + } + IntersectionOn::Right => { + self.infer_binary_type_comparison(other, op, *pos, range)? + } }; builder = builder.add_positive(result); } @@ -4339,6 +4421,7 @@ impl<'db> TypeInferenceBuilder<'db> { left: Type<'db>, op: ast::CmpOp, right: Type<'db>, + range: TextRange, ) -> Result, CompareUnsupportedError<'db>> { // Note: identity (is, is not) for equal builtin types is unreliable and not part of the // language spec. @@ -4348,14 +4431,16 @@ impl<'db> TypeInferenceBuilder<'db> { (Type::Union(union), other) => { let mut builder = UnionBuilder::new(self.db()); for element in union.elements(self.db()) { - builder = builder.add(self.infer_binary_type_comparison(*element, op, other)?); + builder = + builder.add(self.infer_binary_type_comparison(*element, op, other, range)?); } Ok(builder.build()) } (other, Type::Union(union)) => { let mut builder = UnionBuilder::new(self.db()); for element in union.elements(self.db()) { - builder = builder.add(self.infer_binary_type_comparison(other, op, *element)?); + builder = + builder.add(self.infer_binary_type_comparison(other, op, *element, range)?); } Ok(builder.build()) } @@ -4366,6 +4451,7 @@ impl<'db> TypeInferenceBuilder<'db> { op, right, IntersectionOn::Left, + range, ), (left, Type::Intersection(intersection)) => self .infer_binary_intersection_type_comparison( @@ -4373,6 +4459,7 @@ impl<'db> TypeInferenceBuilder<'db> { op, left, IntersectionOn::Right, + range, ), (Type::IntLiteral(n), Type::IntLiteral(m)) => match op { @@ -4403,29 +4490,38 @@ impl<'db> TypeInferenceBuilder<'db> { right_ty: right, }), }, - (Type::IntLiteral(_), Type::Instance(_)) => { - self.infer_binary_type_comparison(KnownClass::Int.to_instance(self.db()), op, right) - } - (Type::Instance(_), Type::IntLiteral(_)) => { - self.infer_binary_type_comparison(left, op, KnownClass::Int.to_instance(self.db())) - } + (Type::IntLiteral(_), Type::Instance(_)) => self.infer_binary_type_comparison( + KnownClass::Int.to_instance(self.db()), + op, + right, + range, + ), + (Type::Instance(_), Type::IntLiteral(_)) => self.infer_binary_type_comparison( + left, + op, + KnownClass::Int.to_instance(self.db()), + range, + ), // Booleans are coded as integers (False = 0, True = 1) (Type::IntLiteral(n), Type::BooleanLiteral(b)) => self.infer_binary_type_comparison( Type::IntLiteral(n), op, Type::IntLiteral(i64::from(b)), + range, ), (Type::BooleanLiteral(b), Type::IntLiteral(m)) => self.infer_binary_type_comparison( Type::IntLiteral(i64::from(b)), op, Type::IntLiteral(m), + range, ), (Type::BooleanLiteral(a), Type::BooleanLiteral(b)) => self .infer_binary_type_comparison( Type::IntLiteral(i64::from(a)), op, Type::IntLiteral(i64::from(b)), + range, ), (Type::StringLiteral(salsa_s1), Type::StringLiteral(salsa_s2)) => { @@ -4456,19 +4552,31 @@ impl<'db> TypeInferenceBuilder<'db> { } } } - (Type::StringLiteral(_), _) => { - self.infer_binary_type_comparison(KnownClass::Str.to_instance(self.db()), op, right) - } - (_, Type::StringLiteral(_)) => { - self.infer_binary_type_comparison(left, op, KnownClass::Str.to_instance(self.db())) - } + (Type::StringLiteral(_), _) => self.infer_binary_type_comparison( + KnownClass::Str.to_instance(self.db()), + op, + right, + range, + ), + (_, Type::StringLiteral(_)) => self.infer_binary_type_comparison( + left, + op, + KnownClass::Str.to_instance(self.db()), + range, + ), - (Type::LiteralString, _) => { - self.infer_binary_type_comparison(KnownClass::Str.to_instance(self.db()), op, right) - } - (_, Type::LiteralString) => { - self.infer_binary_type_comparison(left, op, KnownClass::Str.to_instance(self.db())) - } + (Type::LiteralString, _) => self.infer_binary_type_comparison( + KnownClass::Str.to_instance(self.db()), + op, + right, + range, + ), + (_, Type::LiteralString) => self.infer_binary_type_comparison( + left, + op, + KnownClass::Str.to_instance(self.db()), + range, + ), (Type::BytesLiteral(salsa_b1), Type::BytesLiteral(salsa_b2)) => { let b1 = &**salsa_b1.value(self.db()); @@ -4506,21 +4614,33 @@ impl<'db> TypeInferenceBuilder<'db> { KnownClass::Bytes.to_instance(self.db()), op, right, + range, ), (_, Type::BytesLiteral(_)) => self.infer_binary_type_comparison( left, op, KnownClass::Bytes.to_instance(self.db()), + range, ), (Type::Tuple(_), Type::Instance(InstanceType { class })) if class.is_known(self.db(), KnownClass::VersionInfo) => { - self.infer_binary_type_comparison(left, op, Type::version_info_tuple(self.db())) + self.infer_binary_type_comparison( + left, + op, + Type::version_info_tuple(self.db()), + range, + ) } (Type::Instance(InstanceType { class }), Type::Tuple(_)) if class.is_known(self.db(), KnownClass::VersionInfo) => { - self.infer_binary_type_comparison(Type::version_info_tuple(self.db()), op, right) + self.infer_binary_type_comparison( + Type::version_info_tuple(self.db()), + op, + right, + range, + ) } (Type::Tuple(lhs), Type::Tuple(rhs)) => { // Note: This only works on heterogeneous tuple types. @@ -4528,7 +4648,7 @@ impl<'db> TypeInferenceBuilder<'db> { let rhs_elements = rhs.elements(self.db()); let mut tuple_rich_comparison = - |op| self.infer_tuple_rich_comparison(lhs_elements, op, rhs_elements); + |op| self.infer_tuple_rich_comparison(lhs_elements, op, rhs_elements, range); match op { ast::CmpOp::Eq => tuple_rich_comparison(RichCompareOperator::Eq), @@ -4546,10 +4666,14 @@ impl<'db> TypeInferenceBuilder<'db> { Type::Tuple(lhs), ast::CmpOp::Eq, *ty, + range, ).expect("infer_binary_type_comparison should never return None for `CmpOp::Eq`"); match eq_result { todo @ Type::Dynamic(DynamicType::Todo(_)) => return Ok(todo), + // It's okay to ignore errors here because Python doesn't call `__bool__` + // for different union variants. Instead, this is just for us to + // evaluate a possibly truthy value to `false` or `true`. ty => match ty.bool(self.db()) { Truthiness::AlwaysTrue => eq_count += 1, Truthiness::AlwaysFalse => not_eq_count += 1, @@ -4575,6 +4699,9 @@ impl<'db> TypeInferenceBuilder<'db> { Ok(match eq_result { todo @ Type::Dynamic(DynamicType::Todo(_)) => todo, + // It's okay to ignore errors here because Python doesn't call `__bool__` + // for `is` and `is not` comparisons. This is an implementation detail + // for how we determine the truthiness of a type. ty => match ty.bool(self.db()) { Truthiness::AlwaysFalse => Type::BooleanLiteral(op.is_is_not()), _ => KnownClass::Bool.to_instance(self.db()), @@ -4588,8 +4715,9 @@ impl<'db> TypeInferenceBuilder<'db> { (Type::Instance(left_instance), Type::Instance(right_instance)) => { let rich_comparison = |op| self.infer_rich_comparison(left_instance, right_instance, op); - let membership_test_comparison = - |op| self.infer_membership_test_comparison(left_instance, right_instance, op); + let membership_test_comparison = |op, range: TextRange| { + self.infer_membership_test_comparison(left_instance, right_instance, op, range) + }; match op { ast::CmpOp::Eq => rich_comparison(RichCompareOperator::Eq), ast::CmpOp::NotEq => rich_comparison(RichCompareOperator::Ne), @@ -4597,9 +4725,11 @@ impl<'db> TypeInferenceBuilder<'db> { ast::CmpOp::LtE => rich_comparison(RichCompareOperator::Le), ast::CmpOp::Gt => rich_comparison(RichCompareOperator::Gt), ast::CmpOp::GtE => rich_comparison(RichCompareOperator::Ge), - ast::CmpOp::In => membership_test_comparison(MembershipTestCompareOperator::In), + ast::CmpOp::In => { + membership_test_comparison(MembershipTestCompareOperator::In, range) + } ast::CmpOp::NotIn => { - membership_test_comparison(MembershipTestCompareOperator::NotIn) + membership_test_comparison(MembershipTestCompareOperator::NotIn, range) } ast::CmpOp::Is => { if left.is_disjoint_from(self.db(), right) { @@ -4648,7 +4778,6 @@ impl<'db> TypeInferenceBuilder<'db> { let call_dunder = |op: RichCompareOperator, left: InstanceType<'db>, right: InstanceType<'db>| { - // TODO: How do we want to handle possibly unbound dunder methods? match left.class.class_member(db, op.dunder()) { Symbol::Type(class_member_dunder, Boundness::Bound) => class_member_dunder .try_call( @@ -4693,6 +4822,7 @@ impl<'db> TypeInferenceBuilder<'db> { left: InstanceType<'db>, right: InstanceType<'db>, op: MembershipTestCompareOperator, + range: TextRange, ) -> Result, CompareUnsupportedError<'db>> { let db = self.db(); @@ -4710,11 +4840,10 @@ impl<'db> TypeInferenceBuilder<'db> { } _ => { // iteration-based membership test - match Type::Instance(right).iterate(db) { - IterationOutcome::Iterable { .. } => Some(KnownClass::Bool.to_instance(db)), - IterationOutcome::NotIterable { .. } - | IterationOutcome::PossiblyUnboundDunderIter { .. } => None, - } + Type::Instance(right) + .try_iterate(db) + .map(|_| KnownClass::Bool.to_instance(db)) + .ok() } }; @@ -4724,7 +4853,10 @@ impl<'db> TypeInferenceBuilder<'db> { return ty; } - let truthiness = ty.bool(db); + let truthiness = ty.try_bool(db).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, range); + err.fallback_truthiness() + }); match op { MembershipTestCompareOperator::In => truthiness.into_type(db), @@ -4748,6 +4880,7 @@ impl<'db> TypeInferenceBuilder<'db> { left: &[Type<'db>], op: RichCompareOperator, right: &[Type<'db>], + range: TextRange, ) -> Result, CompareUnsupportedError<'db>> { let left_iter = left.iter().copied(); let right_iter = right.iter().copied(); @@ -4756,46 +4889,49 @@ impl<'db> TypeInferenceBuilder<'db> { for (l_ty, r_ty) in left_iter.zip(right_iter) { let pairwise_eq_result = self - .infer_binary_type_comparison(l_ty, ast::CmpOp::Eq, r_ty) + .infer_binary_type_comparison(l_ty, ast::CmpOp::Eq, r_ty, range) .expect("infer_binary_type_comparison should never return None for `CmpOp::Eq`"); - match pairwise_eq_result { - // If propagation is required, return the result as is - todo @ Type::Dynamic(DynamicType::Todo(_)) => return Ok(todo), - ty => match ty.bool(self.db()) { - // - AlwaysTrue : Continue to the next pair for lexicographic comparison - Truthiness::AlwaysTrue => continue, - // - AlwaysFalse: - // Lexicographic comparisons will always terminate with this pair. - // Complete the comparison and return the result. - // - Ambiguous: - // Lexicographic comparisons might continue to the next pair (if eq_result is true), - // or terminate here (if eq_result is false). - // To account for cases where the comparison terminates here, add the pairwise comparison result to the union builder. - eq_truthiness @ (Truthiness::AlwaysFalse | Truthiness::Ambiguous) => { - let pairwise_compare_result = match op { - RichCompareOperator::Lt - | RichCompareOperator::Le - | RichCompareOperator::Gt - | RichCompareOperator::Ge => { - self.infer_binary_type_comparison(l_ty, op.into(), r_ty)? - } - // For `==` and `!=`, we already figure out the result from `pairwise_eq_result` - // NOTE: The CPython implementation does not account for non-boolean return types - // or cases where `!=` is not the negation of `==`, we also do not consider these cases. - RichCompareOperator::Eq => Type::BooleanLiteral(false), - RichCompareOperator::Ne => Type::BooleanLiteral(true), - }; - - builder = builder.add(pairwise_compare_result); - - if eq_truthiness.is_ambiguous() { - continue; + match pairwise_eq_result + .try_bool(self.db()) + .unwrap_or_else(|err| { + // TODO: We should, whenever possible, pass the range of the left and right elements + // instead of the range of the whole tuple. + err.report_diagnostic(&self.context, range); + err.fallback_truthiness() + }) { + // - AlwaysTrue : Continue to the next pair for lexicographic comparison + Truthiness::AlwaysTrue => continue, + // - AlwaysFalse: + // Lexicographic comparisons will always terminate with this pair. + // Complete the comparison and return the result. + // - Ambiguous: + // Lexicographic comparisons might continue to the next pair (if eq_result is true), + // or terminate here (if eq_result is false). + // To account for cases where the comparison terminates here, add the pairwise comparison result to the union builder. + eq_truthiness @ (Truthiness::AlwaysFalse | Truthiness::Ambiguous) => { + let pairwise_compare_result = match op { + RichCompareOperator::Lt + | RichCompareOperator::Le + | RichCompareOperator::Gt + | RichCompareOperator::Ge => { + self.infer_binary_type_comparison(l_ty, op.into(), r_ty, range)? } + // For `==` and `!=`, we already figure out the result from `pairwise_eq_result` + // NOTE: The CPython implementation does not account for non-boolean return types + // or cases where `!=` is not the negation of `==`, we also do not consider these cases. + RichCompareOperator::Eq => Type::BooleanLiteral(false), + RichCompareOperator::Ne => Type::BooleanLiteral(true), + }; - return Ok(builder.build()); + builder = builder.add(pairwise_compare_result); + + if eq_truthiness.is_ambiguous() { + continue; } - }, + + return Ok(builder.build()); + } } } diff --git a/crates/red_knot_python_semantic/src/types/unpacker.rs b/crates/red_knot_python_semantic/src/types/unpacker.rs index 3173a9dc28..40a20e4e86 100644 --- a/crates/red_knot_python_semantic/src/types/unpacker.rs +++ b/crates/red_knot_python_semantic/src/types/unpacker.rs @@ -57,9 +57,10 @@ impl<'db> Unpacker<'db> { if value.is_iterable() { // If the value is an iterable, then the type that needs to be unpacked is the iterator // type. - value_ty = value_ty - .iterate(self.db()) - .unwrap_with_diagnostic(&self.context, value.as_any_node_ref(self.db())); + value_ty = value_ty.try_iterate(self.db()).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, value.as_any_node_ref(self.db())); + err.fallback_element_type() + }); } self.unpack_inner(target, value.as_any_node_ref(self.db()), value_ty); @@ -155,8 +156,10 @@ impl<'db> Unpacker<'db> { let ty = if ty.is_literal_string() { Type::LiteralString } else { - ty.iterate(self.db()) - .unwrap_with_diagnostic(&self.context, value_expr) + ty.try_iterate(self.db()).unwrap_or_else(|err| { + err.report_diagnostic(&self.context, value_expr); + err.fallback_element_type() + }) }; for target_type in &mut target_types { target_type.push(ty); diff --git a/crates/red_knot_test/src/lib.rs b/crates/red_knot_test/src/lib.rs index cefb50fb78..0c75d54ec2 100644 --- a/crates/red_knot_test/src/lib.rs +++ b/crates/red_knot_test/src/lib.rs @@ -253,10 +253,15 @@ fn run_test( }) .collect(); - if !snapshot_diagnostics.is_empty() { + if snapshot_diagnostics.is_empty() && test.should_snapshot_diagnostics() { + panic!( + "Test `{}` requested snapshotting diagnostics but it didn't produce any.", + test.name() + ); + } else if !snapshot_diagnostics.is_empty() { let snapshot = create_diagnostic_snapshot(db, relative_fixture_path, test, snapshot_diagnostics); - let name = test.name().replace(' ', "_"); + let name = test.name().replace(' ', "_").replace(':', "__"); insta::with_settings!( { snapshot_path => snapshot_path, diff --git a/crates/red_knot_test/src/parser.rs b/crates/red_knot_test/src/parser.rs index 8ed9d40d1d..3e709d95ee 100644 --- a/crates/red_knot_test/src/parser.rs +++ b/crates/red_knot_test/src/parser.rs @@ -595,6 +595,10 @@ impl<'s> Parser<'s> { return self.process_config_block(code); } + if lang == "ignore" { + return Ok(()); + } + if let Some(explicit_path) = self.explicit_path { if !lang.is_empty() && lang != "text" @@ -618,7 +622,7 @@ impl<'s> Parser<'s> { EmbeddedFilePath::Explicit(path) } None => match lang { - "py" => EmbeddedFilePath::Autogenerated(PySourceType::Python), + "py" | "python" => EmbeddedFilePath::Autogenerated(PySourceType::Python), "pyi" => EmbeddedFilePath::Autogenerated(PySourceType::Stub), "" => { bail!("Cannot auto-generate file name for code block with empty language specifier in test `{test_name}`"); diff --git a/knot.schema.json b/knot.schema.json index 08e2c57a18..6f8ad6be7a 100644 --- a/knot.schema.json +++ b/knot.schema.json @@ -651,6 +651,16 @@ } ] }, + "unsupported-bool-conversion": { + "title": "detects boolean conversion where the object incorrectly implements `__bool__`", + "description": "## What it does\nChecks for bool conversions where the object doesn't correctly implement `__bool__`.\n\n## Why is this bad?\nIf an exception is raised when you attempt to evaluate the truthiness of an object,\nusing the object in a boolean context will fail at runtime.\n\n## Examples\n\n```python\nclass NotBoolable:\n __bool__ = None\n\nb1 = NotBoolable()\nb2 = NotBoolable()\n\nif b1: # exception raised here\n pass\n\nb1 and b2 # exception raised here\nnot b1 # exception raised here\nb1 < b2 < b1 # exception raised here\n```", + "default": "error", + "oneOf": [ + { + "$ref": "#/definitions/Level" + } + ] + }, "unsupported-operator": { "title": "detects binary, unary, or comparison expressions where the operands don't support the operator", "description": "## What it does\nChecks for binary expressions, comparisons, and unary expressions where the operands don't support the operator.\n\nTODO #14889",