More test cases/data

.github/workflows/ci.yml

@@ -30,8 +30,7 @@ jobs:
         python -m pip install ruff pytest pytest-mock coverage pyright
     - name: Check ruff
       run: |
-        # Ignore unused imports as they are needed to test modguard
-        ruff check --ignore F401
+        ruff check .
     - name: Test with pytest and report coverage
       run: |
         cd tests

modguard/parsing/public.py

@@ -72,7 +72,6 @@ def _extract_allowlist(self, public_call: ast.Call) -> Optional[list[str]]:
                         for elt in allowlist_value.elts
                         if isinstance(elt, ast.Constant) and isinstance(elt.value, str)
                     ] or None
-        return None
 
     def _add_public_member_from_decorator(
         self, node: Union[ast.FunctionDef, ast.ClassDef], decorator: ast.expr
@@ -239,7 +238,7 @@ def visit_ClassDef(self, node: ast.ClassDef):
         self.depth -= 1
 
 
-def _public_module_prelude(should_import: bool = True) -> str:
+def _public_module_end(should_import: bool = True) -> str:
     if should_import:
         return "import modguard\nmodguard.public()\n"
     return "modguard.public()\n"
@@ -262,8 +261,8 @@ def mark_as_public(file_path: str, member_name: str = ""):
         modguard_public_is_imported = is_modguard_imported(parsed_ast, "public")
         if not member_name:
             file.write(
-                _public_module_prelude(should_import=not modguard_public_is_imported)
-                + file_content
+                file_content +
+                _public_module_end(should_import=not modguard_public_is_imported)
             )
             return
 

tests/example/__init__.py

@@ -1,8 +1,8 @@
 from modguard import Boundary
 
 # intentional import violations
-from .domain_one.interface import domain_one_interface
-from .domain_three.api import public_for_domain_two
+from .domain_one.interface import domain_one_interface, domain_one_var
+from .domain_three.api import PublicForDomainTwo
 from .domain_four.subsystem import private_subsystem_call
 
 # OK import
@@ -11,4 +11,14 @@
 # modguard-ignore
 from .domain_two.other import internal_api
 
+
 Boundary()
+
+
+# Usages
+domain_one_interface()
+example = domain_one_var
+PublicForDomainTwo()
+private_subsystem_call()
+example.domain_four
+internal_api()

tests/example/domain_four/subsystem.py

@@ -1,7 +1,8 @@
-from modguard import Boundary
+import modguard
+import math
 
-Boundary()
+modguard.Boundary()
 
 
 def private_subsystem_call():
-    ...
+    print(math.random())

tests/example/domain_one/__init__.py

@@ -1,4 +1,8 @@
 from modguard import Boundary
-from .interface import domain_one_interface
+from .interface import domain_one_interface, domain_one_var
 
 Boundary()
+
+# Usages
+domain_one_interface()
+local_var = domain_one_var

tests/example/domain_one/interface.py

@@ -4,3 +4,8 @@
 @public(allowlist=["example.domain_two"])
 def domain_one_interface():
     ...
+
+
+domain_one_var = "hello domain two"
+
+public(domain_one_var, allowlist=["example.domain_two"])

tests/example/domain_three/__init__.py

@@ -1,5 +1,11 @@
 from modguard import Boundary
 from .api import public_for_domain_two
-from ..domain_one.interface import domain_one_interface
+from ..domain_one.interface import domain_one_interface, domain_one_var
 
 Boundary()
+
+
+# Usages
+public_for_domain_two()
+domain_one_interface()
+local_var = domain_one_var

tests/example/domain_three/api.py

@@ -3,5 +3,5 @@
 public(allowlist=["example.domain_two"])
 
 
-def public_for_domain_two():
+class PublicForDomainTwo:
     ...

tests/example/domain_two/__init__.py

@@ -1,3 +1,8 @@
 import example.domain_three.api
-from ..domain_one.interface import domain_one_interface
+from ..domain_one.interface import domain_one_interface, domain_one_var
 from ..domain_three import api
+
+api.PublicForDomainTwo()
+example.domain_three.api.PublicForDomainTwo()
+domain_one_interface()
+domain_two_var = domain_one_var

tests/test_check.py

@@ -79,7 +79,7 @@ def test_check_example_dir_end_to_end():
             boundary_path="example.domain_one",
         ),
         ErrorInfo(
-            import_mod_path="example.domain_three.api.public_for_domain_two",
+            import_mod_path="example.domain_three.api.PublicForDomainTwo",
             location="example/__init__.py",
             boundary_path="example.domain_three",
         ),
@@ -93,6 +93,16 @@ def test_check_example_dir_end_to_end():
             location="example/__init__.py",
             boundary_path="example.domain_four.subsystem",
         ),
+        ErrorInfo(
+            location="example/__init__.py",
+            import_mod_path="example.domain_one.interface.domain_one_var",
+            boundary_path="example.domain_one",
+        ),
+        ErrorInfo(
+            location="example/domain_three/__init__.py",
+            import_mod_path="example.domain_one.interface.domain_one_var",
+            boundary_path="example.domain_one",
+        ),
     ]
     check_results = check("example")
 

tests/test_cli.py

@@ -1,5 +1,3 @@
-import io
-import subprocess
 from unittest.mock import Mock
 import pytest
 

tests/test_init.py

@@ -18,6 +18,7 @@ def init_project_from_root(root) -> None:
         # Change back to the original directory
         os.chdir(saved_directory)
 
+
 @pytest.fixture(scope="module")
 def test_root():
     # Create a temporary directory to use as the root for testing
@@ -35,6 +36,7 @@ def test_init_project_with_valid_root(test_root):
         "package3",
         "package4/subpackage",
         "package5/subpackage",
+        "package6/subpackage"
     ]
     for d in test_dirs:
         os.makedirs(os.path.join(test_root, d))
@@ -43,20 +45,22 @@ def test_init_project_with_valid_root(test_root):
 
     # Create some mock Python files with imports and member names
     file_contents = {
-        "package1/__init__.py": "from package4.subpackage import SubPackageClass\n",
-        "package2/__init__.py": "from package5.subpackage import SubPackageClass\n",
-        "package3/__init__.py": "from package1.module1 import Package1Class\nfrom package2.module2 import Package2Class\n",
-        "package4/subpackage/__init__.py": "",
-        "package5/subpackage/__init__.py": "",
+        "package1/__init__.py": "from package6.subpackage.module6 import x",
+        "package2/__init__.py": "",
         "package1/module1.py": "class Package1Class:\n    pass\n",
-        "package2/module2.py": "class Package2Class:\n    pass\n",
+        "package2/module2.py": "def package_2_func():\n    pass\n",
+        "package3/__init__.py": "from package1.module1 import Package1Class\nfrom package2.module2 import package_2_func\n",
+        "package3/module3.py": "",
+        "package4/subpackage/__init__.py": "",
+        "package5/subpackage/__init__.py": "import package3.module3",
+        "package6/subpackage/__init__.py": "",
+        "package6/subpackage/module6.py": "x = 3\n",
     }
 
     for file_path, content in file_contents.items():
         with open(os.path.join(test_root, file_path), "w") as f:
             f.write(content)
 
-
     # Call init_project with the test root
     init_project_from_root(test_root)
 
@@ -68,8 +72,16 @@ def test_init_project_with_valid_root(test_root):
 
     # Check if public members have been marked as expected
     expected_public_files = [
-        ("package1/module1.py", 'import modguard\n@modguard.public\nclass Package1Class:\n    pass\n'),
-        ("package2/module2.py", 'import modguard\n@modguard.public\nclass Package2Class:\n    pass\n'),
+        (
+            "package1/module1.py",
+            "import modguard\n@modguard.public\nclass Package1Class:\n    pass\n",
+        ),
+        (
+            "package2/module2.py",
+            "import modguard\n@modguard.public\ndef package_2_func():\n    pass\n",
+        ),
+        ("package6/subpackage/module6.py","import modguard\nx = 3\nmodguard.public(x)\n",),
+        ('package3/module3.py', 'import modguard\nmodguard.public()\n')
     ]
     for file_path, expected_state in expected_public_files:
         with open(os.path.join(test_root, file_path)) as f:

tests/test_parsing.py

@@ -19,11 +19,13 @@ def test_get_imports():
     assert set(get_imports("example/domain_one/__init__.py")) == {
         "modguard.Boundary",
         "example.domain_one.interface.domain_one_interface",
+        "example.domain_one.interface.domain_one_var",
     }
     assert set(get_imports("example/__init__.py")) == {
         "modguard.Boundary",
         "example.domain_one.interface.domain_one_interface",
-        "example.domain_three.api.public_for_domain_two",
+        "example.domain_three.api.PublicForDomainTwo",
         "example.domain_four",
         "example.domain_four.subsystem.private_subsystem_call",
+        "example.domain_one.interface.domain_one_var",
     }