fix Incorrect invalid-type-var check when persisting get method of a fully-annotated dict #2812

[asukaminato0721]

Summary

Fixes #2812

The cause was: invalid-type-var sanitization already exempted generic Forall methods, but it was not exempting generic overload signatures carried inside Overload and bound-method overloads. I added that handling there, so storing dict[str, str].get on an instance attribute no longer falsely reports leaked _T.

Test Plan

add test

[Copilot]

Pull request overview

Fixes a false-positive invalid-type-var error when persisting a bound overload method (e.g., dict[str, str].get) onto an instance attribute, by ensuring type-variable sanitization treats overload-carried Forall type parameters as locally bound.

Changes:

• Extend invalid-type-var sanitization to collect type parameters bound by Forall overload signatures (both Type::Overload and BoundMethodType::Overload).
• Add a regression test covering assignment of dict[str, str].get to an instance attribute and verifying the resulting callable type.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
pyrefly/lib/alt/class/class_field.rs	Updates type-parameter collection to account for Forall tparams inside overload signatures during attribute type sanitization.
pyrefly/lib/test/attributes.rs	Adds regression test for assigning a bound overload (dict.get) to an attribute and asserting its inferred call types.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[meta-codesync]

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D97669915.

[rchen152]

Review automatically exported from Phabricator review in Meta.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

beartype (https://github.com/beartype/beartype)
- ERROR beartype/_util/cache/map/utilmapunbounded.py:113:14-31: Attribute `_key_to_value_get` cannot depend on type variable `_T`, which is not in the scope of class `CacheUnboundedStrong` [invalid-type-var]

scikit-learn (https://github.com/scikit-learn/scikit-learn)
- ERROR sklearn/cluster/_agglomerative.py:1309:14-26: Attribute `pooling_func` cannot depend on type variable `_ArrayT`, which is not in the scope of class `FeatureAgglomeration` [invalid-type-var]
- ERROR sklearn/cluster/_agglomerative.py:1309:14-26: Attribute `pooling_func` cannot depend on type variable `_ScalarT`, which is not in the scope of class `FeatureAgglomeration` [invalid-type-var]

setuptools (https://github.com/pypa/setuptools)
- ERROR setuptools/command/editable_wheel.py:454:14-19: Attribute `_file` cannot depend on type variable `_StrPathT`, which is not in the scope of class `_LinkTree` [invalid-type-var]
- ERROR setuptools/command/editable_wheel.py:454:14-19: Attribute `_file` cannot depend on type variable `_BytesPathT`, which is not in the scope of class `_LinkTree` [invalid-type-var]

pip (https://github.com/pypa/pip)
- ERROR src/pip/_vendor/rich/theme.py:89:14-17: Attribute `get` cannot depend on type variable `_T`, which is not in the scope of class `ThemeStack` [invalid-type-var]

zope.interface (https://github.com/zopefoundation/zope.interface)
- ERROR src/zope/interface/tests/test_adapter.py:95:14-17: Attribute `get` cannot depend on type variable `_T`, which is not in the scope of class `CustomMapping` [invalid-type-var]

rich (https://github.com/Textualize/rich)
- ERROR rich/theme.py:89:14-17: Attribute `get` cannot depend on type variable `_T`, which is not in the scope of class `ThemeStack` [invalid-type-var]

jinja (https://github.com/pallets/jinja)
- ERROR src/jinja2/environment.py:352:14-21: Attribute `filters` cannot depend on type variable `V`, which is not in the scope of class `Environment` [invalid-type-var]

sphinx (https://github.com/sphinx-doc/sphinx)
- ERROR sphinx/domains/__init__.py:134:14-31: Attribute `objtypes_for_role` cannot depend on type variable `_T`, which is not in the scope of class `Domain` [invalid-type-var]
- ERROR sphinx/domains/__init__.py:135:14-30: Attribute `role_for_objtype` cannot depend on type variable `_T`, which is not in the scope of class `Domain` [invalid-type-var]

[github-actions]

Primer Diff Classification

✅ 8 improvement(s) | 8 project(s) total | -9 errors

8 improvement(s) across beartype, scikit-learn, setuptools, pip, zope.interface, rich, jinja, sphinx.

Project	Verdict	Changes	Error Kinds	Root Cause
beartype	✅ Improvement	-1	invalid-type-var	pyrefly/lib/alt/class/class_field.rs
scikit-learn	✅ Improvement	-1	invalid-type-var	pyrefly/lib/alt/class/class_field.rs
setuptools	✅ Improvement	-1	invalid-type-var	collect_forall_tparams()
pip	✅ Improvement	-1	invalid-type-var	pyrefly/lib/alt/class/class_field.rs
zope.interface	✅ Improvement	-1	invalid-type-var	pyrefly/lib/alt/class/class_field.rs
rich	✅ Improvement	-1	invalid-type-var	pyrefly/lib/alt/class/class_field.rs
jinja	✅ Improvement	-1	invalid-type-var	pyrefly/lib/alt/class/class_field.rs
sphinx	✅ Improvement	-2	invalid-type-var	pyrefly/lib/alt/class/class_field.rs

Detailed analysis

✅ Improvement (8)

beartype (-1)

This is a clear false positive removal. dict.get has overloaded signatures with a generic type parameter _T for the default value. When you store self._key_to_value.get (where _key_to_value is dict[Hashable, object]), the bound method carries these overload signatures. Pyrefly was incorrectly flagging the _T in those overload signatures as an invalid type variable in the scope of CacheUnboundedStrong. The PR correctly extends the exemption to cover overloaded signatures inside both Type::Overload and BoundMethodType::Overload.
Attribution: The fix is in pyrefly/lib/alt/class/class_field.rs. The new collect_overload_tparams function (lines added around the collect_forall_tparams function) collects type parameters from overload signatures. The collect_forall_tparams function was updated to handle Type::Overload and BoundMethodType::Overload cases, adding their type parameters to the forall_bound set so they are exempted from the invalid-type-var check. Previously, only Type::Forall and BoundMethodType::Forall were exempted, missing the overloaded method case.

scikit-learn (-1)

This is a clear false positive removal. The FeatureAgglomeration class at line 1309 assigns self.pooling_func = pooling_func where pooling_func defaults to np.mean. np.mean has overloaded type signatures containing type variables like _ArrayT. These type variables are scoped to the function's overload signatures, not to the FeatureAgglomeration class. Pyrefly was incorrectly reporting that _ArrayT cannot depend on a type variable not in scope of the class, when in fact _ArrayT is properly scoped within the overloaded method signatures. The PR fix correctly extends the exemption logic to cover Overload and BoundMethodType::Overload types, matching the existing exemption for Forall types.
Attribution: The change in pyrefly/lib/alt/class/class_field.rs added the collect_overload_tparams helper function and extended collect_forall_tparams to handle Type::Overload and BoundMethodType::Overload cases. Previously, only Type::Forall and BoundMethodType::Forall were exempted from the invalid-type-var check. Since np.mean has overloaded signatures (its type is an Overload containing Forall variants), the type parameters within those overloaded signatures were not being collected into forall_bound, causing them to be incorrectly flagged. The fix ensures these type parameters are recognized as method-scoped and excluded from the class-scope check.

setuptools (-1)

This is a false positive removal. The copy_file method on Command (the base class of build_py) has a signature using _StrPathT as a method-level TypeVar (e.g., def copy_file(self, infile: StrPath, outfile: _StrPathT, ...) -> tuple[_StrPathT, bool]). When the bound method dist.get_command_obj("build_py").copy_file is stored as self._file in _LinkTree.__init__, pyrefly was incorrectly reporting that _StrPathT is not in the scope of class _LinkTree. However, _StrPathT is scoped to the method's own signature, not to any class — it's a method-level generic parameter. The inferred type of the bound method reference naturally contains this TypeVar, but it shouldn't trigger an invalid-type-var error on the attribute since the TypeVar is part of the callable's own generic signature, not a class-level type parameter that needs to be in scope. The PR correctly extends the exemption logic to handle this case where method-scoped TypeVars appear in bound method types stored as attributes.
Attribution: The change to collect_forall_tparams() in pyrefly/lib/alt/class/class_field.rs added handling for Type::Overload and BoundMethodType::Overload cases. Previously, the invalid-type-var sanitization only exempted Forall and BoundMethodType::Forall, but not overloaded signatures. The new collect_overload_tparams() helper function iterates over all signatures in an overload and collects their type parameters into the forall_bound set, preventing them from being falsely flagged as out-of-scope. This directly fixes the false positive on self._file = dist.get_command_obj('build_py').copy_file where copy_file has overloaded signatures containing _StrPathT.

pip (-1)

This is a clear false positive removal. dict.get is an overloaded generic method with its own type parameter _T. When a bound method self._entries[-1].get (where self._entries is list[dict[str, str]]) is assigned to self.get, the _T in the overloaded signatures is scoped to those signatures, not to ThemeStack. Pyrefly was failing to exempt overloaded method type parameters from the invalid-type-var check, and the PR fixes this by adding handling for Overload and BoundMethodType::Overload in the type parameter collection logic.
Attribution: The fix is in pyrefly/lib/alt/class/class_field.rs. The new collect_overload_tparams helper function iterates over overload signatures and collects their type parameters into the forall_bound set. Two new match arms were added: Type::Overload(overload) and BoundMethodType::Overload(overload), both delegating to this helper. Previously, only Type::Forall and BoundMethodType::Forall were handled, so overloaded methods (like dict.get) had their type parameters missed, causing the false invalid-type-var error. The test case test_bound_overload_assigned_to_attribute in pyrefly/lib/test/attributes.rs directly reproduces the pip/rich scenario.

zope.interface (-1)

This is a clear false positive removal. The dict.get method's type variable _T is scoped to the method's own generic overload signatures, not to the CustomMapping class. Pyrefly's invalid-type-var check was not accounting for type parameters in overloaded signatures carried by bound methods, causing it to incorrectly report that _T was out of scope for CustomMapping. The PR correctly extends the exemption logic to cover Overload and BoundMethodType::Overload variants.
Attribution: The change in pyrefly/lib/alt/class/class_field.rs added the collect_overload_tparams helper function and extended the collect_forall_tparams function to handle Type::Overload and BoundMethodType::Overload variants. Previously, only Type::Forall and BoundMethodType::Forall were exempted from the invalid-type-var check. The new code collects type parameters from overloaded signatures (both standalone Overload types and BoundMethodType::Overload), adding them to the forall_bound set so they are not falsely flagged as leaked type variables. This directly fixes the false positive on self.get = self._data.get in CustomMapping.

rich (-1)

This is a clear false positive removal. dict[str, Style].get is an overloaded method with signatures like get(key: str) -> Style | None and get(key: str, default: _T) -> Style | _T, where _T is the method's own type parameter. Storing this bound method on self.get should not trigger invalid-type-var because _T is scoped to the method's generic signatures, not to ThemeStack. The PR correctly extends the type parameter collection to cover overloaded and bound-method-overloaded types.
Attribution: The change to collect_forall_tparams in pyrefly/lib/alt/class/class_field.rs added handling for Type::Overload and BoundMethodType::Overload variants. Previously, only Type::Forall and BoundMethodType::Forall were handled, so when dict.get (which is an overloaded bound method) was assigned to self.get, its type parameters weren't collected as in-scope, causing the false invalid-type-var error. The new collect_overload_tparams helper iterates over all Forall signatures within an overload and adds their type parameters to the exemption set.

jinja (-1)

This is a clear false positive removal. The Environment class assigns self.filters = DEFAULT_FILTERS.copy() at line 352. DEFAULT_FILTERS is a dict, and .copy() returns a dict. The type variable V referenced in the error comes from the internal overloaded signatures of dict methods (e.g., dict.get has overloads parameterized by a type variable). Pyrefly was incorrectly flagging these internally-bound type variables as leaking into the Environment class scope. The PR correctly extends the exemption logic to cover Overload and BoundMethod(Overload(...)) types, matching the existing exemption for Forall types.
Attribution: The change in pyrefly/lib/alt/class/class_field.rs added the collect_overload_tparams helper function and extended collect_forall_tparams to handle Type::Overload and BoundMethodType::Overload variants. Previously, only Type::Forall and BoundMethodType::Forall were exempted from the invalid-type-var check. The new code collects type parameters from overloaded signatures (both standalone Overload types and BoundMethod wrapping an Overload), preventing them from being flagged as leaked type variables. This directly fixes the false positive on self.filters = DEFAULT_FILTERS.copy() where the dict's overloaded .get method carried type parameters that were incorrectly treated as free.

sphinx (-2)

The removed errors were false positives. self._role2type has type dict[str, list[str]], and self._role2type.get is a bound method whose type includes overloaded generic signatures (with a type parameter _T for the default value). Pyrefly was incorrectly flagging _T as escaping the class scope, but _T is scoped to the get method's own generic signature. The PR correctly fixes this by teaching the invalid-type-var sanitization to recognize type parameters inside Overload and BoundMethod(Overload(...)) types.
Attribution: The fix is in pyrefly/lib/alt/class/class_field.rs. The new collect_overload_tparams function (lines added) collects type parameters from overloaded signatures. The collect_forall_tparams function was updated to handle Type::Overload and BoundMethodType::Overload cases, which previously fell through to the default _ => {} branch. This means the type parameters from dict.get's overloaded signatures are now correctly recognized as method-scoped and excluded from the invalid-type-var check.

---

Was this helpful? React with 👍 or 👎

_{Classification by primer-classifier (8 LLM)}