Adds an allow indirect import option to ForbiddenContract (#92)

* Adds an allow indirect import option to ForbiddenContract

* Fix typing errors

* Address review comments and one minor flake8 error

* Add unit test

* Address review comments

AUTHORS.rst

@@ -11,4 +11,5 @@ Contributors
 * Anthony Sottile - https://github.com/asottile
 * Łukasz Skarżyński - https://github.com/skarzi
 * Daniel Jurczak - https://github.com/danieljurczak
-* Ben Warren - https://github.com/bwarren2
+* Ben Warren - https://github.com/bwarren
+* Aaron Gokaslan - https://github.com/Skylion007

CHANGELOG.rst

@@ -1,6 +1,8 @@
 Changelog
 =========
 
+* Add allow_indirect_imports to Forbidden Contract type
+
 1.2 (2020-09-23)
 ----------------
 

docs/contract_types.rst

@@ -66,6 +66,7 @@ Configuration options:
       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.)
+    - ``allow_indirect_imports``: If ``True``, allow indirect imports to forbidden modules. (Optional.)
 
 Independence
 ------------

src/importlinter/contracts/forbidden.py

@@ -1,3 +1,5 @@
+from typing import Tuple, cast
+
 from importlinter.application import output
 from importlinter.domain import fields, helpers
 from importlinter.domain.contract import Contract, ContractCheck
@@ -7,23 +9,23 @@
 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 set of DirectImports. These imports will be ignored: if the import
                              would cause a contract to be broken, adding it to the set will cause
                              the contract be kept instead. (Optional.)
+        - allow_indirect_imports:  Whether to allow indirect imports to forbidden modules.
+                            "True" or "true" will be treated as True. (Optional.)```
     """
 
     type_name = "forbidden"
 
     source_modules = fields.ListField(subfield=fields.ModuleField())
     forbidden_modules = fields.ListField(subfield=fields.ModuleField())
     ignore_imports = fields.SetField(subfield=fields.DirectImportField(), required=False)
+    allow_indirect_imports = fields.StringField(required=False)
 
     def check(self, graph: ImportGraph) -> ContractCheck:
         is_kept = True
@@ -49,9 +51,20 @@ def check(self, graph: ImportGraph) -> ContractCheck:
                     "chains": [],
                 }
 
-                chains = graph.find_shortest_chains(
-                    importer=source_module.name, imported=forbidden_module.name
-                )
+                if str(self.allow_indirect_imports).lower() == "true":
+                    chains = {
+                        cast(
+                            Tuple[str, ...],
+                            (str(import_det["importer"]), str(import_det["imported"])),
+                        )
+                        for import_det in graph.get_import_details(
+                            importer=source_module.name, imported=forbidden_module.name
+                        )
+                    }
+                else:
+                    chains = graph.find_shortest_chains(
+                        importer=source_module.name, imported=forbidden_module.name
+                    )
                 if chains:
                     is_kept = False
                     for chain in chains:
@@ -121,4 +134,4 @@ def _contains_external_forbidden_modules(self, graph: ImportGraph) -> bool:
         )
 
     def _graph_was_built_with_externals(self) -> bool:
-        return self.session_options.get("include_external_packages") in ("True", "true")
+        return str(self.session_options.get("include_external_packages")).lower() == "true"

tests/unit/contracts/test_forbidden.py

@@ -138,6 +138,19 @@ def test_ignore_imports_tolerates_duplicates(self):
         )
         assert contract.check(graph=graph)
 
+    @pytest.mark.parametrize(
+        "allow_indirect_imports, contract_is_kept",
+        ((None, False), ("false", False), ("True", True), ("true", True), ("anything", False)),
+    )
+    def test_allow_indirect_imports(self, allow_indirect_imports, contract_is_kept):
+        graph = self._build_graph()
+        contract = self._build_contract(
+            forbidden_modules=("mypackage.purple"),
+            allow_indirect_imports=allow_indirect_imports,
+        )
+        contract_check = contract.check(graph=graph)
+        assert contract_check.kept == contract_is_kept
+
     def _build_graph(self):
         graph = ImportGraph()
         for module in (
@@ -185,7 +198,11 @@ def _build_graph(self):
         return graph
 
     def _build_contract(
-        self, forbidden_modules, ignore_imports=None, include_external_packages=False
+        self,
+        forbidden_modules,
+        ignore_imports=None,
+        include_external_packages=False,
+        allow_indirect_imports=None,
     ):
         session_options = {"root_packages": ["mypackage"]}
         if include_external_packages:
@@ -198,6 +215,7 @@ def _build_contract(
                 "source_modules": ("mypackage.one", "mypackage.two", "mypackage.three"),
                 "forbidden_modules": forbidden_modules,
                 "ignore_imports": ignore_imports or [],
+                "allow_indirect_imports": allow_indirect_imports,
             },
         )