Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mypy reports used-before-def with TypeAliases despite from __future__ import annotations #14539

Open
bersbersbers opened this issue Jan 27, 2023 · 10 comments
Assignees
Labels
bug mypy got something wrong topic-possibly-undefined possibly-undefined error code

Comments

@bersbersbers
Copy link

bersbersbers commented Jan 27, 2023

Bug Report

from __future__ import annotations helps avoid NameErrors with later-defined entities in python, but seems to ineffective with use(d)-before-def in mypy.

To Reproduce (edited example code, see edit history for previous version if I mis-edited anything):

"""Bug."""
from __future__ import annotations
from typing import TYPE_CHECKING

# This works (only) with `from __future__ import annotations`:
list_direct: list[Class] = []

# By contrast, mypy reports `used-before-def` either way:
if TYPE_CHECKING:
    ListOfClass = list[Class]
list_via_alias: ListOfClass = []

class Class:
    pass

Expected Behavior

No error

Actual Behavior

bug.py:8: error: Name "MyClass" is used before definition [used-before-def]

Your Environment

  • Mypy version used: mypy 1.0.0+dev.425fb0b4cfaa78cd0bf23fd165e4d6a1170670fe (compiled: no)
  • Mypy command-line flags: none
  • Mypy configuration options from mypy.ini (and other config files): none
  • Python version used: 3.11.1

Related: #14163, #14166

Interestingly, no error on playground on master:
https://mypy-play.net/?mypy=master&python=3.11&gist=f323516535c0d04cf4e0a2f2c985f4ff

@bersbersbers bersbersbers added the bug mypy got something wrong label Jan 27, 2023
@AlexWaygood AlexWaygood added the topic-possibly-undefined possibly-undefined error code label Jan 27, 2023
@AlexWaygood AlexWaygood mentioned this issue Jan 27, 2023
17 tasks
@erictraut
Copy link

I think mypy's behavior is correct here. The from __future__ import annotations affects deferred evaluation of type annotations only. It does not affect evaluation of other expressions such as the RHS of the assignment statement you are using to define FunctionOfMyClass.

@AlexWaygood
Copy link
Member

I think mypy's behavior is correct here. The from __future__ import annotations affects deferred evaluation of type annotations only. It does not affect evaluation of other expressions such as the RHS of the assignment statement you are using to define FunctionOfMyClass.

I think that's arguable — in an ideal world, perhaps mypy would never have allowed forward references in runtime contexts in .py files. Given that it has allowed constructs like this for a long time, however, there may be many users who have forward references like this in if TYPE_CHECKING blocks, implicitly depending on mypy's previous behaviour. Disallowing constructs like this now is potentially a serious backwards-compatibility issue, and (since if TYPE_CHECKING blocks are never executed at runtime), has dubious benefits in this case.

@bersbersbers
Copy link
Author

bersbersbers commented Jan 28, 2023

In my (outsider's) opinion, @erictraut is not wrong, technically. But I'd raise the question what mypy should do here, not what it currently does.

Ultimately, both my uses cases above are for type annotations only, even though one of the two cases passes via definition of a helper TypeAlias. (Edit: I edited the original code to highlight this only difference.) Semantically, I feel these two use cases should work the same.

@bersbersbers bersbersbers changed the title mypy throws use-before-def despite from __future__ import annotations mypy reports used-before-def with TypeAliases despite from __future__ import annotations Jan 29, 2023
@bersbersbers
Copy link
Author

bersbersbers commented Jan 30, 2023

Another point to take into account: pylama/pyflakes/flake8 as well as Pylance report this (as E0602, F821, undefinedVariable, respectively), so it is somewhat less likely that mypys behavior will be hindering many projects. I guess ListOfClass = list["Class"] may be the way to go for now whenever any of the aforementioned linters are involved.

@tabatkins
Copy link

Just updated mypy from 0.960 to 1.3.0 and hit this, and wow this is not correct behavior.

Mypy handles unresolved references just fine; it's the Python runtime that chokes on them. If the reference is in a TYPE_CHECKING conditional, thus specifically hidden from the Python runtime and only meant for mypy, there is thus absolutely no problem with referencing something before it's defined. Mypy knows what you're doing, and Python has no idea it exists; this is manifestly not an error and flagging it as such (and requiring the annoying "wrap it in a string" forward-reference pattern) is user-hostile. And mypy knows about TYPE_CHECKING conditionals, so it can absolutely condition its checks on this.

(Unresolved references outside of a TYPE_CHECKING conditional are clearly an error and mypy would be correct to flag them, as they would cause Python to choke as well.)

In the meantime I'm just going to disable the used-before-def error entirely, which is unfortunate, because this is a useful error to catch. Luckily pylint is also catching this particular error (and I've already had to mark the offending type alias lines to not trigger a pylint error) so I'm not losing any actual coverage.

@erictraut
Copy link

The TYPE_CHECKING conditional is meant as a way to "lie" to the type checker about what is seen at runtime. Because of this, it should be used with great care — and avoided if possible.

Mypy is behaving correctly in this case. This is not a bug. Recommend closing.

@bersbersbers
Copy link
Author

TYPE_CHECKING [...] should be [...] avoided if possible.

Wow, that is news to me. I thought it would make sense to put as much as possible into type checking blocks, in line with the following linter rules that I use:

TC001 | Move application import into a type-checking block
TC002 | Move third-party import into a type-checking block
TC003 | Move built-in import into a type-checking block

https://github.com/snok/flake8-type-checking#error-codes

@erictraut
Copy link

Yikes, those look to me like very bad linting rules. Every time you use TYPE_CHECKING in your code, you're "blinding" the type checker to potential runtime errors because you're telling it to analyze code that is different from what is executed at runtime. In certain rare cases, the use of TYPE_CHECKING is unavoidable, but I recommend avoiding it wherever possible.

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 14, 2023

There are a few reasons why you might want to move as much code as possible inside an if TYPE_CHECKING block:

  1. To minimise the risk of import cycles
  2. To reduce the performance cost associated with importing expensive modules that are only needed for type annotations
  3. To ensure that as little time as possible is spent evaluating annotations at runtime

I agree that it leads to worse type-checking, and I also personally try to use if TYPE_CHECKING blocks as little as possible. However, I can see the motivation behind these rules -- some libraries have very high import costs associated with them; in some code bases it's very difficult to avoid import cycles without guarding imports behind if TYPE_CHECKING blocks; etc.

I think we can be pragmatic and recognise that different people are making different trade-offs in the way they choose to write their code.

@tabatkins
Copy link

The TYPE_CHECKING conditional is meant as a way to "lie" to the type checker about what is seen at runtime. Because of this, it should be used with great care — and avoided if possible.

This is false, weakly in theory but strongly in practice. The Python runtime has a number of behaviors that are actively hostile to type-checking, and which aren't issues in languages that have robust type systems built in - the most important one is circular imports which is impossible to resolve without it, but @AlexWaygood lists several additional important ones. Without TYPE_CHECKING blocks I'd have abandoned mypy fairly quickly, because at best it would have required drastically restructuring my code base for circular-import reasons, and likely would have simply been fundamentally incompatible.

Keeping their use to a minimum is definitely correct practice (using them solely for the purpose of establishing names that the type system needs but the runtime doesn't), but they're not an escape hatch, they're a required feature of the type system due to the fundamental disconnect between Python's module system and a good type system's requirements for a module system.


It wouldn't be unreasonable to, say, flag uses of a TypeAlias variable in runtime code as errors. If said variable is defined in a TYPE_CHECKING block it would actively be helpful, since that is guaranteed to be a ReferenceError at runtime. But saying that one use of a forward-referenced name in code not executed by the runtime (in an annotation) is fine, while another use in code not executed by the runtime (in TYPE_CHECKING) isn't, is just wrong. It's just a beta reduction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-possibly-undefined possibly-undefined error code
Projects
None yet
Development

No branches or pull requests

5 participants