From 5aaad620e960e9726ee5d44dfd2cd76e10bdc4f0 Mon Sep 17 00:00:00 2001 From: goncharu Date: Fri, 11 Sep 2026 18:12:37 -0500 Subject: [PATCH 1/2] Detect REQUIRED_ARG_ADDED when the old field had no prior arguments --- CHANGELOG.md | 4 ++ src/Utils/BreakingChangesFinder.php | 47 ++++++++++++----------- tests/Utils/BreakingChangesFinderTest.php | 42 ++++++++++++++++++++ 3 files changed, 70 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 597124776..784f62506 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Fixed + +- Detect `REQUIRED_ARG_ADDED` in `BreakingChangesFinder::findArgChanges()` when the old field had no prior arguments https://github.com/webonyx/graphql-php/pull/TODO + ## v15.37.2 ### Changed diff --git a/src/Utils/BreakingChangesFinder.php b/src/Utils/BreakingChangesFinder.php index 528cfc8fa..b786d2eb4 100644 --- a/src/Utils/BreakingChangesFinder.php +++ b/src/Utils/BreakingChangesFinder.php @@ -535,33 +535,34 @@ public static function findArgChanges( 'description' => "{$typeName}.{$fieldName} arg {$oldArgDef->name} was removed", ]; } + } - // Check if arg was added to the field - foreach ($newTypeFields[$fieldName]->args as $newTypeFieldArgDef) { - $oldArgDef = null; - foreach ($oldTypeFields[$fieldName]->args as $oldArg) { - if ($oldArg->name === $newTypeFieldArgDef->name) { - $oldArgDef = $oldArg; - } + // Check if arg was added to the field. This must run even when the old + // field had zero args, so it lives outside the loop over $oldField->args. + foreach ($newTypeFields[$fieldName]->args as $newTypeFieldArgDef) { + $oldArgDef = null; + foreach ($oldTypeFields[$fieldName]->args as $oldArg) { + if ($oldArg->name === $newTypeFieldArgDef->name) { + $oldArgDef = $oldArg; } + } - if ($oldArgDef !== null) { - continue; - } + if ($oldArgDef !== null) { + continue; + } - $newTypeName = $newType->name; - $newArgName = $newTypeFieldArgDef->name; - if ($newTypeFieldArgDef->isRequired()) { - $breakingChanges[] = [ - 'type' => self::BREAKING_CHANGE_REQUIRED_ARG_ADDED, - 'description' => "A required arg {$newArgName} on {$newTypeName}.{$fieldName} was added", - ]; - } else { - $dangerousChanges[] = [ - 'type' => self::DANGEROUS_CHANGE_OPTIONAL_ARG_ADDED, - 'description' => "An optional arg {$newArgName} on {$newTypeName}.{$fieldName} was added", - ]; - } + $newTypeName = $newType->name; + $newArgName = $newTypeFieldArgDef->name; + if ($newTypeFieldArgDef->isRequired()) { + $breakingChanges[] = [ + 'type' => self::BREAKING_CHANGE_REQUIRED_ARG_ADDED, + 'description' => "A required arg {$newArgName} on {$newTypeName}.{$fieldName} was added", + ]; + } else { + $dangerousChanges[] = [ + 'type' => self::DANGEROUS_CHANGE_OPTIONAL_ARG_ADDED, + 'description' => "An optional arg {$newArgName} on {$newTypeName}.{$fieldName} was added", + ]; } } } diff --git a/tests/Utils/BreakingChangesFinderTest.php b/tests/Utils/BreakingChangesFinderTest.php index 1fe895d88..9b3a4ba64 100644 --- a/tests/Utils/BreakingChangesFinderTest.php +++ b/tests/Utils/BreakingChangesFinderTest.php @@ -855,6 +855,48 @@ public function testShouldDetectIfANonNullFieldArgumentWasAdded(): void ); } + public function testShouldDetectIfANonNullFieldArgumentWasAddedToAFieldWithNoPriorArgs(): void + { + $oldType = new ObjectType([ + 'name' => 'Type1', + 'fields' => [ + 'field1' => [ + 'type' => Type::string(), + 'args' => [], + ], + ], + ]); + $newType = new ObjectType([ + 'name' => 'Type1', + 'fields' => [ + 'field1' => [ + 'type' => Type::string(), + 'args' => [ + 'newRequiredArg' => Type::nonNull(Type::string()), + ], + ], + ], + ]); + $oldSchema = new Schema([ + 'query' => $this->queryType, + 'types' => [$oldType], + ]); + $newSchema = new Schema([ + 'query' => $this->queryType, + 'types' => [$newType], + ]); + + self::assertSame( + [ + [ + 'type' => BreakingChangesFinder::BREAKING_CHANGE_REQUIRED_ARG_ADDED, + 'description' => 'A required arg newRequiredArg on Type1.field1 was added', + ], + ], + BreakingChangesFinder::findArgChanges($oldSchema, $newSchema)['breakingChanges'] + ); + } + /** @see it('should not flag args with the same type signature as breaking') */ public function testShouldNotFlagArgsWithTheSameTypeSignatureAsBreaking(): void { From 4474cc946f1c4fdc29515bb112769b69a9548ed3 Mon Sep 17 00:00:00 2001 From: goncharu Date: Fri, 11 Sep 2026 18:21:20 -0500 Subject: [PATCH 2/2] Add PR link to changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 784f62506..ad69a6677 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ You can find and compare releases at the [GitHub release page](https://github.co ### Fixed -- Detect `REQUIRED_ARG_ADDED` in `BreakingChangesFinder::findArgChanges()` when the old field had no prior arguments https://github.com/webonyx/graphql-php/pull/TODO +- Detect `REQUIRED_ARG_ADDED` in `BreakingChangesFinder::findArgChanges()` when the old field had no prior arguments https://github.com/webonyx/graphql-php/pull/1976 ## v15.37.2