mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Deny free type variables in instance attributes coming from constructor args

Open sterliakov opened this issue 1 year ago • 7 comments

Feature

Detect free type variables as instance attribute annotations, when they appear from constructor with TypeVar in signature without Generic.

Pitch

It would be especially useful for beginners. People often do not understand Generic necessity under some circumstances, though more experienced user may also accidentally forget about this.

This does not deny type variables in constructor arguments, if they do not become part of attributes annotations (say, __init__(self, x: _T, y: _T): do_something(x, y); super().__init__()).

Simple example (playground link):

from typing import TypeVar

_T = TypeVar('_T')

class Foo:
    def __init__(self, foo: _T, bar: _T):
        self.foo = foo
        self.bar = bar

f = Foo(1, 1)
reveal_type(f.foo)  # N: Revealed type is "_T`-1"

mypy already denies another scenario, which is essentially the same:

...
class Bar:
    x: _T  # E: Type variable "__main__._T" is unbound  [valid-type]

results in

main.py:15: error: Type variable "__main__._T" is unbound  [valid-type]
main.py:15: note: (Hint: Use "Generic[_T]" or "Protocol[_T]" base class to bind "_T" inside a class)
main.py:15: note: (Hint: Use "_T" in function signature to bind "_T" inside a function)

sterliakov avatar Dec 25 '22 20:12 sterliakov

_T in your first code sample is properly bound to __init__, so this isn't technically an error condition, at least not like the other errors you mentioned where the type variable is unbound. You make a good point that the author's intent was probably for _T to be bound to class Foo in this case. Perhaps a warning or a note would be more appropriate than an error.

You're correct that TypeVar scoping rules are confusing, especially for those who are new to generics in Python. PEP 695, if it's accepted, should go a long way toward eliminating this problem.

erictraut avatar Dec 26 '22 06:12 erictraut

@erictraut Could you explain what you mean? _T is bound in __init__, but ends up in foo and bar annotation. foo and bar can be accessed outside of __init__, ant at that point _T is free and completely pointless. It is no different from explicitly annotating foo: _T and bar: _T in a dataclass, isn't it?

(off-topic) Ough, I missed PEP695 somehow, it looks like python going the wrong way, too much typing-specific syntax, current TypeVar is awesome compared to that:(

sterliakov avatar Dec 26 '22 07:12 sterliakov

(off-topic) Ough, I missed PEP695 somehow, it looks like python going the wrong way, too much typing-specific syntax, current TypeVar is awesome compared to that:(

The PEP hasn't been accepted yet. I recommend you join the discussion at https://discuss.python.org/t/pep-695-type-parameter-syntax/21646 if you feel strongly on the topic :)

AlexWaygood avatar Dec 26 '22 10:12 AlexWaygood

The PEP hasn't been accepted yet. I recommend you join the discussion at https://discuss.python.org/t/pep-695-type-parameter-syntax/21646 if you feel strongly on the topic :)

I don't think that my aesthetic preferences like "I want python to remain python, dynamically typed and without lots of special typing syntax - it's what I hate Typescript for" will be taken into account:(

sterliakov avatar Dec 26 '22 11:12 sterliakov

It is no different from explicitly annotating foo: _T and bar: _T in a dataclass, isn't it?

I think these are different cases. Case 1: A reference to an unbound type variable within a type annotation. PEP 484 specifically indicates this is an error, as the type expression has no valid semantic meaning when the type variable has no associated (bound) scope. Case 2: A symbol whose declared or inferred type uses a type variable that remains "unsolved" when the symbol is referenced. This can occur for a number of reasons and is not generally treated as an error. A type checker can choose to leave it unsolved (as mypy does in your example) or convert it to Any.

_T = TypeVar("_T")

class Foo:
    a: _T # Error: Use of unbound type variable _T

    def method(self, x: _T): # Potential warning: Suspicious use of type variable because it occurs only once
        self.x = x  # Not Error: Valid use of type variable

reveal_type(Foo().x) # _T (bound to Foo.method) — no way for _T to be solved here

Thinking more about your proposal, I don't see any legitimate use case for an instance variable whose declared or inferred type includes a method-scoped type variable, so it is probably reasonable to treat this as an error. If this check is implemented, I'd suggest expanding it so it is not limited to constructors.

class Foo:
    def method(self, x: list[T]) -> T:
        self.foo = x  # This should probably be an error as well
        self.bar: list[T]  # This too

erictraut avatar Dec 26 '22 17:12 erictraut

The PEP hasn't been accepted yet. I recommend you join the discussion at https://discuss.python.org/t/pep-695-type-parameter-syntax/21646 if you feel strongly on the topic :)

I don't think that my aesthetic preferences like "I want python to remain python, dynamically typed and without lots of special typing syntax - it's what I hate Typescript for" will be taken into account:(

I don't share your perspective here, but I think the Steering Council very much wants to hear all points of view when considering PEPs like this, so I'd continue to encourage you to make your voice heard on the Discuss thread if you hold these opinions!

AlexWaygood avatar Dec 26 '22 17:12 AlexWaygood

I'd suggest expanding it so it is not limited to constructors.

I revisited this issue once again, here's one case related to example you show, which should not be flagged as error:

from typing import TypeVar

class A: pass

_T = TypeVar('_T', bound=A)

class Foo:
    some_a: A  # Without this line, the error should be flagged

    def method(self, a: _T) -> _T:
        self.some_a = a
        return a

a in method is still a TypeVar, but with proper upper bound, which is equal to (or, more generic, a subtype of) type of some_a. This assignment is perfectly valid: we can still resolve Foo().some_a as having type A, as declared, and we can be sure that assignment in method is valid thanks to the upper bound.

So, according to the proposed rule an assignment should be flagged as an error, iff a function-scoped type variable appears in (inferred or declared) type of any instance attribute. It should not be resolved to it's upper bound, unless there is declared concrete type compatible with it. It should not be resolved to union of variants, unless it is declared explicitly (see below).

from typing import TypeVar

class A: pass
class B: pass

_T = TypeVar('_T', A, B)

class Foo:
    foo: A | B # Without this line, the error should be flagged

    def method(self, a: _T) -> _T:
        self.foo = a  # This should be allowed, since all variants are compatible with declared union type
        return a

sterliakov avatar Dec 28 '22 11:12 sterliakov