Add JSpecify annotations to 10 language package classes#4219
Add JSpecify annotations to 10 language package classes#4219
Conversation
Co-authored-by: dondonz <[email protected]>
Co-authored-by: dondonz <[email protected]>
- Added @NullMarked to ImplementingTypeDefinition interface - Added @NullMarked to InlineFragment with @nullable typeCondition - Added @NullMarked to InputObjectTypeDefinition with @NullUnmarked Builder - Added @NullMarked to InputObjectTypeExtensionDefinition with @NullUnmarked Builder - Added @NullMarked to InputValueDefinition with @nullable defaultValue - Added @NullMarked to InterfaceTypeDefinition with @NullUnmarked Builder - Added @NullMarked to InterfaceTypeExtensionDefinition with @NullUnmarked Builder - Added @NullMarked to ListType with @NullUnmarked Builder - Added @NullMarked to NodeDirectivesBuilder interface - Added @NullMarked to NodeParentTree with @nullable parent - Fixed deepCopy methods with assertNotNull for non-null fields - Fixed InlineFragment constructor to provide default SelectionSet - Fixed Anonymizer to handle nullable typeCondition - Removed completed classes from JSpecify exemption list Co-authored-by: dondonz <[email protected]>
Test Results 335 files ±0 335 suites ±0 5m 4s ⏱️ -1s Results for commit 0fb0195. ± Comparison against base commit 89b4446. This pull request removes 449 and adds 430 tests. Note that renamed tests count towards both.This pull request skips 1 test.♻️ This comment has been updated with latest results. |
| public InlineFragment(TypeName typeCondition) { | ||
| this(typeCondition, emptyList(), null, null, emptyList(), IgnoredChars.EMPTY, emptyMap()); | ||
| public InlineFragment(@Nullable TypeName typeCondition) { | ||
| this(typeCondition, emptyList(), SelectionSet.newSelectionSet().build(), null, emptyList(), IgnoredChars.EMPTY, emptyMap()); |
There was a problem hiding this comment.
This is technically a breaking change, although it's to make this more correct
An inline fragment should not have an empty selection set, as per the specification.
|
This PR will start enforcing selection sets in inline fragments as not nullable, so this is technically a breaking change, but one that's also a bugfix |
Resolve JSpecifyAnnotationsCheck conflict by taking master's exemption list updates. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Test ReportTest Results
Code Coverage (Java 25)
Changed Class Coverage (1 class)
Anonymizer.4 — method details
|
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) <[email protected]>
NamedNode.getName() returns @nullable String, so use Objects.toString to handle null names (e.g. anonymous OperationDefinition). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
| return ImmutableKit.filterAndMap(copy, | ||
| node1 -> node1 instanceof NamedNode, | ||
| node1 -> ((NamedNode) node1).getName()); | ||
| node1 -> Objects.toString(((NamedNode) node1).getName(), "")); |
There was a problem hiding this comment.
This is an awkward consequence of OperationDefinition technically allowing a nullable name.
These classes are annotated with @NullMarked on this branch but were still in the exemption list after the merge from master. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
visitInlineFragment coverage dropped due to NamedNode.getName() returning @nullable — the null-check branch is not exercised in tests. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Annotates 10 classes in
graphql.languagewith JSpecify nullability markers per the established pattern.Classes Annotated
Interfaces:
ImplementingTypeDefinition-@NullMarkedNodeDirectivesBuilder-@NullMarkedClasses with Builders:
InlineFragment-@NullMarkedclass,@NullUnmarkedBuilderInputObjectTypeDefinition+ Extension -@NullMarkedclasses,@NullUnmarkedBuildersInputValueDefinition-@NullMarkedclass,@NullUnmarkedBuilderInterfaceTypeDefinition+ Extension -@NullMarkedclasses,@NullUnmarkedBuildersListType-@NullMarkedclass,@NullUnmarkedBuilderOther:
NodeParentTree-@NullMarkedNullability Decisions
Fields marked
@Nullablebased on GraphQL spec and usage analysis:InlineFragment.typeCondition- Inline fragments can omit type conditions (spec allows bare... { field })InputValueDefinition.defaultValue- Default values are optionalNodeParentTree.parent- Root nodes have no parentDescriptionparameters - Descriptions are optional in GraphQL SDLSourceLocationparameters - Inherited fromAbstractNodepatternTechnical Fixes
deepCopy methods: Added
assertNotNullcalls whereAbstractNode.deepCopy()returns nullable types but target constructors expect non-null:InlineFragment constructor: Provides default
SelectionSetinstead of null since selection sets are required.Anonymizer: Added null check for
typeConditionbefore dereferencing invisitInlineFragment.Removes all 10 classes from JSpecify exemption list.
Original prompt
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.