[ty] Diagnostic for generic classes that reference typevars in enclosing scope #20822

dcreager · 2025-10-12T19:16:17Z

Generic classes are not allowed to bind or reference a typevar from an enclosing scope:

def f[T](x: T, y: T) -> None:
    class Ok[S]: ...
    # error: [invalid-generic-class]
    class Bad1[T]: ...
    # error: [invalid-generic-class]
    class Bad2(Iterable[T]): ...

class C[T]:
    class Ok1[S]: ...
    # error: [invalid-generic-class]
    class Bad1[T]: ...
    # error: [invalid-generic-class]
    class Bad2(Iterable[T]): ...

It does not matter if the class uses PEP 695 or legacy syntax. It does not matter if the enclosing scope is a generic class or function. The generic class cannot even reference an enclosing typevar in its base class list.

This PR adds diagnostics for these cases.

In addition, the PR adds better fallback behavior for generic classes that violate this rule: any enclosing typevars are not included in the class's generic context. (That ensures that we don't inadvertently try to infer specializations for those typevars in places where we shouldn't.) The dulwich ecosystem project has examples of this that were causing new false positives on #20677.

dcreager · 2025-10-12T19:18:44Z

crates/ty_python_semantic/src/types/generics.rs

    /// list.
    pub(crate) fn from_base_classes(
        db: &'db dyn Db,
+        definition: impl FnOnce() -> Definition<'db>,


See the comment below; this is passed in as a callback so that we don't have to query the class's definition unless it actually references legacy typevars in its base class list. Passing the definition in directly was causing some of the salsa query dependency tracking tests to fail.

That's probably worth an inline comment as it's non-obvious for future readers.

However, I do think what you did here is cheating the test rather than fixing the underlying issue. The problem is that calling definition adds a dependency on the ast node. The way you cheated the test is by early-returning in the case we're testing, a class with no type vars! But the same problem still exists for classes with type variables.

The more proper fix here is probably to make whatever calls from_base_classes to be behind a salsa query (at least, in positions where we can reach this call across file boundaries).

The more proper fix here is probably to make whatever calls from_base_classes to be behind a salsa query (at least, in positions where we can reach this call across file boundaries).

That was an easier real fix than I expected! Done

github-actions · 2025-10-12T19:21:31Z

Diagnostic diff on typing conformance tests

Changes were detected when running ty on typing conformance tests

--- old-output.txt	2025-10-13 20:39:45.649900870 +0000
+++ new-output.txt	2025-10-13 20:39:48.939910978 +0000
@@ -432,6 +432,8 @@
 generics_scoping.py:15:1: error[type-assertion-failure] Argument does not have asserted type `str`
 generics_scoping.py:42:1: error[type-assertion-failure] Argument does not have asserted type `str`
 generics_scoping.py:43:1: error[type-assertion-failure] Argument does not have asserted type `bytes`
+generics_scoping.py:65:11: error[invalid-generic-class] Generic class `MyGeneric` must not reference type variables bound in an enclosing scope: `T` referenced in class definition here
+generics_scoping.py:75:11: error[invalid-generic-class] Generic class `Bad` must not reference type variables bound in an enclosing scope: `T` referenced in class definition here
 generics_self_advanced.py:11:24: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@prop1`
 generics_self_advanced.py:18:1: error[type-assertion-failure] Argument does not have asserted type `ParentA`
 generics_self_advanced.py:19:1: error[type-assertion-failure] Argument does not have asserted type `ChildA`
@@ -896,5 +898,5 @@
 typeddicts_usage.py:28:17: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
 typeddicts_usage.py:28:18: error[invalid-key] Invalid key access on TypedDict `Movie`: Unknown key "title"
 typeddicts_usage.py:40:24: error[invalid-type-form] The special form `typing.TypedDict` is not allowed in type expressions. Did you mean to use a concrete TypedDict or `collections.abc.Mapping[str, object]` instead?
-Found 898 diagnostics
+Found 900 diagnostics
 WARN A fatal error occurred while checking some files. Not all project files were analyzed. See the diagnostics list above for details.

github-actions · 2025-10-12T19:22:40Z

`mypy_primer` results

Changes were detected when running on open source projects

nionutils (https://github.com/nion-software/nionutils)
+ nion/utils/Stream.py:552:15: error[invalid-generic-class] Generic class `AValueChangeStreamReactor` must not reference type variables bound in an enclosing scope: `T` referenced in class definition here
- Found 7 diagnostics
+ Found 8 diagnostics

dulwich (https://github.com/dulwich/dulwich)
+ dulwich/config.py:251:15: error[invalid-generic-class] Generic class `UniqueKeysView` must not reference type variables bound in an enclosing scope: `K` referenced in class definition here
+ dulwich/config.py:270:15: error[invalid-generic-class] Generic class `OrderedItemsView` must not reference type variables bound in an enclosing scope: `K` referenced in class definition here
+ dulwich/config.py:270:15: error[invalid-generic-class] Generic class `OrderedItemsView` must not reference type variables bound in an enclosing scope: `V` referenced in class definition here
- Found 184 diagnostics
+ Found 187 diagnostics

Memory usage changes were detected when running on open source projects

trio (https://github.com/python-trio/trio)
-     memo metadata = ~25MB
+     memo metadata = ~26MB

sphinx (https://github.com/sphinx-doc/sphinx)
-     memo metadata = ~42MB
+     memo metadata = ~44MB

prefect (https://github.com/PrefectHQ/prefect)
- TOTAL MEMORY USAGE: ~515MB
+ TOTAL MEMORY USAGE: ~541MB
-     memo metadata = ~89MB
+     memo metadata = ~93MB

github-actions · 2025-10-12T19:26:40Z

`ecosystem-analyzer` results

Lint rule	Added	Removed	Changed
`invalid-generic-class`	4	0	0
`no-matching-overload`	1	0	0
Total	5	0	0

Full report with detailed diff (timing results)

codspeed-hq · 2025-10-12T19:38:23Z

CodSpeed Performance Report

Merging #20822 will not alter performance

_{Comparing dcreager/inherited-legacy-base (8823fd1) with main (2b729b4)}

Summary

✅ 21 untouched
⏩ 30 skipped¹

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. ↩

AlexWaygood

Nice!

crates/ty_python_semantic/src/types/class.rs

crates/ty_python_semantic/src/types/infer/builder.rs

crates/ty_python_semantic/resources/mdtest/generics/scoping.md

MichaReiser

I'm surprised by the performance regression. This doesn't seem to add that much complexity. It might be worth taking a look at what's happening.

This reverts commit cae5c63.

AlexWaygood · 2025-10-13T14:06:33Z

...coping_rules_for_ty…_-_Nested_formal_typeva…_-_Generic_class_within…_(3259718bf20b45a2).snap

heh, this seems like a pre-existing bug in ClassLiteral::header_span() that it doesn't cover the type parameters

Yep I saw that too

crates/ty_python_semantic/src/types/diagnostic.rs

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood · 2025-10-13T14:26:22Z

I'm surprised by the performance regression. This doesn't seem to add that much complexity. It might be worth taking a look at what's happening.

It looks like this may have been fixed by adding the Salsa caching, though there is now a memory-usage regression being reported by mypy_primer

dcreager · 2025-10-13T14:30:37Z

though there is now a memory-usage regression being reported by mypy_primer

That is unsurprising, since we'll have a cached result for the newly cached function for every non-generic class. (And technically for every generic class that is generic via inheritance using legacy typevars) It's not a huge regression, so I would lean towards accepting it.

AlexWaygood

Nice!

crates/ty_python_semantic/src/types/class.rs

MichaReiser · 2025-10-13T20:18:28Z

crates/ty_python_semantic/src/types/class.rs

+    #[salsa::tracked(
+        cycle_fn=generic_context_cycle_recover,
+        cycle_initial=generic_context_cycle_initial,
+        heap_size=ruff_memory_usage::heap_size,
+    )]


Regarding the memory regression. Would it be possible to only cache the cases where we have a type var? Or is this already what this is doing :)

I don't think we can without walking the base class in the same way

MichaReiser · 2025-10-13T20:22:17Z

crates/ty_python_semantic/src/types/infer/builder.rs

+                        if let Some(other_typevar) =
+                            enclosing.binds_named_typevar(self.db(), self_typevar_name)
+                        {
+                            report_rebound_typevar(


Should we move the check whether the rule is enabled further up (to avoid the entire scope walking?)

It might require extracting the following into a lint_severity(name: &LintName) -> Option<Severity> helper

ruff/crates/ty_python_semantic/src/types/context.rs

Lines 404 to 431 in 2ce3aba

// The comment below was copied from the original

// implementation of diagnostic reporting. The code

// has been refactored, but this still kind of looked

// relevant, so I've preserved the note. ---AG

//

// TODO: Don't emit the diagnostic if:

// * The enclosing node contains any syntax errors

// * The rule is disabled for this file. We probably want to introduce a new query that

// returns a rule selector for a given file that respects the package's settings,

// any global pragma comments in the file, and any per-file-ignores.

if !ctx.db.should_check_file(ctx.file) {

return None;

}

let lint_id = LintId::of(lint);

// Skip over diagnostics if the rule

// is disabled.

let (severity, source) = ctx.db.rule_selection(ctx.file).get(lint_id)?;

// If we're not in type checking mode,

// we can bail now.

if ctx.is_in_no_type_check() {

return None;

}

// If this lint is being reported as part of multi-inference of a given expression,

// silence it to avoid duplicated diagnostics.

if ctx.is_in_multi_inference() {

return None;

}

Could also experiment with reviving #18327?

* main: (25 commits) [ty] Diagnostic for generic classes that reference typevars in enclosing scope (#20822) Update Python compatibility from 3.13 to 3.14 in README.md (#20852) [syntax-errors]: break outside loop F701 (#20556) [ty] Treat `Callable`s as bound-method descriptors in special cases (#20802) [ty] Do not bind self to non-positional parameters (#20850) Fix syntax error false positives on parenthesized context managers (#20846) [ty] Remove 'pre-release software' warning (#20817) Render unsupported syntax errors in formatter tests (#20777) [ty] Treat functions, methods, and dynamic types as function-like `Callable`s (#20842) [ty] Move logic for `super()` inference to a new `types::bound_super` submodule (#20840) [ty] Fix false-positive diagnostics on `super()` calls (#20814) [ty] Move `class_member` to `member` module (#20837) [`ruff`] Use DiagnosticTag for more flake8 and numpy rules (#20758) [ty] Prefer declared base class attribute over inferred attribute on subclass (#20764) [ty] Log files that are slow to type check (#20836) Update cargo-bins/cargo-binstall action to v1.15.7 (#20827) Update CodSpeedHQ/action action to v4.1.1 (#20828) Update Rust crate pyproject-toml to v0.13.7 (#20835) Update Rust crate anstream to v0.6.21 (#20829) Update Rust crate libc to v0.2.177 (#20832) ...

…rable * origin/main: (26 commits) [ty] Add separate type for typevar "identity" (#20813) [ty] Diagnostic for generic classes that reference typevars in enclosing scope (#20822) Update Python compatibility from 3.13 to 3.14 in README.md (#20852) [syntax-errors]: break outside loop F701 (#20556) [ty] Treat `Callable`s as bound-method descriptors in special cases (#20802) [ty] Do not bind self to non-positional parameters (#20850) Fix syntax error false positives on parenthesized context managers (#20846) [ty] Remove 'pre-release software' warning (#20817) Render unsupported syntax errors in formatter tests (#20777) [ty] Treat functions, methods, and dynamic types as function-like `Callable`s (#20842) [ty] Move logic for `super()` inference to a new `types::bound_super` submodule (#20840) [ty] Fix false-positive diagnostics on `super()` calls (#20814) [ty] Move `class_member` to `member` module (#20837) [`ruff`] Use DiagnosticTag for more flake8 and numpy rules (#20758) [ty] Prefer declared base class attribute over inferred attribute on subclass (#20764) [ty] Log files that are slow to type check (#20836) Update cargo-bins/cargo-binstall action to v1.15.7 (#20827) Update CodSpeedHQ/action action to v4.1.1 (#20828) Update Rust crate pyproject-toml to v0.13.7 (#20835) Update Rust crate anstream to v0.6.21 (#20829) ...

dcreager added 8 commits October 12, 2025 15:05

add test case

97feecb

don't include containing legacy typevars

7f62ddd

fix failing salsa dependency tests

cae5c63

check again

1865d0d

pre-commit

5476cd1

add error

2a6cb1e

clippy

5118548

snapshots

940c191

dcreager requested review from AlexWaygood, carljm and sharkdp as code owners October 12, 2025 19:16

dcreager added the ty Multi-file analysis & type inference label Oct 12, 2025

dcreager added 2 commits October 12, 2025 15:17

typo

bcd78c4

grr format macro

ba721fb

dcreager commented Oct 12, 2025

View reviewed changes

dcreager added the ecosystem-analyzer label Oct 12, 2025

snapshots

d90c207

AlexWaygood reviewed Oct 12, 2025

View reviewed changes

MichaReiser reviewed Oct 13, 2025

View reviewed changes

dcreager added 6 commits October 13, 2025 09:34

shorter error assertions in diagnostic tests

ae0f00a

indexset → orderset

5965ffe

default

6ca9f93

better spans for diagnostics

23f81f3

move into diagnostics.rs

10a1b54

Revert "fix failing salsa dependency tests"

e44582a

This reverts commit cae5c63.

AlexWaygood reviewed Oct 13, 2025

View reviewed changes

don't cheat on the test

570b30d

AlexWaygood reviewed Oct 13, 2025

View reviewed changes

crates/ty_python_semantic/src/types/diagnostic.rs Outdated Show resolved Hide resolved

Update crates/ty_python_semantic/src/types/diagnostic.rs

c526090

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood approved these changes Oct 13, 2025

View reviewed changes

crates/ty_python_semantic/src/types/class.rs Outdated Show resolved Hide resolved

walk

40c48cc

MichaReiser reviewed Oct 13, 2025

View reviewed changes

don't walk scopes if lint disabled

8823fd1

dcreager merged commit aba0bd5 into main Oct 13, 2025
41 checks passed

dcreager deleted the dcreager/inherited-legacy-base branch October 13, 2025 23:30

	// The comment below was copied from the original
	// implementation of diagnostic reporting. The code
	// has been refactored, but this still kind of looked
	// relevant, so I've preserved the note. ---AG
	//
	// TODO: Don't emit the diagnostic if:
	// * The enclosing node contains any syntax errors
	// * The rule is disabled for this file. We probably want to introduce a new query that
	// returns a rule selector for a given file that respects the package's settings,
	// any global pragma comments in the file, and any per-file-ignores.

	if !ctx.db.should_check_file(ctx.file) {
	return None;
	}
	let lint_id = LintId::of(lint);
	// Skip over diagnostics if the rule
	// is disabled.
	let (severity, source) = ctx.db.rule_selection(ctx.file).get(lint_id)?;
	// If we're not in type checking mode,
	// we can bail now.
	if ctx.is_in_no_type_check() {
	return None;
	}
	// If this lint is being reported as part of multi-inference of a given expression,
	// silence it to avoid duplicated diagnostics.
	if ctx.is_in_multi_inference() {
	return None;
	}

[ty] Diagnostic for generic classes that reference typevars in enclosing scope #20822

[ty] Diagnostic for generic classes that reference typevars in enclosing scope #20822

Uh oh!

Conversation

dcreager commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

github-actions bot commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

github-actions bot commented Oct 12, 2025

ecosystem-analyzer results

Uh oh!

codspeed-hq bot commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #20822 will not alter performance

Summary

Footnotes

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlexWaygood commented Oct 13, 2025

Uh oh!

dcreager commented Oct 13, 2025

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dcreager commented Oct 12, 2025 •

edited

Loading

github-actions bot commented Oct 12, 2025 •

edited

Loading

github-actions bot commented Oct 12, 2025 •

edited

Loading

`mypy_primer` results

`ecosystem-analyzer` results

codspeed-hq bot commented Oct 12, 2025 •

edited

Loading

MichaReiser Oct 13, 2025 •

edited

Loading

AlexWaygood Oct 13, 2025 •

edited

Loading