Skip to content

Specify nullness for nodes in the graphql.language package #3899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dondonz merged 9 commits into from
Aug 10, 2025
Merged

Specify nullness for nodes in the graphql.language package #3899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c5e0ae6
Specify nullness for nodes
mk868 Apr 5, 2025
8446dff
Merge branch 'master' into jspecify-node
dondonz Aug 2, 2025
237ee1f
Set builders to null unmarked
dondonz Aug 3, 2025
d559d7b
Require array and object value to not be null, use empty list instead
dondonz Aug 3, 2025
5dd0424
Fix up nullability annotation
dondonz Aug 9, 2025
0393ee9
Add assertion
dondonz Aug 10, 2025
b124c00
Change to assert
dondonz Aug 10, 2025
28a3301
Merge branch 'master' into jspecify-node
dondonz Aug 10, 2025
d7f087e
Update exemptions
dondonz Aug 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/main/java/graphql/language/AbstractNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,28 @@
import graphql.Assert;
import graphql.PublicApi;
import graphql.collect.ImmutableKit;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

import java.util.List;
import java.util.Map;

import static graphql.collect.ImmutableKit.map;

@PublicApi
@NullMarked
public abstract class AbstractNode<T extends Node> implements Node<T> {

private final SourceLocation sourceLocation;
private final @Nullable SourceLocation sourceLocation;
private final ImmutableList<Comment> comments;
private final IgnoredChars ignoredChars;
private final ImmutableMap<String, String> additionalData;

public AbstractNode(SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars) {
public AbstractNode(@Nullable SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars) {
this(sourceLocation, comments, ignoredChars, ImmutableKit.emptyMap());
}

public AbstractNode(SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
public AbstractNode(@Nullable SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
Assert.assertNotNull(comments, () -> "comments can't be null");
Assert.assertNotNull(ignoredChars, () -> "ignoredChars can't be null");
Assert.assertNotNull(additionalData, () -> "additionalData can't be null");
Expand All @@ -36,7 +39,7 @@ public AbstractNode(SourceLocation sourceLocation, List<Comment> comments, Ignor
}

@Override
public SourceLocation getSourceLocation() {
public @Nullable SourceLocation getSourceLocation() {
return sourceLocation;
}

Expand All @@ -56,15 +59,15 @@ public Map<String, String> getAdditionalData() {
}

@SuppressWarnings("unchecked")
protected <V extends Node> V deepCopy(V nullableObj) {
protected <V extends Node> @Nullable V deepCopy(@Nullable V nullableObj) {
if (nullableObj == null) {
return null;
}
return (V) nullableObj.deepCopy();
}

@SuppressWarnings("unchecked")
protected <V extends Node> List<V> deepCopy(List<? extends Node> list) {
protected <V extends Node> @Nullable List<V> deepCopy(@Nullable List<? extends Node> list) {
if (list == null) {
return null;
}
Expand Down
12 changes: 9 additions & 3 deletions src/main/java/graphql/language/ArrayValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import graphql.collect.ImmutableKit;
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable;

import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -19,13 +22,14 @@
import static graphql.language.NodeChildrenContainer.newNodeChildrenContainer;

@PublicApi
@NullMarked
public class ArrayValue extends AbstractNode<ArrayValue> implements Value<ArrayValue> {

public static final String CHILD_VALUES = "values";
private final ImmutableList<Value> values;

@Internal
protected ArrayValue(List<Value> values, SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
protected ArrayValue(List<Value> values, @Nullable SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
super(sourceLocation, comments, ignoredChars, additionalData);
this.values = ImmutableList.copyOf(values);
}
Expand Down Expand Up @@ -67,7 +71,7 @@ public ArrayValue withNewChildren(NodeChildrenContainer newChildren) {
}

@Override
public boolean isEqualTo(Node o) {
public boolean isEqualTo(@Nullable Node o) {
if (this == o) {
return true;
}
Expand All @@ -87,7 +91,8 @@ public String toString() {

@Override
public ArrayValue deepCopy() {
return new ArrayValue(deepCopy(values), getSourceLocation(), getComments(), getIgnoredChars(), getAdditionalData());
List<Value> copiedValues = assertNotNull(deepCopy(values));
Copy link
Member

@dondonz dondonz Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an assert here to keep NullAway happy

In practice it is not possible to have null values. Have to put this here otherwise you'll get a NullAway error

return new ArrayValue(copiedValues, getSourceLocation(), getComments(), getIgnoredChars(), getAdditionalData());
}

@Override
Expand All @@ -101,6 +106,7 @@ public ArrayValue transform(Consumer<Builder> builderConsumer) {
return builder.build();
}

@NullUnmarked
Copy link
Member

@dondonz dondonz Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After giving a few classes the JSpecify treatment: we now generally annotate Builders with @NullUnmarked

public static final class Builder implements NodeBuilder {
private SourceLocation sourceLocation;
private ImmutableList<Value> values = emptyList();
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/graphql/language/BooleanValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import graphql.PublicApi;
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable;

import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -19,12 +22,13 @@
import static graphql.language.NodeUtil.assertNewChildrenAreEmpty;

@PublicApi
@NullMarked
public class BooleanValue extends AbstractNode<BooleanValue> implements ScalarValue<BooleanValue> {

private final boolean value;

@Internal
protected BooleanValue(boolean value, SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
protected BooleanValue(boolean value, @Nullable SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
super(sourceLocation, comments, ignoredChars, additionalData);
this.value = value;
}
Expand Down Expand Up @@ -59,7 +63,7 @@ public BooleanValue withNewChildren(NodeChildrenContainer newChildren) {
}

@Override
public boolean isEqualTo(Node o) {
public boolean isEqualTo(@Nullable Node o) {
if (this == o) {
return true;
}
Expand Down Expand Up @@ -109,6 +113,7 @@ public BooleanValue transform(Consumer<Builder> builderConsumer) {
return builder.build();
}

@NullUnmarked
public static final class Builder implements NodeBuilder {
private SourceLocation sourceLocation;
private boolean value;
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/graphql/language/EnumValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import graphql.PublicApi;
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable;

import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -20,12 +23,13 @@
import static graphql.language.NodeUtil.assertNewChildrenAreEmpty;

@PublicApi
@NullMarked
public class EnumValue extends AbstractNode<EnumValue> implements Value<EnumValue>, NamedNode<EnumValue> {

private final String name;

@Internal
protected EnumValue(String name, SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
protected EnumValue(String name, @Nullable SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
super(sourceLocation, comments, ignoredChars, additionalData);
this.name = name;
}
Expand Down Expand Up @@ -67,7 +71,7 @@ public EnumValue withNewChildren(NodeChildrenContainer newChildren) {
}

@Override
public boolean isEqualTo(Node o) {
public boolean isEqualTo(@Nullable Node o) {
if (this == o) {
return true;
}
Expand Down Expand Up @@ -111,6 +115,7 @@ public EnumValue transform(Consumer<Builder> builderConsumer) {
return builder.build();
}

@NullUnmarked
public static final class Builder implements NodeBuilder {
private SourceLocation sourceLocation;
private String name;
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/graphql/language/FloatValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import graphql.PublicApi;
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable;

import java.math.BigDecimal;
import java.util.LinkedHashMap;
Expand All @@ -21,12 +24,13 @@
import static graphql.language.NodeUtil.assertNewChildrenAreEmpty;

@PublicApi
@NullMarked
public class FloatValue extends AbstractNode<FloatValue> implements ScalarValue<FloatValue> {

private final BigDecimal value;

@Internal
protected FloatValue(BigDecimal value, SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
protected FloatValue(BigDecimal value, @Nullable SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
super(sourceLocation, comments, ignoredChars, additionalData);
this.value = value;
}
Expand Down Expand Up @@ -68,7 +72,7 @@ public String toString() {
}

@Override
public boolean isEqualTo(Node o) {
public boolean isEqualTo(@Nullable Node o) {
if (this == o) {
return true;
}
Expand Down Expand Up @@ -110,6 +114,7 @@ public static Builder newFloatValue(BigDecimal value) {
return new Builder().value(value);
}

@NullUnmarked
public static final class Builder implements NodeBuilder {
private SourceLocation sourceLocation;
private BigDecimal value;
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/graphql/language/IntValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import graphql.PublicApi;
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable;

import java.math.BigInteger;
import java.util.LinkedHashMap;
Expand All @@ -21,12 +24,13 @@
import static graphql.language.NodeUtil.assertNewChildrenAreEmpty;

@PublicApi
@NullMarked
public class IntValue extends AbstractNode<IntValue> implements ScalarValue<IntValue> {

private final BigInteger value;

@Internal
protected IntValue(BigInteger value, SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
protected IntValue(BigInteger value, @Nullable SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
super(sourceLocation, comments, ignoredChars, additionalData);
this.value = value;
}
Expand Down Expand Up @@ -61,7 +65,7 @@ public IntValue withNewChildren(NodeChildrenContainer newChildren) {
}

@Override
public boolean isEqualTo(Node o) {
public boolean isEqualTo(@Nullable Node o) {
if (this == o) {
return true;
}
Expand Down Expand Up @@ -109,6 +113,7 @@ public IntValue transform(Consumer<Builder> builderConsumer) {
return builder.build();
}

@NullUnmarked
public static final class Builder implements NodeBuilder {
private SourceLocation sourceLocation;
private BigInteger value;
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/graphql/language/Node.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import graphql.PublicApi;
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;
import org.jspecify.annotations.Nullable;

import java.io.Serializable;
import java.util.List;
Expand Down Expand Up @@ -47,6 +48,7 @@ public interface Node<T extends Node> extends Serializable {
/**
* @return the source location where this node occurs
*/
@Nullable
SourceLocation getSourceLocation();

/**
Expand Down Expand Up @@ -82,7 +84,7 @@ public interface Node<T extends Node> extends Serializable {
*
* @return isEqualTo
*/
boolean isEqualTo(Node node);
boolean isEqualTo(@Nullable Node node);

/**
* @return a deep copy of this node
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/graphql/language/NodeBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package graphql.language;

import graphql.PublicApi;
import org.jspecify.annotations.NullUnmarked;

import java.util.List;
import java.util.Map;

@PublicApi
@NullUnmarked
public interface NodeBuilder {

NodeBuilder sourceLocation(SourceLocation sourceLocation);
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/graphql/language/NullValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import graphql.collect.ImmutableKit;
import graphql.util.TraversalControl;
import graphql.util.TraverserContext;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable;

import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -19,10 +22,11 @@
import static graphql.language.NodeUtil.assertNewChildrenAreEmpty;

@PublicApi
@NullMarked
public class NullValue extends AbstractNode<NullValue> implements Value<NullValue> {

@Internal
protected NullValue(SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
protected NullValue(@Nullable SourceLocation sourceLocation, List<Comment> comments, IgnoredChars ignoredChars, Map<String, String> additionalData) {
super(sourceLocation, comments, ignoredChars, additionalData);
}

Expand All @@ -47,7 +51,7 @@ public NullValue withNewChildren(NodeChildrenContainer newChildren) {
}

@Override
public boolean isEqualTo(Node o) {
public boolean isEqualTo(@Nullable Node o) {
if (this == o) {
return true;
}
Expand Down Expand Up @@ -86,6 +90,7 @@ public NullValue transform(Consumer<Builder> builderConsumer) {
return builder.build();
}

@NullUnmarked
public static final class Builder implements NodeBuilder {
private SourceLocation sourceLocation;
private ImmutableList<Comment> comments = emptyList();
Expand Down
Loading