Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update disallowed-name to flag module-level variables #7808

Merged
merged 8 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/data/messages/d/disallowed-name/bad.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
def foo(): # [disallowed-name]
pass
print("apples")
4 changes: 2 additions & 2 deletions doc/data/messages/d/disallowed-name/good.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
def something_meaningful():
pass
def print_fruit():
print("apples")
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions doc/data/messages/d/disallowed-name/pylintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[MAIN]
bad-names=foo,bar,baz
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/3701.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Update ``disallowed-name`` check to flag module-level variables.

Closes #3701
16 changes: 14 additions & 2 deletions pylint/checkers/base/name_checker/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,11 @@ def visit_assignname(self, node: nodes.AssignName) -> None:
inferred_assign_type, nodes.Const
):
self._check_name("const", node.name, node)
else:
self._check_name(
"variable", node.name, node, disallowed_check_only=True
)

# Check names defined in AnnAssign nodes
elif isinstance(
assign_type, nodes.AnnAssign
Expand Down Expand Up @@ -520,6 +525,7 @@ def _check_name(
name: str,
node: nodes.NodeNG,
confidence: interfaces.Confidence = interfaces.HIGH,
disallowed_check_only: bool = False,
) -> None:
"""Check for a name using the type's regexp."""

Expand All @@ -534,7 +540,9 @@ def _should_exempt_from_invalid_name(node: nodes.NodeNG) -> bool:
return
if self._name_disallowed_by_regex(name=name):
self.linter.stats.increase_bad_name(node_type, 1)
self.add_message("disallowed-name", node=node, args=name)
self.add_message(
"disallowed-name", node=node, args=name, confidence=interfaces.HIGH
)
return
regexp = self._name_regexps[node_type]
match = regexp.match(name)
Expand All @@ -546,7 +554,11 @@ def _should_exempt_from_invalid_name(node: nodes.NodeNG) -> bool:
warnings = bad_name_group.setdefault(match.lastgroup, []) # type: ignore[union-attr, arg-type]
warnings.append((node, node_type, name, confidence))

if match is None and not _should_exempt_from_invalid_name(node):
if (
match is None
and not disallowed_check_only
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could've been done in other ways, too, including integrating with _should_exempt_from_invalid_name above but I chose this for simplicity. Lmk if another implementation makes more sense.

and not _should_exempt_from_invalid_name(node)
):
self._raise_name_warning(None, node, node_type, name, confidence)

# Check TypeVar names for variance suffixes
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/a/async_functions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ too-many-branches:26:0:26:26:complex_function:Too many branches (13/12):UNDEFINE
too-many-return-statements:26:0:26:26:complex_function:Too many return statements (10/6):UNDEFINED
dangerous-default-value:57:0:57:14:func:Dangerous default value [] as argument:UNDEFINED
duplicate-argument-name:57:18:57:19:func:Duplicate argument name a in function definition:HIGH
disallowed-name:62:0:62:13:foo:"Disallowed name ""foo""":UNDEFINED
disallowed-name:62:0:62:13:foo:"Disallowed name ""foo""":HIGH
empty-docstring:62:0:62:13:foo:Empty function docstring:HIGH
4 changes: 0 additions & 4 deletions tests/functional/b/blacklisted_name.py

This file was deleted.

1 change: 0 additions & 1 deletion tests/functional/b/blacklisted_name.txt

This file was deleted.

11 changes: 11 additions & 0 deletions tests/functional/d/disallowed_name.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# pylint: disable=missing-docstring,too-few-public-methods

def baz(): # [disallowed-name]
pass

class foo(): # [disallowed-name]
pass

foo = {}.keys() # [disallowed-name]
foo = 42 # [disallowed-name]
aaa = 42 # [invalid-name]
5 changes: 5 additions & 0 deletions tests/functional/d/disallowed_name.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
disallowed-name:3:0:3:7:baz:"Disallowed name ""baz""":HIGH
disallowed-name:6:0:6:9:foo:"Disallowed name ""foo""":HIGH
disallowed-name:9:0:9:3::"Disallowed name ""foo""":HIGH
disallowed-name:10:0:10:3::"Disallowed name ""foo""":HIGH
invalid-name:11:0:11:3::"Constant name ""aaa"" doesn't conform to UPPER_CASE naming style":HIGH
4 changes: 2 additions & 2 deletions tests/functional/n/name/name_good_bad_names_regex.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
disallowed-name:5:0:5:26::"Disallowed name ""explicit_bad_some_constant""":UNDEFINED
disallowed-name:5:0:5:26::"Disallowed name ""explicit_bad_some_constant""":HIGH
invalid-name:7:0:7:28::"Constant name ""snake_case_bad_SOME_CONSTANT"" doesn't conform to snake_case naming style ('([^\\W\\dA-Z][^\\WA-Z]*|__.*__)$' pattern)":HIGH
disallowed-name:19:0:19:27:disallowed_2_snake_case:"Disallowed name ""disallowed_2_snake_case""":UNDEFINED
disallowed-name:19:0:19:27:disallowed_2_snake_case:"Disallowed name ""disallowed_2_snake_case""":HIGH
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@


# Usage should not raise a second error
foo = Ε‚os.join("a", "b")
test = Ε‚os.join("a", "b")
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@


# Usage should not raise a second error
foo = Ε‚os("a", "b")
test = Ε‚os("a", "b")
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import os


foo = [
test = [
f"{Ε‚ol} "
for Ε‚ol in os.listdir(".") # [non-ascii-name]
]
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
# +1: [non-ascii-name]
except AttributeError as Ε‚ol:
# Usage should not raise a second error
foo = Ε‚ol
test = Ε‚ol
Original file line number Diff line number Diff line change
@@ -1 +1 @@
non-ascii-name:9:0:11:14::"Variable name ""Ε‚ol"" contains a non-ASCII character, consider renaming it.":HIGH
non-ascii-name:9:0:11:15::"Variable name ""Ε‚ol"" contains a non-ASCII character, consider renaming it.":HIGH
4 changes: 2 additions & 2 deletions tests/functional/u/unused/unused_private_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ class FalsePositive4756a:
def __bar(self, x):
print(x)
fizz = partialmethod(__bar, 'fizz')
foo = FalsePositive4756a()
foo.fizz()
test = FalsePositive4756a()
test.fizz()

class FalsePositive4756b:
def __get_prop(self):
Expand Down
10 changes: 5 additions & 5 deletions tests/functional/u/use/use_maxsplit_arg.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ def get_string(self) -> str:


# Test with accessors
bar = Foo()
get_first = bar.get_string().split(',')[0] # [use-maxsplit-arg]
get_last = bar.get_string().split(',')[-1] # [use-maxsplit-arg]
test = Foo()
get_first = test.get_string().split(',')[0] # [use-maxsplit-arg]
get_last = test.get_string().split(',')[-1] # [use-maxsplit-arg]

get_mid = bar.get_string().split(',')[1]
get_mid = bar.get_string().split(',')[-2]
get_mid = test.get_string().split(',')[1]
get_mid = test.get_string().split(',')[-2]


# Test with iterating over strings
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/u/use/use_maxsplit_arg.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use-maxsplit-arg:45:12:45:36::Use Foo.class_str.split(',', maxsplit=1)[0] instea
use-maxsplit-arg:46:11:46:35::Use Foo.class_str.rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:47:12:47:37::Use Foo.class_str.split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:48:11:48:36::Use Foo.class_str.rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:56:12:56:39::Use bar.get_string().split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:57:11:57:38::Use bar.get_string().rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:56:12:56:40::Use test.get_string().split(',', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:57:11:57:39::Use test.get_string().rsplit(',', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:66:10:66:22::Use s.split(' ', maxsplit=1)[0] instead:UNDEFINED
use-maxsplit-arg:67:10:67:22::Use s.rsplit(' ', maxsplit=1)[-1] instead:UNDEFINED
use-maxsplit-arg:76:6:76:26::Use Bar.split.split(',', maxsplit=1)[0] instead:UNDEFINED
Expand Down
2 changes: 1 addition & 1 deletion tests/testutils/test_lint_module_output_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def test_lint_module_output_update_effective(
assert (expected_output_file).exists()
assert (
expected_output_file.read_text(encoding="utf8")
== 'disallowed-name:1:0:None:None::"Disallowed name ""foo""":UNDEFINED\n'
== 'disallowed-name:1:0:None:None::"Disallowed name ""foo""":HIGH\n'
)


Expand Down