Grimp types (#154)

Use DetailedImport type hinting

Co-authored-by: Matthew Gamble <matthew.gamble@rea-group.com>

CHANGELOG.rst

@@ -5,6 +5,7 @@ latest
 ------
 
 * Switch from optional dependency of ``toml`` to required dependency of ``tomli`` for Python versions < 3.11.
+* Use DetailedImport type hinting made available in Grimp 2.2.
 
 1.6.0 (2022-12-7)
 -----------------

setup.py

@@ -53,7 +53,7 @@ def read(*names, **kwargs):
     python_requires=">=3.7",
     install_requires=[
         "click>=6",
-        "grimp>=2.0",
+        "grimp>=2.2",
         "tomli>=1.2.1; python_version < '3.11'",
         "typing-extensions>=3.10.0.0",
     ],

src/importlinter/adapters/building.py

@@ -1,6 +1,6 @@
 from typing import List
 
-import grimp  # type: ignore
+import grimp
 from importlinter.application.ports import building as ports
 from importlinter.domain.ports.graph import ImportGraph
 

src/importlinter/contracts/_common.py

@@ -6,7 +6,7 @@
 without warning.
 """
 
-from typing import List, Optional, Tuple, Union, cast
+from typing import List, Optional, Sequence, Tuple, Union, cast
 
 from typing_extensions import TypedDict
 
@@ -18,7 +18,8 @@
 class Link(TypedDict):
     importer: str
     imported: str
-    line_numbers: Tuple[int, ...]
+    # If the graph has been built manually, we may not know the line number.
+    line_numbers: Tuple[Optional[int], ...]
 
 
 Chain = List[Link]
@@ -136,6 +137,18 @@ def _pop_shortest_chains(graph: ImportGraph, importer: str, imported: str):
             yield chain
 
 
+def format_line_numbers(line_numbers: Sequence[Optional[int]]) -> str:
+    """
+    Return a human-readable string of the supplied line numbers.
+
+    Unknown line numbers should be provided as a None value in the sequence. E.g.
+    (None,) will be returned as "l.?".
+    """
+    return ", ".join(
+        "l.?" if line_number is None else f"l.{line_number}" for line_number in line_numbers
+    )
+
+
 def _render_direct_import(
     direct_import,
     first_line: bool = False,
@@ -147,21 +160,21 @@ def _render_direct_import(
         for position, source in enumerate([direct_import] + extra_firsts[:-1]):
             prefix = "& " if position > 0 else ""
             importer = source["importer"]
-            line_numbers = ", ".join(f"l.{n}" for n in source["line_numbers"])
+            line_numbers = format_line_numbers(source["line_numbers"])
             import_strings.append(f"{prefix}{importer} ({line_numbers})")
         importer, imported = extra_firsts[-1]["importer"], extra_firsts[-1]["imported"]
-        line_numbers = ", ".join(f"l.{n}" for n in extra_firsts[-1]["line_numbers"])
+        line_numbers = format_line_numbers(extra_firsts[-1]["line_numbers"])
         import_strings.append(f"& {importer} -> {imported} ({line_numbers})")
     else:
         importer, imported = direct_import["importer"], direct_import["imported"]
-        line_numbers = ", ".join(f"l.{n}" for n in direct_import["line_numbers"])
+        line_numbers = format_line_numbers(direct_import["line_numbers"])
         import_strings.append(f"{importer} -> {imported} ({line_numbers})")
 
     if extra_lasts:
         indent_string = (len(direct_import["importer"]) + 4) * " "
         for destination in extra_lasts:
             imported = destination["imported"]
-            line_numbers = ", ".join(f"l.{n}" for n in destination["line_numbers"])
+            line_numbers = format_line_numbers(destination["line_numbers"])
             import_strings.append(f"{indent_string}& {imported} ({line_numbers})")
 
     for position, import_string in enumerate(import_strings):

src/importlinter/contracts/forbidden.py

@@ -8,6 +8,8 @@
 from importlinter.domain.imports import Module
 from importlinter.domain.ports.graph import ImportGraph
 
+from ._common import format_line_numbers
+
 
 class ForbiddenContract(Contract):
     """
@@ -124,7 +126,7 @@ def render_broken_contract(self, check: "ContractCheck") -> None:
                 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"])
+                    line_numbers = format_line_numbers(direct_import["line_numbers"])
                     import_string = f"{importer} -> {imported} ({line_numbers})"
                     if first_line:
                         output.print_error(f"-   {import_string}", bold=False)

src/importlinter/contracts/layers.py

@@ -1,6 +1,7 @@
 import copy
 from typing import Iterator, List, Optional, Set, Tuple, Union, cast
 
+from grimp import DetailedImport
 from typing_extensions import TypedDict
 
 from importlinter.application import contract_utils, output
@@ -38,6 +39,9 @@ class _LayerChainData(TypedDict):
     chains: List[DetailedChain]
 
 
+_UNKNOWN_LINE_NUMBER = -1
+
+
 class LayersContract(Contract):
     """
     Defines a 'layered architecture' where there is a unidirectional dependency flow.
@@ -291,9 +295,13 @@ def _build_layer_chain_data(
             lower_layer_package=lower_layer_package,
             graph=temp_graph,
         )
+
         collapsed_direct_chains: List[DetailedChain] = []
         for import_details_list in import_details_between_layers:
-            line_numbers = tuple(j["line_number"] for j in import_details_list)
+            line_numbers = tuple(
+                j["line_number"] if j["line_number"] != _UNKNOWN_LINE_NUMBER else None
+                for j in import_details_list
+            )
             collapsed_direct_chains.append(
                 {
                     "chain": [
@@ -311,13 +319,13 @@ def _build_layer_chain_data(
         layer_chain_data: _LayerChainData = {
             "higher_layer": higher_layer_package.name,
             "lower_layer": lower_layer_package.name,
-            "chains": collapsed_direct_chains,  # type: ignore
+            "chains": collapsed_direct_chains,
         }
 
         indirect_chain_data = self._get_indirect_collapsed_chains(
             temp_graph, importer_package=lower_layer_package, imported_package=higher_layer_package
         )
-        layer_chain_data["chains"].extend(indirect_chain_data)  # type: ignore
+        layer_chain_data["chains"].extend(indirect_chain_data)
 
         return layer_chain_data
 
@@ -370,7 +378,9 @@ def _remove_layer(self, graph: ImportGraph, layer_package):
         graph.remove_module(layer_package.name)
 
     @classmethod
-    def _pop_direct_imports(cls, higher_layer_package, lower_layer_package, graph: ImportGraph):
+    def _pop_direct_imports(
+        cls, higher_layer_package, lower_layer_package, graph: ImportGraph
+    ) -> List[List[DetailedImport]]:
         import_details_list = []
         lower_layer_modules = {lower_layer_package.name} | graph.find_descendants(
             lower_layer_package.name
@@ -386,13 +396,13 @@ def _pop_direct_imports(cls, higher_layer_package, lower_layer_package, graph: I
                     )
                     if not import_details:
                         # get_import_details may not return any imports (for example if an import
-                        # has been added without metadata. If nothing is returned, we still want
-                        # to add some details about the import to the list.
+                        # has been added without metadata. If nothing is returned, we still add
+                        # details, to keep the type checker happy.
                         import_details = [
                             {
                                 "importer": lower_layer_module,
                                 "imported": imported_module,
-                                "line_number": "?",
+                                "line_number": _UNKNOWN_LINE_NUMBER,
                                 "line_contents": "",
                             }
                         ]

src/importlinter/domain/helpers.py

@@ -1,6 +1,8 @@
 import itertools
 import re
-from typing import Dict, Iterable, List, Pattern, Set, Tuple, Union, cast
+from typing import Iterable, List, Pattern, Set, Tuple, cast
+
+from grimp import DetailedImport
 
 from importlinter.domain.imports import DirectImport, ImportExpression, Module
 from importlinter.domain.ports.graph import ImportGraph
@@ -10,9 +12,7 @@ class MissingImport(Exception):
     pass
 
 
-def pop_imports(
-    graph: ImportGraph, imports: Iterable[DirectImport]
-) -> List[Dict[str, Union[str, int]]]:
+def pop_imports(graph: ImportGraph, imports: Iterable[DirectImport]) -> List[DetailedImport]:
     """
     Removes the supplied direct imports from the graph.
 
@@ -22,7 +22,7 @@ def pop_imports(
     Raises:
         MissingImport if an import is not present in the graph.
     """
-    removed_imports: List[Dict[str, Union[str, int]]] = []
+    removed_imports: List[DetailedImport] = []
 
     imports_to_remove = _dedupe_imports(imports)
 
@@ -121,7 +121,7 @@ def resolve_import_expressions(
 
 def pop_import_expressions(
     graph: ImportGraph, expressions: Iterable[ImportExpression]
-) -> List[Dict[str, Union[str, int]]]:
+) -> List[DetailedImport]:
     """
     Removes any imports matching the supplied import expressions from the graph.
 
@@ -135,7 +135,7 @@ def pop_import_expressions(
     return pop_imports(graph, imports)
 
 
-def add_imports(graph: ImportGraph, import_details: List[Dict[str, Union[str, int]]]) -> None:
+def add_imports(graph: ImportGraph, import_details: List[DetailedImport]) -> None:
     """
     Adds the supplied import details to the graph.
 

src/importlinter/domain/ports/graph.py

@@ -1,8 +1,10 @@
-from typing import Dict, List, Optional, Set, Tuple, Union
+from typing import List, Optional, Set, Tuple
 
 # N.B. typing_extensions can be changed to typing once drop support for Python 3.7.
 from typing_extensions import Protocol
 
+from grimp import DetailedImport
+
 
 class ImportGraph(Protocol):
     @property
@@ -39,9 +41,7 @@ def find_shortest_chains(self, importer: str, imported: str) -> Set[Tuple[str, .
         """
         raise NotImplementedError
 
-    def get_import_details(
-        self, *, importer: str, imported: str
-    ) -> List[Dict[str, Union[str, int]]]:
+    def get_import_details(self, *, importer: str, imported: str) -> List[DetailedImport]:
         """
         Returns a list of the details of every direct import between two modules, in the form:
         [

tests/unit/application/test_contract_utils.py

@@ -1,5 +1,5 @@
 import pytest
-from grimp.adaptors.graph import ImportGraph  # type: ignore
+from grimp.adaptors.graph import ImportGraph
 
 from importlinter.application.contract_utils import AlertLevel, remove_ignored_imports
 from importlinter.domain.helpers import MissingImport

tests/unit/application/test_use_cases.py

@@ -2,7 +2,7 @@
 from typing import Any, Dict, List, Optional
 
 import pytest
-from grimp.adaptors.graph import ImportGraph  # type: ignore
+from grimp.adaptors.graph import ImportGraph
 
 from importlinter.application.app_config import settings
 from importlinter.application.use_cases import FAILURE, SUCCESS, create_report, lint_imports

tests/unit/contracts/test_forbidden.py

@@ -1,5 +1,5 @@
 import pytest
-from grimp.adaptors.graph import ImportGraph  # type: ignore
+from grimp.adaptors.graph import ImportGraph
 
 from importlinter.configuration import settings
 from importlinter.contracts.forbidden import ForbiddenContract

tests/unit/contracts/test_independence.py

@@ -1,5 +1,5 @@
 import pytest
-from grimp.adaptors.graph import ImportGraph  # type: ignore
+from grimp.adaptors.graph import ImportGraph
 
 from importlinter.application.app_config import settings
 from importlinter.contracts.independence import IndependenceContract