Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/libgraphql-core-v1/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub mod schema_source_map;
pub mod span;
pub mod type_builders;
pub mod types;
pub(crate) mod validators;
pub mod value;

pub use crate::located::Located;
Expand Down
3 changes: 2 additions & 1 deletion crates/libgraphql-core-v1/src/located.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ use crate::span::Span;
///
/// # Example
///
/// ```ignore
/// ```rust
/// # use libgraphql_core_v1 as libgraphql_core;
/// use libgraphql_core::Located;
/// use libgraphql_core::names::TypeName;
/// use libgraphql_core::span::Span;
Expand Down
3 changes: 2 additions & 1 deletion crates/libgraphql-core-v1/src/names/directive_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use std::borrow::Borrow;
///
/// # Construction
///
/// ```ignore
/// ```rust
/// # use libgraphql_core_v1 as libgraphql_core;
/// use libgraphql_core::names::DirectiveName;
///
/// let name = DirectiveName::new("deprecated");
Expand Down
3 changes: 2 additions & 1 deletion crates/libgraphql-core-v1/src/names/enum_value_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use std::borrow::Borrow;
///
/// # Construction
///
/// ```ignore
/// ```rust
/// # use libgraphql_core_v1 as libgraphql_core;
/// use libgraphql_core::names::EnumValueName;
///
/// let name = EnumValueName::new("ACTIVE");
Expand Down
3 changes: 2 additions & 1 deletion crates/libgraphql-core-v1/src/names/field_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use std::borrow::Borrow;
///
/// # Construction
///
/// ```ignore
/// ```rust
/// # use libgraphql_core_v1 as libgraphql_core;
/// use libgraphql_core::names::FieldName;
///
/// let name = FieldName::new("firstName");
Expand Down
3 changes: 2 additions & 1 deletion crates/libgraphql-core-v1/src/names/fragment_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use std::borrow::Borrow;
///
/// # Construction
///
/// ```ignore
/// ```rust
/// # use libgraphql_core_v1 as libgraphql_core;
/// use libgraphql_core::names::FragmentName;
///
/// let name = FragmentName::new("UserFields");
Expand Down
3 changes: 2 additions & 1 deletion crates/libgraphql-core-v1/src/names/type_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use std::borrow::Borrow;
///
/// # Construction
///
/// ```ignore
/// ```rust
/// # use libgraphql_core_v1 as libgraphql_core;
/// use libgraphql_core::names::TypeName;
///
/// let name = TypeName::new("User");
Expand Down
3 changes: 2 additions & 1 deletion crates/libgraphql-core-v1/src/names/variable_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ use std::borrow::Borrow;
///
/// # Construction
///
/// ```ignore
/// ```rust
/// # use libgraphql_core_v1 as libgraphql_core;
/// use libgraphql_core::names::VariableName;
///
/// let name = VariableName::new("userId");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,17 @@ fn builtin_directives_seeded() {
assert!(deprecated.is_builtin());
assert_eq!(deprecated.parameters().len(), 1);
assert!(deprecated.parameters().contains_key("reason"));
// Verify default value
// Verify default value and nullability.
// The `reason` parameter must be non-nullable (String!) per
// the September 2025 spec. A previous bug had it set to
// nullable (true) instead of non-nullable (false).
let reason_param = deprecated.parameters().get("reason")
.expect("reason param not found");
assert!(reason_param.default_value().is_some());
assert!(
!reason_param.type_annotation().nullable(),
"@deprecated reason must be non-nullable (String!)",
);

let specified_by = defs.get("specifiedBy")
.expect("@specifiedBy not found");
Expand Down Expand Up @@ -400,3 +407,41 @@ fn load_str_all_type_kinds() {
assert!(sb.types().contains_key(&TypeName::new("DateTime")));
assert!(sb.types().contains_key(&TypeName::new("CreateInput")));
}

// Regression test for a bug where parse errors returned by
// SchemaBuilder::load_str() carried un-translated spans
// (Span::builtin() with BUILTIN_SOURCE_MAP_ID) instead of
// spans pointing at the source map that was allocated for
// the loaded string. The effect was that diagnostics emitted
// for parse errors were effectively un-locatable in tooling
// because they did not reference the actual input source.
// This test asserts that load_str() now translates parse
// error spans so their source_map_id points at the loaded
// source.
//
// Written by Claude Code, reviewed by a human.
#[test]
fn load_str_parse_error_has_proper_span() {
let mut sb = SchemaBuilder::new();
let result = sb.load_str("type { broken }");
assert!(result.is_err());
let errors = match result {
Err(errs) => errs,
Ok(_) => panic!("expected parse error"),
};
assert!(!errors.is_empty());
assert!(matches!(
errors[0].kind(),
SchemaBuildErrorKind::ParseError { .. },
));
// The span's source_map_id must NOT be the built-in id
// (SourceMapId(0)). It should point to the source map
// created for the loaded string.
let span = errors[0].span();
assert_ne!(
span.source_map_id,
crate::span::BUILTIN_SOURCE_MAP_ID,
"parse error span should reference the loaded source, \
not Span::builtin()",
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn circular_input_field_chain_display() {
vec![],
);
let msg = error.to_string();
assert!(msg.contains("A -> B -> C"), "got: {msg}");
assert!(msg.contains("`A` -> `B` -> `C`"), "got: {msg}");
assert!(msg.contains("circular"), "got: {msg}");
}

Expand Down
12 changes: 11 additions & 1 deletion crates/libgraphql-core-v1/src/schema/type_validation_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl std::error::Error for TypeValidationError {
pub enum TypeValidationErrorKind {
#[error(
"circular non-nullable input field chain: {}",
circular_field_path.join(" -> "),
circular_field_path.iter().map(|t| format!("`{t}`")).collect::<Vec<_>>().join(" -> "),
)]
CircularInputFieldChain {
circular_field_path: Vec<String>,
Expand All @@ -85,6 +85,16 @@ pub enum TypeValidationErrorKind {
undefined_interface_name: String,
},

#[error(
"parameter `{parameter_name}` on directive `@{directive_name}` \
has type `{invalid_type_name}` which is not an input type"
)]
InvalidDirectiveParameterType {
directive_name: String,
invalid_type_name: String,
parameter_name: String,
},

#[error(
"input field `{parent_type_name}.{field_name}` has \
type `{invalid_type_name}` which is not an input type"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,43 @@ fn directive_add_parameter_rejects_duplicate() {
));
}

// Regression test for a bug where
// DirectiveBuilder::add_parameter() emitted the wrong error
// kind when rejecting a `__`-prefixed parameter name. It
// previously returned
// SchemaBuildErrorKind::InvalidDunderPrefixedDirectiveName
// (which describes an invalid directive NAME) when the actual
// problem was the PARAMETER name; the correct variant is
// SchemaBuildErrorKind::InvalidDunderPrefixedParamName. This
// test asserts the corrected behavior so the wrong-variant bug
// cannot reappear.
//
// https://spec.graphql.org/September2025/#sec-Names.Reserved-Names
// Written by Claude Code, reviewed by a human.
#[test]
fn directive_add_parameter_rejects_dunder_prefix() {
let mut builder = DirectiveBuilder::new(
"myDirective", 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!(
matches!(
err.kind(),
SchemaBuildErrorKind::InvalidDunderPrefixedParamName {
param_name,
..
} if param_name == "__bad"
),
"expected InvalidDunderPrefixedParamName, got: {:?}",
err.kind(),
);
}

// Verifies FieldDefBuilder::add_parameter() rejects duplicates.
// https://spec.graphql.org/September2025/#sec-Field-Arguments.Type-Validation
// Written by Claude Code, reviewed by a human.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use crate::error_note::ErrorNote;
use crate::names::DirectiveName;
use crate::names::TypeName;
use crate::schema::TypeValidationError;
use crate::schema::TypeValidationErrorKind;
use crate::types::DirectiveDefinition;
use crate::types::GraphQLType;
use crate::validators::edit_distance::find_similar_names;
use indexmap::IndexMap;

/// Validates custom directive definitions.
///
/// Checks that every parameter on a custom (non-builtin) directive
/// definition references a valid input type. Built-in directives
/// are skipped since they are validated by the spec itself.
///
/// See [Type System Directives](https://spec.graphql.org/September2025/#sec-Type-System.Directives).
pub(crate) fn validate_directive_definitions(
directive_defs: &IndexMap<DirectiveName, DirectiveDefinition>,
types_map: &IndexMap<TypeName, GraphQLType>,
) -> Vec<TypeValidationError> {
let mut errors = Vec::new();

for directive_def in directive_defs.values() {
// Only validate custom directives; built-in directives
// are spec-defined and assumed correct.
if directive_def.is_builtin() {
continue;
}

for (param_name, param) in directive_def.parameters() {
let innermost_type_name =
param.type_annotation().innermost_type_name();
let innermost_type =
types_map.get(innermost_type_name);

if let Some(innermost_type) = innermost_type {
// All directive parameters must be declared with
// an input type.
//
// https://spec.graphql.org/September2025/#sec-Type-System.Directives
if !innermost_type.is_input_type() {
errors.push(TypeValidationError::new(
TypeValidationErrorKind::InvalidDirectiveParameterType {
directive_name:
directive_def.name().to_string(),
invalid_type_name:
innermost_type_name.to_string(),
parameter_name:
param_name.to_string(),
},
param.type_annotation().span(),
vec![ErrorNote::spec(
"https://spec.graphql.org/September2025/#sec-Type-System.Directives",
)],
));
}
} else {
// https://spec.graphql.org/September2025/#sec-Type-System.Directives
let mut notes = Vec::new();
let suggestions = find_similar_names(
innermost_type_name.as_str(),
types_map.keys(),
);
if let Some(best) = suggestions.first() {
notes.push(ErrorNote::help(
format!("did you mean `{best}`?"),
));
}
notes.push(ErrorNote::spec(
"https://spec.graphql.org/September2025/#sec-Types",
));
errors.push(TypeValidationError::new(
TypeValidationErrorKind::UndefinedTypeName {
undefined_type_name:
innermost_type_name.to_string(),
},
param.type_annotation().span(),
notes,
));
}
}
}

errors
}
78 changes: 78 additions & 0 deletions crates/libgraphql-core-v1/src/validators/edit_distance/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use crate::names::TypeName;

/// Finds type names in `candidates` that are within a reasonable
/// edit distance of `name`. Returns at most 3 suggestions,
/// sorted by distance (best first).
///
/// The distance threshold is adaptive: `name.len() / 3 + 1`.
/// Shorter names require closer matches to avoid nonsensical
/// suggestions.
pub(crate) fn find_similar_names<'a>(
name: &str,
candidates: impl Iterator<Item = &'a TypeName>,
) -> Vec<&'a TypeName> {
Comment thread
jeffmo marked this conversation as resolved.
let max_distance = name.len() / 3 + 1;
let mut scored: Vec<(usize, &'a TypeName)> = candidates
.filter_map(|candidate| {
let dist =
levenshtein_distance(name, candidate.as_str());
if dist > 0 && dist <= max_distance {
Some((dist, candidate))
} else {
None
}
})
.collect();

scored.sort_by(|(d1, n1), (d2, n2)| {
d1.cmp(d2).then_with(|| n1.cmp(n2))
});
scored
.into_iter()
.take(3)
.map(|(_, name)| name)
.collect()
}

/// Computes the Levenshtein edit distance between two strings.
///
/// Uses the classic dynamic-programming algorithm with O(min(a,
/// b)) space via a single-row buffer.
fn levenshtein_distance(a: &str, b: &str) -> usize {
let mut a_chars: Vec<char> = a.chars().collect();
let mut b_chars: Vec<char> = b.chars().collect();

// Ensure `b_chars` is the shorter side for space efficiency.
if a_chars.len() < b_chars.len() {
std::mem::swap(&mut a_chars, &mut b_chars);
}

let a_len = a_chars.len();
let b_len = b_chars.len();

let mut prev_row: Vec<usize> =
(0..=b_len).collect();
let mut curr_row: Vec<usize> =
vec![0; b_len + 1];

for i in 1..=a_len {
curr_row[0] = i;
for j in 1..=b_len {
let cost =
if a_chars[i - 1] == b_chars[j - 1] {
0
} else {
1
};
curr_row[j] = (prev_row[j] + 1)
.min(curr_row[j - 1] + 1)
.min(prev_row[j - 1] + cost);
}
std::mem::swap(&mut prev_row, &mut curr_row);
}

prev_row[b_len]
}

#[cfg(test)]
mod tests;
Loading
Loading