jsh9 / pydoclint

A Python docstring linter that checks arguments, returns, yields, and raises sections
https://pypi.org/project/pydoclint/
MIT License
149 stars 15 forks source link

feat: introduce DOC503 for checking specific raised exceptions #161

Closed Amar1729 closed 2 months ago

Amar1729 commented 3 months ago

Closes #158

Compare any raise statements in function bodies to docstring Raises sections and ensures they match. Exceptions specified in a docstring are kept as a sorted list, so duplicate lines will throw DOC503 as well.

Had to introduce a new walker, walk.walk_dfs, but I did add tests for it.

This feature does NOT do any resolution of errors, so something like the following:

try:
    1 / 0
except:
    raise

will not identify any specific exceptions thrown. Similarly, except Exception: raise will expect Exception to be included in the docstring. I think both of those cases are already caught as style violations by certain ruff rules (probably under BLE or TRY?), so it might not be much of a problem.


While this checks names of exceptions it doesn't check order(?) like your issue title mentions, but i'm not entirely sure what was meant by that.

Completely open to feedback / changes, let me know if there's anything that could be cleaned up. In particular, there's one bit I don't love: in visitor.py, i had to do this to parse a common convention when using rest-style docstrings:

elif doc.style == 'sphinx' and raises.description:
    # :raises: Exception: -> 'Exception'
    splitDesc = raises.description.split(':')
    if len(splitDesc) > 1 and ' ' not in splitDesc[0].strip():
        exc = splitDesc[0].strip()
        docRaises.append(exc)

This bit parses docstring lines like :raises: Exception:, which seem to be somewhat common in sphinx docstrings but I don't think are technically allowed by the spec[1][2]? docstring_parser doesn't currently parse those words into type_name. Not sure if this bit of code should be moved into docstring_parser_fork, moved into its own function (I wasn't really sure where else to put it), or if you'd prefer to just not support this line format.

Amar1729 commented 3 months ago

*couple leftover comments, fixed + squashed.

Amar1729 commented 3 months ago

Interestingly, if you have tox installed with python3.8-.11 vs python3.12, flake8-misc will give slightly different results complaining about ISC001 in visitor_helper (implicitly-concatenated string on older versions, but not 3.12). fixed (and squashed, again).

*aaaaaaand, cercis complains about the splits. joining them each into one line each.

Amar1729 commented 3 months ago

ah darn, did the visitor walking a bit naively and am not quite getting the right value (OSError) from a testcase like below. will update. fixed, updating main description.

    try:
        pass
    except OSError as e:
        if e.args[0] == 2 and e.filename:
            fp = None
        else:
            raise
jsh9 commented 3 months ago

Thank you for helping implement this feature! I'm taking a look at it, and it should take me a day or two.

jsh9 commented 2 months ago

Hi @Amar1729 , sorry for taking so long to get to this. I've been busy with other things last month. Everything looks good now, so I'll merge and publish.

Amar1729 commented 2 months ago

@jsh9 no worries, I get it. thanks for coming back around to it!