Skip to content

Commit bb14d56

Browse files
msridharmsridhar
authored
Fail if NullAway is run in JSpecify mode with an incompatible javac version / configuration (#1317)
Fixes #1310 NullAway now fails with an exception if run on an incompatible javac configuration, i.e., a configuration that does not support reading type use annotations from bytecode (see https://github.com/uber/NullAway/wiki/JSpecify-Support#supported-jdk-versions). Specifically, NullAway fails if it is running on JDK version before 22 and the `-XDaddTypeAnnotationsToSymbol=true` has not been passed to `javac`. Note that `javac` versions that do not recognize the `-XDaddTypeAnnotationsToSymbol` flag simply ignore it, so this check does not ensure we are on a compatible JDK. But, it's better than nothing, and doing a more careful check would be hard, since some distributions of the same JDK version may not support the flag. Also, there is hope for this flag to be backported to JDK 17u, and this way we won't need to change our logic when that happens. Documentation of supported versions is at https://github.com/uber/NullAway/wiki/JSpecify-Support#supported-jdk-versions and that URL is included in the error message. When this PR lands, we will update the docs to indicate the stronger version requirement. Rather than passing `"-XDaddTypeAnnotationsToSymbol=true"` in many places in our test code, we introduce a `JSpecifyJavacConfig` test helper class and use it wherever we want to run tests in JSpecify mode, to ensure the right flags are passed together. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Added JDK version validation for JSpecify mode, ensuring compatibility with JDK 22+ and required type annotation configurations. * **Tests** * Enhanced test infrastructure to centralize JSpecify configuration handling across test suites. * Added validation test for JDK compatibility requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 9907586 commit bb14d56

File tree

17 files changed

+269
-158
lines changed

17 files changed

+269
-158
lines changed

‎guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import com.google.errorprone.CompilationTestHelper;
2525
import com.uber.nullaway.NullAway;
26+
import com.uber.nullaway.generics.JSpecifyJavacConfig;
2627
import java.util.Arrays;
2728
import java.util.List;
2829
import org.junit.Assume;
@@ -58,12 +59,11 @@ public void setup() {
5859
jspecifyCompilationHelper =
5960
CompilationTestHelper.newInstance(NullAway.class, getClass())
6061
.setArgs(
61-
List.of(
62-
"-d",
63-
temporaryFolder.getRoot().getAbsolutePath(),
64-
"-XepOpt:NullAway:OnlyNullMarked=true",
65-
"-XepOpt:NullAway:JSpecifyMode=true",
66-
"-XDaddTypeAnnotationsToSymbol=true"));
62+
JSpecifyJavacConfig.withJSpecifyModeArgs(
63+
List.of(
64+
"-d",
65+
temporaryFolder.getRoot().getAbsolutePath(),
66+
"-XepOpt:NullAway:OnlyNullMarked=true")));
6767
}
6868

6969
@Test

‎jdk-annotations/jdk-integration-test/src/test/java/com/uber/nullaway/jdkannotations/JDKIntegrationTest.java‎

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.google.errorprone.CompilationTestHelper;
44
import com.uber.nullaway.NullAway;
5+
import com.uber.nullaway.generics.JSpecifyJavacConfig;
56
import java.util.Arrays;
67
import org.junit.Before;
78
import org.junit.Rule;
@@ -126,12 +127,12 @@ public void libraryModelInnerClassNullableReturnsTest() {
126127
public void libraryModelInnerClassNullableUpperBoundsTest() {
127128
compilationHelper
128129
.setArgs(
129-
Arrays.asList(
130-
"-d",
131-
temporaryFolder.getRoot().getAbsolutePath(),
132-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
133-
"-XepOpt:NullAway:JSpecifyMode=true",
134-
"-XepOpt:NullAway:JarInferEnabled=true"))
130+
JSpecifyJavacConfig.withJSpecifyModeArgs(
131+
Arrays.asList(
132+
"-d",
133+
temporaryFolder.getRoot().getAbsolutePath(),
134+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
135+
"-XepOpt:NullAway:JarInferEnabled=true")))
135136
.addSourceLines(
136137
"Test.java",
137138
"package com.uber;",
@@ -153,11 +154,11 @@ public void libraryModelInnerClassNullableUpperBoundsTest() {
153154
public void libraryModelNullableUpperBoundsWithoutJarInferTest() {
154155
compilationHelper
155156
.setArgs(
156-
Arrays.asList(
157-
"-d",
158-
temporaryFolder.getRoot().getAbsolutePath(),
159-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
160-
"-XepOpt:NullAway:JSpecifyMode=true"))
157+
JSpecifyJavacConfig.withJSpecifyModeArgs(
158+
Arrays.asList(
159+
"-d",
160+
temporaryFolder.getRoot().getAbsolutePath(),
161+
"-XepOpt:NullAway:AnnotatedPackages=com.uber")))
161162
.addSourceLines(
162163
"Test.java",
163164
"package com.uber;",
@@ -174,12 +175,12 @@ public void libraryModelNullableUpperBoundsWithoutJarInferTest() {
174175
public void libraryModelDefaultParameterNullabilityTest() {
175176
compilationHelper
176177
.setArgs(
177-
Arrays.asList(
178-
"-d",
179-
temporaryFolder.getRoot().getAbsolutePath(),
180-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
181-
"-XepOpt:NullAway:JSpecifyMode=true",
182-
"-XepOpt:NullAway:JarInferEnabled=true"))
178+
JSpecifyJavacConfig.withJSpecifyModeArgs(
179+
Arrays.asList(
180+
"-d",
181+
temporaryFolder.getRoot().getAbsolutePath(),
182+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
183+
"-XepOpt:NullAway:JarInferEnabled=true")))
183184
.addSourceLines(
184185
"Test.java",
185186
"package com.uber;",
@@ -198,12 +199,12 @@ public void libraryModelDefaultParameterNullabilityTest() {
198199
public void libraryModelParameterNullabilityTest() {
199200
compilationHelper
200201
.setArgs(
201-
Arrays.asList(
202-
"-d",
203-
temporaryFolder.getRoot().getAbsolutePath(),
204-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
205-
"-XepOpt:NullAway:JSpecifyMode=true",
206-
"-XepOpt:NullAway:JarInferEnabled=true"))
202+
JSpecifyJavacConfig.withJSpecifyModeArgs(
203+
Arrays.asList(
204+
"-d",
205+
temporaryFolder.getRoot().getAbsolutePath(),
206+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
207+
"-XepOpt:NullAway:JarInferEnabled=true")))
207208
.addSourceLines(
208209
"Test.java",
209210
"package com.uber;",
@@ -225,12 +226,12 @@ public void libraryModelParameterNullabilityTest() {
225226
public void nullableArrayTest() {
226227
compilationHelper
227228
.setArgs(
228-
Arrays.asList(
229-
"-d",
230-
temporaryFolder.getRoot().getAbsolutePath(),
231-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
232-
"-XepOpt:NullAway:JSpecifyMode=true",
233-
"-XepOpt:NullAway:JarInferEnabled=true"))
229+
JSpecifyJavacConfig.withJSpecifyModeArgs(
230+
Arrays.asList(
231+
"-d",
232+
temporaryFolder.getRoot().getAbsolutePath(),
233+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
234+
"-XepOpt:NullAway:JarInferEnabled=true")))
234235
.addSourceLines(
235236
"Test.java",
236237
"package com.uber;",
@@ -251,12 +252,12 @@ public void nullableArrayTest() {
251252
public void nullableGenericArrayTest() {
252253
compilationHelper
253254
.setArgs(
254-
Arrays.asList(
255-
"-d",
256-
temporaryFolder.getRoot().getAbsolutePath(),
257-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
258-
"-XepOpt:NullAway:JSpecifyMode=true",
259-
"-XepOpt:NullAway:JarInferEnabled=true"))
255+
JSpecifyJavacConfig.withJSpecifyModeArgs(
256+
Arrays.asList(
257+
"-d",
258+
temporaryFolder.getRoot().getAbsolutePath(),
259+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
260+
"-XepOpt:NullAway:JarInferEnabled=true")))
260261
.addSourceLines(
261262
"Test.java",
262263
"package com.uber;",
@@ -280,12 +281,12 @@ public void nullableGenericArrayTest() {
280281
public void nullableGenericReturnTest() {
281282
compilationHelper
282283
.setArgs(
283-
Arrays.asList(
284-
"-d",
285-
temporaryFolder.getRoot().getAbsolutePath(),
286-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
287-
"-XepOpt:NullAway:JSpecifyMode=true",
288-
"-XepOpt:NullAway:JarInferEnabled=true"))
284+
JSpecifyJavacConfig.withJSpecifyModeArgs(
285+
Arrays.asList(
286+
"-d",
287+
temporaryFolder.getRoot().getAbsolutePath(),
288+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
289+
"-XepOpt:NullAway:JarInferEnabled=true")))
289290
.addSourceLines(
290291
"Test.java",
291292
"package com.uber;",
@@ -306,12 +307,12 @@ public void nullableGenericReturnTest() {
306307
public void genericParameterTest() {
307308
compilationHelper
308309
.setArgs(
309-
Arrays.asList(
310-
"-d",
311-
temporaryFolder.getRoot().getAbsolutePath(),
312-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
313-
"-XepOpt:NullAway:JSpecifyMode=true",
314-
"-XepOpt:NullAway:JarInferEnabled=true"))
310+
JSpecifyJavacConfig.withJSpecifyModeArgs(
311+
Arrays.asList(
312+
"-d",
313+
temporaryFolder.getRoot().getAbsolutePath(),
314+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
315+
"-XepOpt:NullAway:JarInferEnabled=true")))
315316
.addSourceLines(
316317
"Test.java",
317318
"package com.uber;",

‎library-model/library-model-generator-integration-test/src/test/java/com/uber/nullaway/libmodel/LibraryModelIntegrationTest.java‎

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.google.errorprone.CompilationTestHelper;
44
import com.uber.nullaway.NullAway;
5+
import com.uber.nullaway.generics.JSpecifyJavacConfig;
56
import java.util.Arrays;
67
import org.junit.Before;
78
import org.junit.Rule;
@@ -132,12 +133,12 @@ public void libraryModelInnerClassNullableReturnsTest() {
132133
public void libraryModelInnerClassNullableUpperBoundsTest() {
133134
compilationHelper
134135
.setArgs(
135-
Arrays.asList(
136-
"-d",
137-
temporaryFolder.getRoot().getAbsolutePath(),
138-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
139-
"-XepOpt:NullAway:JSpecifyMode=true",
140-
"-XepOpt:NullAway:JarInferEnabled=true"))
136+
JSpecifyJavacConfig.withJSpecifyModeArgs(
137+
Arrays.asList(
138+
"-d",
139+
temporaryFolder.getRoot().getAbsolutePath(),
140+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
141+
"-XepOpt:NullAway:JarInferEnabled=true")))
141142
.addSourceLines(
142143
"Test.java",
143144
"package com.uber;",
@@ -159,11 +160,11 @@ public void libraryModelInnerClassNullableUpperBoundsTest() {
159160
public void libraryModelNullableUpperBoundsWithoutJarInferTest() {
160161
compilationHelper
161162
.setArgs(
162-
Arrays.asList(
163-
"-d",
164-
temporaryFolder.getRoot().getAbsolutePath(),
165-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
166-
"-XepOpt:NullAway:JSpecifyMode=true"))
163+
JSpecifyJavacConfig.withJSpecifyModeArgs(
164+
Arrays.asList(
165+
"-d",
166+
temporaryFolder.getRoot().getAbsolutePath(),
167+
"-XepOpt:NullAway:AnnotatedPackages=com.uber")))
167168
.addSourceLines(
168169
"Test.java",
169170
"package com.uber;",
@@ -180,12 +181,12 @@ public void libraryModelNullableUpperBoundsWithoutJarInferTest() {
180181
public void libraryModelDefaultParameterNullabilityTest() {
181182
compilationHelper
182183
.setArgs(
183-
Arrays.asList(
184-
"-d",
185-
temporaryFolder.getRoot().getAbsolutePath(),
186-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
187-
"-XepOpt:NullAway:JSpecifyMode=true",
188-
"-XepOpt:NullAway:JarInferEnabled=true"))
184+
JSpecifyJavacConfig.withJSpecifyModeArgs(
185+
Arrays.asList(
186+
"-d",
187+
temporaryFolder.getRoot().getAbsolutePath(),
188+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
189+
"-XepOpt:NullAway:JarInferEnabled=true")))
189190
.addSourceLines(
190191
"Test.java",
191192
"package com.uber;",
@@ -204,12 +205,12 @@ public void libraryModelDefaultParameterNullabilityTest() {
204205
public void libraryModelParameterNullabilityTest() {
205206
compilationHelper
206207
.setArgs(
207-
Arrays.asList(
208-
"-d",
209-
temporaryFolder.getRoot().getAbsolutePath(),
210-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
211-
"-XepOpt:NullAway:JSpecifyMode=true",
212-
"-XepOpt:NullAway:JarInferEnabled=true"))
208+
JSpecifyJavacConfig.withJSpecifyModeArgs(
209+
Arrays.asList(
210+
"-d",
211+
temporaryFolder.getRoot().getAbsolutePath(),
212+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
213+
"-XepOpt:NullAway:JarInferEnabled=true")))
213214
.addSourceLines(
214215
"Test.java",
215216
"package com.uber;",
@@ -231,12 +232,12 @@ public void libraryModelParameterNullabilityTest() {
231232
public void nullableArrayTest() {
232233
compilationHelper
233234
.setArgs(
234-
Arrays.asList(
235-
"-d",
236-
temporaryFolder.getRoot().getAbsolutePath(),
237-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
238-
"-XepOpt:NullAway:JSpecifyMode=true",
239-
"-XepOpt:NullAway:JarInferEnabled=true"))
235+
JSpecifyJavacConfig.withJSpecifyModeArgs(
236+
Arrays.asList(
237+
"-d",
238+
temporaryFolder.getRoot().getAbsolutePath(),
239+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
240+
"-XepOpt:NullAway:JarInferEnabled=true")))
240241
.addSourceLines(
241242
"Test.java",
242243
"package com.uber;",
@@ -257,12 +258,12 @@ public void nullableArrayTest() {
257258
public void genericParameterTest() {
258259
compilationHelper
259260
.setArgs(
260-
Arrays.asList(
261-
"-d",
262-
temporaryFolder.getRoot().getAbsolutePath(),
263-
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
264-
"-XepOpt:NullAway:JSpecifyMode=true",
265-
"-XepOpt:NullAway:JarInferEnabled=true"))
261+
JSpecifyJavacConfig.withJSpecifyModeArgs(
262+
Arrays.asList(
263+
"-d",
264+
temporaryFolder.getRoot().getAbsolutePath(),
265+
"-XepOpt:NullAway:AnnotatedPackages=com.uber",
266+
"-XepOpt:NullAway:JarInferEnabled=true")))
266267
.addSourceLines(
267268
"Test.java",
268269
"package com.uber;",

‎nullaway/src/main/java/com/uber/nullaway/NullAway.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
import com.uber.nullaway.dataflow.AccessPathNullnessAnalysis;
9999
import com.uber.nullaway.dataflow.EnclosingEnvironmentNullness;
100100
import com.uber.nullaway.generics.GenericsChecks;
101+
import com.uber.nullaway.generics.JSpecifyJavacConfig;
101102
import com.uber.nullaway.handlers.Handler;
102103
import com.uber.nullaway.handlers.Handlers;
103104
import com.uber.nullaway.handlers.MethodAnalysisContext;
@@ -227,6 +228,8 @@ private enum NullMarking {
227228
@SuppressWarnings("NullAway.Init")
228229
private CodeAnnotationInfo codeAnnotationInfo;
229230

231+
private boolean checkedJDKVersionForJSpecifyMode = false;
232+
230233
private final Config config;
231234

232235
/** Returns the configuration being used for this analysis. */
@@ -1703,6 +1706,17 @@ public Description matchClass(ClassTree tree, VisitorState state) {
17031706
if (codeAnnotationInfo == null) {
17041707
codeAnnotationInfo = CodeAnnotationInfo.instance(state.context);
17051708
}
1709+
if (!checkedJDKVersionForJSpecifyMode) {
1710+
checkedJDKVersionForJSpecifyMode = true;
1711+
if (config.isJSpecifyMode()
1712+
&& !JSpecifyJavacConfig.isValidJavacConfigForJSpecifyMode(state)) {
1713+
String msg =
1714+
"Running NullAway in JSpecify mode requires either JDK 22+"
1715+
+ " or passing the flag -XDaddTypeAnnotationsToSymbol=true to an older JDK that supports it;"
1716+
+ " see https://github.com/uber/NullAway/wiki/JSpecify-Support#supported-jdk-versions for details.";
1717+
throw new IllegalStateException(msg);
1718+
}
1719+
}
17061720
// Check if the class is excluded according to the filter
17071721
// if so, set the flag to match within the class to false
17081722
// NOTE: for this mechanism to work, we rely on the enclosing ClassTree

0 commit comments

Comments
 (0)