feat(compiler): implement @oneOf input objects by abernix · Pull Request #1030 · apollographql/apollo-rs

abernix · 2026-03-27T15:05:33Z

Summary

Implements the @oneOf input objects directive, which became part of the stable GraphQL spec in September 2025. @oneOf lets a schema author say "exactly one of these fields must be provided" — it's a discriminated-union pattern that the type system enforces rather than leaving to resolver logic.

Closes #882.

How it works — three validation layers

The interesting design question is when each constraint should fire. We ended up with three complementary layers:

Schema validation (validation/input_object.rs) — when a developer writes a @oneOf type, every field must already be nullable and carry no default. These aren't runtime concerns; they're structural invariants the schema has to uphold before any query is written.
Static document validation (validation/value.rs) — when a query is compiled, a variable used to satisfy a @oneOf field must itself be declared non-null. A nullable variable is unsafe because it could legally be omitted at runtime, which would make the field-count invariant impossible to enforce without executing the query. This is the rule that most other implementations miss or leave to runtime. We validated against Apollo Kotlin, Hot Chocolate, Strawberry, graphql-java, and juniper — our static check is stricter than juniper and graphql-java, but strictly correct per spec.
Runtime coercion (request/coerce_variable_values) — when variables arrive at execution time, exactly one field must be provided and it must be non-null. This is the canonical spec rule and the last line of defense for literal values or dynamic inputs the static analysis can't see.

What's new in the public API

directive @oneOf on INPUT_OBJECT is now a built-in.
InputObjectType::is_one_of() -> bool — clean check without digging into directives.
__Type.isOneOf: Boolean! in the introspection schema — returns true for @oneOf types, null for everything else (matching the spec's introspection definition).
Five new diagnostic variants covering both schema- and document-level violations.

Fuzz coverage

Two complementary fuzz targets:

one_of — structure-aware, using apollo-smith. We extended smith so it generates @oneOf input types naturally (with all invariants pre-satisfied at generation time), which means the fuzzer spends its budget on schema shape diversity rather than discarding malformed SDL. Asserts that is_one_of() always agrees with directive presence, and all field invariants hold.
one_of_exec — the schema-invariant target can't reach document-level rules, because valid schema SDL and valid executable SDL are different constraint domains. This second target fixes a rich @oneOf schema and fuzzes only the executable document. It confirmed coverage of all five @oneOf-specific code paths in both validation/value.rs and coerce_variable_values.

Version

apollo-compiler bumps to 1.32.0 — minor because is_one_of() and the introspection field are new public API. apollo-smith bumps to 0.15.3 — 0.x minor-y/patch-y because Ty::as_nullable() is new public API.

Notes

The production federation path (Rover + JS composition) preserves @oneOf through the supergraph correctly. Rust in-process composition currently strips all directives from input types during merge — that's a separate pre-existing gap and is tracked as a follow-up, not a blocker for this PR.

Full implementation of the `@oneOf` RFC per GraphQL spec §3.10.1. Schema: - Add `directive `@oneOf` on INPUT_OBJECT` as a built-in - Add `isOneOf: Boolean!` to `__Type` introspection - Validate: `@oneOf` fields must be nullable, must not have default values - InputObjectType::is_one_of() convenience method Executable documents: - Validate: exactly one non-null field must be provided as a literal value - Validate: a variable used in a `@oneOf` field position must be non-null type - Runtime coerce_variable_values and coerce_argument_values enforce the same "exactly one non-null field" invariant at request time Diagnostics (with graphql-js unstable_compat_message parity): - OneOfInputObjectFieldNonNull - OneOfInputObjectFieldHasDefault - OneOfInputObjectWrongNumberOfFields - OneOfInputObjectNullField - OneOfInputObjectNullableVariable Tests: - 15 validation tests in tests/validation/one_of.rs covering all rules, graphql-js test parity, and actual introspection response assertions - 4 runtime coercion unit tests in resolvers/input_coercion.rs Fuzz: - fuzz/fuzz_targets/one_of.rs — semantic-invariant target covering both schema invariants and the executable coercion path (value.rs coverage lifted from 0% to 67%) - fuzz/corpus/one_of/ seed corpus (7 seeds) for rapid convergence

cargo fuzz coverage writes generated profdata and llvm binaries to fuzz/coverage/ — same category as /corpus and /artifacts, not something to check in. In the next commit, we add a second fuzz target that exercises the `@oneOf` executable-document path directly — coverage output will be generated and inspected there.

The schema-invariant fuzz target (one_of.rs) cannot reach the document-level `@oneOf` rules in validation/value.rs because a single text input rarely satisfies both schema and document validity simultaneously. This target fixes a rich `@oneOf` schema covering query, mutation, subscription, list, and nested positions, and fuzzes only the executable document string — directly exercising: - validation/value.rs (0% → 70% fuzz coverage, 100% function coverage) - All three `@oneOf` diagnostic paths confirmed hit by the fuzzer: OneOfInputObjectWrongNumberOfFields 1,660 hits OneOfInputObjectNullField 19 hits OneOfInputObjectNullableVariable 11 hits - resolvers/input_coercion.rs coercion path The 30% uncovered regions in value.rs are all pre-existing non-`@oneOf` value validation paths (Float/Boolean/Enum arguments, RequiredField) that require a different schema to exercise and are out of scope for `@oneOf` testing.

Fills in the CHANGELOG entry with the release date (2026-03-25) and bumps the version to 1.32.0 — the `@oneOf` feature warrants a minor bump because it adds public API (InputObjectType::is_one_of, new diagnostics, new __Type.isOneOf introspection field). Also adds five schema-extension tests that were identified as a gap in the original `@oneOf` implementation: - extending_oneof_type_with_nullable_field_is_valid - extending_oneof_type_with_nonnull_field_is_invalid - extending_oneof_type_with_default_value_is_invalid - adding_oneof_via_extension_with_valid_base_type_is_valid - adding_oneof_via_extension_with_nonnull_field_in_base_is_invalid

…neration apollo-smith - Add Ty::as_nullable() — strips the outermost NonNull wrapper. - DocumentBuilder::input_object_type_definition gains a ~1-in-5 chance of applying `@oneOf`. When it does, every field is forced nullable and stripped of default values, satisfying all spec invariants before the directive is inserted. - Update snapshot for the deterministic seed (three input types now carry `@oneOf`). - Bump version to 0.15.3. fuzz/one_of - Completely rewritten from a plain-text target to a structure-aware target using generate_valid_document. The fuzzer now spends its budget exploring interesting schema shapes rather than discarding malformed SDL, giving far better coverage of the `@oneOf` validation logic. Three invariants are asserted on every valid document: is_one_of() agrees with directive presence, all fields are nullable, and no field carries a default. fuzz/one_of_exec - Add env_logger::try_init() + debug!("{data}") for parity with every other fuzz target in the suite.

clippy::needless_borrow fired on `&schema` at 9 call sites in tests/validation/one_of.rs — the compiler auto-derefs, so the explicit borrow is redundant.

martinbonnin

One comment from graphql/graphql-spec#1211 but looks good otherwise!

Nice test suite!

martinbonnin · 2026-04-01T16:26:45Z

+                );
+            }
+        }
+    }


This will need to implement graphql/graphql-spec#1211 for schemas like this:

type Query { foo(arg: A): Int } input A @oneOf { # oh no, we can't create an instance of A a: A }

You're probably following this better than I am — do you think that's something we should do now? Or ... later? Is there any risk of it not landing?

graphql-js is still missing it. You're safe to add it down the road.

I'd merge this as is and we can revisit later.

It's on the agenda for the May wg.

lrlna · 2026-04-28T11:50:11Z

+        let has_directive = input_obj.directives.get("oneOf").is_some();
+        let is_one_of = input_obj.is_one_of();
+
+        // Invariant: is_one_of() must agree with directive presence.
+        assert_eq!(
+            has_directive, is_one_of,
+            "is_one_of() disagrees with @oneOf directive presence for type `{type_name}`"
+        );


This is a bit of a strange thing to check for: input_obj.is_one_of() on line 39 literlally does the same thing as the line above it. So you are asserting that input_obj.directives.get("oneOf").is_some() is equal to input_obj.directives.get("oneOf").is_some(). This would work if you are going getting the directive in a different way, by walking the entire tree for example, but not for as is.

Comparing a value to itself is certainly genius. 🙂 Let remove that.

I think the remaining invariant checks (all fields nullable, no defaults) are the ones actually exercising the schema-validation rule, and they'll stay.

lrlna · 2026-04-28T12:18:05Z

+fuzz_target!(|data: &str| {
+    let _ = env_logger::try_init();
+    debug!("{data}");
+
+    let schema = schema();
+
+    // Parse and validate the fuzz input as an executable document.
+    // We intentionally discard validation errors — we are looking for panics
+    // and for coverage of the @oneOf validation code paths.
+    let result = ExecutableDocument::parse_and_validate(schema, data, "fuzz.graphql");


We should be generating an executable document in fuzz/src/lib.rs in a new function, something like generate_valid_executable_document. That function should use Unstructured to create the executable document, and you can indeed pass it the schema you have defined above. The router repo sort of does something similar in its fuzz src, albeit using the parser rather than the compiler.

I think I agree in principle. The mechanical scope is chunkier than I'd hoped though since apollo-smith generates schema + operations together on its own internal ****TypeDef types, and I don't think there's an option today that takes an existing apollo_compiler::Schema and uses it to generate only operations against it. I think with_document is the closest hook but it expects an apollo_smith::Document, so we'd need a conversion path?

Two routes:

In this PR, I can add a from_compiler_schema(&Schema) -> apollo_smith::Document conversion in apollo-smith that maps every type kind, then generate_valid_executable_document in fuzz/src/lib.rs can wrap with_document + operation_definition.

OR, and I think this is my preference: Follow-up PR, and instead land this with the existing raw-string target (which genuinely caught coverage of the new code paths during dev: all five @oneOf paths in value.rs and coerce_variable_values get hit), file a follow-up for the structure-aware version.

Happy to do "1" here if you'd prefer. Which do you prefer?

lrlna · 2026-04-28T12:28:13Z

 [package]
 name = "apollo-smith"
-version = "0.15.2" # When bumping, also update README.md
+version = "0.15.3" # When bumping, also update README.md


This is usually bumped when a release go through and in a separate PR. So let's leave it on 0.15.2 until a release happens.

sounds good, i'll circle back with a revert on that.

lrlna · 2026-04-28T12:46:47Z

+    #[error(
+        "variable `${variable}` is of type `{variable_type}` \
+         but must be non-nullable to be used for @oneOf input object `{name}` field `{field}`"
+    )]
+    OneOfInputObjectNullableVariable {
+        name: Name,
+        field: Name,
+        variable: Name,
+        variable_type: Node<ast::Type>,
+    },


Can this be an existing DisallowedVariableUsage diagnostic?

If must be non-nullable to be used for @OneOf input object is a useful addition, it can be added as a help text.

Tried this but the field shape doesn't quite line up. Notes below!

DisallowedVariableUsage carries argument: Name and argument_type: Type but in this case here the variable's location is an input object field, not an argument. Folding into the other variant would mean either repurposing those fields with mismatched names, or renaming them to something neutral. The variant's string identifier is externally observable via mod.rs publicity so the ripple touches consumers matching on diagnostic kind too, right?

Three(?+) paths:

a. Keep OneOfInputObjectNullableVariable as a distinct variant which is most accurate semantically; consumers could match on it cleanly.
b. Rename DisallowedVariableUsage's fields to location_name / location_type and fold both cases. It can be an internal-only field rename, but the variant's meaning widens (which arguably has ripple risk for any consumer matching on it).
c. New generic InvalidVariableUsage variant covering both 👯 same field shape as today, just less @oneOf-specific in the naming of.

Thoughts? Mildly lean toward "a" since the location semantics seem genuinely different but happy to follow b/c if you'd rather see consolidation. Which do you prefer?

lrlna · 2026-04-28T12:56:38Z

+    #[error("`{coordinate}` field of a @oneOf input object must be nullable")]
+    OneOfInputObjectFieldNonNull {
+        coordinate: TypeAttributeCoordinate,
+        definition_location: Option<SourceSpan>,
+    },


This sounds like an existing UnsupportedValueType rule, which can have a help text specifying this field needs to be nullable.

I don't think UnsupportedValueType quite fits here since its fields are {value, ty, definition_location} and scoped to value coercion. This variant here fires at schema validation, where there's no value only a type definition. It appears that schema-validation diagnostics in this codebase keep their own variants (e.g., RecursiveInputObjectDefinition, EmptyInputValueSet, etc.) for the same reason?

If the underlying ask is "make the name less @oneOf-specific," happy to rename to InputObjectFieldNonNull and let the help text (which is already using the shared ONE_OF_FIELD_REQUIREMENTS constant) carry the @oneOf context.

Does that get at the root or is it something else??

lrlna · 2026-04-28T13:10:05Z

+                        if field_val.is_null() {
+                            diagnostics.push(
+                                field_val.location(),
+                                DiagnosticData::OneOfInputObjectNullField {
+                                    name: input_obj.name.clone(),
+                                    field: field_name.clone(),
+                                },
+                            );


Was this diagnostic not being caught already by an UnsupportedValueType? Because if something is declared NonNull in the schema, it should be caught. It doesn't matter if it's a oneOf input object or not.

I'm relatively sure it is not getting caught: to verify, I stashed the new rule this PR introduces, re-ran invalid_one_of_null_field, and the document came back Valid ✅ with no other diagnostic. So the gap seems real.

A reason?: @oneOf fields are declared nullable and that's currently enforced at schema validation time in input_object.rs:108-119 here:

https://github.com/apollographql/apollo-rs/blame/00854481b9a3459ace23395f2e185e5d909de555/crates/apollo-compiler/src/validation/input_object.rs#L108-L119

(If that's wrong, we should discuss!)

So at this point in value.rs, the field's ty.is_non_null() is always false, which means UnsupportedValueType's null branch never fires for the literal-null case. Without this rule, { stringField: null } silently passes static validation and only blows up at runtime coercion.

Where I think you're directionally right though: the diagnostic doesn't have to be @oneOf-specific. I replied on OneOfInputObjectNullField with a generic-variant proposal that could be right / what you're looking for.

lrlna · 2026-04-28T13:12:00Z

+                        } else if let ast::Value::Variable(var_name) = &**field_val {
+                            // The field value is a variable — the variable must be declared
+                            // non-null.  An undefined variable is left to the UndefinedVariable
+                            // rule; we only flag nullable variables that are definitely known.
+                            // https://spec.graphql.org/draft/#sec-All-Variable-Usages-are-Allowed
+                            if let Some(var_def) = var_defs.iter().find(|v| v.name == *var_name) {
+                                if !var_def.ty.is_non_null() {
+                                    diagnostics.push(
+                                        field_val.location(),
+                                        DiagnosticData::OneOfInputObjectNullableVariable {
+                                            name: input_obj.name.clone(),
+                                            field: field_name.clone(),
+                                            variable: var_name.clone(),
+                                            variable_type: var_def.ty.clone(),
+                                        },
+                                    );
+                                }
+                            }
+                        }
+                    }
+                }


Does this not get validated in is_variable_usage_allowed?

So as a double check: I stashed the rule, re-ran invalid_one_of_nullable_variable, and document came back Valid with no other diagnostic. So is_variable_usage_allowed doesn't catch it.

I think for two reasons:

validate_variable_usage is only invoked at the top-level argument boundary (in field.rs:58 and in directive.rs:303), and it bails immediately unless the entireeeeee argument value is a Value::Variable. When the argument is arg: { stringField: $v }, it sees an Object literal and the check is skipped entirely.

Even if it did recurse in,is_variable_usage_allowed's strict branch requires location_ty.is_non_null(). However, @oneOf fields are declared nullable, so that branch is skipped and the fall-through is_assignable_to(location_ty) returns true.

Net net that means: query Q($v: String) { f(arg: { stringField: $v }) } passes static validation entirely without the addition of this rule, leaving the violation to surface only at runtime coercion time.

Side note that this digging around is flagging: I think the recursive variable handling in value.rs:135-158:

apollo-rs/crates/apollo-compiler/src/validation/value.rs

Lines 135 to 158 in 809ef8e

ast::Value::Variable(var_name) => {

if let Some(var_def) = var_defs.iter().find(|v| v.name == *var_name) {

match &type_definition {

schema::ExtendedType::Scalar(_)

| schema::ExtendedType::Enum(_)

| schema::ExtendedType::InputObject(_) => {

// we don't have the actual variable values here, so just

// compare if two Types are the same

// TODO(@goto-bus-stop) This should use the is_assignable_to check

if var_def.ty.inner_named_type() != ty.inner_named_type() {

unsupported_type(diagnostics, arg_value, ty);

}

}

_ => unsupported_type(diagnostics, arg_value, ty),

}

} else {

diagnostics.push(

arg_value.location(),

DiagnosticData::UndefinedVariable {

name: var_name.clone(),

},

);

}

}

...only checks type names, not nullability — so spec rule 5.8.5 isn't fully applied to nested input-object positions even outside @oneOf.

That can be verified locally with a RegularInput { y: String! } test, which passes as Valid. If I'm correct, that's a separate latent gap unrelated to this PR. If you agree, I can file a follow-up issue.

lrlna · 2026-04-28T13:19:25Z

+    #[error(
+        "@oneOf input object `{name}` must specify exactly one key, but {provided} {} given",
+        if *provided == 1 { "was" } else { "were" }
+    )]
+    OneOfInputObjectWrongNumberOfFields { name: Name, provided: usize },


I'd rather have these be non-specific to oneOf. Perhaps this can be UniqueField or UniqueKey, similar to UniqueDirective?

I'm not sure the Unique* parallel quite holds.

All the existing Unique* variants (UniqueVariable, UniqueArgument, UniqueDirective, UniqueInputValue) mean "this thing was provided more than once". i.e. at most one. The @oneOf rule is exactly one, and this diagnostic also fires for zero fields, which doesn't fit the uniqueness framing.

Two alternatives worth considering:

Split into OneOfInputObjectMissingField (zero case) and OneOfInputObjectMultipleFields (2+ case). Seems cleanly defined, but the con is it doubles the variant count for what's a single spec rule.

Keep the single variant pragmatic given "exactly one of N" is, I think, currently @oneOf-"only" in GraphQL.

what's your take? same as before?

lrlna · 2026-04-28T13:21:35Z

+    #[error("`{coordinate}` field of a @oneOf input object must not have a default value")]
+    OneOfInputObjectFieldHasDefault {
+        coordinate: TypeAttributeCoordinate,
+        default_location: Option<SourceSpan>,
+    },


Like one of the comments above, this should also be named independantly of oneOf. Perhaps, just UnsupportedDefault?

Fair. will swap to UnsupportedDefault and move the @oneOf specifics to help text.

lrlna · 2026-04-28T14:08:13Z

@@ -1,6 +1,6 @@
 [package]
 name = "apollo-compiler"
-version = "1.31.1" # When bumping, also update README.md


same comment as with apollo-smith: we usually bump these in a release PR

Yeah, I can revert that. I had this for local testing of something.

lrlna · 2026-04-28T15:52:31Z

The main things to sort out are the diagnostics and whether some of them already run, but otherwise should be good to land.

abernix

Thank you for your review! I've replied to I think all of them here.

abernix · 2026-05-12T07:50:15Z

+    #[error(
+        "@oneOf input object `{name}` must specify exactly one key, but {provided} {} given",
+        if *provided == 1 { "was" } else { "were" }
+    )]
+    OneOfInputObjectWrongNumberOfFields { name: Name, provided: usize },


I'm not sure the Unique* parallel quite holds.

All the existing Unique* variants (UniqueVariable, UniqueArgument, UniqueDirective, UniqueInputValue) mean "this thing was provided more than once". i.e. at most one. The @oneOf rule is exactly one, and this diagnostic also fires for zero fields, which doesn't fit the uniqueness framing.

Two alternatives worth considering:

Split into OneOfInputObjectMissingField (zero case) and OneOfInputObjectMultipleFields (2+ case). Seems cleanly defined, but the con is it doubles the variant count for what's a single spec rule.

Keep the single variant pragmatic given "exactly one of N" is, I think, currently @oneOf-"only" in GraphQL.

what's your take? same as before?

abernix · 2026-05-12T07:52:11Z

+    #[error("`{name}.{field}` value for @oneOf input object must be non-null")]
+    OneOfInputObjectNullField { name: Name, field: Name },


There's a clean middle ground here, I think.

Your underlying point that this shouldn't be @oneOf-specific is fair. However folding directly into UnsupportedValueType seems awkward because its message reads "expected value of type {ty}, found {value}", plus the declared ty is nullable, so "expected String, found null" reads as kinda misleading? (String does allow null at most positions; @oneOf is the special case).

Would you be in favor of a new generic variant like NullValueNotAllowed (or NullValueAtRequiredPosition?) that describes the actual invariant ("this position is treated as required even though the declared type allows null"). It'd be reusable for any future feature with the same shape; and carries the @oneOf specifics in help text.

Want me to take a swing at that?

abernix · 2026-05-12T07:52:34Z

+    #[error("`{coordinate}` field of a @oneOf input object must not have a default value")]
+    OneOfInputObjectFieldHasDefault {
+        coordinate: TypeAttributeCoordinate,
+        default_location: Option<SourceSpan>,
+    },


Fair. will swap to UnsupportedDefault and move the @oneOf specifics to help text.

abernix · 2026-05-12T07:56:07Z

+    #[error(
+        "variable `${variable}` is of type `{variable_type}` \
+         but must be non-nullable to be used for @oneOf input object `{name}` field `{field}`"
+    )]
+    OneOfInputObjectNullableVariable {
+        name: Name,
+        field: Name,
+        variable: Name,
+        variable_type: Node<ast::Type>,
+    },


Tried this but the field shape doesn't quite line up. Notes below!

DisallowedVariableUsage carries argument: Name and argument_type: Type but in this case here the variable's location is an input object field, not an argument. Folding into the other variant would mean either repurposing those fields with mismatched names, or renaming them to something neutral. The variant's string identifier is externally observable via mod.rs publicity so the ripple touches consumers matching on diagnostic kind too, right?

Three(?+) paths:

a. Keep OneOfInputObjectNullableVariable as a distinct variant which is most accurate semantically; consumers could match on it cleanly.
b. Rename DisallowedVariableUsage's fields to location_name / location_type and fold both cases. It can be an internal-only field rename, but the variant's meaning widens (which arguably has ripple risk for any consumer matching on it).
c. New generic InvalidVariableUsage variant covering both 👯 same field shape as today, just less @oneOf-specific in the naming of.

Thoughts? Mildly lean toward "a" since the location semantics seem genuinely different but happy to follow b/c if you'd rather see consolidation. Which do you prefer?

abernix · 2026-05-12T11:51:45Z

+                        } else if let ast::Value::Variable(var_name) = &**field_val {
+                            // The field value is a variable — the variable must be declared
+                            // non-null.  An undefined variable is left to the UndefinedVariable
+                            // rule; we only flag nullable variables that are definitely known.
+                            // https://spec.graphql.org/draft/#sec-All-Variable-Usages-are-Allowed
+                            if let Some(var_def) = var_defs.iter().find(|v| v.name == *var_name) {
+                                if !var_def.ty.is_non_null() {
+                                    diagnostics.push(
+                                        field_val.location(),
+                                        DiagnosticData::OneOfInputObjectNullableVariable {
+                                            name: input_obj.name.clone(),
+                                            field: field_name.clone(),
+                                            variable: var_name.clone(),
+                                            variable_type: var_def.ty.clone(),
+                                        },
+                                    );
+                                }
+                            }
+                        }
+                    }
+                }


So as a double check: I stashed the rule, re-ran invalid_one_of_nullable_variable, and document came back Valid with no other diagnostic. So is_variable_usage_allowed doesn't catch it.

I think for two reasons:

validate_variable_usage is only invoked at the top-level argument boundary (in field.rs:58 and in directive.rs:303), and it bails immediately unless the entireeeeee argument value is a Value::Variable. When the argument is arg: { stringField: $v }, it sees an Object literal and the check is skipped entirely.

Even if it did recurse in,is_variable_usage_allowed's strict branch requires location_ty.is_non_null(). However, @oneOf fields are declared nullable, so that branch is skipped and the fall-through is_assignable_to(location_ty) returns true.

Net net that means: query Q($v: String) { f(arg: { stringField: $v }) } passes static validation entirely without the addition of this rule, leaving the violation to surface only at runtime coercion time.

Side note that this digging around is flagging: I think the recursive variable handling in value.rs:135-158:

apollo-rs/crates/apollo-compiler/src/validation/value.rs

Lines 135 to 158 in 809ef8e

ast::Value::Variable(var_name) => {

if let Some(var_def) = var_defs.iter().find(|v| v.name == *var_name) {

match &type_definition {

schema::ExtendedType::Scalar(_)

| schema::ExtendedType::Enum(_)

| schema::ExtendedType::InputObject(_) => {

// we don't have the actual variable values here, so just

// compare if two Types are the same

// TODO(@goto-bus-stop) This should use the is_assignable_to check

if var_def.ty.inner_named_type() != ty.inner_named_type() {

unsupported_type(diagnostics, arg_value, ty);

}

}

_ => unsupported_type(diagnostics, arg_value, ty),

}

} else {

diagnostics.push(

arg_value.location(),

DiagnosticData::UndefinedVariable {

name: var_name.clone(),

},

);

}

}

...only checks type names, not nullability — so spec rule 5.8.5 isn't fully applied to nested input-object positions even outside @oneOf.

That can be verified locally with a RegularInput { y: String! } test, which passes as Valid. If I'm correct, that's a separate latent gap unrelated to this PR. If you agree, I can file a follow-up issue.

abernix · 2026-05-12T11:53:40Z

 [package]
 name = "apollo-smith"
-version = "0.15.2" # When bumping, also update README.md
+version = "0.15.3" # When bumping, also update README.md


sounds good, i'll circle back with a revert on that.

abernix · 2026-05-12T11:55:24Z

+        let has_directive = input_obj.directives.get("oneOf").is_some();
+        let is_one_of = input_obj.is_one_of();
+
+        // Invariant: is_one_of() must agree with directive presence.
+        assert_eq!(
+            has_directive, is_one_of,
+            "is_one_of() disagrees with @oneOf directive presence for type `{type_name}`"
+        );


Comparing a value to itself is certainly genius. 🙂 Let remove that.

I think the remaining invariant checks (all fields nullable, no defaults) are the ones actually exercising the schema-validation rule, and they'll stay.

abernix · 2026-05-12T11:58:50Z

+fuzz_target!(|data: &str| {
+    let _ = env_logger::try_init();
+    debug!("{data}");
+
+    let schema = schema();
+
+    // Parse and validate the fuzz input as an executable document.
+    // We intentionally discard validation errors — we are looking for panics
+    // and for coverage of the @oneOf validation code paths.
+    let result = ExecutableDocument::parse_and_validate(schema, data, "fuzz.graphql");


I think I agree in principle. The mechanical scope is chunkier than I'd hoped though since apollo-smith generates schema + operations together on its own internal ****TypeDef types, and I don't think there's an option today that takes an existing apollo_compiler::Schema and uses it to generate only operations against it. I think with_document is the closest hook but it expects an apollo_smith::Document, so we'd need a conversion path?

Two routes:

In this PR, I can add a from_compiler_schema(&Schema) -> apollo_smith::Document conversion in apollo-smith that maps every type kind, then generate_valid_executable_document in fuzz/src/lib.rs can wrap with_document + operation_definition.

OR, and I think this is my preference: Follow-up PR, and instead land this with the existing raw-string target (which genuinely caught coverage of the new code paths during dev: all five @oneOf paths in value.rs and coerce_variable_values get hit), file a follow-up for the structure-aware version.

Happy to do "1" here if you'd prefer. Which do you prefer?

abernix · 2026-05-12T12:01:14Z

+    #[error("`{coordinate}` field of a @oneOf input object must be nullable")]
+    OneOfInputObjectFieldNonNull {
+        coordinate: TypeAttributeCoordinate,
+        definition_location: Option<SourceSpan>,
+    },


I don't think UnsupportedValueType quite fits here since its fields are {value, ty, definition_location} and scoped to value coercion. This variant here fires at schema validation, where there's no value only a type definition. It appears that schema-validation diagnostics in this codebase keep their own variants (e.g., RecursiveInputObjectDefinition, EmptyInputValueSet, etc.) for the same reason?

If the underlying ask is "make the name less @oneOf-specific," happy to rename to InputObjectFieldNonNull and let the help text (which is already using the shared ONE_OF_FIELD_REQUIREMENTS constant) carry the @oneOf context.

Does that get at the root or is it something else??

abernix · 2026-05-12T12:17:41Z

+                        if field_val.is_null() {
+                            diagnostics.push(
+                                field_val.location(),
+                                DiagnosticData::OneOfInputObjectNullField {
+                                    name: input_obj.name.clone(),
+                                    field: field_name.clone(),
+                                },
+                            );


I'm relatively sure it is not getting caught: to verify, I stashed the new rule this PR introduces, re-ran invalid_one_of_null_field, and the document came back Valid ✅ with no other diagnostic. So the gap seems real.

A reason?: @oneOf fields are declared nullable and that's currently enforced at schema validation time in input_object.rs:108-119 here:

https://github.com/apollographql/apollo-rs/blame/00854481b9a3459ace23395f2e185e5d909de555/crates/apollo-compiler/src/validation/input_object.rs#L108-L119

(If that's wrong, we should discuss!)

So at this point in value.rs, the field's ty.is_non_null() is always false, which means UnsupportedValueType's null branch never fires for the literal-null case. Without this rule, { stringField: null } silently passes static validation and only blows up at runtime coercion.

Where I think you're directionally right though: the diagnostic doesn't have to be @oneOf-specific. I replied on OneOfInputObjectNullField with a generic-variant proposal that could be right / what you're looking for.

abernix force-pushed the abernix/oneof-us branch from c3a152f to 26ba17f Compare March 27, 2026 16:14

abernix added 5 commits March 27, 2026 18:42

abernix force-pushed the abernix/oneof-us branch 3 times, most recently from 4b0387d to e15a08d Compare March 27, 2026 17:45

fix(lint): remove needless borrows in @oneOf validation tests

0085448

clippy::needless_borrow fired on `&schema` at 9 call sites in tests/validation/one_of.rs — the compiler auto-derefs, so the explicit borrow is redundant.

abernix force-pushed the abernix/oneof-us branch from e15a08d to 0085448 Compare March 27, 2026 18:39

abernix marked this pull request as ready for review March 27, 2026 18:52

abernix requested a review from a team as a code owner March 27, 2026 18:52

martinbonnin approved these changes Apr 1, 2026

View reviewed changes

lrlna requested changes Apr 28, 2026

View reviewed changes

abernix commented May 12, 2026

View reviewed changes

abernix requested a review from lrlna May 12, 2026 17:06

	ast::Value::Variable(var_name) => {
	if let Some(var_def) = var_defs.iter().find(\|v\| v.name == *var_name) {
	match &type_definition {
	schema::ExtendedType::Scalar(_)
	\| schema::ExtendedType::Enum(_)
	\| schema::ExtendedType::InputObject(_) => {
	// we don't have the actual variable values here, so just
	// compare if two Types are the same
	// TODO(@goto-bus-stop) This should use the is_assignable_to check
	if var_def.ty.inner_named_type() != ty.inner_named_type() {
	unsupported_type(diagnostics, arg_value, ty);
	}
	}
	_ => unsupported_type(diagnostics, arg_value, ty),
	}
	} else {
	diagnostics.push(
	arg_value.location(),
	DiagnosticData::UndefinedVariable {
	name: var_name.clone(),
	},
	);
	}
	}

		#[error("`{name}.{field}` value for @oneOf input object must be non-null")]
		OneOfInputObjectNullField { name: Name, field: Name },

Conversation

abernix commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How it works — three validation layers

What's new in the public API

Fuzz coverage

Version

Notes

Uh oh!

martinbonnin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martinbonnin Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrlna commented Apr 28, 2026

Uh oh!

abernix left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abernix commented Mar 27, 2026 •

edited

Loading

martinbonnin Apr 13, 2026 •

edited

Loading