Add JSpecify annotations to 10 language package classes by Copilot · Pull Request #4218 · graphql-java/graphql-java

Copilot · 2026-01-25T06:36:18Z

Annotates 10 classes in graphql.language package with JSpecify nullability annotations and removes them from the exemption list.

Annotated Classes

DirectiveLocation, DirectivesContainer (interface), Document
EnumTypeDefinition, EnumTypeExtensionDefinition, EnumValueDefinition
Field, FieldDefinition, FragmentDefinition, FragmentSpread

Approach

Field nullability: Core fields (name, type, typeCondition) are marked @Nullable to allow test code to create incomplete instances for validation testing.

Getter enforcement: Getters use assertNotNull() to satisfy NamedNode contract and fail fast:

private final @Nullable String name;

@Override
public String getName() {
    return assertNotNull(name, () -> "name cannot be null");
}

Builder leniency: Builders accept nullable values without validation, supporting existing test patterns that create invalid AST nodes.

List copying: deepCopy() methods use assertNotNull() on list results since AbstractNode's deepCopy(List) returns @Nullable List.

Original prompt

Task

Add JSpecify annotations to the following 10 classes in the graphql.language package, following the established JSpecify annotation pattern documented in .claude/commands/jspecify-annotate.md.

Classes to annotate

graphql.language.DirectiveLocation

graphql.language.DirectivesContainer

graphql.language.Document

graphql.language.EnumTypeDefinition

graphql.language.EnumTypeExtensionDefinition

graphql.language.EnumValueDefinition

graphql.language.Field

graphql.language.FieldDefinition

graphql.language.FragmentDefinition

graphql.language.FragmentSpread

Instructions

Follow the JSpecify annotation process documented in .claude/commands/jspecify-annotate.md:

Set each class to be @NullMarked at the class level

Remove all redundant @NonNull annotations that IntelliJ may have added

Check Javadoc for @param tags mentioning "null", "nullable", "may be null"

Check Javadoc @return tags mentioning "null", "optional", "if available"

Inspect method implementations that return null or check for null

Consider GraphQL specification details when determining nullability

For Builder static classes, label them @NullUnmarked and no further annotations needed

Validation

After making changes, run the NullAway compile check:
./gradlew compileJava
If you find NullAway errors, make the smallest possible change to fix them. You can use assertNotNull with a message if needed.

Cleanup

Remove each completed class from the exemption list in src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy

Delete all unused imports from the classes you've annotated

Do not make spacing or formatting changes - only make minimal changes necessary for the annotations

Reference

Look at existing annotated classes like IntValue.java, StringValue.java, AbstractNode.java in the same package for reference patterns.

This pull request was created from Copilot chat.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: dondonz <13839920+dondonz@users.noreply.github.com>

# Conflicts: # .claude/commands/jspecify-annotate.md

github-actions · 2026-02-15T08:16:51Z

Test Results

0 files - 335 0 suites - 335 0s ⏱️ - 5m 5s
0 tests - 5 371 0 ✅ - 5 363 0 💤 - 8 0 ❌ ±0
0 runs - 5 460 0 ✅ - 5 452 0 💤 - 8 0 ❌ ±0

Results for commit baa9a0e. ± Comparison against base commit 89b4446.

github-actions · 2026-02-15T08:16:51Z

Test Results

335 files ±0 335 suites ±0 5m 5s ⏱️ ±0s
5 371 tests ±0 5 362 ✅ - 1 9 💤 +1 0 ❌ ±0
5 460 runs ±0 5 451 ✅ - 1 9 💤 +1 0 ❌ ±0

Results for commit 72ea92a. ± Comparison against base commit 89b4446.

This pull request removes 204 and adds 180 tests. Note that renamed tests count towards both.

	?

	, expected: combo-\"\\\b\f\n\r\t, #4]
                __schema { types { fields { args { type { name fields { name }}}}}}
                __schema { types { fields { type { name fields { name }}}}}
                __schema { types { inputFields { type { inputFields { name }}}}}
                __schema { types { interfaces { fields { type { interfaces { name } } } } } }
                __schema { types { name} }
                __type(name : "t") { name }
                a1: __schema { types { name} }
                a1: __type(name : "t") { name }
…

graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure23@2cc75074 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure24@445bb139 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure25@b9a77c8 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure20@489091bd delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure21@512d6e60 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure22@1de9b505 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure3@73844119 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure4@2f4c2cd4 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure5@561b7d53 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of error args with non null does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_6prov0_closure6@4f89331f delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
…

This pull request skips 1 test.

graphql.schema.fetching.LambdaFetchingSupportTest ‑ different class loaders induce certain behaviours

♻️ This comment has been updated with latest results.

dondonz · 2026-02-15T19:52:44Z

 Note that JSpecify is already used in this repository so it's already imported.

-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.
+**IMPORTANT: Builder classes MUST be annotated with `@NullUnmarked`.** When you encounter a `public static final class Builder` or `public static class Builder` inside a `@NullMarked` class, you MUST:


A number of builder annotations were missed, so I've updated the prompt

Interesting there is a difference between "you can" and "you must" for LLMs

dondonz · 2026-02-17T21:18:35Z

Item for discussion - this now enforces a selection set on fragment definition (as per spec), and now requires fields to have a name

Resolve JSpecifyAnnotationsCheck conflict by taking master's exemption list updates. Accept master's deletion of FragmentsOnCompositeTypeTest from rules/ (moved to validation/). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

NodeVisitor, NodeVisitorStub, SourceLocation, and Type are already annotated with @NullMarked so they should not be in the exemption list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-03-21T07:08:43Z

Test Report

Test Results

Java Version	Total	Passed	Failed	Errors	Skipped
Java 11	5708 (±0)	5652 (±0)	0 (±0)	0 (±0)	56 (±0)
Java 17	5708 (±0)	5651 (±0)	0 (±0)	0 (±0)	57 (±0)
Java 21	5708 (±0)	5651 (±0)	0 (±0)	0 (±0)	57 (±0)
Java 25	5708 (±0)	5651 (±0)	0 (±0)	0 (±0)	57 (±0)
jcstress	32 (±0)	32 (±0)	0 (±0)	0 (±0)	0 (±0)
Total	22864 (±0)	22637 (±0)	0 (±0)	0 (±0)	227 (±0)

Code Coverage (Java 25)

Metric	Covered	Missed	Coverage	vs Master
Lines	28777	3121	90.2%	±0.0%
Branches	8352	1506	84.7%	±0.0%
Methods	7698	1222	86.3%	±0.0%

Changed Class Coverage (4 classes)

Class	Line	Branch	Method
g.l.Document	+0.1% 🟢	±0.0%	±0.0%
g.l.EnumValueDefinition	-2.3% 🔴	±0.0%	±0.0%
g.l.FragmentDefinition $Builder	+0.6% 🟢	±0.0%	±0.0%
g.l.FragmentSpread	+0.8% 🟢	±0.0%	±0.0%

Full HTML report: build artifact jacoco-html-report

Updated: 2026-03-21 07:46:32 UTC

The assertNotNull guards added for JSpecify annotations introduce unreachable throw paths that aren't covered by tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

andimarek

We need to fix the formatting changes first ... to many wrong and not needed line breaks etc

andimarek · 2026-04-19T21:46:02Z

    @Internal
-    protected Document(List<Definition> definitions, SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
+    protected Document(List<Definition> definitions, @Nullable SourceLocation sourceLocation, List<Comment> comments,
+            IgnoredChars ignoredChars, Map<String, String> additionalData) {


there are multiple examples with this wrong formatting ... lets fix that before merging.

andimarek · 2026-04-19T21:46:54Z

    /**
-     * Returns a list of definitions of the specific type.  It uses {@link java.lang.Class#isAssignableFrom(Class)} for the test
+     * Returns a list of definitions of the specific type. It uses
+     * {@link java.lang.Class#isAssignableFrom(Class)} for the test


this is not needed change I think ... more examples below

Copilot AI and others added 4 commits January 24, 2026 20:32

Initial plan

c60fdf6

Add JSpecify annotations to 10 classes in graphql.language package

c02fa4e

Co-authored-by: dondonz <13839920+dondonz@users.noreply.github.com>

Update JSpecify annotation prompt with effectiveness improvements

3db2dde

Co-authored-by: dondonz <13839920+dondonz@users.noreply.github.com>

Initial plan

b0ef95a

Copilot AI assigned Copilot and dondonz Jan 25, 2026

Copilot started work on behalf of dondonz January 25, 2026 06:36 View session

Copilot AI and others added 3 commits January 25, 2026 06:45

Add JSpecify annotations to 10 language classes

7235cf1

Co-authored-by: dondonz <13839920+dondonz@users.noreply.github.com>

Fix FragmentDefinition selectionSet to be nullable for validation tests

53e8423

Co-authored-by: dondonz <13839920+dondonz@users.noreply.github.com>

Make builder methods lenient to allow incomplete test instances

256724a

Co-authored-by: dondonz <13839920+dondonz@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Add JSpecify annotations to 10 classes in graphql.language package~~ Add JSpecify annotations to 10 language package classes Jan 25, 2026

Copilot AI requested a review from dondonz January 25, 2026 07:00

Copilot finished work on behalf of dondonz January 25, 2026 07:00

Copilot AI mentioned this pull request Jan 28, 2026

Review all open JSpecify PRs #4231

Closed

dondonz changed the base branch from copilot/add-jspecify-annotations-to-classes to master February 8, 2026 07:16

dondonz added 4 commits February 15, 2026 16:25

Merge branch 'master' into copilot/add-jspecify-annotations-again

46c212c

# Conflicts: # .claude/commands/jspecify-annotate.md

A selection set is not-nullable

c0cab87

Add directive location

75a9170

Make name not nullable

baa9a0e

dondonz added 3 commits February 15, 2026 19:23

Add more suggestions

1055f0c

More improvements not nullable items according to spec

2cd70a7

Add assert for compile check

ea86f88

dondonz reviewed Feb 15, 2026

View reviewed changes

Comment thread src/main/java/graphql/language/Field.java

dondonz added 2 commits February 15, 2026 21:05

Add nullunmarked on builders

6237b2d

Fix more builders

fced94a

dondonz reviewed Feb 15, 2026

View reviewed changes

dondonz added 2 commits February 16, 2026 07:03

Commit the fixing plan

97abdd1

Fixing tests with new stricter non-nullable requirements

d88fa6b

More test fixes

72ea92a

dondonz marked this pull request as ready for review February 18, 2026 04:38

dondonz and others added 2 commits March 21, 2026 18:00

Remove already-annotated classes from JSpecify exemption list

346b85e

NodeVisitor, NodeVisitorStub, SourceLocation, and Type are already annotated with @NullMarked so they should not be in the exemption list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

dondonz added this to the 26.x is a breaking change release (other fixes as well) milestone Mar 21, 2026

dondonz and others added 2 commits March 21, 2026 18:35

Remove no longer needed file

dec1764

Update test baseline for EnumValueDefinition coverage

53a4a8a

The assertNotNull guards added for JSpecify annotations introduce unreachable throw paths that aren't covered by tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

andimarek requested changes Apr 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JSpecify annotations to 10 language package classes#4218

Add JSpecify annotations to 10 language package classes#4218
Copilot wants to merge 23 commits intomasterfrom
copilot/add-jspecify-annotations-again

Copilot AI commented Jan 25, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Feb 15, 2026

Uh oh!

github-actions bot commented Feb 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

dondonz Feb 15, 2026

Uh oh!

dondonz commented Feb 17, 2026

Uh oh!

github-actions bot commented Mar 21, 2026 •

edited

Loading

Uh oh!

andimarek left a comment

Uh oh!

andimarek Apr 19, 2026

Uh oh!

andimarek Apr 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Annotated Classes

Approach

Task

Classes to annotate

Instructions

Validation

Cleanup

Reference

Uh oh!

github-actions bot commented Feb 15, 2026

Test Results

Uh oh!

github-actions bot commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

dondonz Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

dondonz commented Feb 17, 2026

Uh oh!

github-actions bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Report

Test Results

Code Coverage (Java 25)

Uh oh!

andimarek left a comment

Choose a reason for hiding this comment

Uh oh!

andimarek Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

andimarek Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Jan 25, 2026 •

edited

Loading

github-actions bot commented Feb 15, 2026 •

edited

Loading

github-actions bot commented Mar 21, 2026 •

edited

Loading