Skip to content

Commit

Permalink
Refactor FunctionArgNamesCheck (N803,N804,N805)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jparise committed Oct 22, 2023
1 parent 01df3f3 commit 2263e07
Showing 1 changed file with 28 additions and 25 deletions.
53 changes: 28 additions & 25 deletions src/pep8ext_naming.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,33 +347,36 @@ class FunctionArgNamesCheck(BaseASTCheck):
N805 = "first argument of a method should be named 'self'"

def visit_functiondef(self, node, parents: Iterable, ignore=None):

def arg_name(arg):
return (arg, arg.arg) if arg else (node, arg)

for arg, name in arg_name(node.args.vararg), arg_name(node.args.kwarg):
if name is None or _ignored(name, ignore):
continue
if name.lower() != name:
yield self.err(arg, 'N803', name=name)
return

arg_name_tuples = get_arg_name_tuples(node)
if not arg_name_tuples:
return
arg0, name0 = arg_name_tuples[0]
function_type = getattr(node, 'function_type', _FunctionType.FUNCTION)

if function_type == _FunctionType.METHOD:
if name0 != 'self' and not _ignored(name0, ignore):
yield self.err(arg0, 'N805')
elif function_type == _FunctionType.CLASSMETHOD:
if name0 != 'cls' and not _ignored(name0, ignore):
yield self.err(arg0, 'N804')
for arg, name in arg_name_tuples:
args = node.args.posonlyargs + node.args.args + node.args.kwonlyargs

# Start by applying checks that are specific to the first argument.
#
# Note: The `ignore` check shouldn't be necessary here because we'd
# expect users to explicitly ignore N804/N805 when using names
# other than `self` and `cls` rather than ignoring names like
# `klass` to get around these checks. However, a previous
# implementation allowed for that, so we retain that behavior
# for backwards compatibility.
if args and (name := args[0].arg) and not _ignored(name, ignore):
function_type = getattr(node, 'function_type', None)
if function_type == _FunctionType.METHOD and name != 'self':
yield self.err(args[0], 'N805')
elif function_type == _FunctionType.CLASSMETHOD and name != 'cls':
yield self.err(args[0], 'N804')

# Also add the special *arg and **kwarg arguments for the rest of the
# checks when they're present. We didn't include them above because
# the "first argument" naming checks shouldn't be applied to them
# when they're the only function arguments.
if node.args.vararg:
args.append(node.args.vararg)
if node.args.kwarg:
args.append(node.args.kwarg)

for arg in args:
name = arg.arg
if name.lower() != name and not _ignored(name, ignore):
yield self.err(arg, 'N803', name=name)
return

visit_asyncfunctiondef = visit_functiondef

Expand Down

0 comments on commit 2263e07

Please sign in to comment.