Fix ShallowTypeRefCollector: resolve type refs in applied directive arguments#4288
Merged
andimarek merged 5 commits intographql-java:masterfrom Mar 9, 2026
Merged
Fix ShallowTypeRefCollector: resolve type refs in applied directive arguments#4288andimarek merged 5 commits intographql-java:masterfrom
andimarek merged 5 commits intographql-java:masterfrom
Conversation
…ontexts ShallowTypeRefCollector called scanArgumentType(arg) for GraphQLArgument objects in several places but never scanAppliedDirectives(arg.getAppliedDirectives()), and never descended into enum values to scan their applied directives. This left GraphQLTypeReference objects in applied directive argument types unresolved, causing the AppliedDirectiveArgumentsAreValid validator to spuriously fail with "Invalid argument 'x' for applied directive of name 'y'". Three locations fixed: 1. handleObjectType / handleInterfaceType — field argument applied directives were not scanned. Added scanAppliedDirectives(arg.getAppliedDirectives()) inside the field-argument loop of both methods. 2. handleTypeDef — already scanned applied directives on the enum type itself (via the GraphQLDirectiveContainer branch), but never descended into enum values. Added handleEnumType() which scans applied directives on each GraphQLEnumValueDefinition. 3. handleDirective — directive definition argument applied directives were not scanned. Added scanAppliedDirectives(argument.getAppliedDirectives()) inside the argument loop. SchemaUtil.replaceTypeReferences (used by the standard Builder) does a full deep traversal via getChildrenWithTypeReferences() and handles all of these correctly; this change brings ShallowTypeRefCollector in line with that behavior. Adds four regression tests in FastBuilderTest covering: - Applied directive on object type field argument - Applied directive on interface field argument - Applied directive on enum value - Applied directive on directive definition argument Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
(more internal testing has failed -- stand by...) |
Member
|
@rstata is that ready or are we waiting for more? |
Contributor
Test ReportTest Results
Code Coverage (Java 25)
Changed Class Coverage (1 class)
|
Author
|
oops -- i forgot about this! turns out this version did fix all the problems, viaduct builds and passes all its tests with this fix in place. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ShallowTypeRefCollector(introduced withFastBuilderin #4196) performsshallow scans of types and directives to collect
GraphQLTypeReferenceobjectsfor later replacement. In three places it called
scanArgumentType(arg)on aGraphQLArgumentbut never calledscanAppliedDirectives(arg.getAppliedDirectives()).It also had no handling to descend into
GraphQLEnumValueDefinitionobjects toscan their applied directives.
The result: when
FastBuilderis used with programmatically-constructed typescarrying
GraphQLTypeReferenceobjects in applied directive argument types onfield arguments, enum values, or directive definition arguments, those references
are never resolved. The
AppliedDirectiveArgumentsAreValidvalidator then sees aGraphQLTypeReferencewhere it expects a real input type and throws:(This does not manifest when types are built via
FastSchemaGenerator+ SDL,because
SchemaGeneratorHelper.buildInputTyperesolves types at constructiontime. It manifests when
FastBuilderis used directly with programmatically-builttypes.)
Changes
Three locations in
ShallowTypeRefCollectorfixed:handleObjectType/handleInterfaceType— addedscanAppliedDirectives(arg.getAppliedDirectives())inside the field-argumentloop of both methods, alongside the existing
scanArgumentType(arg)call.handleTypeDef— addedhandleEnumType()which iterates overGraphQLEnumValueDefinitionvalues and scans their applied directives. (Applieddirectives on the enum type itself were already covered by the existing
GraphQLDirectiveContainerbranch.)handleDirective— addedscanAppliedDirectives(argument.getAppliedDirectives())inside the directive-argument loop alongside the existing
scanArgumentTypecall.Adds four regression tests in
FastBuilderTestcovering all three fix locations:applied directive on object type field argument, interface field argument, enum
value, and directive definition argument.
Related
Bug in the
FastBuilder/ShallowTypeRefCollectorintroduced in #4196.