Skip to content

Commit 5b7d12b

Browse files
andimarekandimarek
authored
Merge pull request #4002 from graphql-java/errorprone-jspecify-nullaway-support
Adding errorprone support and fix chained dataloader bug
2 parents bf6c50a + a05a319 commit 5b7d12b

File tree

12 files changed

+239
-92
lines changed

12 files changed

+239
-92
lines changed

‎.github/workflows/master.yml‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,18 @@ jobs:
1919
steps:
2020
- uses: actions/checkout@v4
2121
- uses: gradle/actions/wrapper-validation@v4
22-
- name: Set up JDK 11
22+
- name: Set up JDK 21
2323
uses: actions/setup-java@v4
2424
with:
25-
java-version: '11'
25+
java-version: '21'
2626
distribution: 'corretto'
2727
- name: build test and publish
2828
run: ./gradlew assemble && ./gradlew check --info && ./gradlew publishToSonatype closeAndReleaseSonatypeStagingRepository -x check --info --stacktrace
2929
- name: Publish Test Results
3030
uses: EnricoMi/publish-unit-test-result-action@v2.20.0
3131
if: always()
3232
with:
33-
files: '**/build/test-results/test/TEST-*.xml'
33+
files: |
34+
**/build/test-results/test/TEST-*.xml
35+
**/build/test-results/testWithJava11/TEST-*.xml
36+
**/build/test-results/testWithJava17/TEST-*.xml

‎.github/workflows/pull_request.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ jobs:
2121
steps:
2222
- uses: actions/checkout@v4
2323
- uses: gradle/actions/wrapper-validation@v4
24-
- name: Set up JDK 11
24+
- name: Set up JDK 21
2525
uses: actions/setup-java@v4
2626
with:
27-
java-version: '11'
27+
java-version: '21'
2828
distribution: 'corretto'
2929
- name: build and test
3030
run: ./gradlew assemble && ./gradlew check --info --stacktrace

‎.github/workflows/release.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ jobs:
2121
steps:
2222
- uses: actions/checkout@v4
2323
- uses: gradle/actions/wrapper-validation@v4
24-
- name: Set up JDK 11
24+
- name: Set up JDK 21
2525
uses: actions/setup-java@v4
2626
with:
27-
java-version: '11'
27+
java-version: '21'
2828
distribution: 'corretto'
2929
- name: build test and publish
3030
run: ./gradlew assemble && ./gradlew check --info && ./gradlew publishToSonatype closeAndReleaseSonatypeStagingRepository -x check --info --stacktrace

‎build.gradle‎

Lines changed: 104 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
import java.text.SimpleDateFormat
1+
import net.ltgt.gradle.errorprone.CheckSeverity
2+
import org.jetbrains.kotlin.gradle.dsl.JvmTarget
3+
import org.jetbrains.kotlin.gradle.dsl.KotlinVersion
24

5+
import java.text.SimpleDateFormat
36

47
plugins {
58
id 'java'
@@ -12,11 +15,28 @@ plugins {
1215
id "io.github.gradle-nexus.publish-plugin" version "2.0.0"
1316
id "groovy"
1417
id "me.champeau.jmh" version "0.7.3"
18+
id "net.ltgt.errorprone" version '4.2.0'
19+
//
20+
// Kotlin just for tests - not production code
21+
id 'org.jetbrains.kotlin.jvm' version '2.1.21'
1522
}
1623

1724
java {
1825
toolchain {
19-
languageVersion = JavaLanguageVersion.of(11)
26+
languageVersion = JavaLanguageVersion.of(21) // build on 21 - release on 11
27+
}
28+
}
29+
30+
kotlin {
31+
compilerOptions {
32+
apiVersion = KotlinVersion.KOTLIN_2_0
33+
languageVersion = KotlinVersion.KOTLIN_2_0
34+
jvmTarget = JvmTarget.JVM_11
35+
javaParameters = true
36+
freeCompilerArgs = [
37+
'-Xemit-jvm-type-annotations',
38+
'-Xjspecify-annotations=strict',
39+
]
2040
}
2141
}
2242

@@ -97,19 +117,15 @@ jar {
97117
attributes('Automatic-Module-Name': 'com.graphqljava')
98118
}
99119
}
100-
tasks.withType(GroovyCompile) {
101-
// Options when compiling Java using the Groovy plugin.
102-
// (Groovy itself defaults to UTF-8 for Groovy code)
103-
options.encoding = 'UTF-8'
104-
groovyOptions.forkOptions.memoryMaximumSize = "4g"
105-
}
120+
106121
dependencies {
107-
implementation 'org.antlr:antlr4-runtime:' + antlrVersion
108122
api 'com.graphql-java:java-dataloader:5.0.0'
109123
api 'org.reactivestreams:reactive-streams:' + reactiveStreamsVersion
110124
api "org.jspecify:jspecify:1.0.0"
111-
antlr 'org.antlr:antlr4:' + antlrVersion
125+
126+
implementation 'org.antlr:antlr4-runtime:' + antlrVersion
112127
implementation 'com.google.guava:guava:' + guavaVersion
128+
113129
testImplementation group: 'junit', name: 'junit', version: '4.13.2'
114130
testImplementation 'org.spockframework:spock-core:2.3-groovy-4.0'
115131
testImplementation 'net.bytebuddy:byte-buddy:1.17.5'
@@ -129,9 +145,17 @@ dependencies {
129145
testImplementation 'org.testng:testng:7.11.0' // use for reactive streams test inheritance
130146
testImplementation "com.tngtech.archunit:archunit-junit5:1.4.1"
131147

148+
antlr 'org.antlr:antlr4:' + antlrVersion
149+
132150
// this is needed for the idea jmh plugin to work correctly
133151
jmh 'org.openjdk.jmh:jmh-core:1.37'
134152
jmh 'org.openjdk.jmh:jmh-generator-annprocess:1.37'
153+
154+
errorprone 'com.uber.nullaway:nullaway:0.12.6'
155+
errorprone 'com.google.errorprone:error_prone_core:2.37.0'
156+
157+
// just tests - no Kotlin otherwise
158+
testCompileOnly 'org.jetbrains.kotlin:kotlin-stdlib-jdk8'
135159
}
136160

137161
shadowJar {
@@ -218,6 +242,36 @@ compileJava {
218242
source file("build/generated-src"), sourceSets.main.java
219243
}
220244

245+
tasks.withType(GroovyCompile) {
246+
// Options when compiling Java using the Groovy plugin.
247+
// (Groovy itself defaults to UTF-8 for Groovy code)
248+
options.encoding = 'UTF-8'
249+
sourceCompatibility = '11'
250+
targetCompatibility = '11'
251+
groovyOptions.forkOptions.memoryMaximumSize = "4g"
252+
}
253+
254+
tasks.withType(JavaCompile) {
255+
options.release = 11
256+
options.errorprone {
257+
disableAllChecks = true
258+
check("NullAway", CheckSeverity.ERROR)
259+
//
260+
// end state has us with this config turned on - eg all classes
261+
//
262+
//option("NullAway:AnnotatedPackages", "graphql")
263+
option("NullAway:CustomContractAnnotations", "graphql.Contract")
264+
option("NullAway:OnlyNullMarked", "true")
265+
option("NullAway:JSpecifyMode", "true")
266+
}
267+
// Include to disable NullAway on test code
268+
if (name.toLowerCase().contains("test")) {
269+
options.errorprone {
270+
disable("NullAway")
271+
}
272+
}
273+
}
274+
221275
generateGrammarSource {
222276
includes = ['Graphql.g4']
223277
maxHeapSize = "64m"
@@ -253,6 +307,7 @@ artifacts {
253307
List<TestDescriptor> failedTests = []
254308

255309
test {
310+
useJUnitPlatform()
256311
testLogging {
257312
events "FAILED", "SKIPPED"
258313
exceptionFormat = "FULL"
@@ -265,6 +320,43 @@ test {
265320
}
266321
}
267322

323+
tasks.register('testWithJava17', Test) {
324+
javaLauncher = javaToolchains.launcherFor {
325+
languageVersion = JavaLanguageVersion.of(17)
326+
}
327+
useJUnitPlatform()
328+
testLogging {
329+
events "FAILED", "SKIPPED"
330+
exceptionFormat = "FULL"
331+
}
332+
333+
afterTest { TestDescriptor descriptor, TestResult result ->
334+
if (result.getFailedTestCount() > 0) {
335+
failedTests.add(descriptor)
336+
}
337+
}
338+
339+
}
340+
tasks.register('testWithJava11', Test) {
341+
javaLauncher = javaToolchains.launcherFor {
342+
languageVersion = JavaLanguageVersion.of(11)
343+
}
344+
useJUnitPlatform()
345+
testLogging {
346+
events "FAILED", "SKIPPED"
347+
exceptionFormat = "FULL"
348+
}
349+
350+
afterTest { TestDescriptor descriptor, TestResult result ->
351+
if (result.getFailedTestCount() > 0) {
352+
failedTests.add(descriptor)
353+
}
354+
}
355+
}
356+
test.dependsOn testWithJava17
357+
test.dependsOn testWithJava11
358+
359+
268360
/*
269361
* The gradle.buildFinished callback is deprecated BUT there does not seem to be a decent alternative in gradle 7
270362
* So progress over perfection here
@@ -378,6 +470,5 @@ tasks.withType(GenerateModuleMetadata) {
378470
enabled = false
379471
}
380472

381-
test {
382-
useJUnitPlatform()
383-
}
473+
474+

‎src/main/java/graphql/Assert.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql;
22

33
import org.jspecify.annotations.NullMarked;
4+
import org.jspecify.annotations.Nullable;
45

56
import java.util.Collection;
67
import java.util.function.Supplier;
@@ -20,71 +21,82 @@ public static <T> T assertNotNullWithNPE(T object, Supplier<String> msg) {
2021
throw new NullPointerException(msg.get());
2122
}
2223

23-
public static <T> T assertNotNull(T object) {
24+
@Contract("null -> fail")
25+
public static <T> T assertNotNull(@Nullable T object) {
2426
if (object != null) {
2527
return object;
2628
}
2729
return throwAssert("Object required to be not null");
2830
}
2931

30-
public static <T> T assertNotNull(T object, Supplier<String> msg) {
32+
@Contract("null,_ -> fail")
33+
public static <T> T assertNotNull(@Nullable T object, Supplier<String> msg) {
3134
if (object != null) {
3235
return object;
3336
}
3437
return throwAssert(msg.get());
3538
}
3639

37-
public static <T> T assertNotNull(T object, String constantMsg) {
40+
@Contract("null,_ -> fail")
41+
public static <T> T assertNotNull(@Nullable T object, String constantMsg) {
3842
if (object != null) {
3943
return object;
4044
}
4145
return throwAssert(constantMsg);
4246
}
4347

44-
public static <T> T assertNotNull(T object, String msgFmt, Object arg1) {
48+
@Contract("null,_,_ -> fail")
49+
public static <T> T assertNotNull(@Nullable T object, String msgFmt, Object arg1) {
4550
if (object != null) {
4651
return object;
4752
}
4853
return throwAssert(msgFmt, arg1);
4954
}
5055

51-
public static <T> T assertNotNull(T object, String msgFmt, Object arg1, Object arg2) {
56+
@Contract("null,_,_,_ -> fail")
57+
public static <T> T assertNotNull(@Nullable T object, String msgFmt, Object arg1, Object arg2) {
5258
if (object != null) {
5359
return object;
5460
}
5561
return throwAssert(msgFmt, arg1, arg2);
5662
}
5763

58-
public static <T> T assertNotNull(T object, String msgFmt, Object arg1, Object arg2, Object arg3) {
64+
@Contract("null,_,_,_,_ -> fail")
65+
public static <T> T assertNotNull(@Nullable T object, String msgFmt, Object arg1, Object arg2, Object arg3) {
5966
if (object != null) {
6067
return object;
6168
}
6269
return throwAssert(msgFmt, arg1, arg2, arg3);
6370
}
6471

6572

66-
public static <T> void assertNull(T object, Supplier<String> msg) {
73+
@Contract("!null,_ -> fail")
74+
public static <T> void assertNull(@Nullable T object, Supplier<String> msg) {
6775
if (object == null) {
6876
return;
6977
}
7078
throwAssert(msg.get());
7179
}
7280

73-
public static <T> void assertNull(T object) {
81+
@Contract("!null -> fail")
82+
public static <T> void assertNull(@Nullable Object object) {
7483
if (object == null) {
7584
return;
7685
}
7786
throwAssert("Object required to be null");
7887
}
7988

89+
@Contract("-> fail")
8090
public static <T> T assertNeverCalled() {
8191
return throwAssert("Should never been called");
8292
}
8393

94+
@Contract("_,_-> fail")
8495
public static <T> T assertShouldNeverHappen(String format, Object... args) {
8596
return throwAssert("Internal error: should never happen: %s", format(format, args));
8697
}
8798

99+
@Contract("-> fail")
88100
public static <T> T assertShouldNeverHappen() {
89101
return throwAssert("Internal error: should never happen");
90102
}
@@ -96,6 +108,7 @@ public static <T> Collection<T> assertNotEmpty(Collection<T> collection) {
96108
return collection;
97109
}
98110

111+
// @Contract("null,_-> fail")
99112
public static <T> Collection<T> assertNotEmpty(Collection<T> collection, Supplier<String> msg) {
100113
if (collection == null || collection.isEmpty()) {
101114
throwAssert(msg.get());

‎src/main/java/graphql/Contract.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package graphql;
2+
3+
import java.lang.annotation.Documented;
4+
import java.lang.annotation.ElementType;
5+
import java.lang.annotation.Target;
6+
7+
/**
8+
* Custom contract annotation used for jspecify and NullAway checks.
9+
*
10+
* This is the same as Spring does: we don't want any additional dependencies, therefore we define our own Contract annotation.
11+
*
12+
* @see <a href="https://raw.githubusercontent.com/spring-projects/spring-framework/refs/heads/main/spring-core/src/main/java/org/springframework/lang/Contract.java">Spring Framework Contract</a>
13+
* @see <a href="https://github.com/JetBrains/java-annotations/blob/master/src/jvmMain/java/org/jetbrains/annotations/Contract.java">org.jetbrains.annotations.Contract</a>
14+
* @see <a href="https://github.com/uber/NullAway/wiki/Configuration#custom-contract-annotations">
15+
* NullAway custom contract annotations</a>
16+
*/
17+
@Documented
18+
@Target(ElementType.METHOD)
19+
@Internal
20+
public @interface Contract {
21+
22+
/**
23+
* Describing the contract between call arguments and the returned value.
24+
*/
25+
String value() default "";
26+
27+
}

0 commit comments

Comments
 (0)