From 53d9948d77287dbce3ab253ae41149e179870687 Mon Sep 17 00:00:00 2001 From: heumsi Date: Wed, 1 Apr 2026 00:47:20 +0900 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat:=20Add=20optional=20descriptio?= =?UTF-8?q?n=20field=20to=20rules?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an optional `description` field to rules that is shown in violation output, helping users understand why a rule exists and why a violation was reported. The violation output format is updated so the arrow line is always on its own line for consistency. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/getting-started/configuration.md | 2 ++ docs/guide/rules.md | 5 ++- docs/reference/config-schema.md | 1 + python_dependency_linter/checker.py | 4 +++ python_dependency_linter/config.py | 2 ++ python_dependency_linter/matcher.py | 1 + python_dependency_linter/reporter.py | 6 +++- tests/test_checker.py | 37 +++++++++++++++++++++ tests/test_config.py | 48 +++++++++++++++++++++++++++ tests/test_matcher.py | 17 ++++++++++ tests/test_reporter.py | 45 +++++++++++++++++++++++-- 11 files changed, 164 insertions(+), 4 deletions(-) diff --git a/docs/getting-started/configuration.md b/docs/getting-started/configuration.md index e4e308c..c75e2a4 100644 --- a/docs/getting-started/configuration.md +++ b/docs/getting-started/configuration.md @@ -25,6 +25,7 @@ Create `.python-dependency-linter.yaml` in your project root: rules: - name: domain-isolation modules: contexts.*.domain + description: Domain layer must remain pure # optional allow: standard_library: [dataclasses, typing] third_party: [pydantic] @@ -39,6 +40,7 @@ You can embed the same configuration inside `pyproject.toml` using the `[tool.py [[tool.python-dependency-linter.rules]] name = "domain-isolation" modules = "contexts.*.domain" +description = "Domain layer must remain pure" # optional [tool.python-dependency-linter.rules.allow] standard_library = ["dataclasses", "typing"] diff --git a/docs/guide/rules.md b/docs/guide/rules.md index dde4e91..752998a 100644 --- a/docs/guide/rules.md +++ b/docs/guide/rules.md @@ -10,11 +10,12 @@ Every rule has two required fields and two optional ones: rules: - name: my-rule # Unique identifier for this rule modules: my_app.domain # Which modules this rule applies to + description: ... # (optional) Human-readable description allow: { ... } # (optional) Whitelist of allowed dependencies deny: { ... } # (optional) Blacklist of denied dependencies ``` -The `name` is used in violation output and in `# pdl: ignore` comments. +The `name` is used in violation output and in `# pdl: ignore` comments. The optional `description` is also shown in violation output to explain why the rule exists. ### Fields @@ -22,6 +23,7 @@ The `name` is used in violation output and in `# pdl: ignore` comments. |-------|----------|-------------| | `name` | Yes | Unique identifier. Must match `[a-zA-Z0-9_-]+`. Shown in violation output and referenced in `# pdl: ignore` comments | | `modules` | Yes | Module pattern to apply the rule to. Supports `*`, `**`, and `{name}` captures (see [Patterns](#patterns)) | +| `description` | No | Human-readable description shown in violation output | | `allow` | No | Whitelist of allowed dependencies (see [Allow / Deny](#allow-deny)) | | `deny` | No | Blacklist of denied dependencies (see [Allow / Deny](#allow-deny)) | @@ -335,6 +337,7 @@ from contexts.shared.utils import helper # pdl: ignore |-------|----------|-------------| | `name` | Yes | Unique identifier, shown in output and referenced in `# pdl: ignore` | | `modules` | Yes | Module pattern to apply the rule to | +| `description` | No | Human-readable description shown in violation output | | `allow` | No | Whitelist of allowed dependencies | | `deny` | No | Blacklist of denied dependencies | diff --git a/docs/reference/config-schema.md b/docs/reference/config-schema.md index fee833b..fccee53 100644 --- a/docs/reference/config-schema.md +++ b/docs/reference/config-schema.md @@ -16,6 +16,7 @@ Complete reference for all configuration fields. |-------|------|----------|---------|-------------| | `name` | str | Yes | — | Rule identifier, shown in violation output. Must match `[a-zA-Z0-9_-]+` | | `modules` | str | Yes | — | Module pattern to apply the rule to. Supports `*`, `**`, and `{name}` captures | +| `description` | str | No | `null` | Human-readable description shown in violation output | | `allow` | [DependencyFilter](#dependencyfilter) | No | `null` | Whitelist of allowed dependencies | | `deny` | [DependencyFilter](#dependencyfilter) | No | `null` | Blacklist of denied dependencies | diff --git a/python_dependency_linter/checker.py b/python_dependency_linter/checker.py index 1bb4294..a95eef7 100644 --- a/python_dependency_linter/checker.py +++ b/python_dependency_linter/checker.py @@ -46,6 +46,7 @@ class Violation: imported_module: str category: ImportCategory lineno: int + rule_description: str | None = None def _get_category_list( @@ -100,6 +101,7 @@ def check_import( merged_rule = Rule( name=merged_rule.name, modules=merged_rule.modules, + description=merged_rule.description, allow=_resolve_allow_deny(merged_rule.allow, captures), deny=_resolve_allow_deny(merged_rule.deny, captures), ) @@ -115,6 +117,7 @@ def check_import( imported_module=module, category=category, lineno=import_info.lineno, + rule_description=merged_rule.description, ) # Check allow @@ -131,4 +134,5 @@ def check_import( imported_module=module, category=category, lineno=import_info.lineno, + rule_description=merged_rule.description, ) diff --git a/python_dependency_linter/config.py b/python_dependency_linter/config.py index 92e7253..61916f0 100644 --- a/python_dependency_linter/config.py +++ b/python_dependency_linter/config.py @@ -18,6 +18,7 @@ class AllowDeny: class Rule: name: str modules: str + description: str | None = None allow: AllowDeny | None = None deny: AllowDeny | None = None @@ -54,6 +55,7 @@ def _parse_rules(rules_data: list[dict]) -> list[Rule]: Rule( name=name, modules=r["modules"], + description=r.get("description"), allow=_parse_allow_deny(r.get("allow")), deny=_parse_allow_deny(r.get("deny")), ) diff --git a/python_dependency_linter/matcher.py b/python_dependency_linter/matcher.py index eeb7b6f..49724d3 100644 --- a/python_dependency_linter/matcher.py +++ b/python_dependency_linter/matcher.py @@ -121,6 +121,7 @@ def merge_rules(rules: list[Rule]) -> Rule: merged = Rule( name=merged.name, modules=merged.modules, + description=merged.description, allow=_merge_allow_deny(merged.allow, rule.allow), deny=_merge_allow_deny(merged.deny, rule.deny), ) diff --git a/python_dependency_linter/reporter.py b/python_dependency_linter/reporter.py index b38da41..ac7795e 100644 --- a/python_dependency_linter/reporter.py +++ b/python_dependency_linter/reporter.py @@ -10,8 +10,12 @@ def format_violations(file_path: str, violations: list[Violation]) -> str: lines = [] for v in violations: lines.append(f"{file_path}:{v.lineno}") + if v.rule_description: + lines.append(f" [{v.rule_name}] {v.rule_description}") + else: + lines.append(f" [{v.rule_name}]") arrow = f"{v.source_module} \u2192 {v.imported_module}" - lines.append(f" [{v.rule_name}] {arrow} ({v.category.value})") + lines.append(f" {arrow} ({v.category.value})") lines.append("") return "\n".join(lines) diff --git a/tests/test_checker.py b/tests/test_checker.py index dc288d8..9baa4a6 100644 --- a/tests/test_checker.py +++ b/tests/test_checker.py @@ -47,6 +47,7 @@ def test_allow_whitelist_violation(): assert result.imported_module == "sqlalchemy" assert result.category == ImportCategory.THIRD_PARTY assert result.lineno == 5 + assert result.rule_description is None def test_deny_blacklist_violation(): @@ -223,3 +224,39 @@ def test_check_import_no_captures_backward_compat(): source_module="contexts.boards.domain", ) assert result is None + + +def test_violation_includes_description(): + """Violation should carry rule description when present.""" + rule = Rule( + name="domain-isolation", + modules="contexts.*.domain", + description="Domain layer must remain pure", + allow=AllowDeny(third_party=["pydantic"]), + ) + result = check_import( + import_info=ImportInfo(module="sqlalchemy", lineno=5), + category=ImportCategory.THIRD_PARTY, + merged_rule=rule, + source_module="contexts.boards.domain", + ) + assert isinstance(result, Violation) + assert result.rule_description == "Domain layer must remain pure" + + +def test_deny_violation_includes_description(): + """Deny violation should also carry rule description.""" + rule = Rule( + name="adapters-deny", + modules="contexts.*.adapters", + description="Adapters must not use boto3 directly", + deny=AllowDeny(third_party=["boto3"]), + ) + result = check_import( + import_info=ImportInfo(module="boto3", lineno=3), + category=ImportCategory.THIRD_PARTY, + merged_rule=rule, + source_module="contexts.boards.adapters", + ) + assert isinstance(result, Violation) + assert result.rule_description == "Adapters must not use boto3 directly" diff --git a/tests/test_config.py b/tests/test_config.py index bad4faf..2042489 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -14,6 +14,7 @@ def test_load_yaml_config(): rule = config.rules[0] assert rule.name == "domain-isolation" assert rule.modules == "contexts.*.domain" + assert rule.description is None assert rule.allow is not None assert rule.allow.standard_library == ["dataclasses", "typing"] assert rule.allow.third_party == ["pydantic"] @@ -181,3 +182,50 @@ def test_invalid_rule_name_with_mixed(tmp_path): config_file.write_text(config_content) with pytest.raises(ValueError, match=r"Invalid rule name 'rule name 123'"): load_config(config_file) + + +def test_load_yaml_config_with_description(tmp_path): + config_content = """\ +rules: + - name: domain-isolation + modules: contexts.*.domain + description: Domain layer must remain pure + allow: + third_party: [pydantic] +""" + config_file = tmp_path / "config.yaml" + config_file.write_text(config_content) + config = load_config(config_file) + assert config.rules[0].description == "Domain layer must remain pure" + + +def test_load_yaml_config_without_description(tmp_path): + config_content = """\ +rules: + - name: domain-isolation + modules: contexts.*.domain + allow: + third_party: [pydantic] +""" + config_file = tmp_path / "config.yaml" + config_file.write_text(config_content) + config = load_config(config_file) + assert config.rules[0].description is None + + +def test_load_toml_config_with_description(tmp_path): + config_content = """\ +[tool.python-dependency-linter] + +[[tool.python-dependency-linter.rules]] +name = "domain-isolation" +modules = "contexts.*.domain" +description = "Domain layer must remain pure" + +[tool.python-dependency-linter.rules.allow] +third_party = ["pydantic"] +""" + config_file = tmp_path / "pyproject.toml" + config_file.write_text(config_content) + config = load_config(config_file) + assert config.rules[0].description == "Domain layer must remain pure" diff --git a/tests/test_matcher.py b/tests/test_matcher.py index 3945f6f..dc3bc27 100644 --- a/tests/test_matcher.py +++ b/tests/test_matcher.py @@ -109,6 +109,23 @@ def test_merge_rules_single(): assert merged.allow.third_party == ["pydantic"] +def test_merge_rules_preserves_description(): + rule1 = Rule( + name="r1", + modules="contexts.*.domain", + description="First rule description", + allow=AllowDeny(third_party=["pydantic"]), + ) + rule2 = Rule( + name="r2", + modules="contexts.boards.domain", + description="Second rule description", + allow=AllowDeny(third_party=["attrs"]), + ) + merged = merge_rules([rule1, rule2]) + assert merged.description == "First rule description" + + def test_merge_rules_merges_deny(): rule1 = Rule( name="r1", modules="contexts.*.adapters", deny=AllowDeny(third_party=["boto3"]) diff --git a/tests/test_reporter.py b/tests/test_reporter.py index de984ea..d647ee1 100644 --- a/tests/test_reporter.py +++ b/tests/test_reporter.py @@ -3,7 +3,7 @@ from python_dependency_linter.resolver import ImportCategory -def test_format_violations(): +def test_format_violations_without_description(): violations = [ Violation( rule_name="domain-isolation", @@ -19,10 +19,51 @@ def test_format_violations(): assert "contexts/boards/domain/models.py:6" in output assert "[domain-isolation]" in output assert ( - "contexts.boards.domain → contexts.boards.application.service (local)" in output + "contexts.boards.domain \u2192 contexts.boards.application.service (local)" + in output ) +def test_format_violations_with_description(): + violations = [ + Violation( + rule_name="domain-isolation", + source_module="contexts.boards.domain", + imported_module="contexts.boards.application.service", + category=ImportCategory.LOCAL, + lineno=6, + rule_description="Domain layer must remain pure", + ), + ] + file_path = "contexts/boards/domain/models.py" + output = format_violations(file_path, violations) + + assert "contexts/boards/domain/models.py:6" in output + assert "[domain-isolation] Domain layer must remain pure" in output + assert ( + "contexts.boards.domain \u2192 contexts.boards.application.service (local)" + in output + ) + + +def test_format_violations_arrow_always_on_separate_line(): + """Arrow line should always be on its own line, regardless of description.""" + violations = [ + Violation( + rule_name="r1", + source_module="a.b", + imported_module="c.d", + category=ImportCategory.LOCAL, + lineno=1, + ), + ] + output = format_violations("a/b.py", violations) + lines = output.strip().split("\n") + assert lines[0] == "a/b.py:1" + assert lines[1] == " [r1]" + assert lines[2] == " a.b \u2192 c.d (local)" + + def test_format_violations_multiple(): violations = [ Violation(