Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler): fix validation for fragment spread of interface without implementers #896

Merged
merged 3 commits into from
Aug 26, 2024
Merged

fix(compiler): fix validation for fragment spread of interface without implementers #896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d804c9
fix(compiler): add failing test for spread of empty interface
goto-bus-stop Aug 20, 2024
a85df6f
fix(compiler): always accept spreading ...Ty when parent type is Ty
goto-bus-stop Aug 20, 2024
77a0b3d
Merge branch 'main' into renee/validate-query-with-no-interface-imple…
goto-bus-stop Aug 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/apollo-compiler/src/validation/fragment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 7 additions & 3 deletions crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Comment on lines +7 to 9
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change, just fixing the schema so it doesn't rely on having an interface with 0 implementers


type Query {
get: A
}

fragment recursive on A {
name
}

query {
get { ...recursive }
}
19 changes: 0 additions & 19 deletions crates/apollo-compiler/test_data/diagnostics/0091_recursive_interface_definition.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
────╯

16 changes: 16 additions & 0 deletions crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
type Query {
intf: Intf
}
interface Intf {
field: Int
}

query SelectDirectly {
intf { field }
}

query UsingInlineFragment {
intf {
... on Intf { field }
}
}
232 changes: 232 additions & 0 deletions crates/apollo-compiler/test_data/ok/0116_interface_without_implementations.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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: {},
}
8 changes: 6 additions & 2 deletions ...llo-compiler/test_data/serializer/diagnostics/0091_recursive_interface_definition.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions ...es/apollo-compiler/test_data/serializer/ok/0116_interface_without_implementations.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
type Query {
intf: Intf
}

interface Intf {
field: Int
}

query SelectDirectly {
intf {
field
}
}

query UsingInlineFragment {
intf {
... on Intf {
field
}
}
}