Repository navigation
Conversation
# Conflicts: # docs/class-reference.md # src/Language/Parser.php # src/Language/Printer.php # src/Validator/Rules/VariablesInAllowedPosition.php
spawnia
changed the title
spawnia
added
the
breaking change
simPod
reviewed
Mar 8, 2023
simPod
left a comment
simPod
left a comment
Collaborator
There was a problem hiding this comment.
TBH I'm not really able to evaluate this addition since I don't have the required gql expertise but I don't see any harm in this either.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a separation between “const” and “non-const” GraphQL AST value nodes (notably for list/object literals), and updates parsing/printing/validation utilities to accept the new const node variants where appropriate.
Changes:
- Add
ConstValueNodeand const-specific AST node classes for list/object literals (and fields). - Update
Parser,Printer,TypeInfo, and several validation rules/utilities to handle const node variants. - Update docs/changelog and adjust phpstan type aliases to reflect the new AST model.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Executor/VariablesTest.php | Test comment/docblock adjustments around variable usage. |
| src/Executor/Values.php | Update phpstan value-node typing import/usage for argument maps. |
| src/Language/AST/ArgumentNode.php | Switch phpstan typing to ValueNodeVariants. |
| src/Language/AST/BooleanValueNode.php | Mark boolean literals as implementing ConstValueNode. |
| src/Language/AST/ConstArgumentNode.php | Add const-only argument AST node type. |
| src/Language/AST/ConstListValueNode.php | Add const-only list literal AST node type. |
| src/Language/AST/ConstObjectFieldNode.php | Add const-only object field AST node type. |
| src/Language/AST/ConstObjectValueNode.php | Add const-only object literal AST node type. |
| src/Language/AST/ConstValueNode.php | Add new marker interface + phpstan const variants alias. |
| src/Language/AST/EnumValueNode.php | Mark enum literals as implementing ConstValueNode. |
| src/Language/AST/FloatValueNode.php | Mark float literals as implementing ConstValueNode. |
| src/Language/AST/InputValueDefinitionNode.php | Adjust phpstan typing comment for default values. |
| src/Language/AST/IntValueNode.php | Mark int literals as implementing ConstValueNode. |
| src/Language/AST/ListValueNode.php | Update list literal node interfaces (const/non-const differentiation). |
| src/Language/AST/NullValueNode.php | Mark null literals as implementing ConstValueNode. |
| src/Language/AST/ObjectFieldNode.php | Refine phpstan typing for object field values. |
| src/Language/AST/ObjectValueNode.php | Update object literal node interfaces (const/non-const differentiation). |
| src/Language/AST/StringValueNode.php | Mark string literals as implementing ConstValueNode. |
| src/Language/AST/TypeNode.php | Replace TS-style union comment with phpstan TypeNodeVariants. |
| src/Language/AST/TypeNode.php | Replace TS-style union comment with phpstan TypeNodeVariants. |
| src/Language/AST/ValueNode.php | Replace TS-style union comment with phpstan ValueNodeVariants. |
| src/Language/AST/VariableDefinitionNode.php | Change variable default values to ?ConstValueNode. |
| src/Language/Parser.php | Emit const-specific list/object/field nodes in const contexts; update return typing. |
| src/Language/Printer.php | Print const list/object/object-field nodes; adjust internal printer signature. |
| src/Type/Definition/CustomScalarType.php | Broaden phpstan types for parseLiteral to accept const value nodes. |
| src/Type/Definition/LeafType.php | Update parseLiteral phpdoc to accept const value nodes. |
| src/Utils/AST.php | Accept/handle const list/object nodes in value coercion helpers; use LeafType. |
| src/Utils/TypeInfo.php | Handle const list/object-field nodes when tracking input type/default stacks. |
| src/Validator/Rules/UniqueInputFieldNames.php | Allow const object-field nodes in the rule visitor. |
| src/Validator/Rules/ValuesOfCorrectType.php | Allow const list/object/object-field nodes in validation visitors. |
| src/Validator/Rules/VariablesInAllowedPosition.php | Type variable default values as ?ConstValueNode. |
| docs/class-reference.md | Update generated API docs for new phpstan type aliases and signatures. |
| CHANGELOG.md | Note the AST const/non-const differentiation under “Unreleased”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| namespace GraphQL\Language\AST; | ||
|
|
||
| class ListValueNode extends Node implements ValueNode | ||
| class ListValueNode extends Node implements ValueNode, ConstValueNode |
| namespace GraphQL\Language\AST; | ||
|
|
||
| class ObjectValueNode extends Node implements ValueNode | ||
| class ObjectValueNode extends Node implements ValueNode, ConstValueNode |
Comment on lines
14
to
18
| /** @var NamedTypeNode|ListTypeNode|NonNullTypeNode */ | ||
| public TypeNode $type; | ||
|
|
||
| /** @var VariableNode|NullValueNode|IntValueNode|FloatValueNode|StringValueNode|BooleanValueNode|EnumValueNode|ListValueNode|ObjectValueNode|null */ | ||
| /** @var ValueNodeVariants|null */ | ||
| public ?ValueNode $defaultValue = null; |
Comment on lines
+1010
to
1014
| private function parseConstValue(): ConstValueNode | ||
| { | ||
| // @phpstan-ignore-next-line return type depends on argument | ||
| return $this->parseValueLiteral(true); | ||
| } |
🤖 Generated with Claude Code
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: fb7bb32 | Previous: 32edc67 | Ratio |
|---|---|---|---|
DeferredBench::benchNestedDeferred |
0.002 ms |
0.001 ms |
2 |
DeferredBench::benchManyNestedDeferreds |
9.535 ms |
6.202 ms |
1.54 |
This comment was automatically generated by workflow using github-action-benchmark.
This branch has not been deployed
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.
ConstValueNodeneeds to be usedChangelog
Changed