From da3da38403c3061ff1337460307b4ea15de3279a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sat, 11 Sep 2021 13:38:56 +0200 Subject: [PATCH] Make ``global-variable-not-assigned`` check functions This checker now also checks function defintions in the module and local scope This closes #330 --- ChangeLog | 3 ++- doc/whatsnew/2.11.rst | 3 ++- pylint/checkers/utils.py | 2 +- pylint/checkers/variables.py | 3 +++ tests/functional/g/globals.py | 24 +++++++++++++++++++++++- tests/functional/g/globals.txt | 20 +++++++++++--------- 6 files changed, 42 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0f95359509..61821be252 100644 --- a/ChangeLog +++ b/ChangeLog @@ -78,9 +78,10 @@ Release date: TBA Closes #4900 * The ``global-variable-not-assigned`` checker now catches global variables that are never reassigned in a - local scope + local scope and catches (reassigned) functions Closes #1375 + Closes #330 * Fix ``no-self-use`` and ``docparams extension`` for async functions and methods. diff --git a/doc/whatsnew/2.11.rst b/doc/whatsnew/2.11.rst index 65ff5f3060..6ace2f633d 100644 --- a/doc/whatsnew/2.11.rst +++ b/doc/whatsnew/2.11.rst @@ -81,6 +81,7 @@ Other Changes Closes #4900 * The ``global-variable-not-assigned`` checker now catches global variables that are never reassigned in a - local scope + local scope and catches (reassigned) functions Closes #1375 + Closes #330 diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 22812976ba..6ccbecb4f3 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -1564,5 +1564,5 @@ def is_reassigned_after_current(node: nodes.NodeNG, varname: str) -> bool: """Check if the given variable name is reassigned in the same scope after the current node""" return any( a.name == varname and a.lineno > node.lineno - for a in node.scope().nodes_of_class(nodes.AssignName) + for a in node.scope().nodes_of_class((nodes.AssignName, nodes.FunctionDef)) ) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 9d36a4d418..411e016f5e 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -949,6 +949,9 @@ def visit_global(self, node: nodes.Global) -> None: if anode.frame() is module: # module level assignment break + if isinstance(anode, nodes.FunctionDef) and anode.parent is module: + # module level function assignment + break else: if not_defined_locally_by_import: # global undefined at the module scope diff --git a/tests/functional/g/globals.py b/tests/functional/g/globals.py index 12861c08fa..6440e62fc3 100644 --- a/tests/functional/g/globals.py +++ b/tests/functional/g/globals.py @@ -1,10 +1,13 @@ """Warnings about global statements and usage of global variables.""" +# pylint: disable=invalid-name, redefined-outer-name, missing-function-docstring from __future__ import print_function global CSTE # [global-at-module-level] print(CSTE) # [undefined-variable] CONSTANT = 1 +def FUNC(): + pass def fix_contant(value): """all this is ok, but try not using global ;)""" @@ -25,7 +28,6 @@ def define_constant(): SOMEVAR = 2 -# pylint: disable=invalid-name def global_with_import(): """should only warn for global-statement""" global sys # [global-statement] @@ -43,3 +45,23 @@ def global_operator_assign(): global CONSTANT # [global-statement] print(CONSTANT) CONSTANT += 1 + + +def global_function_assign(): + """Function assigns should only throw a global statement error""" + global CONSTANT # [global-statement] + + def CONSTANT(): + pass + + CONSTANT() + + +def override_func(): + """Overriding a function should only throw a global statement error""" + global FUNC # [global-statement] + + def FUNC(): + pass + + FUNC() diff --git a/tests/functional/g/globals.txt b/tests/functional/g/globals.txt index 1961e5845f..1f07e0e753 100644 --- a/tests/functional/g/globals.txt +++ b/tests/functional/g/globals.txt @@ -1,9 +1,11 @@ -global-at-module-level:4:0::Using the global statement at the module level -undefined-variable:5:6::Undefined variable 'CSTE' -global-statement:11:4:fix_contant:Using the global statement -global-variable-not-assigned:18:4:other:Using global for 'HOP' but no assignment is done -undefined-variable:19:10:other:Undefined variable 'HOP' -global-variable-undefined:24:4:define_constant:Global variable 'SOMEVAR' undefined at the module level -global-statement:31:4:global_with_import:Using the global statement -global-variable-not-assigned:37:4:global_no_assign:Using global for 'CONSTANT' but no assignment is done:HIGH -global-statement:43:4:global_operator_assign:Using the global statement:HIGH +global-at-module-level:5:0::Using the global statement at the module level:HIGH +undefined-variable:6:6::Undefined variable 'CSTE':HIGH +global-statement:14:4:fix_contant:Using the global statement:HIGH +global-variable-not-assigned:21:4:other:Using global for 'HOP' but no assignment is done:HIGH +undefined-variable:22:10:other:Undefined variable 'HOP':HIGH +global-variable-undefined:27:4:define_constant:Global variable 'SOMEVAR' undefined at the module level:HIGH +global-statement:33:4:global_with_import:Using the global statement:HIGH +global-variable-not-assigned:39:4:global_no_assign:Using global for 'CONSTANT' but no assignment is done:HIGH +global-statement:45:4:global_operator_assign:Using the global statement:HIGH +global-statement:52:4:global_function_assign:Using the global statement:HIGH +global-statement:62:4:override_func:Using the global statement:HIGH