-
Couldn't load subscription status.
- Fork 1.6k
[pydoclint] Add docstring-missing-parameter and docstring-extraneous-parameter (DOC101, DOC102)
#13280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #13280 will not alter performanceComparing Summary
|
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| DOC101 | 12194 | 12194 | 0 | 0 | 0 |
| DOC102 | 112 | 112 | 0 | 0 | 0 |
| D415 | 8 | 4 | 4 | 0 | 0 |
| D202 | 4 | 2 | 2 | 0 | 0 |
| D200 | 4 | 2 | 2 | 0 | 0 |
| D212 | 4 | 2 | 2 | 0 | 0 |
| DOC201 | 4 | 2 | 2 | 0 | 0 |
| D400 | 2 | 1 | 1 | 0 | 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. I haven't gone through the ecossytem results but I think there are a few cases where we don't parse the parameter names correctly.
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
Outdated
Show resolved
Hide resolved
...s/ruff_linter__rules__pydoclint__tests__docstring-extraneous-parameter_DOC102_google.py.snap
Outdated
Show resolved
Hide resolved
...s/ruff_linter__rules__pydoclint__tests__docstring-extraneous-parameter_DOC102_google.py.snap
Outdated
Show resolved
Hide resolved
...s/ruff_linter__rules__pydoclint__tests__docstring-extraneous-parameter_DOC102_google.py.snap
Outdated
Show resolved
Hide resolved
Co-authored-by: Micha Reiser <[email protected]>
7b96439 to
4d47b2d
Compare
|
Hmm, there's an overlap with https://docs.astral.sh/ruff/rules/undocumented-param/. Not quiet sure how to resolve the overlap. |
|
|
That's fair. But the google style one is less strict than the new one. It doesn't require that you document the parameters of all functions. It only requires that the parameters match the function's parameters if there's an Arguments section. |
|
We could add an option to only highlight violations when a section is present. I tried something similar #13302. |
|
This PR has been pending for some time. Just want to gently bump this thread to see if there are any updates or if there's anything blocking its progress. This rule would be very useful, so I'm eager to see it merged. |
...s/ruff_linter__rules__pydoclint__tests__docstring-extraneous-parameter_DOC102_google.py.snap
Outdated
Show resolved
Hide resolved
|
Is there anymore feedback here? |
|
Would love to have these extra rules! |
|
Still in progress? |
|
I am waiting for feedback from the astral team |
@augustelalande meantime, would you mind to fix merge conflicts? ;) |
|
Maybe tag maintainer for review? |
|
Micha will be back next week and can weigh in then, but the conflict between |
|
@ntBre I thought the blocker was resolved by @charliermarsh's comment |
|
I think that's the long-term solution, but it's been almost a year, so we'd need to double check that everyone is still on board with that plan. If we have two heavily overlapping rules, we need to think about a conflict warning, a deprecation plan, etc. That only affects It would also make it easier for reviewers coming back to this (or a potential new reviewer like me) to look at two PRs half this size. We can wait and see what Micha thinks, though. He might have a better idea :) |
…20376) ## Summary Implement `docstring-extraneous-parameter` (`DOC102`). This rule checks that all parameters present in a functions docstring are also present in its signature. Split from #13280, per this [comment](#13280 (comment)). Part of #12434. ## Test Plan Test cases added. --------- Co-authored-by: Brent Westbrook <[email protected]>
Summary
Add
docstring-missing-parameteranddocstring-extraneous-parameter(DOC101,DOC102). These rules check that the parameters defined in a functions signature match thos defined in the docstring.Part of #12434.
Test Plan
Test cases added.