Skip to content

Commit 23dd3db

Browse files
committed
Merge origin/master into validation-refactor
Fix NullAway errors from master's @NullMarked additions to GraphQLSchema and GraphQLTypeUtil. Add null checks in OperationValidator and TraversalContext for nullable getCodeRegistry(), unwrapAll(), and type comparison methods. Update JSpecifyAnnotationsCheck exemption list to match master's annotated classes, removing deleted DeferDirective rule entries. Add @NullMarked to ParseAndValidate to match master.
2 parents b65e8c3 + 2f0dc5b commit 23dd3db

File tree

100 files changed

+723909
-405
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

100 files changed

+723909
-405
lines changed
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
The task is to annotate public API classes (marked with `@PublicAPI`) with JSpecify nullability annotations.
2+
3+
Note that JSpecify is already used in this repository so it's already imported.
4+
5+
If you see a builder static class, you can label it `@NullUnmarked` and not need to do anymore for this static class in terms of annotations.
6+
7+
Analyze this Java class and add JSpecify annotations based on:
8+
1. Set the class to be `@NullMarked`
9+
2. Remove all the redundant `@NonNull` annotations that IntelliJ added
10+
3. Check Javadoc @param tags mentioning "null", "nullable", "may be null"
11+
4. Check Javadoc @return tags mentioning "null", "optional", "if available"
12+
5. Method implementations that return null or check for null
13+
6. GraphQL specification details (see details below)
14+
15+
## GraphQL Specification Compliance
16+
This is a GraphQL implementation. When determining nullability, consult the GraphQL specification (https://spec.graphql.org/draft/) for the relevant concept. Key principles:
17+
18+
The spec defines which elements are required (non-null) vs optional (nullable). Look for keywords like "MUST" to indicate when an element is required, and conditional words such as "IF" to indicate when an element is optional.
19+
20+
If a class implements or represents a GraphQL specification concept, prioritize the spec's nullability requirements over what IntelliJ inferred.
21+
22+
## How to validate
23+
Finally, please check all this works by running the NullAway compile check.
24+
25+
If you find NullAway errors, try and make the smallest possible change to fix them. If you must, you can use assertNotNull. Make sure to include a message as well.
26+
27+
## Formatting Guidelines
28+
29+
Do not make spacing or formatting changes. Avoid adjusting whitespace, line breaks, or other formatting when editing code. These changes make diffs messy and harder to review. Only make the minimal changes necessary to accomplish the task.
30+
31+
## Cleaning up
32+
Finally, can you remove this class from the JSpecifyAnnotationsCheck as an exemption
33+
34+
You do not need to run the JSpecifyAnnotationsCheck. Removing the completed class is enough.
35+
36+
Remember to delete all unused imports when you're done from the class you've just annotated.
37+
38+
## Generics Annotations
39+
40+
When annotating generic types and methods, follow these JSpecify rules:
41+
42+
### Type Parameter Bounds
43+
44+
The bound on a type parameter determines whether nullable type arguments are allowed:
45+
46+
| Declaration | Allows `@Nullable` type argument? |
47+
|-------------|----------------------------------|
48+
| `<T>` | ❌ No — `Box<@Nullable String>` is illegal |
49+
| `<T extends @Nullable Object>` | ✅ Yes — `Box<@Nullable String>` is legal |
50+
51+
**When to use `<T extends @Nullable Object>`:**
52+
- When callers genuinely need to parameterize with nullable types
53+
- Example: `DataFetcherResult<T extends @Nullable Object>` — data fetchers may return nullable types
54+
55+
**When to keep `<T>`:**
56+
- When the type parameter represents a concrete non-null object
57+
- Even if some methods return `@Nullable T` (meaning "can be null even if T is non-null")
58+
- Example: `Edge<T>` with `@Nullable T getNode()` — node may be null, but T represents the object type
59+

.github/workflows/master.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ jobs:
2323
- name: build and test
2424
run: ./gradlew ${{matrix.gradle-argument}} --info --stacktrace
2525
- name: Publish Test Results
26-
uses: EnricoMi/publish-unit-test-result-action@v2.21.0
26+
uses: EnricoMi/publish-unit-test-result-action@v2.22.0
2727
if: always()
2828
with:
2929
files: |

.github/workflows/pull_request.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ jobs:
3232
- name: build and test
3333
run: ./gradlew ${{matrix.gradle-argument}} --info --stacktrace
3434
- name: Publish Test Results
35-
uses: EnricoMi/publish-unit-test-result-action@v2.21.0
35+
uses: EnricoMi/publish-unit-test-result-action@v2.22.0
3636
if: always()
3737
with:
3838
files: |

build.gradle

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ plugins {
1111
id 'maven-publish'
1212
id 'antlr'
1313
id 'signing'
14-
id "com.gradleup.shadow" version "9.0.0"
14+
id "com.gradleup.shadow" version "9.3.1"
1515
id "biz.aQute.bnd.builder" version "7.1.0"
1616
id "io.github.gradle-nexus.publish-plugin" version "2.0.0"
1717
id "groovy"
1818
id "me.champeau.jmh" version "0.7.3"
1919
id "net.ltgt.errorprone" version '4.3.0'
2020
//
2121
// Kotlin just for tests - not production code
22-
id 'org.jetbrains.kotlin.jvm' version '2.2.21'
22+
id 'org.jetbrains.kotlin.jvm' version '2.3.0'
2323
}
2424

2525
java {
@@ -129,11 +129,11 @@ dependencies {
129129

130130
testImplementation 'org.junit.jupiter:junit-jupiter:5.14.1'
131131

132-
testImplementation 'org.spockframework:spock-core:2.3-groovy-4.0'
132+
testImplementation 'org.spockframework:spock-core:2.4-groovy-5.0'
133133
testImplementation 'net.bytebuddy:byte-buddy:1.18.1'
134-
testImplementation 'org.objenesis:objenesis:3.4'
135-
testImplementation 'org.apache.groovy:groovy:4.0.28"'
136-
testImplementation 'org.apache.groovy:groovy-json:4.0.28'
134+
testImplementation 'org.objenesis:objenesis:3.5'
135+
testImplementation 'org.apache.groovy:groovy:5.0.4'
136+
testImplementation 'org.apache.groovy:groovy-json:5.0.4'
137137
testImplementation 'com.google.code.gson:gson:2.13.2'
138138
testImplementation 'org.eclipse.jetty:jetty-server:11.0.26'
139139
testImplementation 'com.fasterxml.jackson.core:jackson-databind:2.20.1'
@@ -223,6 +223,15 @@ tasks.named('jmhJar') {
223223
}
224224
}
225225

226+
jmh {
227+
if (project.hasProperty('jmhInclude')) {
228+
includes = [project.property('jmhInclude')]
229+
}
230+
if (project.hasProperty('jmhProfilers')) {
231+
profilers = [project.property('jmhProfilers')]
232+
}
233+
}
234+
226235

227236
task extractWithoutGuava(type: Copy) {
228237
from({ zipTree({ "build/libs/graphql-java-${project.version}.jar" }) }) {

gradle/wrapper/gradle-wrapper.jar

542 Bytes
Binary file not shown.

gradle/wrapper/gradle-wrapper.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
distributionBase=GRADLE_USER_HOME
22
distributionPath=wrapper/dists
3-
distributionUrl=https\://services.gradle.org/distributions/gradle-9.2.0-bin.zip
3+
distributionUrl=https\://services.gradle.org/distributions/gradle-9.3.1-bin.zip
44
networkTimeout=10000
55
validateDistributionUrl=true
66
zipStoreBase=GRADLE_USER_HOME
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package benchmark;
2+
3+
import graphql.schema.idl.FastSchemaGenerator;
4+
import graphql.schema.idl.RuntimeWiring;
5+
import graphql.schema.idl.SchemaGenerator;
6+
import graphql.schema.idl.SchemaParser;
7+
import graphql.schema.idl.TypeDefinitionRegistry;
8+
import org.openjdk.jmh.annotations.Benchmark;
9+
import org.openjdk.jmh.annotations.BenchmarkMode;
10+
import org.openjdk.jmh.annotations.Fork;
11+
import org.openjdk.jmh.annotations.Measurement;
12+
import org.openjdk.jmh.annotations.Mode;
13+
import org.openjdk.jmh.annotations.OutputTimeUnit;
14+
import org.openjdk.jmh.annotations.Scope;
15+
import org.openjdk.jmh.annotations.Setup;
16+
import org.openjdk.jmh.annotations.State;
17+
import org.openjdk.jmh.annotations.Warmup;
18+
import org.openjdk.jmh.infra.Blackhole;
19+
20+
import java.util.concurrent.TimeUnit;
21+
22+
@Warmup(iterations = 2, time = 5)
23+
@Measurement(iterations = 3)
24+
@Fork(2)
25+
@State(Scope.Benchmark)
26+
public class BuildSchemaBenchmark {
27+
28+
static String largeSDL = BenchmarkUtils.loadResource("large-schema-4.graphqls");
29+
30+
private TypeDefinitionRegistry registry;
31+
32+
@Setup
33+
public void setup() {
34+
// Parse SDL once before benchmarks run
35+
registry = new SchemaParser().parse(largeSDL);
36+
}
37+
38+
@Benchmark
39+
@BenchmarkMode(Mode.AverageTime)
40+
@OutputTimeUnit(TimeUnit.MILLISECONDS)
41+
public void benchmarkBuildSchemaAvgTime(Blackhole blackhole) {
42+
blackhole.consume(new SchemaGenerator().makeExecutableSchema(registry, RuntimeWiring.MOCKED_WIRING));
43+
}
44+
45+
@Benchmark
46+
@BenchmarkMode(Mode.AverageTime)
47+
@OutputTimeUnit(TimeUnit.MILLISECONDS)
48+
public void benchmarkBuildSchemaAvgTimeFast(Blackhole blackhole) {
49+
blackhole.consume(new FastSchemaGenerator().makeExecutableSchema(
50+
SchemaGenerator.Options.defaultOptions().withValidation(false),
51+
registry,
52+
RuntimeWiring.MOCKED_WIRING));
53+
}
54+
}

src/jmh/java/benchmark/CreateSchemaBenchmark.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package benchmark;
22

33
import graphql.schema.GraphQLSchema;
4+
import graphql.schema.idl.FastSchemaGenerator;
45
import graphql.schema.idl.RuntimeWiring;
56
import graphql.schema.idl.SchemaGenerator;
67
import graphql.schema.idl.SchemaParser;
@@ -21,7 +22,7 @@
2122
@Fork(2)
2223
public class CreateSchemaBenchmark {
2324

24-
static String largeSDL = BenchmarkUtils.loadResource("large-schema-3.graphqls");
25+
static String largeSDL = BenchmarkUtils.loadResource("large-schema-4.graphqls");
2526

2627
@Benchmark
2728
@BenchmarkMode(Mode.Throughput)
@@ -37,11 +38,26 @@ public void benchmarkLargeSchemaCreateAvgTime(Blackhole blackhole) {
3738
blackhole.consume(createSchema(largeSDL));
3839
}
3940

41+
@Benchmark
42+
@BenchmarkMode(Mode.AverageTime)
43+
@OutputTimeUnit(TimeUnit.MILLISECONDS)
44+
public void benchmarkLargeSchemaCreateAvgTimeFast(Blackhole blackhole) {
45+
blackhole.consume(createSchemaFast(largeSDL));
46+
}
47+
4048
private static GraphQLSchema createSchema(String sdl) {
4149
TypeDefinitionRegistry registry = new SchemaParser().parse(sdl);
4250
return new SchemaGenerator().makeExecutableSchema(registry, RuntimeWiring.MOCKED_WIRING);
4351
}
4452

53+
private static GraphQLSchema createSchemaFast(String sdl) {
54+
TypeDefinitionRegistry registry = new SchemaParser().parse(sdl);
55+
return new FastSchemaGenerator().makeExecutableSchema(
56+
SchemaGenerator.Options.defaultOptions().withValidation(false),
57+
registry,
58+
RuntimeWiring.MOCKED_WIRING);
59+
}
60+
4561
@SuppressWarnings("InfiniteLoopStatement")
4662
/// make this a main method if you want to run it in JProfiler etc..
4763
public static void mainXXX(String[] args) {
@@ -54,4 +70,4 @@ public static void mainXXX(String[] args) {
5470
}
5571
}
5672
}
57-
}
73+
}

src/main/java/graphql/Assert.java

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import java.util.Collection;
77
import java.util.function.Supplier;
8-
import java.util.regex.Pattern;
98

109
import static java.lang.String.format;
1110

@@ -224,8 +223,6 @@ public static void assertFalse(boolean condition, String msgFmt, Object arg1, Ob
224223

225224
private static final String invalidNameErrorMessage = "Name must be non-null, non-empty and match [_A-Za-z][_0-9A-Za-z]* - was '%s'";
226225

227-
private static final Pattern validNamePattern = Pattern.compile("[_A-Za-z][_0-9A-Za-z]*");
228-
229226
/**
230227
* Validates that the Lexical token name matches the current spec.
231228
* currently non null, non empty,
@@ -234,11 +231,37 @@ public static void assertFalse(boolean condition, String msgFmt, Object arg1, Ob
234231
*
235232
* @return the name if valid, or AssertException if invalid.
236233
*/
237-
public static String assertValidName(String name) {
238-
if (name != null && !name.isEmpty() && validNamePattern.matcher(name).matches()) {
234+
public static String assertValidName(@Nullable String name) {
235+
if (name != null && isValidName(name)) {
239236
return name;
240237
}
241-
return throwAssert(invalidNameErrorMessage, name);
238+
return throwAssert(invalidNameErrorMessage, String.valueOf(name));
239+
}
240+
241+
/**
242+
* Fast character-by-character validation without regex.
243+
* Checks if name matches [_A-Za-z][_0-9A-Za-z]*
244+
*/
245+
private static boolean isValidName(String name) {
246+
if (name.isEmpty()) {
247+
return false;
248+
}
249+
250+
// First character must be [_A-Za-z]
251+
char first = name.charAt(0);
252+
if (!(first == '_' || (first >= 'A' && first <= 'Z') || (first >= 'a' && first <= 'z'))) {
253+
return false;
254+
}
255+
256+
// Remaining characters must be [_0-9A-Za-z]
257+
for (int i = 1; i < name.length(); i++) {
258+
char c = name.charAt(i);
259+
if (!(c == '_' || (c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'))) {
260+
return false;
261+
}
262+
}
263+
264+
return true;
242265
}
243266

244267
private static <T> T throwAssert(String format, Object... args) {

src/main/java/graphql/AssertException.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package graphql;
22

33

4+
import org.jspecify.annotations.NullMarked;
5+
46
@PublicApi
7+
@NullMarked
58
public class AssertException extends GraphQLException {
69

710
public AssertException(String message) {

0 commit comments

Comments
 (0)