From f8a69b3a3b8a69a7d81ac71b67f178db5f2fafe5 Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Tue, 26 Aug 2025 11:53:10 -0500 Subject: [PATCH 1/6] feat(compiler): allow SDL to contain built-in type redefinition Updates `SchemaBuilder` to allow SDL to contain built-in meta types (e.g. `__Type`) redefinitions. GraphQL spec doesn't really define the expected behavior for handling SDL with redeclared meta fields (see: [#1036](https://github.com/graphql/graphql-spec/pull/1036), [#1049](https://github.com/graphql/graphql-spec/pull/1049)). `graphql-js` [reference implementation](https://github.com/graphql/graphql-js/blob/v16.11.0/src/utilities/extendSchema.ts#L187) simply ignores those redefinitions even if the underlying type definitions don't match. This PR updates `ignore_builtin_redefinitions` option to also ignore meta types redefinitions (previously it was only allowing built-in scalar redefinitions #990). --- crates/apollo-compiler/src/schema/from_ast.rs | 16 +++---- .../apollo-compiler/tests/validation/types.rs | 47 ++++++++++++++++++- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/crates/apollo-compiler/src/schema/from_ast.rs b/crates/apollo-compiler/src/schema/from_ast.rs index ec92a4ad..f7229d75 100644 --- a/crates/apollo-compiler/src/schema/from_ast.rs +++ b/crates/apollo-compiler/src/schema/from_ast.rs @@ -134,15 +134,15 @@ impl SchemaBuilder { } Entry::Occupied(entry) => { let previous = entry.get(); + if self.ignore_builtin_redefinitions && previous.is_built_in() { + continue; + } + if $is_scalar && previous.is_built_in() { - if self.ignore_builtin_redefinitions { - continue; - } else { - self.errors.push( - $def.location(), - BuildError::BuiltInScalarTypeRedefinition, - ) - } + self.errors.push( + $def.location(), + BuildError::BuiltInScalarTypeRedefinition, + ) } else { self.errors.push( $def.name.location(), diff --git a/crates/apollo-compiler/tests/validation/types.rs b/crates/apollo-compiler/tests/validation/types.rs index 7a23d06e..06308b8f 100644 --- a/crates/apollo-compiler/tests/validation/types.rs +++ b/crates/apollo-compiler/tests/validation/types.rs @@ -2020,7 +2020,7 @@ mod variable_default_values { } #[test] -fn handles_built_in_type_redefinition() { +fn handles_built_in_scalar_redefinition() { let schema = r#" scalar String @@ -2049,3 +2049,48 @@ type Query { .build() .expect("schema parsed successfully"); } + +#[test] +fn handles_built_in_type_redefinition() { + let schema = r#" +type __Directive { + name: String! + description: String! + isRepeatable: String! + args: __InputValue + locations: String! +} + +type Query { + foo: String +} +"#; + + let errors = Schema::parse_and_validate(schema, "schema.graphql") + .expect_err("invalid schema") + .errors; + let expected = expect![[r#" + Error: the type `__Directive` is defined multiple times in the schema + ╭─[ built_in.graphql:87:6 ] + │ + 87 │ type __Directive { + │ ─────┬───── + │ ╰─────── previous definition of `__Directive` here + │ + ├─[ schema.graphql:2:6 ] + │ + 2 │ type __Directive { + │ ─────┬───── + │ ╰─────── `__Directive` redefined here + │ + │ Help: remove or rename one of the definitions, or use `extend` + ────╯ + "#]]; + expected.assert_eq(&errors.to_string()); + + let builder = SchemaBuilder::new().ignore_builtin_redefinitions(); + let _ = builder + .parse(schema, "schema.graphql") + .build() + .expect("schema parsed successfully"); +} From c8efa94d293a9ee7ce045e388ff48ab2487e78c7 Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Tue, 26 Aug 2025 12:04:47 -0500 Subject: [PATCH 2/6] add changelog --- crates/apollo-compiler/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/apollo-compiler/CHANGELOG.md b/crates/apollo-compiler/CHANGELOG.md index dd2a695a..596003dd 100644 --- a/crates/apollo-compiler/CHANGELOG.md +++ b/crates/apollo-compiler/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## Features +- **Update `ignore_builtin_redefinitions` option to allow SDL to contain built-in meta type definitions - [dariuszkuc], [pull/994]** - **Adds `ignore_builtin_redefinitions` option to `SchemaBuilder` to allow SDL to contain built-in scalar definitions - [dariuszkuc], [pull/990]** From c1a0e84ecb514d981774195e0be238af0bfe1eda Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Tue, 26 Aug 2025 14:05:00 -0500 Subject: [PATCH 3/6] update changelog --- crates/apollo-compiler/CHANGELOG.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/apollo-compiler/CHANGELOG.md b/crates/apollo-compiler/CHANGELOG.md index 596003dd..cb11a401 100644 --- a/crates/apollo-compiler/CHANGELOG.md +++ b/crates/apollo-compiler/CHANGELOG.md @@ -21,15 +21,20 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## Features -- **Update `ignore_builtin_redefinitions` option to allow SDL to contain built-in meta type definitions - [dariuszkuc], [pull/994]** -- **Adds `ignore_builtin_redefinitions` option to `SchemaBuilder` to allow SDL to contain built-in - scalar definitions - [dariuszkuc], [pull/990]** +- **Adds `ignore_builtin_redefinitions` option to `SchemaBuilder` to allow SDL to contain built-in type + definitions - [dariuszkuc], [pull/990] and [pull/994]** ## Fixes -- **Fix handling of orphan root type extensions - [dariuszkuc], [pull/993]** -- **Fix possible stack overflow in validation of directive definition arguments with nested types - [dariuszkuc], [pull/987]** -- **Fix `iter_origin()` to be a pub method- [duckki], [pull/989]** +- **Fix handling of orphan root type extensions - [dariuszkuc], [pull/993](#993)** +- **Fix possible stack overflow in validation of directive definition arguments with nested types - [dariuszkuc], [pull/987](#987)** +- **Fix `iter_origin()` to be a pub method- [duckki], [pull/989](#989)** + +[pull/994]: https://github.com/apollographql/apollo-rs/pull/994 +[pull/993]: https://github.com/apollographql/apollo-rs/pull/993 +[pull/990]: https://github.com/apollographql/apollo-rs/pull/990 +[pull/989]: https://github.com/apollographql/apollo-rs/pull/989 +[pull/987]: https://github.com/apollographql/apollo-rs/pull/987 # [1.29.0](https://crates.io/crates/apollo-compiler/1.29.0) - 2025-08-08 From 19cf1f502447e93849fc34cd0a6c507c1763a640 Mon Sep 17 00:00:00 2001 From: Iryna Shestak Date: Wed, 27 Aug 2025 13:32:28 +0200 Subject: [PATCH 4/6] expand built_in_redefinition tests * moves tests to their own file * adds all scalars and types to the test --- .../validation/ignore_builtin_redefinition.rs | 256 ++++++++++++++++++ .../apollo-compiler/tests/validation/mod.rs | 1 + .../apollo-compiler/tests/validation/types.rs | 78 ------ 3 files changed, 257 insertions(+), 78 deletions(-) create mode 100644 crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs diff --git a/crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs b/crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs new file mode 100644 index 00000000..17a31c87 --- /dev/null +++ b/crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs @@ -0,0 +1,256 @@ +use apollo_compiler::schema::SchemaBuilder; +use apollo_compiler::Schema; +use expect_test::expect; + +#[test] +fn handles_built_in_scalar_redefinition() { + let schema = r#" + scalar String + scalar Int + scalar Float + scalar Boolean + scalar ID + + type Query { + foo: String + } + "#; + + let errors = Schema::parse_and_validate(schema, "schema.graphql") + .expect_err("invalid schema") + .errors; + let expected = expect![[r#" + Error: built-in scalar definitions must be omitted + ╭─[ schema.graphql:2:3 ] + │ + 2 │ scalar String + │ ──────┬────── + │ ╰──────── remove this scalar definition + ───╯ + Error: built-in scalar definitions must be omitted + ╭─[ schema.graphql:3:3 ] + │ + 3 │ scalar Int + │ ─────┬──── + │ ╰────── remove this scalar definition + ───╯ + Error: built-in scalar definitions must be omitted + ╭─[ schema.graphql:4:3 ] + │ + 4 │ scalar Float + │ ──────┬───── + │ ╰─────── remove this scalar definition + ───╯ + Error: built-in scalar definitions must be omitted + ╭─[ schema.graphql:5:3 ] + │ + 5 │ scalar Boolean + │ ───────┬────── + │ ╰──────── remove this scalar definition + ───╯ + Error: built-in scalar definitions must be omitted + ╭─[ schema.graphql:6:3 ] + │ + 6 │ scalar ID + │ ────┬──── + │ ╰────── remove this scalar definition + ───╯ + "#]]; + expected.assert_eq(&errors.to_string()); + + let builder = SchemaBuilder::new().ignore_builtin_redefinitions(); + let _ = builder + .parse(schema, "schema.graphql") + .build() + .expect("schema parsed successfully"); +} + +#[test] +fn handles_built_in_type_redefinition() { + let schema = r#" + type __Directive { + name: String! + description: String! + isRepeatable: String! + args: __InputValue + locations: String! + } + + type __Schema { + description: String + types: [__Type!] + queryType: __Type! + mutationType: __Type + subscriptionType: __Type + directives: [__Directive!] + } + + type __Type { + kind: __TypeKind! + name: String + description: String + fields: [__Field!] + interfaces: [__Type!] + possibleTypes: [__Type!] + enumValues: [__EnumValue!] + inputFields: [__InputValue!] + ofType: __Type + specifiedByURL: String + } + + enum __TypeKind { + SCALAR + OBJECT + INTERFACE + UNION + ENUM + INPUT_OBJECT + LIST + } + + type __Field { + name: String! + description: String + args: [__InputValue!]! + type: __Type! + isDeprecated: Boolean! + deprecationReason: String + } + + type __InputValue { + name: String! + description: String + type: __Type! + defaultValue: String + deprecationReason: String + } + + type __EnumValue { + name: String! + description: String + deprecationReason: String + } + + type Query { + foo: String + } + "#; + + let errors = Schema::parse_and_validate(schema, "schema.graphql") + .expect_err("invalid schema") + .errors; + let expected = expect![[r#" + Error: the type `__Directive` is defined multiple times in the schema + ╭─[ built_in.graphql:87:6 ] + │ + 87 │ type __Directive { + │ ─────┬───── + │ ╰─────── previous definition of `__Directive` here + │ + ├─[ schema.graphql:2:11 ] + │ + 2 │ type __Directive { + │ ─────┬───── + │ ╰─────── `__Directive` redefined here + │ + │ Help: remove or rename one of the definitions, or use `extend` + ────╯ + Error: the type `__Schema` is defined multiple times in the schema + ╭─[ built_in.graphql:2:6 ] + │ + 2 │ type __Schema { + │ ────┬─── + │ ╰───── previous definition of `__Schema` here + │ + ├─[ schema.graphql:10:11 ] + │ + 10 │ type __Schema { + │ ────┬─── + │ ╰───── `__Schema` redefined here + │ + │ Help: remove or rename one of the definitions, or use `extend` + ────╯ + Error: the type `__Type` is defined multiple times in the schema + ╭─[ built_in.graphql:17:6 ] + │ + 17 │ type __Type { + │ ───┬── + │ ╰──── previous definition of `__Type` here + │ + ├─[ schema.graphql:19:11 ] + │ + 19 │ type __Type { + │ ───┬── + │ ╰──── `__Type` redefined here + │ + │ Help: remove or rename one of the definitions, or use `extend` + ────╯ + Error: the type `__TypeKind` is defined multiple times in the schema + ╭─[ built_in.graphql:38:6 ] + │ + 38 │ enum __TypeKind { + │ ─────┬──── + │ ╰────── previous definition of `__TypeKind` here + │ + ├─[ schema.graphql:32:11 ] + │ + 32 │ enum __TypeKind { + │ ─────┬──── + │ ╰────── `__TypeKind` redefined here + │ + │ Help: remove or rename one of the definitions, or use `extend` + ────╯ + Error: the type `__Field` is defined multiple times in the schema + ╭─[ built_in.graphql:58:6 ] + │ + 58 │ type __Field { + │ ───┬─── + │ ╰───── previous definition of `__Field` here + │ + ├─[ schema.graphql:42:11 ] + │ + 42 │ type __Field { + │ ───┬─── + │ ╰───── `__Field` redefined here + │ + │ Help: remove or rename one of the definitions, or use `extend` + ────╯ + Error: the type `__InputValue` is defined multiple times in the schema + ╭─[ built_in.graphql:68:6 ] + │ + 68 │ type __InputValue { + │ ──────┬───── + │ ╰─────── previous definition of `__InputValue` here + │ + ├─[ schema.graphql:51:11 ] + │ + 51 │ type __InputValue { + │ ──────┬───── + │ ╰─────── `__InputValue` redefined here + │ + │ Help: remove or rename one of the definitions, or use `extend` + ────╯ + Error: the type `__EnumValue` is defined multiple times in the schema + ╭─[ built_in.graphql:79:6 ] + │ + 79 │ type __EnumValue { + │ ─────┬───── + │ ╰─────── previous definition of `__EnumValue` here + │ + ├─[ schema.graphql:59:11 ] + │ + 59 │ type __EnumValue { + │ ─────┬───── + │ ╰─────── `__EnumValue` redefined here + │ + │ Help: remove or rename one of the definitions, or use `extend` + ────╯ + "#]]; + expected.assert_eq(&errors.to_string()); + + let builder = SchemaBuilder::new().ignore_builtin_redefinitions(); + let _ = builder + .parse(schema, "schema.graphql") + .build() + .expect("schema parsed successfully"); +} diff --git a/crates/apollo-compiler/tests/validation/mod.rs b/crates/apollo-compiler/tests/validation/mod.rs index b9138d6f..7e733bd6 100644 --- a/crates/apollo-compiler/tests/validation/mod.rs +++ b/crates/apollo-compiler/tests/validation/mod.rs @@ -1,4 +1,5 @@ mod field_merging; +mod ignore_builtin_redefinition; mod interface; mod object; mod operation; diff --git a/crates/apollo-compiler/tests/validation/types.rs b/crates/apollo-compiler/tests/validation/types.rs index 06308b8f..3eb6ee6d 100644 --- a/crates/apollo-compiler/tests/validation/types.rs +++ b/crates/apollo-compiler/tests/validation/types.rs @@ -1,10 +1,8 @@ //! Ported from graphql-js, 2023-11-16 //! https://github.com/graphql/graphql-js/blob/0b7590f0a2b65e6210da2e49be0d8e6c27781af2/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts -use apollo_compiler::schema::SchemaBuilder; use apollo_compiler::validation::Valid; use apollo_compiler::ExecutableDocument; use apollo_compiler::Schema; -use expect_test::expect; use expect_test::Expect; use std::sync::OnceLock; use unindent::unindent; @@ -2018,79 +2016,3 @@ mod variable_default_values { ); } } - -#[test] -fn handles_built_in_scalar_redefinition() { - let schema = r#" -scalar String - -type Query { - foo: String -} -"#; - - let errors = Schema::parse_and_validate(schema, "schema.graphql") - .expect_err("invalid schema") - .errors; - let expected = expect![[r#" - Error: built-in scalar definitions must be omitted - ╭─[ schema.graphql:2:1 ] - │ - 2 │ scalar String - │ ──────┬────── - │ ╰──────── remove this scalar definition - ───╯ - "#]]; - expected.assert_eq(&errors.to_string()); - - let builder = SchemaBuilder::new().ignore_builtin_redefinitions(); - let _ = builder - .parse(schema, "schema.graphql") - .build() - .expect("schema parsed successfully"); -} - -#[test] -fn handles_built_in_type_redefinition() { - let schema = r#" -type __Directive { - name: String! - description: String! - isRepeatable: String! - args: __InputValue - locations: String! -} - -type Query { - foo: String -} -"#; - - let errors = Schema::parse_and_validate(schema, "schema.graphql") - .expect_err("invalid schema") - .errors; - let expected = expect![[r#" - Error: the type `__Directive` is defined multiple times in the schema - ╭─[ built_in.graphql:87:6 ] - │ - 87 │ type __Directive { - │ ─────┬───── - │ ╰─────── previous definition of `__Directive` here - │ - ├─[ schema.graphql:2:6 ] - │ - 2 │ type __Directive { - │ ─────┬───── - │ ╰─────── `__Directive` redefined here - │ - │ Help: remove or rename one of the definitions, or use `extend` - ────╯ - "#]]; - expected.assert_eq(&errors.to_string()); - - let builder = SchemaBuilder::new().ignore_builtin_redefinitions(); - let _ = builder - .parse(schema, "schema.graphql") - .build() - .expect("schema parsed successfully"); -} From 88c2f057d7f2e5909f0b1ae38f0ee1fe2da92504 Mon Sep 17 00:00:00 2001 From: Iryna Shestak Date: Wed, 27 Aug 2025 13:33:56 +0200 Subject: [PATCH 5/6] fix doc comment for ignore_builtin_redefinitions --- crates/apollo-compiler/src/schema/from_ast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/apollo-compiler/src/schema/from_ast.rs b/crates/apollo-compiler/src/schema/from_ast.rs index f7229d75..cb044dee 100644 --- a/crates/apollo-compiler/src/schema/from_ast.rs +++ b/crates/apollo-compiler/src/schema/from_ast.rs @@ -78,7 +78,7 @@ impl SchemaBuilder { self } - /// Configure the builder to allow SDL to contain scalar built-in types re-definitions. + /// Configure the builder to allow SDL to contain built-in type re-definitions. /// Re-definitions are going to be effectively ignored and compiler will continue to use /// built-in GraphQL spec definitions. pub fn ignore_builtin_redefinitions(mut self) -> Self { From 0cb5cc97a9dc4dd4aab7a57bd60fe21aba7cad4d Mon Sep 17 00:00:00 2001 From: Iryna Shestak Date: Wed, 27 Aug 2025 13:42:52 +0200 Subject: [PATCH 6/6] change expect_err message --- .../tests/validation/ignore_builtin_redefinition.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs b/crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs index 17a31c87..52238a00 100644 --- a/crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs +++ b/crates/apollo-compiler/tests/validation/ignore_builtin_redefinition.rs @@ -17,7 +17,7 @@ fn handles_built_in_scalar_redefinition() { "#; let errors = Schema::parse_and_validate(schema, "schema.graphql") - .expect_err("invalid schema") + .expect_err("valid schema") .errors; let expected = expect![[r#" Error: built-in scalar definitions must be omitted @@ -137,7 +137,7 @@ fn handles_built_in_type_redefinition() { "#; let errors = Schema::parse_and_validate(schema, "schema.graphql") - .expect_err("invalid schema") + .expect_err("valid schema") .errors; let expected = expect![[r#" Error: the type `__Directive` is defined multiple times in the schema