Skip to content

Adopt code-review profile, tighten static analysis, align docs to ISO-8859-1; harden generator internals#165

Draft
peter-lawrey wants to merge 10 commits intodevelopfrom
adv/code-review
Draft

Adopt code-review profile, tighten static analysis, align docs to ISO-8859-1; harden generator internals#165
peter-lawrey wants to merge 10 commits intodevelopfrom
adv/code-review

Conversation

@peter-lawrey
Copy link
Copy Markdown
Member

@peter-lawrey peter-lawrey commented Oct 28, 2025

This PR formalises our code-review quality gate, updates contributor guidance, and hardens the chronicle-values build and internals without changing public APIs. It also aligns our contributor style guide from ASCII-7 to ISO-8859-1 and relocates project docs into src/main/docs.

Key outcomes:

  • ✅ New -Pcode-review profile (Checkstyle + SpotBugs/FindSecBugs + PMD + JaCoCo + Enforcer).
  • Contributor policy updated to ISO-8859-1; doc links in AGENTS.md fixed to src/main/docs/*.
  • Static-analysis configs added (checkstyle*.xml, pmd-*.xml, spotbugs-exclude.xml) with justified, tagged suppressions.
  • Security & portability hardening in code-gen/runtime: doPrivileged where needed, locale-safe case ops, safer URI handling, explicit UTF-8 for I/O, SpotBugs annotations, and minor generator clean-ups.
  • Docs layout: move decision-log.adoc into src/main/docs/ and add TODO.adoc with follow-ups.

Why

  • Bring this repo in line with the Chronicle quality-gate used across projects.
  • Give agents/contributors unambiguous language/encoding rules (ISO-8859-1) while our Checkstyle still enforces ASCII in sources to protect wire formats.
  • Reduce false positives from static analysis and harden sensitive code paths (reflection, class-loading, URI I/O).
  • Centralise architecture/decision artifacts under src/main/docs and fix broken links from AGENTS.md.

What changed

1) Contributor policy & docs

  • AGENTS.md

    • Language: switch from “ASCII-7” → “ISO-8859-1”; keep “avoid smart quotes/nb-spaces/accents” guidance.
    • Update references to src/main/docs/decision-log.adoc and src/main/docs/project-requirements.adoc.
    • Clarify that agents must honour the ISO-8859-1 guideline.
  • Docs move

    • src/main/adoc/decision-log.adocsrc/main/docs/decision-log.adoc (new file with VAL-tagged entry).
    • New src/main/docs/TODO.adoc capturing post-profile hardening tasks.

ℹ️ Note: Checkstyle still enforces ASCII-only string literals via IllegalTokenText. That remains intentional to protect binary protocols; the contributor guide’s “ISO-8859-1” note governs prose/artifacts, not necessarily Java string literals. We’ll reconcile this in a later pass if needed.

2) Build & Quality Gate

  • pom.xml

    • Add -P code-review profile:

      • maven-checkstyle-plugin (3.6.0 + com.puppycrawl 8.45.1) with src/main/config/checkstyle.xml and suppressions.
      • spotbugs-maven-plugin 4.8.6.6 + findsecbugs 1.14.0 with spotbugs-exclude.xml.
      • maven-pmd-plugin 3.8.0 (rules + excludes).
      • jacoco-maven-plugin 0.8.14 with bundle-level gates (profile sets thresholds to 0.0 initially; project defaults at 80/70% remain for other profiles).
    • Add com.github.spotbugs:spotbugs-annotations (scope provided) and property spotbugs.annotations.version.

  • Config files added under src/main/config/:

    • checkstyle.xml, checkstyle-suppressions.xml
    • pmd-ruleset.xml, pmd-exclude.properties
    • spotbugs-exclude.xml

3) Hardening & static-analysis cleanups (no API change)

  • Security/robustness

    • Constrain SimpleURIClassObject.openInputStream() to file:/ and jar: schemes; make openWriter() explicitly unsupported.
    • Locale-stable casing via Locale.ENGLISH/ROOT in Nullability, PrimitiveBacked*, Utils.capitalize, etc.
    • Add targeted @SuppressFBWarnings with VAL-SPOT-* tags; emit debug log when BridgeClassLoader falls back.
  • Generator/impl tidy-ups

    • Finalise inner generator where appropriate; fix visibility to final or package-private where internal.
    • Use %n in generated code, remove stray System.out.print via Checkstyle rule.
    • Introduce small PMD suppressions with explicit IDs (e.g. VAL-PMD-320/321/322) where the layout algorithm intentionally allocates per-field metadata.
  • ValueModel / CachedCompiler compatibility

    • Add reflective fallback to CachedCompiler#setFileManagerOverride for newer compiler versions; keep old path for older ones.

Backwards-compatibility

  • Public API surface unchanged. Internal class constructor visibility was tightened (ValueBuilder remains package-private; no public API impact).
  • Build inputs: New dev-time dependency on spotbugs-annotations (provided). The code-review profile is opt-in; default mvn verify behaviour is unchanged.
  • Docs paths: If you link to src/main/adoc/…, update to src/main/docs/….

Developer notes

# Normal build (unchanged)
mvn -q verify

# Run the new quality gate locally
mvn -q -Pcode-review verify

# Edit <jacobi.*.coverage> in the profile or run with -Djacoco.line.coverage=… -Djacoco.branch.coverage=…

@peter-lawrey peter-lawrey changed the title Adv/code review Adopt code-review profile, tighten static analysis, align docs to ISO-8859-1; harden generator internals Oct 28, 2025
@sonarqubecloud
Copy link
Copy Markdown

@peter-lawrey peter-lawrey marked this pull request as draft November 3, 2025 10:37
@peter-lawrey peter-lawrey removed the request for review from james-mcsherry November 3, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant