Attribute access with untyped methods and --check-untyped-defs

[srittau]

Using mypy 0.620 and Python 3.7.0 I see strange behavior with descriptors in untyped methods. For example:

from typing import Any

class Getter:
    def __get__(self, obj: Any, _: Any = None) -> int:
        return 4

class Foo:
    x = Getter()

    def foo(self):
        reveal_type(self.x)

    def bar(self) -> None:
        reveal_type(self.x)

Running with mypy --check-untyped-defs I get the following output:

foo.py:11: error: Revealed type is 'Any'
foo.py:14: error: Revealed type is 'builtins.int'

Is this intended?

[gvanrossum]

Yes, mypy doesn't type-check unannotated methods, but it still processes reveal_type() there.

[srittau]

@gvanrossum Please note that I called mypy with --check-untyped-defs! When calling it without this option, I get a warning about reveal_type always returning Any.

[gvanrossum]

Oh, sorry.

[bdarnell]

This isn't just for descriptors; it happens for plain attributes too:

import unittest

class FooTest(unittest.TestCase):
    def setUp(self):
        self.x = 1

    def test_one(self):
        reveal_type(self.x)

    def test_two(self) -> None:
        reveal_type(self.x)

$ mypy --check-untyped-defs /tmp/test.py
/tmp/test.py:8: error: Revealed type is 'Any'
/tmp/test.py:11: error: Revealed type is 'builtins.int'
$ mypy --version
mypy 0.620

[gvanrossum]

So should the subject just be "reveal_type() in untyped methods with --check-untyped-defs incorrectly shows Any"?

[bdarnell]

It's not just reveal_type. If we replace reveal_type with an actual type error, it will only be reported in the annotated method:

import unittest

class FooTest(unittest.TestCase):
    def setUp(self):
        self.x = 1

    def test_one(self):
        self.x + 'a'

    def test_two(self) -> None:
        self.x + 'a'  # line 11

$ mypy --check-untyped-defs /tmp/test.py
/tmp/test.py:11: error: Unsupported operand types for + ("int" and "str")

[srittau]

Yes, I also only used reveal_type to show the problem. I get errors for returning Any from untyped methods that go away as soon as I annotate the function definition.

[gvanrossum]

So is —check-I typed-refs totally broken then? Maybe that should be the topic.

[bdarnell]

It'll report some errors, so not totally broken. It looks like the difference is in the handling of self. In an annotated method, it is given the type of the class in which it is defined, but in an untyped method self is given the type Any just like any other argument.

[JukkaL]

The Any type for self may be by design (to avoid false positives), but it clearly seems surprising. At least it should be clearly documented.

[gvanrossum]

Since this behavior is enabled explicitly by a flag (--check-untyped-defs) I don't believe the Any type for self was by design -- more likely we forgot to special-case self, since we do (reasonably) use Any for the type of all other arguments.

[srittau]

I have analyzed this problem a bit and solution seems non-straightforward. Currently for annotated methods the argument type of self is adjusted in the semantic analyzer:

mypy/mypy/semanal.py

Lines 488 to 501 in e5e4532

	def prepare_method_signature(self, func: FuncDef, info: TypeInfo) -> None:
	"""Check basic signature validity and tweak annotation of self/cls argument."""
	# Only non-static methods are special.
	functype = func.type
	if not func.is_static:
	if not func.arguments:
	self.fail('Method must have at least one argument', func)
	elif isinstance(functype, CallableType):
	self_type = functype.arg_types[0]
	if isinstance(self_type, AnyType):
	leading_type = fill_typevars(info) # type: Type
	if func.is_class or func.name() in ('__new__', '__init_subclass__'):
	leading_type = self.class_type(leading_type)
	func.type = replace_implicit_first_type(functype, leading_type)

It's possible to add a type to unannotated methods here as well, but that wreaks havoc with later checks for unannotated or partially annotated methods when using --disallow-untyped-defs or --disallow-incomplete-defs.

The checker later constructs a type for functions and methods that don't have one yet:

mypy/mypy/checker.py

Lines 755 to 765 in e5e4532

	def check_func_item(self, defn: FuncItem,
	type_override: Optional[CallableType] = None,
	name: Optional[str] = None) -> None:
	"""Type check a function.
	If type_override is provided, use it as the function type.
	"""
	self.dynamic_funcs.append(defn.is_dynamic() and not type_override)
	with self.enter_partial_types(is_function=True):
	typ = self.function_type(defn)

As far as I can see the class type is not available in this method, so it needs to be injected somehow. But I don't like the fact that the self type gets injected in two completely separate locations, depending on whether the method is annotated at all or not.

I see three possible solutions:

1. Extract the self type injection from the semantic analyzer and keep calling it from the analyzer for (partially) annotated methods and call it from the type checker for unannotated ones.
2. Synthesize type information for all unannotated functions and methods in the semantic analyzer and flag them as unannotated. This requires to change all checks for func.type in the type checker to use this new flag. In my limited understanding of mypy internals this is the "correct" fix, but also quite risky.
3. Move self type injection to the type checker and never alter the actual FuncDef instances.

[kylebebak]

For code bases with lots of classes (e.g. Django code bases) this is really misleading.

You pass --check-untyped-defs and think your code is being type checked, but a huge portion of it isn't.

If you annotate any of a function's args or its return type, self correctly resolves to the type of its class. Would it not be feasible to have mypy do this for all untyped functions when passing the --check-untyped-defs flag, for example by adding -> Any to all of these functions behind the scenes before analyzing the code?

IMO as is --check-untyped-defs is totally broken.

[gvanrossum]

@kylebebak It took me a few tries to understand what you're proposing, but basically you're saying that with --check-untyped-defs, mypy should use the same heuristic to determine the type of self (and cls, for class methods) that it uses for annotated functions, and that seems reasonable (and is also what @srittau has been saying). I now concur.

Alas, I am not very familiar with this part of mypy, and I have no idea how easy it would be to implement any of @srittau's proposals. I'm setting the priority to "normal", hopefully @JukkaL or @ilevkivskyi will be able to help.

[kylebebak]

Sorry if that wasn't as clear as it could have been... And yes, that's exactly what I'm proposing.

Hopefully it's not too difficult to change this behavior!

[gvanrossum]

See @srittau’s explanation above. -- --Guido (mobile)

[kylebebak]

According to @srittau , it would be simple to modify prepare_method_signature so that cls and self are typed for unannotated methods as well, the drawback being that this would break the --disallow-untyped-defs and --disallow-incomplete-defs checks.

If this is the only drawback to this solution, and other solutions that allow --check-untyped-defs and --disallow-untyped-defs to function simultaneously are much more costly, then I'd propose we go with the first solution.

We just add a warning to mypy that explains --check-untyped-defs invalidates the disallow flags if they're passed together. We could also add this explanation to the config section of the docs.

The reasoning being that the purpose of one runs sort of contrary, or at least perpendicular, to the purpose of the other. --check-untyped-defs gets you type checking on unannotated methods, which is really important for code intelligence and catching bugs in real time, while the disallow flags are more for enforcing code standards, and can be run somewhere in the CI pipeline.

I don't think it's very important that they work together, and as we've all sort of agreed, currently --check-untyped-defs doesn't work period. Fixing --check-untyped-defs even if it can't be used together with the disallow flags seems like a definite improvement.