updated coding standards
All checks were successful
Build, test and publish / verify (push) Successful in 4m43s

This commit is contained in:
2026-10-30 19:58:03 +01:00
parent 9a69981025
commit f910f1e535
65 changed files with 2827 additions and 724 deletions

255
CODING_STANDARDS.md Normal file
View File

@@ -0,0 +1,255 @@
# Backend coding standards
This document is the authoritative policy for handwritten production and test Java
in `agiledancers-be`. Generated code and Java helpers outside that module are out
of scope. Existing frontend conventions in README.md and agiledancers-fe remain in force.
## Scope and authority
- 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.
- [Java coding guidelines](https://www.geeksforgeeks.org/java/coding-guidelines-in-java/)
supplied source material. Only the rules adopted in this local document apply;
changes to the article do not change project policy.
- 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.
## Implemented checks and manual review
Checkstyle **12.3.1**, PMD/CPD **7.28.0**, and JaCoCo **0.8.14** are pinned.
`tooling/standards/checkstyle.xml` selects individual naming, structure, import and brace checks; no general analysis bundle is enabled.
`tooling/standards/src/main/java/Standards.java` supplies the targeted measurements.
ArchUnit remains in the backend test suite for architectural boundaries.
PMD uses Java 25, the running JDK's `jrt-fs.jar`, compiled backend classes and the
resolved test dependency classpath. Cyclomatic complexity uses PMD's default
options: boolean paths count, assertions do not. Decisions include if/loops,
catch, ternary and throw; non-default switch alternatives count individually,
including comma-separated labels. There is no aggregate class-complexity gate.
Cognitive complexity uses PMD's boolean-operation sequences, nesting, switch,
recursion and flow rules. The scoped visitors retain those algorithms while
including lambdas in their enclosing executable and excluding nested/local type
bodies, whose methods are measured independently. Compact record constructors
use the same visitors and start with cyclomatic complexity one.
Custom checks measure physical code lines from regular parser tokens, including
brace/declaration lines and mixed code/comment lines. They count multiline literal
lines while excluding physically blank lines. Class length excludes nested-type
tokens, preserving lines shared with enclosing code. Other custom measurements
implement the definitions above: nesting, resolved type fan-out, project-owned
superclass depth, public non-accessor methods, and injected collaborators.
Injection includes inherited fields/setters, explicit `@Inject` constructors,
single constructors on bean classes, records' implicit canonical constructors,
and Lombok required/all-argument constructor injection. Qualifier keys use resolved
annotation names and values, including defaults; repeat sites are deduplicated.
The CPD configuration preserves identifiers/literals, uses 100 tokens and disables
skip blocks. Its line-location union counts overlapping occurrences only once.
Checkstyle and targeted checks enforce import/brace/naming/visibility rules,
`@Override`, visible constructor/cast/new-array `var` initializer forms, final parameters, locals (including
loops/catches and lambdas) and initialized fields. PMD's three selected finality
rules additionally cover control-flow assignment and private constructor-initialized
fields. Framework/lifecycle requirements, generated Lombok setters, and external
writes to public/protected fields need review before changing field finality.
No tool can infer whether a comment explains intent, whether a public contract
needs Javadoc, whether a record is appropriate, or whether mutable components are
sufficiently protected. These are **manual rules**, as are ambiguous type inference
and architecture/DI design decisions. Review all `DESIGN` record-component entries
and `REVIEW` signals, even when a build passes, using `docs/backend-review.md`.
Reports never rewrite Java. Formatter application remains S8's separate validate
execution. Generated output under `target` and generated source directories are
outside the handwritten `src/main/java` and `src/test/java` scope. No maintained
file, DTO, configuration class or package is excluded from standards or coverage.
JaCoCo filters compiler/Lombok-generated members; no handwritten-code exclusions
are configured. Line and branch results are published, with only lines gated.
Exact metric waivers live in `tooling/standards/waivers.tsv`; each records a metric,
fully resolved executable signature (or class), exact retained value and the
maintainer agreement/reason. No wildcard waivers are allowed, and a changed value
fails again. A waived blocking result remains visible as `WAIVER`. PMD suppression
annotations/comments cannot silently waive a rule. The only approved waiver is the eight-parameter `Group` canonical constructor,
which preserves its JSON contract while defensively copying both lists. Adding a waiver always requires maintainer agreement.
## Commands and reports
Run from the root with JDK 25:
```sh
mvn -Dformat.mode=check clean verify
# Prepare standalone analysis without running tests:
mvn -pl agiledancers-be test-compile dependency:build-classpath@standards-classpath
mvn -f tooling/standards/pom.xml test exec:java
# Exact local coverage gate after a test/report build:
java tooling/coverage/Coverage.java agiledancers-be/target/site/jacoco/jacoco.xml
```
`verify` runs backend tests, builds resolved analysis inputs, generates JaCoCo
reports, runs checker regression tests and standards analysis, then gates exact
module-wide line coverage. `compile` and `test` retain existing formatting behavior
and do not run the verify-only standards checks. Standalone analysis requires its
classpath preparation; unresolved types fail with the path and preparation advice.
Full metrics and findings: `agiledancers-be/target/standards/report.txt`.
Checker regression results: `tooling/standards/target/surefire-reports`.
Coverage HTML/XML/CSV: `agiledancers-be/target/site/jacoco` (local backend tests)
and `agiledancers-be/target/site/jacoco-combined` (merged browser contribution).
The gate compares integer line counts (`5 * covered >= 4 * total`), rejecting an
empty report and percentages that would round up to 80%.
CI first passes local verify. It mounts the pinned JaCoCo agent into its disposable
backend test container, exposes the agent only on the isolated Docker network,
and uses the pinned CLI to dump `target/playwright.exec` after Playwright. A missing
or empty dump fails. The following commands merge both datasets and gate the result
before the verified-commit marker can authorize publication:
```sh
mvn -pl agiledancers-be jacoco:merge@combined jacoco:report@combined-report exec:exec@combined-coverage
```
CI retains both coverage reports, execution data, standards findings and regression
results with its usual diagnostics artifacts. The agent is mounted only for tests;
published images contain no agent. PIT and Sonar configuration remain outside S7.
See `specs/S7-validation.md` for migration evidence and retained-signal assessment.