Repository: seddonym/import-linter

Issue:
Support "forbidden" contracts within packages (again)
Previously it was possible to write a contract like this:

```
[importlinter:contract:core-factories]
name = "Factories should be split into subpackages"
type = forbidden
allow_indirect_imports = true
source_modules =
    tests.factories
forbidden_modules =
    tests.factories.domain_a
    tests.factories.domain_b
```

...to prevent the following imports:

```python
# tests/factories/__init__.py
from tests.factories.domain_a import factory_1
...
from tests.factories.domain_b import factory_2
```

This stopped working in https://github.com/seddonym/import-linter/commit/2cdb36fb2c4f657a5339d36dd5797bd891e7111d - essentially, the bug was a feature. 

Although this is quite a "dumb" use of import linter - it's little more than a `grep` - it makes sense to express such contracts alongside other more powerful ones for the same codebase.

Would it be possible to support this again?

## Suggestions

- Introduce a separate contract type for this scenario - awkward as the intent is the same.
- Introduce a flag to treat the source modules as modules and not packages*.
- Something more clever where the linter recognises the contract is within the same package.

\*Treating modules as packages seems to be the default / intended behaviour.

Hints from issue comments:
@seddonym: Thanks Ben for these suggestions. I agree we should support this somehow.

I'm leaning towards adding an `as_packages` flag which, if it was set to false, would allow you to do this. That would fit in well with the terminology of [Grimp](https://grimp.readthedocs.io/en/latest/usage.html#ImportGraph.direct_import_exists), the underlying static analysis library: many of the methods there allow you to specify whether to treat the modules `as_packages`.

Open to other ideas though.



@seddonym: Having had more time to mull this over, I think it would the `as_packages` flag would be a good addition. Let me know if you would be interested in submitting a pull request!@NicholasBunn: I _think_ [this PR](https://github.com/seddonym/import-linter/pull/229) should re-introduce this functionality

Solution patch:
diff --git a/AUTHORS.rst b/AUTHORS.rst
index 6e048dd..493d25f 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -24,3 +24,4 @@ Contributors
 * Peter Byfield - https://github.com/Peter554
 * Fabian Binz - https://github.com/fbinz
 * Neil Williams - https://github.com/spladug
+* Nicholas Bunn - https://github.com/NicholasBunn
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index f5f1f02..320419e 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,6 +1,11 @@
 Changelog
 =========
 
+latest
+------
+
+* Add as_packages field to forbidden contracts.
+
 2.2 (2025-02-07)
 ----------------
 
diff --git a/docs/contract_types.rst b/docs/contract_types.rst
index a9af7e5..b4cd150 100644
--- a/docs/contract_types.rst
+++ b/docs/contract_types.rst
@@ -11,8 +11,10 @@ Forbidden modules
 
 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.
+By default, 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. This
+descendant behaviour can be changed by setting ``as_packages`` to ``False``: in that case, only explicitly listed modules will be
+checked, not their descendants.
 
 External packages may also be forbidden.
 
@@ -66,6 +68,9 @@ External packages may also be forbidden.
     - ``unmatched_ignore_imports_alerting``: See :ref:`Shared options`.
     - ``allow_indirect_imports``: If ``True``, allow indirect imports to forbidden modules without interpreting them
       as a reason to mark the contract broken. (Optional.)
+    - ``as_packages``: Whether to treat the source and forbidden modules as packages. If ``False``, each of the modules
+      passed in will be treated as a module rather than a package. Default behaviour is ``True`` (treat modules as
+      packages).
 
 Independence
 ------------
diff --git a/src/importlinter/contracts/forbidden.py b/src/importlinter/contracts/forbidden.py
index f10eff5..799ab81 100644
--- a/src/importlinter/contracts/forbidden.py
+++ b/src/importlinter/contracts/forbidden.py
@@ -30,6 +30,10 @@ class ForbiddenContract(Contract):
         - unmatched_ignore_imports_alerting: Decides how to report when the expression in the
                              `ignore_imports` set is not found in the graph. Valid values are
                              "none", "warn", "error". Default value is "error".
+        - as_packages:       Whether to treat the source and forbidden modules as packages. If
+                             False, each of the modules passed in will be treated as a module
+                             rather than a package. Default behaviour is True (treat modules as
+                             packages).
     """
 
     type_name = "forbidden"
@@ -39,6 +43,7 @@ class ForbiddenContract(Contract):
     ignore_imports = fields.SetField(subfield=fields.ImportExpressionField(), required=False)
     allow_indirect_imports = fields.BooleanField(required=False, default=False)
     unmatched_ignore_imports_alerting = fields.EnumField(AlertLevel, default=AlertLevel.ERROR)
+    as_packages = fields.BooleanField(required=False, default=True)
 
     def check(self, graph: ImportGraph, verbose: bool) -> ContractCheck:
         is_kept = True
@@ -87,10 +92,14 @@ def sort_key(module):
                     }
 
                     if str(self.allow_indirect_imports).lower() == "true":
-                        chains = self._get_direct_chains(source_module, forbidden_module, graph)
+                        chains = self._get_direct_chains(
+                            source_module, forbidden_module, graph, self.as_packages  # type:ignore
+                        )
                     else:
                         chains = graph.find_shortest_chains(
-                            importer=source_module.name, imported=forbidden_module.name
+                            importer=source_module.name,
+                            imported=forbidden_module.name,
+                            as_packages=self.as_packages,  # type:ignore
                         )
                     if chains:
                         is_kept = False
@@ -192,11 +201,23 @@ def _graph_was_built_with_externals(self) -> bool:
         return str(self.session_options.get("include_external_packages")).lower() == "true"
 
     def _get_direct_chains(
-        self, source_package: Module, forbidden_package: Module, graph: ImportGraph
+        self,
+        source_package: Module,
+        forbidden_package: Module,
+        graph: ImportGraph,
+        as_packages: bool,
     ) -> set[tuple[str, ...]]:
         chains: set[tuple[str, ...]] = set()
-        source_modules = self._get_all_modules_in_package(source_package, graph)
-        forbidden_modules = self._get_all_modules_in_package(forbidden_package, graph)
+        source_modules = (
+            self._get_all_modules_in_package(source_package, graph)
+            if as_packages
+            else {source_package}
+        )
+        forbidden_modules = (
+            self._get_all_modules_in_package(forbidden_package, graph)
+            if as_packages
+            else {forbidden_package}
+        )
         for source_module in source_modules:
             imported_module_names = graph.find_modules_directly_imported_by(source_module.name)
             for imported_module_name in imported_module_names:


Test patch:
diff --git a/tests/unit/contracts/test_forbidden.py b/tests/unit/contracts/test_forbidden.py
index 17ed0ab..67dec00 100644
--- a/tests/unit/contracts/test_forbidden.py
+++ b/tests/unit/contracts/test_forbidden.py
@@ -1,3 +1,5 @@
+from typing import Tuple, List, Dict
+
 import pytest
 from grimp.adaptors.graph import ImportGraph
 
@@ -14,91 +16,163 @@ def configure():
 
 
 class TestForbiddenContract:
-    def test_is_kept_when_no_forbidden_modules_imported(self):
+    @pytest.mark.parametrize("as_packages", ("False", "True"))
+    def test_is_kept_when_no_forbidden_modules_imported(
+        self,
+        as_packages: bool,
+    ):
         graph = self._build_graph()
-        contract = self._build_contract(forbidden_modules=("mypackage.blue", "mypackage.yellow"))
+        contract = self._build_contract(
+            forbidden_modules=("mypackage.blue", "mypackage.yellow"),
+            as_packages=as_packages,
+        )
 
         contract_check = contract.check(graph=graph, verbose=False)
 
         assert contract_check.kept
 
-    def test_is_broken_when_forbidden_modules_imported(self):
+    @pytest.mark.parametrize(
+        "as_packages, forbidden_modules, expected_invalid_chains",
+        [
+            (
+                True,
+                (
+                    "mypackage.blue",
+                    "mypackage.green",
+                    "mypackage.yellow",
+                    "mypackage.purple",
+                ),
+                [
+                    {
+                        "upstream_module": "mypackage.green",
+                        "downstream_module": "mypackage.one",
+                        "chains": [
+                            [
+                                {
+                                    "importer": "mypackage.one.alpha",
+                                    "imported": "mypackage.green.beta",
+                                    "line_numbers": (3,),
+                                }
+                            ],
+                            [
+                                {
+                                    "importer": "mypackage.one.alpha.circle",
+                                    "imported": "mypackage.green.beta.sphere",
+                                    "line_numbers": (8,),
+                                },
+                            ],
+                        ],
+                    },
+                    {
+                        "upstream_module": "mypackage.green",
+                        "downstream_module": "mypackage.three",
+                        "chains": [
+                            [
+                                {
+                                    "importer": "mypackage.three",
+                                    "imported": "mypackage.green",
+                                    "line_numbers": (4,),
+                                }
+                            ]
+                        ],
+                    },
+                    {
+                        "upstream_module": "mypackage.purple",
+                        "downstream_module": "mypackage.two",
+                        "chains": [
+                            [
+                                {
+                                    "importer": "mypackage.two",
+                                    "imported": "mypackage.utils",
+                                    "line_numbers": (9,),
+                                },
+                                {
+                                    "importer": "mypackage.utils",
+                                    "imported": "mypackage.purple",
+                                    "line_numbers": (1,),
+                                },
+                            ]
+                        ],
+                    },
+                ],
+            ),
+            (
+                False,
+                (
+                    "mypackage.blue",
+                    "mypackage.green",
+                    "mypackage.yellow",
+                    "mypackage.purple",
+                ),
+                [
+                    {
+                        "upstream_module": "mypackage.green",
+                        "downstream_module": "mypackage.three",
+                        "chains": [
+                            [
+                                {
+                                    "importer": "mypackage.three",
+                                    "imported": "mypackage.green",
+                                    "line_numbers": (4,),
+                                }
+                            ]
+                        ],
+                    },
+                    {
+                        "upstream_module": "mypackage.purple",
+                        "downstream_module": "mypackage.two",
+                        "chains": [
+                            [
+                                {
+                                    "importer": "mypackage.two",
+                                    "imported": "mypackage.utils",
+                                    "line_numbers": (9,),
+                                },
+                                {
+                                    "importer": "mypackage.utils",
+                                    "imported": "mypackage.purple",
+                                    "line_numbers": (1,),
+                                },
+                            ]
+                        ],
+                    },
+                ],
+            ),
+        ],
+    )
+    def test_is_broken_when_forbidden_modules_imported(
+        self,
+        as_packages: str,
+        forbidden_modules: Tuple[str],
+        expected_invalid_chains: List[Dict],
+    ):
         graph = self._build_graph()
         contract = self._build_contract(
-            forbidden_modules=(
-                "mypackage.blue",
-                "mypackage.green",
-                "mypackage.yellow",
-                "mypackage.purple",
-            )
+            forbidden_modules=forbidden_modules,
+            as_packages=as_packages,
         )
 
         contract_check = contract.check(graph=graph, verbose=False)
 
         assert not contract_check.kept
 
-        expected_metadata = {
-            "invalid_chains": [
-                {
-                    "upstream_module": "mypackage.green",
-                    "downstream_module": "mypackage.one",
-                    "chains": [
-                        [
-                            {
-                                "importer": "mypackage.one.alpha",
-                                "imported": "mypackage.green.beta",
-                                "line_numbers": (3,),
-                            }
-                        ],
-                        [
-                            {
-                                "importer": "mypackage.one.alpha.circle",
-                                "imported": "mypackage.green.beta.sphere",
-                                "line_numbers": (8,),
-                            },
-                        ],
-                    ],
-                },
-                {
-                    "upstream_module": "mypackage.green",
-                    "downstream_module": "mypackage.three",
-                    "chains": [
-                        [
-                            {
-                                "importer": "mypackage.three",
-                                "imported": "mypackage.green",
-                                "line_numbers": (4,),
-                            }
-                        ]
-                    ],
-                },
-                {
-                    "upstream_module": "mypackage.purple",
-                    "downstream_module": "mypackage.two",
-                    "chains": [
-                        [
-                            {
-                                "importer": "mypackage.two",
-                                "imported": "mypackage.utils",
-                                "line_numbers": (9,),
-                            },
-                            {
-                                "importer": "mypackage.utils",
-                                "imported": "mypackage.purple",
-                                "line_numbers": (1,),
-                            },
-                        ]
-                    ],
-                },
-            ]
-        }
+        expected_metadata = {"invalid_chains": expected_invalid_chains}
 
         assert expected_metadata == contract_check.metadata
 
-    def test_is_broken_when_forbidden_external_modules_imported(self):
+    @pytest.mark.parametrize(
+        "as_packages",
+        (False, True),
+    )
+    def test_is_broken_when_forbidden_external_modules_imported(
+        self,
+        as_packages: str,
+    ):
         graph = self._build_graph()
         contract = self._build_contract(
-            forbidden_modules=("sqlalchemy", "requests"), include_external_packages=True
+            forbidden_modules=("sqlalchemy", "requests"),
+            include_external_packages=True,
+            as_packages=as_packages,
         )
 
         contract_check = contract.check(graph=graph, verbose=False)
@@ -125,9 +199,22 @@ def test_is_broken_when_forbidden_external_modules_imported(self):
 
         assert expected_metadata == contract_check.metadata
 
-    def test_is_invalid_when_forbidden_externals_but_graph_does_not_include_externals(self):
+    @pytest.mark.parametrize(
+        "as_packages",
+        (
+            ("False"),
+            ("True"),
+        ),
+    )
+    def test_is_invalid_when_forbidden_externals_but_graph_does_not_include_externals(
+        self,
+        as_packages: bool,
+    ):
         graph = self._build_graph()
-        contract = self._build_contract(forbidden_modules=("sqlalchemy", "requests"))
+        contract = self._build_contract(
+            forbidden_modules=("sqlalchemy", "requests"),
+            as_packages=as_packages,
+        )
 
         with pytest.raises(
             ValueError,
@@ -138,7 +225,14 @@ def test_is_invalid_when_forbidden_externals_but_graph_does_not_include_external
         ):
             contract.check(graph=graph, verbose=False)
 
-    def test_ignore_imports_tolerates_duplicates(self):
+    @pytest.mark.parametrize(
+        "as_packages",
+        (
+            ("False"),
+            ("True"),
+        ),
+    )
+    def test_ignore_imports_tolerates_duplicates(self, as_packages: bool):
         graph = self._build_graph()
         contract = self._build_contract(
             forbidden_modules=("mypackage.blue", "mypackage.yellow"),
@@ -148,6 +242,7 @@ def test_ignore_imports_tolerates_duplicates(self):
                 "mypackage.three -> mypackage.green",
             ),
             include_external_packages=False,
+            as_packages=as_packages,
         )
 
         check = contract.check(graph=graph, verbose=False)
@@ -315,46 +410,85 @@ def test_recursive_wildcards_in_forbidden_modules_are_resolved(self):
             ],
         }
 
-    def test_ignore_imports_with_wildcards(self):
+    @pytest.mark.parametrize(
+        "as_packages, forbidden_modules, expected_invalid_chains",
+        [
+            (
+                True,
+                ("mypackage.green",),
+                [
+                    {
+                        "upstream_module": "mypackage.green",
+                        "downstream_module": "mypackage.one",
+                        "chains": [
+                            [
+                                {
+                                    "importer": "mypackage.one.alpha.circle",
+                                    "imported": "mypackage.green.beta.sphere",
+                                    "line_numbers": (8,),
+                                },
+                            ]
+                        ],
+                    },
+                    {
+                        "upstream_module": "mypackage.green",
+                        "downstream_module": "mypackage.three",
+                        "chains": [
+                            [
+                                {
+                                    "importer": "mypackage.three",
+                                    "imported": "mypackage.green",
+                                    "line_numbers": (4,),
+                                },
+                            ]
+                        ],
+                    },
+                ],
+            ),
+            (
+                False,
+                ("mypackage.green",),
+                [
+                    {
+                        "upstream_module": "mypackage.green",
+                        "downstream_module": "mypackage.three",
+                        "chains": [
+                            [
+                                {
+                                    "importer": "mypackage.three",
+                                    "imported": "mypackage.green",
+                                    "line_numbers": (4,),
+                                },
+                            ]
+                        ],
+                    },
+                ],
+            ),
+        ],
+    )
+    def test_ignore_imports_with_wildcards(
+        self,
+        as_packages: str,
+        forbidden_modules: Tuple[str],
+        expected_invalid_chains: List[Dict],
+    ):
         graph = self._build_graph()
         contract = self._build_contract(
-            forbidden_modules=("mypackage.green",),
+            forbidden_modules=forbidden_modules,
             ignore_imports=("mypackage.*.alpha -> mypackage.*.beta",),
+            as_packages=as_packages,
         )
 
         check = contract.check(graph=graph, verbose=False)
         assert check.metadata == {
-            "invalid_chains": [
-                {
-                    "upstream_module": "mypackage.green",
-                    "downstream_module": "mypackage.one",
-                    "chains": [
-                        [
-                            {
-                                "importer": "mypackage.one.alpha.circle",
-                                "imported": "mypackage.green.beta.sphere",
-                                "line_numbers": (8,),
-                            },
-                        ]
-                    ],
-                },
-                {
-                    "upstream_module": "mypackage.green",
-                    "downstream_module": "mypackage.three",
-                    "chains": [
-                        [
-                            {
-                                "importer": "mypackage.three",
-                                "imported": "mypackage.green",
-                                "line_numbers": (4,),
-                            },
-                        ]
-                    ],
-                },
-            ],
+            "invalid_chains": expected_invalid_chains,
         }
 
-    def test_ignore_imports_with_recursive_wildcards(self):
+    @pytest.mark.parametrize("as_packages", (False, True))
+    def test_ignore_imports_with_recursive_wildcards(
+        self,
+        as_packages: str,
+    ):
         graph = self._build_graph()
         contract = self._build_contract(
             forbidden_modules=("mypackage.green",),
@@ -362,6 +496,7 @@ def test_ignore_imports_with_recursive_wildcards(self):
                 "mypackage.**.alpha -> mypackage.**.beta",
                 "mypackage.**.circle -> mypackage.**.sphere",
             ),
+            as_packages=as_packages,
         )
 
         check = contract.check(graph=graph, verbose=False)
@@ -380,7 +515,7 @@ def test_ignore_imports_with_recursive_wildcards(self):
                         ]
                     ],
                 },
-            ],
+            ]
         }
 
     @pytest.mark.parametrize(
@@ -458,7 +593,14 @@ def test_allow_indirect_imports(self, importer, imported, allow_indirect_imports
             ],
         }
 
-    def test_ignore_imports_adds_warnings(self):
+    @pytest.mark.parametrize(
+        "as_packages",
+        (
+            ("False"),
+            ("True"),
+        ),
+    )
+    def test_ignore_imports_adds_warnings(self, as_packages: bool):
         graph = self._build_graph()
         contract = ForbiddenContract(
             name="Forbid contract",
@@ -471,6 +613,7 @@ def test_ignore_imports_adds_warnings(self):
                     "mypackage.*.nonexistent -> mypackage.three",
                 ],
                 "unmatched_ignore_imports_alerting": "warn",
+                "as_packages": as_packages,
             },
         )
 
@@ -481,6 +624,13 @@ def test_ignore_imports_adds_warnings(self):
             "No matches for ignored import mypackage.*.nonexistent -> mypackage.three.",
         }
 
+    @pytest.mark.parametrize(
+        "as_packages",
+        (
+            ("False"),
+            ("True"),
+        ),
+    )
     @pytest.mark.parametrize(
         "module, expected_error",
         (
@@ -504,11 +654,16 @@ def test_ignore_imports_adds_warnings(self):
         ),
     )
     def test_is_invalid_when_subpackages_of_external_packages_are_provided(
-        self, module: str, expected_error: str
+        self,
+        module: str,
+        expected_error: str,
+        as_packages: bool,
     ):
         graph = self._build_graph()
         contract = self._build_contract(
-            forbidden_modules=("mypackage.blue", module), include_external_packages=True
+            forbidden_modules=("mypackage.blue", module),
+            include_external_packages=True,
+            as_packages=as_packages,
         )
 
         with pytest.raises(
@@ -567,7 +722,10 @@ def _build_graph(self):
             line_contents="foo",
         )
         graph.add_import(
-            importer="mypackage.three", imported="sqlalchemy", line_number=1, line_contents="foo"
+            importer="mypackage.three",
+            imported="sqlalchemy",
+            line_number=1,
+            line_contents="foo",
         )
         return graph
 
@@ -578,6 +736,7 @@ def _build_contract(
         include_external_packages=False,
         allow_indirect_imports=None,
         source_modules=None,
+        as_packages=True,
     ):
         session_options = {"root_packages": ["mypackage"]}
         if include_external_packages:
@@ -589,10 +748,12 @@ def _build_contract(
             "forbidden_modules": forbidden_modules,
             "ignore_imports": ignore_imports or [],
         }
+
         if allow_indirect_imports is not None:
             contract_options["allow_indirect_imports"] = (
                 "true" if allow_indirect_imports else "false"
             )
+        contract_options["as_packages"] = "true" if as_packages else "false"
 
         return ForbiddenContract(
             name="Forbid contract",
@@ -777,7 +938,10 @@ def test_verbose(self):
             line_contents="foo",
         )
         graph.add_import(
-            importer="mypackage.three", imported="sqlalchemy", line_number=1, line_contents="foo"
+            importer="mypackage.three",
+            imported="sqlalchemy",
+            line_number=1,
+            line_contents="foo",
         )
         contract = ForbiddenContract(
             name="Forbid contract",