[ty] Add diagnostics for invalid await expressions by theammir · Pull Request #19711 · astral-sh/ruff (original) (raw)

carljm added a commit that referenced this pull request

Aug 13, 2025

Summary

A [passing comment](#19711 (comment)) led me to explore why we didn't report a class attribute as possibly unbound if it was a method and defined in two different conditional branches.

I found that the reason was because of our handling of "conflicting declarations" in place_from_declarations. It returned a Result which would be Err in case of conflicting declarations.

But we only actually care about conflicting declarations when we are actually doing type inference on that scope and might emit a diagnostic about it. And in all cases (including that one), we want to otherwise proceed with the union of the declared types, as if there was no conflict.

In several cases we were failing to handle the union of declared types in the same way as a normal declared type if there was a declared-types conflict. The Result return type made this mistake really easy to make, as we'd match on e.g. Ok(Place::Type(...)) and do one thing, then match on Err(...) and do another, even though really both of those cases should be handled the same.

This PR refactors place_from_declarations to instead return a struct which always represents the declared type we should use in the same way, as well as carrying the conflicting declared types, if any. This struct has a method to allow us to explicitly ignore the declared-types conflict (which is what we want in most cases), as well as a method to get the declared type and the conflict information, in the case where we want to emit a diagnostic on the conflict.

Test Plan

Existing CI; added a test showing that we now understand a multiply-conditionally-defined method as possibly-unbound.

This does trigger issues on a couple new fuzzer seeds, but the issues are just new instances of an already-known (and rarely occurring) problem which I already plan to address in a future PR, so I think it's OK to land as-is.

I happened to build this initially on top of #19711, which adds invalid-await diagnostics, so I also updated some invalid-syntax tests to not await on an invalid type, since the purpose of those tests is to check the syntactic location of the await, not the validity of the awaited type.

dcreager added a commit that referenced this pull request

Aug 14, 2025

• main: [ty] Add diagnostics for invalid await expressions (#19711) [ty] Synthesize read-only properties for all declared members on NamedTuple classes (#19899) [ty] Remove use of ClassBase::try_from_type from super() machinery (#19902) [ty] Speedup project file discovery (#19913) [pyflakes] Add secondary annotation showing previous definition (F811) (#19900) Bump 0.12.9 (#19917) [ty] support kw_only=True for dataclass() and field() (#19677)

KotlinIsland pushed a commit to KotlinIsland/basedpython that referenced this pull request

May 1, 2026

Summary

A [passing comment](astral-sh/ruff#19711 (comment)) led me to explore why we didn't report a class attribute as possibly unbound if it was a method and defined in two different conditional branches.

I found that the reason was because of our handling of "conflicting declarations" in place_from_declarations. It returned a Result which would be Err in case of conflicting declarations.

But we only actually care about conflicting declarations when we are actually doing type inference on that scope and might emit a diagnostic about it. And in all cases (including that one), we want to otherwise proceed with the union of the declared types, as if there was no conflict.

In several cases we were failing to handle the union of declared types in the same way as a normal declared type if there was a declared-types conflict. The Result return type made this mistake really easy to make, as we'd match on e.g. Ok(Place::Type(...)) and do one thing, then match on Err(...) and do another, even though really both of those cases should be handled the same.

This PR refactors place_from_declarations to instead return a struct which always represents the declared type we should use in the same way, as well as carrying the conflicting declared types, if any. This struct has a method to allow us to explicitly ignore the declared-types conflict (which is what we want in most cases), as well as a method to get the declared type and the conflict information, in the case where we want to emit a diagnostic on the conflict.

Test Plan

Existing CI; added a test showing that we now understand a multiply-conditionally-defined method as possibly-unbound.

This does trigger issues on a couple new fuzzer seeds, but the issues are just new instances of an already-known (and rarely occurring) problem which I already plan to address in a future PR, so I think it's OK to land as-is.

I happened to build this initially on top of astral-sh/ruff#19711, which adds invalid-await diagnostics, so I also updated some invalid-syntax tests to not await on an invalid type, since the purpose of those tests is to check the syntactic location of the await, not the validity of the awaited type.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})