From 0acdd1d7e42fe08d3c2b7cfc368c2e477d44e8c2 Mon Sep 17 00:00:00 2001 From: Zen Date: Tue, 17 Jan 2023 03:12:17 +0800 Subject: [PATCH 01/18] Fix type checking used-before-assignment false positive --- pylint/checkers/variables.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 0d72ef9691..7b858d948c 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2204,28 +2204,36 @@ 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 all 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): + branch_nodes = [] + branch = defstmt_parent + while branch.has_elif_block(): + branch_nodes.extend(branch.orelse[0].body) + branch = branch.orelse[0] + branch_nodes.extend(branch.orelse) + for branch_node in branch_nodes: + if isinstance(branch_node, nodes.Assign): defined_in_or_else = any( target.name == node.name - for target in definition.targets + for target in branch_node.targets if isinstance(target, nodes.AssignName) ) elif isinstance( - definition, (nodes.ClassDef, nodes.FunctionDef) + branch_node, (nodes.ClassDef, nodes.FunctionDef) ): - defined_in_or_else = definition.name == node.name - + defined_in_or_else = branch_node.name == node.name + elif isinstance( + branch_node, (nodes.Import, nodes.ImportFrom) + ): + defined_in_or_else = any(node.name == name[0] for name in branch_node.names) if defined_in_or_else: break if not used_in_branch and not defined_in_or_else: maybe_before_assign = True - return maybe_before_assign, annotation_return, use_outer_definition @staticmethod @@ -2341,6 +2349,7 @@ def _is_first_level_self_reference( node.parent.parent, nodes.Arguments ): return (VariableVisitConsumerAction.CONTINUE, None) + return (VariableVisitConsumerAction.RETURN, found_nodes) @staticmethod From 6f85bb9b94c7c241db02d8aafe66579f6c027bc1 Mon Sep 17 00:00:00 2001 From: Zen Date: Tue, 17 Jan 2023 03:15:29 +0800 Subject: [PATCH 02/18] Add functional tests --- .../u/used/used_before_assignment_typing.py | 24 ++++++++++++++++++- .../u/used/used_before_assignment_typing.txt | 10 ++++---- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/tests/functional/u/used/used_before_assignment_typing.py b/tests/functional/u/used/used_before_assignment_typing.py index c0cb484e0d..6e1f871318 100644 --- a/tests/functional/u/used/used_before_assignment_typing.py +++ b/tests/functional/u/used/used_before_assignment_typing.py @@ -1,5 +1,5 @@ """Tests for used-before-assignment for typing related issues""" -# pylint: disable=missing-function-docstring +# pylint: disable=missing-function-docstring,ungrouped-imports from typing import List, Optional, TYPE_CHECKING @@ -8,6 +8,11 @@ if True: # pylint: disable=using-constant-test import math import datetime + import calendar + from urllib.request import urlopen +elif input(): + import calendar +else: from urllib.request import urlopen class MyClass: @@ -108,3 +113,20 @@ 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 imports in TYPE_CHECKING if/elif/else branching""" + def elif_branch_body(self): + cal = calendar.Calendar() + print(calendar.Calendar()) + + def elif_branch_return(self) -> calendar.Calendar: + pass + + def else_branch_body(self): + url_open = urlopen + print(urlopen) + + def else_branch_return(self) -> urlopen: + pass diff --git a/tests/functional/u/used/used_before_assignment_typing.txt b/tests/functional/u/used/used_before_assignment_typing.txt index 680c3430d1..075cef4fe1 100644 --- a/tests/functional/u/used/used_before_assignment_typing.txt +++ b/tests/functional/u/used/used_before_assignment_typing.txt @@ -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:100:20:100:28:VariableAnnotationsGuardedByTypeChecking:Using variable 'datetime' before assignment:HIGH +undefined-variable:22:21:22:28:MyClass.incorrect_typing_method:Undefined variable 'MyClass':UNDEFINED +undefined-variable:27:26:27:33:MyClass.incorrect_nested_typing_method:Undefined variable 'MyClass':UNDEFINED +undefined-variable:32:20:32:27:MyClass.incorrect_default_method:Undefined variable 'MyClass':UNDEFINED +used-before-assignment:93:35:93:39:MyFourthClass.is_close:Using variable 'math' before assignment:HIGH +used-before-assignment:105:20:105:28:VariableAnnotationsGuardedByTypeChecking:Using variable 'datetime' before assignment:HIGH From d99dd8111695184fc7fee6f54243e2db3f35ad92 Mon Sep 17 00:00:00 2001 From: zenlyj Date: Tue, 17 Jan 2023 21:44:03 +0800 Subject: [PATCH 03/18] Add changelog --- doc/whatsnew/fragments/7574.false_positive | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 doc/whatsnew/fragments/7574.false_positive diff --git a/doc/whatsnew/fragments/7574.false_positive b/doc/whatsnew/fragments/7574.false_positive new file mode 100644 index 0000000000..8b4560799c --- /dev/null +++ b/doc/whatsnew/fragments/7574.false_positive @@ -0,0 +1,4 @@ +Fix false positive for ``used-before-assignment`` when +``typing.TYPE_CHECKING`` is used with if/elif/else blocks. + +Closes #7574 From 19d26815ff2d78b566a2cacbf156e9a7146bab54 Mon Sep 17 00:00:00 2001 From: zenlyj Date: Tue, 17 Jan 2023 21:52:31 +0800 Subject: [PATCH 04/18] Revert unnecessary changes --- pylint/checkers/variables.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 7b858d948c..334d2adbef 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2229,11 +2229,13 @@ def _is_variable_violation( branch_node, (nodes.Import, nodes.ImportFrom) ): defined_in_or_else = any(node.name == name[0] for name in branch_node.names) + if defined_in_or_else: break if not used_in_branch and not defined_in_or_else: maybe_before_assign = True + return maybe_before_assign, annotation_return, use_outer_definition @staticmethod @@ -2349,7 +2351,6 @@ def _is_first_level_self_reference( node.parent.parent, nodes.Arguments ): return (VariableVisitConsumerAction.CONTINUE, None) - return (VariableVisitConsumerAction.RETURN, found_nodes) @staticmethod From 51090bd2c36e8a5bda0731d8926b491ce278ebe7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 17 Jan 2023 13:54:45 +0000 Subject: [PATCH 05/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- doc/whatsnew/fragments/7574.false_positive | 2 +- pylint/checkers/variables.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/whatsnew/fragments/7574.false_positive b/doc/whatsnew/fragments/7574.false_positive index 8b4560799c..a7b91b4817 100644 --- a/doc/whatsnew/fragments/7574.false_positive +++ b/doc/whatsnew/fragments/7574.false_positive @@ -1,4 +1,4 @@ Fix false positive for ``used-before-assignment`` when -``typing.TYPE_CHECKING`` is used with if/elif/else blocks. +``typing.TYPE_CHECKING`` is used with if/elif/else blocks. Closes #7574 diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 334d2adbef..115209b8c7 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2225,10 +2225,10 @@ def _is_variable_violation( branch_node, (nodes.ClassDef, nodes.FunctionDef) ): defined_in_or_else = branch_node.name == node.name - elif isinstance( - branch_node, (nodes.Import, nodes.ImportFrom) - ): - defined_in_or_else = any(node.name == name[0] for name in branch_node.names) + elif isinstance(branch_node, (nodes.Import, nodes.ImportFrom)): + defined_in_or_else = any( + node.name == name[0] for name in branch_node.names + ) if defined_in_or_else: break From 775ccc96de8a9eabf4bd9febd6b07e55a1e1fe0e Mon Sep 17 00:00:00 2001 From: Zen Date: Fri, 3 Feb 2023 01:08:48 +0800 Subject: [PATCH 06/18] Add utility function to detect name definition recursively --- pylint/checkers/utils.py | 42 ++++++++++++++++++++++++++++++++++++ pylint/checkers/variables.py | 33 ++++++---------------------- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 27042653b7..280288925f 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -2203,3 +2203,45 @@ def is_class_attr(name: str, klass: nodes.ClassDef) -> bool: return True except astroid.NotFoundError: return False + + +def is_defined(name: str, node: nodes.NodeNG) -> bool: + """Checks whether a node defines the given variable name""" + print(node) + is_defined_so_far = False + + if isinstance(node, nodes.NamedExpr): + return node.target.name == name + + if isinstance(node, (nodes.Import, nodes.ImportFrom)): + return any(node_name[0] == name for node_name in node.names) + + 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.ExceptHandler): + 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()) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 115209b8c7..d58572743c 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2206,32 +2206,13 @@ def _is_variable_violation( # Exempt those definitions that are used inside the type checking # guard or that are defined in all type checking guard branches. used_in_branch = defstmt_parent.parent_of(node) - defined_in_or_else = False - - branch_nodes = [] - branch = defstmt_parent - while branch.has_elif_block(): - branch_nodes.extend(branch.orelse[0].body) - branch = branch.orelse[0] - branch_nodes.extend(branch.orelse) - for branch_node in branch_nodes: - if isinstance(branch_node, nodes.Assign): - defined_in_or_else = any( - target.name == node.name - for target in branch_node.targets - if isinstance(target, nodes.AssignName) - ) - elif isinstance( - branch_node, (nodes.ClassDef, nodes.FunctionDef) - ): - defined_in_or_else = branch_node.name == node.name - elif isinstance(branch_node, (nodes.Import, nodes.ImportFrom)): - defined_in_or_else = any( - node.name == name[0] for name in branch_node.names - ) - - if defined_in_or_else: - break + 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( + utils.is_defined(node.name, content) + for content in defstmt_parent.orelse + ) if not used_in_branch and not defined_in_or_else: maybe_before_assign = True From bf729938e8954871337869a3be3b27925cdb9648 Mon Sep 17 00:00:00 2001 From: Zen Date: Fri, 3 Feb 2023 01:10:26 +0800 Subject: [PATCH 07/18] Update tests to handle more cases --- .../u/used/used_before_assignment_typing.py | 97 ++++++++++++++++--- .../u/used/used_before_assignment_typing.txt | 10 +- 2 files changed, 87 insertions(+), 20 deletions(-) diff --git a/tests/functional/u/used/used_before_assignment_typing.py b/tests/functional/u/used/used_before_assignment_typing.py index 6e1f871318..27b0dd9331 100644 --- a/tests/functional/u/used/used_before_assignment_typing.py +++ b/tests/functional/u/used/used_before_assignment_typing.py @@ -1,5 +1,5 @@ """Tests for used-before-assignment for typing related issues""" -# pylint: disable=missing-function-docstring,ungrouped-imports +# pylint: disable=missing-function-docstring,ungrouped-imports,invalid-name from typing import List, Optional, TYPE_CHECKING @@ -9,11 +9,61 @@ import math import datetime import calendar + import zoneinfo + import array + import types + import enum + import collections + import copy + import pprint + import heapq + import bisect + import weakref + import numbers + import email + import json + import mailbox + import mimetypes + import base64 + import binascii from urllib.request import urlopen elif input(): - import calendar + import calendar, bisect # pylint: disable=multiple-imports + if input() + 1: + import heapq + elif (enum:=None): + pass + else: + print(None if (weakref:='') else True) +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""" @@ -117,16 +167,33 @@ class ConditionalImportGuardedWhenUsed: # pylint: disable=too-few-public-method class TypeCheckingMultiBranch: # pylint: disable=too-few-public-methods,unused-variable """Test imports in TYPE_CHECKING if/elif/else branching""" - def elif_branch_body(self): - cal = calendar.Calendar() - print(calendar.Calendar()) - - def elif_branch_return(self) -> calendar.Calendar: - pass - - def else_branch_body(self): - url_open = urlopen - print(urlopen) - - def else_branch_return(self) -> urlopen: - pass + def elif_branch(self) -> calendar.Calendar: + print(bisect) + return calendar.Calendar() + + def else_branch(self) -> urlopen: + print(zoneinfo) + print(pprint()) + print(collections()) + return urlopen + + def nested_if_else(self) -> enum: + print(heapq) + print(weakref) + return enum + + def used_in_try_except(self) -> array: + print(types) + print(copy) + print(numbers) + return array + + def used_in_loops(self) -> json: + print(email) + print(mailbox) + print(mimetypes) + return json + + def used_in_with(self) -> base64: + print(binascii) + return base64 diff --git a/tests/functional/u/used/used_before_assignment_typing.txt b/tests/functional/u/used/used_before_assignment_typing.txt index 075cef4fe1..28c21a80d4 100644 --- a/tests/functional/u/used/used_before_assignment_typing.txt +++ b/tests/functional/u/used/used_before_assignment_typing.txt @@ -1,5 +1,5 @@ -undefined-variable:22:21:22:28:MyClass.incorrect_typing_method:Undefined variable 'MyClass':UNDEFINED -undefined-variable:27:26:27:33:MyClass.incorrect_nested_typing_method:Undefined variable 'MyClass':UNDEFINED -undefined-variable:32:20:32:27:MyClass.incorrect_default_method:Undefined variable 'MyClass':UNDEFINED -used-before-assignment:93:35:93:39:MyFourthClass.is_close:Using variable 'math' before assignment:HIGH -used-before-assignment:105:20:105:28:VariableAnnotationsGuardedByTypeChecking:Using variable 'datetime' before assignment:HIGH +undefined-variable:72:21:72:28:MyClass.incorrect_typing_method:Undefined variable 'MyClass':UNDEFINED +undefined-variable:77:26:77:33:MyClass.incorrect_nested_typing_method:Undefined variable 'MyClass':UNDEFINED +undefined-variable:82:20:82:27:MyClass.incorrect_default_method:Undefined variable 'MyClass':UNDEFINED +used-before-assignment:143:35:143:39:MyFourthClass.is_close:Using variable 'math' before assignment:HIGH +used-before-assignment:155:20:155:28:VariableAnnotationsGuardedByTypeChecking:Using variable 'datetime' before assignment:HIGH From c594f7531c9aed701578cff90781da8726357a52 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 2 Feb 2023 17:37:28 +0000 Subject: [PATCH 08/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/utils.py | 29 +++++++++++++++++++---------- pylint/checkers/variables.py | 6 ++++-- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 7a47d07d41..7bdc7f6b51 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -2194,17 +2194,20 @@ def is_class_attr(name: str, klass: nodes.ClassDef) -> bool: def is_defined(name: str, node: nodes.NodeNG) -> bool: - """Checks whether a node defines the given variable name""" + """Checks whether a node defines the given variable name.""" is_defined_so_far = False if isinstance(node, nodes.NamedExpr): return node.target.name == name - + if isinstance(node, (nodes.Import, nodes.ImportFrom)): return any(node_name[0] == name for node_name in node.names) - + 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) + 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 @@ -2214,16 +2217,20 @@ def is_defined(name: str, node: nodes.NodeNG) -> bool: if isinstance(node, nodes.AnnAssign): is_defined_so_far = ( - node.value and - isinstance(node.target, nodes.AssignName) and - node.target.name == name + node.value + and isinstance(node.target, nodes.AssignName) + and node.target.name == name ) if isinstance(node, nodes.Assign): is_defined_so_far = any( - any ( + any( ( - (isinstance(elt, nodes.Starred) and isinstance(elt.value, nodes.AssignName) and elt.value.name == name) + ( + 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) @@ -2231,7 +2238,9 @@ def is_defined(name: str, node: nodes.NodeNG) -> bool: for target in node.targets ) - return is_defined_so_far or any(is_defined(name, child) for child in node.get_children()) + return is_defined_so_far or any( + is_defined(name, child) for child in node.get_children() + ) def get_inverse_comparator(op: str) -> str: diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 6a26bb7efa..27fee55b50 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2223,10 +2223,12 @@ def _is_variable_violation( # guard or that are defined in all type checking guard branches. used_in_branch = defstmt_parent.parent_of(node) if defstmt_parent.has_elif_block(): - defined_in_or_else = utils.is_defined(node.name, defstmt_parent.orelse[0]) + defined_in_or_else = utils.is_defined( + node.name, defstmt_parent.orelse[0] + ) else: defined_in_or_else = any( - utils.is_defined(node.name, content) + utils.is_defined(node.name, content) for content in defstmt_parent.orelse ) From 4aafcae180120c72efe6d682a6e9e5012368b9f1 Mon Sep 17 00:00:00 2001 From: Zen Date: Fri, 3 Feb 2023 02:02:43 +0800 Subject: [PATCH 09/18] Resolve CI errors --- pylint/checkers/utils.py | 17 +++++++++-------- pylint/checkers/variables.py | 2 +- .../u/used/used_before_assignment_typing.rc | 2 ++ 3 files changed, 12 insertions(+), 9 deletions(-) create mode 100644 tests/functional/u/used/used_before_assignment_typing.rc diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 7bdc7f6b51..e0b39c7c66 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -2197,11 +2197,15 @@ 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): - return node.target.name == name + if (isinstance(node, nodes.NamedExpr) + and node.target.name == name + ): + return True - if isinstance(node, (nodes.Import, nodes.ImportFrom)): - return any(node_name[0] == name for node_name in node.names) + 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( @@ -2209,10 +2213,7 @@ def is_defined(name: str, node: nodes.NodeNG) -> bool: for item in node.items ) - if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)): - is_defined_so_far = node.name == name - - if isinstance(node, nodes.ExceptHandler): + if isinstance(node, (nodes.ClassDef, nodes.FunctionDef, nodes.ExceptHandler)): is_defined_so_far = node.name == name if isinstance(node, nodes.AnnAssign): diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 27fee55b50..aa8585eee3 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -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, diff --git a/tests/functional/u/used/used_before_assignment_typing.rc b/tests/functional/u/used/used_before_assignment_typing.rc new file mode 100644 index 0000000000..85fc502b37 --- /dev/null +++ b/tests/functional/u/used/used_before_assignment_typing.rc @@ -0,0 +1,2 @@ +[testoptions] +min_pyver=3.8 From 318f30db8115c3ed7d5a1274133c353c26e170f2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 2 Feb 2023 18:04:43 +0000 Subject: [PATCH 10/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/utils.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index e0b39c7c66..aa98db5049 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -2197,13 +2197,11 @@ 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 - ): + 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) + if isinstance(node, (nodes.Import, nodes.ImportFrom)) and any( + node_name[0] == name for node_name in node.names ): return True From 35576ea8a09a8fc9ef4f87e6cd2911e5857541ef Mon Sep 17 00:00:00 2001 From: Zen Lee <53538590+zenlyj@users.noreply.github.com> Date: Fri, 3 Feb 2023 02:51:42 +0800 Subject: [PATCH 11/18] Update comment for easier understanding --- pylint/checkers/variables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index aa8585eee3..6316a58e6e 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2220,7 +2220,7 @@ def _is_variable_violation( ) ): # Exempt those definitions that are used inside the type checking - # guard or that are defined in all 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) if defstmt_parent.has_elif_block(): defined_in_or_else = utils.is_defined( From 352d6de1ecb42bdcbfdfd119edd4844f54a77be1 Mon Sep 17 00:00:00 2001 From: Zen Date: Fri, 3 Feb 2023 04:13:57 +0800 Subject: [PATCH 12/18] Debug is_defined check for except handler name --- pylint/checkers/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index aa98db5049..efec9d1f9e 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -2211,7 +2211,10 @@ def is_defined(name: str, node: nodes.NodeNG) -> bool: for item in node.items ) - if isinstance(node, (nodes.ClassDef, nodes.FunctionDef, nodes.ExceptHandler)): + if isinstance(node, nodes.ExceptHandler): + is_defined_so_far = isinstance(node.name, nodes.AssignName) and node.name.name == name + + if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)): is_defined_so_far = node.name == name if isinstance(node, nodes.AnnAssign): From d1dc39b26b9d8ab2eb51cce3af7edefc03358fb5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 2 Feb 2023 20:16:41 +0000 Subject: [PATCH 13/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index efec9d1f9e..40d88469a0 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -2212,7 +2212,9 @@ def is_defined(name: str, node: nodes.NodeNG) -> bool: ) if isinstance(node, nodes.ExceptHandler): - is_defined_so_far = isinstance(node.name, nodes.AssignName) and node.name.name == name + is_defined_so_far = ( + isinstance(node.name, nodes.AssignName) and node.name.name == name + ) if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)): is_defined_so_far = node.name == name From 0904a521e68549eb17b7e9b20f923bf750efa864 Mon Sep 17 00:00:00 2001 From: Zen Date: Sun, 5 Feb 2023 21:11:45 +0800 Subject: [PATCH 14/18] [cosmetic] Alphabetize imports in functional test --- .../u/used/used_before_assignment_typing.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/functional/u/used/used_before_assignment_typing.py b/tests/functional/u/used/used_before_assignment_typing.py index f3c5eba582..af081daf40 100644 --- a/tests/functional/u/used/used_before_assignment_typing.py +++ b/tests/functional/u/used/used_before_assignment_typing.py @@ -7,34 +7,34 @@ if TYPE_CHECKING: if True: # pylint: disable=using-constant-test import math - import datetime - import calendar - import zoneinfo import array - import types - import enum + import base64 + import binascii + import bisect + import calendar import collections import copy - import pprint - import heapq - import bisect - import weakref - import numbers + import datetime import email + import enum + import heapq import json import mailbox import mimetypes - import base64 - import binascii + import numbers + import pprint + import types + import weakref + import zoneinfo from urllib.request import urlopen elif input(): import calendar, bisect # pylint: disable=multiple-imports if input() + 1: import heapq - elif (enum:=None): + elif (enum := None): pass else: - print(None if (weakref:='') else True) + print(None if (weakref := '') else True) elif input(): try: numbers = None if input() else 1 From 58bdfff2f6e4194fadb033964459339d3c5c46d1 Mon Sep 17 00:00:00 2001 From: Zen Date: Sun, 5 Feb 2023 23:23:06 +0800 Subject: [PATCH 15/18] Shift walrus functional tests to dedicated py38 test file --- .../u/undefined/undefined_variable_py38.py | 20 +++++++++++++++++ .../u/undefined/undefined_variable_py38.txt | 22 +++++++++---------- .../u/used/used_before_assignment_typing.py | 13 ++++------- .../u/used/used_before_assignment_typing.rc | 2 -- .../u/used/used_before_assignment_typing.txt | 10 ++++----- 5 files changed, 40 insertions(+), 27 deletions(-) delete mode 100644 tests/functional/u/used/used_before_assignment_typing.rc diff --git a/tests/functional/u/undefined/undefined_variable_py38.py b/tests/functional/u/undefined/undefined_variable_py38.py index 2612e535f9..ef774e53a6 100644 --- a/tests/functional/u/undefined/undefined_variable_py38.py +++ b/tests/functional/u/undefined/undefined_variable_py38.py @@ -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""" @@ -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 diff --git a/tests/functional/u/undefined/undefined_variable_py38.txt b/tests/functional/u/undefined/undefined_variable_py38.txt index eb979fad23..832d8dd11e 100644 --- a/tests/functional/u/undefined/undefined_variable_py38.txt +++ b/tests/functional/u/undefined/undefined_variable_py38.txt @@ -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 diff --git a/tests/functional/u/used/used_before_assignment_typing.py b/tests/functional/u/used/used_before_assignment_typing.py index af081daf40..1d5f12c244 100644 --- a/tests/functional/u/used/used_before_assignment_typing.py +++ b/tests/functional/u/used/used_before_assignment_typing.py @@ -7,6 +7,7 @@ if TYPE_CHECKING: if True: # pylint: disable=using-constant-test import math + from urllib.request import urlopen import array import base64 import binascii @@ -16,7 +17,6 @@ import copy import datetime import email - import enum import heapq import json import mailbox @@ -24,17 +24,13 @@ import numbers import pprint import types - import weakref import zoneinfo - from urllib.request import urlopen elif input(): import calendar, bisect # pylint: disable=multiple-imports if input() + 1: import heapq - elif (enum := None): - pass else: - print(None if (weakref := '') else True) + import heapq elif input(): try: numbers = None if input() else 1 @@ -180,10 +176,9 @@ def else_branch(self) -> urlopen: print(collections()) return urlopen - def nested_if_else(self) -> enum: + def nested_if_else(self) -> heapq: print(heapq) - print(weakref) - return enum + return heapq def used_in_try_except(self) -> array: print(types) diff --git a/tests/functional/u/used/used_before_assignment_typing.rc b/tests/functional/u/used/used_before_assignment_typing.rc deleted file mode 100644 index 85fc502b37..0000000000 --- a/tests/functional/u/used/used_before_assignment_typing.rc +++ /dev/null @@ -1,2 +0,0 @@ -[testoptions] -min_pyver=3.8 diff --git a/tests/functional/u/used/used_before_assignment_typing.txt b/tests/functional/u/used/used_before_assignment_typing.txt index d091ef2fcd..c0a31fae08 100644 --- a/tests/functional/u/used/used_before_assignment_typing.txt +++ b/tests/functional/u/used/used_before_assignment_typing.txt @@ -1,5 +1,5 @@ -undefined-variable:72:21:72:28:MyClass.incorrect_typing_method:Undefined variable 'MyClass':UNDEFINED -undefined-variable:77:26:77:33:MyClass.incorrect_nested_typing_method:Undefined variable 'MyClass':UNDEFINED -undefined-variable:82:20:82:27:MyClass.incorrect_default_method:Undefined variable 'MyClass':UNDEFINED -used-before-assignment:143:35:143:39:MyFourthClass.is_close:Using variable 'math' before assignment:HIGH -used-before-assignment:156:20:156: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 From 1198e5b86a38c9ed002669cec54cd74d00a423cc Mon Sep 17 00:00:00 2001 From: Zen Date: Sun, 5 Feb 2023 23:51:23 +0800 Subject: [PATCH 16/18] [cosmetic] Rename functions in functional test --- .../u/used/used_before_assignment_typing.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/functional/u/used/used_before_assignment_typing.py b/tests/functional/u/used/used_before_assignment_typing.py index 1d5f12c244..a685bdabc8 100644 --- a/tests/functional/u/used/used_before_assignment_typing.py +++ b/tests/functional/u/used/used_before_assignment_typing.py @@ -165,33 +165,33 @@ class ConditionalImportGuardedWhenUsed: # pylint: disable=too-few-public-method class TypeCheckingMultiBranch: # pylint: disable=too-few-public-methods,unused-variable - """Test imports in TYPE_CHECKING if/elif/else branching""" - def elif_branch(self) -> calendar.Calendar: + """Test for defines in TYPE_CHECKING if/elif/else branching""" + def defined_in_elif_branch(self) -> calendar.Calendar: print(bisect) return calendar.Calendar() - def else_branch(self) -> urlopen: + def defined_in_else_branch(self) -> urlopen: print(zoneinfo) print(pprint()) print(collections()) return urlopen - def nested_if_else(self) -> heapq: + def defined_in_nested_if_else(self) -> heapq: print(heapq) return heapq - def used_in_try_except(self) -> array: + def defined_in_try_except(self) -> array: print(types) print(copy) print(numbers) return array - def used_in_loops(self) -> json: + def defined_in_loops(self) -> json: print(email) print(mailbox) print(mimetypes) return json - def used_in_with(self) -> base64: + def defined_in_with(self) -> base64: print(binascii) return base64 From 4acf0f6cdd4f7cb64332dd2094e023cf3fcd646c Mon Sep 17 00:00:00 2001 From: Zen Date: Mon, 6 Feb 2023 00:24:33 +0800 Subject: [PATCH 17/18] Minor refactor to improve code readability --- pylint/checkers/variables.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 6316a58e6e..e7909f9024 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2222,18 +2222,19 @@ def _is_variable_violation( # Exempt those definitions that are used inside the type checking # guard or that are defined in any elif/else type checking guard branches. used_in_branch = defstmt_parent.parent_of(node) - 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( - utils.is_defined(node.name, content) - for content in defstmt_parent.orelse - ) + 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( + utils.is_defined(node.name, content) + for content in defstmt_parent.orelse + ) - 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 From 2fc8fdaf3577c497a03f341c71d2ea7ba51234cf Mon Sep 17 00:00:00 2001 From: Zen Date: Mon, 6 Feb 2023 00:45:48 +0800 Subject: [PATCH 18/18] Remove exception handler name check in utils.is_defined --- pylint/checkers/utils.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 40d88469a0..062b19a54e 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -2211,11 +2211,6 @@ def is_defined(name: str, node: nodes.NodeNG) -> bool: for item in node.items ) - if isinstance(node, nodes.ExceptHandler): - is_defined_so_far = ( - isinstance(node.name, nodes.AssignName) and node.name.name == name - ) - if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)): is_defined_so_far = node.name == name