Merge pull request #48 from seddonym/forbidden-imports

Add ForbiddenContract

.gitignore

@@ -68,3 +68,5 @@ docs/_build
 .mypy_cache/
 
 .python-version
+
+pip-wheel-metadata

CHANGELOG.rst

@@ -44,3 +44,4 @@ latest
 
 * Add https://pre-commit.com configuration.
 * Use find_shortest_chains instead of find_shortest_chain on the Grimp import graph.
+* Add Forbidden Modules contract type.

README.rst

@@ -33,9 +33,9 @@ architecture within your Python project. It does this by analysing the imports b
 package, and compares this against a set of rules that you provide in a configuration file.
 
 The configuration file contains one or more 'contracts'. Each contract has a specific
-type, which determines the sort of rules it will apply. For example, the ``independence``
-contract type checks that there are no imports, in either direction, between a set
-of subpackages.
+type, which determines the sort of rules it will apply. For example, the ``forbidden``
+contract type allows you to check that certain modules or packages are not imported by
+parts of your project.
 
 Import Linter is particularly useful if you are working on a complex codebase within a team,
 when you want to enforce a particular architectural style. In this case you can add
@@ -53,22 +53,24 @@ Install Import Linter::
     pip install import-linter
 
 Decide on the dependency flows you wish to check. In this example, we have
-decided to make sure that there are no dependencies between ``myproject.foo``
-and ``myproject.bar``, so we will use the ``independence`` contract type.
+decided to make sure that ``myproject.foo`` has dependencies on neither
+``myproject.bar`` nor ``myproject.baz``, so we will use the ``forbidden`` contract type.
 
-Create an ``.importlinter`` file in the root of your project. For example:
+Create an ``.importlinter`` file in the root of your project to define your contract(s). In this case:
 
 .. code-block:: ini
 
     [importlinter]
     root_package = myproject
 
     [importlinter:contract:1]
-    name=Foo and bar are decoupled
-    type=independence
-    modules=
+    name=Foo doesn't import bar or baz
+    type=forbidden
+    source_modules=
         myproject.foo
+    forbidden_modules=
         myproject.bar
+        myproject.baz
 
 Now, from your project root, run::
 
@@ -89,7 +91,7 @@ If your code violates the contract, you will see an error message something like
     Analyzed 23 files, 44 dependencies.
     -----------------------------------
 
-    Foo and bar are decoupled BROKEN
+    Foo doesn't import bar or baz BROKEN
 
     Contracts: 1 broken.
 
@@ -98,8 +100,8 @@ If your code violates the contract, you will see an error message something like
     Broken contracts
     ----------------
 
-    Foo and bar are decoupled
-    -------------------------
+    Foo doesn't import bar or baz
+    -----------------------------
 
     myproject.foo is not allowed to import myproject.bar:
 

docs/contract_types.rst

@@ -2,6 +2,71 @@
 Contract types
 ==============
 
+Forbidden modules
+-----------------
+
+*Type name:* ``forbidden``
+
+Forbidden contracts check that one set of modules are not imported by another set of modules.
+
+Descendants of each module will be checked - so if ``mypackage.one`` is forbidden from importing ``mypackage.two``, then
+``mypackage.one.blue`` will be forbidden from importing ``mypackage.two.green``. Indirect imports will also be checked.
+
+External packages may also be forbidden.
+
+**Examples:**
+
+.. code-block:: ini
+
+    [importlinter]
+    root_package = mypackage
+
+    [importlinter:contract:1]
+    name = My forbidden contract (internal packages only)
+    type = forbidden
+    source_modules =
+        mypackage.one
+        mypackage.two
+        mypackage.three.blue
+    forbidden_modules =
+        mypackage.four
+        mypackage.five.green
+    ignore_imports =
+        mypackage.one.green -> mypackage.utils
+        mypackage.two -> mypackage.four
+
+.. code-block:: ini
+
+    [importlinter]
+    root_package = mypackage
+    include_external_packages = True
+
+    [importlinter:contract:1]
+    name = My forbidden contract (internal and external packages)
+    type = forbidden
+    source_modules =
+        mypackage.one
+        mypackage.two
+    forbidden_modules =
+        mypackage.three
+        django
+        requests
+    ignore_imports =
+        mypackage.one.green -> sqlalchemy
+
+**Configuration options**
+
+Configuration options:
+
+    - ``source_modules``:    A list of modules that should not import the forbidden modules.
+    - ``forbidden_modules``: A list of modules that should not be imported by the source modules. These may include
+      root level external packages (i.e. ``django``, but not ``django.db.models``). If external packages are included,
+      the top level configuration must have ``internal_external_packages = True``.
+    - ``ignore_imports``:
+      A list of imports, each in the form ``mypackage.foo.importer -> mypackage.bar.imported``. These imports
+      will be ignored: if the import would cause a contract to be broken, adding it to the list will cause the
+      contract be kept instead. (Optional.)
+
 Independence
 ------------
 

docs/usage.rst

@@ -33,9 +33,7 @@ Your file must contain an ``importlinter`` section providing top-level (i.e. non
   means it is has been installed using pip, or it's in the current directory. (Required.)
 - ``include_external_packages``:
   Whether to include external packages when building the import graph (see `the Grimp build_graph documentation`_ for
-  more details). This is not currently used by any built in contracts, but it could be used by a custom contract type
-  that wanted to enforce rules relating to packages not in the root package (i.e. in the Python standard library, or in
-  third party libraries). (Optional.)
+  more details). Not every contract type uses this. (Optional.)
 
 .. _the Grimp build_graph documentation: https://grimp.readthedocs.io/en/latest/usage.html#grimp.build_graph
 

setup.cfg

@@ -1,5 +1,5 @@
 [flake8]
-ignore = E731, E203
+ignore = E731, E203, W503
 max-line-length = 100
 exclude = */migrations/*, tests/assets/*
 

src/importlinter/application/use_cases.py

@@ -107,6 +107,7 @@ def _get_built_in_contract_types() -> List[Tuple[str, Type[Contract]]]:
         map(
             _parse_contract_type_string,
             [
+                "forbidden: importlinter.contracts.forbidden.ForbiddenContract",
                 "layers: importlinter.contracts.layers.LayersContract",
                 "independence: importlinter.contracts.independence.IndependenceContract",
             ],

src/importlinter/contracts/forbidden.py

@@ -0,0 +1,127 @@
+from importlinter.application import output
+from importlinter.domain import fields, helpers
+from importlinter.domain.contract import Contract, ContractCheck
+from importlinter.domain.imports import Module
+from importlinter.domain.ports.graph import ImportGraph
+
+
+class ForbiddenContract(Contract):
+    """
+    Forbidden contracts check that one set of modules are not imported by another set of modules.
+
+    Indirect imports will also be checked.
+
+    Configuration options:
+
+        - source_modules:    A list of Modules that should not import the forbidden modules.
+        - forbidden_modules: A list of Modules that should not be imported by the source modules.
+        - ignore_imports:    A list of DirectImports. These imports will be ignored: if the import
+                             would cause a contract to be broken, adding it to the list will cause
+                             the contract be kept instead. (Optional.)
+    """
+
+    type_name = "forbidden"
+
+    source_modules = fields.ListField(subfield=fields.ModuleField())
+    forbidden_modules = fields.ListField(subfield=fields.ModuleField())
+    ignore_imports = fields.ListField(subfield=fields.DirectImportField(), required=False)
+
+    def check(self, graph: ImportGraph) -> ContractCheck:
+        is_kept = True
+        invalid_chains = []
+
+        removed_imports = helpers.pop_imports(  # type: ignore
+            graph, self.ignore_imports if self.ignore_imports else []
+        )
+
+        self._check_all_modules_exist_in_graph(graph)
+        self._check_external_forbidden_modules(graph)
+
+        # We only need to check for illegal imports for forbidden modules that are in the graph.
+        forbidden_modules_in_graph = [
+            m for m in self.forbidden_modules if m.name in graph.modules  # type: ignore
+        ]
+
+        for source_module in self.source_modules:  # type: ignore
+            for forbidden_module in forbidden_modules_in_graph:
+                subpackage_chain_data = {
+                    "upstream_module": forbidden_module.name,
+                    "downstream_module": source_module.name,
+                    "chains": [],
+                }
+
+                chains = graph.find_shortest_chains(
+                    importer=source_module.name, imported=forbidden_module.name
+                )
+                if chains:
+                    is_kept = False
+                    for chain in chains:
+                        chain_data = []
+                        for importer, imported in [
+                            (chain[i], chain[i + 1]) for i in range(len(chain) - 1)
+                        ]:
+                            import_details = graph.get_import_details(
+                                importer=importer, imported=imported
+                            )
+                            line_numbers = tuple(j["line_number"] for j in import_details)
+                            chain_data.append(
+                                {
+                                    "importer": importer,
+                                    "imported": imported,
+                                    "line_numbers": line_numbers,
+                                }
+                            )
+                        subpackage_chain_data["chains"].append(chain_data)
+                if subpackage_chain_data["chains"]:
+                    invalid_chains.append(subpackage_chain_data)
+
+        helpers.add_imports(graph, removed_imports)
+
+        return ContractCheck(kept=is_kept, metadata={"invalid_chains": invalid_chains})
+
+    def render_broken_contract(self, check: "ContractCheck") -> None:
+        count = 0
+        for chains_data in check.metadata["invalid_chains"]:
+            downstream, upstream = chains_data["downstream_module"], chains_data["upstream_module"]
+            output.print_error(f"{downstream} is not allowed to import {upstream}:")
+            output.new_line()
+            count += len(chains_data["chains"])
+            for chain in chains_data["chains"]:
+                first_line = True
+                for direct_import in chain:
+                    importer, imported = direct_import["importer"], direct_import["imported"]
+                    line_numbers = ", ".join(f"l.{n}" for n in direct_import["line_numbers"])
+                    import_string = f"{importer} -> {imported} ({line_numbers})"
+                    if first_line:
+                        output.print_error(f"-   {import_string}", bold=False)
+                        first_line = False
+                    else:
+                        output.indent_cursor()
+                        output.print_error(import_string, bold=False)
+                output.new_line()
+
+            output.new_line()
+
+    def _check_all_modules_exist_in_graph(self, graph: ImportGraph) -> None:
+        for module in self.source_modules:  # type: ignore
+            if module.name not in graph.modules:
+                raise ValueError(f"Module '{module.name}' does not exist.")
+
+    def _check_external_forbidden_modules(self, graph: ImportGraph) -> None:
+        if (
+            self._contains_external_forbidden_modules(graph)
+            and not self._graph_was_built_with_externals()
+        ):
+            raise ValueError(
+                "The top level configuration must have include_external_packages=True "
+                "when there are external forbidden modules."
+            )
+
+    def _contains_external_forbidden_modules(self, graph: ImportGraph) -> bool:
+        root_package = Module(self.session_options["root_package"])
+        return not all(
+            m.is_descendant_of(root_package) for m in self.forbidden_modules  # type: ignore
+        )
+
+    def _graph_was_built_with_externals(self) -> bool:
+        return self.session_options.get("include_external_packages") in ("True", "true")

tests/assets/testpackage/.customkeptcontract.ini

@@ -0,0 +1,11 @@
+[importlinter]
+root_package = testpackage
+contract_types =
+    forbidden_import: tests.helpers.contracts.ForbiddenImportContract
+
+
+[importlinter:contract:one]
+name=Custom kept contract
+type=forbidden_import
+importer=testpackage.utils
+imported=testpackage.low

tests/assets/testpackage/.externalbrokencontract.ini

@@ -1,12 +1,10 @@
 [importlinter]
 root_package = testpackage
 include_external_packages = True
-contract_types =
-    forbidden: tests.helpers.contracts.ForbiddenImportContract
 
 
 [importlinter:contract:one]
-name=External broken contract
+name=External kept contract
 type=forbidden
-importer=testpackage.utils
-imported=pytest
+source_modules=testpackage.high.blue
+forbidden_modules=pytest

tests/assets/testpackage/.externalkeptcontract.ini

@@ -1,12 +1,10 @@
 [importlinter]
 root_package = testpackage
 include_external_packages = True
-contract_types =
-    forbidden: tests.helpers.contracts.ForbiddenImportContract
 
 
 [importlinter:contract:one]
 name=External kept contract
 type=forbidden
-importer=testpackage.high.blue
-imported=pytest
+source_modules=testpackage.high.blue
+forbidden_modules=sqlalchemy

tests/functional/test_lint_imports.py

@@ -13,6 +13,7 @@
         (None, cli.EXIT_STATUS_SUCCESS),
         (".brokencontract.ini", cli.EXIT_STATUS_ERROR),
         (".malformedcontract.ini", cli.EXIT_STATUS_ERROR),
+        (".customkeptcontract.ini", cli.EXIT_STATUS_SUCCESS),
         (".externalkeptcontract.ini", cli.EXIT_STATUS_SUCCESS),
         (".externalbrokencontract.ini", cli.EXIT_STATUS_ERROR),
     ),