From 78826c91b8fd0bada15c58ab5f8d313dfcf18aaa Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sat, 5 Sep 2020 06:00:49 +0300 Subject: [PATCH 01/15] Preventing the D103 error when the function is decorated with @overload. Added an is_overload method in the function class(parser.py). Added an if statement so that the D103 error will not trigger when decorated with @overload(checker.py) Added some tests to see that it's working correctly. --- src/pydocstyle/checker.py | 7 ++++--- src/pydocstyle/parser.py | 8 ++++++++ src/tests/test_cases/test.py | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index cd2846e5..dd331586 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -3,7 +3,6 @@ import ast import string import sys -import textwrap import tokenize as tk from itertools import takewhile, chain from re import compile as re @@ -94,7 +93,7 @@ class ConventionChecker: # Matches anything that fulfills all the following conditions: r"^\s*" # Begins with 0 or more whitespace characters r"(\w+)" # Followed by 1 or more unicode chars, numbers or underscores - # The above is captured as the first group as this is the paramater name. + # The above is captured as the first group as this is the parameter name. r"\s*" # Followed by 0 or more whitespace characters r"(\(.*?\))?" # Matches patterns contained within round brackets. # The `.*?`matches any sequence of characters in a non-greedy @@ -162,8 +161,10 @@ def check_docstring_missing(self, definition, docstring): Method: (lambda: violations.D105() if definition.is_magic else (violations.D107() if definition.is_init else violations.D102())), - Function: violations.D103, NestedFunction: violations.D103, + Function: (lambda: violations.D103() + if not definition.is_overload + else None), Package: violations.D104} return codes[type(definition)]() diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 7ffef855..f0e17b9f 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -141,6 +141,14 @@ def is_public(self): else: return not self.name.startswith('_') + @property + def is_overload(self): + """Return True iff the method decorated with overload.""" + for decorator in self.decorators: + if decorator.name == "overload": + return True + return False + @property def is_test(self): """Return True if this function is a test function/method. diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 20b81fce..1487fc04 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -4,6 +4,7 @@ import os import sys from .expected import Expectation +from typing import overload expectation = Expectation() @@ -54,6 +55,21 @@ def nested(): '' +@overload +def overloaded_func(a: str) -> str: + return 2 + + +@expect('D103: Missing docstring in public function', arg_count=1) +def overloaded_func(a): + return str(a) + + +def overloaded_func(a): + """Foo bar documentation.""" + return str(a) + + @expect('D200: One-line docstring should fit on one line with quotes ' '(found 3)') @expect('D212: Multi-line docstring summary should start at the first line') From fe9741e40622a4969af9993f51d85e2abadfecbd Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sat, 5 Sep 2020 06:23:12 +0300 Subject: [PATCH 02/15] Preventing the D103 error when the function is decorated with @overload. Added an is_overload method in the function class(parser.py). Added an if statement so that the D103 error will not trigger when decorated with @overload(checker.py) Added some tests to see that it's working correctly. --- src/pydocstyle/checker.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index dd331586..75f9f6f8 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -3,6 +3,7 @@ import ast import string import sys +import textwrap import tokenize as tk from itertools import takewhile, chain from re import compile as re From d033b85a51bf86f44090af4618b4c1a0b7c91af9 Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sat, 5 Sep 2020 07:16:44 +0300 Subject: [PATCH 03/15] Added an is_overload method in the function class(parser.py). Added an if statement so that the D103 error will not trigger when decorated with @overload(checker.py) Added some tests to see that it's working correctly. --- src/pydocstyle/checker.py | 1 - src/tests/test_cases/test.py | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index 75f9f6f8..dd331586 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -3,7 +3,6 @@ import ast import string import sys -import textwrap import tokenize as tk from itertools import takewhile, chain from re import compile as re diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 1487fc04..93fefe1c 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -56,13 +56,13 @@ def nested(): @overload -def overloaded_func(a: str) -> str: +def overloaded_func(a: int) -> str: return 2 -@expect('D103: Missing docstring in public function', arg_count=1) -def overloaded_func(a): - return str(a) +@overload +def overloaded_func(a: str) -> str: + return '1' def overloaded_func(a): From f3049df353f338ba3f7d169c2408378a35b5f179 Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 6 Sep 2020 12:25:44 +0300 Subject: [PATCH 04/15] Fixing overload test. --- src/tests/test_cases/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 669fc2cb..f868e74e 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -56,7 +56,7 @@ def nested(): @overload def overloaded_func(a: int) -> str: - return 2 + return '2' @overload From 3389a3df63a5258812aba37de866b8fd719acf4e Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 6 Sep 2020 12:35:32 +0300 Subject: [PATCH 05/15] Fixing overload test. Running isort src/pydocstyle --- src/pydocstyle/checker.py | 394 ++++++++++++++++++++++++-------------- 1 file changed, 255 insertions(+), 139 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index bef218e9..537f1849 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -5,20 +5,36 @@ import sys import textwrap import tokenize as tk -from itertools import takewhile, chain -from re import compile as re from collections import namedtuple +from itertools import chain, takewhile +from re import compile as re from . import violations from .config import IllegalConfiguration -from .parser import (Package, Module, Class, NestedClass, Definition, AllError, - Method, Function, NestedFunction, Parser, StringIO, - ParseError) -from .utils import log, is_blank, pairwise, common_prefix_length, strip_non_alphanumeric -from .wordlists import IMPERATIVE_VERBS, IMPERATIVE_BLACKLIST, stem - - -__all__ = ('check', ) +from .parser import ( + AllError, + Class, + Definition, + Function, + Method, + Module, + NestedClass, + NestedFunction, + Package, + ParseError, + Parser, + StringIO, +) +from .utils import ( + common_prefix_length, + is_blank, + log, + pairwise, + strip_non_alphanumeric, +) +from .wordlists import IMPERATIVE_BLACKLIST, IMPERATIVE_VERBS, stem + +__all__ = ('check',) def check_for(kind, terminal=False): @@ -26,6 +42,7 @@ def decorator(f): f._check_for = kind f._terminal = terminal return f + return decorator @@ -52,7 +69,7 @@ class ConventionChecker: 'References', 'Examples', 'Attributes', - 'Methods' + 'Methods', ) GOOGLE_SECTION_NAMES = ( @@ -111,29 +128,42 @@ class ConventionChecker: ".+" ) - def check_source(self, source, filename, ignore_decorators=None, ignore_inline_noqa=False): + def check_source( + self, + source, + filename, + ignore_decorators=None, + ignore_inline_noqa=False, + ): module = parse(StringIO(source), filename) for definition in module: for this_check in self.checks: terminate = False if isinstance(definition, this_check._check_for): - skipping_all = (definition.skipped_error_codes == 'all') + skipping_all = definition.skipped_error_codes == 'all' decorator_skip = ignore_decorators is not None and any( len(ignore_decorators.findall(dec.name)) > 0 - for dec in definition.decorators) - if (ignore_inline_noqa or not skipping_all) and not decorator_skip: - error = this_check(self, definition, - definition.docstring) + for dec in definition.decorators + ) + if ( + ignore_inline_noqa or not skipping_all + ) and not decorator_skip: + error = this_check( + self, definition, definition.docstring + ) else: error = None errors = error if hasattr(error, '__iter__') else [error] for error in errors: - if error is not None and (ignore_inline_noqa or error.code not in \ - definition.skipped_error_codes): + if error is not None and ( + ignore_inline_noqa + or error.code not in definition.skipped_error_codes + ): partition = this_check.__doc__.partition('.\n') message, _, explanation = partition - error.set_context(explanation=explanation, - definition=definition) + error.set_context( + explanation=explanation, definition=definition + ) yield error if this_check._terminal: terminate = True @@ -143,8 +173,11 @@ def check_source(self, source, filename, ignore_decorators=None, ignore_inline_n @property def checks(self): - all = [this_check for this_check in vars(type(self)).values() - if hasattr(this_check, '_check_for')] + all = [ + this_check + for this_check in vars(type(self)).values() + if hasattr(this_check, '_check_for') + ] return sorted(all, key=lambda this_check: not this_check._terminal) @check_for(Definition, terminal=True) @@ -184,19 +217,33 @@ def check_docstring_missing(self, definition, docstring): NestedFunction: violations.D103, Package: violations.D104, } - if (not docstring and definition.is_public or - docstring and is_blank(ast.literal_eval(docstring))): - codes = {Module: violations.D100, - Class: violations.D101, - NestedClass: violations.D106, - Method: (lambda: violations.D105() if definition.is_magic - else (violations.D107() if definition.is_init - else violations.D102())), - NestedFunction: violations.D103, - Function: (lambda: violations.D103() - if not definition.is_overload - else None), - Package: violations.D104} + if ( + not docstring + and definition.is_public + or docstring + and is_blank(ast.literal_eval(docstring)) + ): + codes = { + Module: violations.D100, + Class: violations.D101, + NestedClass: violations.D106, + Method: ( + lambda: violations.D105() + if definition.is_magic + else ( + violations.D107() + if definition.is_init + else violations.D102() + ) + ), + NestedFunction: violations.D103, + Function: ( + lambda: violations.D103() + if not definition.is_overload + else None + ), + Package: violations.D104, + } return codes[type(definition)]() @check_for(Definition) @@ -234,8 +281,8 @@ def check_no_blank_before(self, function, docstring): # def # and the blank line is not itself followed by an inner function or # class. if not ( - blanks_after_count == 1 and - re(r"\s+(?:(?:class|def|async def)\s|@)").match(after) + blanks_after_count == 1 + and re(r"\s+(?:(?:class|def|async def)\s|@)").match(after) ): yield violations.D202(blanks_after_count) @@ -311,8 +358,11 @@ def check_indent(self, definition, docstring): lines = docstring.split('\n') if len(lines) > 1: # First line and line continuations need no indent. - lines = [line for i, line in enumerate(lines) - if i and not lines[i-1].endswith('\\')] + lines = [ + line + for i, line in enumerate(lines) + if i and not lines[i - 1].endswith('\\') + ] indents = [leading_space(l) for l in lines if not is_blank(l)] if set(' \t') == set(''.join(indents) + indent): yield violations.D206() @@ -332,8 +382,11 @@ def check_newline_after_last_paragraph(self, definition, docstring): """ if docstring: - lines = [l for l in ast.literal_eval(docstring).split('\n') - if not is_blank(l)] + lines = [ + l + for l in ast.literal_eval(docstring).split('\n') + if not is_blank(l) + ] if len(lines) > 1: if docstring.split("\n")[-1].strip() not in ['"""', "'''"]: return violations.D209() @@ -343,8 +396,11 @@ def check_surrounding_whitespaces(self, definition, docstring): """D210: No whitespaces allowed surrounding docstring text.""" if docstring: lines = ast.literal_eval(docstring).split('\n') - if lines[0].startswith(' ') or \ - len(lines) == 1 and lines[0].endswith(' '): + if ( + lines[0].startswith(' ') + or len(lines) == 1 + and lines[0].endswith(' ') + ): return violations.D210() @check_for(Definition) @@ -357,10 +413,14 @@ def check_multi_line_summary_start(self, definition, docstring): """ if docstring: start_triple = [ - '"""', "'''", - 'u"""', "u'''", - 'r"""', "r'''", - 'ur"""', "ur'''" + '"""', + "'''", + 'u"""', + "u'''", + 'r"""', + "r'''", + 'ur"""', + "ur'''", ] lines = ast.literal_eval(docstring).split('\n') @@ -412,9 +472,11 @@ def check_backslashes(self, definition, docstring): # Just check that docstring is raw, check_triple_double_quotes # ensures the correct quotes. - if (docstring - and re(r'\\[^\nuN]').search(docstring) - and not docstring.startswith(('r', 'ur'))): + if ( + docstring + and re(r'\\[^\nuN]').search(docstring) + and not docstring.startswith(('r', 'ur')) + ): return violations.D301() @staticmethod @@ -448,7 +510,9 @@ def check_ends_with_punctuation(self, definition, docstring): question mark, or exclamation point """ - return self._check_ends_with(docstring, ('.', '!', '?'), violations.D415) + return self._check_ends_with( + docstring, ('.', '!', '?'), violations.D415 + ) @check_for(Function) def check_imperative_mood(self, function, docstring): # def context @@ -473,12 +537,9 @@ def check_imperative_mood(self, function, docstring): # def context if correct_forms and check_word not in correct_forms: best = max( correct_forms, - key=lambda f: common_prefix_length(check_word, f) - ) - return violations.D401( - best.capitalize(), - first_word + key=lambda f: common_prefix_length(check_word, f), ) + return violations.D401(best.capitalize(), first_word) @check_for(Function) def check_no_signature(self, function, docstring): # def context @@ -565,26 +626,34 @@ def _is_docstring_section(context): If one of the conditions is true, we will consider the line as a section name. """ - section_name_suffix = \ + section_name_suffix = ( context.line.strip().lstrip(context.section_name.strip()).strip() + ) section_suffix_is_only_colon = section_name_suffix == ':' punctuation = [',', ';', '.', '-', '\\', '/', ']', '}', ')'] - prev_line_ends_with_punctuation = \ - any(context.previous_line.strip().endswith(x) for x in punctuation) + prev_line_ends_with_punctuation = any( + context.previous_line.strip().endswith(x) for x in punctuation + ) - this_line_looks_like_a_section_name = \ + this_line_looks_like_a_section_name = ( is_blank(section_name_suffix) or section_suffix_is_only_colon + ) - prev_line_looks_like_end_of_paragraph = \ + prev_line_looks_like_end_of_paragraph = ( prev_line_ends_with_punctuation or is_blank(context.previous_line) + ) - return (this_line_looks_like_a_section_name and - prev_line_looks_like_end_of_paragraph) + return ( + this_line_looks_like_a_section_name + and prev_line_looks_like_end_of_paragraph + ) @classmethod - def _check_blanks_and_section_underline(cls, section_name, context, indentation): + def _check_blanks_and_section_underline( + cls, section_name, context, indentation + ): """D4{07,08,09,12,14}, D215: Section underline checks. Check for correct formatting for docstring sections. Checks that: @@ -622,9 +691,11 @@ def _check_blanks_and_section_underline(cls, section_name, context, indentation) yield violations.D408(section_name) if non_empty_line.strip() != "-" * len(section_name): - yield violations.D409(len(section_name), - section_name, - len(non_empty_line.strip())) + yield violations.D409( + len(section_name), + section_name, + len(non_empty_line.strip()), + ) if leading_space(non_empty_line) > indentation: yield violations.D215(section_name) @@ -633,11 +704,13 @@ def _check_blanks_and_section_underline(cls, section_name, context, indentation) # If the line index after the dashes is in range (perhaps we have # a header + underline followed by another section header). if line_after_dashes_index < len(context.following_lines): - line_after_dashes = \ - context.following_lines[line_after_dashes_index] + line_after_dashes = context.following_lines[ + line_after_dashes_index + ] if is_blank(line_after_dashes): - rest_of_lines = \ - context.following_lines[line_after_dashes_index:] + rest_of_lines = context.following_lines[ + line_after_dashes_index: + ] if not is_blank(''.join(rest_of_lines)): yield violations.D412(section_name) else: @@ -646,7 +719,9 @@ def _check_blanks_and_section_underline(cls, section_name, context, indentation) yield violations.D414(section_name) @classmethod - def _check_common_section(cls, docstring, definition, context, valid_section_names): + def _check_common_section( + cls, docstring, definition, context, valid_section_names + ): """D4{05,10,11,13}, D214: Section name checks. Check for valid section names. Checks that: @@ -660,15 +735,18 @@ def _check_common_section(cls, docstring, definition, context, valid_section_nam indentation = cls._get_docstring_indent(definition, docstring) capitalized_section = context.section_name.title() - if (context.section_name not in valid_section_names and - capitalized_section in valid_section_names): + if ( + context.section_name not in valid_section_names + and capitalized_section in valid_section_names + ): yield violations.D405(capitalized_section, context.section_name) if leading_space(context.line) > indentation: yield violations.D214(capitalized_section) - if (not context.following_lines or - not is_blank(context.following_lines[-1])): + if not context.following_lines or not is_blank( + context.following_lines[-1] + ): if context.is_last_section: yield violations.D413(capitalized_section) else: @@ -677,9 +755,9 @@ def _check_common_section(cls, docstring, definition, context, valid_section_nam if not is_blank(context.previous_line): yield violations.D411(capitalized_section) - yield from cls._check_blanks_and_section_underline(capitalized_section, - context, - indentation) + yield from cls._check_blanks_and_section_underline( + capitalized_section, context, indentation + ) @classmethod def _check_numpy_section(cls, docstring, definition, context): @@ -693,16 +771,17 @@ def _check_numpy_section(cls, docstring, definition, context): """ indentation = cls._get_docstring_indent(definition, docstring) capitalized_section = context.section_name.title() - yield from cls._check_common_section(docstring, - definition, - context, - cls.NUMPY_SECTION_NAMES) + yield from cls._check_common_section( + docstring, definition, context, cls.NUMPY_SECTION_NAMES + ) suffix = context.line.strip().lstrip(context.section_name) if suffix: yield violations.D406(capitalized_section, context.line.strip()) if capitalized_section == "Parameters": - yield from cls._check_parameters_section(docstring, definition, context) + yield from cls._check_parameters_section( + docstring, definition, context + ) @staticmethod def _check_parameters_section(docstring, definition, context): @@ -717,7 +796,8 @@ def _check_parameters_section(docstring, definition, context): section_level_indent = leading_space(context.line) # Join line continuations, then resplit by line. content = ( - '\n'.join(context.following_lines).replace('\\\n', '').split('\n')) + '\n'.join(context.following_lines).replace('\\\n', '').split('\n') + ) for current_line, next_line in zip(content, content[1:]): # All parameter definitions in the Numpy parameters # section must be at the same indent level as the section @@ -726,9 +806,14 @@ def _check_parameters_section(docstring, definition, context): # and has some string, to ensure that the parameter actually # has a description. # This means, this is a parameter doc with some description - if ((leading_space(current_line) == section_level_indent) - and (len(leading_space(next_line)) > len(leading_space(current_line))) - and next_line.strip()): + if ( + (leading_space(current_line) == section_level_indent) + and ( + len(leading_space(next_line)) + > len(leading_space(current_line)) + ) + and next_line.strip() + ): # In case the parameter has type definitions, it # will have a colon if ":" in current_line: @@ -742,8 +827,9 @@ def _check_parameters_section(docstring, definition, context): parameter_list = parameters.split(",") for parameter in parameter_list: docstring_args.add(parameter.strip()) - yield from ConventionChecker._check_missing_args(docstring_args, definition) - + yield from ConventionChecker._check_missing_args( + docstring_args, definition + ) @staticmethod def _check_args_section(docstring, definition, context): @@ -759,8 +845,9 @@ def _check_args_section(docstring, definition, context): match = ConventionChecker.GOOGLE_ARGS_REGEX.match(line) if match: docstring_args.add(match.group(1)) - yield from ConventionChecker._check_missing_args(docstring_args, definition) - + yield from ConventionChecker._check_missing_args( + docstring_args, definition + ) @staticmethod def _check_missing_args(docstring_args, definition): @@ -787,9 +874,9 @@ def _check_missing_args(docstring_args, definition): ] missing_args = set(function_args) - docstring_args if missing_args: - yield violations.D417(", ".join(sorted(missing_args)), - definition.name) - + yield violations.D417( + ", ".join(sorted(missing_args)), definition.name + ) @classmethod def _check_google_section(cls, docstring, definition, context): @@ -805,18 +892,18 @@ def _check_google_section(cls, docstring, definition, context): which are style-agnostic section checks. """ capitalized_section = context.section_name.title() - yield from cls._check_common_section(docstring, - definition, - context, - cls.GOOGLE_SECTION_NAMES) + yield from cls._check_common_section( + docstring, definition, context, cls.GOOGLE_SECTION_NAMES + ) suffix = context.line.strip().lstrip(context.section_name) if suffix != ":": - yield violations.D416(capitalized_section + ":", context.line.strip()) + yield violations.D416( + capitalized_section + ":", context.line.strip() + ) if capitalized_section in ("Args", "Arguments"): yield from cls._check_args_section(docstring, definition, context) - @staticmethod def _get_section_contexts(lines, valid_section_names): """Generate `SectionContext` objects for valid sections. @@ -839,39 +926,53 @@ def _suspected_as_section(_line): return result in lower_section_names # Finding our suspects. - suspected_section_indices = [i for i, line in enumerate(lines) if - _suspected_as_section(line)] - - SectionContext = namedtuple('SectionContext', ('section_name', - 'previous_line', - 'line', - 'following_lines', - 'original_index', - 'is_last_section')) + suspected_section_indices = [ + i for i, line in enumerate(lines) if _suspected_as_section(line) + ] + + SectionContext = namedtuple( + 'SectionContext', + ( + 'section_name', + 'previous_line', + 'line', + 'following_lines', + 'original_index', + 'is_last_section', + ), + ) # First - create a list of possible contexts. Note that the # `following_lines` member is until the end of the docstring. - contexts = (SectionContext(get_leading_words(lines[i].strip()), - lines[i - 1], - lines[i], - lines[i + 1:], - i, - False) - for i in suspected_section_indices) + contexts = ( + SectionContext( + get_leading_words(lines[i].strip()), + lines[i - 1], + lines[i], + lines[i + 1 :], + i, + False, + ) + for i in suspected_section_indices + ) # Now that we have manageable objects - rule out false positives. - contexts = (c for c in contexts if ConventionChecker._is_docstring_section(c)) + contexts = ( + c for c in contexts if ConventionChecker._is_docstring_section(c) + ) # Now we shall trim the `following lines` field to only reach the # next section name. for a, b in pairwise(contexts, None): end = -1 if b is None else b.original_index - yield SectionContext(a.section_name, - a.previous_line, - a.line, - lines[a.original_index + 1:end], - a.original_index, - b is None) + yield SectionContext( + a.section_name, + a.previous_line, + a.line, + lines[a.original_index + 1 : end], + a.original_index, + b is None, + ) def _check_numpy_sections(self, lines, definition, docstring): """NumPy-style docstring sections checks. @@ -894,8 +995,7 @@ def _check_numpy_sections(self, lines, definition, docstring): Numpy-style section. """ found_any_numpy_section = False - for ctx in self._get_section_contexts(lines, - self.NUMPY_SECTION_NAMES): + for ctx in self._get_section_contexts(lines, self.NUMPY_SECTION_NAMES): found_any_numpy_section = True yield from self._check_numpy_section(docstring, definition, ctx) @@ -919,8 +1019,9 @@ def _check_google_sections(self, lines, definition, docstring): Yields all violation from `_check_google_section` for each valid Google-style section. """ - for ctx in self._get_section_contexts(lines, - self.GOOGLE_SECTION_NAMES): + for ctx in self._get_section_contexts( + lines, self.GOOGLE_SECTION_NAMES + ): yield from self._check_google_section(docstring, definition, ctx) @check_for(Definition) @@ -933,15 +1034,25 @@ def check_docstring_sections(self, definition, docstring): if len(lines) < 2: return - found_numpy = yield from self._check_numpy_sections(lines, definition, docstring) + found_numpy = yield from self._check_numpy_sections( + lines, definition, docstring + ) if not found_numpy: - yield from self._check_google_sections(lines, definition, docstring) + yield from self._check_google_sections( + lines, definition, docstring + ) parse = Parser() -def check(filenames, select=None, ignore=None, ignore_decorators=None, ignore_inline_noqa=False): +def check( + filenames, + select=None, + ignore=None, + ignore_decorators=None, + ignore_inline_noqa=False, +): """Generate docstring errors that exist in `filenames` iterable. By default, the PEP-257 convention is checked. To specifically define the @@ -973,13 +1084,16 @@ def check(filenames, select=None, ignore=None, ignore_decorators=None, ignore_in """ if select is not None and ignore is not None: - raise IllegalConfiguration('Cannot pass both select and ignore. ' - 'They are mutually exclusive.') + raise IllegalConfiguration( + 'Cannot pass both select and ignore. ' + 'They are mutually exclusive.' + ) elif select is not None: checked_codes = select elif ignore is not None: - checked_codes = list(set(violations.ErrorRegistry.get_error_codes()) - - set(ignore)) + checked_codes = list( + set(violations.ErrorRegistry.get_error_codes()) - set(ignore) + ) else: checked_codes = violations.conventions.pep257 @@ -988,9 +1102,9 @@ def check(filenames, select=None, ignore=None, ignore_decorators=None, ignore_in try: with tk.open(filename) as file: source = file.read() - for error in ConventionChecker().check_source(source, filename, - ignore_decorators, - ignore_inline_noqa): + for error in ConventionChecker().check_source( + source, filename, ignore_decorators, ignore_inline_noqa + ): code = getattr(error, 'code', None) if code in checked_codes: yield error @@ -1020,10 +1134,12 @@ def get_leading_words(line): if result is not None: return result.group() + def is_def_arg_private(arg_name): """Return a boolean indicating if the argument name is private.""" return arg_name.startswith("_") + def get_function_args(function_source): """Return the function arguments given the source-code string.""" # We are stripping the whitespace from the left of the From d9bdd5480061fc2825ff47a5dae7fb40a142b768 Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 6 Sep 2020 18:15:08 +0300 Subject: [PATCH 06/15] Added D418 Error: Function decorated with @overload shouldn\'t contain a docstring. --- src/pydocstyle/checker.py | 35 ++++++++++++----------------------- src/pydocstyle/violations.py | 6 ++++++ src/tests/test_cases/test.py | 2 ++ 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index 537f1849..ac1cc2bf 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -194,29 +194,6 @@ def check_docstring_missing(self, definition, docstring): with a single underscore. """ - if ( - not docstring - and definition.is_public - or docstring - and is_blank(ast.literal_eval(docstring)) - ): - codes = { - Module: violations.D100, - Class: violations.D101, - NestedClass: violations.D106, - Method: ( - lambda: violations.D105() - if definition.is_magic - else ( - violations.D107() - if definition.is_init - else violations.D102() - ) - ), - Function: violations.D103, - NestedFunction: violations.D103, - Package: violations.D104, - } if ( not docstring and definition.is_public @@ -571,6 +548,18 @@ def check_capitalized(self, function, docstring): if first_word != first_word.capitalize(): return violations.D403(first_word.capitalize(), first_word) + @check_for(Function) + def check_if_needed(self, function, docstring): + """D418: Function decorated with @overload shouldn't contain a docstring. + + Functions that are decorated with @overload are definitions, + and are for the benefit of the type checker only, + since they will be overwritten by the non-@overload-decorated definition. + + """ + if docstring and function.is_overload: + return violations.D418() + @check_for(Definition) def check_starts_with_this(self, function, docstring): """D404: First word of the docstring should not be `This`. diff --git a/src/pydocstyle/violations.py b/src/pydocstyle/violations.py index 6bf3963d..82eb2081 100644 --- a/src/pydocstyle/violations.py +++ b/src/pydocstyle/violations.py @@ -411,6 +411,11 @@ def to_rst(cls) -> str: 'argument(s) {0} are missing descriptions in {1!r} docstring', ) +D418 = D4xx.create_error( + 'D418', + 'Function decorated with @overload shouldn\'t contain a docstring', +) + class AttrDict(dict): def __getattr__(self, item: str) -> Any: @@ -441,6 +446,7 @@ def __getattr__(self, item: str) -> Any: 'D415', 'D416', 'D417', + 'D418', }, 'numpy': all_errors - { diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index f868e74e..9dd1fe9a 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -61,6 +61,8 @@ def overloaded_func(a: int) -> str: @overload def overloaded_func(a: str) -> str: + # TODO(Find a way to test D418 + # as @overload can't be passed to the @expect function) return '1' From 13ae281cce99a5cafdbe3aae375560f32b4db966 Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 6 Sep 2020 18:58:22 +0300 Subject: [PATCH 07/15] Overloaded functions shouldn't have a definition. --- src/tests/test_cases/test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 9dd1fe9a..3603846b 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -56,14 +56,14 @@ def nested(): @overload def overloaded_func(a: int) -> str: - return '2' + ... @overload def overloaded_func(a: str) -> str: # TODO(Find a way to test D418 # as @overload can't be passed to the @expect function) - return '1' + ... def overloaded_func(a): From 845e4c9ef23882b2e47c20a80d184c2cfeaa3ad8 Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 6 Sep 2020 21:02:18 +0300 Subject: [PATCH 08/15] Tests for D418 error: Functions decorated with @overload --- src/tests/test_cases/test.py | 2 -- src/tests/test_integration.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 3603846b..4a3b0c1a 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -61,8 +61,6 @@ def overloaded_func(a: int) -> str: @overload def overloaded_func(a: str) -> str: - # TODO(Find a way to test D418 - # as @overload can't be passed to the @expect function) ... diff --git a/src/tests/test_integration.py b/src/tests/test_integration.py index 7de3b755..1a549500 100644 --- a/src/tests/test_integration.py +++ b/src/tests/test_integration.py @@ -502,6 +502,35 @@ def foo(): in err) +def test_overload_function(env): + """Functions decorated with @overload trigger D418 error.""" + with env.open('example.py', 'wt') as example: + example.write(textwrap.dedent('''\ + from typing import overload + + + @overload + def overloaded_func(a: int) -> str: + ... + + + @overload + def overloaded_func(a: str) -> str: + """Foo bar documentation.""" + ... + + + def overloaded_func(a): + """Foo bar documentation.""" + return str(a) + + ''')) + env.write_config(ignore="D100") + out, err, code = env.invoke() + assert code == 1 + assert 'D418' in out + + def test_conflicting_select_ignore_config(env): """Test that select and ignore are mutually exclusive.""" env.write_config(select="D100", ignore="D101") From 23bcabd8374509e4bef23463f3d8628465059314 Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 6 Sep 2020 21:40:33 +0300 Subject: [PATCH 09/15] Tests for D418 error: Functions decorated with @overload --- src/tests/test_cases/test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 4a3b0c1a..2476751f 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -61,9 +61,14 @@ def overloaded_func(a: int) -> str: @overload def overloaded_func(a: str) -> str: + """Foo bar documentation.""" ... +expect('overloaded_func', + "D418: Function decorated with @overload shouldn't contain a docstring") + + def overloaded_func(a): """Foo bar documentation.""" return str(a) From 79f9054a9d482be0ba1ac6d00989b777d1ad2b16 Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 6 Sep 2020 21:44:58 +0300 Subject: [PATCH 10/15] Tests for D418 error: Functions decorated with @overload --- src/tests/test_cases/test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 2476751f..b20bc686 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -65,15 +65,15 @@ def overloaded_func(a: str) -> str: ... -expect('overloaded_func', - "D418: Function decorated with @overload shouldn't contain a docstring") - - def overloaded_func(a): """Foo bar documentation.""" return str(a) +expect('overloaded_func', + "D418: Function decorated with @overload shouldn't contain a docstring") + + @expect('D200: One-line docstring should fit on one line with quotes ' '(found 3)') @expect('D212: Multi-line docstring summary should start at the first line') From 33dd57b4ff8f614eea7ebde2b24214c387eeb01b Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sat, 12 Sep 2020 06:10:43 +0300 Subject: [PATCH 11/15] Added Tests for nested_functions/methods that are decorated with @overload checker is also preventing the 102 error in methods that are decorated with @overload. (checker.py) Any suggestions on how to write those if statements more elegantly? I really don't like the nested if statement. --- src/pydocstyle/checker.py | 16 ++++++------- src/pydocstyle/violations.py | 2 +- src/tests/test_cases/test.py | 42 +++++++++++++++++++++++++++++++++-- src/tests/test_integration.py | 32 +++++++++++++++++++++++++- 4 files changed, 80 insertions(+), 12 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index ac1cc2bf..41e3f35f 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -2,8 +2,6 @@ import ast import string -import sys -import textwrap import tokenize as tk from collections import namedtuple from itertools import chain, takewhile @@ -204,13 +202,15 @@ def check_docstring_missing(self, definition, docstring): Module: violations.D100, Class: violations.D101, NestedClass: violations.D106, - Method: ( - lambda: violations.D105() - if definition.is_magic + Method: lambda: violations.D105() + if definition.is_magic + else ( + violations.D107() + if definition.is_init else ( - violations.D107() - if definition.is_init - else violations.D102() + violations.D102() + if not definition.is_overload + else None ) ), NestedFunction: violations.D103, diff --git a/src/pydocstyle/violations.py b/src/pydocstyle/violations.py index 82eb2081..eb2b6d4c 100644 --- a/src/pydocstyle/violations.py +++ b/src/pydocstyle/violations.py @@ -413,7 +413,7 @@ def to_rst(cls) -> str: D418 = D4xx.create_error( 'D418', - 'Function decorated with @overload shouldn\'t contain a docstring', + 'Function/ Method decorated with @overload shouldn\'t contain a docstring', ) diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index b20bc686..49fd471a 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -1,7 +1,6 @@ # No docstring, so we can test D100 from functools import wraps import os -import sys from .expected import Expectation from typing import overload @@ -26,6 +25,23 @@ def method(self=None): def _ok_since_private(self=None): pass + @overload + def overloaded_method(self, a: int) -> str: + ... + + @overload + def overloaded_method(self, a: str) -> str: + """Foo bar documentation.""" + ... + + def overloaded_method(a): + """Foo bar documentation.""" + return str(a) + + expect('overloaded_method', + "D418: Function/ Method decorated with @overload" + " shouldn't contain a docstring") + @expect('D102: Missing docstring in public method') def __new__(self=None): pass @@ -54,6 +70,27 @@ def nested(): '' +def function_with_nesting(): + """Foo bar documentation.""" + @overload + def nested_overloaded_func(a: int) -> str: + ... + + @overload + def nested_overloaded_func(a: str) -> str: + """Foo bar documentation.""" + ... + + def nested_overloaded_func(a): + """Foo bar documentation.""" + return str(a) + + +expect('nested_overloaded_func', + "D418: Function/ Method decorated with @overload" + " shouldn't contain a docstring") + + @overload def overloaded_func(a: int) -> str: ... @@ -71,7 +108,8 @@ def overloaded_func(a): expect('overloaded_func', - "D418: Function decorated with @overload shouldn't contain a docstring") + "D418: Function/ Method decorated with @overload" + " shouldn't contain a docstring") @expect('D200: One-line docstring should fit on one line with quotes ' diff --git a/src/tests/test_integration.py b/src/tests/test_integration.py index 1a549500..a5a5588e 100644 --- a/src/tests/test_integration.py +++ b/src/tests/test_integration.py @@ -3,7 +3,6 @@ from collections import namedtuple import os -import sys import shlex import shutil import pytest @@ -529,6 +528,37 @@ def overloaded_func(a): out, err, code = env.invoke() assert code == 1 assert 'D418' in out + assert 'D103' not in out + + +def test_overload_nested_function(env): + """Nested functions decorated with @overload trigger D418 error.""" + with env.open('example.py', 'wt') as example: + example.write(textwrap.dedent('''\ + from typing import overload + + def function_with_nesting(): + """Valid docstring in public function.""" + @overload + def overloaded_func(a: int) -> str: + ... + + + @overload + def overloaded_func(a: str) -> str: + """Foo bar documentation.""" + ... + + + def overloaded_func(a): + """Foo bar documentation.""" + return str(a) + ''')) + env.write_config(ignore="D100") + out, err, code = env.invoke() + assert code == 1 + assert 'D418' in out + assert 'D103' not in out def test_conflicting_select_ignore_config(env): From 805ee78c94860aaeb8250046751a2597264cc37e Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 13 Sep 2020 21:58:52 +0300 Subject: [PATCH 12/15] Added Tests for valid overloaded functions, valid overloaded Method and overloaded Methods with D418 Error. --- src/tests/test_integration.py | 93 +++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/src/tests/test_integration.py b/src/tests/test_integration.py index a5a5588e..e6871e2d 100644 --- a/src/tests/test_integration.py +++ b/src/tests/test_integration.py @@ -531,6 +531,99 @@ def overloaded_func(a): assert 'D103' not in out +def test_overload_method(env): + """Methods decorated with @overload trigger D418 error.""" + with env.open('example.py', 'wt') as example: + example.write(textwrap.dedent('''\ + from typing import overload + + class ClassWithMethods: + @overload + def overloaded_method(a: int) -> str: + ... + + + @overload + def overloaded_method(a: str) -> str: + """Foo bar documentation.""" + ... + + + def overloaded_method(a): + """Foo bar documentation.""" + return str(a) + + ''')) + env.write_config(ignore="D100") + out, err, code = env.invoke() + assert code == 1 + assert 'D418' in out + assert 'D102' not in out + assert 'D103' not in out + + +def test_overload_method_valid(env): + """Valid case for overload decorated Methods. + + This shouldn't throw any errors. + """ + with env.open('example.py', 'wt') as example: + example.write(textwrap.dedent('''\ + from typing import overload + + class ClassWithMethods: + """Valid docstring in public Class.""" + + @overload + def overloaded_method(a: int) -> str: + ... + + + @overload + def overloaded_method(a: str) -> str: + ... + + + def overloaded_method(a): + """Foo bar documentation.""" + return str(a) + + ''')) + env.write_config(ignore="D100, D203") + out, err, code = env.invoke() + assert code == 0 + + +def test_overload_function_valid(env): + """Valid case for overload decorated functions. + + This shouldn't throw any errors. + """ + with env.open('example.py', 'wt') as example: + example.write(textwrap.dedent('''\ + from typing import overload + + + @overload + def overloaded_func(a: int) -> str: + ... + + + @overload + def overloaded_func(a: str) -> str: + ... + + + def overloaded_func(a): + """Foo bar documentation.""" + return str(a) + + ''')) + env.write_config(ignore="D100") + out, err, code = env.invoke() + assert code == 0 + + def test_overload_nested_function(env): """Nested functions decorated with @overload trigger D418 error.""" with env.open('example.py', 'wt') as example: From f66a3bbbf90cbc3f7309f551b8562bde86c9408a Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 13 Sep 2020 23:14:02 +0300 Subject: [PATCH 13/15] Added Tests for valid overloaded nested functions. --- src/tests/test_integration.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/tests/test_integration.py b/src/tests/test_integration.py index e6871e2d..22f57857 100644 --- a/src/tests/test_integration.py +++ b/src/tests/test_integration.py @@ -654,6 +654,36 @@ def overloaded_func(a): assert 'D103' not in out +def test_overload_nested_function_valid(env): + """Valid case for overload decorated nested functions. + + This shouldn't throw any errors. + """ + with env.open('example.py', 'wt') as example: + example.write(textwrap.dedent('''\ + from typing import overload + + def function_with_nesting(): + """Adding a docstring to a function.""" + @overload + def overloaded_func(a: int) -> str: + ... + + + @overload + def overloaded_func(a: str) -> str: + ... + + + def overloaded_func(a): + """Foo bar documentation.""" + return str(a) + ''')) + env.write_config(ignore="D100") + out, err, code = env.invoke() + assert code == 0 + + def test_conflicting_select_ignore_config(env): """Test that select and ignore are mutually exclusive.""" env.write_config(select="D100", ignore="D101") From 0d4825b8311ffd9513a09db533f5b31ec33c2738 Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 13 Sep 2020 23:45:39 +0300 Subject: [PATCH 14/15] release_notes.rst updated. --- docs/release_notes.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 265bf5fc..93470f4f 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -20,6 +20,16 @@ Bug Fixes * Treat "package" as an imperative verb for D401 (#356). +5.1.2 - September 13th, 2020 +---------------------------- + +New Features + +* Methods, Functions and Nested functions that have a docstring now throw D418 (#511). +* Methods decorated with @overload no longer reported as D102. +* Functions and nested functions decorated with @overload no longer reported as D103. + + 5.1.1 - August 29th, 2020 --------------------------- From ad564b2cb8de8d519f820c3b649e1c3ee01d17bc Mon Sep 17 00:00:00 2001 From: Yuval Raz Date: Sun, 13 Sep 2020 23:58:46 +0300 Subject: [PATCH 15/15] release_notes.rst updated. --- docs/release_notes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 93470f4f..4e555552 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -15,6 +15,9 @@ Major Updates New Features * Add flag to disable `# noqa` comment processing in API (#485). +* Methods, Functions and Nested functions that have a docstring now throw D418 (#511). +* Methods decorated with @overload no longer reported as D102 (#511). +* Functions and nested functions decorated with @overload no longer reported as D103 (#511). Bug Fixes