From 35123e51f682f04d80dd7fd9255e558a6c7368b7 Mon Sep 17 00:00:00 2001 From: Wanja Krah Date: Fri, 30 Oct 2026 17:18:13 +0100 Subject: [PATCH] refinement of S7 - coding standards for the backend --- specs/S15_add-DoD | 3 + specs/S7_define-coding-standards-to-be | 43 ---- .../refined/S7_define-coding-standards-to-be | 200 ++++++++++++++++++ 3 files changed, 203 insertions(+), 43 deletions(-) create mode 100644 specs/S15_add-DoD delete mode 100644 specs/S7_define-coding-standards-to-be create mode 100644 specs/refined/S7_define-coding-standards-to-be diff --git a/specs/S15_add-DoD b/specs/S15_add-DoD new file mode 100644 index 0000000..5c5e528 --- /dev/null +++ b/specs/S15_add-DoD @@ -0,0 +1,3 @@ +- tested +- changelog +- documented \ No newline at end of file diff --git a/specs/S7_define-coding-standards-to-be b/specs/S7_define-coding-standards-to-be deleted file mode 100644 index 3a01d4c..0000000 --- a/specs/S7_define-coding-standards-to-be +++ /dev/null @@ -1,43 +0,0 @@ -As developer -I want anyone, including agents to follow coding standards for the agiledancers-be folder -so that the code stays readable and maintainable. - -The coding style should be desribed in CODING_STANDARDS.md. If the file does not exist, create it. If the file does exist, don't remove instructions for the front-end (Angular). This spec will ony do coding-standards for the backend (Java). - -Make sure agents use these coding_standards as wel when coding or reviewing (with the code-review skill in the skills-folder). - -For the guidelines we start with the guidelines described at https://www.geeksforgeeks.org/java/coding-guidelines-in-java/ as a basis. -When we have a collision with the Java Palentir formatter this project uses, we will always favour the Palentir formatter. -For curly braces, we use the variant: A curly brace is applied at the end of the line that starts the class, method, loop, etc. -Block comments will be enwrapped with /* and */. The actual text is on sererate lines and indented. Example: -/* - This is an example of a blocked comment. - It may have more than one line. -*/ - -if-, for-, and while-loops must always have curly braces. -No star-imports. All imports have to be explicit. -static imports are only allowed in test classes. - -De volgende metrics moeten gehaald worden, met een streefwarde, een review-signal, en een absoluut maximum: -Metric | Streefwaarde | Reviewsignaal | Blokkerende grens -Cyclomatic complexity per methode/constructor | ≤ 5 | > 7 | > 10 -Cognitive complexity per methode | ≤ 10 | > 10 | > 15 -Methodlengte, zonder lege regels en commentaar | ≤ 20 regels | > 30 | > 50 -Parameters per methode | ≤ 3 | > 4 | > 5 -Parameters per constructor | ≤ 4 | > 5 | > 7 -Geïnjecteerde dependencies per klasse | ≤ 4 | > 5 | > 7 -Fan-out per klasse, unieke gebruikte types | ≤ 10 | > 15 | alleen review -Depth of Inheritance Tree, eigen superclasslagen, exclusief Object | ≤ 1 | > 2 | > 3 -Nesting van control flow | ≤ 2 niveaus | > 3 | > 4 -Duplicated lines density | ≤ 1% | > 1% | > 3% -Klasselengte, zonder lege regels en commentaar | ≤ 200 regels | > 300 | > 500 -Publieke methoden per klasse, exclusief triviale accessors | ≤ 10 | > 15 | Geen automatische grens - -De code-review skill wordt hier ook van op de hoogte gebracht. -De test-coverage moet minimaal 80% zijn. Dit mag gehaald worden met een combinatie van unit- en e2e-testen. unit- en e2e-testen mogen afzonderlijk onder de 80% blijven. -Indien mogelijk, wordt de ArchitectureTest geupdate of wordt er een nieuwe test gemaakt om zaken te controleren die met een test te controleren zijn. Verder mogen checks uitgevoerd worden met CheckStyle, FindBugs and PMD, plus eventuele andere checks die ze aanbevelen. Tot slot mogen er PIT-testen worden gemaakt om de code-coverage te checken. -Het toeveogen van een geconfigureerde Sonar in de pipeline is out-of-scope. Dit doen we later met spec S14. - -The current code-base should be updated to adhere to these coding standards. - diff --git a/specs/refined/S7_define-coding-standards-to-be b/specs/refined/S7_define-coding-standards-to-be new file mode 100644 index 0000000..c5cf45a --- /dev/null +++ b/specs/refined/S7_define-coding-standards-to-be @@ -0,0 +1,200 @@ +# S7 — Backend coding standards + +Status: REFINED + +Shared understanding confirmed after the grill-me review. + +## User story + +As a developer, I want developers and agents to follow explicit coding standards +for `agiledancers-be`, so that the code stays readable and maintainable. + +## Scope and authority + +- Create root `CODING_STANDARDS.md` with the complete agreed backend rules. + Preserve any existing frontend instructions. +- Cover handwritten production and test Java in `agiledancers-be`. Exclude + generated code. Dependency injection and production architecture rules apply + where relevant. Frontend and Java helpers outside the backend are out of scope. +- Use https://www.geeksforgeeks.org/java/coding-guidelines-in-java/ as source + material, not as a changing normative dependency. The local document is complete + and authoritative for the adopted rules. +- The project's pinned Palantir formatter wins formatting conflicts, including + comment layout. S8 remains responsible for formatter integration. +- Do not adopt unrelated static-analysis rule bundles. + +## Java conventions + +- Types use UpperCamelCase; methods and variables use lowerCamelCase; constants + use UPPER_SNAKE_CASE; package names are lowercase. +- One top-level type per file; explicit member visibility; `@Override` on + overriding methods. Do not invent visibility modifiers where Java forbids them. +- Opening braces stay on the declaration/control-statement line. All `if`, `for` + and `while` bodies require braces, including enhanced for and do/while bodies. +- No wildcard imports. Static imports are allowed only in test classes. +- Comments explain intent or constraints. Document public contracts with Javadoc + where their meaning is not clear from the declaration; do not require boilerplate + documentation for every accessor. +- Block comments use the following layout, subject to Palantir formatting: + + ```java + /* + This is an example of a block comment. + It may have more than one line. + */ + ``` +- Prefer records for immutable data carriers. Classes are allowed when mutation, + framework requirements or inheritance make records unsuitable. Preserve API and + serialization behavior. Record immutability is shallow: protect mutable + components appropriately. +- Require `final` for parameters, local variables and fields wherever they can + remain assigned once, including loop variables and catch parameters where Java + permits it. Field finality must remain compatible with framework injection and + lifecycle requirements. Permit reassignment when the algorithm needs it; do not + introduce artificial holder objects to avoid reassignment. This rule does not + require final classes or methods. +- Use explicit types unless the initializer visibly identifies the type. `var` is + allowed with `new School(...)` or an explicit cast, but not for method results, + literals or other inferred expressions. For example, + `final var school = new School(...)` is allowed, while a repository result uses + `final School school = repository.find(id)`. Borderline cases require review. +- No additional member-ordering or line-width policy beyond Palantir. + +## Metrics + +Targets guide development. Review signals require assessment, not automatic +failure. Blocking limits fail verification. Strict inequalities matter: a value +exactly at a blocking limit passes. + +| Metric | Target | Review signal | Blocking limit | +| --- | ---: | ---: | ---: | +| Cyclomatic complexity per method/constructor | ≤ 5 | > 7 | > 10 | +| Cognitive complexity per method/constructor | ≤ 10 | > 10 | > 15 | +| Method/constructor length, code lines | ≤ 20 | > 30 | > 50 | +| Parameters per method | ≤ 3 | > 4 | > 5 | +| Parameters per explicitly written constructor | ≤ 4 | > 5 | > 7 | +| Injected dependencies per class | ≤ 4 | > 5 | > 7 | +| Fan-out per class, unique used types | ≤ 10 | > 15 | Review only | +| Own superclass depth, excluding Object | ≤ 1 | > 2 | > 3 | +| Control-flow nesting | ≤ 2 | > 3 | > 4 | +| Production duplicated lines density | ≤ 1% | > 1% | > 3% | +| Class length, code lines | ≤ 200 | > 300 | > 500 | +| Declared public methods per class, excluding trivial accessors | ≤ 10 | > 15 | Review only | + +### Measurement definitions + +- Use the pinned PMD algorithms as authoritative for cyclomatic and cognitive + complexity. Document their exact configuration, including switch and boolean + treatment. Do not accidentally introduce a class-level aggregate complexity gate. +- Measure each method, constructor and explicitly written record constructor + independently. Lambda bodies contribute to their enclosing executable; + methods in nested/local classes are separate executables. +- Parameter limits exclude implicit record constructors and generated members. + Record component counts remain a design review concern. +- Length means physical source lines after Palantir formatting, excluding blank + and comment-only lines. Declaration and brace-only lines count; a line containing + both code and comments counts. Do not substitute statement counts or NCSS. + Measure nested types separately and exclude them from enclosing class length. +- Count distinct injected collaborators by resolved type and qualifier. Include + constructor, field, setter, inherited and implicit constructor injection. + Repeated injection of the same type and qualifier counts once; differently + qualified instances of the same type count separately. +- Fan-out counts distinct explicitly referenced resolved types, including JDK and + framework types. Exclude primitives, the class itself and generated references. +- Inheritance depth counts project-owned superclass layers; exclude Object, + interfaces and external framework ancestry. +- Nesting combines `if`, loops, `switch` and `catch`, with the first structure at + depth one. An `else if` chain stays at one level. `else`, `try` and `finally` + add no level themselves. Lambdas retain surrounding depth; methods in nested + classes start fresh. +- Public-method counts include explicitly declared public instance and static + methods; exclude constructors, inherited methods and trivial accessors. A + trivial accessor only reads or assigns its corresponding field, with no + validation, branching or other work. +- Duplication limits apply only to production code. Duplication in tests is allowed + and has no target, review signal or blocking limit. +- Detect production clones with CPD, minimum 100 tokens, without ignoring + identifier or literal differences. Density is + `100 × distinct duplicated code-line locations / all production code-line locations`. + Exclude blank/comment-only lines from numerator and denominator. Count every + occurrence, including the first; merge overlapping ranges to count each source + line once. This project metric need not equal a future Sonar metric. + +## Enforcement and developer workflow + +- Use explicitly pinned Java-25-compatible Checkstyle and PMD/CPD versions. + Retain ArchUnit for architecture checks and add small targeted custom checks + where standard rules cannot accurately express the agreed semantics. +- Automate every blocking metric and explicit rule where reliable checking is + available. Identify rules requiring manual review explicitly; do not claim + they are automated. Review signals must appear in reports even when builds pass. +- Run standards checks during `mvn verify`; provide documented standalone commands. + Ordinary compile/test commands retain the current S8 formatting behavior. +- Standards checks report violations without rewriting code. CI passes these + checks before image publication. Parser failures or unsupported Java constructs + fail with the affected path and actionable diagnostics, rather than being skipped. +- Add a root AGENTS.md instruction requiring agents to read CODING_STANDARDS.md + before changing backend Java. Project review instructions must explicitly cover + manual rules, review signals in changed code and approved waivers, without + repeating checks already enforced by tooling. +- Keep upstream skill files unchanged. Put project-specific code-review guidance + in a companion file referenced by project instructions and used alongside + `skills/code-review/SKILL.md`. +- Waivers must be explicit, narrowly scoped and carry a recorded reason alongside + code or configuration. Agents need maintainer agreement before introducing one. + No blanket suppressions, exclusions of difficult files or weakened thresholds + merely to make verification pass. + +## Coverage + +- Require at least 80% backend line coverage across handwritten production code + for the module as a whole, not separately per class or package. Compare the + unrounded percentage against 80%. +- Local `mvn verify` must reach 80% using backend unit/integration tests alone. + This intentionally tightens the original combined-tests-only requirement. +- CI additionally instruments the backend exercised by Playwright, merges that + coverage with backend unit/integration coverage and gates the combined result. + The Playwright contribution need not separately reach 80%. +- Exclude generated code only; do not broadly exclude DTOs, configuration or + difficult code. Publish line and branch coverage reports; branch coverage has + no agreed blocking percentage. +- PIT measures mutation-test effectiveness, not line coverage, and is outside + S7's required implementation. Configuring Sonar in the pipeline remains out of + scope for S14. FindBugs/SpotBugs is not an additional required check suite. + +## Existing-code migration + +- Bring existing backend production and test Java into compliance, including + structural refactoring where necessary. +- Migration is complete when all blocking rules pass and all review signals have + been assessed, with rationale recorded for retained signals. Reaching every + target is not required. +- Preserve public APIs, JSON contracts, persistence formats and application + behavior. Any necessary contract change requires a separate decision. +- Separate enforcement/configuration changes from code refactoring for review. + Strengthen tests where migration affects behavior-sensitive code. + +## Acceptance criteria + +- [ ] CODING_STANDARDS.md contains all agreed rules and preserves frontend guidance. +- [ ] Agent and project code-review instructions enforce discovery and review of + these standards without modifying upstream skills. +- [ ] Pinned tools, configurations and targeted checks implement the exact agreed + metrics and identify manual-only rules. +- [ ] Verify and standalone checks report actionable blocking failures and review + signals; CI prevents publication after a failure. +- [ ] Representative violations demonstrate that their checks fail, including + relevant threshold boundaries and custom measurement semantics. +- [ ] Existing handwritten backend production and test code passes all blocking + rules; retained review signals and approved waivers are documented. +- [ ] Local backend tests and combined CI coverage meet the agreed line-coverage + gates; line and branch reports are available. +- [ ] Clean verification and behavior-preservation tests pass after migration. +- [ ] Evidence records tool versions, commands, scope, exclusions, metric and + coverage reports, approved waivers and verification results. + +## Implementation handoff + +This specification records the agreed design. Updating it does not implement +checks, change agent instructions, refactor application code or establish current +coverage. Those are implementation work, with evidence required above. -- 2.49.1