Skip to content

Commit 49a74ec

Browse files
author
Mark Pollack
committed
Run Error Prone's bug checks at ERROR beside NullAway
The JDK 21 profile ran Error Prone with every check disabled except NullAway. It now runs every check Error Prone enables by default, and failOnWarning (javac -Werror) makes WARNING-level checks fail the build as well, along with javac's default warnings such as removal. The profile and its CI job are renamed errorprone. StatementSwitchToExpressionSwitch is disabled as style. Two checks are added: SystemOut, since a stray print corrupts a stdio agent's JSON-RPC stream, and CheckReturnValue over reactor.core.publisher, so a dropped Mono or Flux operator (which never runs) fails the build. The Reactor results that may be dropped are listed with reasons in config/errorprone/reactor-ignorable-results.txt.
1 parent 66499ac commit 49a74ec

4 files changed

Lines changed: 82 additions & 14 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ jobs:
4040
retention-days: 7
4141

4242
# The SDK targets and releases on Java 17, but Error Prone (which runs NullAway) needs
43-
# JDK 21. The parent pom's nullaway profile activates on JDK 21+, so this job is the one
44-
# that enforces the @NullMarked packages at ERROR.
45-
nullaway:
43+
# JDK 21. The parent pom's errorprone profile activates on JDK 21+, so this job is the one
44+
# that enforces Error Prone's bug checks and the @NullMarked packages at ERROR.
45+
errorprone:
4646
runs-on: ubuntu-latest
4747
timeout-minutes: 30
4848

@@ -56,7 +56,7 @@ jobs:
5656
distribution: 'temurin'
5757
cache: maven
5858

59-
- name: Build with Maven (NullAway enforced)
59+
- name: Build with Maven (Error Prone and NullAway enforced)
6060
run: ./mvnw -B clean verify
6161

6262
# Issue #14 only reproduced when the JVM was starved of cores: the agent's lock release

‎CHANGELOG.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,20 @@ Found by the NullAway adoption; each has a test.
9595
- `AcpSchema.deserializeJsonRpcMessage` threw `NullPointerException` for the JSON `null` literal
9696
instead of the documented `IllegalArgumentException`.
9797

98+
Found by enabling Error Prone's bug checks; each has a test.
99+
100+
- `StdioAcpClientTransport` read the agent's stdout and stderr in the platform default charset
101+
while writing to it in UTF-8. On Java 17 that charset follows the platform (Cp1252 on Windows,
102+
US-ASCII under a POSIX locale), so non-ASCII text from the agent arrived garbled. Both are now
103+
read as UTF-8.
104+
- The Streamable HTTP client rejected an initialize response whose `Content-Type` was upper case
105+
when the JVM's default locale was Turkish (`"APPLICATION/JSON".toLowerCase()` is
106+
`"applıcatıon/json"` there). The client's and the servlet's media-type checks, and the `os.name`
107+
check in `AgentParameters`, now lower-case with `Locale.ROOT`.
108+
- In `acp-agent-support`, a handler method whose result was not the ACP method's response type
109+
failed with an opaque `ClassCastException` message; the `INTERNAL_ERROR` now names the method and
110+
both types.
111+
98112
### Build
99113

100114
- Architecture rules (ArchUnit) guard the package structure: acp-core's layers (util and json under the
@@ -108,6 +122,15 @@ Found by the NullAway adoption; each has a test.
108122
job; the JDK 17 build and release are unchanged. Compilation uses `--release 17`, so a JDK 21 build
109123
still compiles against the Java 17 API. `.mvn/jvm.config` opens the javac internals Error Prone needs
110124
(harmless on JDK 17).
125+
- Error Prone's own bug checks run beside NullAway, at ERROR, in the JDK 21 build. The profile is
126+
renamed `errorprone` (from `nullaway`), and so is its CI job. Every check Error Prone enables by
127+
default runs, and `failOnWarning` makes its warnings fail the build too (as it does javac's own
128+
default warnings, such as use of an API deprecated for removal). One check is disabled as style,
129+
`StatementSwitchToExpressionSwitch`. Two more are enabled: `SystemOut`, since anything printed to
130+
standard out corrupts a stdio agent's JSON-RPC stream, and `CheckReturnValue` over
131+
`reactor.core.publisher`, so a Mono or Flux operator whose result is dropped (and never runs)
132+
fails the build; `config/errorprone/reactor-ignorable-results.txt` lists the results that may be
133+
dropped, with reasons. Still main sources only, still JDK 21+ only.
111134

112135
## [0.18.0] - 2026-09-25
113136

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Reactor methods whose result the Error Prone build may drop (CheckReturnValue:ApiExclusionList).
2+
# The errorprone profile makes every reactor.core.publisher result must-use, so that an operator
3+
# whose Mono or Flux is dropped (and so never subscribed) fails the build. One signature per line,
4+
# erased parameter types; a line starting with # matches no method.
5+
#
6+
# subscribe: fire-and-forget subscriptions; the Disposable is not needed to keep them running.
7+
reactor.core.publisher.Mono#subscribe()
8+
reactor.core.publisher.Mono#subscribe(java.util.function.Consumer)
9+
reactor.core.publisher.Mono#subscribe(java.util.function.Consumer,java.util.function.Consumer)
10+
reactor.core.publisher.Mono#subscribe(java.util.function.Consumer,java.util.function.Consumer,java.lang.Runnable)
11+
reactor.core.publisher.Mono#subscribe(java.util.function.Consumer,java.util.function.Consumer,java.lang.Runnable,java.util.function.Consumer)
12+
reactor.core.publisher.Mono#subscribe(java.util.function.Consumer,java.util.function.Consumer,java.lang.Runnable,reactor.util.context.Context)
13+
reactor.core.publisher.Flux#subscribe()
14+
reactor.core.publisher.Flux#subscribe(java.util.function.Consumer)
15+
reactor.core.publisher.Flux#subscribe(java.util.function.Consumer,java.util.function.Consumer)
16+
reactor.core.publisher.Flux#subscribe(java.util.function.Consumer,java.util.function.Consumer,java.lang.Runnable)
17+
reactor.core.publisher.Flux#subscribe(java.util.function.Consumer,java.util.function.Consumer,java.lang.Runnable,java.util.function.Consumer)
18+
reactor.core.publisher.Flux#subscribe(java.util.function.Consumer,java.util.function.Consumer,java.lang.Runnable,reactor.util.context.Context)
19+
# Terminal signals: a failed emit almost always means the sink already terminated, which is the
20+
# outcome the caller wants (a racing emission could also refuse it; accepted for terminal signals).
21+
# tryEmitNext is NOT listed: dropping its result silently loses a message.
22+
reactor.core.publisher.Sinks.Empty#tryEmitEmpty()
23+
reactor.core.publisher.Sinks.Empty#tryEmitError(java.lang.Throwable)
24+
reactor.core.publisher.Sinks.One#tryEmitValue(java.lang.Object)
25+
reactor.core.publisher.Sinks.Many#tryEmitComplete()
26+
reactor.core.publisher.Sinks.Many#tryEmitError(java.lang.Throwable)
27+
# Fluent callback registration: returns the same sink.
28+
reactor.core.publisher.FluxSink#onCancel(reactor.core.Disposable)
29+
reactor.core.publisher.MonoSink#onCancel(reactor.core.Disposable)

‎pom.xml‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
<properties>
6060
<java.version>17</java.version>
6161
<!-- release, not source/target: compiles against the Java 17 API even when the build
62-
runs on a newer JDK (the NullAway profile below runs only on JDK 21+). -->
62+
runs on a newer JDK (the errorprone profile below runs only on JDK 21+). -->
6363
<maven.compiler.release>${java.version}</maven.compiler.release>
6464
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
6565
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
@@ -94,7 +94,7 @@
9494
<owasp.version>13.0.0</owasp.version>
9595
<cyclonedx-maven-plugin.version>2.9.3</cyclonedx-maven-plugin.version>
9696

97-
<!-- Null checking (nullaway profile, JDK 21+ only) -->
97+
<!-- Error Prone and NullAway (errorprone profile, JDK 21+ only) -->
9898
<errorprone.version>2.50.0</errorprone.version>
9999
<nullaway.version>0.13.8</nullaway.version>
100100
</properties>
@@ -157,7 +157,7 @@
157157
</dependency>
158158

159159
<!-- Null-marking annotations (@NullMarked, @Nullable). Annotations only; NullAway
160-
enforces them in the nullaway profile. -->
160+
enforces them in the errorprone profile. -->
161161
<dependency>
162162
<groupId>org.jspecify</groupId>
163163
<artifactId>jspecify</artifactId>
@@ -369,18 +369,31 @@
369369
</build>
370370

371371
<profiles>
372-
<!-- NullAway: enforces the JSpecify null-marking of main sources at ERROR.
372+
<!-- Error Prone: Error Prone's own bug checks and NullAway, at ERROR, on main sources.
373373
Error Prone 2.50 runs only on JDK 21+, while the SDK targets Java 17 and its
374-
release build runs on JDK 17, so the check activates by JDK: every build on
374+
release build runs on JDK 17, so the profile activates by JDK: every build on
375375
JDK 21+ (including the CI job that exists for it) is checked, and a JDK 17
376376
build compiles exactly as before. The javac internals Error Prone needs are
377-
opened in .mvn/jvm.config. Scope, per the canonical Java library guide:
378-
- only NullAway, every other Error Prone check disabled (nullness, not lint);
379-
- only @NullMarked packages (OnlyNullMarked), in JSpecify mode;
377+
opened in .mvn/jvm.config. Scope:
378+
- Error Prone's default-enabled checks, and failOnWarning (javac -Werror), so a
379+
check whose default severity is WARNING fails the build like an ERROR one.
380+
-Werror also fails on javac's own default warnings (removal, for one), which
381+
are worth failing on too; suggestions stay notes. A new Error Prone version
382+
brings its new checks in at ERROR: triage them on upgrade.
383+
- Checks that are pure style for this code base are disabled by name below,
384+
each with its reason. A real exception is a @SuppressWarnings with a reason.
385+
- Beyond the defaults, two bug finders that fit this code base:
386+
CheckReturnValue over reactor.core.publisher: a Mono or Flux operator whose
387+
result is dropped never runs. Results that are fine to drop (subscribe's
388+
Disposable, a terminal tryEmit*, fluent sink callbacks) are listed with
389+
reasons in config/errorprone/reactor-ignorable-results.txt.
390+
SystemOut: the stdio transport owns System.out, and anything else printed
391+
there corrupts the JSON-RPC stream.
392+
- NullAway: only @NullMarked packages (OnlyNullMarked), in JSpecify mode.
380393
- only default-compile: test sources are deliberately unchecked;
381394
- never fork the compiler (compiler-plugin then hides every diagnostic). -->
382395
<profile>
383-
<id>nullaway</id>
396+
<id>errorprone</id>
384397
<activation>
385398
<jdk>[21,)</jdk>
386399
</activation>
@@ -393,11 +406,14 @@
393406
<execution>
394407
<id>default-compile</id>
395408
<configuration>
409+
<failOnWarning>true</failOnWarning>
396410
<compilerArgs>
397411
<arg>-XDcompilePolicy=simple</arg>
398412
<arg>--should-stop=ifError=FLOW</arg>
399413
<arg>-XDaddTypeAnnotationsToSymbol=true</arg>
400-
<arg>-Xplugin:ErrorProne -XepDisableAllChecks -Xep:NullAway:ERROR -XepOpt:NullAway:JSpecifyMode=true -XepOpt:NullAway:OnlyNullMarked=true</arg>
414+
<!-- Disabled, as style for this code base:
415+
StatementSwitchToExpressionSwitch: arrow or colon switch is a style choice. -->
416+
<arg>-Xplugin:ErrorProne -Xep:StatementSwitchToExpressionSwitch:OFF -Xep:SystemOut:ERROR -XepOpt:CheckReturnValue:Packages=reactor.core.publisher -XepOpt:CheckReturnValue:ApiExclusionList=${maven.multiModuleProjectDirectory}/config/errorprone/reactor-ignorable-results.txt -Xep:NullAway:ERROR -XepOpt:NullAway:JSpecifyMode=true -XepOpt:NullAway:OnlyNullMarked=true</arg>
401417
</compilerArgs>
402418
<annotationProcessorPaths>
403419
<path>

0 commit comments

Comments
 (0)