From 10ef161a4250e768ecda819db3c5c5bd382e8cab Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 7 Jul 2026 22:23:37 +0000 Subject: [PATCH] [libgraphql-core-v1] Validate @oneOf input object constraints Implements Task 16.6a from the v1 plan. The @oneOf directive was registered as a builtin since Task 14, but nothing enforced its type-validation constraints -- a schema like `input X @oneOf { a: Int! }` built silently despite being invalid per the September 2025 spec. Per the spec's Input Objects Type Validation rules, every field of a @oneOf input object must have a nullable type and must not declare a default value (each input represents exactly one choice, so a required field or an always-present default would be contradictory). The check lives in the existing InputObjectTypeValidator (already orchestrated by SchemaBuilder::build() step 4, so no new wiring), with two new precisely-targeted TypeValidationErrorKind variants rather than one merged kind -- a field violating both rules reports both errors, and each error's span points at the offending construct (the type annotation for nullability; the field for defaults). Both carry ErrorNote::spec links per the plan's error-note convention. Detection matches the directive annotation by name; argument-level validation of directive applications is out of scope here (owned by Task 16.6f). Tests: 5 unit tests (valid oneOf, non-nullable rejected, defaulted rejected, both-violations reports two errors, non-oneOf control) plus 2 end-to-end build_from_str tests proving the annotation survives the full parse -> build -> validate pipeline. Plan doc updated with completion notes per the Execution Protocol. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw --- .../src/schema/tests/schema_builder_tests.rs | 74 +++++ .../src/schema/type_validation_error.rs | 20 ++ .../validators/input_object_type_validator.rs | 53 +++- .../input_object_type_validator_tests.rs | 288 ++++++++++++++++++ libgraphql-core-v1-plan.md | 25 +- 5 files changed, 454 insertions(+), 6 deletions(-) diff --git a/crates/libgraphql-core-v1/src/schema/tests/schema_builder_tests.rs b/crates/libgraphql-core-v1/src/schema/tests/schema_builder_tests.rs index 9b2bcd5..931c869 100644 --- a/crates/libgraphql-core-v1/src/schema/tests/schema_builder_tests.rs +++ b/crates/libgraphql-core-v1/src/schema/tests/schema_builder_tests.rs @@ -872,3 +872,77 @@ fn build_enum_with_no_values_fails() { }); assert!(has_error, "expected EnumWithNoValues error"); } + +// Verifies end-to-end (parse -> build -> validate) that a +// `@oneOf` input object with a non-nullable field and a defaulted +// field is rejected with both oneOf violations. +// +// See https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn build_oneof_input_object_violations_rejected() { + let result = SchemaBuilder::build_from_str( + "type Query { x: Int }\n\ + input UserLookup @oneOf {\n\ + byId: Int!\n\ + byName: String = \"anonymous\"\n\ + }", + ); + assert!(result.is_err()); + let errors = result.unwrap_err(); + let has_non_nullable_err = errors.errors().iter().any(|e| { + if let SchemaBuildErrorKind::TypeValidation(tve) = e.kind() { + matches!( + tve.kind(), + TypeValidationErrorKind::InvalidNonNullableOneOfInputField { + field_name, + parent_type_name, + } if field_name == "byId" + && parent_type_name == "UserLookup", + ) + } else { + false + } + }); + let has_default_err = errors.errors().iter().any(|e| { + if let SchemaBuildErrorKind::TypeValidation(tve) = e.kind() { + matches!( + tve.kind(), + TypeValidationErrorKind::InvalidOneOfInputFieldWithDefaultValue { + field_name, + parent_type_name, + } if field_name == "byName" + && parent_type_name == "UserLookup", + ) + } else { + false + } + }); + assert!( + has_non_nullable_err, + "expected InvalidNonNullableOneOfInputField for byId", + ); + assert!( + has_default_err, + "expected InvalidOneOfInputFieldWithDefaultValue for byName", + ); +} + +// Verifies end-to-end that a spec-conformant `@oneOf` input +// object (all fields nullable, no defaults) builds successfully. +// +// See https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn build_valid_oneof_input_object_succeeds() { + let schema = SchemaBuilder::build_from_str( + "type Query { x: Int }\n\ + input UserLookup @oneOf {\n\ + byId: Int\n\ + byName: String\n\ + }", + ).unwrap(); + assert!(schema.input_object_type("UserLookup").is_some()); +} diff --git a/crates/libgraphql-core-v1/src/schema/type_validation_error.rs b/crates/libgraphql-core-v1/src/schema/type_validation_error.rs index f543b30..c1c0290 100644 --- a/crates/libgraphql-core-v1/src/schema/type_validation_error.rs +++ b/crates/libgraphql-core-v1/src/schema/type_validation_error.rs @@ -134,6 +134,26 @@ pub enum TypeValidationErrorKind { type_name: String, }, + #[error( + "input field `{parent_type_name}.{field_name}` must have a \ + nullable type because `{parent_type_name}` is a `@oneOf` \ + input object" + )] + InvalidNonNullableOneOfInputField { + field_name: String, + parent_type_name: String, + }, + + #[error( + "input field `{parent_type_name}.{field_name}` must not \ + declare a default value because `{parent_type_name}` is a \ + `@oneOf` input object" + )] + InvalidOneOfInputFieldWithDefaultValue { + field_name: String, + parent_type_name: String, + }, + #[error( "output field `{parent_type_name}.{field_name}` has \ type `{input_type_name}` which is not an output type" diff --git a/crates/libgraphql-core-v1/src/validators/input_object_type_validator.rs b/crates/libgraphql-core-v1/src/validators/input_object_type_validator.rs index e6381b6..e26e4aa 100644 --- a/crates/libgraphql-core-v1/src/validators/input_object_type_validator.rs +++ b/crates/libgraphql-core-v1/src/validators/input_object_type_validator.rs @@ -12,13 +12,15 @@ use indexmap::IndexMap; use std::collections::HashSet; /// Validates an input object type's field type references, -/// input-type legality, and circular non-nullable reference -/// chains. +/// input-type legality, circular non-nullable reference chains, +/// and `@oneOf` constraints. /// /// Per the GraphQL spec, all input object fields must reference /// valid input types (scalars, enums, or other input objects) and /// input object types must not form non-nullable circular /// references (which would make them impossible to construct). +/// Additionally, every field of a `@oneOf` input object must have +/// a nullable type and must not declare a default value. /// /// See [Input Objects](https://spec.graphql.org/September2025/#sec-Input-Objects). pub(crate) struct InputObjectTypeValidator<'a> { @@ -40,6 +42,7 @@ impl<'a> InputObjectTypeValidator<'a> { } pub fn validate(mut self) -> Vec { + self.validate_oneof_constraints(); let fields = self.type_.fields(); self.validate_fields_recursive( self.type_.name(), @@ -50,6 +53,52 @@ impl<'a> InputObjectTypeValidator<'a> { self.errors } + /// Enforces the `@oneOf` input object constraints: every field + /// must have a nullable type and must not declare a default + /// value. + /// + /// See [Input Objects — Type Validation](https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation). + fn validate_oneof_constraints(&mut self) { + let is_oneof = self.type_.directives().iter().any( + |annot| annot.name().as_str() == "oneOf", + ); + if !is_oneof { + return; + } + + for (field_name, field) in self.type_.fields() { + if !field.type_annotation().nullable() { + // https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation + self.errors.push(TypeValidationError::new( + TypeValidationErrorKind::InvalidNonNullableOneOfInputField { + field_name: field_name.to_string(), + parent_type_name: + self.type_.name().to_string(), + }, + field.type_annotation().span(), + vec![ErrorNote::spec( + "https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation", + )], + )); + } + + if field.default_value().is_some() { + // https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation + self.errors.push(TypeValidationError::new( + TypeValidationErrorKind::InvalidOneOfInputFieldWithDefaultValue { + field_name: field_name.to_string(), + parent_type_name: + self.type_.name().to_string(), + }, + field.span(), + vec![ErrorNote::spec( + "https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation", + )], + )); + } + } + } + fn validate_fields_recursive( &mut self, type_name: &'a TypeName, diff --git a/crates/libgraphql-core-v1/src/validators/tests/input_object_type_validator_tests.rs b/crates/libgraphql-core-v1/src/validators/tests/input_object_type_validator_tests.rs index a522cb5..4fa8aa3 100644 --- a/crates/libgraphql-core-v1/src/validators/tests/input_object_type_validator_tests.rs +++ b/crates/libgraphql-core-v1/src/validators/tests/input_object_type_validator_tests.rs @@ -1,3 +1,5 @@ +use crate::directive_annotation::DirectiveAnnotation; +use crate::names::DirectiveName; use crate::names::FieldName; use crate::names::TypeName; use crate::schema::TypeValidationError; @@ -14,6 +16,7 @@ use crate::types::ScalarType; use crate::types::TypeAnnotation; use crate::types::UnionType; use crate::validators::InputObjectTypeValidator; +use crate::value::Value; use indexmap::IndexMap; fn string_scalar() -> GraphQLType { @@ -998,3 +1001,288 @@ fn circular_chain_no_path_leaking_between_cycles() { first cycle): {c_msg}", ); } + +fn oneof_annotation() -> DirectiveAnnotation { + DirectiveAnnotation { + arguments: IndexMap::new(), + name: DirectiveName::new("oneOf"), + span: Span::dummy(), + } +} + +// Verifies that a `@oneOf` input object whose fields are all +// nullable and default-less produces no errors. +// https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// Written by Claude Code, reviewed by a human. +#[test] +fn oneof_with_nullable_defaultless_fields_is_valid() { + let mut fields = IndexMap::new(); + fields.insert( + FieldName::new("byId"), + make_input_field( + "byId", + "UserLookup", + TypeAnnotation::named("Int", /* nullable = */ true), + ), + ); + fields.insert( + FieldName::new("byName"), + make_input_field( + "byName", + "UserLookup", + TypeAnnotation::named("String", /* nullable = */ true), + ), + ); + let input_obj = InputObjectType { + description: None, + directives: vec![oneof_annotation()], + fields, + name: TypeName::new("UserLookup"), + span: Span::dummy(), + }; + + let mut types_map = IndexMap::new(); + types_map.insert(TypeName::new("String"), string_scalar()); + types_map.insert(TypeName::new("Int"), int_scalar()); + + let errors = InputObjectTypeValidator::new( + &input_obj, + &types_map, + ).validate(); + assert!( + errors.is_empty(), + "expected no errors, got: {errors:?}", + ); +} + +// Verifies that a `@oneOf` input object with a non-nullable field +// is rejected with InvalidNonNullableOneOfInputField. +// https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// Written by Claude Code, reviewed by a human. +#[test] +fn oneof_with_non_nullable_field_rejected() { + let mut fields = IndexMap::new(); + fields.insert( + FieldName::new("byId"), + make_input_field( + "byId", + "UserLookup", + TypeAnnotation::named("Int", /* nullable = */ false), + ), + ); + let input_obj = InputObjectType { + description: None, + directives: vec![oneof_annotation()], + fields, + name: TypeName::new("UserLookup"), + span: Span::dummy(), + }; + + let mut types_map = IndexMap::new(); + types_map.insert(TypeName::new("Int"), int_scalar()); + + let errors = InputObjectTypeValidator::new( + &input_obj, + &types_map, + ).validate(); + assert_eq!(errors.len(), 1, "got: {errors:?}"); + assert!(matches!( + errors[0].kind(), + TypeValidationErrorKind::InvalidNonNullableOneOfInputField { + field_name, + parent_type_name, + } if field_name == "byId" && parent_type_name == "UserLookup", + )); +} + +// Verifies that a `@oneOf` input object with a defaulted field is +// rejected with InvalidOneOfInputFieldWithDefaultValue. +// https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// Written by Claude Code, reviewed by a human. +#[test] +fn oneof_with_defaulted_field_rejected() { + let mut field = make_input_field( + "byId", + "UserLookup", + TypeAnnotation::named("Int", /* nullable = */ true), + ); + field.default_value = Some(Value::Int(42)); + let mut fields = IndexMap::new(); + fields.insert(FieldName::new("byId"), field); + let input_obj = InputObjectType { + description: None, + directives: vec![oneof_annotation()], + fields, + name: TypeName::new("UserLookup"), + span: Span::dummy(), + }; + + let mut types_map = IndexMap::new(); + types_map.insert(TypeName::new("Int"), int_scalar()); + + let errors = InputObjectTypeValidator::new( + &input_obj, + &types_map, + ).validate(); + assert_eq!(errors.len(), 1, "got: {errors:?}"); + assert!(matches!( + errors[0].kind(), + TypeValidationErrorKind::InvalidOneOfInputFieldWithDefaultValue { + field_name, + parent_type_name, + } if field_name == "byId" && parent_type_name == "UserLookup", + )); +} + +// Verifies that a `@oneOf` input object with a field that is both +// non-nullable AND defaulted reports both violations. +// https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// Written by Claude Code, reviewed by a human. +#[test] +fn oneof_field_both_non_nullable_and_defaulted_reports_both() { + let mut field = make_input_field( + "byId", + "UserLookup", + TypeAnnotation::named("Int", /* nullable = */ false), + ); + field.default_value = Some(Value::Int(42)); + let mut fields = IndexMap::new(); + fields.insert(FieldName::new("byId"), field); + let input_obj = InputObjectType { + description: None, + directives: vec![oneof_annotation()], + fields, + name: TypeName::new("UserLookup"), + span: Span::dummy(), + }; + + let mut types_map = IndexMap::new(); + types_map.insert(TypeName::new("Int"), int_scalar()); + + let errors = InputObjectTypeValidator::new( + &input_obj, + &types_map, + ).validate(); + assert_eq!(errors.len(), 2, "got: {errors:?}"); +} + +// Verifies that a NON-oneOf input object may freely use +// non-nullable and defaulted fields (control test — the oneOf +// constraints must not leak onto ordinary input objects). +// https://spec.graphql.org/September2025/#sec-Input-Objects +// Written by Claude Code, reviewed by a human. +#[test] +fn non_oneof_allows_non_nullable_and_defaulted_fields() { + let mut field = make_input_field( + "byId", + "UserLookup", + TypeAnnotation::named("Int", /* nullable = */ false), + ); + field.default_value = Some(Value::Int(42)); + let mut fields = IndexMap::new(); + fields.insert(FieldName::new("byId"), field); + let input_obj = InputObjectType { + description: None, + directives: vec![], + fields, + name: TypeName::new("UserLookup"), + span: Span::dummy(), + }; + + let mut types_map = IndexMap::new(); + types_map.insert(TypeName::new("Int"), int_scalar()); + + let errors = InputObjectTypeValidator::new( + &input_obj, + &types_map, + ).validate(); + assert!( + errors.is_empty(), + "expected no errors, got: {errors:?}", + ); +} + +// Verifies that a `@oneOf` field with a non-nullable LIST type +// (`[Int]!`) is rejected: the spec's nullability constraint +// applies to the field's OUTER type. +// https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// Written by Claude Code, reviewed by a human. +#[test] +fn oneof_with_non_nullable_list_field_rejected() { + let mut fields = IndexMap::new(); + fields.insert( + FieldName::new("byIds"), + make_input_field( + "byIds", + "UserLookup", + TypeAnnotation::list( + TypeAnnotation::named("Int", /* nullable = */ true), + /* nullable = */ false, + ), + ), + ); + let input_obj = InputObjectType { + description: None, + directives: vec![oneof_annotation()], + fields, + name: TypeName::new("UserLookup"), + span: Span::dummy(), + }; + + let mut types_map = IndexMap::new(); + types_map.insert(TypeName::new("Int"), int_scalar()); + + let errors = InputObjectTypeValidator::new( + &input_obj, + &types_map, + ).validate(); + assert_eq!(errors.len(), 1, "got: {errors:?}"); + assert!(matches!( + errors[0].kind(), + TypeValidationErrorKind::InvalidNonNullableOneOfInputField { + field_name, + .. + } if field_name == "byIds", + )); +} + +// Verifies that a `@oneOf` field with a nullable list of +// non-nullable elements (`[Int!]`) is ACCEPTED: only the field's +// outer nullability matters; inner element non-nullability is +// irrelevant to the oneOf constraint. +// https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// Written by Claude Code, reviewed by a human. +#[test] +fn oneof_with_nullable_list_of_non_nullable_elements_is_valid() { + let mut fields = IndexMap::new(); + fields.insert( + FieldName::new("byIds"), + make_input_field( + "byIds", + "UserLookup", + TypeAnnotation::list( + TypeAnnotation::named("Int", /* nullable = */ false), + /* nullable = */ true, + ), + ), + ); + let input_obj = InputObjectType { + description: None, + directives: vec![oneof_annotation()], + fields, + name: TypeName::new("UserLookup"), + span: Span::dummy(), + }; + + let mut types_map = IndexMap::new(); + types_map.insert(TypeName::new("Int"), int_scalar()); + + let errors = InputObjectTypeValidator::new( + &input_obj, + &types_map, + ).validate(); + assert!( + errors.is_empty(), + "expected no errors, got: {errors:?}", + ); +} diff --git a/libgraphql-core-v1-plan.md b/libgraphql-core-v1-plan.md index ff9ee7a..69008aa 100644 --- a/libgraphql-core-v1-plan.md +++ b/libgraphql-core-v1-plan.md @@ -3818,10 +3818,25 @@ Fixes validation gaps discovered by the Task 16.5 audit in already-merged work. **Each item below is its own PR** (independently reviewable). **16.6a — `@oneOf` validation (§3.13.5):** -- [ ] Enforce on `@oneOf` input objects: every field nullable, no defaults; wire into +- [x] Enforce on `@oneOf` input objects: every field nullable, no defaults; wire into `build()` (likely inside `input_object_type_validator`) -- [ ] Tests: valid oneOf; non-null field rejected; defaulted field rejected -- [ ] Commit: `[libgraphql-core-v1] Validate @oneOf input object constraints` +- [x] Tests: valid oneOf; non-null field rejected; defaulted field rejected +- [x] Commit: `[libgraphql-core-v1] Validate @oneOf input object constraints` + +**Completion Notes (16.6a):** Implemented as `validate_oneof_constraints()` inside the +existing `InputObjectTypeValidator` (already wired into `build()` step 4 — no new +wiring needed). Two new `TypeValidationErrorKind` variants: +`InvalidNonNullableOneOfInputField` (span = field's type annotation) and +`InvalidOneOfInputFieldWithDefaultValue` (span = field), both carrying +`ErrorNote::spec` links to §3.10 Type Validation. Detection is by directive name +(`"oneOf"`) on the type's annotations — sound because `absorb_directive()` rejects any +user redefinition of the builtin. 9 tests: 7 unit (valid, non-null rejected, default +rejected, both-violations-on-one-field reports two errors, non-oneOf control, +`[Int]!` rejected / `[Int!]` accepted list-nullability pair) + 2 end-to-end via +`build_from_str` proving the annotation survives parse → builder → validator. +**Known gap (blocked on 16.6b):** `@oneOf` applied via `extend input X @oneOf` is not +seen today because ALL type extensions are silently dropped — 16.6b must add a +regression test for oneOf-via-extension when extension merging lands. **16.6b — Type extensions:** Currently `SchemaBuilder` silently drops every `TypeExtension` (`TypeExtension(_) => @@ -3833,6 +3848,8 @@ Currently `SchemaBuilder` silently drops every `TypeExtension` (`TypeExtension(_ type definition in load order) - [ ] Wire `ExtensionOfUndefinedType` + `InvalidExtensionTypeKind` - [ ] Tests: all 6 kinds × (merge, undefined target, kind mismatch, duplicate member/field) +- [ ] Regression test from 16.6a: `extend input X @oneOf` — the merged directive must + trigger the oneOf constraints on X's fields - [ ] Commit: `[libgraphql-core-v1] Implement type extension merging` **16.6c — IsValidImplementation step 2.f (deprecated-field consistency):** @@ -4906,7 +4923,7 @@ no "deferred past 1.0" bucket. (Checklist state below verified against code at T - [x] Output field types must be output types - [x] Parameter types must be input types - [x] Input object circular non-nullable reference detection -- [ ] **(Task 16.6a)** `@oneOf` input objects: all fields must be nullable with no default values (§3.10 *Input Objects* Type Validation; the `@oneOf` directive itself is §3.13.5) +- [x] `@oneOf` input objects: all fields must be nullable with no default values (§3.10 *Input Objects* Type Validation; the `@oneOf` directive itself is §3.13.5) — Task 16.6a - [x] All type references resolve to defined types - [x] Directive argument types must be input types - [ ] **(Task 16.6b)** Extension of undefined types rejected (error kind exists but is never constructed — extensions are currently silently dropped)