[ty] Fix Self resolution for classes nested within methods

[charliermarsh]

Summary

Closes astral-sh/ty#2643.

[astral-sh-bot]

Typing conformance results

No changes detected ✅

[astral-sh-bot]

mypy_primer results

Changes were detected when running on open source projects

rotki (https://github.com/rotki/rotki)
- rotkehlchen/chain/decoding/tools.py:96:44: warning[unused-type-ignore-comment] Unused blanket `type: ignore` directive
- rotkehlchen/chain/decoding/tools.py:99:13: error[invalid-argument-type] Argument to function `decode_transfer_direction` is incorrect: Expected `Sequence[A@BaseDecoderTools]`, found `Unknown | tuple[BTCAddress, ...] | tuple[ChecksumAddress, ...] | tuple[SubstrateAddress, ...] | tuple[SolanaAddress, ...]`
- rotkehlchen/chain/decoding/tools.py:100:62: warning[unused-type-ignore-comment] Unused blanket `type: ignore` directive
+ rotkehlchen/chain/decoding/tools.py:97:13: error[invalid-argument-type] Argument to function `decode_transfer_direction` is incorrect: Expected `BTCAddress | ChecksumAddress | SubstrateAddress | SolanaAddress`, found `A@BaseDecoderTools`
+ rotkehlchen/chain/decoding/tools.py:98:13: error[invalid-argument-type] Argument to function `decode_transfer_direction` is incorrect: Expected `BTCAddress | ChecksumAddress | SubstrateAddress | SolanaAddress | None`, found `A@BaseDecoderTools | None`
- Found 2055 diagnostics
+ Found 2054 diagnostics

core (https://github.com/home-assistant/core)
- homeassistant/util/variance.py:47:12: error[invalid-return-type] Return type does not match returned value: expected `(**_P@ignore_variance) -> _R@ignore_variance`, found `_Wrapped[_P@ignore_variance, int | _R@ignore_variance | float | datetime, _P@ignore_variance, _R@ignore_variance | int | float | datetime]`
- Found 14516 diagnostics
+ Found 14515 diagnostics

No memory usage changes detected ✅

[AlexWaygood]

thanks, that looks a lot more like what I'd expect here!

[AlexWaygood]

I think you can do this using higher-level APIs that mean we don't have to access the raw AST (which is always best avoided where possible, as it hurts our incrementality)

diff --git a/crates/ty_python_semantic/src/types/generics.rs b/crates/ty_python_semantic/src/types/generics.rs
index 79802a4fc3..c50b143f06 100644
--- a/crates/ty_python_semantic/src/types/generics.rs
+++ b/crates/ty_python_semantic/src/types/generics.rs
@@ -8,8 +8,8 @@ use ruff_python_ast as ast;
 use ruff_python_ast::name::Name;
 use rustc_hash::{FxHashMap, FxHashSet};
 
-use crate::semantic_index::definition::{Definition, DefinitionKind};
-use crate::semantic_index::scope::{FileScopeId, NodeWithScopeKind, NodeWithScopeRef, ScopeId};
+use crate::semantic_index::definition::Definition;
+use crate::semantic_index::scope::{FileScopeId, NodeWithScopeKind, ScopeId};
 use crate::semantic_index::{SemanticIndex, semantic_index};
 use crate::types::class::ClassType;
 use crate::types::class_base::ClassBase;
@@ -27,10 +27,9 @@ use crate::types::{
     ClassLiteral, FindLegacyTypeVarsVisitor, IntersectionType, KnownClass, KnownInstanceType,
     MaterializationKind, NormalizedVisitor, Type, TypeContext, TypeMapping,
     TypeVarBoundOrConstraints, TypeVarIdentity, TypeVarInstance, TypeVarKind, TypeVarVariance,
-    UnionType, declaration_type, walk_type_var_bounds,
+    UnionType, binding_type, declaration_type, walk_type_var_bounds,
 };
 use crate::{Db, FxOrderMap, FxOrderSet};
-use ruff_db::parsed::parsed_module;
 
 /// Returns an iterator of any generic context introduced by the given scope or any enclosing
 /// scope.
@@ -163,19 +162,14 @@ pub(crate) fn typing_self<'db>(
     //
     // and the first match would be (method, Outer) -- wrong.
     let containing_scope = typevar_binding_context
-        .and_then(|def| {
-            let DefinitionKind::Function(func_ref) = def.kind(db) else {
-                return None;
-            };
-            let module = parsed_module(db, file).load(db);
-            Some(index.node_scope(NodeWithScopeRef::Function(func_ref.node(&module))))
-        })
-        .unwrap_or_else(|| scope_id.file_scope_id(db));
+        .and_then(|def| binding_type(db, def).as_function_literal())
+        .map(|function| function.literal(db).last_definition(db).body_scope(db))
+        .unwrap_or(scope_id);
 
     bind_typevar(
         db,
         index,
-        containing_scope,
+        containing_scope.file_scope_id(db),
         typevar_binding_context,
         typevar,
     )

[charliermarsh]

I tried this but it hit me with a perf regression and a panic, so I backed out for now, but happy to revisit if it feels important!

[AlexWaygood]

The .node() call means we probably need to add #[salsa::tracked] to the function it's in, or i think we'll have over-invalidation of Salsa queries, which is bad for our incrementality

[charliermarsh]

(Will fix.)

[codspeed-hq]

CodSpeed Performance Report

Merging this PR will not alter performance

_{Comparing charlie/self-inner (2985b0b) with main (38d4f49)}

Summary

✅ 24 untouched benchmarks
⏩ 30 skipped benchmarks¹

Footnotes

1. 30 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩