Skip to content

Commit

Permalink
Allow used variables to be properly consumed when different checks ar…
Browse files Browse the repository at this point in the history
…e enabled / disabled

When everything else was disabled, except `unused-import`, pylint was emitting `unused-import`
even though the imports in questions were used.
The emission occurred due to the fact that disabling all the messages also disabled the calling
of `visit_name`, which deals with `undefined-variable`. This resulted in `visit_name` not marking
as consumed the earlier import as expected.
This fix still allows `visit_name` to be called, but the emission of `undefined-variable` and friends
is controlled via a flag prior to emission call site.

This is somewhat of a "temporary" hack, a better solution would be to separate the emission / checking
of undefined variable from marking the variables as consumed.

Close #3445
  • Loading branch information
PCManticore committed Mar 25, 2020
1 parent 4024949 commit c518188
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 8 deletions.
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ Release date: TBA

Close #2725

* Allow used variables to be properly consumed when different checks are enabled / disabled

Close #3445

* Fix dangerous-default-value rule to account for keyword argument defaults

Close #3373
Expand Down
23 changes: 15 additions & 8 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -936,12 +936,6 @@ def visit_assignname(self, node):
def visit_delname(self, node):
self.visit_name(node)

@utils.check_messages(
"cell-var-from-loop",
"undefined-loop-variable",
"undefined-variable",
"used-before-assignment",
)
def visit_name(self, node):
"""Check that a name is defined in the current scope"""
stmt = node.statement()
Expand All @@ -963,6 +957,14 @@ def visit_name(self, node):
start_index = len(self._to_consume) - 2
else:
start_index = len(self._to_consume) - 1

undefined_variable_is_enabled = self.linter.is_message_enabled(
"undefined-variable"
)
used_before_assignment_is_enabled = self.linter.is_message_enabled(
"used-before-assignment"
)

# iterates through parent scopes, from the inner to the outer
base_scope_type = self._to_consume[start_index].scope_type
# pylint: disable=too-many-nested-blocks; refactoring this block is a pain.
Expand Down Expand Up @@ -1001,7 +1003,9 @@ def visit_name(self, node):
# checks for use before assignment
defnode = utils.assign_parent(current_consumer.to_consume[name][0])

if defnode is not None:
if (
undefined_variable_is_enabled or used_before_assignment_is_enabled
) and defnode is not None:
self._check_late_binding_closure(node, defnode)
defstmt = defnode.statement()
defframe = defstmt.frame()
Expand Down Expand Up @@ -1136,7 +1140,7 @@ def visit_name(self, node):
else:
# we have not found the name, if it isn't a builtin, that's an
# undefined name !
if not (
if undefined_variable_is_enabled and not (
name in astroid.Module.scope_attrs
or utils.is_builtin(name)
or name in self.config.additional_builtins
Expand Down Expand Up @@ -1637,6 +1641,9 @@ def _check_unused_arguments(self, name, node, stmt, argnames):
self.add_message("unused-argument", args=name, node=stmt, confidence=confidence)

def _check_late_binding_closure(self, node, assignment_node):
if not self.linter.is_message_enabled("cell-var-from-loop"):
return

def _is_direct_lambda_call():
return (
isinstance(node_scope.parent, astroid.Call)
Expand Down
8 changes: 8 additions & 0 deletions tests/functional/u/unused_import_everything_disabled.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
"""Test that unused-import is not emitted here when everything else is disabled
https://github.com/PyCQA/pylint/issues/3445
"""
from os import environ

for k, v in environ.items():
print(k, v)
3 changes: 3 additions & 0 deletions tests/functional/u/unused_import_everything_disabled.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[Messages Control]
disable=all
enable=unused-import

0 comments on commit c518188

Please sign in to comment.