Skip to content

Fix used-before-assignment false positive for TYPE_CHECKING if/elif/else usage #8071

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

Merged
merged 21 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0acdd1d
Fix type checking used-before-assignment false positive
zenlyj Jan 16, 2023
6f85bb9
Add functional tests
zenlyj Jan 16, 2023
cce327a
Merge branch 'main' of https://github.com/PyCQA/pylint into false_pos…
zenlyj Jan 17, 2023
d99dd81
Add changelog
zenlyj Jan 17, 2023
19d2681
Revert unnecessary changes
zenlyj Jan 17, 2023
51090bd
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 17, 2023
775ccc9
Add utility function to detect name definition recursively
zenlyj Feb 2, 2023
bf72993
Update tests to handle more cases
zenlyj Feb 2, 2023
04a091d
Merge branch 'main' of https://github.com/PyCQA/pylint into false_pos…
zenlyj Feb 2, 2023
c594f75
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 2, 2023
4aafcae
Resolve CI errors
zenlyj Feb 2, 2023
318f30d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 2, 2023
35576ea
Update comment for easier understanding
zenlyj Feb 2, 2023
352d6de
Debug is_defined check for except handler name
zenlyj Feb 2, 2023
d1dc39b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 2, 2023
0904a52
[cosmetic] Alphabetize imports in functional test
zenlyj Feb 5, 2023
58bdfff
Shift walrus functional tests to dedicated py38 test file
zenlyj Feb 5, 2023
1198e5b
[cosmetic] Rename functions in functional test
zenlyj Feb 5, 2023
4acf0f6
Minor refactor to improve code readability
zenlyj Feb 5, 2023
2fc8fda
Remove exception handler name check in utils.is_defined
zenlyj Feb 5, 2023
c24f62a
Merge branch 'main' of https://github.com/PyCQA/pylint into false_pos…
zenlyj Feb 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/7574.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix false positive for ``used-before-assignment`` when
``typing.TYPE_CHECKING`` is used with if/elif/else blocks.

Closes #7574
49 changes: 49 additions & 0 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2193,6 +2193,55 @@ def is_class_attr(name: str, klass: nodes.ClassDef) -> bool:
return False


def is_defined(name: str, node: nodes.NodeNG) -> bool:
"""Checks whether a node defines the given variable name."""
is_defined_so_far = False

if isinstance(node, nodes.NamedExpr) and node.target.name == name:
return True

if isinstance(node, (nodes.Import, nodes.ImportFrom)) and any(
node_name[0] == name for node_name in node.names
):
return True

if isinstance(node, nodes.With):
is_defined_so_far = any(
isinstance(item[1], nodes.AssignName) and item[1].name == name
for item in node.items
)

if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)):
is_defined_so_far = node.name == name

if isinstance(node, nodes.AnnAssign):
is_defined_so_far = (
node.value
and isinstance(node.target, nodes.AssignName)
and node.target.name == name
)

if isinstance(node, nodes.Assign):
is_defined_so_far = any(
any(
(
(
isinstance(elt, nodes.Starred)
and isinstance(elt.value, nodes.AssignName)
and elt.value.name == name
)
or (isinstance(elt, nodes.AssignName) and elt.name == name)
)
for elt in get_all_elements(target)
)
for target in node.targets
)

return is_defined_so_far or any(
is_defined(name, child) for child in node.get_children()
)


def get_inverse_comparator(op: str) -> str:
"""Returns the inverse comparator given a comparator.

Expand Down
30 changes: 12 additions & 18 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -2036,7 +2036,7 @@ def _in_lambda_or_comprehension_body(
parent = parent.parent
return False

# pylint: disable = too-many-statements, too-many-branches
# pylint: disable = too-many-branches
@staticmethod
def _is_variable_violation(
node: nodes.Name,
Expand Down Expand Up @@ -2220,27 +2220,21 @@ def _is_variable_violation(
)
):
# Exempt those definitions that are used inside the type checking
# guard or that are defined in both type checking guard branches.
# guard or that are defined in any elif/else type checking guard branches.
used_in_branch = defstmt_parent.parent_of(node)
defined_in_or_else = False

for definition in defstmt_parent.orelse:
if isinstance(definition, nodes.Assign):
if not used_in_branch:
if defstmt_parent.has_elif_block():
defined_in_or_else = utils.is_defined(
node.name, defstmt_parent.orelse[0]
)
else:
defined_in_or_else = any(
target.name == node.name
for target in definition.targets
if isinstance(target, nodes.AssignName)
utils.is_defined(node.name, content)
for content in defstmt_parent.orelse
)
elif isinstance(
definition, (nodes.ClassDef, nodes.FunctionDef)
):
defined_in_or_else = definition.name == node.name

if defined_in_or_else:
break

if not used_in_branch and not defined_in_or_else:
maybe_before_assign = True
if not defined_in_or_else:
maybe_before_assign = True

return maybe_before_assign, annotation_return, use_outer_definition

Expand Down
20 changes: 20 additions & 0 deletions tests/functional/u/undefined/undefined_variable_py38.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

# Tests for annotation of variables and potentially undefinition

from typing import TYPE_CHECKING

def typing_and_assignment_expression():
"""The variable gets assigned in an assignment expression"""
Expand Down Expand Up @@ -190,3 +191,22 @@ def expression_in_ternary_operator_inside_container_wrong_position():
if (still_defined := False) == 1:
NEVER_DEFINED_EITHER = 1
print(still_defined)


if TYPE_CHECKING:
import enum
import weakref
elif input():
if input() + 1:
pass
elif (enum := None):
pass
else:
print(None if (weakref := '') else True)
else:
pass

def defined_by_walrus_in_type_checking() -> weakref:
"""Usage of variables defined in TYPE_CHECKING blocks"""
print(enum)
return weakref
22 changes: 11 additions & 11 deletions tests/functional/u/undefined/undefined_variable_py38.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
used-before-assignment:17:15:17:18:typing_and_self_referencing_assignment_expression:Using variable 'var' before assignment:HIGH
used-before-assignment:23:15:23:18:self_referencing_assignment_expression:Using variable 'var' before assignment:HIGH
undefined-variable:48:6:48:16::Undefined variable 'no_default':UNDEFINED
undefined-variable:56:6:56:22::Undefined variable 'again_no_default':UNDEFINED
undefined-variable:82:6:82:19::Undefined variable 'else_assign_1':INFERENCE
undefined-variable:105:6:105:19::Undefined variable 'else_assign_2':INFERENCE
used-before-assignment:140:10:140:16:type_annotation_used_improperly_after_comprehension:Using variable 'my_int' before assignment:HIGH
used-before-assignment:147:10:147:16:type_annotation_used_improperly_after_comprehension_2:Using variable 'my_int' before assignment:HIGH
used-before-assignment:177:12:177:16:expression_in_ternary_operator_inside_container_wrong_position:Using variable 'val3' before assignment:HIGH
used-before-assignment:181:9:181:10::Using variable 'z' before assignment:HIGH
used-before-assignment:188:6:188:19::Using variable 'NEVER_DEFINED' before assignment:CONTROL_FLOW
used-before-assignment:18:15:18:18:typing_and_self_referencing_assignment_expression:Using variable 'var' before assignment:HIGH
used-before-assignment:24:15:24:18:self_referencing_assignment_expression:Using variable 'var' before assignment:HIGH
undefined-variable:49:6:49:16::Undefined variable 'no_default':UNDEFINED
undefined-variable:57:6:57:22::Undefined variable 'again_no_default':UNDEFINED
undefined-variable:83:6:83:19::Undefined variable 'else_assign_1':INFERENCE
undefined-variable:106:6:106:19::Undefined variable 'else_assign_2':INFERENCE
used-before-assignment:141:10:141:16:type_annotation_used_improperly_after_comprehension:Using variable 'my_int' before assignment:HIGH
used-before-assignment:148:10:148:16:type_annotation_used_improperly_after_comprehension_2:Using variable 'my_int' before assignment:HIGH
used-before-assignment:178:12:178:16:expression_in_ternary_operator_inside_container_wrong_position:Using variable 'val3' before assignment:HIGH
used-before-assignment:182:9:182:10::Using variable 'z' before assignment:HIGH
used-before-assignment:189:6:189:19::Using variable 'NEVER_DEFINED' before assignment:CONTROL_FLOW
86 changes: 85 additions & 1 deletion tests/functional/u/used/used_before_assignment_typing.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,65 @@
"""Tests for used-before-assignment for typing related issues"""
# pylint: disable=missing-function-docstring
# pylint: disable=missing-function-docstring,ungrouped-imports,invalid-name


from typing import List, Optional, TYPE_CHECKING

if TYPE_CHECKING:
if True: # pylint: disable=using-constant-test
import math
from urllib.request import urlopen
import array
import base64
import binascii
import bisect
import calendar
import collections
import copy
import datetime
import email
import heapq
import json
import mailbox
import mimetypes
import numbers
import pprint
import types
import zoneinfo
elif input():
import calendar, bisect # pylint: disable=multiple-imports
if input() + 1:
import heapq
else:
import heapq
elif input():
try:
numbers = None if input() else 1
import array
except Exception as e: # pylint: disable=broad-exception-caught
import types
finally:
copy = None
elif input():
for i in range(1,2):
email = None
else: # pylint: disable=useless-else-on-loop
json = None
while input():
import mailbox
else: # pylint: disable=useless-else-on-loop
mimetypes = None
elif input():
with input() as base64:
pass
with input() as temp:
import binascii
else:
from urllib.request import urlopen
zoneinfo: str = ''
def pprint():
pass
class collections: # pylint: disable=too-few-public-methods,missing-class-docstring
pass

class MyClass:
"""Type annotation or default values for first level methods can't refer to their own class"""
Expand Down Expand Up @@ -111,3 +162,36 @@ class ConditionalImportGuardedWhenUsed: # pylint: disable=too-few-public-method
"""Conditional imports also guarded by TYPE_CHECKING when used."""
if TYPE_CHECKING:
print(urlopen)


class TypeCheckingMultiBranch: # pylint: disable=too-few-public-methods,unused-variable
"""Test for defines in TYPE_CHECKING if/elif/else branching"""
def defined_in_elif_branch(self) -> calendar.Calendar:
print(bisect)
return calendar.Calendar()

def defined_in_else_branch(self) -> urlopen:
print(zoneinfo)
print(pprint())
print(collections())
return urlopen

def defined_in_nested_if_else(self) -> heapq:
print(heapq)
return heapq

def defined_in_try_except(self) -> array:
print(types)
print(copy)
print(numbers)
return array

def defined_in_loops(self) -> json:
print(email)
print(mailbox)
print(mimetypes)
return json

def defined_in_with(self) -> base64:
print(binascii)
return base64
10 changes: 5 additions & 5 deletions tests/functional/u/used/used_before_assignment_typing.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
undefined-variable:17:21:17:28:MyClass.incorrect_typing_method:Undefined variable 'MyClass':UNDEFINED
undefined-variable:22:26:22:33:MyClass.incorrect_nested_typing_method:Undefined variable 'MyClass':UNDEFINED
undefined-variable:27:20:27:27:MyClass.incorrect_default_method:Undefined variable 'MyClass':UNDEFINED
used-before-assignment:88:35:88:39:MyFourthClass.is_close:Using variable 'math' before assignment:HIGH
used-before-assignment:101:20:101:28:VariableAnnotationsGuardedByTypeChecking:Using variable 'datetime' before assignment:HIGH
undefined-variable:68:21:68:28:MyClass.incorrect_typing_method:Undefined variable 'MyClass':UNDEFINED
undefined-variable:73:26:73:33:MyClass.incorrect_nested_typing_method:Undefined variable 'MyClass':UNDEFINED
undefined-variable:78:20:78:27:MyClass.incorrect_default_method:Undefined variable 'MyClass':UNDEFINED
used-before-assignment:139:35:139:39:MyFourthClass.is_close:Using variable 'math' before assignment:HIGH
used-before-assignment:152:20:152:28:VariableAnnotationsGuardedByTypeChecking:Using variable 'datetime' before assignment:HIGH