Add warnings for nullable changes

ref: db4cfdc31d
ref: graphql/graphql-js#1096

# Conflicts:
#	tests/Utils/FindBreakingChangesTest.php

src/Utils/FindBreakingChanges.php

@@ -34,20 +34,8 @@ class FindBreakingChanges
     const DANGEROUS_CHANGE_ARG_DEFAULT_VALUE = 'ARG_DEFAULT_VALUE_CHANGE';
     const DANGEROUS_CHANGE_VALUE_ADDED_TO_ENUM = 'VALUE_ADDED_TO_ENUM';
     const DANGEROUS_CHANGE_TYPE_ADDED_TO_UNION = 'TYPE_ADDED_TO_UNION';
-
+    const DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED = 'NULLABLE_INPUT_FIELD_ADDED';
-    /**
+    const DANGEROUS_CHANGE_NULLABLE_ARG_ADDED = 'NULLABLE_ARG_ADDED';
-     * Given two schemas, returns an Array containing descriptions of all the types
-     * of potentially dangerous changes covered by the other functions down below.
-     *
-     * @return array
-     */
-    public static function findDangerousChanges(Schema $oldSchema, Schema $newSchema)
-    {
-        return array_merge(self::findArgChanges($oldSchema, $newSchema)['dangerousChanges'],
-            self::findValuesAddedToEnums($oldSchema, $newSchema),
-            self::findTypesAddedToUnions($oldSchema, $newSchema)
-        );
-    }
 
     /**
      * Given two schemas, returns an Array containing descriptions of all the types
@@ -60,7 +48,8 @@ class FindBreakingChanges
         return array_merge(
             self::findRemovedTypes($oldSchema, $newSchema),
             self::findTypesThatChangedKind($oldSchema, $newSchema),
-            self::findFieldsThatChangedType($oldSchema, $newSchema),
+            self::findFieldsThatChangedTypeOnObjectOrInterfaceTypes($oldSchema, $newSchema),
+            self::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['breakingChanges'],
             self::findTypesRemovedFromUnions($oldSchema, $newSchema),
             self::findValuesRemovedFromEnums($oldSchema, $newSchema),
             self::findArgChanges($oldSchema, $newSchema)['breakingChanges'],
@@ -68,6 +57,22 @@ class FindBreakingChanges
         );
     }
 
+    /**
+     * Given two schemas, returns an Array containing descriptions of all the types
+     * of potentially dangerous changes covered by the other functions down below.
+     *
+     * @return array
+     */
+    public static function findDangerousChanges(Schema $oldSchema, Schema $newSchema)
+    {
+        return array_merge(
+            self::findArgChanges($oldSchema, $newSchema)['dangerousChanges'],
+            self::findValuesAddedToEnums($oldSchema, $newSchema),
+            self::findTypesAddedToUnions($oldSchema, $newSchema),
+            self::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['dangerousChanges']
+        );
+    }
+
     /**
      * Given two schemas, returns an Array containing descriptions of any breaking
      * changes in the newSchema related to removing an entire type.
@@ -195,13 +200,20 @@ class FindBreakingChanges
                             }
                         );
 
-                        if (!$oldArgDef && $newArgDef->getType() instanceof NonNull) {
+                        if (!$oldArgDef) {
                             $newTypeName = $newTypeDefinition->name;
                             $newArgName = $newArgDef->name;
+                            if ($newArgDef->getType() instanceof NonNull) {
                                 $breakingChanges[] = [
                                     'type' => self::BREAKING_CHANGE_NON_NULL_ARG_ADDED,
                                     'description' => "A non-null arg ${newArgName} on ${newTypeName}->${fieldName} was added."
                                 ];
+                            } else {
+                                $dangerousChanges[] = [
+                                    'type' => self::DANGEROUS_CHANGE_NULLABLE_ARG_ADDED,
+                                    'description' => "A nullable arg ${newArgName} on ${newTypeName}->${fieldName} was added."
+                                ];
+                            }
                         }
                     }
                 }
@@ -236,36 +248,18 @@ class FindBreakingChanges
         throw new \TypeError('unknown type ' . $type->name);
     }
 
-    /**
-     * Given two schemas, returns an Array containing descriptions of any breaking
-     * changes in the newSchema related to the fields on a type. This includes if
-     * a field has been removed from a type, if a field has changed type, or if
-     * a non-null field is added to an input type.
-     *
-     * @return array
-     */
-    public static function findFieldsThatChangedType(
-        Schema $oldSchema, Schema $newSchema
-    )
-    {
-        return array_merge(
-            self::findFieldsThatChangedTypeOnObjectOrInterfaceTypes($oldSchema, $newSchema),
-            self::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)
-        );
-    }
-
     /**
      * @param Schema $oldSchema
      * @param Schema $newSchema
      *
      * @return array
      */
-    private static function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(Schema $oldSchema, Schema $newSchema)
+    public static function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(Schema $oldSchema, Schema $newSchema)
     {
         $oldTypeMap = $oldSchema->getTypeMap();
         $newTypeMap = $newSchema->getTypeMap();
 
-        $breakingFieldChanges = [];
+        $breakingChanges = [];
         foreach ($oldTypeMap as $typeName => $oldType) {
             $newType = isset($newTypeMap[$typeName]) ? $newTypeMap[$typeName] : null;
             if (!($oldType instanceof ObjectType || $oldType instanceof InterfaceType) || !($newType instanceof $oldType)) {
@@ -275,7 +269,7 @@ class FindBreakingChanges
             $newTypeFieldsDef = $newType->getFields();
             foreach ($oldTypeFieldsDef as $fieldName => $fieldDefinition) {
                 if (!isset($newTypeFieldsDef[$fieldName])) {
-                    $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_REMOVED, 'description' => "${typeName}->${fieldName} was removed."];
+                    $breakingChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_REMOVED, 'description' => "${typeName}->${fieldName} was removed."];
                 } else {
                     $oldFieldType = $oldTypeFieldsDef[$fieldName]->getType();
                     $newfieldType = $newTypeFieldsDef[$fieldName]->getType();
@@ -284,12 +278,12 @@ class FindBreakingChanges
 
                         $oldFieldTypeString = self::isNamedType($oldFieldType) ? $oldFieldType->name : $oldFieldType;
                         $newFieldTypeString = self::isNamedType($newfieldType) ? $newfieldType->name : $newfieldType;
-                        $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_CHANGED, 'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."];
+                        $breakingChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_CHANGED, 'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."];
                     }
                 }
             }
         }
-        return $breakingFieldChanges;
+        return $breakingChanges;
     }
 
     /**
@@ -305,7 +299,8 @@ class FindBreakingChanges
         $oldTypeMap = $oldSchema->getTypeMap();
         $newTypeMap = $newSchema->getTypeMap();
 
-        $breakingFieldChanges = [];
+        $breakingChanges = [];
+        $dangerousChanges = [];
         foreach ($oldTypeMap as $typeName => $oldType) {
             $newType = isset($newTypeMap[$typeName]) ? $newTypeMap[$typeName] : null;
             if (!($oldType instanceof InputObjectType) || !($newType instanceof InputObjectType)) {
@@ -315,7 +310,10 @@ class FindBreakingChanges
             $newTypeFieldsDef = $newType->getFields();
             foreach ($oldTypeFieldsDef as $fieldName => $fieldDefinition) {
                 if (!isset($newTypeFieldsDef[$fieldName])) {
-                    $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_REMOVED, 'description' => "${typeName}->${fieldName} was removed."];
+                    $breakingChanges[] = [
+                        'type' => self::BREAKING_CHANGE_FIELD_REMOVED,
+                        'description' => "${typeName}->${fieldName} was removed."
+                    ];
                 } else {
                     $oldFieldType = $oldTypeFieldsDef[$fieldName]->getType();
                     $newfieldType = $newTypeFieldsDef[$fieldName]->getType();
@@ -323,18 +321,32 @@ class FindBreakingChanges
                     if (!$isSafe) {
                         $oldFieldTypeString = self::isNamedType($oldFieldType) ? $oldFieldType->name : $oldFieldType;
                         $newFieldTypeString = self::isNamedType($newfieldType) ? $newfieldType->name : $newfieldType;
-                        $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_CHANGED, 'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."];
+                        $breakingChanges[] = [
+                            'type' => self::BREAKING_CHANGE_FIELD_CHANGED,
+                            'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."];
                     }
                 }
             }
+            // Check if a field was added to the input object type
             foreach ($newTypeFieldsDef as $fieldName => $fieldDef) {
-                if (!isset($oldTypeFieldsDef[$fieldName]) && $fieldDef->getType() instanceof NonNull) {
+                if (!isset($oldTypeFieldsDef[$fieldName])) {
                     $newTypeName = $newType->name;
-                    $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED, 'description' => "A non-null field ${fieldName} on input type ${newTypeName} was added."];
+                    if ($fieldDef->getType() instanceof NonNull) {
+                        $breakingChanges[] = [
+                            'type' => self::BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED,
+                            'description' => "A non-null field ${fieldName} on input type ${newTypeName} was added."
+                        ];
+                    } else {
+                        $dangerousChanges[] = [
+                            'type' => self::DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED,
+                            'description' => "A nullable field ${fieldName} on input type ${newTypeName} was added."
+                        ];
                     }
                 }
             }
-        return $breakingFieldChanges;
+        }
+
+        return ['breakingChanges' => $breakingChanges, 'dangerousChanges' => $dangerousChanges];
 
     }
 

tests/Utils/FindBreakingChangesTest.php

@@ -254,7 +254,7 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase
             ])
         ]);
 
-        $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedType($oldSchema, $newSchema));
+        $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedTypeOnObjectOrInterfaceTypes($oldSchema, $newSchema));
     }
 
 
@@ -424,7 +424,7 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase
             ],
         ];
 
-        $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedType($oldSchema, $newSchema));
+        $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['breakingChanges']);
     }
 
     public function testDetectsNonNullFieldAddedToInputType()
@@ -468,7 +468,7 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase
                 'type' => FindBreakingChanges::BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED,
                 'description' => 'A non-null field requiredField on input type InputType1 was added.'
             ],
-            FindBreakingChanges::findFieldsThatChangedType($oldSchema, $newSchema)[0]
+            FindBreakingChanges::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['breakingChanges'][0]
         );
     }
 
@@ -1366,6 +1366,55 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    /**
+     * @it should detect if a nullable field was added to an input
+     */
+    public function testShouldDetectIfANullableFieldWasAddedToAnInput()
+    {
+        $oldInputType = new InputObjectType([
+            'name' => 'InputType1',
+            'fields' => [
+                'field1' => [
+                    'type' => Type::string(),
+                ],
+            ],
+        ]);
+        $newInputType = new InputObjectType([
+            'name' => 'InputType1',
+            'fields' => [
+                'field1' => [
+                    'type' => Type::string(),
+                ],
+                'field2' => [
+                    'type' => Type::int(),
+                ],
+            ],
+        ]);
+
+        $oldSchema = new Schema([
+            'query' => $this->queryType,
+            'types' => [
+                $oldInputType,
+            ]
+        ]);
+
+        $newSchema = new Schema([
+            'query' => $this->queryType,
+            'types' => [
+                $newInputType,
+            ]
+        ]);
+
+        $expectedFieldChanges = [
+            [
+                'description' => 'A nullable field field2 on input type InputType1 was added.',
+                'type' => FindBreakingChanges::DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED
+            ],
+        ];
+
+        $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['dangerousChanges']);
+    }
+
     public function testFindsAllDangerousChanges()
     {
         $enumThatGainsAValueOld = new EnumType([
@@ -1473,4 +1522,64 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase
 
         $this->assertEquals($expectedDangerousChanges, FindBreakingChanges::findDangerousChanges($oldSchema, $newSchema));
     }
+
+    /**
+     * @it should detect if a nullable field argument was added
+     */
+    public function testShouldDetectIfANullableFieldArgumentWasAdded()
+    {
+        $oldType = new ObjectType([
+            'name' => 'Type1',
+            'fields' => [
+                'field1' => [
+                    'type' => Type::string(),
+                    'args' => [
+                        'arg1' => [
+                            'type' => Type::string(),
+                        ],
+                    ],
+                ],
+            ],
+        ]);
+
+        $newType = new ObjectType([
+            'name' => 'Type1',
+            'fields' => [
+                'field1' => [
+                    'type' => Type::string(),
+                    'args' => [
+                        'arg1' => [
+                            'type' => Type::string(),
+                        ],
+                        'arg2' => [
+                            'type' => Type::string(),
+                        ],
+                    ],
+                ],
+            ],
+        ]);
+
+        $oldSchema = new Schema([
+            'query' => $this->queryType,
+            'types' => [
+                $oldType,
+            ]
+        ]);
+
+        $newSchema = new Schema([
+            'query' => $this->queryType,
+            'types' => [
+                $newType,
+            ]
+        ]);
+
+        $expectedFieldChanges = [
+            [
+                'description' => 'A nullable arg arg2 on Type1->field1 was added.',
+                'type' => FindBreakingChanges::DANGEROUS_CHANGE_NULLABLE_ARG_ADDED
+            ],
+        ];
+
+        $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findArgChanges($oldSchema, $newSchema)['dangerousChanges']);
+    }
 }