From ae708c4cf78d6c04763ee2dc744c490c0aab9392 Mon Sep 17 00:00:00 2001 From: Joshua Cannon Date: Mon, 22 Nov 2021 17:02:43 -0600 Subject: [PATCH 1/7] inaccessible --- src/pydocstyle/checker.py | 6 ++++-- src/pydocstyle/parser.py | 27 ++++++++++++++++++++----- src/pydocstyle/violations.py | 8 ++++++++ src/tests/parser_test.py | 38 ++++++++++++++++++++++++++++-------- 4 files changed, 64 insertions(+), 15 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index be74867f..ff4876f9 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, @@ -206,6 +207,7 @@ def check_docstring_missing(self, definition, docstring): Module: violations.D100, Class: violations.D101, NestedClass: violations.D106, + # InaccessibleClass: violations.D121, # currently Method: lambda: violations.D105() if definition.is_magic else ( @@ -217,7 +219,7 @@ def check_docstring_missing(self, definition, docstring): else None ) ), - NestedFunction: violations.D103, + InaccessibleFunction: violations.D109, Function: ( lambda: violations.D103() if not definition.is_overload diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 7165767a..fc1be5dc 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,11 +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): """A Python source code method.""" @@ -289,6 +298,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): @@ -303,6 +313,13 @@ def is_public(self): and self.parent.is_public ) +class InaccessibleClass(Class): + """A Python source code class, which is inaccessible. + + An class is inaccessible if it is defined inside of a function. + """ + + 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..c88930f9 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 class', +) +D123 = D1xx.create_error( + 'D123', + 'Missing docstring in inaccessible 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 582c6cde..515118f2 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,12 @@ 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`' + # @TODO: Test private inaccessible function + # @TODO: Test public/private inaccessible classes def test_class(): """Test parsing of a class.""" @@ -313,6 +326,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 +353,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 +372,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 +400,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 +419,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 +445,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 +462,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 +489,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,6 +507,7 @@ 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`' From 29bde244c61a1caac1d482281bcf59e35b9a816e Mon Sep 17 00:00:00 2001 From: Joshua Cannon Date: Mon, 22 Nov 2021 19:45:28 -0600 Subject: [PATCH 2/7] inaccessible --- src/pydocstyle/checker.py | 4 ++-- src/pydocstyle/config.py | 11 +++++++---- src/pydocstyle/parser.py | 6 ++++-- src/pydocstyle/violations.py | 4 ++-- src/tests/test_cases/functions.py | 13 +++++++++---- src/tests/test_cases/test.py | 6 ++++-- src/tests/test_integration.py | 2 +- 7 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index ff4876f9..ad2a4a62 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -207,7 +207,7 @@ def check_docstring_missing(self, definition, docstring): Module: violations.D100, Class: violations.D101, NestedClass: violations.D106, - # InaccessibleClass: violations.D121, # currently + InaccessibleClass: violations.D121, Method: lambda: violations.D105() if definition.is_magic else ( @@ -219,7 +219,7 @@ def check_docstring_missing(self, definition, docstring): else None ) ), - InaccessibleFunction: violations.D109, + 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 fe3afb3f..0d6d0da7 100644 --- a/src/pydocstyle/config.py +++ b/src/pydocstyle/config.py @@ -183,6 +183,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 = '' @@ -594,10 +595,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 fc1be5dc..86bc1619 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -17,8 +17,7 @@ 'Module', 'Package', 'Function', - 'InaccessibleFunction' - 'Method', + 'InaccessibleFunction' 'Method', 'Class', 'NestedClass', 'InaccessibleClass', @@ -251,6 +250,7 @@ class InaccessibleFunction(Function): is_accessible = False + class Method(Function): """A Python source code method.""" @@ -313,6 +313,7 @@ def is_public(self): and self.parent.is_public ) + class InaccessibleClass(Class): """A Python source code class, which is inaccessible. @@ -321,6 +322,7 @@ class InaccessibleClass(Class): 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 c88930f9..de799026 100644 --- a/src/pydocstyle/violations.py +++ b/src/pydocstyle/violations.py @@ -224,11 +224,11 @@ def to_rst(cls) -> str: ) D121 = D1xx.create_error( 'D121', - 'Missing docstring in inaccessible class', + 'Missing docstring in inaccessible public class', ) D123 = D1xx.create_error( 'D123', - 'Missing docstring in inaccessible function', + 'Missing docstring in inaccessible public function', ) D2xx = ErrorRegistry.create_group('D2', 'Whitespace Issues') diff --git a/src/tests/test_cases/functions.py b/src/tests/test_cases/functions.py index db7b9fab..c6b0ded9 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 8154c960..a2afd1ac 100644 --- a/src/tests/test_cases/test.py +++ b/src/tests/test_cases/test.py @@ -67,10 +67,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 +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 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 From 1584eb879eb1c63b95bb74340c8ef00d09406cb7 Mon Sep 17 00:00:00 2001 From: Joshua Cannon Date: Mon, 22 Nov 2021 19:48:09 -0600 Subject: [PATCH 3/7] class --- src/pydocstyle/parser.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 86bc1619..d36372cb 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -318,6 +318,11 @@ 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 From a8a25f205ebbb24dee781bbbe28c5b7d82e52fd2 Mon Sep 17 00:00:00 2001 From: Joshua Cannon Date: Mon, 22 Nov 2021 20:19:52 -0600 Subject: [PATCH 4/7] more tests --- src/tests/parser_test.py | 128 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 2 deletions(-) diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 515118f2..8b4b2905 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -301,8 +301,49 @@ def inner_function(): assert not inner_function.is_accessible assert str(inner_function) == 'in public inaccessible function `inner_function`' - # @TODO: Test private inaccessible function - # @TODO: Test public/private inaccessible classes +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.""" @@ -510,6 +551,89 @@ class InnerClass(object): 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.""" From d9a53fcb30b17cac7c8f9be3c8cdb6972490b145 Mon Sep 17 00:00:00 2001 From: Joshua Cannon Date: Tue, 23 Nov 2021 08:57:41 -0600 Subject: [PATCH 5/7] Update src/pydocstyle/parser.py --- src/pydocstyle/parser.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index d36372cb..b0eef5fa 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -17,7 +17,8 @@ 'Module', 'Package', 'Function', - 'InaccessibleFunction' 'Method', + 'InaccessibleFunction', + 'Method', 'Class', 'NestedClass', 'InaccessibleClass', From 76af6066886284950244e90debac277cfb9fa480 Mon Sep 17 00:00:00 2001 From: Joshua Cannon Date: Tue, 23 Nov 2021 09:16:42 -0600 Subject: [PATCH 6/7] reease notes --- docs/release_notes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 72332114..92abdb19 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). +* Change handling of "inaccessible" functions and classes (#561). 6.1.1 - May 17th, 2021 --------------------------- From baece1e56c444d09e7f0eda380af01b9f6c8951a Mon Sep 17 00:00:00 2001 From: Joshua Cannon Date: Tue, 23 Nov 2021 09:22:37 -0600 Subject: [PATCH 7/7] Update parser.py --- src/pydocstyle/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index b0eef5fa..5b2da36c 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -17,7 +17,7 @@ 'Module', 'Package', 'Function', - 'InaccessibleFunction', + 'InaccessibleFunction', 'Method', 'Class', 'NestedClass',