diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 72332114..9575ac25 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -12,6 +12,7 @@ New Features * Add support for `property_decorators` config to ignore D401. * Add support for Python 3.10 (#554). +# Add reporting for private definitions (#562) 6.1.1 - May 17th, 2021 --------------------------- diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index be74867f..eb768787 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -14,10 +14,11 @@ Class, Definition, Function, + InaccessibleClass, + InaccessibleFunction, Method, Module, NestedClass, - NestedFunction, Package, ParseError, Parser, @@ -184,7 +185,7 @@ def checks(self): @check_for(Definition, terminal=True) def check_docstring_missing(self, definition, docstring): - """D10{0,1,2,3}: Public definitions should have docstrings. + """D1XX: Definitions should have docstrings. All modules should normally have docstrings. [...] all functions and classes exported by a module should also have docstrings. Public @@ -196,36 +197,36 @@ def check_docstring_missing(self, definition, docstring): with a single underscore. """ + + def violation(code): + code = code if definition.is_public else code + 50 + return getattr(violations, f"D{code}")() + 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() + Module: lambda: 100, + Class: lambda: 101, + NestedClass: lambda: 106, + InaccessibleClass: lambda: 121, + Method: lambda: 105 if definition.is_magic else ( - violations.D107() + 107 if definition.is_init - else ( - violations.D102() - if not definition.is_overload - else None - ) + else (102 if not definition.is_overload else None) ), - NestedFunction: violations.D103, + InaccessibleFunction: lambda: 123, Function: ( - lambda: violations.D103() - if not definition.is_overload - else None + lambda: 103 if not definition.is_overload else None ), - Package: violations.D104, + Package: lambda: 104, } - return codes[type(definition)]() + code = codes[type(definition)]() + return violation(code) if code is not None else None @check_for(Definition) def check_one_liners(self, definition, docstring): diff --git a/src/pydocstyle/config.py b/src/pydocstyle/config.py index fe3afb3f..99bea4dc 100644 --- a/src/pydocstyle/config.py +++ b/src/pydocstyle/config.py @@ -11,7 +11,7 @@ from re import compile as re from .utils import __version__, log -from .violations import ErrorRegistry, conventions +from .violations import ErrorRegistry, conventions, default_ignored try: import toml @@ -594,10 +594,12 @@ def _get_exclusive_error_codes(cls, options): if options.ignore is not None: ignored = cls._expand_error_codes(options.ignore) checked_codes = codes - ignored - elif options.select is not None: - checked_codes = cls._expand_error_codes(options.select) - elif options.convention is not None: - checked_codes = getattr(conventions, options.convention) + else: + codes -= default_ignored + if options.select is not None: + checked_codes = cls._expand_error_codes(options.select) + elif options.convention is not None: + checked_codes = getattr(conventions, options.convention) # To not override the conventions nor the options - copy them. return copy.deepcopy(checked_codes) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 7165767a..5b2da36c 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -17,10 +17,11 @@ 'Module', 'Package', 'Function', - 'NestedFunction', + 'InaccessibleFunction', 'Method', 'Class', 'NestedClass', + 'InaccessibleClass', 'AllError', 'StringIO', 'ParseError', @@ -199,8 +200,9 @@ class Function(Definition): """A Python source code function.""" _nest = staticmethod( - lambda s: {'def': NestedFunction, 'class': NestedClass}[s] + lambda s: {'def': InaccessibleFunction, 'class': InaccessibleClass}[s] ) + is_accessible = True @property def is_public(self): @@ -236,10 +238,18 @@ def is_test(self): return self.name.startswith('test') or self.name == 'runTest' -class NestedFunction(Function): - """A Python source code nested function.""" +class InaccessibleFunction(Function): + """A Python source code function which is inaccessible. - is_public = False + A function is inaccessible if it is defined inside another function. + + Publicness is still evaluated based on the name, to allow devs to signal between public and + private if they so wish. (E.g. if a function returns another function, they may want the inner + function to be documented. Conversely a purely helper inner function might not need to be + documented) + """ + + is_accessible = False class Method(Function): @@ -289,6 +299,7 @@ class Class(Definition): _nest = staticmethod(lambda s: {'def': Method, 'class': NestedClass}[s]) is_public = Function.is_public is_class = True + is_accessible = True class NestedClass(Class): @@ -304,6 +315,20 @@ def is_public(self): ) +class InaccessibleClass(Class): + """A Python source code class, which is inaccessible. + + An class is inaccessible if it is defined inside of a function. + + Publicness is still evaluated based on the name, to allow devs to signal between public and + private if they so wish. (E.g. if a function returns a class, they may want the returned + class to be documented. Conversely a purely helper inner class might not need to be + documented) + """ + + is_accessible = False + + class Decorator(Value): """A decorator for function, method or class.""" diff --git a/src/pydocstyle/violations.py b/src/pydocstyle/violations.py index 60fc064e..64d74b2e 100644 --- a/src/pydocstyle/violations.py +++ b/src/pydocstyle/violations.py @@ -222,6 +222,55 @@ def to_rst(cls) -> str: 'D107', 'Missing docstring in __init__', ) +D121 = D1xx.create_error( + 'D121', + 'Missing docstring in inaccessible public class', +) +D123 = D1xx.create_error( + 'D123', + 'Missing docstring in inaccessible public function', +) +# For private docstrings, we add 50 to the public codes +D150 = D1xx.create_error( + 'D150', + 'Missing docstring in private module', +) +D151 = D1xx.create_error( + 'D151', + 'Missing docstring in private class', +) +D152 = D1xx.create_error( + 'D152', + 'Missing docstring in private method', +) +D153 = D1xx.create_error( + 'D153', + 'Missing docstring in private function', +) +D154 = D1xx.create_error( + 'D154', + 'Missing docstring in private package', +) +D155 = D1xx.create_error( + 'D155', + 'Missing docstring in private magic method', +) +D156 = D1xx.create_error( + 'D156', + 'Missing docstring in private nested class', +) +D157 = D1xx.create_error( + 'D157', + 'Missing docstring in private __init__', +) +D171 = D1xx.create_error( + 'D171', + 'Missing docstring in inaccessible private class', +) +D173 = D1xx.create_error( + 'D173', + 'Missing docstring in inaccessible private function', +) D2xx = ErrorRegistry.create_group('D2', 'Whitespace Issues') D200 = D2xx.create_error( @@ -423,11 +472,25 @@ def __getattr__(self, item: str) -> Any: all_errors = set(ErrorRegistry.get_error_codes()) - +default_ignored = { + "D121", + "D123", + "D150", + "D151", + "D152", + "D153", + "D154", + "D155", + "D156", + "D157", + "D171", + "D173", +} conventions = AttrDict( { 'pep257': all_errors + - default_ignored - { 'D203', 'D212', @@ -449,6 +512,7 @@ def __getattr__(self, item: str) -> Any: 'D418', }, 'numpy': all_errors + - default_ignored - { 'D107', 'D203', @@ -461,6 +525,7 @@ def __getattr__(self, item: str) -> Any: 'D417', }, 'google': all_errors + - default_ignored - { 'D203', 'D204', diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 582c6cde..8b4b2905 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -47,6 +47,7 @@ def do_something(pos_param0, pos_param1, kw_param0="default"): assert function.error_lineno == 2 assert function.source == code.getvalue() assert function.is_public + assert function.is_accessible assert str(function) == 'in public function `do_something`' @@ -76,6 +77,7 @@ def do_something(pos_param0, pos_param1, kw_param0="default"): assert function.error_lineno == 2 assert function.source == code.getvalue() assert function.is_public + assert function.is_accessible assert str(function) == 'in public function `do_something`' @@ -111,6 +113,7 @@ def do_something(pos_param0, pos_param1, kw_param0="default"): return None """) assert function.is_public + assert function.is_accessible assert str(function) == 'in public function `do_something`' @@ -140,6 +143,7 @@ def do_something(): return None """) assert function.is_public + assert function.is_accessible assert str(function) == 'in public function `do_something`' @@ -167,6 +171,7 @@ def inner_function(): assert outer_function.error_lineno == 2 assert outer_function.source == code.getvalue() assert outer_function.is_public + assert outer_function.is_accessible assert str(outer_function) == 'in public function `outer_function`' inner_function, = outer_function.children @@ -183,8 +188,9 @@ def inner_function(): '''This is the inner function.''' return None """) - assert not inner_function.is_public - assert str(inner_function) == 'in private nested function `inner_function`' + assert inner_function.is_public + assert not inner_function.is_accessible + assert str(inner_function) == 'in public inaccessible function `inner_function`' def test_conditional_nested_function(): @@ -211,6 +217,7 @@ def inner_function(): assert outer_function.end == 7 assert outer_function.source == code.getvalue() assert outer_function.is_public + assert outer_function.is_accessible assert str(outer_function) == 'in public function `outer_function`' inner_function, = outer_function.children @@ -226,8 +233,9 @@ def inner_function(): '''This is the inner function.''' return None """) - assert not inner_function.is_public - assert str(inner_function) == 'in private nested function `inner_function`' + assert inner_function.is_public + assert not inner_function.is_accessible + assert str(inner_function) == 'in public inaccessible function `inner_function`' def test_doubly_nested_function(): @@ -254,6 +262,7 @@ def inner_function(): assert outer_function.end == 7 assert outer_function.source == code.getvalue() assert outer_function.is_public + assert outer_function.is_accessible assert str(outer_function) == 'in public function `outer_function`' middle_function, = outer_function.children @@ -270,9 +279,10 @@ def inner_function(): '''This is the inner function.''' return None """) - assert not middle_function.is_public + assert middle_function.is_public + assert not middle_function.is_accessible assert (str(middle_function) == - 'in private nested function `middle_function`') + 'in public inaccessible function `middle_function`') inner_function, = middle_function.children assert inner_function.name == 'inner_function' @@ -287,9 +297,53 @@ def inner_function(): '''This is the inner function.''' return None """) - assert not inner_function.is_public - assert str(inner_function) == 'in private nested function `inner_function`' + assert inner_function.is_public + assert not inner_function.is_accessible + assert str(inner_function) == 'in public inaccessible function `inner_function`' + +def test_private_nested_function(): + """Test parsing of a nested function which looks private.""" + parser = Parser() + code = CodeSnippet("""\ + def outer_function(): + \"""This is the outer function.\""" + if True: + def _inner_function(): + '''This is the inner function.''' + return None + return None + """) + module = parser.parse(code, 'file_path') + + outer_function, = module.children + assert outer_function.name == 'outer_function' + assert outer_function.decorators == [] + assert outer_function.docstring == '"""This is the outer function."""' + assert outer_function.kind == 'function' + assert outer_function.parent == module + assert outer_function.start == 1 + assert outer_function.end == 7 + assert outer_function.source == code.getvalue() + assert outer_function.is_public + assert outer_function.is_accessible + assert str(outer_function) == 'in public function `outer_function`' + inner_function, = outer_function.children + assert inner_function.name == '_inner_function' + assert inner_function.decorators == [] + assert inner_function.docstring == "'''This is the inner function.'''" + assert inner_function.kind == 'function' + assert inner_function.parent == outer_function + assert inner_function.start == 4 + assert inner_function.end == 6 + assert textwrap.dedent(inner_function.source) == textwrap.dedent("""\ + def _inner_function(): + '''This is the inner function.''' + return None + """) + assert not inner_function.is_public + assert not inner_function.is_accessible + assert str(inner_function) == 'in private inaccessible function `_inner_function`' def test_class(): """Test parsing of a class.""" @@ -313,6 +367,7 @@ class TestedClass(object): assert klass.error_lineno == 3 assert klass.source == code.getvalue() assert klass.is_public + assert klass.is_accessible assert str(klass) == 'in public class `TestedClass`' @@ -339,6 +394,7 @@ def do_it(param): assert klass.error_lineno == 1 assert klass.source == code.getvalue() assert klass.is_public + assert klass.is_accessible assert str(klass) == 'in public class `TestedClass`' method, = klass.children @@ -357,6 +413,7 @@ def do_it(param): return None """) assert method.is_public + assert method.is_accessible assert not method.is_magic assert str(method) == 'in public method `do_it`' @@ -384,6 +441,7 @@ def _do_it(param): assert klass.error_lineno == 1 assert klass.source == code.getvalue() assert klass.is_public + assert klass.is_accessible assert str(klass) == 'in public class `TestedClass`' method, = klass.children @@ -402,6 +460,7 @@ def _do_it(param): return None """) assert not method.is_public + assert method.is_accessible assert not method.is_magic assert str(method) == 'in private method `_do_it`' @@ -427,6 +486,7 @@ def __str__(self): assert klass.error_lineno == 1 assert klass.source == code.getvalue() assert klass.is_public + assert klass.is_accessible assert str(klass) == 'in public class `TestedClass`' method, = klass.children[0] @@ -443,6 +503,7 @@ def __str__(self): return "me" """) assert method.is_public + assert method.is_accessible assert method.is_magic assert str(method) == 'in public method `__str__`' @@ -469,6 +530,7 @@ class InnerClass(object): assert outer_class.error_lineno == 2 assert outer_class.source == code.getvalue() assert outer_class.is_public + assert outer_class.is_accessible assert str(outer_class) == 'in public class `OuterClass`' inner_class, = outer_class.children @@ -486,8 +548,92 @@ class InnerClass(object): "An inner docstring." """) assert inner_class.is_public + assert inner_class.is_accessible assert str(inner_class) == 'in public nested class `InnerClass`' +def test_public_inaccessible_class(): + """Test parsing of a class inside a function.""" + parser = Parser() + code = CodeSnippet("""\ + def outer_function(): + ' an outer docstring' + class InnerClass(object): + "An inner docstring." + """) + module = parser.parse(code, 'file_path') + + outer_function, = module.children + assert outer_function.name == 'outer_function' + assert outer_function.decorators == [] + assert outer_function.docstring == "' an outer docstring'" + assert outer_function.kind == 'function' + assert outer_function.parent == module + assert outer_function.start == 1 + assert outer_function.end == 4 + assert outer_function.source == code.getvalue() + assert outer_function.is_public + assert outer_function.is_accessible + assert str(outer_function) == 'in public function `outer_function`' + + inner_class, = outer_function.children + assert inner_class.name == 'InnerClass' + assert inner_class.decorators == [] + assert inner_class.children == [] + assert inner_class.docstring == '"An inner docstring."' + assert inner_class.kind == 'class' + assert inner_class.parent == outer_function + assert inner_class.start == 3 + assert inner_class.end == 4 + assert inner_class.error_lineno == 4 + assert textwrap.dedent(inner_class.source) == textwrap.dedent("""\ + class InnerClass(object): + "An inner docstring." + """) + assert inner_class.is_public + assert not inner_class.is_accessible + assert str(inner_class) == 'in public inaccessible class `InnerClass`' + +def test_private_inaccessible_class(): + """Test parsing of a class inside a function which looks private.""" + parser = Parser() + code = CodeSnippet("""\ + def outer_function(): + ' an outer docstring' + class _InnerClass(object): + "An inner docstring." + """) + module = parser.parse(code, 'file_path') + + outer_function, = module.children + assert outer_function.name == 'outer_function' + assert outer_function.decorators == [] + assert outer_function.docstring == "' an outer docstring'" + assert outer_function.kind == 'function' + assert outer_function.parent == module + assert outer_function.start == 1 + assert outer_function.end == 4 + assert outer_function.source == code.getvalue() + assert outer_function.is_public + assert outer_function.is_accessible + assert str(outer_function) == 'in public function `outer_function`' + + inner_class, = outer_function.children + assert inner_class.name == '_InnerClass' + assert inner_class.decorators == [] + assert inner_class.children == [] + assert inner_class.docstring == '"An inner docstring."' + assert inner_class.kind == 'class' + assert inner_class.parent == outer_function + assert inner_class.start == 3 + assert inner_class.end == 4 + assert inner_class.error_lineno == 4 + assert textwrap.dedent(inner_class.source) == textwrap.dedent("""\ + class _InnerClass(object): + "An inner docstring." + """) + assert not inner_class.is_public + assert not inner_class.is_accessible + assert str(inner_class) == 'in private inaccessible class `_InnerClass`' def test_raise_from(): """Make sure 'raise x from y' doesn't trip the parser.""" diff --git a/src/tests/test_cases/all_import_as.py b/src/tests/test_cases/all_import_as.py index ea25ba62..069b9128 100644 --- a/src/tests/test_cases/all_import_as.py +++ b/src/tests/test_cases/all_import_as.py @@ -13,6 +13,6 @@ def public_func(): pass - +@expect("D153: Missing docstring in private function") def private_func(): pass diff --git a/src/tests/test_cases/functions.py b/src/tests/test_cases/functions.py index db7b9fab..b53b3522 100644 --- a/src/tests/test_cases/functions.py +++ b/src/tests/test_cases/functions.py @@ -23,20 +23,32 @@ def func_with_space_after(): def func_with_inner_func_after(): """Test a function with inner function after docstring.""" - def inner(): + def public_inner(): + pass + + def _private_inner(): pass pass +expect("public_inner", "D123: Missing docstring in inaccessible public function") +expect("_private_inner", "D173: Missing docstring in inaccessible private function") + def func_with_inner_async_func_after(): """Test a function with inner async function after docstring.""" - async def inner(): + async def public_inner_async(): + pass + + async def _private_inner_async(): pass pass +expect("public_inner_async", "D123: Missing docstring in inaccessible public function") +expect("_private_inner_async", "D173: Missing docstring in inaccessible private function") + def fake_decorator(decorated): """Fake decorator used to test decorated inner func.""" @@ -47,30 +59,50 @@ def func_with_inner_decorated_func_after(): """Test a function with inner decorated function after docstring.""" @fake_decorator - def inner(): + def public_inner_decorated(): + pass + + @fake_decorator + def _private_inner_decorated(): pass pass +expect("public_inner_decorated", "D123: Missing docstring in inaccessible public function") +expect("_private_inner_decorated", "D173: Missing docstring in inaccessible private function") + def func_with_inner_decorated_async_func_after(): """Test a function with inner decorated async function after docstring.""" @fake_decorator - async def inner(): + async def public_inner_decorated_async(): + pass + + @fake_decorator + async def _prviate_inner_decorated_async(): pass pass +expect("public_inner_decorated_async", "D123: Missing docstring in inaccessible public function") +expect("_prviate_inner_decorated_async", "D173: Missing docstring in inaccessible private function") + def func_with_inner_class_after(): """Test a function with inner class after docstring.""" - class inner(): + class public_inner_class(): + pass + + class _private_inner_class(): pass pass +expect("public_inner_class", "D121: Missing docstring in inaccessible public class") +expect("_private_inner_class", "D171: Missing docstring in inaccessible private class") + def func_with_weird_backslash(): """Test a function with a weird backslash.\ diff --git a/src/tests/test_cases/nested_class.py b/src/tests/test_cases/nested_class.py index 07257918..2865b780 100644 --- a/src/tests/test_cases/nested_class.py +++ b/src/tests/test_cases/nested_class.py @@ -21,13 +21,25 @@ class PublicNestedClass: class PublicNestedClassInPublicNestedClass: pass + expect('_PrivateNestedClassInPublicNestedClass', + 'D156: Missing docstring in private nested class') + class _PrivateNestedClassInPublicNestedClass: pass + expect('_PrivateNestedClass', + 'D156: Missing docstring in private nested class') + class _PrivateNestedClass: + expect('PublicNestedClassInPrivateNestedClass', + 'D156: Missing docstring in private nested class') + class PublicNestedClassInPrivateNestedClass: pass + expect('_PrivateNestedClassInPrivateNestedClass', + 'D156: Missing docstring in private nested class') + class _PrivateNestedClassInPrivateNestedClass: pass diff --git a/src/tests/test_cases/sections.py b/src/tests/test_cases/sections.py index d671102b..9289cc92 100644 --- a/src/tests/test_cases/sections.py +++ b/src/tests/test_cases/sections.py @@ -277,7 +277,7 @@ def missing_colon_google_style_section(): # noqa: D406, D407 @expect("D417: Missing argument descriptions in the docstring " "(argument(s) y are missing descriptions in " "'bar' docstring)", func_name="bar") -def _test_nested_functions(): +def _test_nested_functions(): # noqa: D153 x = 1 def bar(y=2): # noqa: D207, D213, D406, D407 diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 8154c960..32385bbe 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -22,7 +22,16 @@ class meta: def method(self=None): pass - def _ok_since_private(self=None): + @expect('D152: Missing docstring in private method') + def _private_method(self=None): + pass + + @expect('D152: Missing docstring in private method') + def __private_mangled_method(self=None): + pass + + @expect('D152: Missing docstring in private method') + def _private_fancy_method_(self=None): pass @overload @@ -67,10 +76,11 @@ def __call__(self=None, x=None, y=None, z=None): @expect('D103: Missing docstring in public function') def function(): """ """ + @expect('D123: Missing docstring in inaccessible public function') def ok_since_nested(): pass - @expect('D103: Missing docstring in public function') + @expect('D123: Missing docstring in inaccessible public function') def nested(): '' @@ -94,7 +104,8 @@ def nested_overloaded_func(a): expect('nested_overloaded_func', "D418: Function/ Method decorated with @overload" " shouldn't contain a docstring") - +expect('nested_overloaded_func', + "D123: Missing docstring in inaccessible public function") @overload def overloaded_func(a: int) -> str: @@ -532,3 +543,29 @@ def __init__(self, x): expect(os.path.normcase(__file__ if __file__[-1] != 'c' else __file__[:-1]), 'D100: Missing docstring in public module') + +expect('_PrivateClass', 'D151: Missing docstring in private class') + +class _PrivateClass: + @expect("D157: Missing docstring in private __init__") + def __init__(self=None): + pass + + @expect("D152: Missing docstring in private method") + def public_method_private_class(self=None): + pass + + @expect("D152: Missing docstring in private method") + def private_method_private_class(self=None): + pass + + @expect("D155: Missing docstring in private magic method") + def __str__(self=None): + pass + + class PublicNestedClass: + pass + + expect('PublicNestedClass', 'D156: Missing docstring in private nested class') + + diff --git a/src/tests/test_integration.py b/src/tests/test_integration.py index 7e399cce..1f5a88a6 100644 --- a/src/tests/test_integration.py +++ b/src/tests/test_integration.py @@ -769,7 +769,7 @@ def overloaded_func(a): """Foo bar documentation.""" return str(a) ''')) - env.write_config(ignore="D100") + env.write_config(ignore="D100,D123") out, err, code = env.invoke() assert code == 0