From 491c666f1f8a6ef2f5ccd782a4e4c79ed39321f6 Mon Sep 17 00:00:00 2001 From: TristanKruse Date: Tue, 9 Jun 2026 18:28:11 +0200 Subject: [PATCH] feat: add because rule rationales --- BACKLOG.md | 2 +- README.md | 19 ++++++++++++++ src/archunitpython/common/__init__.py | 7 +++++- .../common/fluentapi/__init__.py | 8 ++++-- .../common/fluentapi/checkable.py | 24 +++++++++++++++++- src/archunitpython/files/fluentapi/files.py | 12 ++++----- .../metrics/fluentapi/metrics.py | 14 +++++------ src/archunitpython/slices/fluentapi/slices.py | 6 ++--- src/archunitpython/testing/assertion.py | 14 ++++++++--- tests/files/test_files_fluentapi.py | 19 ++++++++++++++ tests/integration/test_e2e.py | 25 +++++++++++++++++++ 11 files changed, 126 insertions(+), 24 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 6d4a342..6771cb5 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -12,7 +12,7 @@ This backlog collects product and maintenance ideas from project research. ## P1 - Adoption Workflow - Add an `.archignore` or similar file, modeled after `.gitignore`, for files that should never be analyzed. -- Add a `.because(...)` API so rules can carry user-facing rationale into failure messages and generated architecture documentation. +- [x] Add a `.because(...)` API so rules can carry user-facing rationale into failure messages and generated architecture documentation. - Add configuration-file support for common rules, while keeping the fluent Python API as the primary interface. - Add support for monorepo and multi-package Python projects. diff --git a/README.md b/README.md index 9cca0bc..c2ffe5f 100644 --- a/README.md +++ b/README.md @@ -155,6 +155,25 @@ options = CheckOptions( violations = rule.check(options) ``` +### Explaining Rules With `.because(...)` + +Attach a rationale to a rule so failing assertions explain why the rule exists: + +```python +rule = ( + project_files("src/") + .in_folder("**/controllers/**") + .should_not() + .depend_on_files() + .in_folder("**/database/**") + .because("controllers should stay thin and delegate persistence") +) + +assert_passes(rule) +``` + +When the rule fails, the rationale is included in the assertion message. + ## 🐹 Use Cases Here is an overview of common use cases. diff --git a/src/archunitpython/common/__init__.py b/src/archunitpython/common/__init__.py index e945f0a..0c1296d 100644 --- a/src/archunitpython/common/__init__.py +++ b/src/archunitpython/common/__init__.py @@ -1,6 +1,10 @@ from archunitpython.common.assertion.violation import EmptyTestViolation, Violation from archunitpython.common.error.errors import TechnicalError, UserError -from archunitpython.common.fluentapi.checkable import Checkable, CheckOptions +from archunitpython.common.fluentapi.checkable import ( + Checkable, + CheckOptions, + RuleRationaleMixin, +) from archunitpython.common.logging.types import LoggingOptions from archunitpython.common.types import Filter, Pattern, PatternMatchingOptions @@ -11,6 +15,7 @@ "UserError", "Checkable", "CheckOptions", + "RuleRationaleMixin", "LoggingOptions", "Pattern", "Filter", diff --git a/src/archunitpython/common/fluentapi/__init__.py b/src/archunitpython/common/fluentapi/__init__.py index 273eebc..7e8414e 100644 --- a/src/archunitpython/common/fluentapi/__init__.py +++ b/src/archunitpython/common/fluentapi/__init__.py @@ -1,3 +1,7 @@ -from archunitpython.common.fluentapi.checkable import Checkable, CheckOptions +from archunitpython.common.fluentapi.checkable import ( + Checkable, + CheckOptions, + RuleRationaleMixin, +) -__all__ = ["Checkable", "CheckOptions"] +__all__ = ["Checkable", "CheckOptions", "RuleRationaleMixin"] diff --git a/src/archunitpython/common/fluentapi/checkable.py b/src/archunitpython/common/fluentapi/checkable.py index df3cc4b..10f216c 100644 --- a/src/archunitpython/common/fluentapi/checkable.py +++ b/src/archunitpython/common/fluentapi/checkable.py @@ -3,7 +3,7 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Protocol +from typing import Protocol, TypeVar from archunitpython.common.assertion.violation import Violation from archunitpython.common.logging.types import LoggingOptions @@ -19,6 +19,28 @@ class CheckOptions: ignore_type_checking_imports: bool = False +T = TypeVar("T", bound="RuleRationaleMixin") + + +class RuleRationaleMixin: + """Mixin for checkable rules that can carry a human-readable rationale.""" + + _because_reason: str | None = None + + def because(self: T, reason: str) -> T: + """Attach a rationale explaining why the rule exists.""" + reason = reason.strip() + if not reason: + raise ValueError("Rule rationale must not be empty.") + self._because_reason = reason + return self + + @property + def because_reason(self) -> str | None: + """Return the rationale attached with because(), if any.""" + return self._because_reason + + class Checkable(Protocol): """Protocol for any architecture rule that can be checked. diff --git a/src/archunitpython/files/fluentapi/files.py b/src/archunitpython/files/fluentapi/files.py index 9f31036..422ce32 100644 --- a/src/archunitpython/files/fluentapi/files.py +++ b/src/archunitpython/files/fluentapi/files.py @@ -14,7 +14,7 @@ from archunitpython.common.assertion.violation import EmptyTestViolation, Violation from archunitpython.common.extraction.extract_graph import extract_graph -from archunitpython.common.fluentapi.checkable import CheckOptions +from archunitpython.common.fluentapi.checkable import CheckOptions, RuleRationaleMixin from archunitpython.common.pattern_matching import matches_all_patterns from archunitpython.common.projection.edge_projections import ( per_external_edge, @@ -322,7 +322,7 @@ def _check_empty_test( return None -class CycleFreeFileCondition: +class CycleFreeFileCondition(RuleRationaleMixin): """Checkable that verifies no cycles exist among filtered files.""" def __init__(self, project_path: str | None, filters: list[Filter]) -> None: @@ -350,7 +350,7 @@ def check(self, options: CheckOptions | None = None) -> list[Violation]: return gather_cycle_violations(cycles) -class DependOnFileCondition: +class DependOnFileCondition(RuleRationaleMixin): """Checkable that verifies file dependency rules.""" def __init__( @@ -377,7 +377,7 @@ def check(self, options: CheckOptions | None = None) -> list[Violation]: ) -class DependOnExternalModuleCondition: +class DependOnExternalModuleCondition(RuleRationaleMixin): """Checkable that verifies external module dependency rules.""" def __init__( @@ -409,7 +409,7 @@ def check(self, options: CheckOptions | None = None) -> list[Violation]: ) -class MatchPatternFileCondition: +class MatchPatternFileCondition(RuleRationaleMixin): """Checkable that verifies files match/don't match patterns.""" def __init__( @@ -434,7 +434,7 @@ def check(self, options: CheckOptions | None = None) -> list[Violation]: return gather_regex_matching_violations(nodes, self._check_filters, self._is_negated) -class CustomFileCheckableCondition: +class CustomFileCheckableCondition(RuleRationaleMixin): """Checkable that evaluates a custom condition on files.""" def __init__( diff --git a/src/archunitpython/metrics/fluentapi/metrics.py b/src/archunitpython/metrics/fluentapi/metrics.py index 2824cb4..2e41f7b 100644 --- a/src/archunitpython/metrics/fluentapi/metrics.py +++ b/src/archunitpython/metrics/fluentapi/metrics.py @@ -11,7 +11,7 @@ from typing import Any, Callable from archunitpython.common.assertion.violation import Violation -from archunitpython.common.fluentapi.checkable import CheckOptions +from archunitpython.common.fluentapi.checkable import CheckOptions, RuleRationaleMixin from archunitpython.common.pattern_matching import matches_pattern_classname from archunitpython.common.regex_factory import RegexFactory from archunitpython.common.types import Filter, Pattern @@ -170,7 +170,7 @@ def should_be_above_or_equal(self, threshold: float) -> "ClassMetricCondition": ) -class ClassMetricCondition: +class ClassMetricCondition(RuleRationaleMixin): """Checkable that verifies a class-level metric threshold.""" def __init__( @@ -230,7 +230,7 @@ def should_be_below_or_equal(self, threshold: float) -> "FileMetricCondition": ) -class FileMetricCondition: +class FileMetricCondition(RuleRationaleMixin): """Checkable that verifies a file-level metric threshold.""" def __init__( @@ -364,7 +364,7 @@ def should_be_above(self, threshold: float) -> "DistanceCondition": ) -class DistanceCondition: +class DistanceCondition(RuleRationaleMixin): """Checkable for distance metric thresholds.""" def __init__( @@ -404,7 +404,7 @@ def check(self, options: CheckOptions | None = None) -> list[Violation]: return violations -class ZoneCondition: +class ZoneCondition(RuleRationaleMixin): """Checkable for zone detection (pain/uselessness).""" def __init__(self, project_path: str | None, filters: list[Filter], zone_type: str) -> None: @@ -485,7 +485,7 @@ def should_satisfy( ) -class CustomMetricCondition: +class CustomMetricCondition(RuleRationaleMixin): """Checkable for custom metric thresholds.""" def __init__( @@ -525,7 +525,7 @@ def check(self, options: CheckOptions | None = None) -> list[Violation]: return violations -class CustomAssertionCondition: +class CustomAssertionCondition(RuleRationaleMixin): """Checkable for custom metric assertions.""" def __init__( diff --git a/src/archunitpython/slices/fluentapi/slices.py b/src/archunitpython/slices/fluentapi/slices.py index 31d2d28..70b0dbb 100644 --- a/src/archunitpython/slices/fluentapi/slices.py +++ b/src/archunitpython/slices/fluentapi/slices.py @@ -14,7 +14,7 @@ from archunitpython.common.assertion.violation import Violation from archunitpython.common.extraction.extract_graph import extract_graph -from archunitpython.common.fluentapi.checkable import CheckOptions +from archunitpython.common.fluentapi.checkable import CheckOptions, RuleRationaleMixin from archunitpython.common.projection.project_edges import project_edges from archunitpython.common.projection.types import MapFunction from archunitpython.slices.assertion.admissible_edges import ( @@ -140,7 +140,7 @@ def contain_dependency(self, source: str, target: str) -> "NegativeSliceConditio ) -class PositiveSliceCondition: +class PositiveSliceCondition(RuleRationaleMixin): """Checkable that verifies slices adhere to a diagram.""" def __init__( @@ -176,7 +176,7 @@ def _get_mapper(self) -> MapFunction: return identity() -class NegativeSliceCondition: +class NegativeSliceCondition(RuleRationaleMixin): """Checkable that verifies a specific dependency does NOT exist.""" def __init__( diff --git a/src/archunitpython/testing/assertion.py b/src/archunitpython/testing/assertion.py index 8bb5795..900c2b8 100644 --- a/src/archunitpython/testing/assertion.py +++ b/src/archunitpython/testing/assertion.py @@ -7,7 +7,11 @@ from archunitpython.testing.common.violation_factory import ViolationFactory -def format_violations(violations: list[Violation]) -> str: +def format_violations( + violations: list[Violation], + *, + because: str | None = None, +) -> str: """Format violations into a human-readable string. Args: @@ -19,7 +23,10 @@ def format_violations(violations: list[Violation]) -> str: if not violations: return "No violations found." - lines = [f"Found {len(violations)} architecture violation(s):", ""] + lines = [f"Found {len(violations)} architecture violation(s):"] + if because: + lines.extend(["", f"Because: {because}"]) + lines.append("") for i, violation in enumerate(violations, 1): tv = ViolationFactory.from_violation(violation) lines.append(f" {i}. {tv.message}") @@ -44,4 +51,5 @@ def assert_passes( """ violations = checkable.check(options) if violations: - raise AssertionError(format_violations(violations)) + because = getattr(checkable, "because_reason", None) + raise AssertionError(format_violations(violations, because=because)) diff --git a/tests/files/test_files_fluentapi.py b/tests/files/test_files_fluentapi.py index 77cac5a..f084294 100644 --- a/tests/files/test_files_fluentapi.py +++ b/tests/files/test_files_fluentapi.py @@ -5,6 +5,8 @@ from pathlib import Path from uuid import uuid4 +import pytest + from archunitpython.common.assertion.violation import EmptyTestViolation from archunitpython.common.extraction.extract_graph import clear_graph_cache from archunitpython.files.assertion.custom_file_logic import CustomFileViolation @@ -223,6 +225,23 @@ def test_builder_with_multiple_filters(self): cycle_violations = [v for v in violations if isinstance(v, ViolatingCycle)] assert len(cycle_violations) == 0 + def test_because_adds_rule_rationale(self): + rule = ( + project_files(FIXTURES_DIR) + .in_folder("**/services*") + .should() + .have_no_cycles() + .because("service cycles are hard to refactor") + ) + + assert rule.because_reason == "service cycles are hard to refactor" + + def test_because_rejects_empty_rationale(self): + rule = project_files(FIXTURES_DIR).should().have_no_cycles() + + with pytest.raises(ValueError, match="must not be empty"): + rule.because(" ") + class TestTypeCheckingImports: def setup_method(self): diff --git a/tests/integration/test_e2e.py b/tests/integration/test_e2e.py index 6a440ab..c76e3ae 100644 --- a/tests/integration/test_e2e.py +++ b/tests/integration/test_e2e.py @@ -83,6 +83,19 @@ def test_failing_rule_raises(self): with pytest.raises(AssertionError, match="architecture violation"): assert_passes(rule) + def test_failing_rule_includes_because_rationale(self): + rule = ( + project_files(FIXTURES_DIR) + .in_folder("**/controllers*") + .should_not() + .depend_on_files() + .in_folder("**/services*") + .because("controllers should stay thin") + ) + + with pytest.raises(AssertionError, match="Because: controllers should stay thin"): + assert_passes(rule) + class TestFormatViolations: def test_no_violations(self): @@ -102,6 +115,18 @@ def test_with_violations(self): assert "1 architecture violation" in result assert "Circular dependency" in result + def test_with_because_rationale(self): + from archunitpython.common.projection.types import ProjectedEdge + + violation = ViolatingCycle( + cycle=[ + ProjectedEdge(source_label="a.py", target_label="b.py"), + ProjectedEdge(source_label="b.py", target_label="a.py"), + ] + ) + result = format_violations([violation], because="cycles make changes risky") + assert "Because: cycles make changes risky" in result + class TestSelfTesting: """ArchUnitPython tests its own architecture."""