Optionally test for missing docstrings #33
Conversation
| @@ -93,8 +96,8 @@ def run_checks(self, function): | |||
|
|
|||
| """ | |||
| self.function = function | |||
| if function.docstring is not None: | |||
| try: | |||
| try: | |||
There was a problem hiding this comment.
Could you move the try-block back inside of the conditional? The else-branch can't raise a ParserException, so there's no need for it to be included in the try-block.
| @@ -644,3 +647,80 @@ def test_global_noqa_works_for_syntax_errors(self): | |||
| ]) | |||
| for variant in ['# noqa: *', '# noqa: S001', '# noqa']: | |||
| self.has_no_errors(program.format(variant)) | |||
|
|
|||
| self.config = Configuration( | |||
There was a problem hiding this comment.
Remove this, or comment on its purpose. (It's a little confusing here.)
| raise_on_missing_docstrings=False | ||
| ) | ||
|
|
||
| def test_check_for_missing_docstrings(self): |
There was a problem hiding this comment.
This could probably be broken up into multiple unit tests. If it's in one unit tests because of the hassle of performing configuration, etc., then you could put it into its own TestCase and have a setUp method. If it's broken up, it will make failing tests easier to diagnose. If we somehow broke the check for missing docstrings on functions but not methods, for example, then the method's name would immediately let us know what is wrong.
| # If not docstring is given, and raise on missing is true, and its not | ||
| # a private function, add an error. | ||
| elif self.config.raise_on_missing_docstrings and not function.name.startswith("_"): | ||
| cls = (MissingDocstringForPublicMethod |
There was a problem hiding this comment.
This would probably be more understandable if we moved the if-else to the top level, like
if function.is_method:
self.errors.append(
...
)
else:
self.errors.append(
...
)
That will make it more maintainable as well. (Say, if we decide we need to pass more information for functions, or if we change the __init__ signature for these twe errors.)
It is pretty useful to be able to enforce that docstrings exist. This PR adds a new CLI flag
--raise-on-missing-docstringsthat, as the name implies, raises errors if docstrings for public functions or methods do not exist.Support has also been added to the config object and files.
The error codes
I002: Missing docstring for public method.I003: Missing docstring for public function.are inspired by the
pydocstyleerrors: https://github.com/PyCQA/pydocstyle/blob/cc5a96b356e2f10e8c95f1ca719efa3380237671/src/pydocstyle/violations.py#L171The
I1XXgroup has already been taken so I used theI0XXgroup.I'm not sure if you'd like to guard this behind the
--raise-on-missing-docstringsflag as it is currently implemented or just use it by default.