Skip to content

Conversation

@peter-lawrey
Copy link
Member

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

This PR modernises compiler across three fronts:

  1. Docs & guidance

    • Refines AGENTS.md tables and wording; promotes real-time docs and formal Nine-Box tagging.
    • Moves requirements/decisions to src/main/docs/, adds decision-log.adoc.
    • Streamlines README.adoc (adds :sectnums:, renames sections, adds Documentation & Requirements link).
    • Trims line-wrapping in LICENSE.adoc.
  2. Build & quality profile

    • Bumps parent to 1.27ea2-SNAPSHOT.
    • Adds an opt-in code-review Maven profile with Checkstyle, SpotBugs(+FindSecBugs), PMD, and JaCoCo gates (85% line, 85% branch).
    • Introduces spotbugs-annotations (provided), rule versions as properties, and suppression files.
  3. Runtime compiler hardening & tests

    • Tightens class-name validation (allows module-info/package-info; blocks invalid segments).
    • Adds safe file resolution and NIO-based IO with backup/restore; never closes System.err.
    • Makes compiler options immutable, centralises StandardJavaFileManager creation.
    • Preserves binary compatibility on fileManagerOverride while adding a setter.
    • Substantial test suite expansion for concurrency, IO, safety rails, and binary compatibility.

Motivation

  • Reduce false positives and drift by codifying company-wide doc and tagging rules for agents/humans.
  • Bring the module up to Chronicle’s current quality bar with a single, reproducible -Pcode-review run.
  • Address security and robustness concerns around path handling, writer lifecycle, and class-name acceptance.
  • Set realistic, enforced coverage gates that match today’s baseline (85%) while keeping pressure upward.

Notable Changes

Documentation and guidance

  • AGENTS.md: table alignment, clearer Javadoc “Do/Don’t”, line-break hygiene, Nine-Box taxonomy table.

  • Requirements/decisions now under src/main/docs/; links fixed in AGENTS.md and README.adoc.

  • decision-log.adoc adds:

    • [RC-FN-001] allow hyphenated descriptor class names.
    • [RC-TEST-002] enforce 85% line/branch coverage gates.
  • project-requirements.adoc: JRC-TEST-014 updated to >= 85%.

  • README.adoc: concise sections, :sectnums:, adds “Documentation & Requirements” section.

  • LICENSE.adoc: compact, single-line paragraphs.

Build and quality

  • New properties: pinned versions for Checkstyle, SpotBugs, FindSecBugs, PMD, JaCoCo, Chronicle quality rules.

  • -Pcode-review profile:

    • Checkstyle with Chronicle rules, fail on warning.
    • SpotBugs + FindSecBugs, effort=Max, threshold=Low, fail on error; project-local exclude file.
    • PMD with fail on violation and project-local excludes.
    • JaCoCo prepare/report/check with 85% line / 85% branch.
  • Always define JaCoCo version in the sonar profile too.

  • Adds spotbugs-annotations (provided scope).

  • New config files:

    • src/main/config/spotbugs-exclude.xml (documented suppressions with IDs).
    • src/main/config/pmd-exclude.properties (scaffold with justification comments).

Runtime compiler changes

  • Class name validation: accepts module-info/package-info; per-segment regex prevents invalid characters and trailing hyphens.
  • Path safety: safeResolve(...) and sanitisation in CompilerUtils block traversal and normalise paths.
  • IO: switch to NIO Path/Files; atomic backup/restore; identical content short-circuit; clearer exceptions.
  • Default writer: UTF-8, auto-flush, never closes System.err.
  • Compiler options: stored as unmodifiable list.
  • Standard file manager: centralised, cached access; diagnostic listener simplified.
  • File manager override: legacy public field retained (now volatile and documented); new setFileManagerOverride(...) for source-level use.
  • MyJavaFileManager: small refactor; dedicated inner JavaFileObject classes; clearer Unsafe initialisation; FB suppressions with justifications.
  • Minor tidy-ups (imports, lambdas, comments, visibility).

Tests

  • New and expanded suites covering:

    • Binary compatibility of fileManagerOverride.
    • Multi-threaded compilation with barrier; try-with-resources for classloaders.
    • IO round-trips, backups, sanitisation, inline inputs, error paths.
    • Guardrailed pipeline example (validation vs compile metrics; cache hits).
    • CachedCompiler bytecode map behaviour on success/failure; manager updates; closing behaviour.
    • MyJavaFileManager buffering, delegation, modules APIs where present, and invocation edge cases.

Backward Compatibility

  • Binary: preserved. The public fileManagerOverride field remains, now volatile; callers can also use the new setter.

  • Behavioural:

    • Stricter class-name checks may reject previously accepted but invalid names; module-info/package-info are explicitly allowed.
    • Path traversal will now be rejected; callers passing unsafe relative paths will see IllegalArgumentException.
    • JaCoCo gates raised to 85%/85% under -Pcode-review (opt-in profile).

How to use the quality profile

mvn -q -Pcode-review verify
  • Checkstyle, SpotBugs(+FindSecBugs), PMD, and JaCoCo will run and fail on violations.
  • Suppressions live under src/main/config/. Please add targeted entries with justification comments.

@sonarqubecloud
Copy link

@peter-lawrey peter-lawrey changed the title Adv/code review Raise quality bar, tighten safety, and align docs/CI for Chronicle Compiler Oct 28, 2025
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.

3 participants