Correctly detect required/optional args/fields

Replicates graphql/graphql-js@0adece9

Author: Cito

graphql/utilities/find_breaking_changes.py

@@ -8,8 +8,8 @@
     GraphQLInterfaceType, GraphQLList, GraphQLNamedType, GraphQLNonNull,
     GraphQLObjectType, GraphQLSchema, GraphQLType, GraphQLUnionType,
     is_enum_type, is_input_object_type, is_interface_type, is_list_type,
-    is_named_type, is_non_null_type, is_object_type, is_scalar_type,
-    is_union_type)
+    is_named_type, is_required_argument, is_required_input_field,
+    is_non_null_type, is_object_type, is_scalar_type, is_union_type)
 
 __all__ = [
     'BreakingChange', 'BreakingChangeType',
@@ -36,22 +36,22 @@ class BreakingChangeType(Enum):
     VALUE_REMOVED_FROM_ENUM = 30
     ARG_REMOVED = 40
     ARG_CHANGED_KIND = 41
-    NON_NULL_ARG_ADDED = 50
-    NON_NULL_INPUT_FIELD_ADDED = 51
+    REQUIRED_ARG_ADDED = 50
+    REQUIRED_INPUT_FIELD_ADDED = 51
     INTERFACE_REMOVED_FROM_OBJECT = 60
     DIRECTIVE_REMOVED = 70
     DIRECTIVE_ARG_REMOVED = 71
     DIRECTIVE_LOCATION_REMOVED = 72
-    NON_NULL_DIRECTIVE_ARG_ADDED = 73
+    REQUIRED_DIRECTIVE_ARG_ADDED = 73
 
 
 class DangerousChangeType(Enum):
     ARG_DEFAULT_VALUE_CHANGE = 42
     VALUE_ADDED_TO_ENUM = 31
     INTERFACE_ADDED_TO_OBJECT = 61
     TYPE_ADDED_TO_UNION = 23
-    NULLABLE_INPUT_FIELD_ADDED = 52
-    NULLABLE_ARG_ADDED = 53
+    OPTIONAL_INPUT_FIELD_ADDED = 52
+    OPTIONAL_ARG_ADDED = 53
 
 
 class BreakingChange(NamedTuple):
@@ -214,20 +214,20 @@ def find_arg_changes(
                         f'{old_type.name}.{field_name} arg'
                         f' {arg_name} has changed defaultValue'))
 
-            # Check if a non-null arg was added to the field
+            # Check if arg was added to the field
             for arg_name in new_args:
                 if arg_name not in old_args:
-                    new_arg = new_args[arg_name]
-                    if is_non_null_type(new_arg.type):
+                    new_arg_def = new_args[arg_name]
+                    if is_required_argument(new_arg_def):
                         breaking_changes.append(BreakingChange(
-                            BreakingChangeType.NON_NULL_ARG_ADDED,
-                            f'A non-null arg {arg_name} on'
-                            f' {new_type.name}.{field_name} was added'))
+                            BreakingChangeType.REQUIRED_ARG_ADDED,
+                            f'A required arg {arg_name} on'
+                            f' {type_name}.{field_name} was added'))
                     else:
                         dangerous_changes.append(DangerousChange(
-                            DangerousChangeType.NULLABLE_ARG_ADDED,
-                            f'A nullable arg {arg_name} on'
-                            f' {new_type.name}.{field_name} was added'))
+                            DangerousChangeType.OPTIONAL_ARG_ADDED,
+                            f'An optional arg {arg_name} on'
+                            f' {type_name}.{field_name} was added'))
 
     return BreakingAndDangerousChanges(breaking_changes, dangerous_changes)
 
@@ -343,16 +343,16 @@ def find_fields_that_changed_type_on_input_object_types(
         # Check if a field was added to the input object type
         for field_name in new_type_fields_def:
             if field_name not in old_type_fields_def:
-                if is_non_null_type(new_type_fields_def[field_name].type):
+                if is_required_input_field(new_type_fields_def[field_name]):
                     breaking_changes.append(BreakingChange(
-                        BreakingChangeType.NON_NULL_INPUT_FIELD_ADDED,
-                        f'A non-null field {field_name} on'
-                        f' input type {new_type.name} was added.'))
+                        BreakingChangeType.REQUIRED_INPUT_FIELD_ADDED,
+                        f'A required field {field_name} on'
+                        f' input type {type_name} was added.'))
                 else:
                     dangerous_changes.append(DangerousChange(
-                        DangerousChangeType.NULLABLE_INPUT_FIELD_ADDED,
-                        f'A nullable field {field_name} on'
-                        f' input type {new_type.name} was added.'))
+                        DangerousChangeType.OPTIONAL_INPUT_FIELD_ADDED,
+                        f'An optional field {field_name} on'
+                        f' input type {type_name} was added.'))
 
     return BreakingAndDangerousChanges(breaking_changes, dangerous_changes)
 
@@ -651,13 +651,11 @@ def find_added_non_null_directive_args(
 
         for arg_name, arg in find_added_args_for_directive(
                 old_directive, new_directive).items():
-            if not is_non_null_type(arg.type):
-                continue
-
-            added_non_nullable_args.append(BreakingChange(
-                BreakingChangeType.NON_NULL_DIRECTIVE_ARG_ADDED,
-                f'A non-null arg {arg_name} on directive'
-                f' {new_directive.name} was added'))
+            if is_required_argument(arg):
+                added_non_nullable_args.append(BreakingChange(
+                    BreakingChangeType.REQUIRED_DIRECTIVE_ARG_ADDED,
+                    f'A required arg {arg_name} on directive'
+                    f' {new_directive.name} was added'))
 
     return added_non_nullable_args
 

tests/utilities/test_find_breaking_changes.py

@@ -244,7 +244,7 @@ def should_detect_if_fields_on_input_types_changed_kind_or_were_removed():
         assert find_fields_that_changed_type_on_input_object_types(
             old_schema, new_schema).breaking_changes == expected_field_changes
 
-    def should_detect_if_a_non_null_field_is_added_to_an_input_type():
+    def should_detect_if_a_required_field_is_added_to_an_input_type():
         old_schema = build_schema("""
             input InputType1 {
               field1: String
@@ -259,7 +259,8 @@ def should_detect_if_a_non_null_field_is_added_to_an_input_type():
             input InputType1 {
                 field1: String
                 requiredField: Int!
-                optionalField: Boolean
+                optionalField1: Boolean
+                optionalField2: Boolean! = false
             }
 
             type Query {
@@ -268,8 +269,8 @@ def should_detect_if_a_non_null_field_is_added_to_an_input_type():
             """)
 
         expected_field_changes = [
-            (BreakingChangeType.NON_NULL_INPUT_FIELD_ADDED,
-             'A non-null field requiredField on input type'
+            (BreakingChangeType.REQUIRED_INPUT_FIELD_ADDED,
+             'A required field requiredField on input type'
              ' InputType1 was added.')]
 
         assert find_fields_that_changed_type_on_input_object_types(
@@ -462,7 +463,7 @@ def should_detect_if_a_field_argument_has_changed_type():
              'Type1.field1 arg arg15 has changed type from [[Int]!]'
                 ' to [[Int!]!]')]
 
-    def should_detect_if_a_non_null_field_argument_was_added():
+    def should_detect_if_a_required_field_argument_was_added():
         old_schema = build_schema("""
             type Type1 {
               field1(arg1: String): String
@@ -475,7 +476,12 @@ def should_detect_if_a_non_null_field_argument_was_added():
 
         new_schema = build_schema("""
             type Type1 {
-              field1(arg1: String, newRequiredArg: String!, newOptionalArg: Int): String
+              field1(
+                arg1: String,
+                newRequiredArg: String!
+                newOptionalArg1: Int
+                newOptionalArg2: Int! = 0
+              ): String
             }
 
             type Query {
@@ -484,8 +490,8 @@ def should_detect_if_a_non_null_field_argument_was_added():
             """)  # noqa
 
         assert find_arg_changes(old_schema, new_schema).breaking_changes == [
-            (BreakingChangeType.NON_NULL_ARG_ADDED,
-             'A non-null arg newRequiredArg on Type1.field1 was added')]
+            (BreakingChangeType.REQUIRED_ARG_ADDED,
+             'A required arg newRequiredArg on Type1.field1 was added')]
 
     def should_not_flag_args_with_the_same_type_signature_as_breaking():
         old_schema = build_schema("""
@@ -700,8 +706,8 @@ def should_detect_all_breaking_changes():
              'DirectiveThatIsRemoved was removed'),
             (BreakingChangeType.DIRECTIVE_ARG_REMOVED,
              'arg1 was removed from DirectiveThatRemovesArg'),
-            (BreakingChangeType.NON_NULL_DIRECTIVE_ARG_ADDED,
-             'A non-null arg arg1 on directive'
+            (BreakingChangeType.REQUIRED_DIRECTIVE_ARG_ADDED,
+             'A required arg arg1 on directive'
              ' NonNullDirectiveAdded was added'),
             (BreakingChangeType.DIRECTIVE_LOCATION_REMOVED,
              'QUERY was removed from DirectiveName')]
@@ -746,18 +752,22 @@ def should_detect_if_a_directive_argument_was_removed():
             (BreakingChangeType.DIRECTIVE_ARG_REMOVED,
              'arg1 was removed from DirectiveWithArg')]
 
-    def should_detect_if_a_non_nullable_directive_argument_was_added():
+    def should_detect_if_an_optional_directive_argument_was_added():
         old_schema = build_schema("""
             directive @DirectiveName on FIELD_DEFINITION
             """)
 
         new_schema = build_schema("""
-            directive @DirectiveName(arg1: Boolean!) on FIELD_DEFINITION
+            directive @DirectiveName(
+              newRequiredArg: String!
+              newOptionalArg1: Int
+              newOptionalArg2: Int! = 0
+            ) on FIELD_DEFINITION
             """)
 
         assert find_added_non_null_directive_args(old_schema, new_schema) == [
-            (BreakingChangeType.NON_NULL_DIRECTIVE_ARG_ADDED,
-             'A non-null arg arg1 on directive DirectiveName was added')]
+            (BreakingChangeType.REQUIRED_DIRECTIVE_ARG_ADDED, 'A required arg'
+             ' newRequiredArg on directive DirectiveName was added')]
 
     def should_detect_locations_removed_from_a_directive():
         d1 = GraphQLDirective('Directive Name', locations=[
@@ -904,7 +914,7 @@ def should_detect_if_a_type_was_added_to_a_union_type():
             (DangerousChangeType.TYPE_ADDED_TO_UNION,
              'Type2 was added to union type UnionType1.')]
 
-    def should_detect_if_a_nullable_field_was_added_to_an_input():
+    def should_detect_if_an_optional_field_was_added_to_an_input():
         old_schema = build_schema("""
             input InputType1 {
                 field1: String
@@ -927,8 +937,8 @@ def should_detect_if_a_nullable_field_was_added_to_an_input():
             """)
 
         expected_field_changes = [
-            (DangerousChangeType.NULLABLE_INPUT_FIELD_ADDED,
-             'A nullable field field2 on input type InputType1 was added.')]
+            (DangerousChangeType.OPTIONAL_INPUT_FIELD_ADDED,
+             'An optional field field2 on input type InputType1 was added.')]
 
         assert find_fields_that_changed_type_on_input_object_types(
             old_schema, new_schema).dangerous_changes == expected_field_changes
@@ -1007,7 +1017,7 @@ def should_find_all_dangerous_changes():
         assert find_dangerous_changes(
             old_schema, new_schema) == expected_dangerous_changes
 
-    def should_detect_if_a_nullable_field_argument_was_added():
+    def should_detect_if_an_optional_field_argument_was_added():
         old_schema = build_schema("""
             type Type1 {
               field1(arg1: String): String
@@ -1029,5 +1039,5 @@ def should_detect_if_a_nullable_field_argument_was_added():
             """)
 
         assert find_arg_changes(old_schema, new_schema).dangerous_changes == [
-            (DangerousChangeType.NULLABLE_ARG_ADDED,
-             'A nullable arg arg2 on Type1.field1 was added')]
+            (DangerousChangeType.OPTIONAL_ARG_ADDED,
+             'An optional arg arg2 on Type1.field1 was added')]