Repository: apollographql/apollo-rs
Pull request #896 (2024-08-20 10:25:37)

fix(compiler): fix validation for fragment spread of interface without implementers
This `Intf` has no implementations. As written in the spec, doing a `... on Intf` fragment spread should never work, as the set of possible types is empty and can never intersect with the parent type. However, implementations like graphql-js and graphql-go have an early check, accepting the fragment if the type condition is equal to the parent type. This tests reproduces that.

We may want to align with graphql-js and graphql-go rather than the spec here for compatibility? Though it's not something that's likely to happen in the real world.

Ref https://github.com/graphql/graphql-spec/issues/1109

<!-- ROUTER-649 -->

duckki: I agree that Apollo's validator is spec-compliant currently, while others are not :)

At the same time, this could be an oversight in the spec. It's good to be able to test queries before writing an implementation (despite the weirdness of conditioning on the parent type).

Patch:
diff --git a/crates/apollo-compiler/src/validation/fragment.rs b/crates/apollo-compiler/src/validation/fragment.rs
index 92f0f78..b3c275d 100644
--- a/crates/apollo-compiler/src/validation/fragment.rs
+++ b/crates/apollo-compiler/src/validation/fragment.rs
@@ -57,6 +57,13 @@ fn validate_fragment_spread_type(
     selection: &executable::Selection,
     context: OperationValidationContext<'_>,
 ) {
+    // Treat a spread that's just literally on the parent type as always valid:
+    // by spec text, it shouldn't be, but graphql-{js,java,go} and others all do this.
+    // See https://github.com/graphql/graphql-spec/issues/1109
+    if type_condition == against_type {
+        return;
+    }
+
     // Another diagnostic will be raised if the type condition was wrong.
     // We reduce noise by silencing other issues with the fragment.
     let Some(type_condition_definition) = schema.types.get(type_condition) else {


Test patch:
diff --git a/crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.graphql b/crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.graphql
index 602f7bf..4255062 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.graphql
@@ -4,13 +4,17 @@ interface A implements B {
 interface B implements A {
   name: String
 }
-fragment recursive on A {
-  name
+type Impl implements A & B {
+  name: String
 }
-
 type Query {
   get: A
 }
+
+fragment recursive on A {
+  name
+}
+
 query {
   get { ...recursive }
 }
diff --git a/crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.txt b/crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.txt
index 04de99e..125724d 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.txt
@@ -23,23 +23,4 @@ Error: interface `B` declares that it implements `A`, but to do so it must also
    │ │       
    │ ╰─────── B must also be implemented here
 ───╯
-Error: fragment `recursive` with type condition `A` cannot be applied to `A`
-    ╭─[0091_recursive_interface_definition.graphql:15:9]
-    │
-  1 │ ╭───▶ interface A implements B {
-    ┆ ┆     
-  3 │ ├───▶ }
-    │ │         
-    │ ╰───────── type condition `A` is not assignable to this type
-    │ 
-  7 │   ╭─▶ fragment recursive on A {
-    ┆   ┆   
-  9 │   ├─▶ }
-    │   │       
-    │   ╰─────── fragment declared with type condition `A` here
-    │ 
- 15 │         get { ...recursive }
-    │               ──────┬─────  
-    │                     ╰─────── fragment `recursive` cannot be applied
-────╯
 
diff --git a/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.graphql b/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.graphql
new file mode 100644
index 0000000..dd18d37
--- /dev/null
+++ b/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.graphql
@@ -0,0 +1,16 @@
+type Query {
+  intf: Intf
+}
+interface Intf {
+  field: Int
+}
+
+query SelectDirectly {
+  intf { field }
+}
+
+query UsingInlineFragment {
+  intf {
+    ... on Intf { field }
+  }
+}
diff --git a/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.txt b/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.txt
new file mode 100644
index 0000000..760f6ee
--- /dev/null
+++ b/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.txt
@@ -0,0 +1,232 @@
+Schema {
+    sources: {
+        1: SourceFile {
+            path: "built_in.graphql",
+            source_text: include_str!("built_in.graphql"),
+        },
+        45: SourceFile {
+            path: "0116_interface_without_implementations.graphql",
+            source_text: "type Query {\n  intf: Intf\n}\ninterface Intf {\n  field: Int\n}\n\nquery SelectDirectly {\n  intf { field }\n}\n\nquery UsingInlineFragment {\n  intf {\n    ... on Intf { field }\n  }\n}\n",
+        },
+    },
+    schema_definition: SchemaDefinition {
+        description: None,
+        directives: [],
+        query: Some(
+            ComponentName {
+                origin: Definition,
+                name: "Query",
+            },
+        ),
+        mutation: None,
+        subscription: None,
+    },
+    directive_definitions: {
+        "skip": built_in_directive!("skip"),
+        "include": built_in_directive!("include"),
+        "deprecated": built_in_directive!("deprecated"),
+        "specifiedBy": built_in_directive!("specifiedBy"),
+    },
+    types: {
+        "__Schema": built_in_type!("__Schema"),
+        "__Type": built_in_type!("__Type"),
+        "__TypeKind": built_in_type!("__TypeKind"),
+        "__Field": built_in_type!("__Field"),
+        "__InputValue": built_in_type!("__InputValue"),
+        "__EnumValue": built_in_type!("__EnumValue"),
+        "__Directive": built_in_type!("__Directive"),
+        "__DirectiveLocation": built_in_type!("__DirectiveLocation"),
+        "Int": built_in_type!("Int"),
+        "Float": built_in_type!("Float"),
+        "String": built_in_type!("String"),
+        "Boolean": built_in_type!("Boolean"),
+        "ID": built_in_type!("ID"),
+        "Query": Object(
+            0..27 @45 ObjectType {
+                description: None,
+                name: "Query",
+                implements_interfaces: {},
+                directives: [],
+                fields: {
+                    "intf": Component {
+                        origin: Definition,
+                        node: 15..25 @45 FieldDefinition {
+                            description: None,
+                            name: "intf",
+                            arguments: [],
+                            ty: Named(
+                                "Intf",
+                            ),
+                            directives: [],
+                        },
+                    },
+                },
+            },
+        ),
+        "Intf": Interface(
+            28..59 @45 InterfaceType {
+                description: None,
+                name: "Intf",
+                implements_interfaces: {},
+                directives: [],
+                fields: {
+                    "field": Component {
+                        origin: Definition,
+                        node: 47..57 @45 FieldDefinition {
+                            description: None,
+                            name: "field",
+                            arguments: [],
+                            ty: Named(
+                                "Int",
+                            ),
+                            directives: [],
+                        },
+                    },
+                },
+            },
+        ),
+    },
+}
+ExecutableDocument {
+    sources: {
+        1: SourceFile {
+            path: "built_in.graphql",
+            source_text: include_str!("built_in.graphql"),
+        },
+        45: SourceFile {
+            path: "0116_interface_without_implementations.graphql",
+            source_text: "type Query {\n  intf: Intf\n}\ninterface Intf {\n  field: Int\n}\n\nquery SelectDirectly {\n  intf { field }\n}\n\nquery UsingInlineFragment {\n  intf {\n    ... on Intf { field }\n  }\n}\n",
+        },
+    },
+    operations: OperationMap {
+        anonymous: None,
+        named: {
+            "SelectDirectly": 61..102 @45 Operation {
+                operation_type: Query,
+                name: Some(
+                    "SelectDirectly",
+                ),
+                variables: [],
+                directives: [],
+                selection_set: SelectionSet {
+                    ty: "Query",
+                    selections: [
+                        Field(
+                            86..100 @45 Field {
+                                definition: 15..25 @45 FieldDefinition {
+                                    description: None,
+                                    name: "intf",
+                                    arguments: [],
+                                    ty: Named(
+                                        "Intf",
+                                    ),
+                                    directives: [],
+                                },
+                                alias: None,
+                                name: "intf",
+                                arguments: [],
+                                directives: [],
+                                selection_set: SelectionSet {
+                                    ty: "Intf",
+                                    selections: [
+                                        Field(
+                                            93..98 @45 Field {
+                                                definition: 47..57 @45 FieldDefinition {
+                                                    description: None,
+                                                    name: "field",
+                                                    arguments: [],
+                                                    ty: Named(
+                                                        "Int",
+                                                    ),
+                                                    directives: [],
+                                                },
+                                                alias: None,
+                                                name: "field",
+                                                arguments: [],
+                                                directives: [],
+                                                selection_set: SelectionSet {
+                                                    ty: "Int",
+                                                    selections: [],
+                                                },
+                                            },
+                                        ),
+                                    ],
+                                },
+                            },
+                        ),
+                    ],
+                },
+            },
+            "UsingInlineFragment": 104..172 @45 Operation {
+                operation_type: Query,
+                name: Some(
+                    "UsingInlineFragment",
+                ),
+                variables: [],
+                directives: [],
+                selection_set: SelectionSet {
+                    ty: "Query",
+                    selections: [
+                        Field(
+                            134..170 @45 Field {
+                                definition: 15..25 @45 FieldDefinition {
+                                    description: None,
+                                    name: "intf",
+                                    arguments: [],
+                                    ty: Named(
+                                        "Intf",
+                                    ),
+                                    directives: [],
+                                },
+                                alias: None,
+                                name: "intf",
+                                arguments: [],
+                                directives: [],
+                                selection_set: SelectionSet {
+                                    ty: "Intf",
+                                    selections: [
+                                        InlineFragment(
+                                            145..166 @45 InlineFragment {
+                                                type_condition: Some(
+                                                    "Intf",
+                                                ),
+                                                directives: [],
+                                                selection_set: SelectionSet {
+                                                    ty: "Intf",
+                                                    selections: [
+                                                        Field(
+                                                            159..164 @45 Field {
+                                                                definition: 47..57 @45 FieldDefinition {
+                                                                    description: None,
+                                                                    name: "field",
+                                                                    arguments: [],
+                                                                    ty: Named(
+                                                                        "Int",
+                                                                    ),
+                                                                    directives: [],
+                                                                },
+                                                                alias: None,
+                                                                name: "field",
+                                                                arguments: [],
+                                                                directives: [],
+                                                                selection_set: SelectionSet {
+                                                                    ty: "Int",
+                                                                    selections: [],
+                                                                },
+                                                            },
+                                                        ),
+                                                    ],
+                                                },
+                                            },
+                                        ),
+                                    ],
+                                },
+                            },
+                        ),
+                    ],
+                },
+            },
+        },
+    },
+    fragments: {},
+}
diff --git a/crates/apollo-compiler/test_data/serializer/diagnostics/0091_recursive_interface_definition.graphql b/crates/apollo-compiler/test_data/serializer/diagnostics/0091_recursive_interface_definition.graphql
index bed952e..2e417d9 100644
--- a/crates/apollo-compiler/test_data/serializer/diagnostics/0091_recursive_interface_definition.graphql
+++ b/crates/apollo-compiler/test_data/serializer/diagnostics/0091_recursive_interface_definition.graphql
@@ -6,14 +6,18 @@ interface B implements A {
   name: String
 }
 
-fragment recursive on A {
-  name
+type Impl implements A & B {
+  name: String
 }
 
 type Query {
   get: A
 }
 
+fragment recursive on A {
+  name
+}
+
 query {
   get {
     ...recursive
diff --git a/crates/apollo-compiler/test_data/serializer/ok/0116_interface_without_implementations.graphql b/crates/apollo-compiler/test_data/serializer/ok/0116_interface_without_implementations.graphql
new file mode 100644
index 0000000..55eaa4d
--- /dev/null
+++ b/crates/apollo-compiler/test_data/serializer/ok/0116_interface_without_implementations.graphql
@@ -0,0 +1,21 @@
+type Query {
+  intf: Intf
+}
+
+interface Intf {
+  field: Int
+}
+
+query SelectDirectly {
+  intf {
+    field
+  }
+}
+
+query UsingInlineFragment {
+  intf {
+    ... on Intf {
+      field
+    }
+  }
+}