diff --git a/.claude/commands/jspecify-annotate.md b/.claude/commands/jspecify-annotate.md deleted file mode 100644 index fde80eb6d..000000000 --- a/.claude/commands/jspecify-annotate.md +++ /dev/null @@ -1,68 +0,0 @@ -The task is to annotate public API classes (marked with `@PublicAPI`) with JSpecify nullability annotations. - -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. - -Analyze this Java class and add JSpecify annotations based on: -1. Set the class to be `@NullMarked` -2. Remove all the redundant `@NonNull` annotations that IntelliJ added -3. Check Javadoc @param tags mentioning "null", "nullable", "may be null" -4. Check Javadoc @return tags mentioning "null", "optional", "if available" -5. Method implementations that return null or check for null -6. GraphQL specification details (see details below) - -## API Compatibility and Breaking Changes - -When adding JSpecify annotations, **DO NOT break existing public APIs**. -- **Do not remove interfaces** from public classes (e.g., if a class implements `NamedNode`, it must continue to do so). -- **Be extremely careful when changing methods to return `@Nullable`**. If an interface contract (or widespread ecosystem usage) expects a non-null return value, changing it to `@Nullable` is a breaking change that will cause compilation errors or `NullPointerException`s for callers. For example, if a method returned `null` implicitly but its interface requires non-null, you must honor the non-null contract (e.g., returning an empty string or default value instead of `null`). -- **Do not change the binary signature** of methods or constructors in a way that breaks backwards compatibility. - -## GraphQL Specification Compliance -This is a GraphQL implementation. When determining nullability, consult the GraphQL specification (https://spec.graphql.org/draft/) for the relevant concept. Key principles: - -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. - -If a class implements or represents a GraphQL specification concept, prioritize the spec's nullability requirements over what IntelliJ inferred. - -## How to validate -Finally, please check all this works by running the NullAway compile check. - -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. - -## Formatting Guidelines - -- **Zero Formatting Changes**: Do NOT reformat the code. -- **Minimise Whitespace/Newline Changes**: Do not add or remove blank lines unless absolutely necessary. Keep the diff as clean as possible to ease the review process. -- 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. - -## Cleaning up -Finally, can you remove this class from the JSpecifyAnnotationsCheck as an exemption - -You do not need to run the JSpecifyAnnotationsCheck. Removing the completed class is enough. - -Remember to delete all unused imports when you're done from the class you've just annotated. - -## Generics Annotations - -When annotating generic types and methods, follow these JSpecify rules: - -### Type Parameter Bounds - -The bound on a type parameter determines whether nullable type arguments are allowed: - -| Declaration | Allows `@Nullable` type argument? | -|-------------|----------------------------------| -| `` | ❌ No — `Box<@Nullable String>` is illegal | -| `` | ✅ Yes — `Box<@Nullable String>` is legal | - -**When to use ``:** -- When callers genuinely need to parameterize with nullable types -- Example: `DataFetcherResult` — data fetchers may return nullable types - -**When to keep ``:** -- When the type parameter represents a concrete non-null object -- Even if some methods return `@Nullable T` (meaning "can be null even if T is non-null") -- Example: `Edge` with `@Nullable T getNode()` — node may be null, but T represents the object type - diff --git a/build.gradle b/build.gradle index 0d183d4a8..89e65c14c 100644 --- a/build.gradle +++ b/build.gradle @@ -90,7 +90,7 @@ def getDevelopmentVersion() { gitRevParse.waitForProcessOutput(gitRevParseOutput, gitRevParseError) def branchName = gitRevParseOutput.toString().trim().replaceAll('[/\\\\]', '-') - return makeDevelopmentVersion(["0.0.0", branchName, "SNAPSHOT"]) + return makeDevelopmentVersion(["0.0.0", sanitizedBranchName, "SNAPSHOT"]) } def reactiveStreamsVersion = '1.0.3' diff --git a/src/main/java/graphql/language/Comment.java b/src/main/java/graphql/language/Comment.java index a7a546fac..65096c678 100644 --- a/src/main/java/graphql/language/Comment.java +++ b/src/main/java/graphql/language/Comment.java @@ -1,6 +1,8 @@ package graphql.language; import graphql.PublicApi; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.io.Serializable; @@ -8,11 +10,12 @@ * A single-line comment. These are comments that start with a {@code #} in source documents. */ @PublicApi +@NullMarked public class Comment implements Serializable { public final String content; - public final SourceLocation sourceLocation; + public final @Nullable SourceLocation sourceLocation; - public Comment(String content, SourceLocation sourceLocation) { + public Comment(String content, @Nullable SourceLocation sourceLocation) { this.content = content; this.sourceLocation = sourceLocation; } @@ -21,7 +24,7 @@ public String getContent() { return content; } - public SourceLocation getSourceLocation() { + public @Nullable SourceLocation getSourceLocation() { return sourceLocation; } } diff --git a/src/main/java/graphql/language/Definition.java b/src/main/java/graphql/language/Definition.java index f0e7d74dc..5402bfe3c 100644 --- a/src/main/java/graphql/language/Definition.java +++ b/src/main/java/graphql/language/Definition.java @@ -2,8 +2,10 @@ import graphql.PublicApi; +import org.jspecify.annotations.NullMarked; @PublicApi +@NullMarked public interface Definition extends Node { } diff --git a/src/main/java/graphql/language/IgnoredChar.java b/src/main/java/graphql/language/IgnoredChar.java index eaa1689d0..d316a4d6b 100644 --- a/src/main/java/graphql/language/IgnoredChar.java +++ b/src/main/java/graphql/language/IgnoredChar.java @@ -1,6 +1,8 @@ package graphql.language; import graphql.PublicApi; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.io.Serializable; import java.util.Objects; @@ -12,6 +14,7 @@ * This costs more memory but for certain use cases (like editors) this maybe be useful */ @PublicApi +@NullMarked public class IgnoredChar implements Serializable { public enum IgnoredCharKind { @@ -51,7 +54,7 @@ public String toString() { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (this == o) { return true; } diff --git a/src/main/java/graphql/language/IgnoredChars.java b/src/main/java/graphql/language/IgnoredChars.java index ce3a1ad59..241b4cc74 100644 --- a/src/main/java/graphql/language/IgnoredChars.java +++ b/src/main/java/graphql/language/IgnoredChars.java @@ -3,6 +3,7 @@ import com.google.common.collect.ImmutableList; import graphql.PublicApi; import graphql.collect.ImmutableKit; +import org.jspecify.annotations.NullMarked; import java.io.Serializable; import java.util.List; @@ -14,6 +15,7 @@ * This costs more memory but for certain use cases (like editors) this maybe be useful */ @PublicApi +@NullMarked public class IgnoredChars implements Serializable { private final ImmutableList left; diff --git a/src/main/java/graphql/language/Node.java b/src/main/java/graphql/language/Node.java index 962934d76..917594b87 100644 --- a/src/main/java/graphql/language/Node.java +++ b/src/main/java/graphql/language/Node.java @@ -4,6 +4,7 @@ import graphql.PublicApi; import graphql.util.TraversalControl; import graphql.util.TraverserContext; +import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; import java.io.Serializable; @@ -21,6 +22,7 @@ * Every Node is immutable */ @PublicApi +@NullMarked public interface Node extends Serializable { /** diff --git a/src/main/java/graphql/language/NodeChildrenContainer.java b/src/main/java/graphql/language/NodeChildrenContainer.java index d2e1b06fe..a986ebca7 100644 --- a/src/main/java/graphql/language/NodeChildrenContainer.java +++ b/src/main/java/graphql/language/NodeChildrenContainer.java @@ -1,6 +1,9 @@ package graphql.language; import graphql.PublicApi; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.NullUnmarked; +import org.jspecify.annotations.Nullable; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -15,6 +18,7 @@ * Container of children of a {@link Node}. */ @PublicApi +@NullMarked public class NodeChildrenContainer { private final Map> children = new LinkedHashMap<>(); @@ -27,7 +31,7 @@ public List getChildren(String key) { return (List) children.getOrDefault(key, emptyList()); } - public T getChildOrNull(String key) { + public @Nullable T getChildOrNull(String key) { List result = children.getOrDefault(key, emptyList()); if (result.size() > 1) { throw new IllegalStateException("children " + key + " is not a single value"); @@ -61,6 +65,7 @@ public boolean isEmpty() { return this.children.isEmpty(); } + @NullUnmarked public static class Builder { private final Map> children = new LinkedHashMap<>(); diff --git a/src/main/java/graphql/language/NodeVisitor.java b/src/main/java/graphql/language/NodeVisitor.java index 2ed79570b..ca40709f5 100644 --- a/src/main/java/graphql/language/NodeVisitor.java +++ b/src/main/java/graphql/language/NodeVisitor.java @@ -3,11 +3,13 @@ import graphql.PublicApi; import graphql.util.TraversalControl; import graphql.util.TraverserContext; +import org.jspecify.annotations.NullMarked; /** * Used by {@link NodeTraverser} to visit {@link Node}. */ @PublicApi +@NullMarked public interface NodeVisitor { TraversalControl visitArgument(Argument node, TraverserContext data); diff --git a/src/main/java/graphql/language/NodeVisitorStub.java b/src/main/java/graphql/language/NodeVisitorStub.java index f0fcd6b3f..b5d00073c 100644 --- a/src/main/java/graphql/language/NodeVisitorStub.java +++ b/src/main/java/graphql/language/NodeVisitorStub.java @@ -3,11 +3,13 @@ import graphql.PublicApi; import graphql.util.TraversalControl; import graphql.util.TraverserContext; +import org.jspecify.annotations.NullMarked; /** * Convenient implementation of {@link NodeVisitor} for easy subclassing methods handling different types of Nodes in one method. */ @PublicApi +@NullMarked public class NodeVisitorStub implements NodeVisitor { @Override public TraversalControl visitArgument(Argument node, TraverserContext context) { diff --git a/src/main/java/graphql/language/SourceLocation.java b/src/main/java/graphql/language/SourceLocation.java index a4ae90a03..b97cc103a 100644 --- a/src/main/java/graphql/language/SourceLocation.java +++ b/src/main/java/graphql/language/SourceLocation.java @@ -7,24 +7,27 @@ import graphql.schema.GraphQLSchemaElement; import graphql.schema.GraphQLTypeUtil; import graphql.schema.idl.SchemaGenerator; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.io.Serializable; import java.util.Objects; @PublicApi +@NullMarked public class SourceLocation implements Serializable { public static final SourceLocation EMPTY = new SourceLocation(-1, -1); private final int line; private final int column; - private final String sourceName; + private final @Nullable String sourceName; public SourceLocation(int line, int column) { this(line, column, null); } - public SourceLocation(int line, int column, String sourceName) { + public SourceLocation(int line, int column, @Nullable String sourceName) { this.line = line; this.column = column; this.sourceName = sourceName; @@ -38,12 +41,12 @@ public int getColumn() { return column; } - public String getSourceName() { + public @Nullable String getSourceName() { return sourceName; } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (this == o) { return true; } @@ -91,7 +94,7 @@ public String toString() { * * @return the source location if available or null if it's not. */ - public static SourceLocation getLocation(GraphQLSchemaElement schemaElement) { + public static @Nullable SourceLocation getLocation(GraphQLSchemaElement schemaElement) { if (schemaElement instanceof GraphQLModifiedType) { schemaElement = GraphQLTypeUtil.unwrapAllAs((GraphQLModifiedType) schemaElement); } diff --git a/src/main/java/graphql/language/Type.java b/src/main/java/graphql/language/Type.java index a3141e56d..85d06564d 100644 --- a/src/main/java/graphql/language/Type.java +++ b/src/main/java/graphql/language/Type.java @@ -2,8 +2,10 @@ import graphql.PublicApi; +import org.jspecify.annotations.NullMarked; @PublicApi +@NullMarked public interface Type extends Node { } diff --git a/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy b/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy index 7800edbe2..c1bf74ddb 100644 --- a/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy +++ b/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy @@ -99,8 +99,6 @@ class JSpecifyAnnotationsCheck extends Specification { "graphql.language.AstSignature", "graphql.language.AstSorter", "graphql.language.AstTransformer", - "graphql.language.Comment", - "graphql.language.Definition", "graphql.language.DescribedNode", "graphql.language.Description", "graphql.language.Directive", @@ -115,8 +113,6 @@ class JSpecifyAnnotationsCheck extends Specification { "graphql.language.FieldDefinition", "graphql.language.FragmentDefinition", "graphql.language.FragmentSpread", - "graphql.language.IgnoredChar", - "graphql.language.IgnoredChars", "graphql.language.ImplementingTypeDefinition", "graphql.language.InlineFragment", "graphql.language.InputObjectTypeDefinition", @@ -125,12 +121,8 @@ class JSpecifyAnnotationsCheck extends Specification { "graphql.language.InterfaceTypeDefinition", "graphql.language.InterfaceTypeExtensionDefinition", "graphql.language.ListType", - "graphql.language.Node", - "graphql.language.NodeChildrenContainer", "graphql.language.NodeDirectivesBuilder", "graphql.language.NodeParentTree", - "graphql.language.NodeVisitor", - "graphql.language.NodeVisitorStub", "graphql.language.ScalarTypeDefinition", "graphql.language.ScalarTypeExtensionDefinition", "graphql.language.SchemaDefinition", @@ -138,8 +130,6 @@ class JSpecifyAnnotationsCheck extends Specification { "graphql.language.Selection", "graphql.language.SelectionSet", "graphql.language.SelectionSetContainer", - "graphql.language.SourceLocation", - "graphql.language.Type", "graphql.language.TypeKind", "graphql.language.TypeName", "graphql.language.UnionTypeDefinition",