Fix false positive for super() call with constrained type variable - #21931
Open
Ananthr16 wants to merge 1 commit into
Open
Fix false positive for super() call with constrained type variable#21931Ananthr16 wants to merge 1 commit into
Ananthr16 wants to merge 1 commit into
Conversation
expand_typevars checks a method with a value-restricted type variable once per constraint value, substituting the value into a copy of the function body. super() calls inside that body were still resolved against the original, unsubstituted self type, since the checker scope always holds the original function, not the substituted copy. This made the expected argument type for an inherited member show the literal type variable instead of the value being checked for this pass, producing a spurious argument-type mismatch on every call. expand_typevars now also returns the substitution mapping for each copy, and check_func_def uses it to compute the correctly substituted self/cls type while checking that copy's body. super() consults this type instead of recomputing an unsubstituted one from the class. Fixes python#17757. Fixes python#14774.
Contributor
|
Diff from mypy_primer, showing the effect of this PR on open source code: anyio (https://github.com/agronholm/anyio)
+ src/anyio/_core/_tempfile.py:343: error: Unused "type: ignore" comment [unused-ignore]
+ src/anyio/_core/_tempfile.py:357: error: Unused "type: ignore" comment [unused-ignore]
+ src/anyio/_core/_tempfile.py:364: error: Unused "type: ignore" comment [unused-ignore]
+ src/anyio/_core/_tempfile.py:424: error: Unused "type: ignore" comment [unused-ignore]
+ src/anyio/_core/_tempfile.py:424: error: Argument 1 to "write" of "AsyncFile" has incompatible type "Buffer | str"; expected "str" [arg-type]
+ src/anyio/_core/_tempfile.py:424: note: Error code "arg-type" not covered by "type: ignore[misc]" comment
+ src/anyio/_core/_tempfile.py:424: error: Argument 1 to "write" of "AsyncFile" has incompatible type "Buffer | str"; expected "Buffer" [arg-type]
+ src/anyio/_core/_tempfile.py:452: error: Unused "type: ignore" comment [unused-ignore]
+ src/anyio/_core/_tempfile.py:452: error: Argument 1 to "writelines" of "AsyncFile" has incompatible type "Iterable[str] | Iterable[Buffer]"; expected "Iterable[str]" [arg-type]
+ src/anyio/_core/_tempfile.py:452: note: Error code "arg-type" not covered by "type: ignore[misc]" comment
+ src/anyio/_core/_tempfile.py:452: error: Argument 1 to "writelines" of "AsyncFile" has incompatible type "Iterable[str] | Iterable[Buffer]"; expected "Iterable[Buffer]" [arg-type]
xarray (https://github.com/pydata/xarray)
+ xarray/core/resample.py:61: error: Unused "type: ignore" comment [unused-ignore]
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17757.
Fixes #14774.
Problem
super().<method>(...)inside a method of a class with a value-restricted (constrained) type variable raises a false-positivearg-typeerror, even when the argument's type matches the constrained type variable exactly:Same root cause with a distinct subclass type variable bound through the base class (#17757's repro):
Root cause
checker.py'sexpand_typevarstype-checks a method with a value-restricted type variable once per constraint value, producing a copy of the function viaexpand_funcwith that value substituted everywhere it appears in the function's own AST (parameter types, local variable types, etc.).The self/cls argument is usually unannotated, so its
Var.typeisNone-- there's nothing there forexpand_func's substitution to rewrite. Whencheckexpr.py's_super_arg_typesresolves a zero-argumentsuper(), it falls back tofill_typevars(e.info), which recomputes the self type fresh from the class'sTypeInfo-- entirely unaware of which constraint value is currently being checked.analyze_member_accessthen substitutes the base class's generic parameter using this unsubstituted self type, so the expected argument type for the inherited method stays the literal, unsubstituted type variable (T/N), while the actual argument (correctly substituted byexpand_func) is a concrete member type (float/int). Hence the mismatch, on every one of the type variable's constraint values.(There's also a separate, subtler version of the same gap: even an explicitly annotated self argument's substituted type is inaccessible from
_super_arg_types, becausecheck_func_defpushes the originaldefnonto the checker scope for body-checking -- not the substituted copyexpand_typevarsproduced -- soself.chk.scope.current_function()can never see the substitution either way.)Fix
expand_typevarsnow also returns the type variable substitution mapping alongside each(FuncItem, CallableType)pair it produces.check_func_defuses that mapping to compute the correctly self/cls type for the copy currently being checked (TypeChecker.self_type_for_expansion), and stores it on a newself.expanding_self_typeattribute for the duration of checking that copy's body._super_arg_typesconsultsself.chk.expanding_self_typefirst, before falling back to the existing (and, for this scenario, stale) checks.This is scoped narrowly to the value-restricted-type-variable expansion path:
expanding_self_typeisNonewhenever there's no substitution mapping (i.e. for the overwhelming majority of methods, which don't have constrained type variables), so_super_arg_typesfalls through to its existing behavior unchanged in that case.Testing
Added three regression cases to
check-generics.test:testConstrainedGenericSuperNoFalsePositiveSameTypeVar-- False positive error "incompatible type" calling super() on a generic class with TypeVar constraints #14774's repro.testConstrainedGenericSuperNoFalsePositiveDistinctTypeVar-- Generic subclass of a generic class, using a constrained TypeVar is mishandled #17757's repro (subclass type variable bound through the base).testConstrainedGenericSuperClassmethodNoFalsePositive-- same scenario through a classmethod, to cover thecls/TypeTypebranch of the self-type computation.I also re-verified the existing
testConstrainedGenericSupertest, which intentionally passes mismatched concretestr/bytesliterals to a method expecting a single constrained type variable for both parameters (a genuine error, unrelated to this bug). That test's expected output needed updating: previously mypy reported both arguments asexpected "AnyStr"(the literal, unsubstituted type variable name) for both of the two expansion passes; with this fix the error message correctly reports the concrete type expected in each pass (expected "bytes"in one pass,expected "str"in the other), which is strictly more informative and still correctly flags the call as an error.python -m mypy --config-file mypy_self_check.ini -p mypy: clean.python runtests.py check-generics.test check-typevar-values.test check-generic-subtyping.test: all pass (199/54/54 respectively, plus skips).python runtests.py pytest-fast: 12849 passed, 351 skipped, 8 xfailed -- no regressions.python runtests.py pytest-slow: 37 failed / 18 passed, but identical failure set on unmodifiedmain(daemon/socket tests and a mypyc C-unit-test build step, both environment-dependent in this sandbox, unrelated to this change) -- confirmed via a side-by-side run.black --check/ruff checkon the two changed.pyfiles: clean.Out of scope
There's a third, related-looking issue, #13566, with a similarly-worded false positive from an explicit
Base.__init__(self)call (rather thansuper()) on a class with a plain, unconstrained type variable. I checked it against this fix and it's unaffected -- different mechanism, no value-restricted type variable involved. Left it alone rather than trying to fold it into this PR.