Skip to content

Commit 0f47810

Browse files
committed
red_knot_python_semantic: improve diagnostics for unsupported boolean conversions
This mostly only improves things for incorrect arguments and for an incorrect return type. It doesn't do much to improve the case where `__bool__` isn't callable and leaves the union/other cases untouched completely. I picked this one because, at first glance, this _looked_ like a lower hanging fruit. The conceptual improvement here is pretty straight-forward: add annotations for relevant data. But it took me a bit to figure out how to connect all of the pieces.
1 parent eb1d251 commit 0f47810

19 files changed

+107
-35
lines changed

crates/red_knot_python_semantic/resources/mdtest/conditional/if_expression.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,6 @@ def _(flag: bool):
4242
class NotBoolable:
4343
__bool__: int = 3
4444

45-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable"
45+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`"
4646
3 if NotBoolable() else 4
4747
```

crates/red_knot_python_semantic/resources/mdtest/conditional/if_statement.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,10 @@ def _(flag: bool):
154154
class NotBoolable:
155155
__bool__: int = 3
156156

157-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable"
157+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`"
158158
if NotBoolable():
159159
...
160-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable"
160+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`"
161161
elif NotBoolable():
162162
...
163163
```

crates/red_knot_python_semantic/resources/mdtest/conditional/match.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ class NotBoolable:
292292
def _(target: int, flag: NotBoolable):
293293
y = 1
294294
match target:
295-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable"
295+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`"
296296
case 1 if flag:
297297
y = 2
298298
case 2:

crates/red_knot_python_semantic/resources/mdtest/expression/assert.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
class NotBoolable:
55
__bool__: int = 3
66

7-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable"
7+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`"
88
assert NotBoolable()
99
```

crates/red_knot_python_semantic/resources/mdtest/expression/boolean.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ if NotBoolable():
123123
class NotBoolable:
124124
__bool__: None = None
125125

126-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable"
126+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`"
127127
if NotBoolable():
128128
...
129129
```
@@ -135,7 +135,7 @@ def test(cond: bool):
135135
class NotBoolable:
136136
__bool__: int | None = None if cond else 3
137137

138-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable"
138+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`"
139139
if NotBoolable():
140140
...
141141
```
@@ -149,7 +149,7 @@ def test(cond: bool):
149149

150150
a = 10 if cond else NotBoolable()
151151

152-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `Literal[10] | NotBoolable`; its `__bool__` method isn't callable"
152+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `Literal[10] | NotBoolable`"
153153
if a:
154154
...
155155
```

crates/red_knot_python_semantic/resources/mdtest/loops/while_loop.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ def _(flag: bool, flag2: bool):
123123
class NotBoolable:
124124
__bool__: int = 3
125125

126-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable"
126+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `NotBoolable`"
127127
while NotBoolable():
128128
...
129129
```

crates/red_knot_python_semantic/resources/mdtest/narrow/truthiness.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ def _(
270270
if af:
271271
reveal_type(af) # revealed: type[AmbiguousClass] & ~AlwaysFalsy
272272

273-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `MetaDeferred`; the return type of its bool method (`MetaAmbiguous`) isn't assignable to `bool"
273+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `MetaDeferred`"
274274
if d:
275275
# TODO: Should be `Unknown`
276276
reveal_type(d) # revealed: type[DeferredClass] & ~AlwaysFalsy

crates/red_knot_python_semantic/resources/mdtest/snapshots/instances.md_-_Binary_operations_on_instances_-_Operations_involving_types_with_invalid_`__bool__`_methods.snap

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,13 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/binary/instances.m
2424
# Diagnostics
2525

2626
```
27-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable
27+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
2828
--> /src/mdtest_snippet.py:7:8
2929
|
3030
6 | # error: [unsupported-bool-conversion]
3131
7 | 10 and a and True
3232
| ^
3333
|
34+
info: `__bool__` on `NotBoolable` must be callable
3435
3536
```

crates/red_knot_python_semantic/resources/mdtest/snapshots/membership_test.md_-_Comparison___Membership_Test_-_Return_type_that_doesn't_implement_`__bool__`_correctly.snap

+4-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/comparison/instanc
2828
# Diagnostics
2929

3030
```
31-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable
31+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
3232
--> /src/mdtest_snippet.py:9:1
3333
|
3434
8 | # error: [unsupported-bool-conversion]
@@ -37,17 +37,19 @@ error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for t
3737
10 | # error: [unsupported-bool-conversion]
3838
11 | 10 not in WithContains()
3939
|
40+
info: `__bool__` on `NotBoolable` must be callable
4041

4142
```
4243

4344
```
44-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable
45+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
4546
--> /src/mdtest_snippet.py:11:1
4647
|
4748
9 | 10 in WithContains()
4849
10 | # error: [unsupported-bool-conversion]
4950
11 | 10 not in WithContains()
5051
| ^^^^^^^^^^^^^^^^^^^^^^^^
5152
|
53+
info: `__bool__` on `NotBoolable` must be callable
5254

5355
```

crates/red_knot_python_semantic/resources/mdtest/snapshots/not.md_-_Unary_not_-_Object_that_implements_`__bool__`_incorrectly.snap

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/unary/not.md
2222
# Diagnostics
2323

2424
```
25-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable
25+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
2626
--> /src/mdtest_snippet.py:5:1
2727
|
2828
4 | # error: [unsupported-bool-conversion]
2929
5 | not NotBoolable()
3030
| ^^^^^^^^^^^^^^^^^
3131
|
32+
info: `__bool__` on `NotBoolable` must be callable
3233
3334
```

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/comparison/instanc
3333
# Diagnostics
3434

3535
```
36-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable
36+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
3737
--> /src/mdtest_snippet.py:12:1
3838
|
3939
11 | # error: [unsupported-bool-conversion]
@@ -42,11 +42,12 @@ error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for t
4242
13 | # error: [unsupported-bool-conversion]
4343
14 | 10 < Comparable() < Comparable()
4444
|
45+
info: `__bool__` on `NotBoolable` must be callable
4546
4647
```
4748

4849
```
49-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable
50+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
5051
--> /src/mdtest_snippet.py:14:1
5152
|
5253
12 | 10 < Comparable() < 20
@@ -56,5 +57,6 @@ error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for t
5657
15 |
5758
16 | Comparable() < Comparable() # fine
5859
|
60+
info: `__bool__` on `NotBoolable` must be callable
5961
6062
```

crates/red_knot_python_semantic/resources/mdtest/snapshots/tuples.md_-_Comparison___Tuples_-_Chained_comparisons_with_elements_that_incorrectly_implement_`__bool__`.snap

+2-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.
3434
# Diagnostics
3535
3636
```
37-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable | Literal[False]`; its `__bool__` method isn't callable
37+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable | Literal[False]`
3838
--> /src/mdtest_snippet.py:15:1
3939
|
4040
14 | # error: [unsupported-bool-conversion]
@@ -43,5 +43,6 @@ error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for t
4343
16 |
4444
17 | a < b # fine
4545
|
46+
info: `__bool__` on `NotBoolable | Literal[False]` must be callable
4647
4748
```

crates/red_knot_python_semantic/resources/mdtest/snapshots/tuples.md_-_Comparison___Tuples_-_Equality_with_elements_that_incorrectly_implement_`__bool__`.snap

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,13 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/comparison/tuples.
2626
# Diagnostics
2727

2828
```
29-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable
29+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
3030
--> /src/mdtest_snippet.py:9:1
3131
|
3232
8 | # error: [unsupported-bool-conversion]
3333
9 | (A(),) == (A(),)
3434
| ^^^^^^^^^^^^^^^^
3535
|
36+
info: `__bool__` on `NotBoolable` must be callable
3637
3738
```

crates/red_knot_python_semantic/resources/mdtest/snapshots/unsupported_bool_conversion.md_-_Different_ways_that_`UNSUPPORTED_BOOL_CONVERSION`_can_occur_-_Has_a_`__bool__`_attribute,_but_it's_not_callable.snap

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,13 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unsupp
2424
# Diagnostics
2525

2626
```
27-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; its `__bool__` method isn't callable
27+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
2828
--> /src/mdtest_snippet.py:7:8
2929
|
3030
6 | # error: [unsupported-bool-conversion]
3131
7 | 10 and a and True
3232
| ^
3333
|
34+
info: `__bool__` must be callable
3435
3536
```

crates/red_knot_python_semantic/resources/mdtest/snapshots/unsupported_bool_conversion.md_-_Different_ways_that_`UNSUPPORTED_BOOL_CONVERSION`_can_occur_-_Has_a_`__bool__`_method,_but_has_an_incorrect_return_type.snap

+11-1
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,22 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unsupp
2525
# Diagnostics
2626

2727
```
28-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; the return type of its bool method (`str`) isn't assignable to `bool
28+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
2929
--> /src/mdtest_snippet.py:8:8
3030
|
3131
7 | # error: [unsupported-bool-conversion]
3232
8 | 10 and a and True
3333
| ^
3434
|
35+
info: `str` is not assignable to `bool`
36+
--> /src/mdtest_snippet.py:2:9
37+
|
38+
1 | class NotBoolable:
39+
2 | def __bool__(self) -> str:
40+
| -------- ^^^ Incorrect return type
41+
| |
42+
| Method defined here
43+
3 | return "wat"
44+
|
3545
3646
```

crates/red_knot_python_semantic/resources/mdtest/snapshots/unsupported_bool_conversion.md_-_Different_ways_that_`UNSUPPORTED_BOOL_CONVERSION`_can_occur_-_Has_a_`__bool__`_method,_but_has_incorrect_parameters.snap

+11-1
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,22 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/diagnostics/unsupp
2525
# Diagnostics
2626

2727
```
28-
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`; it incorrectly implements `__bool__`
28+
error: lint:unsupported-bool-conversion: Boolean conversion is unsupported for type `NotBoolable`
2929
--> /src/mdtest_snippet.py:8:8
3030
|
3131
7 | # error: [unsupported-bool-conversion]
3232
8 | 10 and a and True
3333
| ^
3434
|
35+
info: `__bool__` methods must only have a `self` parameter
36+
--> /src/mdtest_snippet.py:2:9
37+
|
38+
1 | class NotBoolable:
39+
2 | def __bool__(self, foo):
40+
| --------^^^^^^^^^^^ Incorrect parameters
41+
| |
42+
| Method defined here
43+
3 | return False
44+
|
3545
3646
```

crates/red_knot_python_semantic/resources/mdtest/type_api.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ class InvalidBoolDunder:
235235
def __bool__(self) -> int:
236236
return 1
237237

238-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `InvalidBoolDunder`; the return type of its bool method (`int`) isn't assignable to `bool"
238+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `InvalidBoolDunder`"
239239
static_assert(InvalidBoolDunder())
240240
```
241241

crates/red_knot_python_semantic/resources/mdtest/unary/not.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ class MethodBoolInvalid:
187187
def __bool__(self) -> int:
188188
return 0
189189

190-
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `MethodBoolInvalid`; the return type of its bool method (`int`) isn't assignable to `bool"
190+
# error: [unsupported-bool-conversion] "Boolean conversion is unsupported for type `MethodBoolInvalid`"
191191
# revealed: bool
192192
reveal_type(not MethodBoolInvalid())
193193

crates/red_knot_python_semantic/src/types.rs

+55-12
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ use diagnostic::{
1010
CALL_POSSIBLY_UNBOUND_METHOD, INVALID_CONTEXT_MANAGER, INVALID_SUPER_ARGUMENT, NOT_ITERABLE,
1111
UNAVAILABLE_IMPLICIT_SUPER_ARGUMENTS,
1212
};
13-
use ruff_db::diagnostic::create_semantic_syntax_diagnostic;
13+
use ruff_db::diagnostic::{
14+
create_semantic_syntax_diagnostic, Annotation, Severity, Span, SubDiagnostic,
15+
};
1416
use ruff_db::files::{File, FileRange};
1517
use ruff_python_ast::name::Name;
1618
use ruff_python_ast::{self as ast, AnyNodeRef};
@@ -5763,30 +5765,71 @@ impl<'db> BoolError<'db> {
57635765
Self::IncorrectArguments {
57645766
not_boolable_type, ..
57655767
} => {
5766-
builder.into_diagnostic(format_args!(
5767-
"Boolean conversion is unsupported for type `{}`; \
5768-
it incorrectly implements `__bool__`",
5768+
let mut diag = builder.into_diagnostic(format_args!(
5769+
"Boolean conversion is unsupported for type `{}`",
57695770
not_boolable_type.display(context.db())
57705771
));
5772+
let mut sub = SubDiagnostic::new(
5773+
Severity::Info,
5774+
"`__bool__` methods must only have a `self` parameter",
5775+
);
5776+
if let Some((func_span, parameter_span)) = not_boolable_type
5777+
.member(context.db(), "__bool__")
5778+
.into_lookup_result()
5779+
.ok()
5780+
.and_then(|quals| quals.inner_type().parameter_span(context.db(), None))
5781+
{
5782+
sub.annotate(
5783+
Annotation::primary(parameter_span).message("Incorrect parameters"),
5784+
);
5785+
sub.annotate(Annotation::secondary(func_span).message("Method defined here"));
5786+
}
5787+
diag.sub(sub);
57715788
}
57725789
Self::IncorrectReturnType {
57735790
not_boolable_type,
57745791
return_type,
57755792
} => {
5776-
builder.into_diagnostic(format_args!(
5777-
"Boolean conversion is unsupported for type `{not_boolable}`; \
5778-
the return type of its bool method (`{return_type}`) \
5779-
isn't assignable to `bool",
5793+
let mut diag = builder.into_diagnostic(format_args!(
5794+
"Boolean conversion is unsupported for type `{not_boolable}`",
57805795
not_boolable = not_boolable_type.display(context.db()),
5781-
return_type = return_type.display(context.db())
57825796
));
5797+
let mut sub = SubDiagnostic::new(
5798+
Severity::Info,
5799+
format_args!(
5800+
"`{return_type}` is not assignable to `bool`",
5801+
return_type = return_type.display(context.db()),
5802+
),
5803+
);
5804+
if let Some((func_span, return_type_span)) = not_boolable_type
5805+
.member(context.db(), "__bool__")
5806+
.into_lookup_result()
5807+
.ok()
5808+
.and_then(|quals| quals.inner_type().return_type_span(context.db()))
5809+
{
5810+
sub.annotate(
5811+
Annotation::primary(return_type_span).message("Incorrect return type"),
5812+
);
5813+
sub.annotate(Annotation::secondary(func_span).message("Method defined here"));
5814+
}
5815+
diag.sub(sub);
57835816
}
57845817
Self::NotCallable { not_boolable_type } => {
5785-
builder.into_diagnostic(format_args!(
5786-
"Boolean conversion is unsupported for type `{}`; \
5787-
its `__bool__` method isn't callable",
5818+
let mut diag = builder.into_diagnostic(format_args!(
5819+
"Boolean conversion is unsupported for type `{}`",
57885820
not_boolable_type.display(context.db())
57895821
));
5822+
let sub = SubDiagnostic::new(
5823+
Severity::Info,
5824+
format_args!(
5825+
"`__bool__` on `{}` must be callable",
5826+
not_boolable_type.display(context.db())
5827+
),
5828+
);
5829+
// TODO: It would be nice to create an annotation here for
5830+
// where `__bool__` is defined. At time of writing, I couldn't
5831+
// figure out a straight-forward way of doing this. ---AG
5832+
diag.sub(sub);
57905833
}
57915834
Self::Union { union, .. } => {
57925835
let first_error = union

0 commit comments

Comments
 (0)