[Validation] Allow safe divergence

Related GraphQL commit https://github.com/graphql/graphql-js/tree/d71e063fdd1d4c376b4948147e54438b6f1e13de

Author: syrusakbary

graphql/core/validation/rules/overlapping_fields_can_be_merged.py

@@ -36,11 +36,27 @@ def find_conflicts(self, field_map):
 
         return conflicts
 
-    def find_conflict(self, response_name, pair1, pair2):
-        ast1, def1 = pair1
-        ast2, def2 = pair2
+    def find_conflict(self, response_name, field1, field2):
+        parent_type1, ast1, def1 = field1
+        parent_type2, ast2, def2 = field2
 
-        if ast1 is ast2 or self.compared_set.has(ast1, ast2):
+        # Not a pair
+        if ast1 is ast2:
+            return
+
+        # If the statically known parent types could not possibly apply at the same
+        # time, then it is safe to permit them to diverge as they will not present
+        # any ambiguity by differing.
+        # It is known that two parent types could never overlap if they are
+        # different Object types. Interface or Union types might overlap - if not
+        # in the current state of the schema, then perhaps in some future version,
+        # thus may not safely diverge.
+        if parent_type1 != parent_type2 and \
+            isinstance(parent_type1, GraphQLObjectType) and \
+            isinstance(parent_type2, GraphQLObjectType):
+            return
+
+        if self.compared_set.has(ast1, ast2):
             return
 
         self.compared_set.add(ast1, ast2)
@@ -192,7 +208,7 @@ def collect_field_asts_and_defs(self, parent_type, selection_set, visited_fragme
                     field_def = parent_type.get_fields().get(field_name)
 
                 response_name = selection.alias.value if selection.alias else field_name
-                ast_and_defs[response_name].append((selection, field_def))
+                ast_and_defs[response_name].append((parent_type, selection, field_def))
 
             elif isinstance(selection, ast.InlineFragment):
                 type_condition = selection.type_condition

tests/core_validation/test_overlapping_fields_can_be_merged.py

@@ -1,5 +1,5 @@
 from graphql.core.language.location import SourceLocation as L
-from graphql.core.type.definition import GraphQLObjectType, GraphQLArgument, GraphQLNonNull, GraphQLUnionType, \
+from graphql.core.type.definition import GraphQLObjectType, GraphQLArgument, GraphQLNonNull, GraphQLInterfaceType, \
     GraphQLList, GraphQLField
 from graphql.core.type.scalars import GraphQLString, GraphQLInt, GraphQLID
 from graphql.core.type.schema import GraphQLSchema
@@ -79,6 +79,19 @@ def test_same_aliases_with_different_field_targets():
     ], sort_list=False)
 
 
+def test_same_aliases_allowed_on_nonoverlapping_fields():
+    expect_passes_rule(OverlappingFieldsCanBeMerged, '''
+    fragment sameAliasesWithDifferentFieldTargets on Pet {
+        ... on Dog {
+            name
+        }
+        ... on Cat {
+            name: nickname
+        }
+    }
+    ''')
+
+
 def test_alias_masking_direct_field_access():
     expect_fails_rule(OverlappingFieldsCanBeMerged, '''
     fragment aliasMaskingDirectFieldAccess on Dog {
@@ -90,6 +103,28 @@ def test_alias_masking_direct_field_access():
     ], sort_list=False)
 
 
+def test_diferent_args_second_adds_an_argument():
+    expect_fails_rule(OverlappingFieldsCanBeMerged, '''
+    fragment conflictingArgs on Dog {
+        doesKnowCommand
+        doesKnowCommand(dogCommand: HEEL)
+    }
+    ''', [
+        fields_conflict('doesKnowCommand', 'they have differing arguments', L(3, 9), L(4, 9))
+    ], sort_list=False)
+
+
+def test_diferent_args_second_missing_an_argument():
+    expect_fails_rule(OverlappingFieldsCanBeMerged, '''
+    fragment conflictingArgs on Dog {
+        doesKnowCommand(dogCommand: SIT)
+        doesKnowCommand
+    }
+    ''', [
+        fields_conflict('doesKnowCommand', 'they have differing arguments', L(3, 9), L(4, 9))
+    ], sort_list=False)
+
+
 def test_conflicting_args():
     expect_fails_rule(OverlappingFieldsCanBeMerged, '''
     fragment conflictingArgs on Dog {
@@ -101,6 +136,18 @@ def test_conflicting_args():
     ], sort_list=False)
 
 
+def test_allows_different_args_where_no_conflict_is_possible():
+    expect_passes_rule(OverlappingFieldsCanBeMerged, '''
+    fragment conflictingArgs on Pet {
+        ... on Dog {
+            name(surname: true)
+        }
+        ... on Cat {
+            name
+        }
+    }
+    ''')
+
 def test_conflicting_directives():
     expect_fails_rule(OverlappingFieldsCanBeMerged, '''
     fragment conflictingDirectiveArgs on Dog {
@@ -276,25 +323,37 @@ def test_reports_deep_conflict_to_nearest_common_ancestor():
     ], sort_list=False)
 
 
+SomeBox = GraphQLInterfaceType('SomeBox', {
+    'unrelatedField': GraphQLField(GraphQLString)
+}, resolve_type=lambda *_: StringBox)
+
 StringBox = GraphQLObjectType('StringBox', {
-    'scalar': GraphQLField(GraphQLString)
-})
+    'scalar': GraphQLField(GraphQLString),
+    'unrelatedField': GraphQLField(GraphQLString)
+}, interfaces=[SomeBox])
 
 IntBox = GraphQLObjectType('IntBox', {
-    'scalar': GraphQLField(GraphQLInt)
-})
+    'scalar': GraphQLField(GraphQLInt),
+    'unrelatedField': GraphQLField(GraphQLString)
+}, interfaces=[SomeBox])
 
-NonNullStringBox1 = GraphQLObjectType('NonNullStringBox1', {
+NonNullStringBox1 = GraphQLInterfaceType('NonNullStringBox1', {
     'scalar': GraphQLField(GraphQLNonNull(GraphQLString))
-})
+}, resolve_type=lambda *_: StringBox)
+
+NonNullStringBox1Impl = GraphQLObjectType('NonNullStringBox1Impl', {
+    'scalar': GraphQLField(GraphQLNonNull(GraphQLString)),
+    'unrelatedField': GraphQLField(GraphQLString)
+}, interfaces=[ SomeBox, NonNullStringBox1 ])
 
-NonNullStringBox2 = GraphQLObjectType('NonNullStringBox2', {
+NonNullStringBox2 = GraphQLInterfaceType('NonNullStringBox2', {
     'scalar': GraphQLField(GraphQLNonNull(GraphQLString))
-})
+}, resolve_type=lambda *_: StringBox)
 
-BoxUnion = GraphQLUnionType('BoxUnion', [
-    StringBox, IntBox, NonNullStringBox1, NonNullStringBox2
-], resolve_type=lambda *_: StringBox)
+NonNullStringBox2Impl = GraphQLObjectType('NonNullStringBox2Impl', {
+    'scalar': GraphQLField(GraphQLNonNull(GraphQLString)),
+    'unrelatedField': GraphQLField(GraphQLString)
+}, interfaces=[ SomeBox, NonNullStringBox2 ])
 
 Connection = GraphQLObjectType('Connection', {
     'edges': GraphQLField(GraphQLList(GraphQLObjectType('Edge', {
@@ -306,33 +365,48 @@ def test_reports_deep_conflict_to_nearest_common_ancestor():
 })
 
 schema = GraphQLSchema(GraphQLObjectType('QueryRoot', {
-    'boxUnion': GraphQLField(BoxUnion),
+    'someBox': GraphQLField(SomeBox),
     'connection': GraphQLField(Connection)
 }))
 
 
-def test_conflicting_scalar_return_types():
+def test_conflicting_return_types_which_potentially_overlap():
     expect_fails_rule_with_schema(schema, OverlappingFieldsCanBeMerged, '''
     {
-        boxUnion {
+        someBox {
             ...on IntBox {
                 scalar
             }
-            ...on StringBox {
+            ...on NonNullStringBox1 {
                 scalar
             }
         }
     }
 
     ''', [
-        fields_conflict('scalar', 'they return differing types Int and String', L(5, 17), L(8, 17))
+        fields_conflict('scalar', 'they return differing types Int and String!', L(5, 17), L(8, 17))
     ], sort_list=False)
 
 
+def test_allows_differing_return_types_which_cannot_overlap():
+    expect_passes_rule_with_schema(schema, OverlappingFieldsCanBeMerged, '''
+    {
+        someBox {
+            ...on IntBox {
+                scalar
+            }
+            ...on StringBox {
+                scalar
+            }
+        }
+    }
+    ''')
+
+
 def test_same_wrapped_scalar_return_types():
     expect_passes_rule_with_schema(schema, OverlappingFieldsCanBeMerged, '''
     {
-        boxUnion {
+        someBox {
             ...on NonNullStringBox1 {
               scalar
             }