Correctly detect publicity of modules inside directories (#494)

* Fix Module.is_public() when the module is not a the root

* Add PR reference

* Use pathlib

* Use pathlib instead of os for test_module_publicity

* Update release notes for #493

* Use forward slash '/' operator instead of .joinpath()

* Fix pull-request number in release notes

* Fix publicity of module in private package

* Update test_module_publicity docstring

* Add test for directory starting with double underscore

* Make packages containing double-underscore public

* Add test to assert __init__ module is public

* Make modules in a __private_package private

* Fix lint errors from lines being too long

* Update publicity.rst with more information

* Parameterize module publicity tests and include .py file extension in test path parameters

* Make module publicity determination respect $PYTHONPATH

* Fix line-length issue

* Reword comment

* Add tests with the same path over different sys.path cases

* Add note about checking sys.path for determining publicity

* Apply suggestions from code review

Co-authored-by: Thibault Derousseaux <[email protected]>
Co-authored-by: Thibault Derousseaux <[email protected]>
Co-authored-by: Sambhav Kothari <[email protected]>

Author: 3 people

.gitignore

@@ -54,3 +54,6 @@ docs/snippets/error_code_table.rst
 
 # PyCharm files
 .idea
+
+# VS Code
+.vscode/

docs/release_notes.rst

@@ -20,8 +20,8 @@ Bug Fixes
   The bug caused some argument names to go unreported in D417 (#448).
 * Fixed an issue where skipping errors on module level docstring via #noqa
   failed when there where more prior comments (#446).
-* Support backslash-continued descriptions in docstrings
-  (#472).
+* Support backslash-continued descriptions in docstrings (#472).
+* Correctly detect publicity of modules inside directories (#470, #494).
 
 
 5.0.2 - January 8th, 2020

docs/snippets/publicity.rst

@@ -10,7 +10,9 @@ Publicity for all constructs is determined as follows: a construct is
 considered *public* if -
 
 1. Its immediate parent is public *and*
-2. Its name does not contain a single leading underscore.
+2. Its name does *not* start with a single or double underscore.
+
+    a. Note, names that start and end with a double underscore are *public* (e.g. ``__init__.py``).
 
 A construct's immediate parent is the construct that contains it. For example,
 a method's parent is a class object. A class' parent is usually a module, but
@@ -25,6 +27,12 @@ a class called ``_Foo`` is considered private. A method ``bar`` in ``_Foo`` is
 also considered private since its parent is a private class, even though its
 name does not begin with a single underscore.
 
+Note, a module's parent is recursively checked upward until we reach a directory
+in ``sys.path`` to avoid considering the complete filepath of a module.
+For example, consider the module ``/_foo/bar/baz.py``.
+If ``PYTHONPATH`` is set to ``/``, then ``baz.py`` is *private*.
+If ``PYTHONPATH`` is set to ``/_foo/``, then ``baz.py`` is *public*.
+
 Modules are parsed to look if ``__all__`` is defined. If so, only those top
 level constructs are considered public. The parser looks for ``__all__``
 defined as a literal list or tuple. As the parser doesn't execute the module,

src/pydocstyle/parser.py

@@ -1,10 +1,12 @@
 """Python code parser."""
 
+import sys
 import textwrap
 import tokenize as tk
 from itertools import chain, dropwhile
 from re import compile as re
 from io import StringIO
+from pathlib import Path
 
 from .utils import log
 
@@ -117,7 +119,36 @@ def is_public(self):
 
         This helps determine if it requires a docstring.
         """
-        return not self.name.startswith('_') or self.name.startswith('__')
+        module_name = Path(self.name).stem
+        return (
+            not self._is_inside_private_package() and
+            self._is_public_name(module_name)
+        )
+
+    def _is_inside_private_package(self):
+        """Return True if the module is inside a private package."""
+        path = Path(self.name).parent  # Ignore the actual module's name.
+        syspath = [Path(p) for p in sys.path]  # Convert to pathlib.Path.
+
+        # Bail if we are at the root directory or in `PYTHONPATH`.
+        while path != path.parent and path not in syspath:
+            if self._is_private_name(path.name):
+                return True
+            path = path.parent
+
+        return False
+
+    def _is_public_name(self, module_name):
+        """Determine whether a "module name" (i.e. module or package name) is public."""
+        return (
+            not module_name.startswith('_') or (
+                module_name.startswith('__') and module_name.endswith('__')
+            )
+        )
+
+    def _is_private_name(self, module_name):
+        """Determine whether a "module name" (i.e. module or package name) is private."""
+        return not self._is_public_name(module_name)
 
     def __str__(self):
         return 'at module level'

src/tests/parser_test.py

@@ -4,6 +4,7 @@
 import sys
 import pytest
 import textwrap
+from pathlib import Path
 
 from pydocstyle.parser import Parser, ParseError
 
@@ -562,19 +563,75 @@ def test_matrix_multiplication_with_decorators(code):
     assert inner_function.decorators[0].name == 'a'
 
 
-def test_module_publicity():
-    """Test that a module that has a single leading underscore is private."""
+@pytest.mark.parametrize("public_path", (
+    Path(""),
+    Path("module.py"),
+    Path("package") / "module.py",
+    Path("package") / "__init__.py",
+    Path("") / "package" / "module.py",
+    Path("") / "__dunder__" / "package" / "module.py"
+))
+def test_module_publicity_with_public_path(public_path):
+    """Test module publicity with public path.
+
+    Module names such as my_module.py are considered public.
+
+    Special "dunder" modules,
+    with leading and trailing double-underscores (e.g. __init__.py) are public.
+
+    The same rules for publicity apply to both packages and modules.
+    """
     parser = Parser()
     code = CodeSnippet("")
-
-    module = parser.parse(code, "filepath")
+    module = parser.parse(code, str(public_path))
     assert module.is_public
 
-    module = parser.parse(code, "_filepath")
+
+@pytest.mark.parametrize("private_path", (
+    # single underscore
+    Path("_private_module.py"),
+    Path("_private_package") / "module.py",
+    Path("_private_package") / "package" / "module.py",
+    Path("") / "_private_package" / "package" / "module.py",
+
+    # double underscore
+    Path("__private_module.py"),
+    Path("__private_package") / "module.py",
+    Path("__private_package") / "package" / "module.py",
+    Path("") / "__private_package" / "package" / "module.py"
+))
+def test_module_publicity_with_private_paths(private_path):
+    """Test module publicity with private path.
+
+    Module names starting with single or double-underscore are private.
+    For example, _my_private_module.py and __my_private_module.py.
+
+    Any module within a private package is considered private.
+
+    The same rules for publicity apply to both packages and modules.
+    """
+    parser = Parser()
+    code = CodeSnippet("")
+    module = parser.parse(code, str(private_path))
     assert not module.is_public
 
-    module = parser.parse(code, "__filepath")
-    assert module.is_public
+
+@pytest.mark.parametrize("syspath,is_public", (
+    ("/", False),
+    ("_foo/", True),
+))
+def test_module_publicity_with_different_sys_path(syspath,
+                                                  is_public,
+                                                  monkeypatch):
+    """Test module publicity for same path and different sys.path."""
+    parser = Parser()
+    code = CodeSnippet("")
+
+    monkeypatch.syspath_prepend(syspath)
+
+    path = Path("_foo") / "bar" / "baz.py"
+    module = parser.parse(code, str(path))
+    assert module.is_public == is_public
 
 
 def test_complex_module():