PyCQA / pydocstyle

docstring style checker
http://pydocstyle.org
MIT License
1.11k stars 188 forks source link

Check for extra/extraneous arguments #622

Open jveitchmichaelis opened 1 year ago

jveitchmichaelis commented 1 year ago

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 commented 1 year ago

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

jveitchmichaelis commented 1 year ago

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 commented 1 year ago

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)