mirror of
https://github.com/astral-sh/ruff.git
synced 2026-05-06 08:56:57 -04:00
[ty] Emit more specific diagnostics for "possibly unbound" errors from context manager dunder methods invoked on a union. (#24662)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? - Does this PR follow our AI policy (https://github.com/astral-sh/.github/blob/main/AI_POLICY.md)? --> ## Summary As part of https://github.com/astral-sh/ty/issues/940, this helps us emit more specific diagnostics for possibly unbound context manager dunders (e.g., `__enter__`, `__exit__`) invoked on a union type. Where previously the following snippet would produce just the top-level diagnostic commented below: ```py class Context: def __enter__(self): ... def __exit__(self, *args): ... class NotContext: pass def _(x: Context | NotContext): # error: [invalid-context-manager] "Object of type `Context | NotContext` cannot be used with `with` because the methods `__enter__` and `__exit__` are possibly unbound" with x: pass ``` We will now produce two further "info" sub-diagnostics: ``` info: `NotContext` does not implement `__enter__` info: `NotContext` does not implement `__exit__` ``` ## Approach - This implements the approach suggested by `@carljm` [here](https://github.com/astral-sh/ruff/pull/20199#pullrequestreview-3178490835) from a previous attempt to address https://github.com/astral-sh/ty/issues/940; it extends `CallDunderError::PossiblyUnbound` with a new `unbound_on` field that stores a list of the union members on which a particular dunder is unbound. We create the new, richer error with a new `UnionType.try_call_dunder_with_policy` method that looks up the dunder on each member of the union, and then aggregates the results. This is supersedes the previous `UnionType.map_with_boundness_and_qualifiers` approach, and allows us to preserve the per-member binding information that we use to produce the more detailed diagnostic. - There are two alternatives to this approach that I considered but rejected: - Rebuild the specific union member diagnositic information at each callsite, and only when relevant. This was the approach originally taken by https://github.com/astral-sh/ruff/pull/20199, but I think it will lead to some unnecessary code duplication across callsites (of which there are at least three more). - Refactor such that `UnionType.map_with_boundness_and_qualifiers` such that it no longer loses member-specific binding information when producing its result. This would have required an extension to `PlaceAndQualifiers`, which would have a large blast radius and also introduce overhead in several cases where member-specific information for unions is not necessary. - There are more implicit dunder calls that can benefit from the new shape of `CallDunderError::PossiblyUnbound`, but I have intentionally deferred those to [a follow-up](https://github.com/astral-sh/ruff/pull/24676) in order to first collect feedback on a more targeted changeset. - The first three commits in this PR (926bcec8d8,65dc3fbd5e,988e81d027) are "prefactors" that do not change any observable behaviour. The fourth (2422844d9a281240aad89148e9052e5e1208fbdf) actually implements the improvement, and deserves the most scrutiny. ## Test Plan Please see updated mdtests and associated snapshots.
This commit is contained in:
@@ -351,6 +351,34 @@ def _(flag: bool):
|
||||
reveal_type(x) # revealed: int
|
||||
```
|
||||
|
||||
## Union type as iterable where one union element has a non-callable `__iter__`
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
When one union element has a callable `__iter__` and another has a non-callable `__iter__`
|
||||
attribute, the error should be "may not be iterable" (hedged), not "is not iterable" (definitive) —
|
||||
because at runtime the value might be the iterable variant.
|
||||
|
||||
```py
|
||||
class TestIter:
|
||||
def __next__(self) -> int:
|
||||
return 42
|
||||
|
||||
class Test:
|
||||
def __iter__(self) -> TestIter:
|
||||
return TestIter()
|
||||
|
||||
class NotIter:
|
||||
# `__iter__` is present but not callable
|
||||
__iter__: int = 32
|
||||
|
||||
def _(flag: bool):
|
||||
iterable = Test() if flag else NotIter()
|
||||
# error: [not-iterable]
|
||||
for x in iterable:
|
||||
reveal_type(x) # revealed: int | Unknown
|
||||
```
|
||||
|
||||
## Union type as iterator where one union element has no `__next__` method
|
||||
|
||||
```py
|
||||
|
||||
+42
@@ -0,0 +1,42 @@
|
||||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
|
||||
---
|
||||
mdtest name: async.md - Async with statements - Context expression with possibly-unbound union variants
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/with/async.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | async def _(flag: bool):
|
||||
2 | class Manager1:
|
||||
3 | def __aenter__(self) -> str:
|
||||
4 | return "foo"
|
||||
5 |
|
||||
6 | def __aexit__(self, exc_type, exc_value, traceback): ...
|
||||
7 |
|
||||
8 | class NotAContextManager: ...
|
||||
9 | context_expr = Manager1() if flag else NotAContextManager()
|
||||
10 |
|
||||
11 | # error: [invalid-context-manager] "Object of type `Manager1 | NotAContextManager` cannot be used with `async with` because the methods `__aenter__` and `__aexit__` are possibly missing"
|
||||
12 | async with context_expr as f:
|
||||
13 | reveal_type(f) # revealed: str
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error[invalid-context-manager]: Object of type `Manager1 | NotAContextManager` cannot be used with `async with` because the methods `__aenter__` and `__aexit__` are possibly missing
|
||||
--> src/mdtest_snippet.py:12:16
|
||||
|
|
||||
12 | async with context_expr as f:
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
info: `NotAContextManager` does not implement `__aenter__` or `__aexit__`
|
||||
|
||||
```
|
||||
+46
@@ -0,0 +1,46 @@
|
||||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
|
||||
---
|
||||
mdtest name: for.md - For loops - Union type as iterable where one union element has a non-callable `__iter__`
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/loops/for.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | class TestIter:
|
||||
2 | def __next__(self) -> int:
|
||||
3 | return 42
|
||||
4 |
|
||||
5 | class Test:
|
||||
6 | def __iter__(self) -> TestIter:
|
||||
7 | return TestIter()
|
||||
8 |
|
||||
9 | class NotIter:
|
||||
10 | # `__iter__` is present but not callable
|
||||
11 | __iter__: int = 32
|
||||
12 |
|
||||
13 | def _(flag: bool):
|
||||
14 | iterable = Test() if flag else NotIter()
|
||||
15 | # error: [not-iterable]
|
||||
16 | for x in iterable:
|
||||
17 | reveal_type(x) # revealed: int | Unknown
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error[not-iterable]: Object of type `Test | NotIter` may not be iterable
|
||||
--> src/mdtest_snippet.py:16:14
|
||||
|
|
||||
16 | for x in iterable:
|
||||
| ^^^^^^^^
|
||||
|
|
||||
info: Its `__iter__` attribute (with type `(bound method Test.__iter__() -> TestIter) | int`) may not be callable
|
||||
|
||||
```
|
||||
+47
@@ -0,0 +1,47 @@
|
||||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
|
||||
---
|
||||
mdtest name: sync.md - With statements - Context expression where one union variant has a non-callable dunder
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/with/sync.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | def _(flag: bool):
|
||||
2 | class GoodManager:
|
||||
3 | def __enter__(self) -> str:
|
||||
4 | return "foo"
|
||||
5 |
|
||||
6 | def __exit__(self, exc_type, exc_value, traceback): ...
|
||||
7 |
|
||||
8 | class BadManager:
|
||||
9 | def __enter__(self) -> str:
|
||||
10 | return "bar"
|
||||
11 |
|
||||
12 | # `__exit__` is present but not callable
|
||||
13 | __exit__: int = 32
|
||||
14 |
|
||||
15 | context_expr = GoodManager() if flag else BadManager()
|
||||
16 |
|
||||
17 | # error: [invalid-context-manager] "Object of type `GoodManager | BadManager` cannot be used with `with` because it does not correctly implement `__exit__`"
|
||||
18 | with context_expr as f:
|
||||
19 | reveal_type(f) # revealed: str
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error[invalid-context-manager]: Object of type `GoodManager | BadManager` cannot be used with `with` because it does not correctly implement `__exit__`
|
||||
--> src/mdtest_snippet.py:18:10
|
||||
|
|
||||
18 | with context_expr as f:
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
|
||||
```
|
||||
+47
@@ -0,0 +1,47 @@
|
||||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
|
||||
---
|
||||
mdtest name: sync.md - With statements - Context expression with overlapping possibly-unbound union variants
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/with/sync.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | def _(flag1: bool, flag2: bool):
|
||||
2 | class GoodManager:
|
||||
3 | def __enter__(self) -> str:
|
||||
4 | return "foo"
|
||||
5 |
|
||||
6 | def __exit__(self, exc_type, exc_value, traceback): ...
|
||||
7 |
|
||||
8 | class MissingExitManager:
|
||||
9 | def __enter__(self) -> str:
|
||||
10 | return "bar"
|
||||
11 |
|
||||
12 | class NotAContextManager: ...
|
||||
13 | context_expr = GoodManager() if flag1 else MissingExitManager() if flag2 else NotAContextManager()
|
||||
14 |
|
||||
15 | # error: [invalid-context-manager] "Object of type `GoodManager | MissingExitManager | NotAContextManager` cannot be used with `with` because the methods `__enter__` and `__exit__` are possibly missing"
|
||||
16 | with context_expr as f:
|
||||
17 | reveal_type(f) # revealed: str
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error[invalid-context-manager]: Object of type `GoodManager | MissingExitManager | NotAContextManager` cannot be used with `with` because the methods `__enter__` and `__exit__` are possibly missing
|
||||
--> src/mdtest_snippet.py:16:10
|
||||
|
|
||||
16 | with context_expr as f:
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
info: `NotAContextManager` does not implement `__enter__` or `__exit__`
|
||||
info: `MissingExitManager` does not implement `__exit__`
|
||||
|
||||
```
|
||||
+42
@@ -0,0 +1,42 @@
|
||||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
|
||||
---
|
||||
mdtest name: sync.md - With statements - Context expression with possibly-unbound union variants
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/with/sync.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | def _(flag: bool):
|
||||
2 | class Manager1:
|
||||
3 | def __enter__(self) -> str:
|
||||
4 | return "foo"
|
||||
5 |
|
||||
6 | def __exit__(self, exc_type, exc_value, traceback): ...
|
||||
7 |
|
||||
8 | class NotAContextManager: ...
|
||||
9 | context_expr = Manager1() if flag else NotAContextManager()
|
||||
10 |
|
||||
11 | # error: [invalid-context-manager] "Object of type `Manager1 | NotAContextManager` cannot be used with `with` because the methods `__enter__` and `__exit__` are possibly missing"
|
||||
12 | with context_expr as f:
|
||||
13 | reveal_type(f) # revealed: str
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error[invalid-context-manager]: Object of type `Manager1 | NotAContextManager` cannot be used with `with` because the methods `__enter__` and `__exit__` are possibly missing
|
||||
--> src/mdtest_snippet.py:12:10
|
||||
|
|
||||
12 | with context_expr as f:
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
info: `NotAContextManager` does not implement `__enter__` or `__exit__`
|
||||
|
||||
```
|
||||
@@ -102,6 +102,8 @@ async def main():
|
||||
|
||||
## Context expression with possibly-unbound union variants
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
```py
|
||||
async def _(flag: bool):
|
||||
class Manager1:
|
||||
|
||||
@@ -142,6 +142,8 @@ with Manager():
|
||||
|
||||
## Context expression with possibly-unbound union variants
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
```py
|
||||
def _(flag: bool):
|
||||
class Manager1:
|
||||
@@ -158,6 +160,60 @@ def _(flag: bool):
|
||||
reveal_type(f) # revealed: str
|
||||
```
|
||||
|
||||
## Context expression with overlapping possibly-unbound union variants
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
```py
|
||||
def _(flag1: bool, flag2: bool):
|
||||
class GoodManager:
|
||||
def __enter__(self) -> str:
|
||||
return "foo"
|
||||
|
||||
def __exit__(self, exc_type, exc_value, traceback): ...
|
||||
|
||||
class MissingExitManager:
|
||||
def __enter__(self) -> str:
|
||||
return "bar"
|
||||
|
||||
class NotAContextManager: ...
|
||||
context_expr = GoodManager() if flag1 else MissingExitManager() if flag2 else NotAContextManager()
|
||||
|
||||
# error: [invalid-context-manager] "Object of type `GoodManager | MissingExitManager | NotAContextManager` cannot be used with `with` because the methods `__enter__` and `__exit__` are possibly missing"
|
||||
with context_expr as f:
|
||||
reveal_type(f) # revealed: str
|
||||
```
|
||||
|
||||
## Context expression where one union variant has a non-callable dunder
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
If every union element implements the context manager protocol but at least one implements it
|
||||
incorrectly (e.g. with a non-callable `__exit__` attribute), the diagnostic should reflect that —
|
||||
*not* report the dunder as "possibly missing".
|
||||
|
||||
```py
|
||||
def _(flag: bool):
|
||||
class GoodManager:
|
||||
def __enter__(self) -> str:
|
||||
return "foo"
|
||||
|
||||
def __exit__(self, exc_type, exc_value, traceback): ...
|
||||
|
||||
class BadManager:
|
||||
def __enter__(self) -> str:
|
||||
return "bar"
|
||||
|
||||
# `__exit__` is present but not callable
|
||||
__exit__: int = 32
|
||||
|
||||
context_expr = GoodManager() if flag else BadManager()
|
||||
|
||||
# error: [invalid-context-manager] "Object of type `GoodManager | BadManager` cannot be used with `with` because it does not correctly implement `__exit__`"
|
||||
with context_expr as f:
|
||||
reveal_type(f) # revealed: str
|
||||
```
|
||||
|
||||
## Context expression with "sometimes" callable `__enter__` method
|
||||
|
||||
```py
|
||||
|
||||
@@ -3654,7 +3654,7 @@ impl<'db> Type<'db> {
|
||||
TypeContext::default(),
|
||||
) {
|
||||
Ok(bindings) => bindings.return_type(db),
|
||||
Err(CallDunderError::PossiblyUnbound(bindings)) => bindings.return_type(db),
|
||||
Err(CallDunderError::PossiblyUnbound { bindings, .. }) => bindings.return_type(db),
|
||||
|
||||
// TODO: emit a diagnostic
|
||||
Err(CallDunderError::MethodNotAvailable) => return None,
|
||||
@@ -4774,36 +4774,12 @@ impl<'db> Type<'db> {
|
||||
tcx: TypeContext<'db>,
|
||||
policy: MemberLookupPolicy,
|
||||
) -> Result<Bindings<'db>, CallDunderError<'db>> {
|
||||
// For intersection types, call the dunder on each element separately and combine
|
||||
// the results. This avoids intersecting bound methods (which often collapses to Never)
|
||||
// and instead intersects the return types.
|
||||
//
|
||||
// TODO: we might be able to remove this after fixing
|
||||
// https://github.com/astral-sh/ty/issues/2428.
|
||||
if let Type::Intersection(intersection) = self {
|
||||
// Using `positive()` rather than `positive_elements_or_object()` is safe
|
||||
// here because `object` does not define any of the dunders that are called
|
||||
// through this path without `MRO_NO_OBJECT_FALLBACK` (e.g. `__await__`,
|
||||
// `__iter__`, `__enter__`, `__bool__`).
|
||||
let positive = intersection.positive(db);
|
||||
return intersection.try_call_dunder_with_policy(db, name, argument_types, tcx, policy);
|
||||
}
|
||||
|
||||
let mut successful_bindings = Vec::with_capacity(positive.len());
|
||||
let mut last_error = None;
|
||||
|
||||
for element in positive {
|
||||
match element.try_call_dunder_with_policy(db, name, argument_types, tcx, policy) {
|
||||
Ok(bindings) => successful_bindings.push(bindings),
|
||||
Err(err) => last_error = Some(err),
|
||||
}
|
||||
}
|
||||
|
||||
if successful_bindings.is_empty() {
|
||||
// TODO we are only showing one of the errors here; should we aggregate them
|
||||
// somehow or show all of them?
|
||||
return Err(last_error.unwrap_or(CallDunderError::MethodNotAvailable));
|
||||
}
|
||||
|
||||
return Ok(Bindings::from_intersection(self, successful_bindings));
|
||||
if let Type::Union(union) = self {
|
||||
return union.try_call_dunder_with_policy(db, name, argument_types, tcx, policy);
|
||||
}
|
||||
|
||||
// Implicit calls to dunder methods never access instance members, so we pass
|
||||
@@ -4828,7 +4804,10 @@ impl<'db> Type<'db> {
|
||||
.check_types(db, &constraints, argument_types, tcx, &[])?;
|
||||
|
||||
if boundness == Definedness::PossiblyUndefined {
|
||||
return Err(CallDunderError::PossiblyUnbound(Box::new(bindings)));
|
||||
return Err(CallDunderError::PossiblyUnbound {
|
||||
bindings: Box::new(bindings),
|
||||
unbound_on: None,
|
||||
});
|
||||
}
|
||||
Ok(bindings)
|
||||
}
|
||||
@@ -4863,7 +4842,10 @@ impl<'db> Type<'db> {
|
||||
.check_types(db, &constraints, argument_types, tcx, &[])?;
|
||||
|
||||
if boundness == Definedness::PossiblyUndefined {
|
||||
return Err(CallDunderError::PossiblyUnbound(Box::new(bindings)));
|
||||
return Err(CallDunderError::PossiblyUnbound {
|
||||
bindings: Box::new(bindings),
|
||||
unbound_on: None,
|
||||
});
|
||||
}
|
||||
Ok(bindings)
|
||||
}
|
||||
@@ -6380,6 +6362,121 @@ impl<'db> Type<'db> {
|
||||
}
|
||||
}
|
||||
|
||||
impl<'db> IntersectionType<'db> {
|
||||
// Calls the dunder on each element separately and combines the results.
|
||||
// This avoids intersecting bound methods (which often collapses to Never)
|
||||
// and instead intersects the return types.
|
||||
//
|
||||
// TODO: we might be able to remove this after fixing
|
||||
// https://github.com/astral-sh/ty/issues/2428.
|
||||
fn try_call_dunder_with_policy(
|
||||
self,
|
||||
db: &'db dyn Db,
|
||||
name: &str,
|
||||
argument_types: &mut CallArguments<'_, 'db>,
|
||||
tcx: TypeContext<'db>,
|
||||
policy: MemberLookupPolicy,
|
||||
) -> Result<Bindings<'db>, CallDunderError<'db>> {
|
||||
// Using `positive()` rather than `positive_elements_or_object()` is safe
|
||||
// here because `object` does not define any of the dunders that are called
|
||||
// through this path without `MRO_NO_OBJECT_FALLBACK` (e.g. `__await__`,
|
||||
// `__iter__`, `__enter__`, `__bool__`).
|
||||
let positive = self.positive(db);
|
||||
let mut successful_bindings = Vec::with_capacity(positive.len());
|
||||
let mut last_error = None;
|
||||
|
||||
for element in positive {
|
||||
match element.try_call_dunder_with_policy(db, name, argument_types, tcx, policy) {
|
||||
Ok(bindings) => successful_bindings.push(bindings),
|
||||
Err(err) => last_error = Some(err),
|
||||
}
|
||||
}
|
||||
|
||||
if successful_bindings.is_empty() {
|
||||
// TODO we are only showing one of the errors here; should we aggregate
|
||||
// them somehow or show all of them?
|
||||
return Err(last_error.unwrap_or(CallDunderError::MethodNotAvailable));
|
||||
}
|
||||
|
||||
Ok(Bindings::from_intersection(
|
||||
Type::Intersection(self),
|
||||
successful_bindings,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
impl<'db> UnionType<'db> {
|
||||
// Performs a lookup for the dunder on each union member separately, then
|
||||
// aggregates the results.
|
||||
//
|
||||
// This alternative to aggregating the dunder lookups with
|
||||
// `UnionType.map_with_boundness_and_qualifiers` preserves the information
|
||||
// necessary to emit more precise diagnostics for "possibly unbound" errors.
|
||||
fn try_call_dunder_with_policy(
|
||||
self,
|
||||
db: &'db dyn Db,
|
||||
name: &str,
|
||||
argument_types: &mut CallArguments<'_, 'db>,
|
||||
tcx: TypeContext<'db>,
|
||||
policy: MemberLookupPolicy,
|
||||
) -> Result<Bindings<'db>, CallDunderError<'db>> {
|
||||
let elements = self.elements(db);
|
||||
let mut builder = UnionBuilder::new(db);
|
||||
let mut unbound_on: Vec<Type<'db>> = Vec::new();
|
||||
let mut any_defined = false;
|
||||
let mut possibly_undefined = false;
|
||||
|
||||
for element in elements {
|
||||
match element
|
||||
.member_lookup_with_policy(
|
||||
db,
|
||||
name.into(),
|
||||
policy | MemberLookupPolicy::NO_INSTANCE_FALLBACK,
|
||||
)
|
||||
.place
|
||||
{
|
||||
Place::Defined(DefinedPlace {
|
||||
ty,
|
||||
definedness: Definedness::PossiblyUndefined,
|
||||
..
|
||||
}) => {
|
||||
builder = builder.add(ty);
|
||||
any_defined = true;
|
||||
possibly_undefined = true;
|
||||
}
|
||||
Place::Defined(DefinedPlace { ty, .. }) => {
|
||||
builder = builder.add(ty);
|
||||
any_defined = true;
|
||||
}
|
||||
Place::Undefined => {
|
||||
unbound_on.push(*element);
|
||||
possibly_undefined = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if !any_defined {
|
||||
return Err(CallDunderError::MethodNotAvailable);
|
||||
}
|
||||
|
||||
let dunder_callable = builder.build();
|
||||
let constraints = ConstraintSetBuilder::new();
|
||||
let bindings = dunder_callable
|
||||
.bindings(db)
|
||||
.match_parameters(db, argument_types)
|
||||
.check_types(db, &constraints, argument_types, tcx, &[])?;
|
||||
|
||||
if possibly_undefined {
|
||||
return Err(CallDunderError::PossiblyUnbound {
|
||||
bindings: Box::new(bindings),
|
||||
unbound_on: (!unbound_on.is_empty()).then(|| unbound_on.into_boxed_slice()),
|
||||
});
|
||||
}
|
||||
|
||||
Ok(bindings)
|
||||
}
|
||||
}
|
||||
|
||||
impl<'db> From<&Type<'db>> for Type<'db> {
|
||||
fn from(value: &Type<'db>) -> Self {
|
||||
*value
|
||||
@@ -7305,7 +7402,7 @@ impl<'db> AwaitError<'db> {
|
||||
);
|
||||
}
|
||||
}
|
||||
Self::Call(CallDunderError::PossiblyUnbound(bindings)) => {
|
||||
Self::Call(CallDunderError::PossiblyUnbound { bindings, .. }) => {
|
||||
diag.info("`__await__` may be missing");
|
||||
if let Some(definition_spans) = bindings.callable_type().function_spans(db) {
|
||||
diag.annotate(
|
||||
|
||||
@@ -80,7 +80,9 @@ impl<'db> Type<'db> {
|
||||
Ok(type_to_truthiness(return_type))
|
||||
}
|
||||
|
||||
Err(CallDunderError::PossiblyUnbound(outcome)) => {
|
||||
Err(CallDunderError::PossiblyUnbound {
|
||||
bindings: outcome, ..
|
||||
}) => {
|
||||
let return_type = outcome.return_type(db);
|
||||
if !return_type.is_assignable_to(db, KnownClass::Bool.to_instance(db)) {
|
||||
// The type has a `__bool__` method, but it doesn't return a
|
||||
|
||||
@@ -196,7 +196,15 @@ pub(super) enum CallDunderError<'db> {
|
||||
/// The type has the specified dunder method and it is callable
|
||||
/// with the specified arguments without any binding errors
|
||||
/// but it is possibly unbound.
|
||||
PossiblyUnbound(Box<Bindings<'db>>),
|
||||
PossiblyUnbound {
|
||||
// Describes the places where the dunder was indeed defined.
|
||||
bindings: Box<Bindings<'db>>,
|
||||
|
||||
// Lists the types on which the dunder was undefined (e.g., the specific
|
||||
// members of a union on which the dunder was missing). `None` means
|
||||
// that the call path does not track where the dunder may be unbound.
|
||||
unbound_on: Option<Box<[Type<'db>]>>,
|
||||
},
|
||||
|
||||
/// The dunder method with the specified name is missing.
|
||||
MethodNotAvailable,
|
||||
@@ -207,7 +215,7 @@ impl<'db> CallDunderError<'db> {
|
||||
match self {
|
||||
Self::MethodNotAvailable | Self::CallError(CallErrorKind::NotCallable, _) => None,
|
||||
Self::CallError(_, bindings) => Some(bindings.return_type(db)),
|
||||
Self::PossiblyUnbound(bindings) => Some(bindings.return_type(db)),
|
||||
Self::PossiblyUnbound { bindings, .. } => Some(bindings.return_type(db)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -236,7 +244,7 @@ impl From<CallDunderError<'_>> for CallBinOpError {
|
||||
fn from(value: CallDunderError<'_>) -> Self {
|
||||
match value {
|
||||
CallDunderError::CallError(_, _) => Self::CallError,
|
||||
CallDunderError::MethodNotAvailable | CallDunderError::PossiblyUnbound(_) => {
|
||||
CallDunderError::MethodNotAvailable | CallDunderError::PossiblyUnbound { .. } => {
|
||||
CallBinOpError::NotSupported
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
use crate::{
|
||||
Db,
|
||||
Db, FxOrderSet,
|
||||
types::{
|
||||
CallArguments, CallDunderError, Type, TypeContext, call::CallErrorKind,
|
||||
context::InferContext, diagnostic::INVALID_CONTEXT_MANAGER,
|
||||
@@ -127,9 +127,7 @@ impl<'db> ContextManagerError<'db> {
|
||||
exit_error: _,
|
||||
mode: _,
|
||||
} => match enter_error {
|
||||
CallDunderError::PossiblyUnbound(call_outcome) => {
|
||||
Some(call_outcome.return_type(db))
|
||||
}
|
||||
CallDunderError::PossiblyUnbound { bindings, .. } => Some(bindings.return_type(db)),
|
||||
CallDunderError::CallError(CallErrorKind::NotCallable, _) => None,
|
||||
CallDunderError::CallError(_, bindings) => Some(bindings.return_type(db)),
|
||||
CallDunderError::MethodNotAvailable => None,
|
||||
@@ -143,6 +141,16 @@ impl<'db> ContextManagerError<'db> {
|
||||
context_expression_type: Type<'db>,
|
||||
context_expression_node: ast::AnyNodeRef,
|
||||
) {
|
||||
fn unbound_on<'db>(error: &CallDunderError<'db>) -> FxOrderSet<Type<'db>> {
|
||||
match error {
|
||||
CallDunderError::PossiblyUnbound {
|
||||
unbound_on: Some(unbound_on),
|
||||
..
|
||||
} => unbound_on.iter().copied().collect(),
|
||||
_ => FxOrderSet::default(),
|
||||
}
|
||||
}
|
||||
|
||||
let Some(builder) = context.report_lint(&INVALID_CONTEXT_MANAGER, context_expression_node)
|
||||
else {
|
||||
return;
|
||||
@@ -162,12 +170,11 @@ impl<'db> ContextManagerError<'db> {
|
||||
let format_call_dunder_error = |call_dunder_error: &CallDunderError<'db>, name: &str| {
|
||||
match call_dunder_error {
|
||||
CallDunderError::MethodNotAvailable => format!("it does not implement `{name}`"),
|
||||
CallDunderError::PossiblyUnbound(_) => {
|
||||
CallDunderError::PossiblyUnbound { .. } => {
|
||||
format!("the method `{name}` may be missing")
|
||||
}
|
||||
// TODO: Use more specific error messages for the different error cases.
|
||||
// E.g. hint toward the union variant that doesn't correctly implement enter,
|
||||
// distinguish between a not callable `__enter__` attribute and a wrong signature.
|
||||
// E.g. distinguish between a not callable `__enter__` attribute and a wrong signature.
|
||||
CallDunderError::CallError(_, _) => {
|
||||
format!("it does not correctly implement `{name}`")
|
||||
}
|
||||
@@ -179,9 +186,10 @@ impl<'db> ContextManagerError<'db> {
|
||||
error_b: &CallDunderError<'db>,
|
||||
name_b: &str| {
|
||||
match (error_a, error_b) {
|
||||
(CallDunderError::PossiblyUnbound(_), CallDunderError::PossiblyUnbound(_)) => {
|
||||
format!("the methods `{name_a}` and `{name_b}` are possibly missing")
|
||||
}
|
||||
(
|
||||
CallDunderError::PossiblyUnbound { .. },
|
||||
CallDunderError::PossiblyUnbound { .. },
|
||||
) => format!("the methods `{name_a}` and `{name_b}` are possibly missing"),
|
||||
(CallDunderError::MethodNotAvailable, CallDunderError::MethodNotAvailable) => {
|
||||
format!("it does not implement `{name_a}` and `{name_b}`")
|
||||
}
|
||||
@@ -226,6 +234,58 @@ impl<'db> ContextManagerError<'db> {
|
||||
formatted_errors,
|
||||
));
|
||||
|
||||
match self {
|
||||
Self::Exit { exit_error, .. } => {
|
||||
let exit_unbound_on = unbound_on(exit_error);
|
||||
for ty in &exit_unbound_on {
|
||||
diag.info(format_args!(
|
||||
"`{}` does not implement `{exit_method}`",
|
||||
ty.display(db)
|
||||
));
|
||||
}
|
||||
}
|
||||
Self::Enter(enter_error, _) => {
|
||||
let enter_unbound_on = unbound_on(enter_error);
|
||||
for ty in &enter_unbound_on {
|
||||
diag.info(format_args!(
|
||||
"`{}` does not implement `{enter_method}`",
|
||||
ty.display(db)
|
||||
));
|
||||
}
|
||||
}
|
||||
Self::EnterAndExit {
|
||||
enter_error,
|
||||
exit_error,
|
||||
..
|
||||
} => {
|
||||
let enter_unbound_on = unbound_on(enter_error);
|
||||
let exit_unbound_on = unbound_on(exit_error);
|
||||
|
||||
for ty in &enter_unbound_on {
|
||||
if exit_unbound_on.contains(ty) {
|
||||
diag.info(format_args!(
|
||||
"`{}` does not implement `{enter_method}` or `{exit_method}`",
|
||||
ty.display(db)
|
||||
));
|
||||
} else {
|
||||
diag.info(format_args!(
|
||||
"`{}` does not implement `{enter_method}`",
|
||||
ty.display(db)
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
for ty in &exit_unbound_on {
|
||||
if !enter_unbound_on.contains(ty) {
|
||||
diag.info(format_args!(
|
||||
"`{}` does not implement `{exit_method}`",
|
||||
ty.display(db)
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let (alt_mode, alt_enter_method, alt_exit_method, alt_with_kw) = match mode {
|
||||
EvaluationMode::Sync => ("async", "__aenter__", "__aexit__", "async with"),
|
||||
EvaluationMode::Async => ("sync", "__enter__", "__exit__", "with"),
|
||||
|
||||
@@ -5053,7 +5053,7 @@ pub(crate) fn report_invalid_or_unsupported_base(
|
||||
|
||||
match mro_entries_call_error {
|
||||
CallDunderError::MethodNotAvailable => {}
|
||||
CallDunderError::PossiblyUnbound(_) => {
|
||||
CallDunderError::PossiblyUnbound { .. } => {
|
||||
explain_mro_entries(&mut diagnostic);
|
||||
diagnostic.info(format_args!(
|
||||
"Type `{}` may have an `__mro_entries__` attribute, but it may be missing",
|
||||
|
||||
@@ -1072,14 +1072,15 @@ pub fn definitions_for_unary_op<'db>(
|
||||
Ok(bindings) => bindings,
|
||||
Err(CallDunderError::MethodNotAvailable) => return None,
|
||||
Err(
|
||||
CallDunderError::PossiblyUnbound(bindings)
|
||||
CallDunderError::PossiblyUnbound { bindings, .. }
|
||||
| CallDunderError::CallError(_, bindings),
|
||||
) => *bindings,
|
||||
}
|
||||
}
|
||||
Err(CallDunderError::MethodNotAvailable) => return None,
|
||||
Err(
|
||||
CallDunderError::PossiblyUnbound(bindings) | CallDunderError::CallError(_, bindings),
|
||||
CallDunderError::PossiblyUnbound { bindings, .. }
|
||||
| CallDunderError::CallError(_, bindings),
|
||||
) => *bindings,
|
||||
};
|
||||
|
||||
|
||||
@@ -2506,7 +2506,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||
// above) as a fallback for dynamic attribute assignment.
|
||||
match setattr_dunder_call_result {
|
||||
// If __setattr__ succeeded, allow the assignment.
|
||||
Ok(_) | Err(CallDunderError::PossiblyUnbound(_)) => true,
|
||||
Ok(_) | Err(CallDunderError::PossiblyUnbound { .. }) => true,
|
||||
Err(CallDunderError::CallError(..)) => {
|
||||
if emit_diagnostics
|
||||
&& let Some(builder) =
|
||||
@@ -2879,7 +2879,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||
}
|
||||
|
||||
match delattr_dunder_call_result {
|
||||
Ok(_) | Err(CallDunderError::PossiblyUnbound(_)) => {
|
||||
Ok(_) | Err(CallDunderError::PossiblyUnbound { .. }) => {
|
||||
if self.validate_final_attribute_deletion(
|
||||
target,
|
||||
object_ty,
|
||||
@@ -2949,7 +2949,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||
}
|
||||
|
||||
match delete_dunder_call_result {
|
||||
Ok(_) | Err(CallDunderError::PossiblyUnbound(_)) => return true,
|
||||
Ok(_) | Err(CallDunderError::PossiblyUnbound { .. }) => return true,
|
||||
Err(CallDunderError::CallError(kind, bindings)) => {
|
||||
if emit_diagnostics {
|
||||
let failure = CallError(kind, bindings);
|
||||
@@ -4208,7 +4208,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||
let value_ty = infer_value_ty(self, TypeContext::default());
|
||||
binary_return_ty(self, value_ty)
|
||||
}
|
||||
Err(CallDunderError::PossiblyUnbound(outcome)) => {
|
||||
Err(CallDunderError::PossiblyUnbound {
|
||||
bindings: outcome, ..
|
||||
}) => {
|
||||
let value_ty = outcome.type_for_argument(&call_arguments, 0);
|
||||
UnionType::from_two_elements(
|
||||
db,
|
||||
@@ -4785,7 +4787,10 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||
}
|
||||
|
||||
if boundness == Definedness::PossiblyUndefined {
|
||||
return Err(CallDunderError::PossiblyUnbound(Box::new(bindings)));
|
||||
return Err(CallDunderError::PossiblyUnbound {
|
||||
bindings: Box::new(bindings),
|
||||
unbound_on: None,
|
||||
});
|
||||
}
|
||||
Ok(bindings)
|
||||
}
|
||||
|
||||
@@ -894,7 +894,7 @@ fn infer_membership_test_comparison<'db>(
|
||||
Ok(bindings) => Some(bindings.return_type(db)),
|
||||
// If `__contains__` is not available or possibly unbound,
|
||||
// fall back to iteration-based membership test.
|
||||
Err(CallDunderError::MethodNotAvailable | CallDunderError::PossiblyUnbound(_)) => right
|
||||
Err(CallDunderError::MethodNotAvailable | CallDunderError::PossiblyUnbound { .. }) => right
|
||||
.try_iterate(db)
|
||||
.map(|_| KnownClass::Bool.to_instance(db))
|
||||
.ok(),
|
||||
|
||||
@@ -283,7 +283,10 @@ impl<'db> Type<'db> {
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(CallDunderError::PossiblyUnbound(dunder_aiter_bindings)) => {
|
||||
Err(CallDunderError::PossiblyUnbound {
|
||||
bindings: dunder_aiter_bindings,
|
||||
..
|
||||
}) => {
|
||||
let iterator = dunder_aiter_bindings.return_type(db);
|
||||
match try_call_dunder_anext_on_iterator(iterator) {
|
||||
Ok(_) => Err(IterationError::IterCallError {
|
||||
@@ -361,7 +364,10 @@ impl<'db> Type<'db> {
|
||||
}
|
||||
|
||||
// `__iter__` is possibly unbound...
|
||||
Err(CallDunderError::PossiblyUnbound(dunder_iter_outcome)) => {
|
||||
Err(CallDunderError::PossiblyUnbound {
|
||||
bindings: dunder_iter_outcome,
|
||||
..
|
||||
}) => {
|
||||
let iterator = dunder_iter_outcome.return_type(db);
|
||||
|
||||
match try_call_dunder_next_on_iterator(iterator) {
|
||||
@@ -514,13 +520,14 @@ impl<'db> IterationError<'db> {
|
||||
dunder_getitem_error,
|
||||
} => match dunder_getitem_error {
|
||||
CallDunderError::MethodNotAvailable => Some(*dunder_next_return),
|
||||
CallDunderError::PossiblyUnbound(dunder_getitem_outcome) => {
|
||||
Some(UnionType::from_two_elements(
|
||||
db,
|
||||
*dunder_next_return,
|
||||
dunder_getitem_outcome.return_type(db),
|
||||
))
|
||||
}
|
||||
CallDunderError::PossiblyUnbound {
|
||||
bindings: dunder_getitem_outcome,
|
||||
..
|
||||
} => Some(UnionType::from_two_elements(
|
||||
db,
|
||||
*dunder_next_return,
|
||||
dunder_getitem_outcome.return_type(db),
|
||||
)),
|
||||
CallDunderError::CallError(CallErrorKind::NotCallable, _) => {
|
||||
Some(*dunder_next_return)
|
||||
}
|
||||
@@ -685,7 +692,7 @@ impl<'db> IterationError<'db> {
|
||||
iterator_type = iterator.display(db),
|
||||
));
|
||||
}
|
||||
CallDunderError::PossiblyUnbound(_) => {
|
||||
CallDunderError::PossiblyUnbound { .. } => {
|
||||
reporter.may_not(format_args!(
|
||||
"Its `{dunder_iter_name}` method returns an object of type `{iterator_type}`, \
|
||||
which may not have a `{dunder_next_name}` method",
|
||||
@@ -739,7 +746,7 @@ impl<'db> IterationError<'db> {
|
||||
and it doesn't have a `__getitem__` method",
|
||||
);
|
||||
}
|
||||
CallDunderError::PossiblyUnbound(_) => {
|
||||
CallDunderError::PossiblyUnbound { .. } => {
|
||||
reporter
|
||||
.may_not("It may not have an `__iter__` method or a `__getitem__` method");
|
||||
}
|
||||
@@ -805,7 +812,7 @@ impl<'db> IterationError<'db> {
|
||||
reporter
|
||||
.is_not("It doesn't have an `__iter__` method or a `__getitem__` method");
|
||||
}
|
||||
CallDunderError::PossiblyUnbound(_) => {
|
||||
CallDunderError::PossiblyUnbound { .. } => {
|
||||
reporter.is_not(
|
||||
"It has no `__iter__` method and it may not have a `__getitem__` method",
|
||||
);
|
||||
|
||||
@@ -799,7 +799,7 @@ impl<'db> Type<'db> {
|
||||
Ok(outcome) => {
|
||||
return Ok(outcome.return_type(db));
|
||||
}
|
||||
Err(CallDunderError::PossiblyUnbound(bindings)) => {
|
||||
Err(CallDunderError::PossiblyUnbound { bindings, .. }) => {
|
||||
return Err(SubscriptError::new(
|
||||
bindings.return_type(db),
|
||||
SubscriptErrorKind::DunderPossiblyUnbound {
|
||||
@@ -845,7 +845,7 @@ impl<'db> Type<'db> {
|
||||
Ok(bindings) => {
|
||||
return Ok(bindings.return_type(db));
|
||||
}
|
||||
Err(CallDunderError::PossiblyUnbound(bindings)) => {
|
||||
Err(CallDunderError::PossiblyUnbound { bindings, .. }) => {
|
||||
return Err(SubscriptError::new(
|
||||
bindings.return_type(db),
|
||||
SubscriptErrorKind::DunderPossiblyUnbound {
|
||||
|
||||
Reference in New Issue
Block a user