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

false negative: used-before-assignment w/ TYPE_CHECKING guard #8893

Closed
Redoubts opened this issue Jul 27, 2023 · 9 comments · Fixed by #9990
Closed

false negative: used-before-assignment w/ TYPE_CHECKING guard #8893

Redoubts opened this issue Jul 27, 2023 · 9 comments · Fixed by #9990
Labels
C: used-before-assignment Issues related to 'used-before-assignment' check False Negative 🦋 No message is emitted but something is wrong with the code typing
Milestone

Comments

@Redoubts
Copy link

Redoubts commented Jul 27, 2023

Bug description

Consider the following code:

from __future__ import annotations

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    import datetime

class Z:
    def something(self) -> None:
        z = datetime.datetime.now() # not imported at runtime

Pylint can and does flag the z = ... line as used-before-assignment. However, if we use datetime somewhere as a type annotation, say in

from __future__ import annotations

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    import datetime


def x() -> datetime.datetime: # this is good
    return datetime.datetime.now() # this should be bad


class Z:
    def something(self) -> None:
        z = datetime.datetime.now() # this should still be bad

pylint returns "10/10, would lint again". pylint should flag the used-before-assignment errors here, since this code will blow up at runtime when called.

Command used

pylint --disable all --enable E <file>

Pylint output

* without type annotation
% pylint --disable all --enable E xyz.py
************* Module xyz
xyz.py:15:12: E0601: Using variable 'datetime' before assignment (used-before-assignment)

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

* with type annotation
% pylint --disable all --enable E xyz.py

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

Expected behavior

consistent used-before-assignment lints.

Pylint version

% pylint --version                      
pylint 2.17.3
astroid 2.15.4
Python 3.10.12 (main, Jun 12 2023, 08:08:55) [GCC 11.2.0]

OS / Environment

Debian

@Redoubts Redoubts added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jul 27, 2023
@Redoubts
Copy link
Author

Kinda feels like the obverse of #8111

@nickdrozd
Copy link
Contributor

This was partially fixed on the main brach by #8198 (I think).

if TYPE_CHECKING:
    import datetime

def f():
    return datetime.datetime.now()  # flagged -- good

def g() -> datetime.datetime:
    return datetime.datetime.now()  # not flagged -- bad

Perhaps it's a strange case, but there is also a false negative for classes defined in the TYPE_CHECKING block:

if TYPE_CHECKING:
    class X:
        pass

def f():
    return X()  # not flagged -- bad

def g() -> X:
    return X()  # not flagged -- bad

@nickdrozd nickdrozd added typing False Negative 🦋 No message is emitted but something is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jul 27, 2023
@Redoubts
Copy link
Author

Yeah I think that’s interesting that only the first issue is flagged. In my worst case, only the first datetime usage is flagged as well.

@jacobtylerwalls jacobtylerwalls added the C: used-before-assignment Issues related to 'used-before-assignment' check label Jul 28, 2023
@hofbi
Copy link

hofbi commented Sep 5, 2023

I observe a quite similar issue which was introduced in v3.0.0a7, v3.0.0a6 works fine.

Steps to reproduce:

@dataclass
class Foo:
    my_list: list[Bar]

which fails with E0601: Using variable 'list' before assignment (used-before-assignment). It can be fixed by using the typing.List over list for the type hint, which it not what's recommended by pyupgrade.

@DanielNoord
Copy link
Collaborator

@hofbi Does that file include from __future__ import annotations?

@hofbi
Copy link

hofbi commented Sep 5, 2023

@DanielNoord No, no __future__ imports

@jacobtylerwalls
Copy link
Member

#9587 was a duplicate for g() in #8893 (comment).

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Sep 30, 2024

if TYPE_CHECKING behaves the same way as if False so my impression is just that this was never implemented because there was never a use case for it until the TYPE_CHECKING constant.

Shooting from the hip, I bet in get_next_to_consume() we could filter out nodes where in_type_checking_block(other_node) is true and the current node meets the conditions for where runtime imports are needed.

@jacobtylerwalls
Copy link
Member

However, #8431 added a different approach for this, so it's worth examining whether we should tweak it or drop it. It probably shouldn't be assuming that we can have one approach to type-checking imports per scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: used-before-assignment Issues related to 'used-before-assignment' check False Negative 🦋 No message is emitted but something is wrong with the code typing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants