diff --git a/docs/usage.rst b/docs/usage.rst index 9ae57819..511ba680 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -250,15 +250,18 @@ Higher level analysis Find dependencies that don't conform to the supplied layered architecture. - 'Layers' is a software architecture pattern in which a list of sibling modules/packages have a dependency direction + 'Layers' is a software architecture pattern in which a list of modules/packages have a dependency direction from high to low. In other words, a higher layer would be allowed to import a lower layer, but not the other way around. - Additionally, multiple layers can be grouped together at the same level; for example ``mypackage.utils`` and - ``mypackage.logging`` might sit at the bottom, so they cannot import from any other layers. When a simple set - of sibling layers is passed then they must be independent, so any dependencies in either direction will be - treated as illegal. To allow imports between siblings in a layer then an explicit ``Level`` can be passed instead, - and ``independent`` set to ``False``. + Additionally, multiple modules can be grouped together at the same layer; + for example ``mypackage.utils`` and ``mypackage.logging`` might sit at the bottom, so they + cannot import from any other layers. To specify that multiple modules should + be treated as siblings within a single layer, pass a :class:`.Layer`. The ``Layer.independent`` + field can be used to specify whether the sibling modules should be treated as independent + - should imports between sibling modules be forbidden (default) or allowed? For backwards + compatibility it is also possible to pass a simple ``set[str]`` to describe a layer. In this + case the sibling modules within the layer will be considered independent. Note: each returned :class:`.PackageDependency` does not include all possible illegal :class:`.Route` objects. Instead, once an illegal :class:`.Route` is found, the algorithm will temporarily remove it from the graph before continuing @@ -279,25 +282,24 @@ Higher level analysis ), ) - Example with independent sibling layers:: + Example with independent sibling modules:: dependencies = graph.find_illegal_dependencies_for_layers( layers=( "red", # Imports between green and blue are forbidden. - # Equivalent to grimp.Level("green", "blue", independent=True) - {"green", "blue"}, + grimp.Layer("green", "blue"), "yellow", ), ) - Example with sibling layers that allow imports between the siblings:: + Example with sibling modules that allow imports between the sibling modules:: dependencies = graph.find_illegal_dependencies_for_layers( layers=( "red", # Imports between green and blue are allowed. - grimp.Level("green", "blue", independent=False), + grimp.Layer("green", "blue", independent=False), "yellow", ), ) @@ -316,8 +318,8 @@ Higher level analysis }, ) - :param Sequence[Level | str | set[str]] layers: A sequence, each element of which consists either of the name of a layer - module, a set of sibling layers, or a :class:`.Level`. If ``containers`` are also specified, + :param Sequence[Layer | str | set[str]] layers: A sequence, each element of which consists either of a :class:`.Layer` + the name of a layer module or a set of sibling layers. If ``containers`` are also specified, then these names must be relative to the container. The order is from higher to lower level layers. *Any layers that don't exist in the graph will be ignored.* :param set[str] containers: The parent modules of the layers, as absolute names that you could @@ -330,17 +332,19 @@ Higher level analysis :raises grimp.exceptions.NoSuchContainer: if a container is not a module in the graph. -.. class:: Level +.. class:: Layer - A level within a layered architecture. + A layer within a layered architecture. - .. attribute:: layers + .. attribute:: module_tails - ``set[str]``: The sibling layers within this level. + ``set[str]``: The tails of the names of the sibling modules within this layer. + When ``containers`` are used then these names must be relative to + the container, hence the naming "tails". .. attribute:: independent - ``bool``: Whether the sibling layers within this level are required to be independent. + ``bool``: Whether the sibling modules within this layer are required to be independent. .. class:: PackageDependency diff --git a/src/grimp/__init__.py b/src/grimp/__init__.py index 4f4fe47e..76b308e8 100644 --- a/src/grimp/__init__.py +++ b/src/grimp/__init__.py @@ -2,7 +2,7 @@ from .application.ports.graph import DetailedImport, ImportGraph from .domain.analysis import PackageDependency, Route -from .domain.valueobjects import DirectImport, Module, Level +from .domain.valueobjects import DirectImport, Module, Layer from .main import build_graph __all__ = [ @@ -13,5 +13,5 @@ "PackageDependency", "Route", "build_graph", - "Level", + "Layer", ] diff --git a/src/grimp/adaptors/_layers.py b/src/grimp/adaptors/_layers.py index f7e017ab..09e2050c 100644 --- a/src/grimp/adaptors/_layers.py +++ b/src/grimp/adaptors/_layers.py @@ -11,27 +11,27 @@ from grimp.domain.analysis import PackageDependency from grimp.exceptions import NoSuchContainer -from grimp.domain.valueobjects import Level +from grimp.domain.valueobjects import Layer -def layers_to_levels(layers: Sequence[Level | str | set[str]]) -> tuple[Level, ...]: +def parse_layers(layers: Sequence[Layer | str | set[str]]) -> tuple[Layer, ...]: """ - Convert the levels within the passed `layers` into `Level`s. + Convert the passed raw `layers` into `Layer`s. """ out_layers = [] for layer in layers: - if isinstance(layer, Level): + if isinstance(layer, Layer): out_layers.append(layer) elif isinstance(layer, str): - out_layers.append(Level(layer, independent=True)) + out_layers.append(Layer(layer, independent=True)) else: - out_layers.append(Level(*tuple(layer), independent=True)) + out_layers.append(Layer(*tuple(layer), independent=True)) return tuple(out_layers) def find_illegal_dependencies( graph: ImportGraph, - levels: Sequence[Level], + layers: Sequence[Layer], containers: set[str], ) -> set[PackageDependency]: """ @@ -45,7 +45,8 @@ def find_illegal_dependencies( try: rust_package_dependency_tuple = rust.find_illegal_dependencies( levels=tuple( - {"layers": level.layers, "independent": level.independent} for level in levels + {"layers": layer.module_tails, "independent": layer.independent} + for layer in layers ), containers=set(containers), importeds_by_importer=graph._importeds_by_importer, diff --git a/src/grimp/adaptors/graph.py b/src/grimp/adaptors/graph.py index 527e3da9..cab4ce95 100644 --- a/src/grimp/adaptors/graph.py +++ b/src/grimp/adaptors/graph.py @@ -6,7 +6,7 @@ from grimp.algorithms.shortest_path import bidirectional_shortest_path from grimp.application.ports import graph from grimp.domain.analysis import PackageDependency -from grimp.domain.valueobjects import Module, Level +from grimp.domain.valueobjects import Module, Layer from grimp.exceptions import ModuleNotPresent from . import _layers @@ -365,12 +365,12 @@ def chain_exists(self, importer: str, imported: str, as_packages: bool = False) def find_illegal_dependencies_for_layers( self, - layers: Sequence[Level | str | set[str]], + layers: Sequence[Layer | str | set[str]], containers: set[str] | None = None, ) -> set[PackageDependency]: - levels = _layers.layers_to_levels(layers) + layers = _layers.parse_layers(layers) return _layers.find_illegal_dependencies( - graph=self, levels=levels, containers=containers or set() + graph=self, layers=layers, containers=containers or set() ) # Private methods diff --git a/src/grimp/application/ports/graph.py b/src/grimp/application/ports/graph.py index 4d7768f6..b95af2dd 100644 --- a/src/grimp/application/ports/graph.py +++ b/src/grimp/application/ports/graph.py @@ -6,7 +6,7 @@ from typing_extensions import TypedDict from grimp.domain.analysis import PackageDependency -from grimp.domain.valueobjects import Level +from grimp.domain.valueobjects import Layer class DetailedImport(TypedDict): @@ -272,27 +272,29 @@ def chain_exists(self, importer: str, imported: str, as_packages: bool = False) def find_illegal_dependencies_for_layers( self, - layers: Sequence[Level | str | set[str]], + layers: Sequence[Layer | str | set[str]], containers: set[str] | None = None, ) -> set[PackageDependency]: """ Find dependencies that don't conform to the supplied layered architecture. - 'Layers' is an architectural pattern in which a list of sibling modules/packages + 'Layers' is an architectural pattern in which a list of modules/packages have a dependency direction from high to low. In other words, a higher layer would be allowed to import a lower layer, but not the other way around. - Additionally, multiple layers can be grouped together at the same level; for example - `mypackage.utils` and `mypackage.logging` might sit at the bottom, so they cannot - import from any other layers. When a simple set of sibling layers is passed then they - must be independent, so any dependencies in either direction will be treated as illegal. - To allow imports between siblings in a layer then an explicit `Level` can be passed instead, - and `independent` set to `False`. + Additionally, multiple modules can be grouped together at the same layer; + for example `mypackage.utils` and `mypackage.logging` might sit at the bottom, so they + cannot import from any other layers. To specify that multiple modules should + be treated as siblings within a single layer, pass a `Layer`. The `Layer.independent` + field can be used to specify whether the sibling modules should be treated as independent + - should imports between sibling modules be forbidden (default) or allowed? For backwards + compatibility it is also possible to pass a simple `set[str]` to describe a layer. In this + case the sibling modules within the layer will be considered independent. Arguments: - - layers: A sequence, each element of which consists either of the name of a layer - module, a set of sibling layers, or a `Level`. If containers + - layers: A sequence, each element of which consists either of a `Layer`, the name + of a layer module or a set of sibling modules. If containers are also specified, then these names must be relative to the container. The order is from higher to lower level layers. Any layers that don't exist in the graph will be ignored. diff --git a/src/grimp/domain/valueobjects.py b/src/grimp/domain/valueobjects.py index 9c6810ca..a31468a2 100644 --- a/src/grimp/domain/valueobjects.py +++ b/src/grimp/domain/valueobjects.py @@ -86,21 +86,21 @@ def __hash__(self) -> int: return hash((str(self), self.line_contents)) -class Level(ValueObject): +class Layer(ValueObject): """ - A level within a layered architecture. + A layer within a layered architecture. - If level.independent is True then the layers within the level are considered + If layer.independent is True then the modules within the layer are considered independent. This is the default. """ def __init__( self, - *layers: str, + *module_tails: str, independent: bool = True, ) -> None: - self.layers = set(layers) + self.module_tails = set(module_tails) self.independent = independent def __str__(self) -> str: - return f"{self.layers}, independent={self.independent}" + return f"{self.module_tails}, independent={self.independent}" diff --git a/tests/unit/adaptors/graph/test_layers.py b/tests/unit/adaptors/graph/test_layers.py index 0abc8077..e85d2b25 100644 --- a/tests/unit/adaptors/graph/test_layers.py +++ b/tests/unit/adaptors/graph/test_layers.py @@ -9,7 +9,7 @@ from grimp import PackageDependency, Route from grimp.adaptors.graph import ImportGraph from grimp.exceptions import NoSuchContainer -from grimp.domain.valueobjects import Level +from grimp.domain.valueobjects import Layer class TestSingleOrNoContainer: @@ -398,8 +398,8 @@ def test_no_illegal_imports(self, specify_container: bool): @pytest.mark.parametrize( "sequence_type, expect_independent", ( - (lambda layers: Level(*layers), True), - (lambda layers: Level(*layers, independent=False), False), + (lambda layers: Layer(*layers), True), + (lambda layers: Layer(*layers, independent=False), False), (set, True), (frozenset, True), (tuple, True),