Add Error Prone and NullAway static analysis to build#75
Open
bernardladenthin wants to merge 18 commits into
Open
Add Error Prone and NullAway static analysis to build#75bernardladenthin wants to merge 18 commits into
bernardladenthin wants to merge 18 commits into
Conversation
Enables Error Prone 2.30.0 (last release supporting -source 8) and NullAway 0.10.26 as javac plugins. NullAway is enforced as ERROR on the main package (net.ladenthin) and disabled for test compilation where JMH @setup fields and intentional null arguments would otherwise trip it. .mvn/jvm.config supplies the --add-exports / --add-opens required by Error Prone when running on JDK 16+.
Empirically verified that current Error Prone supports compiling -source 8/-target 8 on JDK 21 when -XDaddTypeAnnotationsToSymbol=true is passed, contrary to my earlier assumption that 2.30.0 was the last Java-8-capable release. Only the JDK required to *run* Error Prone has moved forward (see google/error-prone#4867).
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
The jvm.config format doesn't allow comments, so the SPDX header lives in a .license sidecar per REUSE specification.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
CodeQL's autobuild runs 'mvn package -DskipTests' which previously
still triggered the exec-maven-plugin jcstress execution bound to the
test phase, failing because test classes weren't compiled. Adding
<skip>${skipTests}</skip> to that execution propagates -DskipTests.
Separately, the earlier Error Prone commit set <annotationProcessorPaths>
which makes the compiler ignore classpath-based processors. That broke
jcstress and JMH annotation processing during test-compile. Adding both
processors to the test-compile path with combine.children="append"
restores them while preserving Error Prone + NullAway.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #75 +/- ##
============================================
- Coverage 97.60% 97.58% -0.02%
Complexity 93 93
============================================
Files 1 1
Lines 250 248 -2
Branches 32 32
============================================
- Hits 244 242 -2
Partials 6 6 ☔ View full report in Codecov by Sentry. |
|
coverage: 100.0%. remained the same — claude/gracious-carson-gRcSp into main |
Enforces Palantir Java Format 2.66.0 on all main and test sources via the spotless-maven-plugin. The spotless:check goal is bound to the verify phase so CI fails on formatting drift; run 'mvn spotless:apply' to fix. The initial apply touches most files and is mechanical.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Adds a Build Commands subsection covering how to run the existing JMH benchmarks under exec-maven-plugin, with filter syntax, the gc allocation profiler, and async-profiler (flamegraph) invocation.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Runs SpotBugs at default effort/threshold on main classes during the verify phase, augmented with the fb-contrib and findsecbugs detector plugins. The single initial finding - EI_EXPOSE_REP on getOutputStream/getInputStream - is suppressed via spotbugs-exclude.xml because those streams are the public API of StreamBuffer.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Marks the Deque buffer with @GuardedBy("bufferLock") so Error Prone enforces that every access happens under the bufferLock monitor. Propagates the annotation to the two methods that touch buffer outside an explicit synchronized block - trim() and isTrimShouldBeExecuted() - since their docstrings already require callers to hold bufferLock. Adds error_prone_annotations as a compile dependency.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Whitebox unit tests intentionally call isTrimShouldBeExecuted() without holding bufferLock to verify the predicate logic in isolation. GuardedBy stays enforced for main code.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Encodes two structural invariants as JUnit tests:
- mainCodeStaysLeaf: the published library must only depend on the
JDK and the Error Prone GuardedBy annotation; any new runtime
dependency will surface as a failing test.
- dequeFieldsArePrivate: Deque fields must be private, preventing
the internal buffer from being exposed via a future getter or
package-private leak.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Makes both libraries available for the existing test suite:
- Awaitility 4.2.2: replaces Thread.sleep + poll patterns with
await().until(...) for deterministic concurrency assertions.
- ConcurrentUnit 0.4.6: Waiter.assertEquals/assertTrue surface
assertion failures from background threads back to the test
thread (JUnit can't see them otherwise).
No tests refactored in this commit; libraries are added now so future
refactors are local, single-purpose changes.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Lincheck's model checker enumerates thread schedules for write, close, available and isClosed operations. read() is excluded because blocking reads are incompatible with the schedule enumeration model checker - jcstress remains the test for concurrent read+write races. The test runs 20 iterations of 500 invocations each across 2 threads with 3 actors per thread. Local run: ~9s.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
Wires com.vmlens:api as a test dependency and the vmlens-maven-plugin inside a 'vmlens' profile. Default 'mvn test' is unchanged; running 'mvn -Pvmlens test' drives tests written against AllInterleavings through every possible thread interleaving. Off by default because vmlens overhead is too high for every build.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
The vmlens profile now excludes StreamBufferLincheckTest because vmlens load-time instrumentation conflicts with the bytecode Lincheck generates for TestThreadExecution (java.lang.VerifyError, stack map mismatch). The default test job still runs Lincheck. Adds a vmlens job to publish.yml that runs in parallel with the regular Test (JDK 21) job, uploads target/vmlens-report as an artifact, and does not block coverage reporting.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
- read_parallelClose_noDeadlock: replaces Thread.sleep(500) with
await().until(reader.getState() in WAITING/TIMED_WAITING). The
test now blocks only as long as it actually takes the reader
thread to park inside read(), instead of guessing 500ms.
- concurrentReadWrite_stressTest: pipes background-thread
IOExceptions through net.jodah.concurrentunit.Waiter so a real
failure surfaces with its original stack trace instead of being
re-thrown as a RuntimeException and lost to
Thread.UncaughtExceptionHandler.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
The vmlens-maven-plugin runs its own forked surefire and does not pick up excludes configured on the standalone maven-surefire-plugin. Moving the **/StreamBufferLincheckTest.java exclude into the vmlens plugin's own <excludes> block (preserving its **/*$* default for inner classes) is what actually skips the Lincheck test under vmlens. Local: mvn -Pvmlens test now passes in ~4 min with 269/270 tests (Lincheck skipped, runs in the default test job).
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
… Exception Sonar's reliability rule java:S1181 flags methods declaring Throwable because it lumps Errors with checked Exceptions. Waiter.resume() only throws AssertionError (unchecked) and Thread.join() throws InterruptedException; throws Exception is sufficient and matches the original signature.
bernardladenthin
temporarily deployed
to
startgate
GitHub Actions
Inactive
- Fix Reliability bug at StreamBufferTest:858: hoist os.close() out of the assertThrows lambda so only writeAnyValue() can produce the expected IOException (java:S5778). - Add explicit assertions to six tests Sonar flagged as having none (assertDoesNotThrow / assertTimeoutPreemptively / state checks). - Refactor write_nullArrayWithOffset_throwsNPE so the assertThrows lambda has exactly one possibly-throwing call. - Rename WriteMethod constants to UPPER_SNAKE_CASE to satisfy the Java naming convention (java:S115). - Explain why the IOException catches in ConcurrentWriteRace are intentionally empty (race with concurrent close()).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NullAwayerrors enforced for thenet.ladenthinpackage.mvn/jvm.configto support Error Prone's compiler plugin access on Java 9+Test plan
Related issues / PRs
Checklist
CONTRIBUTING.mdandCODE_OF_CONDUCT.mdSECURITY.md)https://claude.ai/code/session_01FBmnengTN8krb1Vw46gd2Z