diff --git a/ChangeLog b/ChangeLog index 186689ec45..b2b8e15d8c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -174,6 +174,16 @@ Release date: TBA Closes #578 +* Added optional extension ``redefined-loop-name`` to emit messages when a loop variable + is redefined in the loop body. + + Closes #5072 + +* Changed message type from ``redefined-outer-name`` to ``redefined-loop-name`` + (optional extension) for redefinitions of outer loop variables by inner loops. + + Closes #5608 + * The ``ignore-mixin-members`` option has been deprecated. You should now use the new ``ignored-checks-for-mixins`` option. diff --git a/doc/data/messages/r/redefined-loop-name/bad.py b/doc/data/messages/r/redefined-loop-name/bad.py new file mode 100644 index 0000000000..1299a499c0 --- /dev/null +++ b/doc/data/messages/r/redefined-loop-name/bad.py @@ -0,0 +1,2 @@ +for name in names: + name = name.lower() # [redefined-loop-name] diff --git a/doc/data/messages/r/redefined-loop-name/good.py b/doc/data/messages/r/redefined-loop-name/good.py new file mode 100644 index 0000000000..ae24348cea --- /dev/null +++ b/doc/data/messages/r/redefined-loop-name/good.py @@ -0,0 +1,2 @@ +for name in names: + lowercased_name = name.lower() diff --git a/doc/data/messages/r/redefined-loop-name/pylintrc b/doc/data/messages/r/redefined-loop-name/pylintrc new file mode 100644 index 0000000000..2785fc59e0 --- /dev/null +++ b/doc/data/messages/r/redefined-loop-name/pylintrc @@ -0,0 +1,2 @@ +[MASTER] +load-plugins=pylint.extensions.redefined_loop_name, diff --git a/doc/whatsnew/2.14.rst b/doc/whatsnew/2.14.rst index afa590bb28..b7030f110a 100644 --- a/doc/whatsnew/2.14.rst +++ b/doc/whatsnew/2.14.rst @@ -46,6 +46,11 @@ New checkers Closes #4525 +* Added optional extension ``redefined-loop-name`` to emit messages when a loop variable + is redefined in the loop body. + + Closes #5072 + * Added new message called ``duplicate-value`` which identifies duplicate values inside sets. Closes #5880 @@ -226,3 +231,8 @@ Other Changes attribute to itself without any prior assignment. Closes #1555 + +* Changed message type from ``redefined-outer-name`` to ``redefined-loop-name`` + (optional extension) for redefinitions of outer loop variables by inner loops. + + Closes #5608 diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index dee3fe9cea..2b1e771c72 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -1743,3 +1743,11 @@ def in_type_checking_block(node: nodes.NodeNG) -> bool: and ancestor.test.as_string() in TYPING_TYPE_CHECKS_GUARDS for ancestor in node.node_ancestors() ) + + +@lru_cache() +def in_for_else_branch(parent: nodes.NodeNG, stmt: nodes.Statement) -> bool: + """Returns True if stmt is inside the else branch for a parent For stmt.""" + return isinstance(parent, nodes.For) and any( + else_stmt.parent_of(stmt) or else_stmt == stmt for else_stmt in parent.orelse + ) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index a52daab6cc..41667efc7b 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -132,13 +132,6 @@ def _is_from_future_import(stmt, name): return None -def in_for_else_branch(parent, stmt): - """Returns True if stmt in inside the else branch for a parent For stmt.""" - return isinstance(parent, nodes.For) and any( - else_stmt.parent_of(stmt) or else_stmt == stmt for else_stmt in parent.orelse - ) - - @lru_cache(maxsize=1000) def overridden_method(klass, name): """Get overridden method if any.""" @@ -448,7 +441,7 @@ def _has_locals_call_after_node(stmt, scope): "W0621": ( "Redefining name %r from outer scope (line %s)", "redefined-outer-name", - "Used when a variable's name hides a name defined in the outer scope.", + "Used when a variable's name hides a name defined in an outer scope or except handler.", ), "W0622": ( "Redefining built-in %r", @@ -960,7 +953,7 @@ class VariablesChecker(BaseChecker): Checks for * unused variables / imports * undefined variables - * redefinition of variable from builtins or from an outer scope + * redefinition of variable from builtins or from an outer scope or except handler * use of variable before assignment * __all__ consistency * self/cls assignment @@ -1061,7 +1054,6 @@ def __init__(self, linter=None): super().__init__(linter) self._to_consume: list[NamesConsumer] = [] self._checking_mod_attr = None - self._loop_variables = [] self._type_annotation_names = [] self._except_handler_names_queue: list[ tuple[nodes.ExceptHandler, nodes.AssignName] @@ -1078,31 +1070,7 @@ def open(self) -> None: "undefined-loop-variable" ) - @utils.only_required_for_messages("redefined-outer-name") - def visit_for(self, node: nodes.For) -> None: - assigned_to = [a.name for a in node.target.nodes_of_class(nodes.AssignName)] - - # Only check variables that are used - dummy_rgx = self.linter.config.dummy_variables_rgx - assigned_to = [var for var in assigned_to if not dummy_rgx.match(var)] - - for variable in assigned_to: - for outer_for, outer_variables in self._loop_variables: - if variable in outer_variables and not in_for_else_branch( - outer_for, node - ): - self.add_message( - "redefined-outer-name", - args=(variable, outer_for.fromlineno), - node=node, - ) - break - - self._loop_variables.append((node, assigned_to)) - - @utils.only_required_for_messages("redefined-outer-name") def leave_for(self, node: nodes.For) -> None: - self._loop_variables.pop() self._store_type_annotation_names(node) def visit_module(self, node: nodes.Module) -> None: @@ -2227,7 +2195,7 @@ def _loopvar_name(self, node: astroid.Name) -> None: for i, stmt in enumerate(astmts[1:]): if astmts[i].statement(future=True).parent_of( stmt - ) and not in_for_else_branch(astmts[i].statement(future=True), stmt): + ) and not utils.in_for_else_branch(astmts[i].statement(future=True), stmt): continue _astmts.append(stmt) astmts = _astmts diff --git a/pylint/extensions/redefined_loop_name.py b/pylint/extensions/redefined_loop_name.py new file mode 100644 index 0000000000..9bcfffce69 --- /dev/null +++ b/pylint/extensions/redefined_loop_name.py @@ -0,0 +1,89 @@ +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE +# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt + +"""Optional checker to warn when loop variables are overwritten in the loop's body.""" + +from __future__ import annotations + +from astroid import nodes + +from pylint import checkers +from pylint.checkers import utils +from pylint.interfaces import HIGH +from pylint.lint import PyLinter + + +class RedefinedLoopName(checkers.BaseChecker): + + name = "redefined-loop-name" + + msgs = { + "W2901": ( + "Redefining %r from loop (line %s)", + "redefined-loop-name", + "Used when a loop variable is overwritten in the loop body.", + ), + } + + def __init__(self, linter: PyLinter) -> None: + super().__init__(linter) + self._loop_variables: list[ + tuple[nodes.For, list[str], nodes.LocalsDictNodeNG] + ] = [] + + @utils.only_required_for_messages("redefined-loop-name") + def visit_assignname(self, node: nodes.AssignName) -> None: + assign_type = node.assign_type() + if not isinstance(assign_type, (nodes.Assign, nodes.AugAssign)): + return + node_scope = node.scope() + for outer_for, outer_variables, outer_for_scope in self._loop_variables: + if node_scope is not outer_for_scope: + continue + if node.name in outer_variables and not utils.in_for_else_branch( + outer_for, node + ): + self.add_message( + "redefined-loop-name", + args=(node.name, outer_for.fromlineno), + node=node, + confidence=HIGH, + ) + break + + @utils.only_required_for_messages("redefined-loop-name") + def visit_for(self, node: nodes.For) -> None: + assigned_to = [a.name for a in node.target.nodes_of_class(nodes.AssignName)] + # Only check variables that are used + assigned_to = [ + var + for var in assigned_to + if not self.linter.config.dummy_variables_rgx.match(var) + ] + + node_scope = node.scope() + for variable in assigned_to: + for outer_for, outer_variables, outer_for_scope in self._loop_variables: + if node_scope is not outer_for_scope: + continue + if variable in outer_variables and not utils.in_for_else_branch( + outer_for, node + ): + self.add_message( + "redefined-loop-name", + args=(variable, outer_for.fromlineno), + node=node, + confidence=HIGH, + ) + break + + self._loop_variables.append((node, assigned_to, node.scope())) + + @utils.only_required_for_messages("redefined-loop-name") + def leave_for(self, node: nodes.For) -> None: # pylint: disable=unused-argument + self._loop_variables.pop() + + +def register(linter: PyLinter) -> None: + linter.register_checker(RedefinedLoopName(linter)) diff --git a/tests/functional/c/cellvar_escaping_loop.py b/tests/functional/c/cellvar_escaping_loop.py index d7485f7cfc..c3f7c030d9 100644 --- a/tests/functional/c/cellvar_escaping_loop.py +++ b/tests/functional/c/cellvar_escaping_loop.py @@ -72,7 +72,7 @@ def good_case9(): def good_case10(): - """Ignore when a loop variable is showdowed by an inner function""" + """Ignore when a loop variable is shadowed by an inner function""" lst = [] for i in range(10): # pylint: disable=unused-variable def func(): diff --git a/tests/functional/ext/redefined_loop_name/redefined_loop_name.py b/tests/functional/ext/redefined_loop_name/redefined_loop_name.py new file mode 100644 index 0000000000..0af49f9bfd --- /dev/null +++ b/tests/functional/ext/redefined_loop_name/redefined_loop_name.py @@ -0,0 +1,53 @@ +"""Tests for redefinitions of loop variables inside the loop body. + +See: https://github.com/PyCQA/pylint/issues/5608 +""" +# pylint: disable=invalid-name + +lines = ["1\t", "2\t"] +for line in lines: + line = line.strip() # [redefined-loop-name] + +for i in range(8): + for j in range(8): + for i in range(j): # [redefined-loop-name] + j = i # [redefined-loop-name] + print(i, j) + +for i in range(8): + for j in range(8): + for k in range(j): + k = (i, j) # [redefined-loop-name] + print(i, j, k) + +lines = [(1, "1\t"), (2, "2\t")] +for i, line in lines: + line = line.strip() # [redefined-loop-name] + +line = "no warning" + +for i in range(10): + i += 1 # [redefined-loop-name] + + +def outer(): + """No redefined-loop-name for variables in nested scopes""" + for i1 in range(5): + def inner(): + """No warning, because i has a new scope""" + for i1 in range(3): + print(i1) + print(i1) + inner() + + +def outer2(): + """Similar, but with an assignment instead of homonymous loop variables""" + for i1 in range(5): + def inner(): + """No warning, because i has a new scope""" + for _ in range(3): + i1 = 0 + print(i1) + print(i1) + inner() diff --git a/tests/functional/ext/redefined_loop_name/redefined_loop_name.rc b/tests/functional/ext/redefined_loop_name/redefined_loop_name.rc new file mode 100644 index 0000000000..2785fc59e0 --- /dev/null +++ b/tests/functional/ext/redefined_loop_name/redefined_loop_name.rc @@ -0,0 +1,2 @@ +[MASTER] +load-plugins=pylint.extensions.redefined_loop_name, diff --git a/tests/functional/ext/redefined_loop_name/redefined_loop_name.txt b/tests/functional/ext/redefined_loop_name/redefined_loop_name.txt new file mode 100644 index 0000000000..58e47fc758 --- /dev/null +++ b/tests/functional/ext/redefined_loop_name/redefined_loop_name.txt @@ -0,0 +1,6 @@ +redefined-loop-name:9:4:9:8::Redefining 'line' from loop (line 8):HIGH +redefined-loop-name:13:8:15:23::Redefining 'i' from loop (line 11):HIGH +redefined-loop-name:14:12:14:13::Redefining 'j' from loop (line 12):HIGH +redefined-loop-name:20:12:20:13::Redefining 'k' from loop (line 19):HIGH +redefined-loop-name:25:4:25:8::Redefining 'line' from loop (line 24):HIGH +redefined-loop-name:30:4:30:5::Redefining 'i' from loop (line 29):HIGH diff --git a/tests/functional/r/reused_outer_loop_variable.py b/tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.py similarity index 68% rename from tests/functional/r/reused_outer_loop_variable.py rename to tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.py index b244e99f5c..1a62749768 100644 --- a/tests/functional/r/reused_outer_loop_variable.py +++ b/tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.py @@ -4,22 +4,22 @@ # Simple nested loop for i in range(10): - for i in range(10): #[redefined-outer-name] + for i in range(10): #[redefined-loop-name] print(i) # When outer loop unpacks a tuple for i, i_again in enumerate(range(10)): - for i in range(10): #[redefined-outer-name] + for i in range(10): #[redefined-loop-name] print(i, i_again) # When inner loop unpacks a tuple for i in range(10): - for i, i_again in range(10): #[redefined-outer-name] + for i, i_again in range(10): #[redefined-loop-name] print(i, i_again) # With nested tuple unpacks for (a, (b, c)) in [(1, (2, 3))]: - for i, a in range(10): #[redefined-outer-name] + for i, a in range(10): #[redefined-loop-name] print(i, a, b, c) # Ignores when in else @@ -35,3 +35,8 @@ for _ in range(10): for _ in range(10): print("Hello") + +# Unpacking +for i, *j in [(1, 2, 3, 4)]: + for j in range(i): # [redefined-loop-name] + print(j) diff --git a/tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.rc b/tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.rc new file mode 100644 index 0000000000..2785fc59e0 --- /dev/null +++ b/tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.rc @@ -0,0 +1,2 @@ +[MASTER] +load-plugins=pylint.extensions.redefined_loop_name, diff --git a/tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.txt b/tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.txt new file mode 100644 index 0000000000..ea26e01d1f --- /dev/null +++ b/tests/functional/ext/redefined_loop_name/reused_outer_loop_variable.txt @@ -0,0 +1,5 @@ +redefined-loop-name:7:4:8:16::Redefining 'i' from loop (line 6):HIGH +redefined-loop-name:12:4:13:25::Redefining 'i' from loop (line 11):HIGH +redefined-loop-name:17:4:18:25::Redefining 'i' from loop (line 16):HIGH +redefined-loop-name:22:4:23:25::Redefining 'a' from loop (line 21):HIGH +redefined-loop-name:41:4:42:16::Redefining 'j' from loop (line 40):HIGH diff --git a/tests/functional/r/redefine_loop.py b/tests/functional/r/redefine_loop.py index 012ba136ba..be7b512052 100644 --- a/tests/functional/r/redefine_loop.py +++ b/tests/functional/r/redefine_loop.py @@ -1,7 +1,8 @@ -"""Test case for variable redefined in inner loop.""" +"""No message is emitted for variables overwritten in inner loops by default. +See redefined-loop-name optional checker.""" for item in range(0, 5): print("hello") - for item in range(5, 10): #[redefined-outer-name] + for item in range(5, 10): print(item) print("yay") print(item) diff --git a/tests/functional/r/redefine_loop.txt b/tests/functional/r/redefine_loop.txt deleted file mode 100644 index 358389c0f3..0000000000 --- a/tests/functional/r/redefine_loop.txt +++ /dev/null @@ -1 +0,0 @@ -redefined-outer-name:4:4:6:20::Redefining name 'item' from outer scope (line 2):UNDEFINED diff --git a/tests/functional/r/reused_outer_loop_variable.txt b/tests/functional/r/reused_outer_loop_variable.txt deleted file mode 100644 index 3729dc776a..0000000000 --- a/tests/functional/r/reused_outer_loop_variable.txt +++ /dev/null @@ -1,4 +0,0 @@ -redefined-outer-name:7:4:8:16::Redefining name 'i' from outer scope (line 6):UNDEFINED -redefined-outer-name:12:4:13:25::Redefining name 'i' from outer scope (line 11):UNDEFINED -redefined-outer-name:17:4:18:25::Redefining name 'i' from outer scope (line 16):UNDEFINED -redefined-outer-name:22:4:23:25::Redefining name 'a' from outer scope (line 21):UNDEFINED diff --git a/tests/functional/r/reused_outer_loop_variable_py3.py b/tests/functional/r/reused_outer_loop_variable_py3.py deleted file mode 100644 index 6eaa1f51c2..0000000000 --- a/tests/functional/r/reused_outer_loop_variable_py3.py +++ /dev/null @@ -1,5 +0,0 @@ -"""Python >= 3 tests for redefining an outer loop's variable.""" - -for i, *j in [(1, 2, 3, 4)]: - for j in range(i): #[redefined-outer-name] - print(j) diff --git a/tests/functional/r/reused_outer_loop_variable_py3.txt b/tests/functional/r/reused_outer_loop_variable_py3.txt deleted file mode 100644 index acaa9c847f..0000000000 --- a/tests/functional/r/reused_outer_loop_variable_py3.txt +++ /dev/null @@ -1 +0,0 @@ -redefined-outer-name:4:4:5:16::Redefining name 'j' from outer scope (line 3):UNDEFINED diff --git a/tests/functional/t/too/too_many_nested_blocks.py b/tests/functional/t/too/too_many_nested_blocks.py index e7c0079578..24c8fe4ef0 100644 --- a/tests/functional/t/too/too_many_nested_blocks.py +++ b/tests/functional/t/too/too_many_nested_blocks.py @@ -9,7 +9,7 @@ def my_function(): while True: try: if True: - i += 1 + print(i) except IOError: pass @@ -18,7 +18,7 @@ def my_function(): if i == 2: while True: try: - i += 1 + print(i) except IOError: pass