language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-31 12:05:57 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[ty] defer inference of legacy TypeVar bound/constraints/defaults (#20598)

Browse source 
## Summary

This allows us to handle self-referential bounds/constraints/defaults
without panicking.

Handles more cases from https://github.com/astral-sh/ty/issues/256

This also changes the way we infer the types of legacy TypeVars. Rather
than understanding a constructor call to `typing[_extension].TypeVar`
inside of any (arbitrarily nested) expression, and having to use a
special `assigned_to` field of the semantic index to try to best-effort
figure out what name the typevar was assigned to, we instead understand
the creation of a legacy `TypeVar` only in the supported syntactic
position (RHS of a simple un-annotated assignment with one target). In
any other position, we just infer it as creating an opaque instance of
`typing.TypeVar`. (This behavior matches all other type checkers.)

So we now special-case TypeVar creation in `TypeInferenceBuilder`, as a
special case of an assignment definition, rather than deeper inside call
binding. This does mean we re-implement slightly more of
argument-parsing, but in practice this is minimal and easy to handle
correctly.

This is easier to implement if we also make the RHS of a simple (no
unpacking) one-target assignment statement no longer a standalone
expression. Which is fine to do, because simple one-target assignments
don't need to infer the RHS more than once. This is a bonus performance
(0-3% across various projects) and significant memory-usage win, since
most assignment statements are simple one-target assignment statements,
meaning we now create many fewer standalone-expression salsa
ingredients.

This change does mean that inference of manually-constructed
`TypeAliasType` instances can no longer find its Definition in
`assigned_to`, which regresses go-to-definition for these aliases. In a
future PR, `TypeAliasType` will receive the same treatment that
`TypeVar` did in this PR (moving its special-case inference into
`TypeInferenceBuilder` and supporting it only in the correct syntactic
position, and lazily inferring its value type to support recursion),
which will also fix the go-to-definition regression. (I decided a
temporary edge-case regression is better in this case than doubling the
size of this PR.)

This PR also tightens up and fixes various aspects of the validation of
`TypeVar` creation, as seen in the tests.

We still (for now) treat all typevars as instances of `typing.TypeVar`,
even if they were created using `typing_extensions.TypeVar`. This means
we'll wrongly error on e.g. `T.__default__` on Python 3.11, even if `T`
is a `typing_extensions.TypeVar` instance at runtime. We share this
wrong behavior with both mypy and pyrefly. It will be easier to fix
after we pull in https://github.com/python/typeshed/pull/14840.

There are some issues that showed up here with typevar identity and
`MarkTypeVarsInferable`; the fix here (using the new `original` field
and `is_identical_to` methods on `BoundTypeVarInstance` and
`TypeVarInstance`) is a bit kludgy, but it can go away when we eliminate
`MarkTypeVarsInferable`.

## Test Plan

Added and updated mdtests.

### Conformance suite impact

The impact here is all positive:

* We now correctly error on a legacy TypeVar with exactly one constraint
type given.
* We now correctly error on a legacy TypeVar with both an upper bound
and constraints specified.

### Ecosystem impact

Basically none; in the setuptools case we just issue slightly different
errors on an invalid TypeVar definition, due to the modified validation
code.

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This commit is contained in:
Carl Meyer 2025-10-09 14:08:37 -07:00 • committed by GitHub
parent b086ffe921
commit 8248193ed9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 1441 additions and 408 deletions
Download patch file Download diff file Expand all files Collapse all files

22
crates/ty_python_semantic/src/semantic_index/builder.rs
View file

@ -1165,10 +1165,6 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
target: &'ast ast::Expr,
value: Expression<'db>,
) {
// We only handle assignments to names and unpackings here, other targets like
// attribute and subscript are handled separately as they don't create a new
// definition.
let current_assignment = match target {
ast::Expr::List(_) | ast::Expr::Tuple(_) => {
if matches!(unpackable, Unpackable::Comprehension { .. }) {
@ -1628,10 +1624,22 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
debug_assert_eq!(&self.current_assignments, &[]);
self.visit_expr(&node.value);
let value = self.add_standalone_assigned_expression(&node.value, node);
for target in &node.targets {
self.add_unpackable_assignment(&Unpackable::Assign(node), target, value);
// Optimization for the common case: if there's just one target, and it's not an
// unpacking, and the target is a simple name, we don't need the RHS to be a
// standalone expression at all.
if let [target] = &node.targets[..]
&& target.is_name_expr()
{
self.push_assignment(CurrentAssignment::Assign { node, unpack: None });
self.visit_expr(target);
self.pop_assignment();
} else {
let value = self.add_standalone_assigned_expression(&node.value, node);
for target in &node.targets {
self.add_unpackable_assignment(&Unpackable::Assign(node), target, value);
}
}
}
ast::Stmt::AnnAssign(node) => {