TYPE_CHECKS_GUARDS fooled by import alias

[mscuthbert]

Bug description

# pylint: disable=missing-module-docstring,missing-class-docstring,too-few-public-methods                               
# pylint: disable=missing-function-docstring                                                                            
from __future__ import annotations
import typing as t

class Cls:
    def func(self, dd: defaultdict):
        # This import makes the definition work.                                                                        
        from collections import defaultdict

        obj = defaultdict()
        obj.update(dd)
        return obj

if t.TYPE_CHECKING:
    # This import makes the annotations work.                                                                           
    from collections import defaultdict

Configuration

Default

Command used

pylint redefined_outer.py

Pylint output

************* Module redefined_outer
redefined_outer.py:10: [W0621(redefined-outer-name), Cls.func] Redefining name 'defaultdict' from outer scope (line 19)

Expected behavior

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Whether TYPE_CHECKING is imported from typing or called as typing.TYPE_CHECKING or imported as an alias should not matter to whether TYPE_CHECKING is called.

I looked at the code and realize that this is a difficult bug to fix -- it probably would involve putting a Mock around typing.TYPE_CHECKING before the module is imported so that the ast tree can report whether it's in a TYPE_CHECKING context or not. This might be a bug left open for a while, but I do think it's an actual issue that might not be resolved until Python has a syntax similar to TypeScript's "from collections import type defaultdict"

I've noticed the same issue with @t.overload on mypy not properly recognizing it as a overload decorator, so the solution was something I could see.

Because it can be a pain to constantly update a from typing import .... statement at the top of a module when adding types, my project, music21 uses a standard of importing typing as t at the top of each module so it's easy to know that t.Union[t.Dict[t.Optional[str]] will always be available. (When our min Py version goes to 3.9/3.10, there'll be a lot less of this).

Thanks for an amazing tool!

Pylint version

pylint 2.15.3
astroid 2.12.10
Python 3.10.6 (v3.10.6:9c7b4bd164, Aug  1 2022, 17:13:48) [Clang 13.0.0 (clang-1300.0.29.30)]

OS / Environment

MacOS 12.6

Additional dependencies

None

[Pierre-Sassoulas]

@cdce8p I think this was intentional for better performance, right ?

[mscuthbert]

I didn't see anything in the #2834 close discussion about it being intentional, but that might have come up in TYPE_CHECKS_GUARDS original intention.

(apologies: it's PyCharm's type system that can't do @t.overload not mypy).

[mscuthbert]

Aha in #6787 a code fix for unused-import that referenced typing as t was introduced -- it doesn't look too slow to use here as well. (it makes sense, @jacobtylerwalls has contributed mightily to the music21 project, so of course he would have already found this in a different context).

[jacobtylerwalls]

You're quite right; this diff fixes it:

diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py
index 38a3a7cc2..efe4aedae 100644
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -1230,8 +1230,7 @@ class VariablesChecker(BaseChecker):
                 # Do not take in account redefined names for the purpose
                 # of type checking.:
                 if any(
-                    isinstance(definition.parent, nodes.If)
-                    and definition.parent.test.as_string() in TYPING_TYPE_CHECKS_GUARDS
+                    in_type_checking_block(definition)
                     for definition in globs[name]
                 ):
                     continue

@mscuthbert would you like to do the PR? See #7525

[cdce8p]

@cdce8p I think this was intentional for better performance, right ?

Yeah. I didn't want to do inference for each name. The question quickly becomes where do you stop? What about this for example?

from typing import TYPE_CHECKING as TC

You also don't have to use if TYPE_CHECKING. Any condition which evaluates to false at runtime would work. Mypy has a config value which can be used for it: always_true.

For the initial implementation I though that supporting only typing.TYPE_CHECKING and just TYPE_CHECKING might be enough.

If you've found a solution which isn't too costly and works, that would be fine.