From 869837d1486eadaf2c8ebc75efff670518593980 Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 17 Sep 2022 00:55:11 +0200 Subject: [PATCH 1/3] Fix false positive for ``global-variable-not-assigned`` when a global variable is re-assigned via a ``ImportFrom`` node. Closes #4809 --- doc/whatsnew/fragments/4809.false_positive | 3 +++ pylint/checkers/variables.py | 3 ++- tests/functional/g/globals.py | 8 ++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 doc/whatsnew/fragments/4809.false_positive diff --git a/doc/whatsnew/fragments/4809.false_positive b/doc/whatsnew/fragments/4809.false_positive new file mode 100644 index 0000000000..5a531bb318 --- /dev/null +++ b/doc/whatsnew/fragments/4809.false_positive @@ -0,0 +1,3 @@ +Fix false positive for ``global-variable-not-assigned`` when a global variable is re-assigned via a ``ImportFrom`` node. + +Closes #4809 diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index a426ad2470..dc0b6304ba 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -1319,7 +1319,8 @@ def visit_global(self, node: nodes.Global) -> None: assign_nodes = [] not_defined_locally_by_import = not any( - isinstance(local, nodes.Import) for local in locals_.get(name, ()) + isinstance(local, (nodes.Import, nodes.ImportFrom)) + for local in locals_.get(name, ()) ) if ( not utils.is_reassigned_after_current(node, name) diff --git a/tests/functional/g/globals.py b/tests/functional/g/globals.py index 78538c0426..50b0a33c6b 100644 --- a/tests/functional/g/globals.py +++ b/tests/functional/g/globals.py @@ -89,3 +89,11 @@ class CLASS(): pass CLASS() + + +# Regression test for https://github.com/PyCQA/pylint/issues/4809 +# Don't emit global-variable-not-assigned +def import_it(): + global argp, namedtuple + import argparse as argp + from collections import namedtuple From 643d7c95557680b619423dd5ef9c5dd3e2c233ca Mon Sep 17 00:00:00 2001 From: Mark Byrne Date: Sat, 17 Sep 2022 01:06:41 +0200 Subject: [PATCH 2/3] Move test next to the `Import` node case & remove duplicate test. Update `unused-variable` tests. --- tests/functional/g/globals.py | 23 +++++++------------ tests/functional/g/globals.txt | 14 +++++------ tests/functional/u/unused/unused_variable.py | 2 +- tests/functional/u/unused/unused_variable.txt | 3 +-- 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/tests/functional/g/globals.py b/tests/functional/g/globals.py index 50b0a33c6b..3960df6548 100644 --- a/tests/functional/g/globals.py +++ b/tests/functional/g/globals.py @@ -31,9 +31,15 @@ def define_constant(): def global_with_import(): - """should only warn for global-statement""" + """should only warn for global-statement when using `Import` node""" global sys # [global-statement] - import sys # pylint: disable=import-outside-toplevel + import sys + + +def global_with_import_from(): + """should only warn for global-statement when using `ImportFrom` node""" + global namedtuple # [global-statement] + from collections import namedtuple def global_no_assign(): @@ -75,11 +81,6 @@ def FUNC(): FUNC() -def func(): - """Overriding a global with an import should only throw a global statement error""" - global sys # [global-statement] - - import sys def override_class(): """Overriding a class should only throw a global statement error""" @@ -89,11 +90,3 @@ class CLASS(): pass CLASS() - - -# Regression test for https://github.com/PyCQA/pylint/issues/4809 -# Don't emit global-variable-not-assigned -def import_it(): - global argp, namedtuple - import argparse as argp - from collections import namedtuple diff --git a/tests/functional/g/globals.txt b/tests/functional/g/globals.txt index 8c1db54fdf..a378720939 100644 --- a/tests/functional/g/globals.txt +++ b/tests/functional/g/globals.txt @@ -5,10 +5,10 @@ global-variable-not-assigned:23:4:23:14:other:Using global for 'HOP' but no assi undefined-variable:24:10:24:13:other:Undefined variable 'HOP':UNDEFINED global-variable-undefined:29:4:29:18:define_constant:Global variable 'SOMEVAR' undefined at the module level:UNDEFINED global-statement:35:4:35:14:global_with_import:Using the global statement:UNDEFINED -global-variable-not-assigned:41:4:41:19:global_no_assign:Using global for 'CONSTANT' but no assignment is done:UNDEFINED -global-statement:47:4:47:19:global_del:Using the global statement:UNDEFINED -global-statement:54:4:54:19:global_operator_assign:Using the global statement:UNDEFINED -global-statement:61:4:61:19:global_function_assign:Using the global statement:UNDEFINED -global-statement:71:4:71:15:override_func:Using the global statement:UNDEFINED -global-statement:80:4:80:14:func:Using the global statement:UNDEFINED -global-statement:86:4:86:16:override_class:Using the global statement:UNDEFINED +global-statement:41:4:41:21:global_with_import_from:Using the global statement:UNDEFINED +global-variable-not-assigned:47:4:47:19:global_no_assign:Using global for 'CONSTANT' but no assignment is done:UNDEFINED +global-statement:53:4:53:19:global_del:Using the global statement:UNDEFINED +global-statement:60:4:60:19:global_operator_assign:Using the global statement:UNDEFINED +global-statement:67:4:67:19:global_function_assign:Using the global statement:UNDEFINED +global-statement:77:4:77:15:override_func:Using the global statement:UNDEFINED +global-statement:87:4:87:16:override_class:Using the global statement:UNDEFINED diff --git a/tests/functional/u/unused/unused_variable.py b/tests/functional/u/unused/unused_variable.py index c811d00b1c..f81fddefda 100644 --- a/tests/functional/u/unused/unused_variable.py +++ b/tests/functional/u/unused/unused_variable.py @@ -94,7 +94,7 @@ def test_global(): variables through imports. """ # pylint: disable=redefined-outer-name - global PATH, OS, collections, deque # [global-variable-not-assigned, global-variable-not-assigned] + global PATH, OS, collections, deque # [global-statement] from os import path as PATH import os as OS import collections diff --git a/tests/functional/u/unused/unused_variable.txt b/tests/functional/u/unused/unused_variable.txt index 1bea139771..7b1fa834db 100644 --- a/tests/functional/u/unused/unused_variable.txt +++ b/tests/functional/u/unused/unused_variable.txt @@ -13,8 +13,7 @@ unused-import:55:4:55:38:unused_import_from:Unused namedtuple imported from coll unused-import:59:4:59:40:unused_import_in_function:Unused hexdigits imported from string:UNDEFINED unused-variable:64:4:64:10:hello:Unused variable 'my_var':UNDEFINED unused-variable:75:4:75:8:function:Unused variable 'aaaa':UNDEFINED -global-variable-not-assigned:97:4:97:39:test_global:Using global for 'PATH' but no assignment is done:UNDEFINED -global-variable-not-assigned:97:4:97:39:test_global:Using global for 'deque' but no assignment is done:UNDEFINED +global-statement:97:4:97:39:test_global:Using the global statement:UNDEFINED unused-import:103:4:103:28:test_global:Unused platform imported from sys:UNDEFINED unused-import:104:4:104:38:test_global:Unused version imported from sys as VERSION:UNDEFINED unused-import:105:4:105:15:test_global:Unused import this:UNDEFINED From 6a0523939b7d4334b1594aedc7df60cc7ba83f1c Mon Sep 17 00:00:00 2001 From: Mark Byrne <31762852+mbyrnepr2@users.noreply.github.com> Date: Sun, 18 Sep 2022 16:18:05 +0200 Subject: [PATCH 3/3] Update doc/whatsnew/fragments/4809.false_positive Co-authored-by: Jacob Walls --- doc/whatsnew/fragments/4809.false_positive | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/4809.false_positive b/doc/whatsnew/fragments/4809.false_positive index 5a531bb318..a33be0ea50 100644 --- a/doc/whatsnew/fragments/4809.false_positive +++ b/doc/whatsnew/fragments/4809.false_positive @@ -1,3 +1,3 @@ -Fix false positive for ``global-variable-not-assigned`` when a global variable is re-assigned via a ``ImportFrom`` node. +Fix false positive for ``global-variable-not-assigned`` when a global variable is re-assigned via an ``ImportFrom`` node. Closes #4809