Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,20 @@ async def main() -> None:
await PossiblyUnbound() # error: [invalid-await]
```

## Union type where one member lacks `__await__`

```py
class Awaitable:
def __await__(self):
yield

class NotAwaitable: ...

async def _(flag: bool) -> None:
x = Awaitable() if flag else NotAwaitable()
await x # error: [invalid-await]
```

## `__await__` definition with extra arguments

Currently, the signature of `__await__` isn't checked for conformity with the `Awaitable` protocol
Expand Down
18 changes: 17 additions & 1 deletion crates/ty_python_semantic/resources/mdtest/mro.md
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ class Foo(EitherOr): ...
## `__bases__` is a union of a dynamic type and valid bases

If a dynamic type such as `Any` or `Unknown` is one of the elements in the union, and all other
types *would be* valid class bases, we do not emit an `invalid-base` or `unsupported-base`
types _would be_ valid class bases, we do not emit an `invalid-base` or `unsupported-base`
diagnostic, and we use the dynamic type as a base to prevent further downstream errors.

```py
Expand Down Expand Up @@ -457,6 +457,22 @@ class BadSub1(Bad1()): ... # error: [invalid-base]
class BadSub2(Bad2()): ... # error: [invalid-base]
```

For a union base where one member lacks `__mro_entries__`, `invalid-base` should be emitted with a
sub-diagnostic identifying the problematic union member:

```py
def _(flag: bool):
class HasMroEntries:
def __mro_entries__(self, bases: tuple[type, ...]) -> tuple[type, ...]:
return ()

class NoMroEntries: ...

base = HasMroEntries() if flag else NoMroEntries()

class Foo(base): ... # error: [invalid-base]
```

## `__bases__` lists with duplicate bases

<!-- snapshot-diagnostics -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,6 @@ error[not-iterable]: Object of type `Test | Literal[42]` may not be iterable
| ^^^^^^^^^^^^^^^^^^^^^^
|
info: It may not have an `__iter__` method and it doesn't have a `__getitem__` method
info: `Literal[42]` does not implement `__iter__`

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
source: crates/ty_test/src/lib.rs
expression: snapshot
---

---
mdtest name: invalid_await.md - Invalid await diagnostics - Union type where one member lacks `__await__`
mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_await.md
---

# Python source files

## mdtest_snippet.py

```
1 | class Awaitable:
2 | def __await__(self):
3 | yield
4 |
5 | class NotAwaitable: ...
6 |
7 | async def _(flag: bool) -> None:
8 | x = Awaitable() if flag else NotAwaitable()
9 | await x # error: [invalid-await]
```

# Diagnostics

```
error[invalid-await]: `Awaitable | NotAwaitable` is not awaitable
--> src/mdtest_snippet.py:9:11
|
9 | await x # error: [invalid-await]
| ^
|
::: src/mdtest_snippet.py:2:9
|
2 | def __await__(self):
| --------------- method defined here
|
info: `__await__` may be missing
info: `NotAwaitable` does not implement `__await__`

```
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/mro.md
14 |
15 | class BadSub1(Bad1()): ... # error: [invalid-base]
16 | class BadSub2(Bad2()): ... # error: [invalid-base]
17 | def _(flag: bool):
18 | class HasMroEntries:
19 | def __mro_entries__(self, bases: tuple[type, ...]) -> tuple[type, ...]:
20 | return ()
21 |
22 | class NoMroEntries: ...
23 |
24 | base = HasMroEntries() if flag else NoMroEntries()
25 |
26 | class Foo(base): ... # error: [invalid-base]
```

# Diagnostics
Expand Down Expand Up @@ -82,3 +92,17 @@ info: An instance type is only a valid class base if it has a valid `__mro_entri
info: Type `Bad2` has an `__mro_entries__` method, but it does not return a tuple of types

```

```
error[invalid-base]: Invalid class base with type `HasMroEntries | NoMroEntries`
--> src/mdtest_snippet.py:26:15
|
26 | class Foo(base): ... # error: [invalid-base]
| ^^^^
|
info: Definition of class `Foo` will raise `TypeError` at runtime
info: An instance type is only a valid class base if it has a valid `__mro_entries__` method
info: Type `HasMroEntries | NoMroEntries` may have an `__mro_entries__` attribute, but it may be missing
info: `NoMroEntries` does not implement `__mro_entries__`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would appreciate feedback on whether or not we should consolidate or eliminate some info diagnostics in this case. The current pattern is clear and each sub-diagnostic is informative, but 4 separate diagnostics might be considered a bit excessive.


```
13 changes: 12 additions & 1 deletion crates/ty_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7402,8 +7402,19 @@ impl<'db> AwaitError<'db> {
);
}
}
Self::Call(CallDunderError::PossiblyUnbound { bindings, .. }) => {
Self::Call(CallDunderError::PossiblyUnbound {
bindings,
unbound_on,
}) => {
diag.info("`__await__` may be missing");
if let Some(unbound_on) = unbound_on {
for ty in unbound_on {
diag.info(format_args!(
"`{}` does not implement `__await__`",
ty.display(db)
));
}
}
if let Some(definition_spans) = bindings.callable_type().function_spans(db) {
diag.annotate(
Annotation::secondary(definition_spans.signature)
Expand Down
10 changes: 9 additions & 1 deletion crates/ty_python_semantic/src/types/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5093,12 +5093,20 @@ pub(crate) fn report_invalid_or_unsupported_base(

match mro_entries_call_error {
CallDunderError::MethodNotAvailable => {}
CallDunderError::PossiblyUnbound { .. } => {
CallDunderError::PossiblyUnbound { unbound_on, .. } => {
explain_mro_entries(&mut diagnostic);
diagnostic.info(format_args!(
"Type `{}` may have an `__mro_entries__` attribute, but it may be missing",
base_type.display(db)
));
if let Some(unbound_on) = unbound_on {
for ty in unbound_on {
diagnostic.info(format_args!(
"`{}` does not implement `__mro_entries__`",
ty.display(db)
));
}
}
}
CallDunderError::CallError(CallErrorKind::NotCallable, _) => {
explain_mro_entries(&mut diagnostic);
Expand Down
108 changes: 60 additions & 48 deletions crates/ty_python_semantic/src/types/iteration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ impl<'db> Type<'db> {
// `__iter__` is possibly unbound...
Err(CallDunderError::PossiblyUnbound {
bindings: dunder_iter_outcome,
..
unbound_on: unbound_on_iter,
}) => {
let iterator = dunder_iter_outcome.return_type(db);

Expand All @@ -390,6 +390,7 @@ impl<'db> Type<'db> {
.map_err(|dunder_getitem_error| {
IterationError::PossiblyUnboundIterAndGetitemError {
dunder_next_return,
unbound_on_iter,
dunder_getitem_error,
}
})
Expand Down Expand Up @@ -453,6 +454,10 @@ pub(super) enum IterationError<'db> {
/// The type of the object returned by the `__next__` method on the iterator.
/// (The iterator being the type returned by the `__iter__` method on the iterable.)
dunder_next_return: Type<'db>,
/// For union types, the elements where `__iter__` was completely undefined.
/// Used to emit per-element info sub-diagnostics identifying the problematic members.
/// When this is omitted, it is because we don't care to track where exactly the methods were unbound.
unbound_on_iter: Option<Box<[Type<'db>]>>,
/// The error we encountered when we tried to call `__getitem__` on the iterable.
dunder_getitem_error: CallDunderError<'db>,
},
Expand Down Expand Up @@ -517,6 +522,7 @@ impl<'db> IterationError<'db> {

Self::PossiblyUnboundIterAndGetitemError {
dunder_next_return,
unbound_on_iter: _,
dunder_getitem_error,
} => match dunder_getitem_error {
CallDunderError::MethodNotAvailable => Some(*dunder_next_return),
Expand Down Expand Up @@ -737,73 +743,79 @@ impl<'db> IterationError<'db> {
}

Self::PossiblyUnboundIterAndGetitemError {
unbound_on_iter,
dunder_getitem_error,
..
} => match dunder_getitem_error {
CallDunderError::MethodNotAvailable => {
reporter.may_not(
} => {
let mut diag = match dunder_getitem_error {
CallDunderError::MethodNotAvailable => reporter.may_not(
"It may not have an `__iter__` method \
and it doesn't have a `__getitem__` method",
);
}
CallDunderError::PossiblyUnbound { .. } => {
reporter
.may_not("It may not have an `__iter__` method or a `__getitem__` method");
}
CallDunderError::CallError(CallErrorKind::NotCallable, bindings) => {
reporter.may_not(format_args!(
"It may not have an `__iter__` method \
and its `__getitem__` attribute has type `{dunder_getitem_type}`, \
which is not callable",
dunder_getitem_type = bindings.callable_type().display(db),
));
}
CallDunderError::CallError(CallErrorKind::PossiblyNotCallable, bindings)
if bindings.is_single() =>
{
reporter.may_not(
"It may not have an `__iter__` method \
and its `__getitem__` attribute may not be callable",
);
}
CallDunderError::CallError(CallErrorKind::PossiblyNotCallable, bindings) => {
reporter.may_not(format_args!(
"It may not have an `__iter__` method \
and its `__getitem__` attribute (with type `{dunder_getitem_type}`) \
may not be callable",
dunder_getitem_type = bindings.callable_type().display(db),
));
}
CallDunderError::CallError(CallErrorKind::BindingError, bindings)
if bindings.is_single() =>
{
reporter
.may_not(
),
CallDunderError::PossiblyUnbound { .. } => reporter
.may_not("It may not have an `__iter__` method or a `__getitem__` method"),
CallDunderError::CallError(CallErrorKind::NotCallable, bindings) => reporter
.may_not(format_args!(
"It may not have an `__iter__` method \
and its `__getitem__` attribute has type `{dunder_getitem_type}`, \
which is not callable",
dunder_getitem_type = bindings.callable_type().display(db),
)),
CallDunderError::CallError(CallErrorKind::PossiblyNotCallable, bindings)
if bindings.is_single() =>
{
reporter.may_not(
"It may not have an `__iter__` method \
and its `__getitem__` attribute may not be callable",
)
}
CallDunderError::CallError(CallErrorKind::PossiblyNotCallable, bindings) => {
reporter.may_not(format_args!(
"It may not have an `__iter__` method \
and its `__getitem__` attribute (with type `{dunder_getitem_type}`) \
may not be callable",
dunder_getitem_type = bindings.callable_type().display(db),
))
}
CallDunderError::CallError(CallErrorKind::BindingError, bindings)
if bindings.is_single() =>
{
let mut diag = reporter.may_not(
"It may not have an `__iter__` method \
and its `__getitem__` method has an incorrect signature \
for the old-style iteration protocol",
)
.info(
);
diag.info(
"`__getitem__` must be at least as permissive as \
`def __getitem__(self, key: int): ...` \
to satisfy the old-style iteration protocol",
);
}
CallDunderError::CallError(CallErrorKind::BindingError, bindings) => {
reporter
.may_not(format_args!(
diag
}
CallDunderError::CallError(CallErrorKind::BindingError, bindings) => {
let mut diag = reporter.may_not(format_args!(
"It may not have an `__iter__` method \
and its `__getitem__` method (with type `{dunder_getitem_type}`) \
may have an incorrect signature for the old-style iteration protocol",
dunder_getitem_type = bindings.callable_type().display(db),
))
.info(
));
diag.info(
"`__getitem__` must be at least as permissive as \
`def __getitem__(self, key: int): ...` \
to satisfy the old-style iteration protocol",
);
diag
}
};
if let Some(unbound_on) = unbound_on_iter.as_deref() {
for ty in unbound_on.iter().copied() {
diag.info(format_args!(
"`{}` does not implement `__iter__`",
ty.display(db)
));
}
}
},
}

Self::UnboundIterAndGetitemError {
dunder_getitem_error,
Expand Down
Loading