Guard GraphQL built-in scalars from incorrect mutation renaming#70
Open
FionaBronwen wants to merge 1 commit intofionabronwen/graphql-scalar-idfrom
Open
Guard GraphQL built-in scalars from incorrect mutation renaming#70FionaBronwen wants to merge 1 commit intofionabronwen/graphql-scalar-idfrom
FionaBronwen wants to merge 1 commit intofionabronwen/graphql-scalar-idfrom
Conversation
134b92a to
397b7e5
Compare
82569eb to
621c9a7
Compare
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
isGraphQLBuiltinScalar()guard to prevent TypeSpec built-in scalars (string,boolean,int32,float32,float64) from being incorrectly renamed by the scalar mutation engine when they inherit a mapping from an ancestor in the extends chain (e.g.,float32→float→numeric→Numeric)isStdScalar+ name lookup) to avoid false positives from user-defined scalarsthat share a built-in name in a different namespace
float32from being renamed to Numeric due togetScalarMappingwalking the extends chain tonumeric, which has a mapping entry. GraphQL'sFloatis IEEE 754 double-precision, so bothfloat32andfloat64map to it natively — no custom scalar needed.Test plan
Added the following unit tests:
float32is not renamed despite inheritingNumericmapping from numeric ancestorfloat64is not renamed (also maps to GraphQLFloat)int32is not renamed (maps to GraphQLInt)int64is still correctly renamed toLong(not a GraphQL built-in)All 110 tests pass