Skip to content

Commit 975beed

Browse files
committed
Refactor FunctionArgNamesCheck (N803,N804,N805)
The previous implementation repeated the N803 (lowercase name) check three times. This change reworks the function's logic to avoid code duplication. It also let me remove some nested function calls. In the process, I noticed two additional things that could be improved (but I'm only doing one of them here): 1. We were previously returning after the first naming error, perhaps as an optimization, but I think it's more helpful to return all of the errors we can detect from within this check. 2. The "ignored names" set is (unnecessarily?) used for the N804/N805 first argument checks. We have some test cases that expected this behavior, so I'm retaining it for backwards compatibility.
1 parent 01df3f3 commit 975beed

File tree

1 file changed

+28
-30
lines changed

1 file changed

+28
-30
lines changed

src/pep8ext_naming.py

+28-30
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,6 @@
3131
FUNC_NODES = (ast.FunctionDef, ast.AsyncFunctionDef)
3232

3333

34-
def get_arg_name_tuples(node):
35-
groups = (node.args.posonlyargs, node.args.args, node.args.kwonlyargs)
36-
return [(arg, arg.arg) for args in groups for arg in args]
37-
38-
3934
class _ASTCheckMeta(type):
4035
def __init__(cls, class_name, bases, namespace):
4136
cls.codes = tuple(code for code in namespace if code.startswith('N'))
@@ -347,33 +342,36 @@ class FunctionArgNamesCheck(BaseASTCheck):
347342
N805 = "first argument of a method should be named 'self'"
348343

349344
def visit_functiondef(self, node, parents: Iterable, ignore=None):
350-
351-
def arg_name(arg):
352-
return (arg, arg.arg) if arg else (node, arg)
353-
354-
for arg, name in arg_name(node.args.vararg), arg_name(node.args.kwarg):
355-
if name is None or _ignored(name, ignore):
356-
continue
357-
if name.lower() != name:
358-
yield self.err(arg, 'N803', name=name)
359-
return
360-
361-
arg_name_tuples = get_arg_name_tuples(node)
362-
if not arg_name_tuples:
363-
return
364-
arg0, name0 = arg_name_tuples[0]
365-
function_type = getattr(node, 'function_type', _FunctionType.FUNCTION)
366-
367-
if function_type == _FunctionType.METHOD:
368-
if name0 != 'self' and not _ignored(name0, ignore):
369-
yield self.err(arg0, 'N805')
370-
elif function_type == _FunctionType.CLASSMETHOD:
371-
if name0 != 'cls' and not _ignored(name0, ignore):
372-
yield self.err(arg0, 'N804')
373-
for arg, name in arg_name_tuples:
345+
args = node.args.posonlyargs + node.args.args + node.args.kwonlyargs
346+
347+
# Start by applying checks that are specific to the first argument.
348+
#
349+
# Note: The `ignore` check shouldn't be necessary here because we'd
350+
# expect users to explicitly ignore N804/N805 when using names
351+
# other than `self` and `cls` rather than ignoring names like
352+
# `klass` to get around these checks. However, a previous
353+
# implementation allowed for that, so we retain that behavior
354+
# for backwards compatibility.
355+
if args and (name := args[0].arg) and not _ignored(name, ignore):
356+
function_type = getattr(node, 'function_type', None)
357+
if function_type == _FunctionType.METHOD and name != 'self':
358+
yield self.err(args[0], 'N805')
359+
elif function_type == _FunctionType.CLASSMETHOD and name != 'cls':
360+
yield self.err(args[0], 'N804')
361+
362+
# Also add the special *arg and **kwarg arguments for the rest of the
363+
# checks when they're present. We didn't include them above because
364+
# the "first argument" naming checks shouldn't be applied to them
365+
# when they're the function's only argument(s).
366+
if node.args.vararg:
367+
args.append(node.args.vararg)
368+
if node.args.kwarg:
369+
args.append(node.args.kwarg)
370+
371+
for arg in args:
372+
name = arg.arg
374373
if name.lower() != name and not _ignored(name, ignore):
375374
yield self.err(arg, 'N803', name=name)
376-
return
377375

378376
visit_asyncfunctiondef = visit_functiondef
379377

0 commit comments

Comments
 (0)