Properly identify undocumented parameters and add new message called missing-any-param-doc #5097
Conversation
This commit fixes the problem where non documented parameters where not being identified properly. Also, it adds a new message called missing-any-param-doc for when a function has no parameter and type doc at all.
This commit adds new test cases for the missing-param-doc and missing-type-doc messages. Also, it contains tests for the new message missing-any-param-doc.
This commit adds the new message where needed in other files instead of triggering both missing-param-doc and missing-type-doc.
Pull Request Test Coverage Report for Build 1325392201
💛 - Coveralls |
| 'Missing any documentation in "%s"', | ||
| "missing-any-param-doc", | ||
| "Please add parameter and/or type documentation.", | ||
| ), |
There was a problem hiding this comment.
We could add an old names here, the ideal would be to not force to disable missing-any-param when the old one was already disabled. But they are not really equivalent so maybe you were right to not add it.
There was a problem hiding this comment.
what does the old names contain exactly?
There was a problem hiding this comment.
You can search for C0111 (the old name for missing-function-docstring) for an example :)
There was a problem hiding this comment.
So you say that instead of adding an old names, we can add a warning in the 2.12 what's new file?
There was a problem hiding this comment.
Yes, something like "A warning might appear if you disabled missing-param-docand missing-type-doc because it became missing-any-param-doc when there isn't any doc."
There was a problem hiding this comment.
I added my changes in the what's new for 2.11 and under 2.11.2 in ChangeLog, should I move them to 2.12 then?
There was a problem hiding this comment.
Yes sorry I did not catch that, this is a "new feature" :)
Pierre-Sassoulas
added
Enhancement ✨
Pierre-Sassoulas
added
Hacktoberfest
and removed
Blocked 🚧
for more information, see https://pre-commit.ci
| \n # description starts on a new line | ||
| \s* (.*) # description | ||
| \s* (.*) # optional description |
There was a problem hiding this comment.
Could you explain why there's a change here, I don't get it ?
There was a problem hiding this comment.
With the '\n', the cases where there was only type documentation could not be identified (in cases with only type doc, there is no new line). This is probably why the message worked only if one param was documented correctly.
There was a problem hiding this comment.
Ho, so this is in fact a very technical regex bug fix ! "Fixed a 10+ year old regex bug by removing 2 characters" should be a badge you can pin on your github profile 😄
There was a problem hiding this comment.
Hahaha that's true I guess
| missing_type_doc = (expected_argument_names - params_with_type) - ( | ||
| not_needed_type_in_docstring | expected_but_ignored_argument_names | ||
| ) | ||
| if ( |
There was a problem hiding this comment.
Thank you @ksaketou very nice change. I have a small question bout a change that I don't understand but this will definitely be in 2.12 :)
for more information, see https://pre-commit.ci
|
Thank you!:) |
doc/whatsnew/<current release.rst>.Type of Changes
Description
This PR fixes the problem where undocumented parameters where not being spotted correctly
by
missing-param-docandmissing-type-doc. Also, a new message has been added calledmissing-any-param-docwhich is triggered when a function has neither parameter nor parametertype documentation.
Related issue
Closes #3799