Imported from david181222/Paragon (
.claude/skills/solid/SKILL.md). Install upstream withnpx skills add david181222/Paragon --skill solid. Copyright stays with the author.
SOLID Auditor — Java · C# · JavaScript
Contract-level audit of object-oriented code against the five SOLID principles. Every finding carries evidence (file:line), a concrete failure mode, a blast-radius estimate, and a patch — plus, for serious findings, executable tests that fail on the current code and pass on the fix.
Scope is deliberately three languages. The protocol depends on what each language's compiler enforces, and precision matters more than breadth: auditing JavaScript with Java assumptions produces confident, wrong findings.
Operating stance
SOLID is a cost model, not a rulebook. The principles exist to lower the cost of change. A violation that costs nothing is a note, not a finding. Rank by consequence, never by principle count.
Over-abstraction is a violation too. Most tooling detects only under-abstraction. The dominant disease in young codebases is the opposite: interfaces with one implementation and no second in sight, DI for stable dependencies, factories that build one thing. Flag these as OVER- findings on the same severity scale.
LSP is the only principle whose violations are bugs. SRP, OCP, ISP and DIP cost maintenance time. LSP costs correctness — wrong behavior in code that compiles. Weight severity accordingly. This is also why references/lsp.md is far longer than the other four modules: that asymmetry is deliberate, not an oversight.
Report the causal chain. A forced inheritance (LSP) usually descends from a fat base class (SRP) exposed through a fat interface (ISP). Fixing the leaf without naming the root produces a patch that regresses in two sprints.
Never claim a violation you cannot demonstrate. If you cannot point at a caller that would break, a state that would corrupt, or a change that would require editing N files, downgrade to INFO and say so.
Step 1 — Triage: pick the mode, obey its cap
Do this first, always. The three modes have hard output ceilings; exceeding them is a failure of the skill, not thoroughness.
| Mode | Trigger | What to run | Hard cap |
|---|---|---|---|
| SPOT | One design question, one or two classes, "should this be inheritance or composition", a snippet pasted in chat | The relevant principle module only. No scanner, no report template. | ≤ 15 lines. Verdict + one reason + the patch. No headings. |
| HIERARCHY | One class hierarchy, one interface and its implementers, one file, "is this class OK" | Phases 0, 2 (relevant principles), 3, 4, then Phase 7 checks 1–2. Scanner optional. | ≤ 2 pages. Report sections 3, 4, 5 only. |
| REPO | "Audit this project", a directory, a repository, a PR | All phases, full report. Phase 7 included — it is part of the deliverable, not an extra. | Sections 1–9. No cap, but sample by risk if the codebase is large. |
| DIFF | A pull request, a branch, "review this change", staged changes | Phases 0 (light), 2, 4 on the changed surface only, then Phase 7 check 1 | ≤ 1 page. Findings introduced by this diff, nothing else. |
When the mode is ambiguous, pick the smaller one and offer the larger: "Spot answer above. Say the word and I'll audit the whole hierarchy." An unrequested full report is how these audits get ignored.
DIFF mode has one non-obvious rule. Audit the changed types plus their supertypes and subtypes, even when untouched. An LSP violation can be introduced by editing the base class alone — tightening a precondition in the base retroactively breaks nothing, but loosening a postcondition in the base invalidates every subtype's contract, and a naive diff review never sees it.
Phase 0 — Reconnaissance
Map before reading. Never open source files at random.
- Detect language and toolchain from
pom.xml/build.gradle,*.csproj/*.sln,package.json(+tsconfig.jsonif present). Read the target section ofreferences/languages.mdbefore Phase 2. Not optional: R1 and R7 change meaning per language and R3 is compiler-enforced in exactly one of the three. - Check strictness configuration. Missing
@Overrideenforcement (Java),<Nullable>disable</Nullable>(C#), absence of any type checking or lint on classes (JS) — each silently disables checks the team believes they have. A weak configuration is aHIGHfinding in its own right; the table is inlanguages.md. - Build the inheritance graph — supertype, subtypes, depth, overridden vs. inherited members. Run the scanner (Phase 1) to bootstrap it.
- Identify the polymorphic surface — which types are actually used through a base type or interface (base-typed variables, collections, parameters). A hierarchy never used polymorphically cannot break LSP at runtime; its findings are
LOW. In JavaScript this surface is invisible in the class declarations — it is defined by what functions receive and what properties they touch. Derive it from call sites or you will audit the wrong thing. - Identify the actors (SRP sense): which humans or systems request change to this code. Without this list, SRP degenerates into line counting.
- Check for tests. See the guardrail below — this determines what you are allowed to prescribe.
- Read the consumer's composition root, even when the audit scope is a single library:
Program.cs/Startup.cs,main, the Spring@Configuration,server.js. A whole class of defect is invisible from inside the audited scope and lives only in the seam: an objectnew-ed inside a library while its owner is registered per-request; a dependency the container never registers; a service that is never registered at all and whose entire code path is therefore dead. Neither file is wrong on its own — the pair is. Budget one read for this; it is among the highest-yield minutes in the protocol.
State the map explicitly before Phase 1: language + strictness, class/interface counts, deepest hierarchy, the 3–5 most polymorphic types, actors, test coverage of the affected types.
Evidence guardrail — read before prescribing anything
Two of this protocol's strongest methods depend on artifacts that may not exist:
- No test suite for the base types → every contract item is inferred, low confidence. In that state, do not prescribe structural refactors (inheritance removal, hierarchy splits, interface extraction). Prescribe characterization tests first, and say plainly that the structural recommendation is blocked until the current behavior is pinned down. Refactoring unverified code on inferred contracts is how audits cause outages.
- No usable git history → OCP variation axes cannot be measured, only guessed. Say so and downgrade every OCP finding that rests on speculation to
INFO.
Stating these limits costs one paragraph and is what separates an audit from an opinion.
Phase 1 — Automated smell scan
python scripts/smell_scan.py <path> --lang auto --min-severity MEDIUM --json > /tmp/solid_smells.json
Detects, for the three target languages: unsupported-operation overrides, exceptions thrown on the success path (LSP-THROW-ON-SUCCESS), exceptions re-wrapped by string concatenation (LSP-REWRAP), empty/no-op overrides, instanceof and type-tag branching, C# new member hiding, Java accidental overloads, async overrides of sync methods (JS), arity mismatches (JS), constructor calls to overridable methods, frozen-state bypass, ambient dependencies, concrete infrastructure instantiation, god files and single-implementation interfaces.
The scanner produces candidates, not findings. Each rule carries a precision hint (high / medium / low) — spend reading time accordingly. Every hit is promoted with evidence or discarded, and the report states the discard rate: a scan with 80% false positives on this codebase is itself information about this codebase.
Rule ID → where to read:
| Rule prefix | Module |
|---|---|
LSP-* |
references/lsp.md (rule ref in the hit maps to R1–R10) |
ISP-* |
references/isp.md |
SRP-* |
references/srp.md |
OCP-* |
references/ocp.md |
DIP-* |
references/dip.md |
Phase 2 — Per-principle analysis
Lazy loading applies to the five principle modules below and to nothing else. Open one only when Phase 0/1 produced evidence that it applies. Reading all five on a codebase with no inheritance wastes context that belongs to the user's code.
Do not extend lazy loading to the output assets.
references/languages.md,references/refactorings.md,assets/substitution-tests.mdandreferences/example-report.mdare not principle modules — they are the templates and decision tables that Phases 0, 4, 5 and 6 depend on, and each of those phases states plainly that it requires them. Treating them as "lazy" is a documented failure mode of this skill: it produces tests written from scratch that do not compile, composition/aggregation chosen by intuition, and a report at the wrong density. If you are running REPO mode, you will read all four before you finish.
| Principle | Load when | Reference | Core question |
|---|---|---|---|
| LSP | Any extends, implements, prototype chain, or duck-typed substitution exists |
references/lsp.md |
Can a subtype replace its supertype without a caller noticing? |
| ISP | Any interface, abstract class, or shared method-shape contract exists | references/isp.md |
Is any client forced to depend on members it never calls? |
| SRP | Always (every codebase has modules) | references/srp.md |
Does this module have exactly one actor who can request its change? |
| OCP | Conditional chains over types/tags exist, or git history is available | references/ocp.md |
Can new behavior be added without editing existing tested code? |
| DIP | Layers, I/O, or injected collaborators exist | references/dip.md |
Does policy depend on abstractions it owns? |
Order is fixed: LSP → ISP → SRP → OCP → DIP. LSP first because its violations are correctness bugs, and because the substitution failures found there hand you the evidence for the ISP and SRP findings that caused them. DIP last because correct seam placement depends on knowing the responsibilities and variation axes first.
Phase 3 — Contract inference
Real codebases do not declare preconditions, postconditions or invariants. Reconstruct them before judging any subtype. This phase is what separates an audit from a linter. Sources in priority order (later overrides earlier):
- Signatures and types — nullability,
final/readonly/Object.freeze, generics bounds, checked exception declarations. - Constructor validation — every check is an invariant candidate.
- Guard clauses in the base implementation — these are the declared preconditions.
- Documentation — Javadoc
@throws/@param, XML docs, JSDoc. Markdeclared. - Tests. The base type's suite is its executable specification: every assertion is a postcondition, every input used proves the precondition accepts it. Highest-quality source and the most commonly skipped — read the tests before the subclasses.
- Caller behavior. No null check after a call implies a non-null postcondition. No defensive copy implies immutability. No
tryimplies "does not throw". A cached instance implies an immutability contract. Caller assumptions are contract whether or not anyone wrote them down — breaking them breaks production. - Frequency argument. If every subtype but one honors a guarantee, the guarantee is the contract and the outlier is the violation. State this reasoning when you use it.
Output a contract table per base type with a declared | inferred column. For CRITICAL findings resting on inferred items, ask the user to confirm before prescribing a structural refactor.
Phase 4 — Prescription
Every finding gets a patch. Read references/refactorings.md before writing the first patch — not optional, and not replaceable by the short form below. Two things live there and nowhere else: the composition/aggregation/association table (§2), which is the only way to get the UML relation right, and the migration sequencing rules (§4). The short form is an index, not a substitute:
- Subtype rejects part of the base contract (throws, no-ops, strengthens preconditions) → the base demands too much → role interfaces (ISP split), each type implementing only what it honors.
- Subtype reuses base code but is not a base behavior → replace inheritance with delegation.
- Shared life cycle, one owns the other → composition (whole constructs the part, never exposes it by reference).
- Independently owned collaborators → aggregation (inject the part, never
newit inside). - Behavior varies along one axis → strategy, injected at construction.
- Variation is additive layering (cache, retry, log) → decorator.
- Client branches on concrete type → push the branch into a polymorphic method; if the type set is genuinely closed and the operation set open, use sealed types + exhaustive matching and say why.
- Mutable subtype of an immutable base → split the hierarchy at the mutability boundary.
- Subtype adds value-carrying fields to an equality-bearing base → composition, or make the base abstract.
Prescribe the cheapest fix that removes the failure mode. If relaxing a precondition by two lines fixes it, do not propose a strategy-pattern rewrite. Ambition in a refactor plan is a cost, not a virtue.
Phase 5 — Executable verification
For every CRITICAL and HIGH LSP finding, generate a substitution test suite: one parameterized test running the base contract against every subtype.
Open assets/substitution-tests.md and fill in the templates. JUnit 5, xUnit and Jest/Vitest, pre-written and parameterized. Writing them from scratch instead is the single most common way this phase degrades: it is slower, and it loses assertions the templates already encode (C# new-hiding detection via R9, JS sync/async via R2, the reflection trick that keeps the subtype list from going stale).
Before writing a single test, answer these four. Each one has silently broken a generated suite:
- Are the types under test reachable from a test assembly?
internal(C#) / package-private (Java) / non-exported (JS) types are not. C#: add[assembly: InternalsVisibleTo("<Project>.Tests")]— the right answer, because it preserves the encapsulation the report defends in §8. Never widen a type topublicjust to test it. - Does every test compile as written? No
/* fill this in */placeholders in constructor arguments, no invented helper classes. If a type needs an awkward object graph to construct, that difficulty is a DIP finding — report it instead of hiding it behind a placeholder. - Will each test still compile after its own patch? A test asserting against members the patch renames is a diagnostic test: mark it as such, say it must be deleted or renamed at that migration step, and name the test that replaces it.
- Is the assertion still meaningful after the patch? A test comparing a constant against itself passes trivially and proves nothing. Diagnostic tests document a defect now; the suite that survives must assert behavior.
The suite asserts: preconditions are not strengthened (inputs valid for the base succeed for every subtype), postconditions hold, invariants survive mutator sequences, no undeclared exception type escapes, and the mutability contract is respected. Add property-based tests (jqwik, FsCheck, fast-check) when the invariant is expressible over generated input — random mutator sequences are the only reliable way to catch history-rule violations.
The test must fail on current code and pass after the patch. Run it both ways when the environment allows, and paste the failure output into the report. A failing test is evidence; a paragraph is an opinion.
Also update the base type's Javadoc/XML doc/JSDoc with the now-explicit contract, so the next subclass author cannot violate it by accident.
Phase 6 — Report
Severity
| Level | Meaning | Trigger |
|---|---|---|
CRITICAL |
Wrong behavior reachable in production today | Substitutable subtype breaks the base contract on a live path; corrupted invariant; silently swallowed operation |
HIGH |
Bug awaiting the next caller, or change cost that already hurts | Contract violation on a path not yet exercised; a change requires editing >3 tested files; weak toolchain configuration |
MEDIUM |
Structural debt, bounded blast radius | Fat interface with 2 clients; SRP violation inside one module |
LOW |
Style/consistency, no measured cost | Hierarchy never used polymorphically; naming |
INFO |
Observation, not a violation | Deliberate trade-off; framework-imposed pattern; speculation that could not be measured |
OVER-* |
Over-abstraction (OVER-MEDIUM, etc.); also emit category: over-abstraction |
Single-implementation interface, speculative extension point, DI for stable dependencies |
Blast radius, for every CRITICAL/HIGH: call sites depending on the polymorphic type, subtypes affected, whether it crosses a public API boundary, whether the fix is source-compatible.
The severity label and the engineering verdict must agree. Writing HIGH in the header and "no real impact today" in the same finding is the most common calibration error in this protocol, and it discredits the whole report — a reader who catches one stops trusting the other seventeen. When they disagree, the engineering verdict wins and the label moves. Two specific traps:
- A member nothing reads. Before assigning
HIGHto a corrupted or badly-generated value, grep for its readers across every file type, templates and views included. Zero readers means the defect is latent — say so explicitly and justify the severity on some other member that is read, or lower it. - Cost that belongs to a different finding. "Changing this requires editing 5 files" often measures the root-cause finding, not the one you are labelling. Attribute the cost once, to the finding that actually carries it.
Measured debt
Report the raw numbers so they can be tracked: overrides that throw/no-op/strengthen preconditions ÷ total overrides; type-interrogation sites per KLOC; mean interface width and mean fraction used per client; actors per module; files touched by the last N similar feature commits (git — the only empirical OCP measurement); concrete imports crossing layer boundaries.
Offer — do not create unprompted — a solid-baseline.json for ratchet mode: subsequent runs report only new violations. It is the only version of this audit that survives contact with a real backlog.
Report structure (REPO mode)
Read references/example-report.md before writing. It calibrates density — evidence over prose, patches over advice — and it is faster to match an example than to invent a register. Sections:
# Auditoría SOLID — <proyecto>
## 1. Resumen ejecutivo (≤10 lines: what will break, and the one root cause)
## 2. Mapa del sistema (language/strictness, graph, polymorphic surface, actors, test coverage)
## 3. Contratos reconstruidos (per base type; declared vs inferred)
## 4. Hallazgos (by severity, then blast radius)
### [SEVERITY][PRINCIPLE-##] Título
- **Evidencia:** file:line + minimal excerpt
- **Regla violada:** e.g. LSP-R4 (precondición fortalecida)
- **Modo de falla:** the concrete scenario where a caller breaks
- **Radio de impacto:** call sites, subtypes, API boundary
- **Causa raíz:** upstream finding, if any
- **Parche:** before/after, real code from this codebase
- **Riesgo de migración:** source-compatible / requires caller changes / breaks public API
## 5. Reestructuración propuesta (target design + Mermaid classDiagram + migration order)
## 6. Verificación ejecutable (contract tests + failure output)
## 7. Deuda medida (metrics table, scanner discard rate)
## 8. Lo que NO hay que cambiar (mandatory)
## 9. Verificación de la auditoría (Phase 7 output: recalibrations, findings added, blind spots)
Section 8 is not filler. A report that flags everything gets discarded wholesale; one that explicitly defends the correct parts of the design earns the credibility to be acted on.
Academic mode
When the user is writing a university deliverable (rubric, course report, defense of a design), the stance above conflicts with grading: a rubric counts a violation whether or not it costs anything. Do not abandon the honest analysis — add a mapping. For each finding, give the rubric-facing statement ("viola LSP: la subclase fortalece la precondición del método base") alongside the engineering verdict ("impacto real: nulo, la jerarquía no se usa polimórficamente"). The user gets full marks and still knows the truth.
Language
Write the report in the language the user is writing in. Identifiers, keywords and exception type names stay in their original form.
Phase 7 — Self-verification
Phase 5 verifies the code with tests. This phase verifies the audit. Both are needed and they catch different things.
Run this before delivering. It is not optional in REPO mode, and it is not the user's job to ask for it.
A protocol whose stance is "never claim a violation you cannot demonstrate" has to check its own claims. Empirically this pass changes a third of the findings in a first-draft audit — recalibrating some, adding others — and every one of those changes is one the reader would otherwise have found first.
Four checks, in order:
-
Re-read every finding against the source. Not against your notes and not against the report — against the file. Confirm the line numbers, the quoted excerpt, and that the claimed behavior is what the code does. Discard anything that does not survive, and report the retraction count: an audit that retracts nothing has probably not checked.
-
Cross-check severity against the engineering verdict, using the rules in the Severity section above. Any finding whose label and verdict disagree is recalibrated here, and the recalibration is stated in §9 rather than silently applied.
-
Sweep for defects noted but never promoted. Findings acquire an ID; observations written into a contract table, a footnote, or an aside do not — and then they vanish from the count, the migration order and the reader's attention. Grep your own draft for defects described outside §4 and either give each an ID or delete it. Check every cross-reference resolves to the ID it names.
-
Hunt false negatives in the two places they hide. Passive re-reading finds almost nothing; go to these deliberately:
- The rest of a method you already judged. Finding one defect in a method marks it "audited" and the remaining lines stop being read. Re-read every method that carries a finding, end to end.
- The seam between the audited scope and its consumers (Phase 0 step 7). Lifecycle registration, dead wiring, objects constructed on one side of the boundary and owned on the other. A scope defined by folder cannot see these by construction.
Report the outcome in §9, including what the checks cost you: findings confirmed unchanged, retracted, recalibrated, added; and — most useful to the next reader — the blind spot that produced each late finding. A verification section that reports "everything was already correct" is evidence the pass was skipped, not evidence of a good first draft.
Scope discipline
Single file or single hierarchy → audit that, note in one line what else you noticed. Repository too large to read → sample by risk (deepest hierarchies, most-imported modules, most type-interrogation hits) and state explicitly what was sampled and what was skipped.
Design mode (no code yet, "how should I model X"): skip Phases 0–3, use references/refactorings.md as the decision tree, produce the target design and the contract tests as an executable spec.
Reference index
Lazy — load on evidence (the five principle modules):
references/lsp.md— the substitution protocol: R1–R10, contract inference, call-site analysis, false positives, worked examples. The core module.references/isp.md— client-usage matrix, role interface extraction.references/srp.md— actor mapping, cohesion, god class decomposition.references/ocp.md— variation axes via git history, speculative generality guard.references/dip.md— dependency direction, abstraction ownership, over-injection guard.
Required by their phase — not subject to lazy loading:
references/languages.md— Java / C# / JavaScript enforceability, gotchas, toolchain flags. Phase 0.references/refactorings.md— decision tree, patch catalog, composition vs aggregation, migration sequencing. Phase 4.assets/substitution-tests.md— JUnit 5 / xUnit / Jest contract-test templates. Phase 5.references/example-report.md— a filled short report at the target density. Phase 6.
Tooling:
scripts/smell_scan.py— candidate scanner. Bootstraps the inheritance graph and flags mechanical smells; it does not find semantic defects (a method whose success path throws, a method that computes without applying, an event that is never raised). Those come from reading. Treat a quiet scan as information about the scanner, never as evidence the code is clean.
Changelog
- 2.1 — Added Phase 7 (verification) and report §9; ended lazy loading of the four phase-required assets, which was producing non-compiling tests and intuition-based composition/aggregation choices; added the consumer composition-root read to Phase 0 (seam defects); added the severity-vs-verdict consistency rule; added the pre-writing checklist to Phase 5; new C# scanner rules for exception-as-return-channel and exception re-wrapping;
namein frontmatter aligned with the directory (solid). - 2.0 — Focused on Java, C#, JavaScript (+TS delta). Added triage modes with output caps, DIFF/PR mode, evidence guardrail, academic mode, lazy module loading, test templates, example report, scanner precision hints and
--min-severity. - 1.0 — Initial: 7-language coverage, LSP R1–R10, five principle modules.