diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 76c7d680..7c8470b4 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/modguard/parsing/public.py b/modguard/parsing/public.py index 53ab20b0..f5d86854 100644 --- a/modguard/parsing/public.py +++ b/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 diff --git a/tests/example/__init__.py b/tests/example/__init__.py index 32333adc..9cea5708 100644 --- a/tests/example/__init__.py +++ b/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_usage = domain_one_var +PublicForDomainTwo() +private_subsystem_call() +example.domain_four +internal_api() diff --git a/tests/example/domain_four/subsystem.py b/tests/example/domain_four/subsystem.py index e3832dc6..805c1ca8 100644 --- a/tests/example/domain_four/subsystem.py +++ b/tests/example/domain_four/subsystem.py @@ -1,6 +1,6 @@ -from modguard import Boundary +import modguard -Boundary() +modguard.Boundary() def private_subsystem_call(): diff --git a/tests/example/domain_one/__init__.py b/tests/example/domain_one/__init__.py index dea55c34..de94670a 100644 --- a/tests/example/domain_one/__init__.py +++ b/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 diff --git a/tests/example/domain_one/interface.py b/tests/example/domain_one/interface.py index 5a793010..7ae93a44 100644 --- a/tests/example/domain_one/interface.py +++ b/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"]) diff --git a/tests/example/domain_three/__init__.py b/tests/example/domain_three/__init__.py index 5a34dcd6..039022cd 100644 --- a/tests/example/domain_three/__init__.py +++ b/tests/example/domain_three/__init__.py @@ -1,5 +1,9 @@ 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 +domain_one_interface() +local_var = domain_one_var diff --git a/tests/example/domain_three/api.py b/tests/example/domain_three/api.py index 360146f1..cc0a9ef0 100644 --- a/tests/example/domain_three/api.py +++ b/tests/example/domain_three/api.py @@ -3,5 +3,5 @@ public(allowlist=["example.domain_two"]) -def public_for_domain_two(): +class PublicForDomainTwo: ... diff --git a/tests/example/domain_two/__init__.py b/tests/example/domain_two/__init__.py index 8a55c428..9fd26566 100644 --- a/tests/example/domain_two/__init__.py +++ b/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 diff --git a/tests/test_check.py b/tests/test_check.py index b4b6a221..05a67e33 100644 --- a/tests/test_check.py +++ b/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") diff --git a/tests/test_cli.py b/tests/test_cli.py index 45f3fba2..4996589c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,5 +1,3 @@ -import io -import subprocess from unittest.mock import Mock import pytest diff --git a/tests/test_init.py b/tests/test_init.py index 692a25cc..27b65642 100644 --- a/tests/test_init.py +++ b/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: diff --git a/tests/test_parsing.py b/tests/test_parsing.py index bababbe9..64bdc79e 100644 --- a/tests/test_parsing.py +++ b/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", }