Repository: jwbargsten/pytest-archon (language: python)

Issue:
Add regex matching
Currently we only allow fnmatch-style patterns. Perhaps we can add a `is_regex` flag to use regex matching


Discussion / hints:

Fix patch:
diff --git a/README.md b/README.md
index b127131..42acde1 100644
--- a/README.md
+++ b/README.md
@@ -20,8 +20,8 @@ pip install pytest-archon
 
 ## Usage
 
-_pytest-archon_ can be used to define architectural boundaries from (unit) tests. Because
-they're tests, they can be closely tied to the actual application.
+_pytest-archon_ can be used to define architectural boundaries from (unit) tests.
+Because they're tests, they can be closely tied to the actual application.
 
 You can use _pytest-archon_ in tests by simply importing the `archrule` function. Using
 this function you can construct import tests:
@@ -42,29 +42,36 @@ def test_rule_basic():
 ```
 
 - To match the modules and constraints,
-  [fnmatch](https://docs.python.org/3/library/fnmatch.html) syntax is used.
+  [fnmatch](https://docs.python.org/3/library/fnmatch.html) syntax is used (the
+  default). You can also use
+  [regular expressions](https://docs.python.org/3/library/re.html#regular-expression-syntax)
+  by supplying the `use_regex=True` argument to `archrule()`. Example: `archrule(..., use_regex=True).match(...)`.
 - `.exclude()` is optional
 - `.should_import()` and `.should_not_import()` can be combined and can occur multiple
   times.
 - `.may_import()` can be used in combination with `.should_not_import()`.
 - `.check()` needs either a module object or a string
 
-The `check()` method can have a few optional parameters, that alter the way the checks are performed.
-
-* Without parameters, the whole file is checked for imports. So imports done in functions and methods are also found. Transitive dependencies are also checked
-* Option `only_toplevel_imports=True` will only check for toplevel imports. Conditional imports and import in functions and methods are ignored.
-* `skip_type_checking=True` will check all imports, but skip imports defined in `if typing.TYPE_CHECKING` blocks.
-* `only_direct_imports=True` will only check for imports performed by the module directly and will not check transitive imports.
-* If  `only_toplevel_imports=True`  is set, `skip_type_checking=True` has no effect.
-* Options can be combined.
-
-|   | Check toplevel imports | Check `TYPE_CHECKING` imports | Check conditional imports, and imports in functions and methods | Check transitive imports |
-|---|:---:|:---:|:---:|:---:|
-| no options enabled | ✓  | ✓ | ✓  | ✓  |
-| `skip_type_checking=True` | ✓  | ✗ | ✓  | ✓  |
-| `only_toplevel_imports=True` | ✓  | ✗ | ✗ | ✓  |
-| `only_direct_imports=True` | ✓  | ✓  | ✓  | ✗ |
-
+The `check()` method can have a few optional parameters, that alter the way the checks
+are performed.
+
+- Without parameters, the whole file is checked for imports. So imports done in
+  functions and methods are also found. Transitive dependencies are also checked
+- Option `only_toplevel_imports=True` will only check for toplevel imports. Conditional
+  imports and import in functions and methods are ignored.
+- `skip_type_checking=True` will check all imports, but skip imports defined in
+  `if typing.TYPE_CHECKING` blocks.
+- `only_direct_imports=True` will only check for imports performed by the module
+  directly and will not check transitive imports.
+- If `only_toplevel_imports=True` is set, `skip_type_checking=True` has no effect.
+- Options can be combined.
+
+|                              | Check toplevel imports | Check `TYPE_CHECKING` imports | Check conditional imports, and imports in functions and methods | Check transitive imports |
+| ---------------------------- | :--------------------: | :---------------------------: | :-------------------------------------------------------------: | :----------------------: |
+| no options enabled           |           ✓            |               ✓               |                                ✓                                |            ✓             |
+| `skip_type_checking=True`    |           ✓            |               ✗               |                                ✓                                |            ✓             |
+| `only_toplevel_imports=True` |           ✓            |               ✗               |                                ✗                                |            ✓             |
+| `only_direct_imports=True`   |           ✓            |               ✓               |                                ✓                                |            ✗             |
 
 ## Example
 
@@ -87,4 +94,5 @@ def test_domain():
 - [Dependency Cruiser](https://github.com/sverweij/dependency-cruiser) (Javascript)
 - [import-linter](https://github.com/seddonym/import-linter) (Python)
 - [pytestarch](https://pypi.org/project/pytestarch/) (Python)
-- [Maintain A Clean Architecture With Dependency Rules - sourcery.ai](https://sourcery.ai/blog/dependency-rules/) (Python)
+- [Maintain A Clean Architecture With Dependency Rules - sourcery.ai](https://sourcery.ai/blog/dependency-rules/)
+  (Python)


Test patch:
diff --git a/pytest_archon/rule.py b/pytest_archon/rule.py
index baa86e0..87975eb 100644
--- a/pytest_archon/rule.py
+++ b/pytest_archon/rule.py
@@ -1,6 +1,8 @@
 from __future__ import annotations
+from dataclasses import dataclass
+import re
 
-from fnmatch import fnmatch
+from fnmatch import fnmatchcase
 from types import ModuleType
 from typing import Iterable, Sequence
 
@@ -8,40 +10,64 @@
 from pytest_archon.failure import add_failure  # type: ignore[import]
 
 
-def archrule(name: str, comment: str | None = None) -> Rule:
+@dataclass
+class RulePattern:
+    is_regex: bool
+    pattern: str
+
+    def match(self, k: str):
+        if self.is_regex:
+            return re.search(self.pattern, k)
+        else:
+            return fnmatchcase(k, self.pattern)
+
+    def __str__(self):
+        if self.is_regex:
+            return f"regex pattern /{self.pattern}/"
+        else:
+            return f"glob pattern /{self.pattern}/"
+
+
+def _as_rule_patterns(use_regex, patterns):
+    return [RulePattern(is_regex=use_regex, pattern=p) for p in patterns]
+
+
+def archrule(name: str, comment: str | None = None, *, use_regex: bool = False) -> Rule:
     """Define a new architectural rule with a name and an optional comment."""
-    return Rule(name, comment=comment)
+    return Rule(name, comment=comment, use_regex=use_regex)
 
 
 # https://peps.python.org/pep-0451/
 # the path is the package path: where the submodules are in
 class Rule:
-    def __init__(self, name: str, comment: str | None) -> None:
+    def __init__(self, name: str, comment: str | None, use_regex: bool = False) -> None:
         """Define a new architectural rule with a name and a comment."""
         self.name = name
         self.comment = comment
+        self.use_regex = use_regex
 
-    def match(self, *pattern: str) -> RuleTargets:
+    def match(self, *pattern: str, **kwargs) -> RuleTargets:
         """A glob pattern for modules this rule should match."""
-        return RuleTargets(self).match(*pattern)
+        return RuleTargets(self).match(*pattern, **kwargs)
 
-    def exclude(self, *pattern: str) -> RuleTargets:
+    def exclude(self, *pattern: str, **kwargs) -> RuleTargets:
         """A glob pattern for modules this rule should exclude from matching.
 
         Exclusion takes precedence of matching.
         """
-        return RuleTargets(self).exclude(*pattern)
+        return RuleTargets(self).exclude(*pattern, **kwargs)
 
 
 class RuleTargets:
     def __init__(self, rule: Rule) -> None:
         self.rule = rule
-        self.match_criteria: list[str] = []
-        self.exclude_criteria: list[str] = []
+        self.match_criteria: list[RulePattern] = []
+        self.exclude_criteria: list[RulePattern] = []
 
     def match(self, *pattern: str) -> RuleTargets:
         """A glob pattern for modules this rule should match."""
-        self.match_criteria.extend(pattern)
+
+        self.match_criteria.extend(_as_rule_patterns(self.rule.use_regex, pattern))
         return self
 
     def exclude(self, *pattern: str) -> RuleTargets:
@@ -49,10 +75,10 @@ def exclude(self, *pattern: str) -> RuleTargets:
 
         Exclusion takes precedence of matching.
         """
-        self.exclude_criteria.extend(pattern)
+        self.exclude_criteria.extend(_as_rule_patterns(self.rule.use_regex, pattern))
         return self
 
-    def should_not_import(self, *pattern: str) -> RuleConstraints:
+    def should_not_import(self, *pattern: str, **kwargs) -> RuleConstraints:
         """Define a constraint that the defined modules should
         not import modules that match the given pattern.
 
@@ -60,9 +86,9 @@ def should_not_import(self, *pattern: str) -> RuleConstraints:
 
         E.g. 'mymodule.submodule', 'mymodule.*'
         """
-        return RuleConstraints(self.rule, self).should_not_import(*pattern)
+        return RuleConstraints(self.rule, self).should_not_import(*pattern, **kwargs)
 
-    def should_import(self, *pattern: str) -> RuleConstraints:
+    def should_import(self, *pattern: str, **kwargs) -> RuleConstraints:
         """Define a constraint that the defined modules should
         import modules that match the given pattern.
 
@@ -70,23 +96,23 @@ def should_import(self, *pattern: str) -> RuleConstraints:
 
         E.g. 'mymodule.submodule', 'mymodule.*'
         """
-        return RuleConstraints(self.rule, self).should_import(*pattern)
+        return RuleConstraints(self.rule, self).should_import(*pattern, **kwargs)
 
-    def may_import(self, *pattern: str) -> RuleConstraints:
+    def may_import(self, *pattern: str, **kwargs) -> RuleConstraints:
         """Loosen the constraints from should_import and
         should_not_import: modules matching may_import are
         excluded/ignored from the constraint check.
         """
-        return RuleConstraints(self.rule, self).may_import(*pattern)
+        return RuleConstraints(self.rule, self).may_import(*pattern, **kwargs)
 
 
 class RuleConstraints:
     def __init__(self, rule: Rule, targets: RuleTargets) -> None:
         self.rule = rule
         self.targets = targets
-        self.forbidden: list[str] = []
-        self.required: list[str] = []
-        self.ignored: list[str] = []
+        self.forbidden: list[RulePattern] = []
+        self.required: list[RulePattern] = []
+        self.ignored: list[RulePattern] = []
 
     def should_not_import(self, *pattern: str) -> RuleConstraints:
         """Define a constraint that the defined modules should
@@ -96,7 +122,7 @@ def should_not_import(self, *pattern: str) -> RuleConstraints:
 
         E.g. 'mymodule.submodule', 'mymodule.*'
         """
-        self.forbidden.extend(pattern)
+        self.forbidden.extend(_as_rule_patterns(self.rule.use_regex, pattern))
         return self
 
     def should_import(self, *pattern: str) -> RuleConstraints:
@@ -107,7 +133,7 @@ def should_import(self, *pattern: str) -> RuleConstraints:
 
         E.g. 'mymodule.submodule', 'mymodule.*'
         """
-        self.required.extend(pattern)
+        self.required.extend(_as_rule_patterns(self.rule.use_regex, pattern))
         return self
 
     def may_import(self, *pattern: str) -> RuleConstraints:
@@ -115,7 +141,7 @@ def may_import(self, *pattern: str) -> RuleConstraints:
         should_not_import: modules matching may_import are
         excluded/ignored from the constraint check.
         """
-        self.ignored.extend(pattern)
+        self.ignored.extend(_as_rule_patterns(self.rule.use_regex, pattern))
         return self
 
     def check(
@@ -156,16 +182,18 @@ def check(
 
         candidates: list[str] = []
         for mp in match_criteria:
-            candidates.extend(k for k in all_imports.keys() if fnmatch(k, mp))
+            candidates.extend(k for k in all_imports.keys() if mp.match(k))
         for ep in exclude_criteria:
-            candidates = [k for k in candidates if not fnmatch(k, ep)]
+            candidates = [k for k in candidates if not ep.match(k)]
 
+        match_criteria_pretty = [str(c) for c in match_criteria]
+        exclude_criteria_pretty = [str(c) for c in exclude_criteria]
         if not candidates:
             add_failure(
                 rule_name,
                 rule_comment,
-                f"NO CANDIDATES MATCHED. Match criteria: {match_criteria}, "
-                f"exclude_criteria: {exclude_criteria}",
+                f"NO CANDIDATES MATCHED. Match criteria: {match_criteria_pretty}, "
+                f"exclude_criteria: {exclude_criteria_pretty}",
             )
             return
 
@@ -178,21 +206,21 @@ def check(
                 add_failure(
                     rule_name,
                     rule_comment,
-                    f"module '{candidate}' is missing REQUIRED imports matching pattern /{constraint}/",
+                    f"module '{candidate}' is missing REQUIRED imports matching {constraint}",
                 )
 
             for constraint, path in self._check_forbidden_constraints(candidate, import_map):
                 add_failure(
                     rule_name,
                     rule_comment,
-                    f"module '{candidate}' has FORBIDDEN import {path[-1]} (matched by /{constraint}/) ",
+                    f"module '{candidate}' has FORBIDDEN import {path[-1]} (matched by {constraint}) ",
                     path,
                 )
 
     def _check_required_constraints(self, module: str, all_imports: ImportMap):
         for constraint in self.required:
             if not any(
-                imp for path in recurse_imports(module, all_imports) if fnmatch(imp := path[-1], constraint)
+                imp for path in recurse_imports(module, all_imports) if constraint.match(imp := path[-1])
             ):
                 yield constraint
 
@@ -205,8 +233,8 @@ def _check_forbidden_constraints(
             yield from (
                 (constraint, path)
                 for path in recurse_imports(module, all_imports)
-                if fnmatch(path[-1], constraint)
-                and not any(fnmatch(m, ignore) for ignore in self.ignored for m in path)
+                if constraint.match(path[-1])
+                and not any(ignore.match(m) for ignore in self.ignored for m in path)
             )
 
 
diff --git a/tests/test_rule.py b/tests/test_rule.py
index 620f3ca..9797f9e 100644
--- a/tests/test_rule.py
+++ b/tests/test_rule.py
@@ -19,6 +19,17 @@ def test_rule_exclusion():
     )
 
 
+def test_rule_exclusion_regex():
+    (
+        archrule("rule exclusion", use_regex=True)
+        .exclude(r"^pytest_archon$")
+        .match(".*")
+        .exclude(r"^pytest_archon\..lugin")
+        .should_not_import(r"^pytest\warchon\.rule$")
+        .check("pytest_archon")
+    )
+
+
 def test_rule_should_import():
     (
         archrule("rule exclusion")
@@ -98,7 +109,9 @@ def test_required_transitive_dependency_fails(create_testset):
     assert failures
 
     assert "FAILED Rule 'rule exclusion':" in longrepr
-    assert "- module 'abcz.moduleA' is missing REQUIRED imports matching pattern /abcz.moduleD/" in longrepr
+    assert (
+        "- module 'abcz.moduleA' is missing REQUIRED imports matching glob pattern /abcz.moduleD/" in longrepr
+    )
 
 
 def test_forbidden_transitive_dependency_fails(create_testset):
@@ -118,5 +131,5 @@ def test_forbidden_transitive_dependency_fails(create_testset):
     assert longrepr
     assert "FAILED Rule 'rule exclusion':" in longrepr
     assert "- module 'abcz.moduleA' has FORBIDDEN import" in longrepr
-    assert "abcz.moduleD (matched by /abcz.moduleD/)" in longrepr
+    assert "abcz.moduleD (matched by glob pattern /abcz.moduleD/)" in longrepr
     assert "abcz.moduleA ↣ abcz.moduleB ↣ abcz.moduleC ↣ abcz.moduleD" in longrepr