pytype icon indicating copy to clipboard operation
pytype copied to clipboard

Class instance assignment to a `ClassVar` via `self` is not flagged

Open houglum opened this issue 4 years ago • 1 comments

Looking at PEP 526, it mentions that:

"[...] it is useful to distinguish [class variables] by marking class variables as annotated with types wrapped in ClassVar[...]. In this way a type checker may flag accidental assignments to attributes with the same name on instances."

Initially, I was curious whether Pytype would also catch mutations, not just assignments, using the provided example code. While checking this, I noticed that even instance member assignments (e.g. self.attr_supposed_to_be_classvar = 3) are not flagged as errors by Pytype. Here is the example code I ran that Pytype detected no errors in -- it performs assignments and mutations to a class instance's attributes that have names already set as ClassVars on that class:

from typing import ClassVar, List


class Foo(object):

  # Make these `ClassVar`s; we want them to be shared across all instances, not
  # for each instance to have its own copy.
  classvar_list: ClassVar[List[int]] = [1, 10]
  classvar_str: ClassVar[str] = 'Value from ClassVar declaration'

  def __init__(self):
    self.instance_int: int = 1


def TryAssignmentToClassVarFromInstance(my_foo: Foo) -> None:
  # Sanity check: These should definitely fail according to PEP 526, but they
  # don't!
  my_foo.classvar_list = []
  my_foo.classvar_str = 'Value assigned from class instance! Uh oh!'


def TryMutatingClassVarFromInstance(my_foo: Foo) -> None:
  # I was originally curious if mutations (rather than outright assignments)
  # would fail:
  my_foo.classvar_list.append(99)


def main() -> None:
  local_foo: Foo = Foo()
  # Changing an instance variable is OK, as expected:
  local_foo.instance_int = 2

  # Changing class variables via a class instance is not supposed to be OK:
  TryAssignmentToClassVarFromInstance(local_foo)
  TryMutatingClassVarFromInstance(local_foo)


if __name__ == '__main__':
  main()

Takeaways:

  • The code in TryAssignmentToClassVarFromInstance should certainly be flagged according to PEP 526, but Pytype allows it.
  • I'm not sure whether the code in TryMutatingClassVarFromInstance should fail, or whether it fails under other type checkers.

houglum avatar Oct 13 '21 01:10 houglum

Huh, I searched through the pytype code, and it looks like we don't support ClassVar at all; we ignore it entirely and just grab the contained type. I'm surprised this hasn't come up before.

rchen152 avatar Oct 13 '21 20:10 rchen152