astroid.nodes.node_classes.If.is_typing_guard() is very fragile

[matusvalo]

Steps to reproduce

Suppose project with module bar having TYPE_CHECKING constant. If any code is using this constant, astroid wrongly assumes that this part of code is part of typing.TYPE_CHECKING:

>>> node = astroid.extract_node('''
... from bar import TYPE_CHECKING
... if TYPE_CHECKING:
...     from xyz import a
... ''')
>>> node.is_typing_guard()
True

Current behavior

Astroid assumes any constant with name TYPE_CHECKING is typing guard.

Expected behavior

Only TYPE_CHECKING from typing module should be considered as typing guard

>>> node = astroid.extract_node('''
... from bar import TYPE_CHECKING
... if TYPE_CHECKING:
...     from xyz import a
... ''')
>>> node.is_typing_guard()
False

Additional notes

There are more less likely code snippets that won't be correctly recognized by astroid e.g.: if TYPE_CHECKING is True. Astroid should try to support at least the most likely ones - e.g. since TYPE_CHECKING is bool constant we should support at least if TYPE_CHECKING is True form.

python -c "from astroid import __pkginfo__; print(__pkginfo__.version)" output

master branch

[Pierre-Sassoulas]

Thank you for opening the issue, you're right.

[cdce8p]

I don't see this as an issue. Some arguments

• Is there any reason to define a variable called TYPE_CHECKING that should be used for anything other than type checking?
• Even if you do from bar import TYPE_CHECKING, you probably did from typing import TYPE_CHECKING in bar.
• Currently, there isn't an easy way to infer Compare nodes (i.e. if TYPE_CHECKING is True) as there is no infer method for it.
• Both, the Python and mypy docs, write only if TYPE_CHECKING:

https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING
https://mypy.readthedocs.io/en/stable/runtime_troubles.html#typing-type-checking

--
Do you have an actual code example were this is causing a problem?

[matusvalo]

Is there any reason to define a variable called TYPE_CHECKING that should be used for anything other than type checking?

I think this is wrong question. The problem is that package/module is a namespace by itself where author should be able to define any name (except reservered dunder names) without clashing other modules/packages. This is violated currently by astroid because you cannot use TYPE_CHECKING and this is wrong. This is more like include in C language.

Even if you do from bar import TYPE_CHECKING, you probably did from typing import TYPE_CHECKING in bar.

No, maybe I am not using typing module at all :-) and TYPE_CHECKING is my own global variable.

Currently, there isn't an easy way to infer Compare nodes (i.e. if TYPE_CHECKING is True) as there is no infer method for it.
Both, the Python and mypy docs, write only if TYPE_CHECKING:

The docs of typing module says about TYPE_CHECKING following: A special constant that is assumed to be True. So valid is also expression TYPE_CHECKING is True. Hence in theory we should support all cases. But this not doable in real world so we should define what statement we support and what not. If the discussion ends with if TYPE_CHECKING, I am totally fine with that.

[cdce8p]

TYPE_CHECKING

We don't have that many options here. The current check is below. Unfortunately, I don't know of any possibility to get to the module name itself. The only option would be to use infer or lookup, but both of those will give use a Const (bool) node.
https://github.com/PyCQA/astroid/blob/f236e36729265602143ace2d969b44e53e4d57c1/astroid/node_classes.py#L3516-L3518

The question I have is, is it really necessary to add anything extra there? is_typing_guard is only used by pylint to disable some import checks in a typing import block. Nothing more. You won't per se lose anything if it's wrongly assumed to be TYPE_CHECKING.

if TYPE_CHECKING

The idea, especially with the context that anything other is not easily possible at the moment, is to only support

if TYPE_CHECKING:
    ...

and nothing more. As I mentioned, this is the recommended way to use the constant and thus easy to justify. If at some point someone likes to redo the pylint documentation (there are a few open issues about that I believe), maybe we can include a note about that.

[jacobtylerwalls]

I just looked and astroid's is_typing_guard() is already deprecated, so I don't think improvements are to be recommended at this point.

pylint currently has two versions of the util. One is already more robust against indentation (i.e. multiple ancestors), and pylint-dev/pylint#6787 will make it even more robust. This is the util used for import-error. Other checkers might be using the less robust util, currently, however.