From aeb4e49936c4aec3b6d83f8e620d35ef943ea1d3 Mon Sep 17 00:00:00 2001 From: Original Gary <276612211+OpenGaryBot@users.noreply.github.com> Date: Thu, 14 May 2026 08:14:55 +1000 Subject: [PATCH] fix: wire advocacy phases into every language config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `desloppify scan` was silently skipping advocacy_language, advocacy_security, and advocacy_tool_presence detectors on 9 of 11 languages — only javascript and go had manual wiring. Confirmed via e2e: scan a Python file containing "kill two birds with one stone" + "whitelist" → status shows the Advocacy language dimension exists with "10 checks" but 0 findings; direct call to detect_advocacy_language() on the same file finds 4 instances. Root cause: each LangConfig.phases list is built manually. javascript appended advocacy phases via `cfg.phases.append(...)` after generic_lang returned; go inlined them in its phases list. python, typescript, csharp, cxx, dart, gdscript, rust, and every generic-only language never had them. Fix: move advocacy wiring into the shared tail (shared_subjective_duplicates_tail) that every language config already calls. One canonical location, one shared helper (shared_advocacy_phases), no per-language churn when phases evolve. Removed the now-duplicate manual wiring in javascript and go. Also: added the 3 advocacy phase labels to SHARED_PHASE_LABELS in capabilities.py so capability_report stops bucketing them under "linting" in the langs command output. Verified post-fix: scan on the same Python test file now prints `[15/20] Advocacy language... 4 instances → 4 issues` and `show advocacy_language` returns the persisted findings with full suggestion/alternative data. All existing tests pass (after updating test_phase_builders.py assertions for the new tail shape). Regression test added: test_advocacy_phase_coverage.py asserts every dedicated LangConfig AND generic_lang include all 3 advocacy phases. Would have caught the original bug. --- .../_framework/base/phase_builders.py | 29 +++++++++- .../generic_support/capabilities.py | 3 + desloppify/languages/go/__init__.py | 6 -- desloppify/languages/javascript/__init__.py | 9 +-- .../common/test_advocacy_phase_coverage.py | 58 +++++++++++++++++++ .../tests/lang/common/test_phase_builders.py | 46 +++++++++------ 6 files changed, 119 insertions(+), 32 deletions(-) create mode 100644 desloppify/tests/lang/common/test_advocacy_phase_coverage.py diff --git a/desloppify/languages/_framework/base/phase_builders.py b/desloppify/languages/_framework/base/phase_builders.py index ab00dabb5..94e1380bc 100644 --- a/desloppify/languages/_framework/base/phase_builders.py +++ b/desloppify/languages/_framework/base/phase_builders.py @@ -80,12 +80,36 @@ def factory() -> DetectorPhase: detector_phase_boilerplate_duplication = SHARED_PHASE_FACTORIES["boilerplate_duplication"] +def shared_advocacy_phases() -> list[DetectorPhase]: + """Open Paws advocacy detector phases shared across all language configs. + + Every language config that wants advocacy enforcement (which is every + language in this fork) includes these. Lazy-imported to keep advocacy + code from loading when the framework is consumed outside this fork. + """ + from desloppify.languages._framework.phases_advocacy import ( + detector_phase_advocacy_language, + detector_phase_advocacy_security, + detector_phase_advocacy_tool_presence, + ) + return [ + detector_phase_advocacy_language(), + detector_phase_advocacy_security(), + detector_phase_advocacy_tool_presence(), + ] + + def shared_subjective_duplicates_tail( *, pre_duplicates: list[DetectorPhase] | None = None, ) -> list[DetectorPhase]: - """Shared review tail: subjective review, optional custom phases, then duplicates.""" - phases = [detector_phase_subjective_review()] + """Shared review tail: advocacy phases, subjective review, optional custom phases, then duplicates. + + Advocacy phases run first in the tail so their findings land in state + before subjective review reads them. + """ + phases: list[DetectorPhase] = list(shared_advocacy_phases()) + phases.append(detector_phase_subjective_review()) if pre_duplicates: phases.extend(pre_duplicates) phases.append(detector_phase_boilerplate_duplication()) @@ -102,5 +126,6 @@ def shared_subjective_duplicates_tail( "detector_phase_test_coverage", "EXCLUSIVE_DETECTOR_MODULES", "SHARED_PHASE_FACTORIES", + "shared_advocacy_phases", "shared_subjective_duplicates_tail", ] diff --git a/desloppify/languages/_framework/generic_support/capabilities.py b/desloppify/languages/_framework/generic_support/capabilities.py index e52e2ca46..64e58cdcb 100644 --- a/desloppify/languages/_framework/generic_support/capabilities.py +++ b/desloppify/languages/_framework/generic_support/capabilities.py @@ -25,6 +25,9 @@ "Responsibility cohesion", "Unused imports", "Signature analysis", + "Advocacy language", + "Advocacy security", + "Advocacy tools", } ) diff --git a/desloppify/languages/go/__init__.py b/desloppify/languages/go/__init__.py index 06fbb837b..5a7a878d6 100644 --- a/desloppify/languages/go/__init__.py +++ b/desloppify/languages/go/__init__.py @@ -13,10 +13,6 @@ detector_phase_test_coverage, shared_subjective_duplicates_tail, ) -from desloppify.languages._framework.phases_advocacy import ( - detector_phase_advocacy_language, - detector_phase_advocacy_security, -) from desloppify.languages._framework.base.types import DetectorPhase, LangConfig from desloppify.languages._framework.generic_support.core import make_tool_phase from desloppify.languages._framework.registry.registration import register_full_plugin @@ -73,8 +69,6 @@ def __init__(self): detector_phase_signature(), detector_phase_test_coverage(), detector_phase_security(), - detector_phase_advocacy_language(), - detector_phase_advocacy_security(), *shared_subjective_duplicates_tail(), ], fixers={}, diff --git a/desloppify/languages/javascript/__init__.py b/desloppify/languages/javascript/__init__.py index 86984f2c0..bb39ad4a8 100644 --- a/desloppify/languages/javascript/__init__.py +++ b/desloppify/languages/javascript/__init__.py @@ -3,10 +3,6 @@ from __future__ import annotations from desloppify.languages._framework.generic_support.core import generic_lang -from desloppify.languages._framework.phases_advocacy import ( - detector_phase_advocacy_language, - detector_phase_advocacy_security, -) from desloppify.languages._framework.treesitter import JS_SPEC from desloppify.languages.javascript._zones import JS_ZONE_RULES from desloppify.languages.javascript import test_coverage as js_test_coverage_hooks @@ -35,9 +31,8 @@ test_coverage_module=js_test_coverage_hooks, ) -# Append Open Paws advocacy phases to the generic config. -cfg.phases.append(detector_phase_advocacy_language()) -cfg.phases.append(detector_phase_advocacy_security()) +# Advocacy phases (advocacy_language, advocacy_security, advocacy_tool_presence) +# are wired in via shared_subjective_duplicates_tail() — no per-language append needed. __all__ = [ "generic_lang", diff --git a/desloppify/tests/lang/common/test_advocacy_phase_coverage.py b/desloppify/tests/lang/common/test_advocacy_phase_coverage.py new file mode 100644 index 000000000..54e067b2e --- /dev/null +++ b/desloppify/tests/lang/common/test_advocacy_phase_coverage.py @@ -0,0 +1,58 @@ +"""Regression: every language config must include the advocacy detector phases. + +This was silently broken pre-fix: only javascript and go wired in advocacy +phases manually. All other 9 language configs skipped them, so `desloppify scan` +returned 0 advocacy findings on any non-JS/Go codebase — defeating the fork's +core differentiator. + +The fix moved advocacy into shared_subjective_duplicates_tail() so it's +universal. This test guards that wiring. +""" + +from __future__ import annotations + +import importlib + +import pytest + +ADVOCACY_LABELS = {"Advocacy language", "Advocacy security", "Advocacy tools"} + +# Languages with a dedicated LangConfig (vs generic_lang plugins). +_DEDICATED_LANGUAGES = [ + "python", + "typescript", + "csharp", + "cxx", + "dart", + "gdscript", + "go", + "rust", +] + + +@pytest.mark.parametrize("lang_name", _DEDICATED_LANGUAGES) +def test_dedicated_lang_config_includes_advocacy_phases(lang_name: str) -> None: + mod = importlib.import_module(f"desloppify.languages.{lang_name}") + cfg = mod.Config() + phase_labels = {p.label for p in cfg.phases} + missing = ADVOCACY_LABELS - phase_labels + assert not missing, ( + f"language '{lang_name}' is missing advocacy phases: {sorted(missing)}. " + "All Open Paws-fork language configs must include the advocacy phases " + "via shared_subjective_duplicates_tail()." + ) + + +def test_generic_lang_plugin_includes_advocacy_phases() -> None: + """generic_lang (used by javascript + 18 other languages) also gets advocacy.""" + from desloppify.languages._framework.generic_support.core import generic_lang + cfg = generic_lang( + name="_test_advocacy_generic", + extensions=[".x"], + tools=[{"label": "t", "cmd": "echo", "fmt": "gnu", "id": "test_adv_det", "tier": 2}], + ) + phase_labels = {p.label for p in cfg.phases} + missing = ADVOCACY_LABELS - phase_labels + assert not missing, ( + f"generic_lang is missing advocacy phases: {sorted(missing)}" + ) diff --git a/desloppify/tests/lang/common/test_phase_builders.py b/desloppify/tests/lang/common/test_phase_builders.py index 5beedea21..68460e8dc 100644 --- a/desloppify/tests/lang/common/test_phase_builders.py +++ b/desloppify/tests/lang/common/test_phase_builders.py @@ -80,24 +80,33 @@ def test_all_factories_produce_callable_run(): # ── shared_subjective_duplicates_tail ───────────────────────── -def test_shared_tail_default_has_three_phases(): - """Default tail: subjective review, boilerplate duplication, duplicates.""" +_ADVOCACY_LABELS = ["Advocacy language", "Advocacy security", "Advocacy tools"] + + +def test_shared_tail_default_has_six_phases(): + """Default tail: 3 advocacy phases, subjective review, boilerplate duplication, duplicates.""" phases = shared_subjective_duplicates_tail() - assert len(phases) == 3 - assert phases[0].label == "Subjective review" - assert phases[1].label == "Boilerplate duplication" - assert phases[2].label == "Duplicates" + assert len(phases) == 6 + labels = [p.label for p in phases] + assert labels[:3] == _ADVOCACY_LABELS + assert labels[3] == "Subjective review" + assert labels[4] == "Boilerplate duplication" + assert labels[5] == "Duplicates" def test_shared_tail_with_pre_duplicates_inserts_in_middle(): """Extra phases go between subjective review and boilerplate duplication.""" custom = DetectorPhase("Custom detector", lambda p, lang: ([], {})) phases = shared_subjective_duplicates_tail(pre_duplicates=[custom]) - assert len(phases) == 4 - assert phases[0].label == "Subjective review" - assert phases[1].label == "Custom detector" - assert phases[2].label == "Boilerplate duplication" - assert phases[3].label == "Duplicates" + assert len(phases) == 7 + labels = [p.label for p in phases] + assert labels == [ + *_ADVOCACY_LABELS, + "Subjective review", + "Custom detector", + "Boilerplate duplication", + "Duplicates", + ] def test_shared_tail_with_multiple_pre_duplicates(): @@ -105,9 +114,10 @@ def test_shared_tail_with_multiple_pre_duplicates(): custom_a = DetectorPhase("Alpha", lambda p, lang: ([], {})) custom_b = DetectorPhase("Beta", lambda p, lang: ([], {})) phases = shared_subjective_duplicates_tail(pre_duplicates=[custom_a, custom_b]) - assert len(phases) == 5 + assert len(phases) == 8 labels = [p.label for p in phases] assert labels == [ + *_ADVOCACY_LABELS, "Subjective review", "Alpha", "Beta", @@ -119,15 +129,17 @@ def test_shared_tail_with_multiple_pre_duplicates(): def test_shared_tail_empty_pre_duplicates_same_as_default(): """Empty list for pre_duplicates behaves like None.""" phases = shared_subjective_duplicates_tail(pre_duplicates=[]) - assert len(phases) == 3 + assert len(phases) == 6 def test_shared_tail_slow_flags(): - """Last two phases (boilerplate duplication + duplicates) are slow.""" + """Only the last two phases (boilerplate duplication + duplicates) are slow.""" phases = shared_subjective_duplicates_tail() - assert phases[0].slow is False # subjective review - assert phases[1].slow is True # boilerplate duplication - assert phases[2].slow is True # duplicates + # Advocacy + subjective review are fast + for fast_phase in phases[:-2]: + assert fast_phase.slow is False + assert phases[-2].slow is True # boilerplate duplication + assert phases[-1].slow is True # duplicates # ── Treesitter phase factories ────────────────────────────────