Emit new message ``used-global-in-annotation`` instead of ``used-prior-global-declaration``

[Davichet-e]

Steps to reproduce

I have a namedtuple that I want to mutate on a function:

foo = collections.namedtuple("foo", "")

def bar() -> foo # Here pylint warns me
    global foo
    foo = collections.namedtuple("foo", "a b")
    return foo

Current behavior

Pylint warning me on the function return type annotation that is invalid for this reason: Name 'foo' is used prior to global declaration: pylint(used-prior-global-declaration)

Expected behavior

It should not warn at all since when I run the code, it runs without errors.

pylint --version output

pylint: 2.3.1 python: 3.7.3

[PCManticore]

Thanks, I can reproduce the issue.

[DanielNoord]

I think we should be able to solve this bug, however, wouldn't it be better to pass the global variable to the function and re-assign the functions output to the variable? I don't think modifying global variables is recommended. Therefore, I would suggest changing the code that is currently throwing this error to start throwing a function-modifying-global-variable (name could be improved..) message. Users who still would like to do this can then disable this message.

[Pierre-Sassoulas]

Agree. Note that this warning exists in pylint it's W0603 global-statement.

[DanielNoord]

Yes. I was thinking of a new warning based on the fact that the typing includes a global variable. However, the result can only be achieved by using the global statement somewhere in the function. So raising W0603 on the line that includes the typing seems to be enough!

[DanielNoord]

Pinging @cdce8p as I need some help with the order of typing resolution here.

foo = int

def bar() -> foo # Here pylint warns me
    global foo
    foo = str
    return foo()

pyright complains about (rightfully I think) as it thinks bar() should return an int while it doesn't. The case of the original report is more like:

foo = int

def bar() -> foo # Here pylint warns me
    global foo
    foo = int
    return foo()

That would be fine, but then the only way for us to determine whether or not we should raise used-prior-global-declaration is by comparing types, which is not our main competence.

I would be okay with used-prior-global-declaration also meaning: "don't use global for variables you use as type annotations". Or splitting this into a new message used-global-in-annotation and let users turn this off as they wish. This pattern seems likely to cause troubles.

[cdce8p]

This pattern seems likely to cause troubles.

I agree. This looks like bad code. Tbh. I can't think of a situation where you would want to use a type alias (like foo) together with global at the moment. Just because it's valid, doesn't mean it's good.

I would be okay with used-prior-global-declaration also meaning: "don't use global for variables you use as type annotations".

🤔 Although the pattern is bad, it is valid. The annotation is evaluated in the module scope, we don't care about the function body itself here. So used-prior-global-declaration doesn't really fit. No matter how type checkers will resolve that.

Or splitting this into a new message used-global-in-annotation and let users turn this off as they wish.

That might be the best option here and most importantly captures the root issue: Using a type alias both in an annotation and with a global statement.

[DanielNoord]

Or splitting this into a new message used-global-in-annotation and let users turn this off as they wish.

That might be the best option here and most importantly captures the root issue: Using a type alias both in an annotation and with a global statement.

Thanks! Refocusing this issue then as I agree with you!