Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Check for extra/extraneous arguments #622

Open
jveitchmichaelis opened this issue Jan 7, 2023 · 3 comments
Open

Check for extra/extraneous arguments #622

jveitchmichaelis opened this issue Jan 7, 2023 · 3 comments

Comments

@jveitchmichaelis
Copy link

jveitchmichaelis commented Jan 7, 2023

It would be useful to check for parameters in the docstring that are not in the signature - I couldn't see an issue for this though it seems like an obvious addition. For example:

"""Test function."""

def test(arg):
    """Lorem Ipsum.

    Args:
        arg: description 1
        arg_2: description 2

    """
    return a

pydocstyle --convention google <doc>.py will pass with no errors, despite arg_2 being present in the docstring. Typically this can arise if there's a refactor and an argument gets dropped. If arg is removed then we do get a D417 (no description for argument).

It seems like here all we'd have to do is check the opposite case, e.g.

D417 is the set difference of arguments defined in the function and the docstring arguments. Note, the docstring calls it a list - this should be changed to be explicit that it's a set IMO.

function_args.difference(docstring_args) -> returns the empty set if all function_args are present in docstring_args

docstring_args.difference(function_args) -> returns the empty set if all docstring_args are present in function_args

i.e. the example above:

`{"arg"}.difference({"arg", "arg_2"}) -> empty
`{"arg, arg_2"}.difference({"arg"}) -> {"arg_2"}

So we'd just add a new violation (e.g. D420):

extra_args = docstring_args - set(function_args)
if extra_args:
    yield violations.D420(
        ", ".join(sorted(extra_args)), definition.name
    )

While I'm thinking about it, it wouldn't be a bad idea to check for duplicate docstring entries either, but this would need to be done prior to forming set(docstring_args). Happy to PR this if this seems reasonable, but I wanted to check where the violation should be raised - e.g. within check_missing or in a new check? Violations are quite tightly scoped, but it would be fairly clean to check these at the same time. I see we have other places where we can raise multiple violations within the same check, so I think it'd be fine to do both here.

@sigmavirus24
Copy link
Member

I'm pretty sure darglint does this already and does it better.

@jveitchmichaelis
Copy link
Author

jveitchmichaelis commented Jan 8, 2023

Thanks! Unfortunately darglint has been marked archived/read-only as of last month (unless someone took over maintenance? It's not clear from the repo)

@Pierre-Sassoulas
Copy link
Member

The doc param extension in pylint could also do what you want:

a.py:3:0: W9016: "arg" missing in parameter type documentation (missing-type-doc)
a.py:3:0: W9017: "arg_2" differing in parameter documentation (differing-param-doc)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants