mirror of https://github.com/astral-sh/ruff
[ty] Fix flow of associated member states during star imports (#21776)
## Summary Star-imports can not just affect the state of symbols that they pull in, they can also affect the state of members that are associated with those symbols. For example, if `obj.attr` was previously narrowed from `int | None` to `int`, and a star-import now overwrites `obj`, then the narrowing on `obj.attr` should be "reset". This PR keeps track of the state of associated members during star imports and properly models the flow of their corresponding state through the control flow structure that we artificially create for star-imports. See [this comment](https://github.com/astral-sh/ty/issues/1355#issuecomment-3607125005) for an explanation why this caused ty to see certain `asyncio` symbols as not being accessible on Python 3.14. closes https://github.com/astral-sh/ty/issues/1355 ## Ecosystem impact ```diff async-utils (https://github.com/mikeshardmind/async-utils) - src/async_utils/bg_loop.py:115:31: error[invalid-argument-type] Argument to bound method `set_task_factory` is incorrect: Expected `_TaskFactory | None`, found `def eager_task_factory[_T_co](loop: AbstractEventLoop | None, coro: Coroutine[Any, Any, _T_co@eager_task_factory], *, name: str | None = None, context: Context | None = None) -> Task[_T_co@eager_task_factory]` - Found 30 diagnostics + Found 29 diagnostics mitmproxy (https://github.com/mitmproxy/mitmproxy) + mitmproxy/utils/asyncio_utils.py:96:60: warning[unused-ignore-comment] Unused blanket `type: ignore` directive - test/conftest.py:37:31: error[invalid-argument-type] Argument to bound method `set_task_factory` is incorrect: Expected `_TaskFactory | None`, found `def eager_task_factory[_T_co](loop: AbstractEventLoop | None, coro: Coroutine[Any, Any, _T_co@eager_task_factory], *, name: str | None = None, context: Context | None = None) -> Task[_T_co@eager_task_factory]` ``` All of these seem to be correct, they give us a different type for `asyncio` symbols that are now imported from different `sys.version_info` branches (where we previously failed to recognize some of these as statically true/false). ```diff dd-trace-py (https://github.com/DataDog/dd-trace-py) - ddtrace/contrib/internal/asyncio/patch.py:39:12: error[invalid-argument-type] Argument to function `unwrap` is incorrect: Expected `WrappedFunction`, found `def create_task[_T](self, coro: Coroutine[Any, Any, _T@create_task] | Generator[Any, None, _T@create_task], *, name: object = None) -> Task[_T@create_task]` + ddtrace/contrib/internal/asyncio/patch.py:39:12: error[invalid-argument-type] Argument to function `unwrap` is incorrect: Expected `WrappedFunction`, found `def create_task[_T](self, coro: Generator[Any, None, _T@create_task] | Coroutine[Any, Any, _T@create_task], *, name: object = None) -> Task[_T@create_task]` ``` Similar, but only results in a diagnostic change. ## Test Plan Added a regression test
This commit is contained in:
parent
4488e9d47d
commit
1f4f8d9950
|
|
@ -1336,6 +1336,69 @@ reveal_type(g) # revealed: Unknown
|
|||
reveal_type(h) # revealed: Unknown
|
||||
```
|
||||
|
||||
## Star-imports can affect member states
|
||||
|
||||
If a star-import pulls in a symbol that was previously defined in the importing module (e.g. `obj`),
|
||||
it can affect the state of associated member expressions (e.g. `obj.attr` or `obj[0]`). In the test
|
||||
below, note how the types of the corresponding attribute expressions change after the star import
|
||||
affects the object:
|
||||
|
||||
`common.py`:
|
||||
|
||||
```py
|
||||
class C:
|
||||
attr: int | None
|
||||
```
|
||||
|
||||
`exporter.py`:
|
||||
|
||||
```py
|
||||
from common import C
|
||||
|
||||
def flag() -> bool:
|
||||
return True
|
||||
|
||||
should_be_imported: C = C()
|
||||
|
||||
if flag():
|
||||
might_be_imported: C = C()
|
||||
|
||||
if False:
|
||||
should_not_be_imported: C = C()
|
||||
```
|
||||
|
||||
`main.py`:
|
||||
|
||||
```py
|
||||
from common import C
|
||||
|
||||
should_be_imported = C()
|
||||
might_be_imported = C()
|
||||
should_not_be_imported = C()
|
||||
|
||||
# We start with the plain attribute types:
|
||||
reveal_type(should_be_imported.attr) # revealed: int | None
|
||||
reveal_type(might_be_imported.attr) # revealed: int | None
|
||||
reveal_type(should_not_be_imported.attr) # revealed: int | None
|
||||
|
||||
# Now we narrow the types by assignment:
|
||||
should_be_imported.attr = 1
|
||||
might_be_imported.attr = 1
|
||||
should_not_be_imported.attr = 1
|
||||
|
||||
reveal_type(should_be_imported.attr) # revealed: Literal[1]
|
||||
reveal_type(might_be_imported.attr) # revealed: Literal[1]
|
||||
reveal_type(should_not_be_imported.attr) # revealed: Literal[1]
|
||||
|
||||
# This star import adds bindings for `should_be_imported` and `might_be_imported`:
|
||||
from exporter import *
|
||||
|
||||
# As expected, narrowing is "reset" for the first two variables, but not for the third:
|
||||
reveal_type(should_be_imported.attr) # revealed: int | None
|
||||
reveal_type(might_be_imported.attr) # revealed: int | None
|
||||
reveal_type(should_not_be_imported.attr) # revealed: Literal[1]
|
||||
```
|
||||
|
||||
## Cyclic star imports
|
||||
|
||||
Believe it or not, this code does *not* raise an exception at runtime!
|
||||
|
|
|
|||
|
|
@ -1616,9 +1616,12 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
|||
|
||||
let star_import_predicate = self.add_predicate(star_import.into());
|
||||
|
||||
let associated_member_ids = self.place_tables[self.current_scope()]
|
||||
.associated_place_ids(ScopedPlaceId::Symbol(symbol_id));
|
||||
let pre_definition = self
|
||||
.current_use_def_map()
|
||||
.single_symbol_place_snapshot(symbol_id);
|
||||
.single_symbol_snapshot(symbol_id, associated_member_ids);
|
||||
|
||||
let pre_definition_reachability =
|
||||
self.current_use_def_map().reachability;
|
||||
|
||||
|
|
|
|||
|
|
@ -801,6 +801,13 @@ pub(super) struct FlowSnapshot {
|
|||
reachability: ScopedReachabilityConstraintId,
|
||||
}
|
||||
|
||||
/// A snapshot of the state of a single symbol (e.g. `obj`) and all of its associated members
|
||||
/// (e.g. `obj.attr`, `obj["key"]`).
|
||||
pub(super) struct SingleSymbolSnapshot {
|
||||
symbol_state: PlaceState,
|
||||
associated_member_states: FxHashMap<ScopedMemberId, PlaceState>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(super) struct UseDefMapBuilder<'db> {
|
||||
/// Append-only array of [`DefinitionState`].
|
||||
|
|
@ -991,13 +998,26 @@ impl<'db> UseDefMapBuilder<'db> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Snapshot the state of a single place at the current point in control flow.
|
||||
/// Snapshot the state of a single symbol and all of its associated members, at the current
|
||||
/// point in control flow.
|
||||
///
|
||||
/// This is only used for `*`-import reachability constraints, which are handled differently
|
||||
/// to most other reachability constraints. See the doc-comment for
|
||||
/// [`Self::record_and_negate_star_import_reachability_constraint`] for more details.
|
||||
pub(super) fn single_symbol_place_snapshot(&self, symbol: ScopedSymbolId) -> PlaceState {
|
||||
self.symbol_states[symbol].clone()
|
||||
pub(super) fn single_symbol_snapshot(
|
||||
&self,
|
||||
symbol: ScopedSymbolId,
|
||||
associated_member_ids: &[ScopedMemberId],
|
||||
) -> SingleSymbolSnapshot {
|
||||
let symbol_state = self.symbol_states[symbol].clone();
|
||||
let mut associated_member_states = FxHashMap::default();
|
||||
for &member_id in associated_member_ids {
|
||||
associated_member_states.insert(member_id, self.member_states[member_id].clone());
|
||||
}
|
||||
SingleSymbolSnapshot {
|
||||
symbol_state,
|
||||
associated_member_states,
|
||||
}
|
||||
}
|
||||
|
||||
/// This method exists solely for handling `*`-import reachability constraints.
|
||||
|
|
@ -1033,14 +1053,14 @@ impl<'db> UseDefMapBuilder<'db> {
|
|||
&mut self,
|
||||
reachability_id: ScopedReachabilityConstraintId,
|
||||
symbol: ScopedSymbolId,
|
||||
pre_definition_state: PlaceState,
|
||||
pre_definition: SingleSymbolSnapshot,
|
||||
) {
|
||||
let negated_reachability_id = self
|
||||
.reachability_constraints
|
||||
.add_not_constraint(reachability_id);
|
||||
|
||||
let mut post_definition_state =
|
||||
std::mem::replace(&mut self.symbol_states[symbol], pre_definition_state);
|
||||
std::mem::replace(&mut self.symbol_states[symbol], pre_definition.symbol_state);
|
||||
|
||||
post_definition_state
|
||||
.record_reachability_constraint(&mut self.reachability_constraints, reachability_id);
|
||||
|
|
@ -1055,6 +1075,30 @@ impl<'db> UseDefMapBuilder<'db> {
|
|||
&mut self.narrowing_constraints,
|
||||
&mut self.reachability_constraints,
|
||||
);
|
||||
|
||||
// And similarly for all associated members:
|
||||
for (member_id, pre_definition_member_state) in pre_definition.associated_member_states {
|
||||
let mut post_definition_state = std::mem::replace(
|
||||
&mut self.member_states[member_id],
|
||||
pre_definition_member_state,
|
||||
);
|
||||
|
||||
post_definition_state.record_reachability_constraint(
|
||||
&mut self.reachability_constraints,
|
||||
reachability_id,
|
||||
);
|
||||
|
||||
self.member_states[member_id].record_reachability_constraint(
|
||||
&mut self.reachability_constraints,
|
||||
negated_reachability_id,
|
||||
);
|
||||
|
||||
self.member_states[member_id].merge(
|
||||
post_definition_state,
|
||||
&mut self.narrowing_constraints,
|
||||
&mut self.reachability_constraints,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn record_reachability_constraint(
|
||||
|
|
|
|||
Loading…
Reference in New Issue