Skip to content

Commit

Permalink
Support wildcards in module ignores (#103) (#106)
Browse files Browse the repository at this point in the history
Support wildcards in module ignores

Co-authored-by: kasium <15907922+kasium@users.noreply.github.com>
  • Loading branch information
seddonym and kasium committed Sep 21, 2021
1 parent 56738f7 commit 67a7707
Show file tree
Hide file tree
Showing 23 changed files with 680 additions and 55 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Changelog
latest
------

* Wildcard support for ignored imports.
* Convert TOML booleans to strings in UserOptions, to make consistent with INI file parsing.

1.2.4 (2021-08-09)
Expand Down
37 changes: 25 additions & 12 deletions docs/contract_types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,7 @@ Configuration options:
- ``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.)
- ``ignore_imports``: 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.)

Expand Down Expand Up @@ -96,10 +93,8 @@ They do this by checking that there are no imports in any direction between the
**Configuration options**

- ``modules``: A list of modules/subpackages that should be independent from each other.
- ``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.)
- ``ignore_imports``: See :ref:`Shared options`.


Layers
------
Expand Down Expand Up @@ -187,14 +182,32 @@ won't complain.
- ``containers``:
List of the parent modules of the layers, as *absolute names* that you could import, such as
``mypackage.foo``. (Optional.)
- ``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.)
- ``ignore_imports``: See :ref:`Shared options`.



Custom contract types
---------------------

If none of the built in contract types meets your needs, you can define a custom contract type: see
:doc:`custom_contract_types`.


.. _Shared options:

Options used by multiple contracts
----------------------------------

- ``ignore_imports``: Optional 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.

Wildcards (in the form of ``*``) are supported. These can stand in for a module names, but they do not extend to
subpackages.

Examples:

- ``mypackage.*``: matches ``mypackage.foo`` but not ``mypackage.foo.bar``.
- ``mypackage.*.baz``: matches ``mypackage.foo.baz`` but not ``mypackage.foo.bar.baz``.
- ``mypackage.*.*``: matches ``mypackage.foo.bar`` and ``mypackage.foobar.baz``.
- ``mypackage.foo*``: not a valid expression. (The wildcard must replace a whole module name.)
6 changes: 3 additions & 3 deletions src/importlinter/contracts/forbidden.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class ForbiddenContract(Contract):
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
- ignore_imports: A set of ImportExpressions. 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.
Expand All @@ -24,14 +24,14 @@ class ForbiddenContract(Contract):

source_modules = fields.ListField(subfield=fields.ModuleField())
forbidden_modules = fields.ListField(subfield=fields.ModuleField())
ignore_imports = fields.SetField(subfield=fields.DirectImportField(), required=False)
ignore_imports = fields.SetField(subfield=fields.ImportExpressionField(), required=False)
allow_indirect_imports = fields.StringField(required=False)

def check(self, graph: ImportGraph) -> ContractCheck:
is_kept = True
invalid_chains = []

helpers.pop_imports(
helpers.pop_import_expressions(
graph, self.ignore_imports if self.ignore_imports else [] # type: ignore
)

Expand Down
6 changes: 3 additions & 3 deletions src/importlinter/contracts/independence.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,21 @@ class IndependenceContract(Contract):
Configuration options:
- modules: A list of Modules that should be independent from each other.
- ignore_imports: A set of DirectImports. These imports will be ignored: if the import
- ignore_imports: A set of ImportExpressions. 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.)
"""

type_name = "independence"

modules = fields.ListField(subfield=fields.ModuleField())
ignore_imports = fields.SetField(subfield=fields.DirectImportField(), required=False)
ignore_imports = fields.SetField(subfield=fields.ImportExpressionField(), required=False)

def check(self, graph: ImportGraph) -> ContractCheck:
is_kept = True
invalid_chains = []

helpers.pop_imports(
helpers.pop_import_expressions(
graph, self.ignore_imports if self.ignore_imports else [] # type: ignore
)

Expand Down
6 changes: 3 additions & 3 deletions src/importlinter/contracts/layers.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class LayersContract(Contract):
- layers: An ordered list of layers. Each layer is the name of a module relative
to its parent package. The order is from higher to lower level layers.
- containers: A list of the parent Modules of the layers (optional).
- ignore_imports: A set of DirectImports. These imports will be ignored: if the import
- ignore_imports: A set of ImportExpressions. 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.)
"""
Expand All @@ -51,14 +51,14 @@ class LayersContract(Contract):

layers = fields.ListField(subfield=LayerField())
containers = fields.ListField(subfield=fields.StringField(), required=False)
ignore_imports = fields.SetField(subfield=fields.DirectImportField(), required=False)
ignore_imports = fields.SetField(subfield=fields.ImportExpressionField(), required=False)

def check(self, graph: ImportGraph) -> ContractCheck:
is_kept = True
invalid_chains = []

direct_imports_to_ignore = self.ignore_imports if self.ignore_imports else []
helpers.pop_imports(graph, direct_imports_to_ignore) # type: ignore
helpers.pop_import_expressions(graph, direct_imports_to_ignore) # type: ignore

if self.containers:
self._validate_containers(graph)
Expand Down
26 changes: 20 additions & 6 deletions src/importlinter/domain/fields.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import abc
from typing import Generic, Iterable, List, Set, TypeVar, Union

from importlinter.domain.imports import DirectImport, Module
from importlinter.domain.imports import Module, ImportExpression

FieldValue = TypeVar("FieldValue")

Expand Down Expand Up @@ -109,16 +109,30 @@ def parse(self, raw_data: Union[str, List]) -> Module:
return Module(StringField().parse(raw_data))


class DirectImportField(Field):
class ImportExpressionField(Field):
"""
A field for DirectImports.
A field for ImportExpressions.
Expects raw data in the form: "mypackage.foo.importer -> mypackage.bar.imported".
Expects raw data in the form:
"mypackage.foo.importer -> mypackage.bar.imported".
In addition, it handles wildcards:
"mypackage.*.importer -> mypackage.bar.*"
"""

def parse(self, raw_data: Union[str, List]) -> DirectImport:
def parse(self, raw_data: Union[str, List]) -> ImportExpression:
string = StringField().parse(raw_data)
importer, _, imported = string.partition(" -> ")

if not (importer and imported):
raise ValidationError('Must be in the form "package.importer -> package.imported".')
return DirectImport(importer=Module(importer), imported=Module(imported))

self._validate_wildcard(importer)
self._validate_wildcard(imported)

return ImportExpression(importer=importer, imported=imported)

def _validate_wildcard(self, expression: str) -> None:
for part in expression.split("."):
if len(part) > 1 and "*" in part:
raise ValidationError("A wildcard can only replace a whole module.")
94 changes: 91 additions & 3 deletions src/importlinter/domain/helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from typing import Dict, Iterable, List, Union
import itertools
import re
from typing import Dict, Iterable, List, Pattern, Set, Tuple, Union, cast

from importlinter.domain.imports import DirectImport
from importlinter.domain.imports import DirectImport, ImportExpression, Module
from importlinter.domain.ports.graph import ImportGraph


Expand All @@ -18,7 +20,7 @@ def pop_imports(
The list of import details that were removed, including any additional metadata.
Raises:
MissingImport if the import is not present in the graph.
MissingImport if an import is not present in the graph.
"""
removed_imports: List[Dict[str, Union[str, int]]] = []
for import_to_remove in imports:
Expand All @@ -34,6 +36,57 @@ def pop_imports(
return removed_imports


def import_expressions_to_imports(
graph: ImportGraph, expressions: Iterable[ImportExpression]
) -> List[DirectImport]:
"""
Returns a list of imports in a graph, given some import expressions.
Raises:
MissingImport if an import is not present in the graph. For a wildcarded import expression,
this is raised if there is not at least one match.
"""
imports: Set[DirectImport] = set()
for expression in expressions:
matched = False
for (importer, imported) in _expression_to_modules(expression, graph):
import_details = graph.get_import_details(
importer=importer.name, imported=imported.name
)
if import_details:
for individual_import_details in import_details:
imports.add(
DirectImport(
importer=Module(cast(str, individual_import_details["importer"])),
imported=Module(cast(str, individual_import_details["imported"])),
line_number=cast(int, individual_import_details["line_number"]),
line_contents=cast(str, individual_import_details["line_contents"]),
)
)
matched = True
if not matched:
raise MissingImport(
f"Ignored import expression {expression} didn't match anything in the graph."
)
return list(imports)


def pop_import_expressions(
graph: ImportGraph, expressions: Iterable[ImportExpression]
) -> List[Dict[str, Union[str, int]]]:
"""
Removes any imports matching the supplied import expressions from the graph.
Returns:
The list of imports that were removed, including any additional metadata.
Raises:
MissingImport if an import is not present in the graph. For a wildcarded import expression,
this is raised if there is not at least one match.
"""
imports = import_expressions_to_imports(graph, expressions)
return pop_imports(graph, imports)


def add_imports(graph: ImportGraph, import_details: List[Dict[str, Union[str, int]]]) -> None:
"""
Adds the supplied import details to the graph.
Expand All @@ -55,3 +108,38 @@ def add_imports(graph: ImportGraph, import_details: List[Dict[str, Union[str, in
line_number=details["line_number"],
line_contents=details["line_contents"],
)


def _to_pattern(expression: str) -> Pattern:
"""
Function which translates an import expression into a regex pattern.
"""
pattern_parts = []
for part in expression.split("."):
if "*" == part:
pattern_parts.append(part.replace("*", r"[^\.]+"))
else:
pattern_parts.append(part)
return re.compile(r"^" + r"\.".join(pattern_parts) + r"$")


def _expression_to_modules(
expression: ImportExpression, graph: ImportGraph
) -> Iterable[Tuple[Module, Module]]:
if not expression.has_wildcard_expression():
return [(Module(expression.importer), Module(expression.imported))]

importer = []
imported = []

importer_pattern = _to_pattern(expression.importer)
imported_expression = _to_pattern(expression.imported)

for module in graph.modules:

if importer_pattern.match(module):
importer.append(Module(module))
if imported_expression.match(module):
imported.append(Module(module))

return itertools.product(set(importer), set(imported))
23 changes: 23 additions & 0 deletions src/importlinter/domain/imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,26 @@ def __str__(self) -> str:

def __hash__(self) -> int:
return hash((str(self), self.line_contents))


class ImportExpression(ValueObject):
"""
A user-submitted expression describing an import or set of imports.
Sets of imports are notated using * wildcards.
These wildcards can stand in for a module name or part of a name, but they do
not extend to subpackages.
For example, "mypackage.*" refers to every child subpackage of mypackage.
It does not, however, include more distant descendants such as mypackage.foo.bar.
"""

def __init__(self, importer: str, imported: str) -> None:
self.importer = importer
self.imported = imported

def has_wildcard_expression(self) -> bool:
return "*" in self.imported or "*" in self.importer

def __str__(self) -> str:
return "{} -> {}".format(self.importer, self.imported)
11 changes: 11 additions & 0 deletions tests/assets/testpackage/.setup.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[tool.importlinter]
root_package = "testpackage"

[[tool.importlinter.contracts]]
name = "Test independence contract"
type = "independence"
modules = ["testpackage.high.blue", "testpackage.high.green"]
ignore_imports = [
"testpackage.utils -> testpackage.high.green",
"testpackage.*.blue.* -> testpackage.indirect.*",
]
1 change: 1 addition & 0 deletions tests/assets/testpackage/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ modules=
testpackage.high.green
ignore_imports=
testpackage.utils -> testpackage.high.green
testpackage.*.blue.* -> testpackage.indirect.*
1 change: 1 addition & 0 deletions tests/assets/testpackage/testpackage/high/blue/one.py
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
from testpackage import utils
from testpackage.indirect import yellow
1 change: 1 addition & 0 deletions tests/assets/testpackage/testpackage/high/blue/two.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from testpackage.indirect import brown
Empty file.
1 change: 1 addition & 0 deletions tests/assets/testpackage/testpackage/indirect/brown.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import testpackage.high.green
1 change: 1 addition & 0 deletions tests/assets/testpackage/testpackage/indirect/yellow.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import testpackage.high.green

0 comments on commit 67a7707

Please sign in to comment.