Skip to content

Unsoundness from unbound type variables in attributes #6520

Closed

Description

@msullivan

Minimized from some real code at dropbox exhibiting this failure in a nasty way.

The following code typechecks:

from typing import TypeVar, Generic

T = TypeVar('T')
class A:
    def __init__(self, x: T) -> None:
        self.x = x
    def get(self) -> T:
        return self.x

x: int = A('lol').get()

Here, T is not bound in the class but ends up in the attribute, where it is then matched with the unbound return type variable in get.
(Interestingly, it also works if get returns a different unbound type variable)

A reveal type reports: Revealed type is 'T`-1'

Activity

  1. ilevkivskyi commented on Mar 6, 2019

    @ilevkivskyi
    Member

    This is a consequence of the fact that we allow methods to be generic "on their own", i.e. this is perfectly valid:

    T = TypeVar('T')
    class C:
        def meth(self, x: T) -> T: ...
    

    But then there is #2885.

    Note there is similar (but not directly related) #3172 that proposes to reject use of class type variables in method bodies of static methods. In general, we might want to perform some overhaul of type variable scopes and clearly document the rules. Currently it often works how it works.

  2. msullivan commented on Mar 6, 2019

    @msullivan
    CollaboratorAuthor

    It seems more that the problem is the treatment of the type variable's interaction with the attribute?

  3. ilevkivskyi commented on Mar 6, 2019

    @ilevkivskyi
    Member

    Yes, probably the simplest solution would be to prohibit storing such type on self attribute, but it may be not as easy as it looks, because type variable scopes are not accessible it type checker, while the assignment is analyzed. There is a hacky way of just rejecting all type variables with negative ids (all variables bound by functions have negative ids, while those bound by class have positive ids). This however looks fragile.

    In general, as I already said we might want to re-think how type variable binding works in general (maybe at least store a pointer to the binder for every TypeVarType, currently we just copy id and other things from the binder in TypeVarType constructor).

  4. ilevkivskyi commented on Nov 7, 2019

    @ilevkivskyi
    Member

    It turns out this is a duplicate of an older issue #1903, but I am going to close the other one, since it has less discussion.

  5. pirate commented on Nov 21, 2019

    @pirate

    Apologies if this is off-topic, but is there any way to use TypeVars as class attributes that can be overwritten by subclasses, but have them resolve within the context of the subclass when checking?

    How would one go about doing something like this?

    TypeForAPI = TypeVar('TypeForAPI')
    TypeForDB = TypeVar('TypeForDB')
    
    class Parser:
        type_for_api: TypeForAPI = str
        type_for_db: TypeForDB = str
    
        @classmethod
        def dump_to_db(cls, value: TypeForAPI) -> TypeForDB:
            return cls.type_for_db(value)
    
        @classmethod
        def dump_to_api(cls, value: TypeForDB) -> TypeForAPI:
            return cls.type_for_api(value)
    
    
    class NumberParser(Parser):
        type_for_db: TypeForDB = float
        type_for_api: TypeForAPI = Decimal
    
    class DateParser(Parser):
        type_for_db: TypeForDB = datetime.date
        type_for_api: TypeForAPI = str
    
        @classmethod
        def dump_to_db(cls, value: TypeForAPI) -> TypeForDB:
            return dateutil.parse(value).date()
    
        @classmethod
        def dump_to_api(cls, value: TypeForDB) -> TypeForAPI:
            return value.isoformat()

    Appears to be superficially related to #3625 (I'm having the same issue with typevars on metaclasses elsewhere in the same file that example was taken from ^, but it could be a red herring).

  6. hauntsaninja commented on Dec 28, 2024

    @hauntsaninja
    Collaborator

    Ivan fixed this a few releases ago.

    I think pirate's example is unrelated. You can maybe accomplish this by making Parser generic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions