mirror of
				https://github.com/astral-sh/ruff.git
				synced 2025-10-31 12:05:57 +00:00 
			
		
		
		
	[ty] Bind Self typevar to method context (#20366)
Fixes: https://github.com/astral-sh/ty/issues/1173 <!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary This PR will change the logic of binding Self type variables to bind self to the immediate function that it's used on. Since we are binding `self` to methods and not the class itself we need to ensure that we bind self consistently. The fix is to traverse scopes containing the self and find the first function inside a class and use that function to bind the typevar for self. If no such scope is found we fallback to the normal behavior. Using Self outside of a class scope is not legal anyway. ## Test Plan Added a new mdtest. Checked the diagnostics that are not emitted anymore in [primer results](https://github.com/astral-sh/ruff/pull/20366#issuecomment-3289411424). It looks good altough I don't completely understand what was wrong before. --------- Co-authored-by: Douglas Creager <dcreager@dcreager.net>
This commit is contained in:
		
							parent
							
								
									3fcbe8bde6
								
							
						
					
					
						commit
						05622ae757
					
				
					 2 changed files with 47 additions and 5 deletions
				
			
		|  | @ -30,9 +30,7 @@ class Shape: | |||
| 
 | ||||
|     def nested_func_without_enclosing_binding(self): | ||||
|         def inner(x: Self): | ||||
|             # TODO: revealed: Self@nested_func_without_enclosing_binding | ||||
|             # (The outer method binds an implicit `Self`) | ||||
|             reveal_type(x)  # revealed: Self@inner | ||||
|             reveal_type(x)  # revealed: Self@nested_func_without_enclosing_binding | ||||
|         inner(self) | ||||
| 
 | ||||
|     def implicit_self(self) -> Self: | ||||
|  | @ -239,4 +237,36 @@ reveal_type(D().instance_method) | |||
| reveal_type(D.class_method) | ||||
| ``` | ||||
| 
 | ||||
| In nested functions `self` binds to the method. So in the following example the `self` in `C.b` is | ||||
| bound at `C.f`. | ||||
| 
 | ||||
| ```py | ||||
| from typing import Self | ||||
| from ty_extensions import generic_context | ||||
| 
 | ||||
| class C[T](): | ||||
|     def f(self: Self): | ||||
|         def b(x: Self): | ||||
|             reveal_type(x)  # revealed: Self@f | ||||
|         reveal_type(generic_context(b))  # revealed: None | ||||
| 
 | ||||
| reveal_type(generic_context(C.f))  # revealed: tuple[Self@f] | ||||
| ``` | ||||
| 
 | ||||
| Even if the `Self` annotation appears first in the nested function, it is the method that binds | ||||
| `Self`. | ||||
| 
 | ||||
| ```py | ||||
| from typing import Self | ||||
| from ty_extensions import generic_context | ||||
| 
 | ||||
| class C: | ||||
|     def f(self: "C"): | ||||
|         def b(x: Self): | ||||
|             reveal_type(x)  # revealed: Self@f | ||||
|         reveal_type(generic_context(b))  # revealed: None | ||||
| 
 | ||||
| reveal_type(generic_context(C.f))  # revealed: None | ||||
| ``` | ||||
| 
 | ||||
| [self attribute]: https://typing.python.org/en/latest/spec/generics.html#use-in-attribute-annotations | ||||
|  |  | |||
|  | @ -2,6 +2,7 @@ use std::borrow::Cow; | |||
| 
 | ||||
| use crate::types::constraints::ConstraintSet; | ||||
| 
 | ||||
| use itertools::Itertools; | ||||
| use ruff_db::parsed::ParsedModuleRef; | ||||
| use ruff_python_ast as ast; | ||||
| use rustc_hash::FxHashMap; | ||||
|  | @ -18,8 +19,8 @@ use crate::types::tuple::{TupleSpec, TupleType, walk_tuple_type}; | |||
| use crate::types::{ | ||||
|     ApplyTypeMappingVisitor, BoundTypeVarInstance, FindLegacyTypeVarsVisitor, HasRelationToVisitor, | ||||
|     IsEquivalentVisitor, KnownClass, KnownInstanceType, MaterializationKind, NormalizedVisitor, | ||||
|     Type, TypeMapping, TypeRelation, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarVariance, | ||||
|     UnionType, binding_type, declaration_type, | ||||
|     Type, TypeMapping, TypeRelation, TypeVarBoundOrConstraints, TypeVarInstance, TypeVarKind, | ||||
|     TypeVarVariance, UnionType, binding_type, declaration_type, | ||||
| }; | ||||
| use crate::{Db, FxOrderSet}; | ||||
| 
 | ||||
|  | @ -82,6 +83,17 @@ pub(crate) fn bind_typevar<'db>( | |||
|     typevar_binding_context: Option<Definition<'db>>, | ||||
|     typevar: TypeVarInstance<'db>, | ||||
| ) -> Option<BoundTypeVarInstance<'db>> { | ||||
|     // typing.Self is treated like a legacy typevar, but doesn't follow the same scoping rules. It is always bound to the outermost method in the containing class.
 | ||||
|     if matches!(typevar.kind(db), TypeVarKind::TypingSelf) { | ||||
|         for ((_, inner), (_, outer)) in index.ancestor_scopes(containing_scope).tuple_windows() { | ||||
|             if outer.kind().is_class() { | ||||
|                 if let NodeWithScopeKind::Function(function) = inner.node() { | ||||
|                     let definition = index.expect_single_definition(function.node(module)); | ||||
|                     return Some(typevar.with_binding_context(db, definition)); | ||||
|                 } | ||||
|             } | ||||
|         } | ||||
|     } | ||||
|     enclosing_generic_contexts(db, module, index, containing_scope) | ||||
|         .find_map(|enclosing_context| enclosing_context.binds_typevar(db, typevar)) | ||||
|         .or_else(|| { | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Shaygan Hooshyari
						Shaygan Hooshyari