diff --git a/crates/ty_python_semantic/resources/mdtest/import/star.md b/crates/ty_python_semantic/resources/mdtest/import/star.md index 5b00e49812..8ac7ea4e02 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/star.md +++ b/crates/ty_python_semantic/resources/mdtest/import/star.md @@ -655,7 +655,7 @@ from b import * reveal_type(X) # revealed: bool ``` -## Visibility constraints +## Reachability constraints If an `importer` module contains a `from exporter import *` statement in its global namespace, the statement will *not* necessarily import *all* symbols that have definitions in `exporter.py`'s @@ -664,13 +664,13 @@ imported by the `*` import if at least one definition for that symbol is visible `exporter.py`'s global scope. For example, say that `exporter.py` contains a symbol `X` in its global scope, and the definition -for `X` in `exporter.py` has visibility constraints vis1. The +for `X` in `exporter.py` has reachability constraints c1. The `from exporter import *` statement in `importer.py` creates a definition for `X` in `importer`, and -there are visibility constraints vis2 on the import statement in -`importer.py`. This means that the overall visibility constraints on the `X` definnition created by -the import statement in `importer.py` will be vis1 AND vis2. +there are reachability constraints c2 on the import statement in +`importer.py`. This means that the overall reachability constraints on the `X` definition created by +the import statement in `importer.py` will be c1 AND c2. -A visibility constraint in the external module must be understood and evaluated whether or not its +A reachability constraint in the external module must be understood and evaluated whether or not its truthiness can be statically determined. ### Statically known branches in the external module @@ -712,11 +712,19 @@ reveal_type(Y) # revealed: Unknown # to be dead code given the `python-version` configuration. # Thus this still reveals `Literal[True]`. reveal_type(Z) # revealed: Literal[True] + +# Make sure that reachability constraints are also correctly applied +# for nonlocal lookups: +def _(): + reveal_type(X) # revealed: bool + # error: [unresolved-reference] + reveal_type(Y) # revealed: Unknown + reveal_type(Z) # revealed: bool ``` -### Multiple `*` imports with always-false visibility constraints +### Multiple `*` imports with always-false reachability constraints -Our understanding of visibility constraints in an external module remains accurate, even if there +Our understanding of reachability constraints in an external module remains accurate, even if there are multiple `*` imports from that module. ```toml @@ -745,7 +753,7 @@ from exporter import * reveal_type(Z) # revealed: Literal[True] ``` -### Ambiguous visibility constraints +### Ambiguous reachability constraints Some constraints in the external module may resolve to an "ambiguous truthiness". For these, we should emit `possibly-unresolved-reference` diagnostics when they are used in the module in which @@ -775,7 +783,7 @@ reveal_type(A) # revealed: Unknown | Literal[1] reveal_type(B) # revealed: Unknown | Literal[2, 3] ``` -### Visibility constraints in the importing module +### Reachability constraints in the importing module `exporter.py`: @@ -796,7 +804,7 @@ if coinflip(): reveal_type(A) # revealed: Unknown | Literal[1] ``` -### Visibility constraints in the exporting module *and* the importing module +### Reachability constraints in the exporting module *and* the importing module ```toml [environment] diff --git a/crates/ty_python_semantic/resources/mdtest/terminal_statements.md b/crates/ty_python_semantic/resources/mdtest/terminal_statements.md index 1a9f168438..baec20573d 100644 --- a/crates/ty_python_semantic/resources/mdtest/terminal_statements.md +++ b/crates/ty_python_semantic/resources/mdtest/terminal_statements.md @@ -622,9 +622,9 @@ def return_from_nested_if(cond1: bool, cond2: bool): ## Statically known terminal statements -We model reachability using the same visibility constraints that we use to model statically known -bounds. In this example, we see that the `return` statement is always executed, and therefore that -the `"b"` assignment is not visible to the `reveal_type`. +We model reachability using the same constraints that we use to model statically known bounds. In +this example, we see that the `return` statement is always executed, and therefore that the `"b"` +assignment is not visible to the `reveal_type`. ```py def _(cond: bool): diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 4f0ffd5a67..68e6a6847f 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -1316,15 +1316,38 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { referenced_module, ); + let star_import_predicate = self.add_predicate(star_import.into()); + let pre_definition = self.current_use_def_map().single_place_snapshot(symbol_id); + let pre_definition_reachability = + self.current_use_def_map().reachability; + + // Temporarily modify the reachability to include the star import predicate, + // in order for the new definition to pick it up. + let reachability_constraints = + &mut self.current_use_def_map_mut().reachability_constraints; + let star_import_reachability = + reachability_constraints.add_atom(star_import_predicate); + let definition_reachability = reachability_constraints + .add_and_constraint( + pre_definition_reachability, + star_import_reachability, + ); + self.current_use_def_map_mut().reachability = definition_reachability; + self.push_additional_definition(symbol_id, node_ref); + self.current_use_def_map_mut() .record_and_negate_star_import_reachability_constraint( - star_import, + star_import_reachability, symbol_id, pre_definition, ); + + // Restore the reachability to its pre-definition state + self.current_use_def_map_mut().reachability = + pre_definition_reachability; } continue; diff --git a/crates/ty_python_semantic/src/semantic_index/use_def.rs b/crates/ty_python_semantic/src/semantic_index/use_def.rs index b17459c7aa..f975c4bf85 100644 --- a/crates/ty_python_semantic/src/semantic_index/use_def.rs +++ b/crates/ty_python_semantic/src/semantic_index/use_def.rs @@ -248,7 +248,6 @@ use crate::semantic_index::place::{ }; use crate::semantic_index::predicate::{ Predicate, PredicateOrLiteral, Predicates, PredicatesBuilder, ScopedPredicateId, - StarImportPlaceholderPredicate, }; use crate::semantic_index::reachability_constraints::{ ReachabilityConstraints, ReachabilityConstraintsBuilder, ScopedReachabilityConstraintId, @@ -844,7 +843,7 @@ impl<'db> UseDefMapBuilder<'db> { /// This method exists solely for handling `*`-import reachability constraints. /// /// The reason why we add reachability constraints for [`Definition`]s created by `*` imports - /// is laid out in the doc-comment for [`StarImportPlaceholderPredicate`]. But treating these + /// is laid out in the doc-comment for `StarImportPlaceholderPredicate`. But treating these /// reachability constraints in the use-def map the same way as all other reachability constraints /// was shown to lead to [significant regressions] for small codebases where typeshed /// dominates. (Although `*` imports are not common generally, they are used in several @@ -872,12 +871,10 @@ impl<'db> UseDefMapBuilder<'db> { /// [significant regressions]: https://github.com/astral-sh/ruff/pull/17286#issuecomment-2786755746 pub(super) fn record_and_negate_star_import_reachability_constraint( &mut self, - star_import: StarImportPlaceholderPredicate<'db>, + reachability_id: ScopedReachabilityConstraintId, symbol: ScopedPlaceId, pre_definition_state: PlaceState, ) { - let predicate_id = self.add_predicate(star_import.into()); - let reachability_id = self.reachability_constraints.add_atom(predicate_id); let negated_reachability_id = self .reachability_constraints .add_not_constraint(reachability_id);