Files
tech-log-backend/docs/superpowers/specs/2026-08-02-warning-zero-build-design.md

22 KiB
Raw Permalink Blame History

Warning-Zero Build Refactoring Design

Date: 2026-08-02 Status: Approved design, pending written-spec review Scope: Java compilation, Error Prone, Checkstyle, SpotBugs, test JVM diagnostics, expected-negative shell-contract output, and intentional legacy/architecture-test compatibility seams.

Goal

Make the standard repository build both functionally green and warning-clean. A successful build must no longer conceal compiler warnings, test-source SpotBugs findings, ignored Checkstyle findings, deprecated third-party API calls, or expected-negative subprocess diagnostics that look like real failures.

The final local proof is a fresh ./gradlew clean build --warning-mode=all --no-daemon --console=plain with:

  • exit code zero;
  • zero compiler/Error Prone warnings;
  • zero Checkstyle and SpotBugs findings in every executed source set;
  • zero SpotBugs ended with exit code 1 messages;
  • zero OpenJDK CDS warnings from test JVMs;
  • no successful Redis lab contract printing its expected-negative child diagnostics;
  • only the five currently intentional optional-adapter/TestKit skips, with no qualification lane silently skipped.

Baseline Evidence

The fresh pre-change command completed successfully in 20 minutes 26 seconds with 283 of 283 tasks executed. Success did not mean warning-clean:

  • 123 compiler warning diagnostics across 19 warning rules (122 distinct file-line/rule coordinates because one line emits two separate removal diagnostics);
  • one test-source SpotBugs DMI_RANDOM_USED_ONLY_ONCE finding;
  • ten OpenJDK CDS warning lines from Mockito-using test JVMs;
  • 82 redis-lab: expected-negative stderr lines;
  • five intentional skipped tests;
  • no test failure, compiler error, Checkstyle finding, SpotBugs analysis error, or missing analysis class.

The Gradle Problems report is an informational index over compiler diagnostics, not a separate defect. It must become empty as a consequence of removing the underlying warnings; it must not be hidden.

Warning inventory traceability

Rule Diagnostic instances Required resolution
removal 46 Exact legacy lifecycle/suppression policy in section 4
MissingOverride 16 Add annotations to the implementing test fakes in section 2
StringCaseLocaleUsage 10 Locale.ROOT behavior fixes and test cleanup in sections 12
SameNameButDifferent 9 Qualify the two Redis nested enum types in section 2
DefaultCharset 9 Explicit UTF-8 test data in sections 12
ArrayRecordComponent 7 Exact record policies and copy regressions in section 2
CanonicalDuration 5 Duration.ofDays(3) in section 2
StringSplitter 4 ETag scanner plus three grammar-specific test fixes in sections 12
EmptyCatch 4 Cleanup failure propagation in section 1
StringConcatToTextBlock 2 Byte-identical text blocks in section 2
InvalidBlockTag 2 Inline-code annotation names in section 2
BigDecimalLiteralDouble 2 Method-only intentional-fixture suppressions in section 5
TypeParameterUnusedInFormals 1 Spring Session method-only suppression in section 2
ThreadLocalUsage 1 Instance-isolation regression and field-only suppression in section 2
ReferenceEquality 1 Redis catalog identity regression and constructor-only suppression in section 2
MissingSummary 1 Public Javadoc summary in section 2
JavaTimeDefaultTimeZone 1 Fixed date/explicit zone in section 1
FutureReturnValueIgnored 1 Observe the future in section 1
BooleanLiteral 1 Literal assertion cleanup in section 2

This table accounts for all 123 Error Prone/compiler-warning diagnostics. The separate -Xlint:deprecation,unchecked inventory is covered by the third-party migrations and exact legacy seam policy below; it is not allowed to disappear through a source-set suppression.

Non-Goals

  • Do not remove the legacy poster-image endpoint, StoredObjectResponse, raw-key compatibility data, or legacy object-storage adapters during warning cleanup.
  • Do not switch the sample runtime from legacy to publication mode without the separately required API, data-adoption, dual-read, and external-consumer approvals.
  • Do not apply module-wide or task-wide suppression for removal, deprecation, unchecked, or Error Prone rules.
  • Do not weaken architecture rules or change deliberately forbidden bytecode merely to silence a fixture warning.
  • Do not make quarantine tests blocking; their separate sunset and reporting policy remains unchanged.

Design Principles

  1. Fix behavior defects at their source before applying any suppression.
  2. Use suppression only where a framework signature, identity invariant, intentional violation fixture, or approved compatibility seam makes the warning inapplicable.
  3. Scope every suppression to the smallest class, method, field, constructor, or fixture that explains it, with a nearby rationale.
  4. Replace deprecated third-party APIs with their typed current equivalents and verify behavior, not only compilation.
  5. Capture expected-negative diagnostics and assert them exactly; never discard stderr globally.
  6. Add blocking gates only after the current warning inventory is clean.

Component Design

1. Real behavior defects

Locale-independent identifiers

Use Locale.ROOT for security roles, notification configuration keys, repository ACL names, and test comparisons. Add Turkish-default-locale regressions that restore the original default locale in finally:

  • JwtToAuthenticatedPrincipalConverter: admin must always become ROLE_ADMIN;
  • RoutingNotifier: diagnostic keys for EMAIL must remain app.notification.routes.email...;
  • RepoStatsAclMapper: IDEA/Repo must normalize to idea/repo.

This is a correctness fix: the current code can generate dotless/dotted Turkish-I variants in authorization and operational identifiers.

Quote-aware ETag list parsing

Do not replace String.split(",") with another delimiter-only splitter. A comma is legal inside a quoted opaque entity tag. ETags will use a small scanner that:

  • splits only on commas outside a quoted string;
  • preserves weak-tag prefixes and the existing trimming behavior;
  • treats malformed/unclosed quotes as non-matching input rather than guessing a token;
  • preserves wildcard and ordinary multi-value behavior.

Regressions cover a single comma-bearing tag, a mixed list containing a weak comma-bearing tag, ordinary lists, wildcard, stale values, blank input, and malformed quoting.

Asynchronous and cleanup failures

  • AsyncGracefulShutdownBehaviorTest retains the returned Future<?> and observes get() so a background assertion or exception cannot disappear.
  • Outbox test cleanup methods propagate or wrap resource-destruction failures with the original cause instead of using empty catches.
  • Tests use fixed dates, UTF-8, and explicit locale rather than host defaults.

2. Production warning cleanup with preserved invariants

Redis primitive ownership

RedisPrimitiveInvocation intentionally requires descriptor object identity. Value equality would admit a descriptor created by another catalog and weaken the closed-catalog invariant. Keep the reference comparison, add an exact constructor-level ReferenceEquality suppression, and add a regression proving value-equal but non-identical cross-catalog descriptors are rejected.

Qualify both nested ExpectedKind types with their enclosing record names rather than renaming the types. This removes SameNameButDifferent without changing bytecode or package-local consumers.

Framework-owned generic signature

RedisVersionedSession.<T>getAttribute(String) must retain Spring Session's inherited signature. Apply a method-only TypeParameterUnusedInFormals suppression with the interface-contract reason.

Instance-owned retry context

OutboundRetryPolicy keeps its instance ThreadLocal. Making it static would leak call context between policy instances on the same thread. Add a field-only ThreadLocalUsage suppression and a regression proving policy A's context is invisible to policy B and is cleared by endCall().

Array-bearing records

  • NotificationCiphertext retains its public array components because it already clones inputs and accessors, implements content-based equality/hash code, and redacts toString. Add focused defensive-copy/equality/redaction tests and an exact record-level suppression.
  • The four internal session command/outcome records in VersionedRedisSessionStore remain internal transport envelopes. Preserve defensive copies, document that generated record equality is not their contract, add constructor/accessor copy tests, and suppress ArrayRecordComponent on each exact record.
  • The private test fake in RedisVersionedSessionRepositoryTest receives the same exact nested-type treatment; no public type is changed.

Mechanical behavior-neutral fixes

  • Express 72 hours as Duration.ofDays(3) in application/bootstrap/sample settings and matching tests.
  • Add the missing public Javadoc summary in TracingSampleRateResolver, and render annotation names such as @WebMvcTest as inline {@code ...} rather than accidental block tags.
  • Add missing @Override annotations in sample test fakes.
  • Replace readability-only string concatenations with text blocks where the literal bytes remain identical.
  • Replace Boolean wrapper comparisons with boolean literals.
  • For the three test-only delimiter warnings, preserve each existing grammar explicitly: retain CSV empty-token filtering with a limit-bearing split or scanner, scan mapping-path segments without changing leading/trailing-empty behavior, and parse the single HTTP byte-range hyphen with an asserted indexOf boundary. These are not allowed to inherit the ETag scanner because their grammars differ.

3. Third-party API migration

Jackson 3

In LocalJsonSchemaRegistry, replace deprecated JsonNode.isTextual()/textValue() with isString()/stringValue(). Existing type guards remain, and JSON schema identity/reference/value tests prove identical acceptance and rejection behavior.

In DeterministicEnvelopeWriter, replace the deprecated convenience call with jsonFactory.createGenerator(ObjectWriteContext.empty(), output, JsonEncoding.UTF8), the non-deprecated Jackson 3.0.2 overload. Preserve canonical byte output; the existing deterministic envelope golden tests are the behavior gate.

Lettuce

Convert both finite canonical scores to BigDecimal, build one inclusive Range<? extends Number> for each invocation, and call the typed zcount(key, range) and zrangebyscoreWithScores(key, range, Limit.create(offset, count)) overloads. Preserve inclusive bounds, offset, count, and exact reply mapping. A dynamic-proxy regression verifies both typed overloads are selected; sorted-set primitive contract tests verify results.

AWS SDK retry

Replace old RetryPolicy and core EqualJitterBackoffStrategy with StandardRetryStrategy, the retries API half-jitter exponential backoff, maxAttempts, and ClientOverrideConfiguration.Builder.retryStrategy. Tests assert maximum attempts and normal versus throttling backoff configuration. The focused object-storage check must cover provider assembly; compile-only success is insufficient.

Testcontainers Toxiproxy

Use the Testcontainers 2 toxiproxy package and a typed ToxiproxyClient/Proxy with an explicit exposed proxy port. Fault tests must still prove cut and restore behavior against MinIO. Dependency and lock changes stay inside the object-storage leaf.

Remaining JDK/generic deprecations

  • Replace deprecated new URL(String) test construction with URI.create(...).toURL().
  • Replace the varargs thenReturn(firstFuture, secondFuture) stub in S3ConditionalObjectControlStoreTest with two chained single-value thenReturn(...) calls, so Mockito does not create the unchecked generic CompletableFuture<PutObjectResponse>[] array.
  • Resolve every -Xlint:deprecation,unchecked location individually; do not suppress the source set.

4. Legacy object-storage compatibility seam

The canonical object-storage ports and sample publication path already exist. The legacy runtime is still selected in local/test configuration and cannot be deleted solely to silence warnings.

Keep @Deprecated(forRemoval = true) on the genuinely replaced whole-byte contracts:

  • ObjectStoragePort;
  • StoredObject;
  • ObjectStorageSettings.

Apply removal suppression only to exact compatibility owners:

  • ObjectStoragePort for its legacy receipt return type;
  • FilesystemObjectStorageAdapter and S3ObjectStorageAdapter;
  • UploadPosterImageUseCase;
  • the legacy bean method in PosterImageApiConfig;
  • LegacyPosterImageController;
  • PosterWebMapper.toStoredObjectResponse;
  • named legacy characterization test classes and single legacy-receipt test methods.

The six application.storage.migration types and AdoptLegacyPosterImageUseCase are the mechanism used to complete data adoption and currently have no replacement. Change their lifecycle marker from @Deprecated(forRemoval = true) to plain @Deprecated; use exact deprecation suppression only inside adoption implementation/configuration. Keep the application-core architecture contract requiring forRemoval=true only for ObjectStoragePort and StoredObject. Keep the adapter-owned ObjectStorageSettings marker and add its lifecycle assertion in the object-storage leaf.

This keeps migration debt visible without falsely claiming that the migration mechanism itself is ready for removal.

5. Test/static-analysis/output cleanup

SpotBugs

Reuse one static SecureRandom in RedisPrimitiveRuntimeServiceTest rather than constructing a one-shot generator. After all test reports are clean, make every ordinary and custom test-source SpotBugs task included by check blocking. SpotBugs analysis errors and missing classes remain separately fail-closed.

Intentional architecture fixtures

Keep prohibited BigDecimal(double/float) constructor bytecode and apply method-only BigDecimalLiteralDouble suppressions. Fix unrelated warnings in allowed fixtures normally. A suppression must never replace the forbidden operation the ArchUnit test is supposed to detect.

Redis lab expected failures

Change assert_fails to capture stdout/stderr per case, assert a non-zero exit and the exact expected diagnostic, reject extra lines, and print the capture only when the assertion fails. Do not redirect to /dev/null and do not silence the Gradle Exec task globally.

Mockito/CDS

Provide mockito-core to test JVMs as an explicit startup -javaagent through a relocatable Gradle argument provider. This removes reliance on Java 21+ runtime self-attachment. Add test-JVM-only -Xshare:off because Mockito's bootstrap append otherwise prints the harmless CDS warning. No production JVM argument changes.

Skips

Retain exactly these five intentional app-bootstrap contract skips:

  • emailNotificationAdapterRunsOnlyWhenConfigured();
  • slackNotificationAdapterRunsOnlyWhenConfigured();
  • redisCacheAdapterRunsOnlyWhenEnabled();
  • messagingBrokerAdapterRunsOnlyWhenConfigured();
  • DisabledOptionalAdapterFixture.wouldFailIfItEverRan().

Qualification tasks continue to require positive discovery, at least one executed test, zero skips, and fresh XML, so this policy cannot turn a selected qualification lane green without execution. Any additional skip, or any of these five moving outside its named optional-adapter contract, fails the inventory check.

6. Warning-zero enforcement

After all existing warnings are removed:

  • configure every leaf JavaCompile task with -Werror, -Xlint:deprecation, and -Xlint:unchecked in the root build policy;
  • retain Error Prone on the same compile tasks so its warnings are promoted by -Werror;
  • remove the root checkstyleTest/spotbugsTest warning-only policy and the app-bootstrap sampleOffTest, functionalTest, and conditionalTransportTest Checkstyle/SpotBugs ignoreFailures overrides, making every such task included by check blocking;
  • retain exact suppression comments as the only approved exception mechanism;
  • run Gradle with --warning-mode=fail in the warning-clean CI lane so Gradle API deprecations also fail rather than print.

quarantineTest remains non-blocking by design. Protected AWS/Docker qualifications remain separate environment evidence and are not converted into local unit tests.

File Ownership and Expected Change Groups

Root build policy

  • src/build.gradle
  • src/gradle/test-jvm-agents.gradle, defining the relocatable Mockito -javaagent argument provider and test-only -Xshare:off policy, applied once by the root build
  • .github/workflows/ci-quality-gates.yml, adding --warning-mode=fail to the blocking quality-gates Gradle invocation

Production leaves

  • src/application-core
  • src/adapter/inbound/web
  • src/adapter/outbound/cache-redis
  • src/adapter/outbound/fileserver
  • src/adapter/outbound/httpclient
  • src/adapter/outbound/identifier
  • src/adapter/outbound/messaging
  • src/adapter/outbound/notification
  • src/adapter/outbound/objectstorage
  • src/adapter/outbound/persistence-jpa
  • src/app-bootstrap
  • src/sample-portfolio
  • src/shared-contract

Every focused command is derived from the owning leaf's gradle_path in src/config/architecture/modules.json; no production dependency edge changes are permitted unless the registry is deliberately updated and its architecture verifier passes.

Tests and shell contract

  • owning leaf tests adjacent to every behavior change
  • exact architecture violation fixtures under app-bootstrap test sources
  • infra/redis-lab/test/redis-lab-contract.sh

Implementation Sequence

  1. Add failing behavioral regressions for locale, ETag parsing, async exception observation, cleanup propagation, Redis descriptor identity, and retry-context isolation.
  2. Implement those behavior fixes and run owner-focused tests.
  3. Remove behavior-neutral compiler/Error Prone warnings per leaf, using only exact justified suppressions.
  4. Migrate Jackson, Lettuce, AWS SDK, Testcontainers, URL, and generic stubs; run their focused behavior/qualification tests.
  5. Correct legacy lifecycle markers and exact compatibility suppressions; run application-core, object-storage, sample, and architecture contracts.
  6. Clean test-only warnings, SpotBugs, Mockito/CDS, and Redis-lab output.
  7. Enable blocking compiler, Checkstyle, SpotBugs, and Gradle warning gates.
  8. Run focused checks, architecture validators, dependency locks, full tests, full check, and the fresh warning-clean build.
  9. Update the LLM Wiki branch note and the warning-debt error note with resolved evidence or exact remaining environmental blockers.

Verification Strategy

Focused verification

  • Each behavior change follows RED → GREEN with the owning leaf test.
  • Static-only warning fixes use the exact compileJava, compileTestJava, Checkstyle, or SpotBugs task as the failing/passing executable contract.
  • Third-party API migrations run behavior tests that exercise request mapping, retry/backoff, sorted-set bounds, schema parsing, or network-fault cut/restore semantics.
  • Legacy suppressions are checked by architecture tests that reject old imports outside the named compatibility surface.

Repository verification

Run from src/:

./gradlew test --no-daemon --console=plain
./gradlew check --no-daemon --console=plain
./gradlew build --warning-mode=fail --no-daemon --console=plain
./gradlew clean build --warning-mode=all --no-daemon --console=plain
./gradlew verifyCleanArchitectureDependencies verifyRuntimeModuleMembership \
  verifyDependencyLocks verifyPublicPathSnapshot verifyEnvKeys \
  --no-daemon --console=plain

Also verify the real gate matrix, wrapper contract, shell syntax, warning-report XML, skipped-test inventory, and git diff --check.

Failure Handling

  • If a suggested warning fix changes a public signature or weakens an identity/security invariant, retain the behavior and use an exact documented suppression backed by a regression.
  • If three attempted fixes in one warning family fail or expose cross-module coupling, stop that family and revisit the design instead of stacking suppressions.
  • If the AWS retry or Toxiproxy migration cannot reproduce old behavior, report that qualification as blocked; do not claim warning-zero by suppressing the deprecation.
  • If a warning originates only in generated code, prove the generated source owner and configure that exact generated boundary; do not disable warnings for handwritten sources.

Risks and Mitigations

  • ETag grammar regression: use quote-aware focused tests before replacing the parser.
  • Authorization drift: test role normalization under Turkish locale.
  • Redis catalog weakening: retain identity comparison and test cross-catalog rejection.
  • AWS retry semantic drift: assert maximum attempts and backoff classes/policies, then run the object-storage provider tests.
  • Legacy data stranding: preserve legacy activation and characterization until the separate data/API migration gates are approved.
  • Hidden diagnostics: capture-and-assert expected stderr; never discard it.
  • Suppression creep: exact annotations plus architecture/import checks prevent module-wide exemptions.
  • Build duration: use owner-focused RED/GREEN loops and reserve full clean builds for integration checkpoints and final proof.

Acceptance Criteria

The work is complete only when:

  1. All behavior regressions and focused owner checks pass.
  2. Every compiler task passes with -Werror, deprecation lint, unchecked lint, and Error Prone.
  3. Every ordinary/custom Checkstyle and SpotBugs task included by check is blocking and clean.
  4. Legacy warnings are limited to no output because exact compatibility code is explicitly and locally justified; no module/task-wide suppression exists.
  5. The Redis lab successful contract prints only its success summary and unexpected child diagnostics still fail the test with captured evidence.
  6. Test JVMs print no CDS/self-attachment warning.
  7. Full test, check, build, dependency, architecture, runtime-membership, env, public-path, wrapper, gate-matrix, shell, and diff validators pass.
  8. The final fresh clean-build log contains no warning:, deprecated/unchecked Note:, SpotBugs non-zero message, OpenJDK warning, or leaked expected-negative Redis diagnostic.
  9. LLM Wiki capture records commands, results, resolved warning counts, suppressions, and any environment-only qualification not executed locally.