Skip to content

Commit

Permalink
Support recursive wildcards in module ignores
Browse files Browse the repository at this point in the history
  • Loading branch information
ngnpope authored and seddonym committed Jun 7, 2023
1 parent 30e6133 commit a181b6f
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 4 deletions.
1 change: 1 addition & 0 deletions AUTHORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ Contributors
* Petter Friberg - https://github.com/flaeppe
* James Owen - https://github.com/leamingrad
* Matthew Gamble - https://github.com/mwgamble
* Nick Pope - https://github.com/ngnpope
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
Changelog
=========

1.10.0 (Unreleased)
-------------------

* Recursive wildcard support for ignored imports.

1.9.0 (2023-05-13)
------------------

Expand Down
8 changes: 7 additions & 1 deletion docs/contract_types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,20 @@ Options used by multiple contracts
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.
subpackages. A recursive wildcard (in the form of ``**``) is also supported.

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.)
- ``mypackage.**``: matches ``mypackage.foo.bar`` and ``mypackage.foobar.baz``.

This comment has been minimized.

Copy link
@seddonym

seddonym Jun 7, 2023

Owner

BTW I'm going to tweak these slightly as I'm not sure we need so much here.

- ``mypackage.**.qux``: matches ``mypackage.foo.bar.qux`` and ``mypackage.foobar.baz.qux``.
- ``mypackage.foo**``: not a valid expression. (The wildcard must replace a whole module name.)
- ``mypackage.**.*.qux``: not a valid expression. (A recursive wildcard cannot be followed by a wildcard.)
- ``mypackage.**.**.qux``: not a valid expression. (A recursive wildcard cannot be followed by a wildcard.)
- ``mypackage.*.**.qux``: not a valid expression. (A wildcard cannot be followed by a recursive wildcard.)

- ``unmatched_ignore_imports_alerting``: The alerting level for handling expressions supplied in ``ignore_imports``
that do not match any imports in the graph. Choices are:
Expand Down
12 changes: 11 additions & 1 deletion src/importlinter/domain/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ class ImportExpressionField(Field):
In addition, it handles wildcards:
"mypackage.*.importer -> mypackage.bar.*"
"mypackage.**.importer -> mypackage.bar.**"
"""

def parse(self, raw_data: Union[str, List]) -> ImportExpression:
Expand All @@ -180,9 +181,18 @@ def parse(self, raw_data: Union[str, List]) -> ImportExpression:
return ImportExpression(importer=importer, imported=imported)

def _validate_wildcard(self, expression: str) -> None:
last_wildcard = None
for part in expression.split("."):
if len(part) > 1 and "*" in part:
if "**" == last_wildcard and ("*" == part or "**" == part):
raise ValidationError("A recursive wildcard cannot be followed by a wildcard.")
if "*" == last_wildcard and "**" == part:
raise ValidationError("A wildcard cannot be followed by a recursive wildcard.")
if "*" == part or "**" == part:
last_wildcard = part
continue
if "*" in part:
raise ValidationError("A wildcard can only replace a whole module.")
last_wildcard = None


class EnumField(Field):
Expand Down
2 changes: 2 additions & 0 deletions src/importlinter/domain/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ def _to_pattern(expression: str) -> Pattern:
for part in expression.split("."):
if "*" == part:
pattern_parts.append(part.replace("*", r"[^\.]+"))
elif "**" == part:
pattern_parts.append(part.replace("*", r"[^\.]+(?:\.[^\.]+)*?"))
else:
pattern_parts.append(part)
return re.compile(r"^" + r"\.".join(pattern_parts) + r"$")
Expand Down
59 changes: 58 additions & 1 deletion tests/unit/contracts/test_forbidden.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,14 @@ def test_is_broken_when_forbidden_modules_imported(self):
"imported": "mypackage.green.beta",
"line_numbers": (3,),
}
]
],
[
{
"importer": "mypackage.one.alpha.circle",
"imported": "mypackage.green.beta.sphere",
"line_numbers": (8,),
},
],
],
},
{
Expand Down Expand Up @@ -153,6 +160,48 @@ def test_ignore_imports_with_wildcards(self):
ignore_imports=("mypackage.*.alpha -> mypackage.*.beta",),
)

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,),
},
]
],
},
],
}

def test_ignore_imports_with_recursive_wildcards(self):
graph = self._build_graph()
contract = self._build_contract(
forbidden_modules=("mypackage.green",),
ignore_imports=(
"mypackage.**.alpha -> mypackage.**.beta",
"mypackage.**.circle -> mypackage.**.sphere",
),
)

check = contract.check(graph=graph, verbose=False)
assert check.metadata == {
"invalid_chains": [
Expand Down Expand Up @@ -251,11 +300,13 @@ def _build_graph(self):
for module in (
"one",
"one.alpha",
"one.alpha.circle",
"two",
"three",
"blue",
"green",
"green.beta",
"green.beta.sphere",
"yellow",
"purple",
"utils",
Expand All @@ -269,6 +320,12 @@ def _build_graph(self):
line_number=3,
line_contents="foo",
)
graph.add_import(
importer="mypackage.one.alpha.circle",
imported="mypackage.green.beta.sphere",
line_number=8,
line_contents="foo",
)
graph.add_import(
importer="mypackage.three",
imported="mypackage.green",
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/contracts/test_independence.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,8 +422,11 @@ def test_extra_firsts_and_lasts(self):
(["mypackage.indirect -> mypackage.b"], True),
# Wildcards
(["mypackage.a -> *.irrelevant"], False),
(["mypackage.a -> **.irrelevant"], False),
(["*.a -> *.indirect"], True),
(["**.a -> **.indirect"], True),
(["mypackage.* -> mypackage.b"], True),
(["mypackage.** -> mypackage.b"], True),
),
)
def test_ignore_imports(ignore_imports, is_kept):
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/contracts/test_layers.py
Original file line number Diff line number Diff line change
Expand Up @@ -776,13 +776,21 @@ class TestIgnoreImports:
"mypackage.low.black -> mypackage.medium.orange",
# Wildcards.
"*.low.black -> mypackage.medium.orange",
"**.low.black -> mypackage.medium.orange",
"mypackage.*.black -> mypackage.medium.orange",
"mypackage.**.black -> mypackage.medium.orange",
"mypackage.low.* -> mypackage.medium.orange",
"mypackage.low.** -> mypackage.medium.orange",
"mypackage.low.black -> *.medium.orange",
"mypackage.low.black -> **.medium.orange",
"mypackage.low.black -> mypackage.*.orange",
"mypackage.low.black -> mypackage.**.orange",
"mypackage.low.black -> mypackage.medium.*",
"mypackage.low.black -> mypackage.medium.**",
"mypackage.*.black -> mypackage.*.orange",
"mypackage.**.black -> mypackage.**.orange",
"mypackage.*.* -> mypackage.*.*",
"mypackage.** -> mypackage.**",
],
)
def test_one_ignored_from_each_chain_means_contract_is_kept(self, expression):
Expand Down
34 changes: 33 additions & 1 deletion tests/unit/domain/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,22 @@ class TestModuleField(BaseFieldTest):
"*.*.* -> mypackage.*.foo.*",
ImportExpression(importer="*.*.*", imported="mypackage.*.foo.*"),
),
(
"mypackage.foo.** -> mypackage.bar",
ImportExpression(importer="mypackage.foo.**", imported="mypackage.bar"),
),
(
"mypackage.foo.**.baz -> mypackage.bar",
ImportExpression(importer="mypackage.foo.**.baz", imported="mypackage.bar"),
),
(
"mypackage.foo -> mypackage.bar.**",
ImportExpression(importer="mypackage.foo", imported="mypackage.bar.**"),
),
(
"** -> mypackage.**.foo.*",
ImportExpression(importer="**", imported="mypackage.**.foo.*"),
),
# Invalid expressions
# -------------------
(
Expand All @@ -134,7 +150,23 @@ class TestModuleField(BaseFieldTest):
ValidationError("A wildcard can only replace a whole module."),
),
(
"mypackage.**.bar -> mypackage.baz",
"mypackage.foo.bar** -> mypackage.bar",
ValidationError("A wildcard can only replace a whole module."),
),
(
"mypackage.**.*.foo -> mypackage.bar",
ValidationError("A recursive wildcard cannot be followed by a wildcard."),
),
(
"mypackage.**.**.foo -> mypackage.bar",
ValidationError("A recursive wildcard cannot be followed by a wildcard."),
),
(
"mypackage.*.**.foo -> mypackage.bar",
ValidationError("A wildcard cannot be followed by a recursive wildcard."),
),
(
"mypackage.foo.b**z -> mypackage.bar",
ValidationError("A wildcard can only replace a whole module."),
),
),
Expand Down
28 changes: 28 additions & 0 deletions tests/unit/domain/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,34 @@ class TestResolveImportExpressions:
],
set(DIRECT_IMPORTS[3:5]),
),
(
"Importer recursive wildcard",
[
ImportExpression(importer="mypackage.**", imported="mypackage.blue"),
],
{DIRECT_IMPORTS[1]},
),
(
"Imported recursive wildcard",
[
ImportExpression(importer="mypackage.green", imported="mypackage.**"),
],
set(DIRECT_IMPORTS[0:2]),
),
(
"Importer and imported recursive wildcards",
[
ImportExpression(importer="mypackage.**", imported="mypackage.**"),
],
set(DIRECT_IMPORTS[0:6]),
),
(
"Inner recursive wildcard",
[
ImportExpression(importer="mypackage.**.cats", imported="mypackage.**.dogs"),
],
set(DIRECT_IMPORTS[3:5]),
),
(
"Multiple expressions, non-overlapping",
[
Expand Down
4 changes: 4 additions & 0 deletions tests/unit/domain/test_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ def test_equality(self, first, second, expected):
("mypackage.foo", "mypackage.*", True),
("mypackage.*", "mypackage.*", True),
("mypackage.*.foo", "mypackage.*.bar", True),
("mypackage.**", "mypackage.bar", True),
("mypackage.foo", "mypackage.**", True),
("mypackage.**", "mypackage.**", True),
("mypackage.**.foo", "mypackage.**.bar", True),
],
)
def test_has_wildcard_expression(self, importer, imported, has_wildcard_expression):
Expand Down

0 comments on commit a181b6f

Please sign in to comment.