graphql-rust / juniper

GraphQL server library for Rust
Other
5.72k stars 425 forks source link

Spanned arguments #1206

Closed audunhalland closed 1 year ago

audunhalland commented 1 year ago

This change improves diagnostics when performing manual/domain-specific post-validation of arguments using LookAheadArguments. In the case of an error, the produced error message may be able to refer to the exact position in the problematic GraphQL arguments.

I figure this breaking change can be seriously considered, as Juniper looks set for a breaking release anyway.

audunhalland commented 1 year ago

A change that could be done to improve this is to somehow borrow the (start, end) tuple in LookAheadArguments instead of copying it. But the problem is that start and end are two independent fields of Spanning<T>, and those two fields can't be borrowed together without also borrowing the item: T, resulting in a leaky abstraction.

Edit: I propose a redesigned Spanning like this:

struct Spanning<T> {
    pub item: T,
    pub span: Span
}

struct Span {
    pub start: SourcePosition,
    pub end: SourcePosition,
}
tyranron commented 1 year ago

@audunhalland could you provide some meaningful examples where do you want this?

audunhalland commented 1 year ago

@tyranron thank you for the review! I think the Spanning redesign should be a separate PR and I'll instead rebase this one.

audunhalland commented 1 year ago

That PR is up at https://github.com/graphql-rust/juniper/pull/1207

audunhalland commented 1 year ago

I'll close this PR and open a new one on top of https://github.com/graphql-rust/juniper/pull/1208 later.