refinement of S7 - coding standards for the backend
All checks were successful
Build, test and publish / verify (push) Successful in 1m45s
All checks were successful
Build, test and publish / verify (push) Successful in 1m45s
This commit is contained in:
200
specs/refined/S7_define-coding-standards-to-be
Normal file
200
specs/refined/S7_define-coding-standards-to-be
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user