[libgraphql-core-v1] Task 15: Validators (object/interface, union, input object, directive)#99
Conversation
…put object, directive)
There was a problem hiding this comment.
Pull request overview
Implements cross-type schema validation in libgraphql-core-v1 during SchemaBuilder::build(), adding per-type validators for object/interface implementation rules, unions, input objects, and directive definitions.
Changes:
- Added 4 validators (object/interface, union, input object, directive definition) plus comprehensive unit tests.
- Updated type-validation error display formatting for circular input field chains (wrapping identifiers in backticks).
- Exposed a new internal
validatorsmodule and updated the implementation plan notes.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| libgraphql-core-v1-plan.md | Marks validator work as completed and documents implementation notes. |
| crates/libgraphql-core-v1/src/lib.rs | Adds internal validators module to the crate. |
| crates/libgraphql-core-v1/src/validators/mod.rs | Declares validator submodules and re-exports validator entry points. |
| crates/libgraphql-core-v1/src/validators/object_or_interface_type_validator.rs | Validates interface implementation correctness and input/output type legality for fields/params. |
| crates/libgraphql-core-v1/src/validators/union_type_validator.rs | Validates union member existence and member type-kind constraints. |
| crates/libgraphql-core-v1/src/validators/input_object_type_validator.rs | Validates input field types and detects non-nullable circular input object reference chains. |
| crates/libgraphql-core-v1/src/validators/directive_definition_validator.rs | Validates custom directive parameter types are input types. |
| crates/libgraphql-core-v1/src/validators/tests/mod.rs | Wires up validator unit tests. |
| crates/libgraphql-core-v1/src/validators/tests/object_or_interface_type_validator_tests.rs | Adds test coverage for object/interface implementation validation rules. |
| crates/libgraphql-core-v1/src/validators/tests/union_type_validator_tests.rs | Adds test coverage for union member validation rules. |
| crates/libgraphql-core-v1/src/validators/tests/input_object_type_validator_tests.rs | Adds test coverage for input object field validation and cycle detection. |
| crates/libgraphql-core-v1/src/validators/tests/directive_definition_validator_tests.rs | Adds test coverage for directive definition parameter validation. |
| crates/libgraphql-core-v1/src/schema/type_validation_error.rs | Updates error formatting for circular input field chains. |
| crates/libgraphql-core-v1/src/schema/tests/type_validation_error_tests.rs | Adjusts tests to match updated error display formatting. |
Comments suppressed due to low confidence (1)
crates/libgraphql-core-v1/src/schema/type_validation_error.rs:66
CircularInputFieldChainDisplay wraps each path element in backticks, but the validator currently buildscircular_field_pathelements that already include backticks (e.g. "A" / "A.b"). This will produce double backticks in real error messages (e.g. "A"). Prefer storing raw path segments (no backticks) incircular_field_pathand only formatting in Display, or revert this Display change and keep formatting at the source consistently.
#[error(
"circular non-nullable input field chain: {}",
circular_field_path.iter().map(|t| format!("`{t}`")).collect::<Vec<_>>().join(" -> "),
)]
CircularInputFieldChain {
circular_field_path: Vec<String>,
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Code review by graphql-rust-reviewer (Opus 1M) — 3 bugs, 4 test gaps, 1 spec gap, 1 nit: Bugs (must fix):
Spec gap: Test gaps: Nit: |
…ng, directive param error, spec URLs, add tests
|
All 9 items from the review addressed:
Also fixed: all multi-line spec URLs consolidated to single lines, recursive child validator uses correct interface set. 174 tests pass. |
…add regression test rule to plan
jeffmo
left a comment
There was a problem hiding this comment.
Code review of Task 15: Validators.
Overall this is a solid implementation. The validators cover the key spec rules correctly, tests are well-structured with appropriate spec references, and the code is clean. I found 5 findings -- 1 bug, 2 test quality issues, and 2 style nits.
All tests pass, clippy is clean (with --tests), and spec references use the current September 2025 edition.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeffmo
left a comment
There was a problem hiding this comment.
Round 6 code review -- final sanity pass. All tests pass, clippy is clean, imports are sorted, enum variants are alphabetically ordered, spec URLs are September 2025. Reviewed all 22 changed .rs files. Two low-severity observations below; no bugs or spec compliance gaps found.
…ameterType, edit-distance suggestions, find_similar_names everywhere
0cabee3 to
7d7a9a2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…lan notes, update PR summary
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… avoid double allocation, fix test focus
Summary
Implements 4 type-system validators for cross-type validation of GraphQL schemas. These validators are designed to be called from
SchemaBuilder::build()(Task 16) but are not yet wired into the build pipeline —build()is currentlytodo!().ObjectOrInterfaceTypeValidator
T: HasFieldsAndInterfaces— validates both ObjectType and InterfaceTypeUnionTypeValidator
EmptyUnionType) is aSchemaBuildErrorKind— will be added inbuild()(Task 16)InputObjectTypeValidator
!is_input_type()instead ofas_object().is_some()— now correctly rejects Interface and Union types as input field typesDirectiveDefinitionValidator (NEW — absent in v0)
InvalidDirectiveParameterTypeerror variantRemoved: type_reference_validator
All type reference validation is distributed across the per-type validators — no separate pass needed.
Cross-cutting
Note: Validators are not yet wired into
SchemaBuilder::build()— that is Task 16.Test plan
cargo test --package libgraphql-core-v1)cargo clippy --tests --package libgraphql-core-v1 -- -D warningscleancargo check --tests --package libgraphql-core-v1clean🤖 Generated with Claude Code