diff options
author | Jacob Walls <jacobtylerwalls@gmail.com> | 2021-12-11 03:10:28 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-12-11 09:10:28 +0100 |
commit | bd55b27d41542e3ca1f031f986b6151f6cac457f (patch) | |
tree | 4600946172fe87d234cae6cf784ee66734cf3362 | |
parent | a51a5486ebff7543ae4fb6943fac2558947fa974 (diff) | |
download | pylint-git-bd55b27d41542e3ca1f031f986b6151f6cac457f.tar.gz |
Fix #4761: Emit `used-before-assignment` where single assignment only made in except blocks (#5402)
Co-authored-by: Daniƫl van Noord <13665637+DanielNoord@users.noreply.github.com>
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
-rw-r--r-- | ChangeLog | 13 | ||||
-rw-r--r-- | doc/whatsnew/2.13.rst | 7 | ||||
-rw-r--r-- | pylint/checkers/variables.py | 67 | ||||
-rw-r--r-- | pylintrc | 2 | ||||
-rw-r--r-- | tests/functional/u/undefined/undefined_variable.py | 4 | ||||
-rw-r--r-- | tests/functional/u/undefined/undefined_variable.txt | 2 | ||||
-rw-r--r-- | tests/functional/u/use/used_before_assignment_issue4761.py | 12 | ||||
-rw-r--r-- | tests/functional/u/use/used_before_assignment_issue4761.txt | 1 | ||||
-rw-r--r-- | tests/lint/test_utils.py | 2 |
9 files changed, 97 insertions, 13 deletions
@@ -11,6 +11,13 @@ Release date: TBA .. Put new features here and also in 'doc/whatsnew/2.13.rst' +* ``used-before-assignment`` now assumes that assignments in except blocks + may not have occurred and warns accordingly. + + Closes #4761 + +* ``used-before-assignment`` now checks names in try blocks. + * Some files in ``pylint.testutils`` were deprecated. In the future imports should be done from the ``pylint.testutils.functional`` namespace directly. @@ -18,6 +25,9 @@ Release date: TBA Closes #4716 +* The ``PyLinter`` class will now be initialiazed with a ``TextReporter`` + as its reporter if none is provided. + * Fix false positive - Allow unpacking of ``self`` in a subclass of ``typing.NamedTuple``. Closes #5312 @@ -36,9 +46,6 @@ Release date: TBA Insert your changelog randomly, it will reduce merge conflicts (Ie. not necessarily at the end) -* The ``PyLinter`` class will now be initialiazed with a ``TextReporter`` - as its reporter if none is provided. - What's New in Pylint 2.12.2? ============================ diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst index 0713e343a..55535638e 100644 --- a/doc/whatsnew/2.13.rst +++ b/doc/whatsnew/2.13.rst @@ -31,6 +31,13 @@ Other Changes Closes #5323 +* ``used-before-assignment`` now assumes that assignments in except blocks + may not have occurred and warns accordingly. + + Closes #4761 + +* ``used-before-assignment`` now checks names in try blocks. + * Require Python ``3.6.2`` to run pylint. Closes #5065 diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index f2bba355c..13b8589af 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -534,7 +534,7 @@ MSGS = { ScopeConsumer = collections.namedtuple( - "ScopeConsumer", "to_consume consumed scope_type" + "ScopeConsumer", "to_consume consumed consumed_uncertain scope_type" ) @@ -544,17 +544,24 @@ class NamesConsumer: """ def __init__(self, node, scope_type): - self._atomic = ScopeConsumer(copy.copy(node.locals), {}, scope_type) + self._atomic = ScopeConsumer( + copy.copy(node.locals), {}, collections.defaultdict(list), scope_type + ) self.node = node def __repr__(self): to_consumes = [f"{k}->{v}" for k, v in self._atomic.to_consume.items()] consumed = [f"{k}->{v}" for k, v in self._atomic.consumed.items()] + consumed_uncertain = [ + f"{k}->{v}" for k, v in self._atomic.consumed_uncertain.items() + ] to_consumes = ", ".join(to_consumes) consumed = ", ".join(consumed) + consumed_uncertain = ", ".join(consumed_uncertain) return f""" to_consume : {to_consumes} consumed : {consumed} +consumed_uncertain: {consumed_uncertain} scope_type : {self._atomic.scope_type} """ @@ -570,6 +577,19 @@ scope_type : {self._atomic.scope_type} return self._atomic.consumed @property + def consumed_uncertain(self) -> DefaultDict[str, List[nodes.NodeNG]]: + """ + Retrieves nodes filtered out by get_next_to_consume() that may not + have executed, such as statements in except blocks. Checkers that + want to treat the statements as executed (e.g. for unused-variable) + may need to add them back. + + TODO: A pending PR will extend this to nodes in try blocks when + evaluating their corresponding except and finally blocks. + """ + return self._atomic.consumed_uncertain + + @property def scope_type(self): return self._atomic.scope_type @@ -589,7 +609,9 @@ scope_type : {self._atomic.scope_type} def get_next_to_consume(self, node): """ - Return a list of the nodes that define `node` from this scope. + Return a list of the nodes that define `node` from this scope. If it is + uncertain whether a node will be consumed, such as for statements in + except blocks, add it to self.consumed_uncertain instead of returning it. Return None to indicate a special case that needs to be handled by the caller. """ name = node.name @@ -617,9 +639,28 @@ scope_type : {self._atomic.scope_type} found_nodes = [ n for n in found_nodes - if not isinstance(n.statement(), nodes.ExceptHandler) - or n.statement().parent_of(node) + if not isinstance(n.statement(future=True), nodes.ExceptHandler) + or n.statement(future=True).parent_of(node) + ] + + # Filter out assignments in an Except clause that the node is not + # contained in, assuming they may fail + if found_nodes: + filtered_nodes = [ + n + for n in found_nodes + if not ( + isinstance(n.statement(future=True).parent, nodes.ExceptHandler) + and isinstance( + n.statement(future=True).parent.parent, nodes.TryExcept + ) + ) + or n.statement(future=True).parent.parent_of(node) ] + filtered_nodes_set = set(filtered_nodes) + difference = [n for n in found_nodes if n not in filtered_nodes_set] + self.consumed_uncertain[node.name] += difference + found_nodes = filtered_nodes return found_nodes @@ -1042,6 +1083,14 @@ class VariablesChecker(BaseChecker): if action is VariableVisitConsumerAction.CONTINUE: continue if action is VariableVisitConsumerAction.CONSUME: + # pylint: disable-next=fixme + # TODO: remove assert after _check_consumer return value better typed + assert found_nodes is not None, "Cannot consume an empty list of nodes." + # Any nodes added to consumed_uncertain by get_next_to_consume() + # should be added back so that they are marked as used. + # They will have already had a chance to emit used-before-assignment. + # We check here instead of before every single return in _check_consumer() + found_nodes += current_consumer.consumed_uncertain[node.name] current_consumer.mark_as_consumed(node.name, found_nodes) if action in { VariableVisitConsumerAction.RETURN, @@ -1135,6 +1184,12 @@ class VariablesChecker(BaseChecker): return (VariableVisitConsumerAction.CONTINUE, None) if not found_nodes: self.add_message("used-before-assignment", args=node.name, node=node) + if current_consumer.consumed_uncertain[node.name]: + # If there are nodes added to consumed_uncertain by + # get_next_to_consume() because they might not have executed, + # return a CONSUME action so that _undefined_and_used_before_checker() + # will mark them as used + return (VariableVisitConsumerAction.CONSUME, found_nodes) return (VariableVisitConsumerAction.RETURN, found_nodes) self._check_late_binding_closure(node) @@ -2370,7 +2425,7 @@ class VariablesChecker(BaseChecker): name = METACLASS_NAME_TRANSFORMS.get(name, name) if name: # check enclosing scopes starting from most local - for scope_locals, _, _ in self._to_consume[::-1]: + for scope_locals, _, _, _ in self._to_consume[::-1]: found_nodes = scope_locals.get(name, []) for found_node in found_nodes: if found_node.lineno <= klass.lineno: @@ -332,7 +332,7 @@ max-locals=25 max-returns=11 # Maximum number of branch for function / method body -max-branches=26 +max-branches=27 # Maximum number of statements in function / method body max-statements=100 diff --git a/tests/functional/u/undefined/undefined_variable.py b/tests/functional/u/undefined/undefined_variable.py index f4f22e523..6ce9aaa6e 100644 --- a/tests/functional/u/undefined/undefined_variable.py +++ b/tests/functional/u/undefined/undefined_variable.py @@ -35,7 +35,7 @@ LMBD = lambda x, y=doesnotexist: x+y # [undefined-variable] LMBD2 = lambda x, y: x+z # [undefined-variable] try: - POUET # don't catch me + POUET # [used-before-assignment] except NameError: POUET = 'something' @@ -45,7 +45,7 @@ except Exception: # pylint:disable = broad-except POUETT = 'something' try: - POUETTT # don't catch me + POUETTT # [used-before-assignment] except: # pylint:disable = bare-except POUETTT = 'something' diff --git a/tests/functional/u/undefined/undefined_variable.txt b/tests/functional/u/undefined/undefined_variable.txt index c5a70f7a7..bee5ab475 100644 --- a/tests/functional/u/undefined/undefined_variable.txt +++ b/tests/functional/u/undefined/undefined_variable.txt @@ -8,7 +8,9 @@ undefined-variable:31:4:31:10:bad_default:Undefined variable 'augvar':UNDEFINED undefined-variable:32:8:32:14:bad_default:Undefined variable 'vardel':UNDEFINED undefined-variable:34:19:34:31:<lambda>:Undefined variable 'doesnotexist':UNDEFINED undefined-variable:35:23:35:24:<lambda>:Undefined variable 'z':UNDEFINED +used-before-assignment:38:4:38:9::Using variable 'POUET' before assignment:UNDEFINED used-before-assignment:43:4:43:10::Using variable 'POUETT' before assignment:UNDEFINED +used-before-assignment:48:4:48:11::Using variable 'POUETTT' before assignment:UNDEFINED used-before-assignment:56:4:56:9::Using variable 'PLOUF' before assignment:UNDEFINED used-before-assignment:65:11:65:14:if_branch_test:Using variable 'xxx' before assignment:UNDEFINED used-before-assignment:91:23:91:32:test_arguments:Using variable 'TestClass' before assignment:UNDEFINED diff --git a/tests/functional/u/use/used_before_assignment_issue4761.py b/tests/functional/u/use/used_before_assignment_issue4761.py new file mode 100644 index 000000000..ab6fc765b --- /dev/null +++ b/tests/functional/u/use/used_before_assignment_issue4761.py @@ -0,0 +1,12 @@ +"""used-before-assignment (E0601)""" +def function(): + """Consider that except blocks may not execute.""" + try: + pass + except ValueError: + some_message = 'some message' + + if not some_message: # [used-before-assignment] + return 1 + + return some_message diff --git a/tests/functional/u/use/used_before_assignment_issue4761.txt b/tests/functional/u/use/used_before_assignment_issue4761.txt new file mode 100644 index 000000000..cf8fd209a --- /dev/null +++ b/tests/functional/u/use/used_before_assignment_issue4761.txt @@ -0,0 +1 @@ +used-before-assignment:9:11:9:23:function:Using variable 'some_message' before assignment:UNDEFINED diff --git a/tests/lint/test_utils.py b/tests/lint/test_utils.py index f7294f353..44703982e 100644 --- a/tests/lint/test_utils.py +++ b/tests/lint/test_utils.py @@ -15,7 +15,7 @@ def test_prepare_crash_report(tmp_path: PosixPath) -> None: template_path = prepare_crash_report( ex, str(python_file), str(tmp_path / "pylint-crash-%Y.txt") ) - assert str(tmp_path) in str(template_path) + assert str(tmp_path) in str(template_path) # pylint: disable=used-before-assignment with open(template_path, encoding="utf8") as f: template_content = f.read() assert python_content in template_content |