Skip to content

Commit 5121125

Browse files
vladarvladar
committed
Correctly detect required/optional args/fields
See graphql/graphql-js#1465 (comment)
1 parent ba7e60f commit 5121125

File tree

3 files changed

+45
-35
lines changed

3 files changed

+45
-35
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@
66
- **BREAKING:** Removal of `VariablesDefaultValueAllowed` validation rule. All variables may now specify a default value.
77
- **BREAKING:** renamed `ProvidedNonNullArguments` to `ProvidedRequiredArguments` (no longer require values to be provided to non-null arguments which provide a default value).
88
- **BREAKING:** `GraphQL\Deferred` now extends `GraphQL\Executor\Promise\Adapter\SyncPromise`
9+
- **BREAKING:** renamed following types of dangerous/breaking changes (returned by `BreakingChangesFinder`):
10+
- `NON_NULL_ARG_ADDED` to `REQUIRED_ARG_ADDED`
11+
- `NON_NULL_INPUT_FIELD_ADDED` to `REQUIRED_INPUT_FIELD_ADDED`
12+
- `NON_NULL_DIRECTIVE_ARG_ADDED` to `REQUIRED_DIRECTIVE_ARG_ADDED`
13+
- `NULLABLE_INPUT_FIELD_ADDED` to `OPTIONAL_INPUT_FIELD_ADDED`
14+
- `NULLABLE_ARG_ADDED` to `OPTIONAL_ARG_ADDED`
915
- Add schema validation: Input Objects must not contain non-nullable circular references (#492)
1016
- Added retrieving query complexity once query has been completed (#316)
1117
- Allow input types to be passed in from variables using \stdClass instead of associative arrays (#535)

‎src/Utils/BreakingChangesFinder.php

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,19 @@ class BreakingChangesFinder
3939
public const BREAKING_CHANGE_VALUE_REMOVED_FROM_ENUM = 'VALUE_REMOVED_FROM_ENUM';
4040
public const BREAKING_CHANGE_ARG_REMOVED = 'ARG_REMOVED';
4141
public const BREAKING_CHANGE_ARG_CHANGED_KIND = 'ARG_CHANGED_KIND';
42-
public const BREAKING_CHANGE_NON_NULL_ARG_ADDED = 'NON_NULL_ARG_ADDED';
43-
public const BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED = 'NON_NULL_INPUT_FIELD_ADDED';
42+
public const BREAKING_CHANGE_REQUIRED_ARG_ADDED = 'REQUIRED_ARG_ADDED';
43+
public const BREAKING_CHANGE_REQUIRED_INPUT_FIELD_ADDED = 'REQUIRED_INPUT_FIELD_ADDED';
4444
public const BREAKING_CHANGE_INTERFACE_REMOVED_FROM_OBJECT = 'INTERFACE_REMOVED_FROM_OBJECT';
4545
public const BREAKING_CHANGE_DIRECTIVE_REMOVED = 'DIRECTIVE_REMOVED';
4646
public const BREAKING_CHANGE_DIRECTIVE_ARG_REMOVED = 'DIRECTIVE_ARG_REMOVED';
4747
public const BREAKING_CHANGE_DIRECTIVE_LOCATION_REMOVED = 'DIRECTIVE_LOCATION_REMOVED';
48-
public const BREAKING_CHANGE_NON_NULL_DIRECTIVE_ARG_ADDED = 'NON_NULL_DIRECTIVE_ARG_ADDED';
48+
public const BREAKING_CHANGE_REQUIRED_DIRECTIVE_ARG_ADDED = 'REQUIRED_DIRECTIVE_ARG_ADDED';
4949
public const DANGEROUS_CHANGE_ARG_DEFAULT_VALUE_CHANGED = 'ARG_DEFAULT_VALUE_CHANGE';
5050
public const DANGEROUS_CHANGE_VALUE_ADDED_TO_ENUM = 'VALUE_ADDED_TO_ENUM';
5151
public const DANGEROUS_CHANGE_INTERFACE_ADDED_TO_OBJECT = 'INTERFACE_ADDED_TO_OBJECT';
5252
public const DANGEROUS_CHANGE_TYPE_ADDED_TO_UNION = 'TYPE_ADDED_TO_UNION';
53-
public const DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED = 'NULLABLE_INPUT_FIELD_ADDED';
54-
public const DANGEROUS_CHANGE_NULLABLE_ARG_ADDED = 'NULLABLE_ARG_ADDED';
53+
public const DANGEROUS_CHANGE_OPTIONAL_INPUT_FIELD_ADDED = 'OPTIONAL_INPUT_FIELD_ADDED';
54+
public const DANGEROUS_CHANGE_OPTIONAL_ARG_ADDED = 'OPTIONAL_ARG_ADDED';
5555

5656
/**
5757
* Given two schemas, returns an Array containing descriptions of all the types
@@ -328,15 +328,15 @@ public static function findFieldsThatChangedTypeOnInputObjectTypes(
328328
}
329329

330330
$newTypeName = $newType->name;
331-
if ($fieldDef->getType() instanceof NonNull) {
331+
if ($fieldDef->isRequired()) {
332332
$breakingChanges[] = [
333-
'type' => self::BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED,
334-
'description' => "A non-null field ${fieldName} on input type ${newTypeName} was added.",
333+
'type' => self::BREAKING_CHANGE_REQUIRED_INPUT_FIELD_ADDED,
334+
'description' => "A required field ${fieldName} on input type ${newTypeName} was added.",
335335
];
336336
} else {
337337
$dangerousChanges[] = [
338-
'type' => self::DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED,
339-
'description' => "A nullable field ${fieldName} on input type ${newTypeName} was added.",
338+
'type' => self::DANGEROUS_CHANGE_OPTIONAL_INPUT_FIELD_ADDED,
339+
'description' => "An optional field ${fieldName} on input type ${newTypeName} was added.",
340340
];
341341
}
342342
}
@@ -539,7 +539,7 @@ static function ($arg) use ($oldArgDef) : bool {
539539
),
540540
];
541541
}
542-
// Check if a non-null arg was added to the field
542+
// Check if arg was added to the field
543543
foreach ($newTypeFields[$fieldName]->args as $newTypeFieldArgDef) {
544544
$oldArgs = $oldTypeFields[$fieldName]->args;
545545
$oldArgDef = Utils::find(
@@ -555,15 +555,15 @@ static function ($arg) use ($newTypeFieldArgDef) : bool {
555555

556556
$newTypeName = $newType->name;
557557
$newArgName = $newTypeFieldArgDef->name;
558-
if ($newTypeFieldArgDef->getType() instanceof NonNull) {
558+
if ($newTypeFieldArgDef->isRequired()) {
559559
$breakingChanges[] = [
560-
'type' => self::BREAKING_CHANGE_NON_NULL_ARG_ADDED,
561-
'description' => "A non-null arg ${newArgName} on ${newTypeName}.${fieldName} was added",
560+
'type' => self::BREAKING_CHANGE_REQUIRED_ARG_ADDED,
561+
'description' => "A required arg ${newArgName} on ${newTypeName}.${fieldName} was added",
562562
];
563563
} else {
564564
$dangerousChanges[] = [
565-
'type' => self::DANGEROUS_CHANGE_NULLABLE_ARG_ADDED,
566-
'description' => "A nullable arg ${newArgName} on ${newTypeName}.${fieldName} was added",
565+
'type' => self::DANGEROUS_CHANGE_OPTIONAL_ARG_ADDED,
566+
'description' => "An optional arg ${newArgName} on ${newTypeName}.${fieldName} was added",
567567
];
568568
}
569569
}
@@ -712,13 +712,13 @@ public static function findAddedNonNullDirectiveArgs(Schema $oldSchema, Schema $
712712
$oldSchemaDirectiveMap[$newDirective->name],
713713
$newDirective
714714
) as $arg) {
715-
if (! $arg->getType() instanceof NonNull) {
715+
if (! $arg->isRequired()) {
716716
continue;
717717
}
718718
$addedNonNullableArgs[] = [
719-
'type' => self::BREAKING_CHANGE_NON_NULL_DIRECTIVE_ARG_ADDED,
719+
'type' => self::BREAKING_CHANGE_REQUIRED_DIRECTIVE_ARG_ADDED,
720720
'description' => sprintf(
721-
'A non-null arg %s on directive %s was added',
721+
'A required arg %s on directive %s was added',
722722
$arg->name,
723723
$newDirective->name
724724
),

‎tests/Utils/BreakingChangesFinderTest.php

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ public function testShouldDetectIfFieldsOnInputTypesChangedKindOrWereRemoved() :
486486
}
487487

488488
/**
489-
* @see it('should detect if a non-null field is added to an input type')
489+
* @see it('should detect if a required field is added to an input type')
490490
*/
491491
public function testShouldDetectIfANonNullFieldIsAddedToAnInputType() : void
492492
{
@@ -500,9 +500,13 @@ public function testShouldDetectIfANonNullFieldIsAddedToAnInputType() : void
500500
$newInputType = new InputObjectType([
501501
'name' => 'InputType1',
502502
'fields' => [
503-
'field1' => Type::string(),
504-
'requiredField' => Type::nonNull(Type::int()),
505-
'optionalField' => Type::boolean(),
503+
'field1' => Type::string(),
504+
'requiredField' => Type::nonNull(Type::int()),
505+
'optionalField1' => Type::boolean(),
506+
'optionalField2' => [
507+
'type' => Type::nonNull(Type::boolean()),
508+
'defaultValue' => false,
509+
],
506510
],
507511
]);
508512

@@ -518,8 +522,8 @@ public function testShouldDetectIfANonNullFieldIsAddedToAnInputType() : void
518522

519523
$expected = [
520524
[
521-
'type' => BreakingChangesFinder::BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED,
522-
'description' => 'A non-null field requiredField on input type InputType1 was added.',
525+
'type' => BreakingChangesFinder::BREAKING_CHANGE_REQUIRED_INPUT_FIELD_ADDED,
526+
'description' => 'A required field requiredField on input type InputType1 was added.',
523527
],
524528
];
525529

@@ -889,8 +893,8 @@ public function testShouldDetectIfANonNullFieldArgumentWasAdded() : void
889893

890894
$expected = [
891895
[
892-
'type' => BreakingChangesFinder::BREAKING_CHANGE_NON_NULL_ARG_ADDED,
893-
'description' => 'A non-null arg newRequiredArg on Type1.field1 was added',
896+
'type' => BreakingChangesFinder::BREAKING_CHANGE_REQUIRED_ARG_ADDED,
897+
'description' => 'A required arg newRequiredArg on Type1.field1 was added',
894898
],
895899
];
896900

@@ -1299,8 +1303,8 @@ public function testShouldDetectAllBreakingChanges() : void
12991303
'description' => 'arg1 was removed from DirectiveThatRemovesArg',
13001304
],
13011305
[
1302-
'type' => BreakingChangesFinder::BREAKING_CHANGE_NON_NULL_DIRECTIVE_ARG_ADDED,
1303-
'description' => 'A non-null arg arg1 on directive NonNullDirectiveAdded was added',
1306+
'type' => BreakingChangesFinder::BREAKING_CHANGE_REQUIRED_DIRECTIVE_ARG_ADDED,
1307+
'description' => 'A required arg arg1 on directive NonNullDirectiveAdded was added',
13041308
],
13051309
[
13061310
'type' => BreakingChangesFinder::BREAKING_CHANGE_DIRECTIVE_LOCATION_REMOVED,
@@ -1441,8 +1445,8 @@ public function testShouldDetectIfANonNullableDirectiveArgumentWasAdded() : void
14411445

14421446
$expectedBreakingChanges = [
14431447
[
1444-
'type' => BreakingChangesFinder::BREAKING_CHANGE_NON_NULL_DIRECTIVE_ARG_ADDED,
1445-
'description' => 'A non-null arg arg1 on directive DirectiveName was added',
1448+
'type' => BreakingChangesFinder::BREAKING_CHANGE_REQUIRED_DIRECTIVE_ARG_ADDED,
1449+
'description' => 'A required arg arg1 on directive DirectiveName was added',
14461450
],
14471451
];
14481452

@@ -1761,8 +1765,8 @@ public function testShouldDetectIfANullableFieldWasAddedToAnInput() : void
17611765

17621766
$expectedFieldChanges = [
17631767
[
1764-
'description' => 'A nullable field field2 on input type InputType1 was added.',
1765-
'type' => BreakingChangesFinder::DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED,
1768+
'description' => 'An optional field field2 on input type InputType1 was added.',
1769+
'type' => BreakingChangesFinder::DANGEROUS_CHANGE_OPTIONAL_INPUT_FIELD_ADDED,
17661770
],
17671771
];
17681772

@@ -1962,8 +1966,8 @@ public function testShouldDetectIfANullableFieldArgumentWasAdded() : void
19621966

19631967
$expectedFieldChanges = [
19641968
[
1965-
'description' => 'A nullable arg arg2 on Type1.field1 was added',
1966-
'type' => BreakingChangesFinder::DANGEROUS_CHANGE_NULLABLE_ARG_ADDED,
1969+
'description' => 'An optional arg arg2 on Type1.field1 was added',
1970+
'type' => BreakingChangesFinder::DANGEROUS_CHANGE_OPTIONAL_ARG_ADDED,
19671971
],
19681972
];
19691973

0 commit comments

Comments
 (0)