diff --git a/docs/contract_types.rst b/docs/contract_types.rst index 23c48c55..8e4c098c 100644 --- a/docs/contract_types.rst +++ b/docs/contract_types.rst @@ -318,6 +318,27 @@ Note: you are not allowed to mix different kinds of separators on the same line. mypackage.blue | mypackage.green : mypackage.yellow # Invalid as it mixes separators. mypackage.low +Modular modules +--------------- + +*Type name:* ``modular`` + +Modular contracts check that there are no cycles between the child packages of the package. + +**Example:** + +.. code-block:: ini + + [importlinter:contract:my-modular-contract] + name = My modular contract + type = modular + modules = + mypackage.foo + +**Configuration options** + + - ``modules``: A list of modules/subpackages that should be modular. + Custom contract types --------------------- diff --git a/src/importlinter/application/use_cases.py b/src/importlinter/application/use_cases.py index ed4db07c..adbd31d3 100644 --- a/src/importlinter/application/use_cases.py +++ b/src/importlinter/application/use_cases.py @@ -248,6 +248,7 @@ def _get_built_in_contract_types() -> List[Tuple[str, Type[Contract]]]: "forbidden: importlinter.contracts.forbidden.ForbiddenContract", "layers: importlinter.contracts.layers.LayersContract", "independence: importlinter.contracts.independence.IndependenceContract", + "modular: importlinter.contracts.modular.ModularContract", ], ) ) diff --git a/src/importlinter/contracts/modular.py b/src/importlinter/contracts/modular.py new file mode 100644 index 00000000..d2780449 --- /dev/null +++ b/src/importlinter/contracts/modular.py @@ -0,0 +1,57 @@ +from __future__ import annotations + +from grimp import ImportGraph + +from importlinter.application import output +from importlinter.domain import fields +from importlinter.domain.contract import Contract, ContractCheck +from importlinter.domain.helpers import _to_pattern +from importlinter.domain.imports import Module + + +# TODO: import from helpers once https://github.com/seddonym/import-linter/pull/220 is merged +def _resolve_wildcards(value: str, graph: ImportGraph) -> set[Module]: + pattern = _to_pattern(value) + return {Module(module) for module in graph.modules if pattern.match(module)} + + +class ModularContract(Contract): + """ + Modular contracts check that one set of modules has no children with circular dependencies. + Indirect imports will also be checked. + Configuration options: + - modules: A list of Modules that should be modular. + """ + + type_name = "modular" + + modules = fields.ListField(subfield=fields.ModuleField()) + + def check(self, graph: ImportGraph, verbose: bool) -> ContractCheck: + violations = {} + for module in self.modules: # type: ignore + direct_submodules = _resolve_wildcards(f"{module.name}.*", graph) + dependencies = graph.find_illegal_dependencies_for_layers( + layers=({y.name for y in direct_submodules},), + ) + violations[module.name] = sorted( + { + f"{dependency.imported} <-> {dependency.importer}" + for dependency in dependencies + if graph.find_shortest_chains(dependency.imported, dependency.importer) + } + ) + + kept = all(len(violation) == 0 for violation in violations.values()) + return ContractCheck(kept=kept, warnings=None, metadata={"violations": violations}) + + def render_broken_contract(self, check: "ContractCheck") -> None: + for module_name, violations in check.metadata["violations"].items(): + output.print( + f"child modules of {module_name} must be modular and thus circular dependencies " + "are not allowed:" + ) + output.new_line() + for violation in violations: + output.print_error(f"- {violation}") + output.new_line() diff --git a/tests/unit/contracts/test_modular.py b/tests/unit/contracts/test_modular.py new file mode 100644 index 00000000..b0d8e630 --- /dev/null +++ b/tests/unit/contracts/test_modular.py @@ -0,0 +1,172 @@ +from __future__ import annotations + +import pytest +from grimp.adaptors.graph import ImportGraph + +from importlinter.application.app_config import settings +from importlinter.contracts.modular import ModularContract +from importlinter.domain.contract import ContractCheck +from tests.adapters.printing import FakePrinter +from tests.adapters.timing import FakeTimer + + +@pytest.fixture(scope="module", autouse=True) +def configure(): + settings.configure(TIMER=FakeTimer()) + + +class TestModularContract: + def _build_default_graph(self): + graph = ImportGraph() + for module in ( + "mypackage", + "mypackage.blue", + "mypackage.blue.alpha", + "mypackage.blue.beta", + "mypackage.blue.beta.foo", + "mypackage.green", + "mypackage.yellow", + "mypackage.yellow.gamma", + "mypackage.yellow.delta", + "mypackage.other", + ): + graph.add_module(module) + return graph + + def _check_default_contract(self, graph): + contract = ModularContract( + name="Modular contract", + session_options={"root_packages": ["mypackage"]}, + contract_options={"modules": ("mypackage",)}, + ) + return contract.check(graph=graph, verbose=False) + + def test_when_modules_are_modular(self): + graph = self._build_default_graph() + graph.add_import( + importer="mypackage.blue", + imported="mypackage.other", + line_number=10, + line_contents="-", + ) + graph.add_import( + importer="mypackage.other", + imported="mypackage.green", + line_number=11, + line_contents="-", + ) + + contract_check = self._check_default_contract(graph) + + assert contract_check.kept, contract_check.metadata + + def test_non_modular_bidirectional(self): + graph = self._build_default_graph() + graph.add_import( + importer="mypackage.blue", + imported="mypackage.other", + line_number=10, + line_contents="-", + ) + graph.add_import( + importer="mypackage.other", + imported="mypackage.green", + line_number=11, + line_contents="-", + ) + graph.add_import( + importer="mypackage.other", + imported="mypackage.blue", + line_number=10, + line_contents="-", + ) + + contract_check = self._check_default_contract(graph) + + assert not contract_check.kept + + expected_metadata = { + "violations": { + "mypackage": [ + "mypackage.blue <-> mypackage.other", + "mypackage.other <-> mypackage.blue", + ] + } + } + + assert expected_metadata == contract_check.metadata + + def test_non_modular_circular(self): + graph = self._build_default_graph() + graph.add_import( + importer="mypackage.blue", + imported="mypackage.other", + line_number=10, + line_contents="-", + ) + graph.add_import( + importer="mypackage.other", + imported="mypackage.green", + line_number=11, + line_contents="-", + ) + graph.add_import( + importer="mypackage.green", + imported="mypackage.blue", + line_number=10, + line_contents="-", + ) + + contract_check = self._check_default_contract(graph) + + assert not contract_check.kept + + expected_metadata = { + "violations": { + "mypackage": [ + "mypackage.blue <-> mypackage.green", + "mypackage.green <-> mypackage.other", + "mypackage.other <-> mypackage.blue", + ] + } + } + assert expected_metadata == contract_check.metadata + + +def test_render_broken_contract(): + settings.configure(PRINTER=FakePrinter()) + contract = ModularContract( + name="Modular contract", + session_options={"root_packages": ["mypackage"]}, + contract_options={"modules": ["mypackage", "mypackage.green"]}, + ) + check = ContractCheck( + kept=False, + metadata={ + "violations": { + "mypackage": [ + "mypackage.blue.foo <-> mypackage.utils.red", + "mypackage.blue.red <-> mypackage.utils.yellow", + ], + "mypackage.green": [ + "mypackage.green.a.b <-> mypackage.green.b.a", + ], + } + }, + ) + + contract.render_broken_contract(check) + + settings.PRINTER.pop_and_assert( + """ + child modules of mypackage must be modular and thus circular dependencies are not allowed: + + - mypackage.blue.foo <-> mypackage.utils.red + - mypackage.blue.red <-> mypackage.utils.yellow + + child modules of mypackage.green must be modular and thus circular dependencies are not allowed: + + - mypackage.green.a.b <-> mypackage.green.b.a + + """ # noqa + )