From 7a9d91b221783a20fbe109da80dd703b54e5a117 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 03:03:38 +0000 Subject: [PATCH 1/2] [libgraphql-core-v1] Add spec notes to all build-level errors Implements Task 16.6d. Task 13 mandated that every validation error carry an ErrorNote::spec, but the rule only landed for the four type validators -- 41 SchemaBuildError construction sites across the type builders (30) and SchemaBuilder (11) carried empty notes and lacked the spec-URL comment convention. Every site now attaches the most precise September 2025 anchor (verified against the spec source; notable corrections: duplicate type/directive definitions cite #sec-Schema's uniqueness rules, and InvalidEnumValueName cites the #sec-Enum-Value grammar production rather than the enum Type-Validation section, which only covers uniqueness). URLs fired from multiple sites live in a new crate-private spec_urls module so they cannot diverge; the literal URL remains in the comment above each site per convention. The TypeValidation wrapper in build() now propagates the inner TypeValidationError's notes onto the outer SchemaBuildError, so notes() surfaces spec notes uniformly regardless of error layer. Two deliberate exceptions, inline-commented: ParseError (grammar level; parser-supplied notes pass through verbatim) and SourceMapLimitExceeded (a u16 implementation limit, not a spec rule). Tests: 8 per-builder spec-note assertions, a note-propagation test, and the meta-test all_build_level_errors_carry_spec_notes -- a kitchen-sink invalid schema triggering 22 distinct error kinds that asserts EVERY returned error carries a Spec note except the two documented exceptions, locking the convention against regression. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw --- crates/libgraphql-core-v1/src/lib.rs | 1 + .../src/schema/schema_builder.rs | 87 ++++- .../src/schema/tests/schema_builder_tests.rs | 229 +++++++++++++ crates/libgraphql-core-v1/src/spec_urls.rs | 82 +++++ .../src/type_builders/directive_builder.rs | 17 +- .../src/type_builders/enum_type_builder.rs | 12 +- .../src/type_builders/field_def_builder.rs | 6 +- .../input_object_type_builder.rs | 14 +- .../type_builders/interface_type_builder.rs | 14 +- .../src/type_builders/object_type_builder.rs | 12 +- .../src/type_builders/scalar_type_builder.rs | 6 +- .../tests/builder_validation_tests.rs | 308 ++++++++++++++++++ .../src/type_builders/union_type_builder.rs | 8 +- libgraphql-core-v1-plan.md | 30 +- 14 files changed, 775 insertions(+), 51 deletions(-) create mode 100644 crates/libgraphql-core-v1/src/spec_urls.rs diff --git a/crates/libgraphql-core-v1/src/lib.rs b/crates/libgraphql-core-v1/src/lib.rs index 1fc3731..e359d55 100644 --- a/crates/libgraphql-core-v1/src/lib.rs +++ b/crates/libgraphql-core-v1/src/lib.rs @@ -38,6 +38,7 @@ pub mod operation_kind; pub mod schema; pub mod schema_source_map; pub mod span; +pub(crate) mod spec_urls; pub mod type_builders; pub mod types; pub(crate) mod validators; diff --git a/crates/libgraphql-core-v1/src/schema/schema_builder.rs b/crates/libgraphql-core-v1/src/schema/schema_builder.rs index d7cbe35..92d3529 100644 --- a/crates/libgraphql-core-v1/src/schema/schema_builder.rs +++ b/crates/libgraphql-core-v1/src/schema/schema_builder.rs @@ -16,6 +16,7 @@ use crate::schema::schema_errors::SchemaErrors; use crate::schema_source_map::SchemaSourceMap; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::conversion_helpers::enum_value_from_builder; use crate::type_builders::conversion_helpers::field_def_from_builder; @@ -113,7 +114,8 @@ impl SchemaBuilder { /// Seeds the five built-in scalar types: `Boolean`, `Float`, /// `ID`, `Int`, `String`. /// - /// See [Built-in Scalars](https://spec.graphql.org/September2025/#sec-Scalars.Built-in-Scalars). + /// See [Built-in + /// Scalars](https://spec.graphql.org/September2025/#sec-Scalars.Built-in-Scalars). fn seed_builtin_scalars(&mut self) { for (kind, name) in [ (ScalarKind::Boolean, "Boolean"), @@ -138,7 +140,10 @@ impl SchemaBuilder { /// Seeds the five built-in directives: `@skip`, `@include`, /// `@deprecated`, `@specifiedBy`, `@oneOf`. /// - /// See [Built-in Directives](https://spec.graphql.org/September2025/#sec-Type-System.Directives.Built-in-Directives). + /// See [Built-in Directives][built-in-directives]. + /// + /// [built-in-directives]: + /// https://spec.graphql.org/September2025/#sec-Type-System.Directives.Built-in-Directives fn seed_builtin_directives(&mut self) { // @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT self.directive_defs.insert( @@ -294,6 +299,7 @@ impl SchemaBuilder { // Check duplicate if let Some(existing) = self.types.get(&name) { + // https://spec.graphql.org/September2025/#sec-Schema return Err(SchemaBuildError::new( SchemaBuildErrorKind::DuplicateTypeDefinition { type_name: name.to_string(), @@ -304,6 +310,7 @@ impl SchemaBuilder { "first defined here", existing.span(), ), + ErrorNote::spec(spec_urls::SCHEMA), ], )); } @@ -341,7 +348,12 @@ impl SchemaBuilder { // Reject redefinition of built-in directives if let Some(existing) = self.directive_defs.get(&name) { if existing.is_builtin() { - // https://spec.graphql.org/September2025/#sec-Type-System.Directives + // "All directives within a GraphQL schema must + // have unique names" -- built-in directives are + // provided by the implementation, so redefining + // one always collides. + // + // https://spec.graphql.org/September2025/#sec-Schema return Err(SchemaBuildError::new( SchemaBuildErrorKind::RedefinitionOfBuiltinDirective { name: name.to_string(), @@ -352,9 +364,11 @@ impl SchemaBuilder { "first defined here", existing.span(), ), + ErrorNote::spec(spec_urls::SCHEMA), ], )); } + // https://spec.graphql.org/September2025/#sec-Schema return Err(SchemaBuildError::new( SchemaBuildErrorKind::DuplicateDirectiveDefinition { name: name.to_string(), @@ -365,6 +379,7 @@ impl SchemaBuilder { "first defined here", existing.span(), ), + ErrorNote::spec(spec_urls::SCHEMA), ], )); } @@ -408,6 +423,9 @@ impl SchemaBuilder { ) { Ok(id) => SourceMapId(id), Err(_) => { + // No spec note: this is an implementation limit + // of this crate (u16 source-map IDs), not a rule + // from the GraphQL specification. return Err(vec![SchemaBuildError::new( SchemaBuildErrorKind::SourceMapLimitExceeded, Span::builtin(), @@ -451,6 +469,10 @@ impl SchemaBuilder { span: note_span, } }).collect(); + // No schema-level spec note: parse errors + // are grammar-level, and any notes (spec + // ones included) are supplied by the parser + // and translated verbatim above. SchemaBuildError::new( SchemaBuildErrorKind::ParseError { message: e.message().to_string(), @@ -591,7 +613,8 @@ impl SchemaBuilder { /// `extend schema { ... }` extension. Rebinding an /// already-bound root operation kind is an error. /// - /// See [Root Operation Types](https://spec.graphql.org/September2025/#sec-Root-Operation-Types). + /// See [Root Operation + /// Types](https://spec.graphql.org/September2025/#sec-Root-Operation-Types). fn load_root_operations( &mut self, root_operations: &[ast::RootOperationTypeDefinition<'_>], @@ -808,13 +831,14 @@ impl SchemaBuilder { }; if !self.types.contains_key(&query_type_name) { + // https://spec.graphql.org/September2025/#sec-Root-Operation-Types self.errors.push(SchemaBuildError::new( SchemaBuildErrorKind::NoQueryOperationTypeDefined, self.query_type_name .as_ref() .map(|(_, span)| *span) .unwrap_or(Span::builtin()), - vec![], + vec![ErrorNote::spec(spec_urls::ROOT_OPERATION_TYPES)], )); } @@ -852,46 +876,66 @@ impl SchemaBuilder { match graphql_type { GraphQLType::Object(obj) => { if obj.fields().is_empty() { + // https://spec.graphql.org/September2025/#sec-Objects.Type-Validation self.errors.push(SchemaBuildError::new( SchemaBuildErrorKind::EmptyObjectOrInterfaceType { type_kind: graphql_type.type_kind(), type_name: obj.name().to_string(), }, obj.span(), - vec![], + vec![ + ErrorNote::spec( + spec_urls::OBJECTS_TYPE_VALIDATION, + ), + ], )); } }, GraphQLType::Interface(iface) => { if iface.fields().is_empty() { + // https://spec.graphql.org/September2025/#sec-Interfaces.Type-Validation self.errors.push(SchemaBuildError::new( SchemaBuildErrorKind::EmptyObjectOrInterfaceType { type_kind: graphql_type.type_kind(), type_name: iface.name().to_string(), }, iface.span(), - vec![], + vec![ + ErrorNote::spec( + spec_urls::INTERFACES_TYPE_VALIDATION, + ), + ], )); } }, GraphQLType::Union(union_t) => { if union_t.members().is_empty() { + // https://spec.graphql.org/September2025/#sec-Unions.Type-Validation self.errors.push(SchemaBuildError::new( SchemaBuildErrorKind::EmptyUnionType { type_name: union_t.name().to_string(), }, union_t.span(), - vec![], + vec![ + ErrorNote::spec( + spec_urls::UNIONS_TYPE_VALIDATION, + ), + ], )); } }, GraphQLType::Enum(enum_t) if enum_t.values().is_empty() => { + // https://spec.graphql.org/September2025/#sec-Enums.Type-Validation self.errors.push(SchemaBuildError::new( SchemaBuildErrorKind::EnumWithNoValues { type_name: enum_t.name().to_string(), }, enum_t.span(), - vec![], + vec![ + ErrorNote::spec( + spec_urls::ENUMS_TYPE_VALIDATION, + ), + ], )); }, _ => {}, @@ -942,13 +986,17 @@ impl SchemaBuilder { ); validation_errors.extend(directive_errs); - // Wrap TypeValidationErrors into SchemaBuildErrors. + // Wrap TypeValidationErrors into SchemaBuildErrors, + // propagating the inner notes (every TypeValidationError + // carries its own spec-reference note) so that + // SchemaBuildError::notes() surfaces them uniformly. for tve in validation_errors { let span = tve.span(); + let notes = tve.notes().to_vec(); self.errors.push(SchemaBuildError::new( SchemaBuildErrorKind::TypeValidation(tve), span, - vec![], + notes, )); } @@ -1002,18 +1050,24 @@ impl SchemaBuilder { // mutation/subscription. Query missing is handled // separately via NoQueryOperationTypeDefined. if operation != OperationKind::Query { + // https://spec.graphql.org/September2025/#sec-Root-Operation-Types self.errors.push(SchemaBuildError::new( SchemaBuildErrorKind::RootOperationTypeNotDefined { operation, type_name: name.to_string(), }, span, - vec![], + vec![ + ErrorNote::spec( + spec_urls::ROOT_OPERATION_TYPES, + ), + ], )); } return; }; if !matches!(graphql_type, GraphQLType::Object(_)) { + // https://spec.graphql.org/September2025/#sec-Root-Operation-Types self.errors.push(SchemaBuildError::new( SchemaBuildErrorKind::RootOperationTypeNotObjectType { actual_kind: graphql_type.type_kind(), @@ -1021,7 +1075,7 @@ impl SchemaBuilder { type_name: name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::ROOT_OPERATION_TYPES)], )); } } @@ -1154,7 +1208,7 @@ fn merge_fielded_type_extension( type_name: data.name.to_string(), }, field.span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); continue; } @@ -1188,7 +1242,8 @@ fn merge_fielded_type_extension( /// appended (duplicates rejected, `__`-prefixed field names /// rejected) and extension directives are appended. /// -/// See [Input Object Extensions](https://spec.graphql.org/September2025/#sec-Input-Object-Extensions). +/// See [Input Object +/// Extensions](https://spec.graphql.org/September2025/#sec-Input-Object-Extensions). fn merge_input_object_type_extension( input_object_type: &mut InputObjectType, ext: PendingInputObjectTypeExtension, @@ -1204,7 +1259,7 @@ fn merge_input_object_type_extension( type_name: input_object_type.name.to_string(), }, field.span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); continue; } 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 ce8aed8..efbdff8 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 @@ -1232,3 +1232,232 @@ fn build_deprecated_required_directive_argument_rejected() { "expected DeprecatedRequiredDirectiveParameter, got: {errors:?}", ); } + +// ----------------------------------------------------------- +// Spec-note coverage on build-level errors (Task 16.6d) +// ----------------------------------------------------------- + +// Verifies that building a schema with no Query root operation +// type produces a NoQueryOperationTypeDefined error carrying a +// spec-reference note pointing at the root operation type +// rules ("The query root operation type must be provided and +// must be an Object type"). +// +// See https://spec.graphql.org/September2025/#sec-Root-Operation-Types +// +// Written by Claude Code, reviewed by a human. +#[test] +fn no_query_operation_type_error_has_spec_note() { + let errors = SchemaBuilder::build_from_str( + "type Foo { x: Int }", + ).unwrap_err(); + let err = errors.errors().iter().find(|e| { + matches!( + e.kind(), + SchemaBuildErrorKind::NoQueryOperationTypeDefined, + ) + }).expect("expected NoQueryOperationTypeDefined error"); + assert!( + err.notes().iter().any(|n| { + n.kind == ErrorNoteKind::Spec + && n.message.contains("#sec-Root-Operation-Types") + }), + "expected a #sec-Root-Operation-Types spec note, got: {:?}", + err.notes(), + ); +} + +// Verifies that when build() wraps a TypeValidationError into a +// SchemaBuildError, the inner error's notes (which always +// include a spec-reference note per Task 13's convention) are +// propagated onto the wrapping SchemaBuildError so that +// SchemaBuildError::notes() surfaces them uniformly. +// +// See https://spec.graphql.org/September2025/#IsValidImplementation() +// +// Written by Claude Code, reviewed by a human. +#[test] +fn type_validation_errors_propagate_notes_to_schema_build_error() { + let errors = SchemaBuilder::build_from_str( + "type Query implements NonExistent { x: Int }", + ).unwrap_err(); + let err = errors.errors().iter().find(|e| { + matches!(e.kind(), SchemaBuildErrorKind::TypeValidation(_)) + }).expect("expected a TypeValidation error"); + let SchemaBuildErrorKind::TypeValidation(tve) = err.kind() else { + unreachable!("find() matched TypeValidation above"); + }; + assert_eq!( + err.notes(), + tve.notes(), + "wrapper notes should mirror the inner error's notes", + ); + assert!( + err.notes().iter().any(|n| n.kind == ErrorNoteKind::Spec), + "expected a propagated spec note, got: {:?}", + err.notes(), + ); +} + +// Sweeping meta-test locking in the Task 16.6d convention that +// EVERY build-level SchemaBuildError carries at least one +// spec-reference note. Builds a kitchen-sink invalid schema +// that triggers (at least) one error from every build-level +// error family that can be reached through source text -- +// duplicate definitions, reserved `__` names (definition, +// from_ast, and extension-merge paths), root-operation-type +// violations, empty types, extension errors, and wrapped +// type-validation errors -- then asserts that every returned +// error carries a Spec note. +// +// Documented exceptions (cannot co-occur with these errors and +// deliberately carry no schema-level spec note): +// - ParseError: grammar-level; notes are supplied by the parser +// (load_str() returns parse errors early, so none can appear +// in this build). +// - SourceMapLimitExceeded: an implementation limit of this +// crate, not a GraphQL spec rule. +// +// See https://spec.graphql.org/September2025/#sec-Schema and +// the per-rule anchors referenced from each construction site. +// +// Written by Claude Code, reviewed by a human. +#[test] +fn all_build_level_errors_carry_spec_notes() { + let source = "\ + schema { query: BadRoot, mutation: MissingMutation }\n\ + schema { query: Whatever }\n\ + enum BadRoot { A }\n\ + type Dup { x: Int }\n\ + type Dup { x: Int }\n\ + type __BadType { x: Int }\n\ + scalar __BadScalar\n\ + interface __BadIface { x: Int }\n\ + union __BadUnion = Dup\n\ + enum __BadEnum { A }\n\ + input __BadInput { a: Int }\n\ + directive @__badDir on FIELD\n\ + type DupField { x: Int, x: Int }\n\ + type DunderField { __x: Int }\n\ + type DupArg { f(a: Int, a: Int): Int }\n\ + type DunderArg { f(__a: Int): Int }\n\ + interface SelfImpl implements SelfImpl { x: Int }\n\ + interface Base { id: ID }\n\ + type ImplTwice implements Base & Base { id: ID }\n\ + union DupMember = Dup | Dup\n\ + enum DupValue { A A }\n\ + input DupInputField { a: Int, a: Int }\n\ + directive @skip on FIELD\n\ + directive @custom on FIELD\n\ + directive @custom on FIELD\n\ + type EmptyObj\n\ + interface EmptyIface\n\ + union EmptyUnion\n\ + enum EmptyEnum\n\ + extend type NeverDefined { y: Int }\n\ + extend enum Dup { X }\n\ + extend type Dup { __z: Int }\n\ + input InputTarget { a: Int }\n\ + extend input InputTarget { __w: Int }\n\ + type BadImpl implements Undefined { x: Int }\n\ + "; + let errors = SchemaBuilder::build_from_str(source).unwrap_err(); + let kinds: Vec<_> = errors.errors().iter() + .map(|e| e.kind()) + .collect(); + + macro_rules! assert_has_kind { + ($pat:pat) => { + assert!( + kinds.iter().any(|k| matches!(k, $pat)), + "expected an error matching {}, got: {kinds:?}", + stringify!($pat), + ); + }; + } + + // Every previously-noteless build-level error family fires + // at least once. + assert_has_kind!(SchemaBuildErrorKind::DuplicateTypeDefinition { .. }); + assert_has_kind!( + SchemaBuildErrorKind::DuplicateDirectiveDefinition { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::RedefinitionOfBuiltinDirective { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::DuplicateOperationDefinition { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::RootOperationTypeNotDefined { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::RootOperationTypeNotObjectType { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::InvalidDunderPrefixedTypeName { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::InvalidDunderPrefixedFieldName { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::InvalidDunderPrefixedParamName { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::InvalidDunderPrefixedDirectiveName { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::DuplicateFieldNameDefinition { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::DuplicateParameterDefinition { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::InvalidSelfImplementingInterface { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::DuplicateInterfaceImplementsDeclaration { .. } + ); + assert_has_kind!(SchemaBuildErrorKind::DuplicateUnionMember { .. }); + assert_has_kind!( + SchemaBuildErrorKind::DuplicateEnumValueDefinition { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::EmptyObjectOrInterfaceType { + type_kind: GraphQLTypeKind::Object, + .. + } + ); + assert_has_kind!( + SchemaBuildErrorKind::EmptyObjectOrInterfaceType { + type_kind: GraphQLTypeKind::Interface, + .. + } + ); + assert_has_kind!(SchemaBuildErrorKind::EmptyUnionType { .. }); + assert_has_kind!(SchemaBuildErrorKind::EnumWithNoValues { .. }); + assert_has_kind!( + SchemaBuildErrorKind::ExtensionOfUndefinedType { .. } + ); + assert_has_kind!( + SchemaBuildErrorKind::InvalidExtensionTypeKind { .. } + ); + assert_has_kind!(SchemaBuildErrorKind::TypeValidation(_)); + + // The convention itself: every error carries >= 1 Spec note. + for err in errors.errors() { + assert!( + !matches!( + err.kind(), + SchemaBuildErrorKind::ParseError { .. }, + ), + "kitchen-sink source should parse cleanly", + ); + assert!( + err.notes().iter().any(|n| n.kind == ErrorNoteKind::Spec), + "error missing a spec note: {:?} (notes: {:?})", + err.kind(), + err.notes(), + ); + } +} diff --git a/crates/libgraphql-core-v1/src/spec_urls.rs b/crates/libgraphql-core-v1/src/spec_urls.rs new file mode 100644 index 0000000..8d620cf --- /dev/null +++ b/crates/libgraphql-core-v1/src/spec_urls.rs @@ -0,0 +1,82 @@ +//! Centralized GraphQL specification (September 2025 edition) +//! URLs attached to build-level errors as +//! [`ErrorNote::spec`](crate::error_note::ErrorNote::spec) notes. +//! +//! Every error-construction site also carries an inline `//` +//! comment with the same URL directly above it (per project +//! convention). Rules that are enforced from multiple sites +//! reference these shared constants so the note values cannot +//! drift apart. + +/// Grammar rule for enum values: a `Name`, but not `true`, +/// `false`, or `null`. +/// +/// +pub(crate) const ENUM_VALUE: &str = + "https://spec.graphql.org/September2025/#sec-Enum-Value"; + +/// Enum type validation rules (e.g. "An Enum type must define +/// one or more unique enum values"). +/// +/// +pub(crate) const ENUMS_TYPE_VALIDATION: &str = + "https://spec.graphql.org/September2025/#sec-Enums.Type-Validation"; + +/// Input object type validation rules (unique input field +/// names, one or more input fields, etc). +/// +/// +pub(crate) const INPUT_OBJECTS_TYPE_VALIDATION: &str = + "https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation"; + +/// Interface type validation rules (unique field names, one or +/// more fields, no self-implementation, etc). +/// +/// +pub(crate) const INTERFACES_TYPE_VALIDATION: &str = + "https://spec.graphql.org/September2025/#sec-Interfaces.Type-Validation"; + +/// Object type validation rules (unique field names, one or +/// more fields, unique argument names, unique `implements` +/// declarations, etc). +/// +/// +pub(crate) const OBJECTS_TYPE_VALIDATION: &str = + "https://spec.graphql.org/September2025/#sec-Objects.Type-Validation"; + +/// Reserved names rule: names must not begin with `__` (two +/// underscores) outside of the introspection system. +/// +/// +pub(crate) const RESERVED_NAMES: &str = + "https://spec.graphql.org/September2025/#sec-Names.Reserved-Names"; + +/// Root operation type rules ("The query root operation type +/// must be provided and must be an Object type", etc). +/// +/// +pub(crate) const ROOT_OPERATION_TYPES: &str = + "https://spec.graphql.org/September2025/#sec-Root-Operation-Types"; + +/// Schema-level validity rules ("All types within a GraphQL +/// schema must have unique names", "All directives within a +/// GraphQL schema must have unique names", etc). +/// +/// +pub(crate) const SCHEMA: &str = + "https://spec.graphql.org/September2025/#sec-Schema"; + +/// Directive definition type validation rules (unique argument +/// names, no `__`-prefixed names, etc). +/// +/// +pub(crate) const TYPE_SYSTEM_DIRECTIVES_TYPE_VALIDATION: &str = + "https://spec.graphql.org/September2025/\ + #sec-Type-System.Directives.Type-Validation"; + +/// Union type validation rules (one or more unique member +/// types, etc). +/// +/// +pub(crate) const UNIONS_TYPE_VALIDATION: &str = + "https://spec.graphql.org/September2025/#sec-Unions.Type-Validation"; diff --git a/crates/libgraphql-core-v1/src/type_builders/directive_builder.rs b/crates/libgraphql-core-v1/src/type_builders/directive_builder.rs index 8c4dfae..6b2f445 100644 --- a/crates/libgraphql-core-v1/src/type_builders/directive_builder.rs +++ b/crates/libgraphql-core-v1/src/type_builders/directive_builder.rs @@ -1,8 +1,10 @@ +use crate::error_note::ErrorNote; use crate::names::DirectiveName; use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::parameter_def_builder::ParameterDefBuilder; use crate::types::DirectiveLocationKind; @@ -11,7 +13,8 @@ use libgraphql_parser::ast; /// Builder for constructing a /// [`DirectiveDefinition`](crate::types::DirectiveDefinition). /// -/// See [Type System Directives](https://spec.graphql.org/September2025/#sec-Type-System.Directives). +/// See [Type System +/// Directives](https://spec.graphql.org/September2025/#sec-Type-System.Directives). #[derive(Debug)] pub struct DirectiveBuilder { pub(crate) description: Option, @@ -41,7 +44,7 @@ impl DirectiveBuilder { name: name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } Ok(Self { @@ -96,7 +99,7 @@ impl DirectiveBuilder { type_name: None, }, param.span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } if self.parameters.iter().any(|p| p.name == param.name) { @@ -108,7 +111,11 @@ impl DirectiveBuilder { type_name: None, }, param.span, - vec![], + vec![ + ErrorNote::spec( + spec_urls::TYPE_SYSTEM_DIRECTIVES_TYPE_VALIDATION, + ), + ], )); } self.parameters.push(param); @@ -147,7 +154,7 @@ impl DirectiveBuilder { ast_helpers::span_from_ast( ast_dir.name.span, source_map_id, ), - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } for param in &ast_dir.arguments { diff --git a/crates/libgraphql-core-v1/src/type_builders/enum_type_builder.rs b/crates/libgraphql-core-v1/src/type_builders/enum_type_builder.rs index cd5e35a..e37b868 100644 --- a/crates/libgraphql-core-v1/src/type_builders/enum_type_builder.rs +++ b/crates/libgraphql-core-v1/src/type_builders/enum_type_builder.rs @@ -1,9 +1,11 @@ use crate::directive_annotation::DirectiveAnnotation; +use crate::error_note::ErrorNote; use crate::names::TypeName; use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::conversion_helpers::enum_value_from_builder; use crate::type_builders::enum_value_def_builder::EnumValueDefBuilder; @@ -44,7 +46,7 @@ impl EnumTypeBuilder { type_name: name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } Ok(Self { @@ -76,14 +78,14 @@ impl EnumTypeBuilder { || name_str == "false" || name_str == "null" { - // https://spec.graphql.org/September2025/#sec-Enums.Type-Validation + // https://spec.graphql.org/September2025/#sec-Enum-Value return Err(SchemaBuildError::new( SchemaBuildErrorKind::InvalidEnumValueName { type_name: self.name.to_string(), value_name: value.name.to_string(), }, value.span, - vec![], + vec![ErrorNote::spec(spec_urls::ENUM_VALUE)], )); } if self.values.iter().any(|v| v.name == value.name) { @@ -94,7 +96,7 @@ impl EnumTypeBuilder { value_name: value.name.to_string(), }, value.span, - vec![], + vec![ErrorNote::spec(spec_urls::ENUMS_TYPE_VALIDATION)], )); } self.values.push(value); @@ -139,7 +141,7 @@ impl EnumTypeBuilder { ast_helpers::span_from_ast( ast_enum.name.span, source_map_id, ), - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } for dir in &ast_enum.directives { diff --git a/crates/libgraphql-core-v1/src/type_builders/field_def_builder.rs b/crates/libgraphql-core-v1/src/type_builders/field_def_builder.rs index dfbd9db..4ed1c1e 100644 --- a/crates/libgraphql-core-v1/src/type_builders/field_def_builder.rs +++ b/crates/libgraphql-core-v1/src/type_builders/field_def_builder.rs @@ -1,9 +1,11 @@ use crate::directive_annotation::DirectiveAnnotation; +use crate::error_note::ErrorNote; use crate::names::FieldName; use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::parameter_def_builder::ParameterDefBuilder; use crate::types::TypeAnnotation; @@ -74,7 +76,7 @@ impl FieldDefBuilder { type_name: None, }, param.span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } if self.parameters.iter().any(|p| p.name == param.name) { @@ -86,7 +88,7 @@ impl FieldDefBuilder { type_name: None, }, param.span, - vec![], + vec![ErrorNote::spec(spec_urls::OBJECTS_TYPE_VALIDATION)], )); } self.parameters.push(param); diff --git a/crates/libgraphql-core-v1/src/type_builders/input_object_type_builder.rs b/crates/libgraphql-core-v1/src/type_builders/input_object_type_builder.rs index 1abf7fe..c6c56be 100644 --- a/crates/libgraphql-core-v1/src/type_builders/input_object_type_builder.rs +++ b/crates/libgraphql-core-v1/src/type_builders/input_object_type_builder.rs @@ -1,9 +1,11 @@ use crate::directive_annotation::DirectiveAnnotation; +use crate::error_note::ErrorNote; use crate::names::TypeName; use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::conversion_helpers::input_field_from_builder; use crate::type_builders::input_field_def_builder::InputFieldDefBuilder; @@ -44,7 +46,7 @@ impl InputObjectTypeBuilder { type_name: name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } Ok(Self { @@ -79,7 +81,7 @@ impl InputObjectTypeBuilder { type_name: self.name.to_string(), }, field.span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } if self.fields.iter().any(|f| f.name == field.name) { @@ -90,7 +92,11 @@ impl InputObjectTypeBuilder { type_name: self.name.to_string(), }, field.span, - vec![], + vec![ + ErrorNote::spec( + spec_urls::INPUT_OBJECTS_TYPE_VALIDATION, + ), + ], )); } self.fields.push(field); @@ -135,7 +141,7 @@ impl InputObjectTypeBuilder { ast_helpers::span_from_ast( ast_input.name.span, source_map_id, ), - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } for dir in &ast_input.directives { diff --git a/crates/libgraphql-core-v1/src/type_builders/interface_type_builder.rs b/crates/libgraphql-core-v1/src/type_builders/interface_type_builder.rs index 1a187b9..190d623 100644 --- a/crates/libgraphql-core-v1/src/type_builders/interface_type_builder.rs +++ b/crates/libgraphql-core-v1/src/type_builders/interface_type_builder.rs @@ -1,10 +1,12 @@ use crate::directive_annotation::DirectiveAnnotation; +use crate::error_note::ErrorNote; use crate::located::Located; use crate::names::TypeName; use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::conversion_helpers::field_def_from_builder; use crate::type_builders::field_def_builder::FieldDefBuilder; @@ -47,7 +49,7 @@ impl InterfaceTypeBuilder { type_name: name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } Ok(Self { @@ -83,7 +85,7 @@ impl InterfaceTypeBuilder { type_name: self.name.to_string(), }, field.span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } if self.fields.iter().any(|f| f.name == field.name) { @@ -94,7 +96,7 @@ impl InterfaceTypeBuilder { type_name: self.name.to_string(), }, field.span, - vec![], + vec![ErrorNote::spec(spec_urls::INTERFACES_TYPE_VALIDATION)], )); } self.fields.push(field); @@ -116,7 +118,7 @@ impl InterfaceTypeBuilder { interface_name: self.name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::INTERFACES_TYPE_VALIDATION)], )); } if self.implements.iter().any(|l| l.value == iface) { @@ -127,7 +129,7 @@ impl InterfaceTypeBuilder { type_name: self.name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::INTERFACES_TYPE_VALIDATION)], )); } self.implements.push(Located { value: iface, span }); @@ -173,7 +175,7 @@ impl InterfaceTypeBuilder { ast_helpers::span_from_ast( ast_iface.name.span, source_map_id, ), - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } for iface in &ast_iface.implements { diff --git a/crates/libgraphql-core-v1/src/type_builders/object_type_builder.rs b/crates/libgraphql-core-v1/src/type_builders/object_type_builder.rs index 87ed246..d319646 100644 --- a/crates/libgraphql-core-v1/src/type_builders/object_type_builder.rs +++ b/crates/libgraphql-core-v1/src/type_builders/object_type_builder.rs @@ -1,10 +1,12 @@ use crate::directive_annotation::DirectiveAnnotation; +use crate::error_note::ErrorNote; use crate::located::Located; use crate::names::TypeName; use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::conversion_helpers::field_def_from_builder; use crate::type_builders::field_def_builder::FieldDefBuilder; @@ -47,7 +49,7 @@ impl ObjectTypeBuilder { type_name: name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } Ok(Self { @@ -83,7 +85,7 @@ impl ObjectTypeBuilder { type_name: self.name.to_string(), }, field.span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } if self.fields.iter().any(|f| f.name == field.name) { @@ -94,7 +96,7 @@ impl ObjectTypeBuilder { type_name: self.name.to_string(), }, field.span, - vec![], + vec![ErrorNote::spec(spec_urls::OBJECTS_TYPE_VALIDATION)], )); } self.fields.push(field); @@ -117,7 +119,7 @@ impl ObjectTypeBuilder { type_name: self.name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::OBJECTS_TYPE_VALIDATION)], )); } self.implements.push(Located { value: iface, span }); @@ -163,7 +165,7 @@ impl ObjectTypeBuilder { ast_helpers::span_from_ast( ast_obj.name.span, source_map_id, ), - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } for iface in &ast_obj.implements { diff --git a/crates/libgraphql-core-v1/src/type_builders/scalar_type_builder.rs b/crates/libgraphql-core-v1/src/type_builders/scalar_type_builder.rs index f33dc20..73b8602 100644 --- a/crates/libgraphql-core-v1/src/type_builders/scalar_type_builder.rs +++ b/crates/libgraphql-core-v1/src/type_builders/scalar_type_builder.rs @@ -1,9 +1,11 @@ use crate::directive_annotation::DirectiveAnnotation; +use crate::error_note::ErrorNote; use crate::names::TypeName; use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::into_graphql_type::IntoGraphQLType; use crate::types::GraphQLType; @@ -42,7 +44,7 @@ impl ScalarTypeBuilder { type_name: name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } Ok(Self { @@ -99,7 +101,7 @@ impl ScalarTypeBuilder { ast_helpers::span_from_ast( ast_scalar.name.span, source_map_id, ), - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } for dir in &ast_scalar.directives { diff --git a/crates/libgraphql-core-v1/src/type_builders/tests/builder_validation_tests.rs b/crates/libgraphql-core-v1/src/type_builders/tests/builder_validation_tests.rs index 5e2d2ff..3468133 100644 --- a/crates/libgraphql-core-v1/src/type_builders/tests/builder_validation_tests.rs +++ b/crates/libgraphql-core-v1/src/type_builders/tests/builder_validation_tests.rs @@ -1,9 +1,12 @@ +use crate::error_note::ErrorNoteKind; +use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::Span; use crate::type_builders::DirectiveBuilder; use crate::type_builders::EnumTypeBuilder; use crate::type_builders::EnumValueDefBuilder; use crate::type_builders::FieldDefBuilder; +use crate::type_builders::InputFieldDefBuilder; use crate::type_builders::InputObjectTypeBuilder; use crate::type_builders::InterfaceTypeBuilder; use crate::type_builders::ObjectTypeBuilder; @@ -368,3 +371,308 @@ fn field_add_parameter_rejects_duplicate() { SchemaBuildErrorKind::DuplicateParameterDefinition { .. }, )); } + +// ----------------------------------------------------------- +// Spec-note coverage (Task 16.6d) +// ----------------------------------------------------------- + +// Asserts that `err` carries at least one spec-reference note +// whose URL contains the given anchor fragment. +fn assert_spec_note(err: &SchemaBuildError, anchor: &str) { + assert!( + err.notes().iter().any(|n| { + n.kind == ErrorNoteKind::Spec && n.message.contains(anchor) + }), + "expected a spec note containing `{anchor}`, got: {:?}", + err.notes(), + ); +} + +// Verifies that every ObjectTypeBuilder error path attaches a +// spec-reference note pointing at the rule it enforces: +// reserved `__` names -> Reserved Names; duplicate fields and +// duplicate `implements` declarations -> Objects Type +// Validation. +// +// See https://spec.graphql.org/September2025/#sec-Names.Reserved-Names +// and https://spec.graphql.org/September2025/#sec-Objects.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn object_builder_errors_carry_spec_notes() { + let err = ObjectTypeBuilder::new( + "__Bad", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + let mut builder = ObjectTypeBuilder::new( + "User", Span::builtin(), + ).unwrap(); + let err = builder.add_field(FieldDefBuilder::new( + "__bad", + TypeAnnotation::named("String", /* nullable = */ true), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + builder.add_field(FieldDefBuilder::new( + "id", + TypeAnnotation::named("ID", /* nullable = */ false), + Span::builtin(), + )).unwrap(); + let err = builder.add_field(FieldDefBuilder::new( + "id", + TypeAnnotation::named("ID", /* nullable = */ false), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Objects.Type-Validation"); + + builder.add_implements("Node", Span::builtin()).unwrap(); + let err = builder.add_implements( + "Node", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Objects.Type-Validation"); +} + +// Verifies that every FieldDefBuilder error path attaches a +// spec-reference note: reserved `__` parameter names -> +// Reserved Names; duplicate parameter names -> Objects Type +// Validation (rule 2.4: unique argument names per field). +// +// See https://spec.graphql.org/September2025/#sec-Names.Reserved-Names +// and https://spec.graphql.org/September2025/#sec-Objects.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn field_def_builder_errors_carry_spec_notes() { + let mut builder = FieldDefBuilder::new( + "users", + TypeAnnotation::named("User", /* nullable = */ true), + Span::builtin(), + ); + let err = builder.add_parameter(ParameterDefBuilder::new( + "__bad", + TypeAnnotation::named("Int", /* nullable = */ true), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + builder.add_parameter(ParameterDefBuilder::new( + "first", + TypeAnnotation::named("Int", /* nullable = */ true), + Span::builtin(), + )).unwrap(); + let err = builder.add_parameter(ParameterDefBuilder::new( + "first", + TypeAnnotation::named("Int", /* nullable = */ true), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Objects.Type-Validation"); +} + +// Verifies that every InterfaceTypeBuilder error path attaches +// a spec-reference note: reserved `__` names -> Reserved Names; +// duplicate fields, self-implementation, and duplicate +// `implements` declarations -> Interfaces Type Validation. +// +// See https://spec.graphql.org/September2025/#sec-Names.Reserved-Names +// and https://spec.graphql.org/September2025/#sec-Interfaces.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn interface_builder_errors_carry_spec_notes() { + let err = InterfaceTypeBuilder::new( + "__Bad", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + let mut builder = InterfaceTypeBuilder::new( + "Node", Span::builtin(), + ).unwrap(); + let err = builder.add_field(FieldDefBuilder::new( + "__bad", + TypeAnnotation::named("String", /* nullable = */ true), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + builder.add_field(FieldDefBuilder::new( + "id", + TypeAnnotation::named("ID", /* nullable = */ false), + Span::builtin(), + )).unwrap(); + let err = builder.add_field(FieldDefBuilder::new( + "id", + TypeAnnotation::named("ID", /* nullable = */ false), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Interfaces.Type-Validation"); + + let err = builder.add_implements( + "Node", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Interfaces.Type-Validation"); + + builder.add_implements("Base", Span::builtin()).unwrap(); + let err = builder.add_implements( + "Base", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Interfaces.Type-Validation"); +} + +// Verifies that every UnionTypeBuilder error path attaches a +// spec-reference note: reserved `__` names -> Reserved Names; +// duplicate members -> Unions Type Validation ("one or more +// unique member types"). +// +// See https://spec.graphql.org/September2025/#sec-Names.Reserved-Names +// and https://spec.graphql.org/September2025/#sec-Unions.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn union_builder_errors_carry_spec_notes() { + let err = UnionTypeBuilder::new( + "__Bad", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + let mut builder = UnionTypeBuilder::new( + "SearchResult", Span::builtin(), + ).unwrap(); + builder.add_member("User", Span::builtin()).unwrap(); + let err = builder.add_member( + "User", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Unions.Type-Validation"); +} + +// Verifies that every EnumTypeBuilder error path attaches a +// spec-reference note: reserved `__` names -> Reserved Names; +// `true`/`false`/`null` value names -> the EnumValue grammar +// rule ("Name but not true, false or null"); duplicate values +// -> Enums Type Validation ("one or more unique enum values"). +// +// See https://spec.graphql.org/September2025/#sec-Names.Reserved-Names, +// https://spec.graphql.org/September2025/#sec-Enum-Value, and +// https://spec.graphql.org/September2025/#sec-Enums.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn enum_builder_errors_carry_spec_notes() { + let err = EnumTypeBuilder::new( + "__Bad", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + let mut builder = EnumTypeBuilder::new( + "Status", Span::builtin(), + ).unwrap(); + let err = builder.add_value( + EnumValueDefBuilder::new("null", Span::builtin()), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Enum-Value"); + + builder.add_value( + EnumValueDefBuilder::new("ACTIVE", Span::builtin()), + ).unwrap(); + let err = builder.add_value( + EnumValueDefBuilder::new("ACTIVE", Span::builtin()), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Enums.Type-Validation"); +} + +// Verifies that every InputObjectTypeBuilder error path +// attaches a spec-reference note: reserved `__` names -> +// Reserved Names; duplicate input fields -> Input Objects Type +// Validation. +// +// See https://spec.graphql.org/September2025/#sec-Names.Reserved-Names +// and https://spec.graphql.org/September2025/#sec-Input-Objects.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn input_object_builder_errors_carry_spec_notes() { + let err = InputObjectTypeBuilder::new( + "__Bad", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + let mut builder = InputObjectTypeBuilder::new( + "CreateInput", Span::builtin(), + ).unwrap(); + let err = builder.add_field(InputFieldDefBuilder::new( + "__bad", + TypeAnnotation::named("String", /* nullable = */ true), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + builder.add_field(InputFieldDefBuilder::new( + "name", + TypeAnnotation::named("String", /* nullable = */ false), + Span::builtin(), + )).unwrap(); + let err = builder.add_field(InputFieldDefBuilder::new( + "name", + TypeAnnotation::named("String", /* nullable = */ false), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Input-Objects.Type-Validation"); +} + +// Verifies that ScalarTypeBuilder::new() attaches a +// spec-reference note when rejecting a reserved `__` name. +// +// See https://spec.graphql.org/September2025/#sec-Names.Reserved-Names +// +// Written by Claude Code, reviewed by a human. +#[test] +fn scalar_builder_errors_carry_spec_notes() { + let err = ScalarTypeBuilder::new( + "__Bad", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); +} + +// Verifies that every DirectiveBuilder error path attaches a +// spec-reference note: reserved `__` directive/parameter names +// -> Reserved Names; duplicate parameters -> the Directives +// Type Validation rules (rule 5.2: unique argument names). +// +// See https://spec.graphql.org/September2025/#sec-Names.Reserved-Names +// and +// https://spec.graphql.org/September2025/#sec-Type-System.Directives.Type-Validation +// +// Written by Claude Code, reviewed by a human. +#[test] +fn directive_builder_errors_carry_spec_notes() { + let err = DirectiveBuilder::new( + "__bad", Span::builtin(), + ).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + let mut builder = DirectiveBuilder::new( + "auth", Span::builtin(), + ).unwrap(); + builder.add_location(DirectiveLocationKind::FieldDefinition); + let err = builder.add_parameter(ParameterDefBuilder::new( + "__bad", + TypeAnnotation::named("String", /* nullable = */ true), + Span::builtin(), + )).unwrap_err(); + assert_spec_note(&err, "#sec-Names.Reserved-Names"); + + builder.add_parameter(ParameterDefBuilder::new( + "role", + TypeAnnotation::named("String", /* nullable = */ true), + Span::builtin(), + )).unwrap(); + let err = builder.add_parameter(ParameterDefBuilder::new( + "role", + TypeAnnotation::named("String", /* nullable = */ true), + Span::builtin(), + )).unwrap_err(); + assert_spec_note( + &err, "#sec-Type-System.Directives.Type-Validation", + ); +} diff --git a/crates/libgraphql-core-v1/src/type_builders/union_type_builder.rs b/crates/libgraphql-core-v1/src/type_builders/union_type_builder.rs index e0d4fff..8d72319 100644 --- a/crates/libgraphql-core-v1/src/type_builders/union_type_builder.rs +++ b/crates/libgraphql-core-v1/src/type_builders/union_type_builder.rs @@ -1,10 +1,12 @@ use crate::directive_annotation::DirectiveAnnotation; +use crate::error_note::ErrorNote; use crate::located::Located; use crate::names::TypeName; use crate::schema::SchemaBuildError; use crate::schema::SchemaBuildErrorKind; use crate::span::SourceMapId; use crate::span::Span; +use crate::spec_urls; use crate::type_builders::ast_helpers; use crate::type_builders::into_graphql_type::IntoGraphQLType; use crate::types::GraphQLType; @@ -43,7 +45,7 @@ impl UnionTypeBuilder { type_name: name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } Ok(Self { @@ -79,7 +81,7 @@ impl UnionTypeBuilder { type_name: self.name.to_string(), }, span, - vec![], + vec![ErrorNote::spec(spec_urls::UNIONS_TYPE_VALIDATION)], )); } self.members.push(Located { value: member, span }); @@ -124,7 +126,7 @@ impl UnionTypeBuilder { ast_helpers::span_from_ast( ast_union.name.span, source_map_id, ), - vec![], + vec![ErrorNote::spec(spec_urls::RESERVED_NAMES)], )); } for member in &ast_union.members { diff --git a/libgraphql-core-v1-plan.md b/libgraphql-core-v1-plan.md index 9f5251a..e001b16 100644 --- a/libgraphql-core-v1-plan.md +++ b/libgraphql-core-v1-plan.md @@ -3945,10 +3945,34 @@ each; 3 extension-path regressions). Task 13's "every validation error includes a `Spec` note" only landed for the 4 type validators; all `SchemaBuildError`s constructed in `schema_builder.rs` carry empty notes and no spec-URL comments. -- [ ] Add `ErrorNote::spec` + inline spec-URL comment at every build-level +- [x] Add `ErrorNote::spec` + inline spec-URL comment at every build-level error-construction site -- [ ] Tests assert notes present -- [ ] Commit: `[libgraphql-core-v1] Add spec notes to build-level errors` +- [x] Tests assert notes present +- [x] Commit: `[libgraphql-core-v1] Add spec notes to build-level errors` + +**Completion Notes (16.6d):** 41 sites gained `ErrorNote::spec` (+ spec-URL comment +where missing): 30 across `type_builders/*.rs` (new/add_*/from_ast paths) + 11 in +`schema_builder.rs` (duplicates, root-op checks, empty types, extension-merge dunder +sites). New crate-private `spec_urls` module holds shared URL consts for rules fired +from multiple sites (Reserved-Names alone: 17 sites) so notes can't diverge; literal +URL still in the `//` comment above each site per convention. `TypeValidation` +wrapper in `build()` now clones the inner `TypeValidationError` notes onto the +`SchemaBuildError` so `notes()` surfaces Task-13 spec notes uniformly. Anchor +decisions: duplicate type/directive defs + `RedefinitionOfBuiltinDirective` → +`#sec-Schema` ("All types/directives ... must have unique names"; builtin-redef +comment updated from `#sec-Type-System.Directives`); `InvalidEnumValueName` → +`#sec-Enum-Value` (grammar "Name but not `true`/`false`/`null`"; previous +`#sec-Enums.Type-Validation` comment only covers uniqueness); `FieldDefBuilder` +param errors → `#sec-Objects.Type-Validation` (builder shared w/ interfaces; +identical rule text there). Deliberate no-spec-note exceptions (inline-commented): +`ParseError` (grammar-level; parser-supplied notes translated verbatim) and +`SourceMapLimitExceeded` (crate impl limit, not a spec rule). Anchors verified +against the September2025 tag of graphql-spec Sections 2–3 (raw GitHub; spec site +403s from env). 11 tests added: 8 per-builder spec-note tests, NoQuery-note test, +TypeValidation note-propagation test, and `all_build_level_errors_carry_spec_notes` +— a kitchen-sink invalid schema asserting every returned `SchemaBuildError` (22 +distinct kinds triggered, object+interface empty-type cases asserted separately) +carries ≥1 `Spec` note, permanently locking the convention. **16.6e — Hygiene + small missing APIs:** - [ ] Fix stale rustdoc: `validators/mod.rs` ("build() is currently todo!()" — false), From fb8f39610a6728e8cbfaba20d94686cd71110a38 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 03:11:04 +0000 Subject: [PATCH 2/2] [libgraphql-core-v1] 16.6d review follow-ups: counts, meta-test caveat, notes doc - Correct the plan completion note's site count (42 = 30 + 12, not 41) - Replace the "permanently locking" claim with an accurate caveat: the meta-test guards kinds reachable from its fixed kitchen-sink source, so new error kinds must extend it alongside their own tests - Document on SchemaBuildError::notes() that TypeValidation errors mirror the inner TypeValidationError notes (read one or the other) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01TipQtpvuLuHjHmRsVEgwJw --- .../src/schema/schema_build_error.rs | 9 +++++++++ libgraphql-core-v1-plan.md | 12 +++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/libgraphql-core-v1/src/schema/schema_build_error.rs b/crates/libgraphql-core-v1/src/schema/schema_build_error.rs index 5d454a4..f2f0539 100644 --- a/crates/libgraphql-core-v1/src/schema/schema_build_error.rs +++ b/crates/libgraphql-core-v1/src/schema/schema_build_error.rs @@ -38,7 +38,16 @@ impl SchemaBuildError { } pub fn kind(&self) -> &SchemaBuildErrorKind { &self.kind } + + /// Notes attached to this error. + /// + /// For [`SchemaBuildErrorKind::TypeValidation`] errors these + /// MIRROR the inner [`TypeValidationError`]'s notes — read one + /// or the other, not both, or notes will appear duplicated. + /// + /// [`TypeValidationError`]: crate::schema::TypeValidationError pub fn notes(&self) -> &[ErrorNote] { &self.notes } + pub fn span(&self) -> Span { self.span } } diff --git a/libgraphql-core-v1-plan.md b/libgraphql-core-v1-plan.md index e001b16..bfabfd6 100644 --- a/libgraphql-core-v1-plan.md +++ b/libgraphql-core-v1-plan.md @@ -3950,10 +3950,10 @@ notes and no spec-URL comments. - [x] Tests assert notes present - [x] Commit: `[libgraphql-core-v1] Add spec notes to build-level errors` -**Completion Notes (16.6d):** 41 sites gained `ErrorNote::spec` (+ spec-URL comment -where missing): 30 across `type_builders/*.rs` (new/add_*/from_ast paths) + 11 in -`schema_builder.rs` (duplicates, root-op checks, empty types, extension-merge dunder -sites). New crate-private `spec_urls` module holds shared URL consts for rules fired +**Completion Notes (16.6d):** 42 sites gained `ErrorNote::spec` (+ spec-URL comment +where missing): 30 across `type_builders/*.rs` (new/add_*/from_ast paths) + 12 in +`schema_builder.rs` (3 duplicates, 3 root-op checks, 4 empty-type checks, 2 +extension-merge dunder sites). New crate-private `spec_urls` module holds shared URL consts for rules fired from multiple sites (Reserved-Names alone: 17 sites) so notes can't diverge; literal URL still in the `//` comment above each site per convention. `TypeValidation` wrapper in `build()` now clones the inner `TypeValidationError` notes onto the @@ -3972,7 +3972,9 @@ against the September2025 tag of graphql-spec Sections 2–3 (raw GitHub; spec s TypeValidation note-propagation test, and `all_build_level_errors_carry_spec_notes` — a kitchen-sink invalid schema asserting every returned `SchemaBuildError` (22 distinct kinds triggered, object+interface empty-type cases asserted separately) -carries ≥1 `Spec` note, permanently locking the convention. +carries ≥1 `Spec` note. NOTE: the meta-test guards the kinds reachable from its +fixed source — a future error kind not triggered by that schema escapes it, so new +error kinds must extend the kitchen-sink alongside their own tests. **16.6e — Hygiene + small missing APIs:** - [ ] Fix stale rustdoc: `validators/mod.rs` ("build() is currently todo!()" — false),