mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-02 06:41:23 +00:00
[red-knot] Implicit instance attributes in generic methods (#17769)
## Summary Add the ability to detect instance attribute assignments in class methods that are generic. This does not address the code duplication mentioned in #16928. I can open a ticket for this after this has been merged. closes #16928 ## Test Plan Added regression test.
This commit is contained in:
parent
17050e2ec5
commit
3cf44e401a
4 changed files with 69 additions and 11 deletions
|
@ -0,0 +1,9 @@
|
||||||
|
# Regression test for an issue that came up while working
|
||||||
|
# on https://github.com/astral-sh/ruff/pull/17769
|
||||||
|
|
||||||
|
class C:
|
||||||
|
def method[T](self, x: T) -> T:
|
||||||
|
def inner():
|
||||||
|
self.attr = 1
|
||||||
|
|
||||||
|
C().attr
|
|
@ -1949,6 +1949,27 @@ reveal_type(C.a_type) # revealed: type
|
||||||
reveal_type(C.a_none) # revealed: None
|
reveal_type(C.a_none) # revealed: None
|
||||||
```
|
```
|
||||||
|
|
||||||
|
### Generic methods
|
||||||
|
|
||||||
|
We also detect implicit instance attributes on methods that are themselves generic. We have an extra
|
||||||
|
test for this because generic functions have an extra type-params scope in between the function body
|
||||||
|
scope and the outer scope, so we need to make sure that our implementation can still recognize `f`
|
||||||
|
as a method of `C` here:
|
||||||
|
|
||||||
|
```toml
|
||||||
|
[environment]
|
||||||
|
python-version = "3.12"
|
||||||
|
```
|
||||||
|
|
||||||
|
```py
|
||||||
|
class C:
|
||||||
|
def f[T](self, t: T) -> T:
|
||||||
|
self.x: int = 1
|
||||||
|
return t
|
||||||
|
|
||||||
|
reveal_type(C().x) # revealed: int
|
||||||
|
```
|
||||||
|
|
||||||
## Enum classes
|
## Enum classes
|
||||||
|
|
||||||
Enums are not supported yet; attribute access on an enum class is inferred as `Todo`.
|
Enums are not supported yet; attribute access on an enum class is inferred as `Todo`.
|
||||||
|
|
|
@ -18,7 +18,8 @@ use crate::semantic_index::builder::SemanticIndexBuilder;
|
||||||
use crate::semantic_index::definition::{Definition, DefinitionNodeKey, Definitions};
|
use crate::semantic_index::definition::{Definition, DefinitionNodeKey, Definitions};
|
||||||
use crate::semantic_index::expression::Expression;
|
use crate::semantic_index::expression::Expression;
|
||||||
use crate::semantic_index::symbol::{
|
use crate::semantic_index::symbol::{
|
||||||
FileScopeId, NodeWithScopeKey, NodeWithScopeRef, Scope, ScopeId, ScopedSymbolId, SymbolTable,
|
FileScopeId, NodeWithScopeKey, NodeWithScopeRef, Scope, ScopeId, ScopeKind, ScopedSymbolId,
|
||||||
|
SymbolTable,
|
||||||
};
|
};
|
||||||
use crate::semantic_index::use_def::{EagerBindingsKey, ScopedEagerBindingsId, UseDefMap};
|
use crate::semantic_index::use_def::{EagerBindingsKey, ScopedEagerBindingsId, UseDefMap};
|
||||||
use crate::Db;
|
use crate::Db;
|
||||||
|
@ -109,12 +110,26 @@ pub(crate) fn attribute_assignments<'db, 's>(
|
||||||
let index = semantic_index(db, file);
|
let index = semantic_index(db, file);
|
||||||
let class_scope_id = class_body_scope.file_scope_id(db);
|
let class_scope_id = class_body_scope.file_scope_id(db);
|
||||||
|
|
||||||
ChildrenIter::new(index, class_scope_id).filter_map(|(file_scope_id, maybe_method)| {
|
ChildrenIter::new(index, class_scope_id).filter_map(|(child_scope_id, scope)| {
|
||||||
maybe_method.node().as_function()?;
|
let (function_scope_id, function_scope) =
|
||||||
let attribute_table = index.instance_attribute_table(file_scope_id);
|
if scope.node().scope_kind() == ScopeKind::Annotation {
|
||||||
|
// This could be a generic method with a type-params scope.
|
||||||
|
// Go one level deeper to find the function scope. The first
|
||||||
|
// descendant is the (potential) function scope.
|
||||||
|
let function_scope_id = scope.descendants().start;
|
||||||
|
(function_scope_id, index.scope(function_scope_id))
|
||||||
|
} else {
|
||||||
|
(child_scope_id, scope)
|
||||||
|
};
|
||||||
|
|
||||||
|
function_scope.node().as_function()?;
|
||||||
|
let attribute_table = index.instance_attribute_table(function_scope_id);
|
||||||
let symbol = attribute_table.symbol_id_by_name(name)?;
|
let symbol = attribute_table.symbol_id_by_name(name)?;
|
||||||
let use_def = &index.use_def_maps[file_scope_id];
|
let use_def = &index.use_def_maps[function_scope_id];
|
||||||
Some((use_def.instance_attribute_bindings(symbol), file_scope_id))
|
Some((
|
||||||
|
use_def.instance_attribute_bindings(symbol),
|
||||||
|
function_scope_id,
|
||||||
|
))
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -183,13 +183,26 @@ impl<'db> SemanticIndexBuilder<'db> {
|
||||||
fn is_method_of_class(&self) -> Option<FileScopeId> {
|
fn is_method_of_class(&self) -> Option<FileScopeId> {
|
||||||
let mut scopes_rev = self.scope_stack.iter().rev();
|
let mut scopes_rev = self.scope_stack.iter().rev();
|
||||||
let current = scopes_rev.next()?;
|
let current = scopes_rev.next()?;
|
||||||
|
|
||||||
|
if self.scopes[current.file_scope_id].kind() != ScopeKind::Function {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
let parent = scopes_rev.next()?;
|
let parent = scopes_rev.next()?;
|
||||||
|
|
||||||
match (
|
match self.scopes[parent.file_scope_id].kind() {
|
||||||
self.scopes[current.file_scope_id].kind(),
|
ScopeKind::Class => Some(parent.file_scope_id),
|
||||||
self.scopes[parent.file_scope_id].kind(),
|
ScopeKind::Annotation => {
|
||||||
) {
|
// If the function is generic, the parent scope is an annotation scope.
|
||||||
(ScopeKind::Function, ScopeKind::Class) => Some(parent.file_scope_id),
|
// In this case, we need to go up one level higher to find the class scope.
|
||||||
|
let grandparent = scopes_rev.next()?;
|
||||||
|
|
||||||
|
if self.scopes[grandparent.file_scope_id].kind() == ScopeKind::Class {
|
||||||
|
Some(grandparent.file_scope_id)
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
}
|
||||||
_ => None,
|
_ => None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue