From 5d804c9d9ebf5b9bc965a58059fed6c6ea98f7b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Tue, 20 Aug 2024 12:22:42 +0200 Subject: [PATCH 1/2] fix(compiler): add failing test for spread of empty interface `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 --- ..._interface_without_implementations.graphql | 16 ++++++++++++++ ..._interface_without_implementations.graphql | 21 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.graphql create mode 100644 crates/apollo-compiler/test_data/serializer/ok/0116_interface_without_implementations.graphql 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 000000000..982ca230c --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.graphql @@ -0,0 +1,16 @@ +type Query { + intf: Intf +} +interface Intf { + nothing: Int +} + +query SelectDirectly { + intf { nothing } +} + +query UsingInlineFragment { + intf { + ... on Intf { nothing } + } +} 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 000000000..15fcc0fac --- /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 { + nothing: Int +} + +query SelectDirectly { + intf { + nothing + } +} + +query UsingInlineFragment { + intf { + ... on Intf { + nothing + } + } +} From a85df6f91ce7e9d9154724b57365c8dd61325977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Tue, 20 Aug 2024 14:09:41 +0200 Subject: [PATCH 2/2] fix(compiler): always accept spreading ...Ty when parent type is Ty --- .../src/validation/fragment.rs | 7 + ...091_recursive_interface_definition.graphql | 10 +- .../0091_recursive_interface_definition.txt | 19 -- ..._interface_without_implementations.graphql | 6 +- ...0116_interface_without_implementations.txt | 232 ++++++++++++++++++ ...091_recursive_interface_definition.graphql | 8 +- ..._interface_without_implementations.graphql | 6 +- 7 files changed, 258 insertions(+), 30 deletions(-) create mode 100644 crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.txt diff --git a/crates/apollo-compiler/src/validation/fragment.rs b/crates/apollo-compiler/src/validation/fragment.rs index 92f0f7805..b3c275d2e 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 { 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 602f7bf85..425506204 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 04de99efb..125724d67 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 index 982ca230c..dd18d373b 100644 --- a/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.graphql +++ b/crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.graphql @@ -2,15 +2,15 @@ type Query { intf: Intf } interface Intf { - nothing: Int + field: Int } query SelectDirectly { - intf { nothing } + intf { field } } query UsingInlineFragment { intf { - ... on Intf { nothing } + ... 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 000000000..760f6ee24 --- /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 bed952e84..2e417d952 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 index 15fcc0fac..55eaa4d41 100644 --- 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 @@ -3,19 +3,19 @@ type Query { } interface Intf { - nothing: Int + field: Int } query SelectDirectly { intf { - nothing + field } } query UsingInlineFragment { intf { ... on Intf { - nothing + field } } }