Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Spanned arguments #1209

Merged
merged 5 commits into from
Nov 9, 2023
Merged

Spanned arguments #1209

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
71c6304
feat: Include Spanning in Arguments
audunhalland Mar 6, 2023
429cd09
feat: Include spanning in LookAheadArguments
audunhalland Mar 13, 2023
73b31ac
chore: Update CHANGELOG.md
audunhalland Nov 9, 2023
7c21d34
Remove `Spanning` for `meta::Argument::default_value`
tyranron Nov 9, 2023
cae5497
Bikeshed
tyranron Nov 9, 2023
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
124 changes: 89 additions & 35 deletions juniper/src/executor/look_ahead.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::HashMap;

use crate::{
ast::{Directive, Fragment, InputValue, Selection},
parser::Spanning,
parser::{Span, Spanning},
value::ScalarValue,
};

Expand All @@ -18,6 +18,8 @@ pub enum Applies<'a> {
OnlyType(&'a str),
}

type BorrowedSpanning<'a, T> = Spanning<T, &'a Span>;

/// A JSON-like value that can is used as argument in the query execution
///
/// In contrast to `InputValue` these values do only contain constants,
Expand All @@ -28,37 +30,57 @@ pub enum LookAheadValue<'a, S: 'a> {
Null,
Scalar(&'a S),
Enum(&'a str),
List(Vec<LookAheadValue<'a, S>>),
Object(Vec<(&'a str, LookAheadValue<'a, S>)>),
List(Vec<BorrowedSpanning<'a, LookAheadValue<'a, S>>>),
Object(
Vec<(
BorrowedSpanning<'a, &'a str>,
BorrowedSpanning<'a, LookAheadValue<'a, S>>,
)>,
),
}

impl<'a, S> LookAheadValue<'a, S>
where
S: ScalarValue,
{
fn from_input_value(input_value: &'a InputValue<S>, vars: &'a Variables<S>) -> Self {
match *input_value {
InputValue::Null => LookAheadValue::Null,
InputValue::Scalar(ref s) => LookAheadValue::Scalar(s),
InputValue::Enum(ref e) => LookAheadValue::Enum(e),
fn from_input_value(
input_value: BorrowedSpanning<'a, &'a InputValue<S>>,
vars: &'a Variables<S>,
) -> BorrowedSpanning<'a, Self> {
fn borrow_spanning<T>(span: &Span, item: T) -> BorrowedSpanning<'_, T> {
Spanning { item, span }
}

let span = &input_value.span;

match input_value.item {
InputValue::Null => borrow_spanning(span, LookAheadValue::Null),
InputValue::Scalar(ref s) => borrow_spanning(span, LookAheadValue::Scalar(s)),
InputValue::Enum(ref e) => borrow_spanning(span, LookAheadValue::Enum(e)),
InputValue::Variable(ref name) => vars
.get(name)
.map(|v| Self::from_input_value(v, vars))
.unwrap_or(LookAheadValue::Null),
InputValue::List(ref l) => LookAheadValue::List(
l.iter()
.map(|i| LookAheadValue::from_input_value(&i.item, vars))
.collect(),
.map(|v| Self::from_input_value(borrow_spanning(span, v), vars))
.unwrap_or(borrow_spanning(span, LookAheadValue::Null)),
InputValue::List(ref l) => borrow_spanning(
span,
LookAheadValue::List(
l.iter()
.map(|i| LookAheadValue::from_input_value(i.as_ref(), vars))
.collect(),
),
),
InputValue::Object(ref o) => LookAheadValue::Object(
o.iter()
.map(|(n, i)| {
(
&n.item as &str,
LookAheadValue::from_input_value(&i.item, vars),
)
})
.collect(),
InputValue::Object(ref o) => borrow_spanning(
span,
LookAheadValue::Object(
o.iter()
.map(|(n, i)| {
(
borrow_spanning(&n.span, n.item.as_str()),
LookAheadValue::from_input_value(i.as_ref(), vars),
)
})
.collect(),
),
),
}
}
Expand All @@ -68,7 +90,7 @@ where
#[derive(Debug, Clone, PartialEq)]
pub struct LookAheadArgument<'a, S: 'a> {
name: &'a str,
value: LookAheadValue<'a, S>,
value: BorrowedSpanning<'a, LookAheadValue<'a, S>>,
}

impl<'a, S> LookAheadArgument<'a, S>
Expand All @@ -81,7 +103,7 @@ where
) -> Self {
LookAheadArgument {
name: name.item,
value: LookAheadValue::from_input_value(&value.item, vars),
value: LookAheadValue::from_input_value(value.as_ref(), vars),
}
}

Expand All @@ -92,7 +114,12 @@ where

/// The value of the argument
pub fn value(&'a self) -> &LookAheadValue<'a, S> {
&self.value
&self.value.item
}

/// The input source span of the argument
pub fn span(&self) -> &Span {
self.value.span
}
}

Expand Down Expand Up @@ -145,7 +172,7 @@ where
.find(|item| item.0.item == "if")
.map(|(_, v)| {
if let LookAheadValue::Scalar(s) =
LookAheadValue::from_input_value(&v.item, vars)
LookAheadValue::from_input_value(v.as_ref(), vars).item
{
s.as_bool().unwrap_or(false)
} else {
Expand All @@ -160,7 +187,7 @@ where
.find(|item| item.0.item == "if")
.map(|(_, v)| {
if let LookAheadValue::Scalar(b) =
LookAheadValue::from_input_value(&v.item, vars)
LookAheadValue::from_input_value(v.as_ref(), vars).item
{
b.as_bool().map(::std::ops::Not::not).unwrap_or(false)
} else {
Expand Down Expand Up @@ -472,12 +499,12 @@ impl<'a, S> LookAheadMethods<'a, S> for LookAheadSelection<'a, S> {

#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::{collections::HashMap, ops::Range};

use crate::{
ast::{Document, OwnedDocument},
graphql_vars,
parser::UnlocatedParseResult,
parser::{SourcePosition, UnlocatedParseResult},
schema::model::SchemaType,
validation::test_harness::{MutationRoot, QueryRoot, SubscriptionRoot},
value::{DefaultScalarValue, ScalarValue},
Expand Down Expand Up @@ -509,6 +536,13 @@ mod tests {
fragments
}

fn span(range: Range<(usize, usize, usize)>) -> Span {
Span {
start: SourcePosition::new(range.start.0, range.start.1, range.start.2),
end: SourcePosition::new(range.end.0, range.end.1, range.end.2),
}
}

#[test]
fn check_simple_query() {
let docs = parse_document_source::<DefaultScalarValue>(
Expand Down Expand Up @@ -711,12 +745,17 @@ query Hero {
&fragments,
)
.unwrap();
let span0 = span((32, 2, 18)..(38, 2, 24));
let span1 = span((77, 4, 24)..(81, 4, 28));
let expected = LookAheadSelection {
name: "hero",
alias: None,
arguments: vec![LookAheadArgument {
name: "episode",
value: LookAheadValue::Enum("EMPIRE"),
value: Spanning {
item: LookAheadValue::Enum("EMPIRE"),
span: &span0,
},
}],
applies_for: Applies::All,
children: vec![
Expand All @@ -732,7 +771,10 @@ query Hero {
alias: None,
arguments: vec![LookAheadArgument {
name: "uppercase",
value: LookAheadValue::Scalar(&DefaultScalarValue::Boolean(true)),
value: Spanning {
item: LookAheadValue::Scalar(&DefaultScalarValue::Boolean(true)),
span: &span1,
},
}],
children: Vec::new(),
applies_for: Applies::All,
Expand Down Expand Up @@ -768,12 +810,16 @@ query Hero($episode: Episode) {
&fragments,
)
.unwrap();
let span0 = span((51, 2, 18)..(59, 2, 26));
let expected = LookAheadSelection {
name: "hero",
alias: None,
arguments: vec![LookAheadArgument {
name: "episode",
value: LookAheadValue::Enum("JEDI"),
value: Spanning {
item: LookAheadValue::Enum("JEDI"),
span: &span0,
},
}],
applies_for: Applies::All,
children: vec![
Expand Down Expand Up @@ -821,12 +867,16 @@ query Hero($episode: Episode) {
&fragments,
)
.unwrap();
let span0 = span((51, 2, 18)..(59, 2, 26));
let expected = LookAheadSelection {
name: "hero",
alias: None,
arguments: vec![LookAheadArgument {
name: "episode",
value: LookAheadValue::Null,
value: Spanning {
item: LookAheadValue::Null,
span: &span0,
},
}],
applies_for: Applies::All,
children: vec![LookAheadSelection {
Expand Down Expand Up @@ -1121,12 +1171,16 @@ fragment comparisonFields on Character {
&fragments,
)
.unwrap();
let span0 = span((85, 2, 11)..(88, 2, 14));
let expected = LookAheadSelection {
name: "hero",
alias: None,
arguments: vec![LookAheadArgument {
name: "id",
value: LookAheadValue::Scalar(&DefaultScalarValue::Int(42)),
value: Spanning {
item: LookAheadValue::Scalar(&DefaultScalarValue::Int(42)),
span: &span0,
},
}],
applies_for: Applies::All,
children: vec![
Expand Down
14 changes: 11 additions & 3 deletions juniper/src/parser/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ impl Span {

/// Data structure used to wrap items into a [`Span`].
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct Spanning<T> {
pub struct Spanning<T, Sp = Span> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added generic argument probably influences how this is documented, but not sure.

The point of the generic is to avoid making a separate type like BorrowedSpanning, but I'm up for improvement suggestions.

The ability to borrow a Span can lead to slightly more performant code in various places. Currently only in look_ahead, other future suggestions are e.g. the input validation rules.

/// Wrapped item.
pub item: T,

/// [`Span`] of the wrapped item.
pub span: Span,
pub span: Sp,
}

impl<T> Spanning<T> {
impl<T> Spanning<T, Span> {
#[doc(hidden)]
pub fn new(span: Span, item: T) -> Self {
Self { item, span }
Expand Down Expand Up @@ -126,6 +126,14 @@ impl<T> Spanning<T> {
pub fn and_then<O, F: Fn(T) -> Option<O>>(self, f: F) -> Option<Spanning<O>> {
f(self.item).map(|item| Spanning::new(self.span, item))
}

/// Make a Spanning that contains a borrowed item and a borrowed span.
pub(crate) fn as_ref(&self) -> Spanning<&'_ T, &'_ Span> {
Spanning {
item: &self.item,
span: &self.span,
}
}
}

impl<T: fmt::Display> fmt::Display for Spanning<T> {
Expand Down
6 changes: 3 additions & 3 deletions juniper/src/schema/meta.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
schema::model::SchemaType,
types::base::TypeKind,
value::{DefaultScalarValue, ParseScalarValue},
FieldError,
FieldError, Spanning,
};

/// Whether an item is deprecated, with context.
Expand Down Expand Up @@ -202,7 +202,7 @@ pub struct Argument<'a, S> {
#[doc(hidden)]
pub arg_type: Type<'a>,
#[doc(hidden)]
pub default_value: Option<InputValue<S>>,
pub default_value: Option<Spanning<InputValue<S>>>,
audunhalland marked this conversation as resolved.
Show resolved Hide resolved
}

impl<'a, S> Argument<'a, S> {
Expand Down Expand Up @@ -739,7 +739,7 @@ impl<'a, S> Argument<'a, S> {
/// Overwrites any previously set default value.
#[must_use]
pub fn default_value(mut self, val: InputValue<S>) -> Self {
self.default_value = Some(val);
self.default_value = Some(Spanning::unlocated(val));
self
}
}
Expand Down
2 changes: 1 addition & 1 deletion juniper/src/schema/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ impl<'a, S: ScalarValue + 'a> Argument<'a, S> {

#[graphql(name = "defaultValue")]
fn default_value_(&self) -> Option<String> {
self.default_value.as_ref().map(ToString::to_string)
self.default_value.as_ref().map(|v| v.item.to_string())
}
}

Expand Down
2 changes: 1 addition & 1 deletion juniper/src/schema/translate/graphql_parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ impl GraphQLParserTranslator {
default_value: input
.default_value
.as_ref()
.map(|x| GraphQLParserTranslator::translate_value(x)),
.map(|x| GraphQLParserTranslator::translate_value(&x.item)),
directives: vec![],
}
}
Expand Down
3 changes: 2 additions & 1 deletion juniper/src/types/async_await.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ where
m.item
.iter()
.filter_map(|(k, v)| {
v.item.clone().into_const(exec_vars).map(|v| (k.item, v))
let value = v.item.clone().into_const(exec_vars)?;
Some((k.item, Spanning::new(v.span, value)))
})
.collect()
}),
Expand Down
13 changes: 10 additions & 3 deletions juniper/src/types/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ pub enum TypeKind {
/// Field argument container
#[derive(Debug)]
pub struct Arguments<'a, S = DefaultScalarValue> {
args: Option<IndexMap<&'a str, InputValue<S>>>,
args: Option<IndexMap<&'a str, Spanning<InputValue<S>>>>,
}

impl<'a, S> Arguments<'a, S> {
#[doc(hidden)]
pub fn new(
mut args: Option<IndexMap<&'a str, InputValue<S>>>,
mut args: Option<IndexMap<&'a str, Spanning<InputValue<S>>>>,
meta_args: &'a Option<Vec<Argument<S>>>,
) -> Self
where
Expand Down Expand Up @@ -117,10 +117,16 @@ impl<'a, S> Arguments<'a, S> {
self.args
.as_ref()
.and_then(|args| args.get(name))
.map(|spanning| &spanning.item)
.map(InputValue::convert)
.transpose()
.map_err(IntoFieldError::into_field_error)
}

/// Gets a direct reference to the spanned argument input value
pub fn get_input_value(&self, name: &str) -> Option<&Spanning<InputValue<S>>> {
self.args.as_ref().and_then(|args| args.get(name))
}
}

/// Primary trait used to resolve GraphQL values.
Expand Down Expand Up @@ -472,7 +478,8 @@ where
m.item
.iter()
.filter_map(|(k, v)| {
v.item.clone().into_const(exec_vars).map(|v| (k.item, v))
let value = v.item.clone().into_const(exec_vars)?;
Some((k.item, Spanning::new(v.span, value)))
})
.collect()
}),
Expand Down
Loading
Loading