Skip to content

Add JSpecify annotations to language package nodes - redo (12 more) #4257

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 10 commits into from
Mar 21, 2026
Merged

Add JSpecify annotations to language package nodes - redo (12 more) #4257

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b261a43
Add corrections to prompt
dondonz Feb 21, 2026
4df4205
Add first lot of changes
dondonz Feb 22, 2026
8fb836e
Add operation definition
dondonz Feb 22, 2026
298fea2
Add pretty ast printer
dondonz Feb 22, 2026
82712c7
Override name as non null
dondonz Feb 22, 2026
7a9e810
Add operation validator
dondonz Feb 22, 2026
dd49296
Adjust AST Printer
dondonz Feb 22, 2026
9e31132
Account for a nullable op name
dondonz Feb 22, 2026
1063251
Fix test for non-null selection set
dondonz Feb 22, 2026
f4e0f1b
Make OperationDefinition.operation non-null, default to QUERY
dondonz Mar 8, 2026
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
Prev Previous commit
Next Next commit
Add operation definition
  • Loading branch information
@dondonz
dondonz committed Feb 22, 2026
commit 8fb836ef7b7b3775fb0b8fa1b5eeec54f7de56d3
37 changes: 17 additions & 20 deletions src/main/java/graphql/language/OperationDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
import graphql.language.NodeUtil.DirectivesHolder;
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.ArrayList;
import java.util.LinkedHashMap;
Expand All @@ -22,15 +25,16 @@
import static graphql.language.NodeChildrenContainer.newNodeChildrenContainer;

@PublicApi
@NullMarked
public class OperationDefinition extends AbstractNode<OperationDefinition> implements Definition<OperationDefinition>, SelectionSetContainer<OperationDefinition>, DirectivesContainer<OperationDefinition>, NamedNode<OperationDefinition> {

public enum Operation {
QUERY, MUTATION, SUBSCRIPTION
}

private final String name;
private final @Nullable String name;

private final Operation operation;
private final @Nullable Operation operation;
private final ImmutableList<VariableDefinition> variableDefinitions;
private final DirectivesHolder directives;
private final SelectionSet selectionSet;
Expand All @@ -40,12 +44,12 @@ public enum Operation {
public static final String CHILD_SELECTION_SET = "selectionSet";

@Internal
protected OperationDefinition(String name,
Operation operation,
protected OperationDefinition(@Nullable String name,
@Nullable Operation operation,
List<VariableDefinition> variableDefinitions,
List<Directive> directives,
SelectionSet selectionSet,
SourceLocation sourceLocation,
@Nullable SourceLocation sourceLocation,
List<Comment> comments,
IgnoredChars ignoredChars,
Map<String, String> additionalData) {
Expand All @@ -57,15 +61,6 @@ protected OperationDefinition(String name,
this.selectionSet = selectionSet;
}

public OperationDefinition(String name,

@dondonz dondonz Feb 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These constructors date back to 2018, no longer needed with the builders we have today.

I am removing these lines because these parameters should be not-nullable.

I would usually first deprecate the methods, then remove, but if I keep them here with a deprecated annotation, the nullability annotations will not be correct.

@dondonz dondonz Feb 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is technically a breaking change

Operation operation) {
this(name, operation, emptyList(), emptyList(), null, null, emptyList(), IgnoredChars.EMPTY, emptyMap());
}

public OperationDefinition(String name) {
this(name, null, emptyList(), emptyList(), null, null, emptyList(), IgnoredChars.EMPTY, emptyMap());
}

@bbakerman bbakerman Mar 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fair enough - cruft must be removed at some point

@Override
public List<Node> getChildren() {
List<Node> result = new ArrayList<>();
Expand Down Expand Up @@ -93,11 +88,12 @@ public OperationDefinition withNewChildren(NodeChildrenContainer newChildren) {
);
}

public String getName() {
@Override
public @Nullable String getName() {
return name;
}

public Operation getOperation() {
public @Nullable Operation getOperation() {

@bbakerman bbakerman Mar 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ummmmmm can this be null??? if its not specified surely is defaults to QUERY??

@dondonz dondonz Mar 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep I agree, should have defaulted to query instead

@dondonz dondonz Mar 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It does in the parser as well

return operation;
}

Expand Down Expand Up @@ -130,7 +126,7 @@ public SelectionSet getSelectionSet() {
}

@Override
public boolean isEqualTo(Node o) {
public boolean isEqualTo(@Nullable Node o) {
if (this == o) {
return true;
}
Expand All @@ -148,9 +144,9 @@ public boolean isEqualTo(Node o) {
public OperationDefinition deepCopy() {
return new OperationDefinition(name,
operation,
deepCopy(variableDefinitions),
deepCopy(directives.getDirectives()),
deepCopy(selectionSet),
assertNotNull(deepCopy(variableDefinitions), "variableDefinitions deepCopy should not return null"),
assertNotNull(deepCopy(directives.getDirectives()), "directives deepCopy should not return null"),
assertNotNull(deepCopy(selectionSet), "selectionSet deepCopy should not return null"),
getSourceLocation(),
getComments(),
getIgnoredChars(),
Expand Down Expand Up @@ -183,6 +179,7 @@ public OperationDefinition transform(Consumer<Builder> builderConsumer) {
return builder.build();
}

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