diff --git a/docs/release_notes.rst b/docs/release_notes.rst index bb4595e..e85f467 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -35,6 +35,7 @@ New Features * Allow for hanging indent when documenting args in Google style. (#449) * Add support for `property_decorators` config to ignore D401. * Add support for Python 3.10 (#554). +* Change handling of "inaccessible" functions and classes (#561). * Replace D10X errors with D419 if docstring exists but is empty (#559). Bug Fixes diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index 5cefc76..827dbe1 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -15,10 +15,11 @@ Class, Definition, Function, + InaccessibleClass, + InaccessibleFunction, Method, Module, NestedClass, - NestedFunction, Package, ParseError, Parser, @@ -204,6 +205,7 @@ def check_docstring_missing(self, definition, docstring): Module: violations.D100, Class: violations.D101, NestedClass: violations.D106, + InaccessibleClass: violations.D121, Method: lambda: violations.D105() if definition.is_magic else ( @@ -215,7 +217,7 @@ def check_docstring_missing(self, definition, docstring): else None ) ), - NestedFunction: violations.D103, + InaccessibleFunction: violations.D123, Function: ( lambda: violations.D103() if not definition.is_overload diff --git a/src/pydocstyle/config.py b/src/pydocstyle/config.py index c05f7dc..231cf35 100644 --- a/src/pydocstyle/config.py +++ b/src/pydocstyle/config.py @@ -188,6 +188,7 @@ class ConfigurationParser: ) BASE_ERROR_SELECTION_OPTIONS = ('ignore', 'select', 'convention') + DEFAULT_IGNORE = {"D121", "D123"} DEFAULT_MATCH_RE = r'(?!test_).*\.py' DEFAULT_MATCH_DIR_RE = r'[^\.].*' DEFAULT_IGNORE_DECORATORS_RE = '' @@ -599,10 +600,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 -= cls.DEFAULT_IGNORE + 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 cc768bb..f7539e3 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 8156921..1c84022 100644 --- a/src/pydocstyle/violations.py +++ b/src/pydocstyle/violations.py @@ -222,6 +222,14 @@ 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', +) D2xx = ErrorRegistry.create_group('D2', 'Whitespace Issues') D200 = D2xx.create_error( diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 582c6cd..8b4b290 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/functions.py b/src/tests/test_cases/functions.py index db7b9fa..c6b0ded 100644 --- a/src/tests/test_cases/functions.py +++ b/src/tests/test_cases/functions.py @@ -28,15 +28,17 @@ def inner(): pass +expect("inner", "D123: Missing docstring in inaccessible public function") def func_with_inner_async_func_after(): """Test a function with inner async function after docstring.""" - async def inner(): + async def inner_async(): pass pass +expect("inner_async", "D123: Missing docstring in inaccessible public function") def fake_decorator(decorated): """Fake decorator used to test decorated inner func.""" @@ -47,30 +49,33 @@ def func_with_inner_decorated_func_after(): """Test a function with inner decorated function after docstring.""" @fake_decorator - def inner(): + def inner_decorated(): pass pass +expect("inner_decorated", "D123: Missing docstring in inaccessible public 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 inner_decorated_async(): pass pass +expect("inner_decorated_async", "D123: Missing docstring in inaccessible public function") def func_with_inner_class_after(): """Test a function with inner class after docstring.""" - class inner(): + class inner_class(): pass pass +expect("inner_class", "D121: Missing docstring in inaccessible public class") def func_with_weird_backslash(): """Test a function with a weird backslash.\ diff --git a/src/tests/test_cases/test.py b/src/tests/test_cases/test.py index 1cbd8ec..cb5a2f9 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -67,6 +67,7 @@ def __call__(self=None, x=None, y=None, z=None): @expect('D419: Docstring is empty') def function(): """ """ + @expect('D123: Missing docstring in inaccessible public function') def ok_since_nested(): pass @@ -94,7 +95,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: diff --git a/src/tests/test_integration.py b/src/tests/test_integration.py index 2f2f57c..861fe3a 100644 --- a/src/tests/test_integration.py +++ b/src/tests/test_integration.py @@ -829,7 +829,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