Skip to content

Commit

Permalink
feat: ConstValue in parser (#1057)
Browse files Browse the repository at this point in the history
GraphQL allows Values to appear in two different positions: labelled as
whether those positions are constant or not. Const position values
cannot have variables - for example in the schema where variables don't
exist.

This PR adds support for these constant values, based off the same
underlying value store but with the reader layer exposing a slightly
more limited API.
  • Loading branch information
obmarg authored Oct 3, 2024
1 parent cc1d87f commit 984fbb0
Show file tree
Hide file tree
Showing 45 changed files with 5,263 additions and 3,663 deletions.
3 changes: 2 additions & 1 deletion cynic-parser/ast-generator/domain/executable.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type Argument @file(name: "argument") {
type VariableDefinition @file(name: "variable") {
name: String!
ty: Type!
default_value: Value
default_value: ConstValue
directives: [Directive]
}

Expand All @@ -73,6 +73,7 @@ scalar OperationType @inline
# so we make them scalars and implement them by hand
scalar Type @file(name: "types")
scalar Value @file(name: "value")
scalar ConstValue @file(name: "value")

# String is built in, but easier to implement stuff if its just in the .graphql file
# It is also special cased a bit in the rust code
Expand Down
6 changes: 3 additions & 3 deletions cynic-parser/ast-generator/domain/type_system.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ type InputValueDefinition @file(name: "input_values") {
name: String!
ty: Type!
description: Description
default_value: Value
default_value: ConstValue
default_value_span: Span!
directives: [Directive!]!
span: Span!
Expand All @@ -107,7 +107,7 @@ type Directive @file(name: "directives") {

type Argument @file(name: "arguments") {
name: String!
value: Value!
value: ConstValue!
span: Span!
}

Expand All @@ -123,7 +123,7 @@ scalar OperationType @inline
# Type & Value are kind of special cases that aren't worth automating
# so we make them scalars and implement them by hand
scalar Type @file(name: "types")
scalar Value @file(name: "value")
scalar ConstValue @file(name: "value")

# String is built in, but easier to implement stuff if its just in the .graphql file
# It is also special cased a bit in the rust code
Expand Down
2 changes: 1 addition & 1 deletion cynic-parser/ast-generator/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub fn imports(
use super::{
#(#reader_imports)*
ids::{#(#id_imports)*},
ReadContext, #id_trait
#id_trait
};
})
}
21 changes: 18 additions & 3 deletions cynic-parser/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,11 @@ pub enum Error {
/// Malformed string literal
MalformedStringLiteral(crate::common::MalformedStringError),

/// Malformed string literal
/// Malformed directive location
MalformedDirectiveLocation(usize, String, usize),

/// Variable found in const position
VariableInConstPosition(usize, String, usize),
}

impl Error {
Expand All @@ -70,6 +73,7 @@ impl Error {
Error::Lexical(error) => error.span(),
Error::MalformedStringLiteral(error) => error.span(),
Error::MalformedDirectiveLocation(lhs, _, rhs) => Span::new(*lhs, *rhs),
Error::VariableInConstPosition(lhs, _, rhs) => Span::new(*lhs, *rhs),
}
}
}
Expand All @@ -81,9 +85,10 @@ impl std::error::Error for Error {
| Error::UnrecognizedEof { .. }
| Error::UnrecognizedToken { .. }
| Error::ExtraToken { .. }
| Error::MalformedDirectiveLocation(..) => None,
| Error::MalformedStringLiteral(..)
| Error::MalformedDirectiveLocation(..)
| Error::VariableInConstPosition(..) => None,
Error::Lexical(error) => Some(error),
Error::MalformedStringLiteral(error) => Some(error),
}
}
}
Expand Down Expand Up @@ -145,6 +150,13 @@ impl fmt::Display for Error {
}
Ok(())
}
Error::VariableInConstPosition(_, name, _) => {
write!(
f,
"the variable ${name} was found in a position that does not allow variables"
)?;
Ok(())
}
}
}
}
Expand Down Expand Up @@ -178,6 +190,9 @@ impl From<lalrpop_util::ParseError<usize, lexer::Token<'_>, AdditionalErrors>> f
ParseError::User {
error: AdditionalErrors::MalformedDirectiveLocation(lhs, location, rhs),
} => Error::MalformedDirectiveLocation(lhs, location, rhs),
ParseError::User {
error: AdditionalErrors::VariableInConstPosition(lhs, name, rhs),
} => Error::MalformedDirectiveLocation(lhs, name, rhs),
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions cynic-parser/src/errors/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,16 @@ impl Error {
None,
)
}

Error::VariableInConstPosition(_, _, _) => {
let span = self.span();
(
self.to_string(),
Label::new(span.start..span.end)
.with_message("only non-variable values can be used here"),
None,
)
}
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions cynic-parser/src/executable/generated.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{ids, iter::Iter, types, ExecutableId, ReadContext};
use super::{ids, iter::Iter, types, ExecutableId};

/// A prelude module for all the generated modules
///
Expand All @@ -13,7 +13,7 @@ mod prelude {
executable::{
ids::StringId,
iter::{IdReader, Iter},
ExecutableDocument,
ExecutableDocument, ReadContext,
},
AstLookup,
};
Expand All @@ -31,5 +31,5 @@ mod value {
// Note: This is just a requirement for some of the generated stuff
// that assumes it'll be here

pub use crate::values::Value;
pub use crate::values::{ConstValue, Value};
}
2 changes: 1 addition & 1 deletion cynic-parser/src/executable/generated/argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::prelude::*;
use super::{
ids::{ArgumentId, ValueId},
value::Value,
ExecutableId, ReadContext,
ExecutableId,
};
#[allow(unused_imports)]
use std::fmt::{self, Write};
Expand Down
2 changes: 1 addition & 1 deletion cynic-parser/src/executable/generated/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::prelude::*;
use super::{
argument::Argument,
ids::{ArgumentId, DirectiveId},
ExecutableId, ReadContext,
ExecutableId,
};
#[allow(unused_imports)]
use std::fmt::{self, Write};
Expand Down
2 changes: 1 addition & 1 deletion cynic-parser/src/executable/generated/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::{
directive::Directive,
ids::{DirectiveId, FragmentDefinitionId, SelectionId},
selections::Selection,
ExecutableId, ReadContext,
ExecutableId,
};
#[allow(unused_imports)]
use std::fmt::{self, Write};
Expand Down
2 changes: 1 addition & 1 deletion cynic-parser/src/executable/generated/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::{
ids::{DirectiveId, OperationDefinitionId, SelectionId, VariableDefinitionId},
selections::Selection,
variable::VariableDefinition,
ExecutableId, ReadContext,
ExecutableId,
};
#[allow(unused_imports)]
use std::fmt::{self, Write};
Expand Down
2 changes: 1 addition & 1 deletion cynic-parser/src/executable/generated/selections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::{
ids::{
ArgumentId, DirectiveId, FieldSelectionId, FragmentSpreadId, InlineFragmentId, SelectionId,
},
ExecutableId, ReadContext,
ExecutableId,
};
#[allow(unused_imports)]
use std::fmt::{self, Write};
Expand Down
10 changes: 5 additions & 5 deletions cynic-parser/src/executable/generated/variable.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
use super::prelude::*;
use super::{
directive::Directive,
ids::{DirectiveId, TypeId, ValueId, VariableDefinitionId},
ids::{ConstValueId, DirectiveId, TypeId, VariableDefinitionId},
types::Type,
value::Value,
ExecutableId, ReadContext,
value::ConstValue,
ExecutableId,
};
#[allow(unused_imports)]
use std::fmt::{self, Write};

pub struct VariableDefinitionRecord {
pub name: StringId,
pub ty: TypeId,
pub default_value: Option<ValueId>,
pub default_value: Option<ConstValueId>,
pub directives: IdRange<DirectiveId>,
}

Expand All @@ -28,7 +28,7 @@ impl<'a> VariableDefinition<'a> {
let document = self.0.document;
document.read(document.lookup(self.0.id).ty)
}
pub fn default_value(&self) -> Option<Value<'a>> {
pub fn default_value(&self) -> Option<ConstValue<'a>> {
let document = self.0.document;
document
.lookup(self.0.id)
Expand Down
4 changes: 3 additions & 1 deletion cynic-parser/src/executable/ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::num::NonZeroU32;
use super::{storage::*, types::TypeRecord, ExecutableDocument};
use crate::{common::IdRange, AstLookup};

pub use crate::values::ids::ValueId;
pub use crate::values::ids::{ConstValueId, ValueId};

macro_rules! make_id {
($name:ident, $output:ident, $field:ident) => {
Expand Down Expand Up @@ -70,7 +70,9 @@ make_id!(InlineFragmentId, InlineFragmentRecord, inline_fragments);
make_id!(FragmentSpreadId, FragmentSpreadRecord, fragment_spreads);

make_id!(DirectiveId, DirectiveRecord, directives);

make_id!(ArgumentId, ArgumentRecord, arguments);

impl_id_range_ops!(DirectiveId);
impl_id_range_ops!(ArgumentId);

Expand Down
8 changes: 8 additions & 0 deletions cynic-parser/src/executable/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,11 @@ impl ExecutableId for crate::values::ids::ValueId {
document.values.read(self)
}
}

impl ExecutableId for crate::values::ids::ConstValueId {
type Reader<'a> = crate::values::ConstValue<'a>;

fn read(self, document: &super::ExecutableDocument) -> Self::Reader<'_> {
document.values.read(self)
}
}
93 changes: 66 additions & 27 deletions cynic-parser/src/parser/executable.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
executable::{
storage::*, ids::*, writer::ExecutableAstWriter
},
values::{storage::*, ids::ValueId, self},
values::{storage::*, ids::{ValueId, ConstValueId}, self},
common::{
OperationType, IdRange, WrappingType, TypeWrappers,
unquote_string, unquote_block_string, trim_block_string_whitespace
Expand Down Expand Up @@ -67,8 +67,8 @@ VariableDefinition: () = {
}
}

DefaultValue: ValueId = {
"=" <value:Value> => {
DefaultValue: ConstValueId = {
"=" <value:ConstValue> => {
value
}
}
Expand Down Expand Up @@ -156,6 +156,40 @@ Argument: ArgumentRecord = {
}
}

ConstValue: ConstValueId = {
<record:ConstValueRecord> => {
ast.values.const_value(record)
}
}

ConstValueRecord: ValueRecord = {
<scalar:ScalarValueRecord> => scalar,
<start:@L> "[" <values:ConstValueRecord*> "]" <end:@R> => {
let id = ast.values.list(values);
ValueRecord {
span: Span::new(start, end),
kind: ValueKind::List(id)
}
},
<start:@L> "{" <fields:ConstObjectField*> "}" <end:@R> => {
let fields = ast.values.const_fields(fields);
ValueRecord {
span: Span::new(start, end),
kind: ValueKind::Object(fields)
}
},
}

ConstObjectField: (values::ids::StringId, Span, ConstValueId) = {
<name_start:@L> <name:Name> <name_end:@R> ":" <value:ConstValue> => {
(
values::ids::StringId::from_executable_id(name),
Span::new(name_start, name_end),
value
)
}
}

Value: ValueId = {
<record:ValueRecord> => {
ast.values.value(record)
Expand All @@ -169,6 +203,35 @@ ValueRecord: ValueRecord = {
kind: ValueKind::Variable(values::ids::StringId::from_executable_id(name))
}
},
<scalar:ScalarValueRecord> => scalar,
<start:@L> "[" <values:ValueRecord*> "]" <end:@R> => {
let id = ast.values.list(values);
ValueRecord {
span: Span::new(start, end),
kind: ValueKind::List(id)
}
},
<start:@L> "{" <fields:ObjectField*> "}" <end:@R> => {
let fields = ast.values.fields(fields);
ValueRecord {
span: Span::new(start, end),
kind: ValueKind::Object(fields)
}
},
}

ObjectField: (values::ids::StringId, Span, ValueId) = {
<name_start:@L> <name:Name> <name_end:@R> ":" <value:Value> => {
(
values::ids::StringId::from_executable_id(name),
Span::new(name_start, name_end),
value
)
}
}


ScalarValueRecord: ValueRecord = {
<start:@L> <int:IntegerLiteral> <end:@R> => {
ValueRecord {
span: Span::new(start, end),
Expand Down Expand Up @@ -217,20 +280,6 @@ ValueRecord: ValueRecord = {
kind: ValueKind::Null
}
},
<start:@L> "[" <values:ValueRecord*> "]" <end:@R> => {
let id = ast.values.list(values);
ValueRecord {
span: Span::new(start, end),
kind: ValueKind::List(id)
}
},
<start:@L> "{" <fields:ObjectField*> "}" <end:@R> => {
let fields = ast.values.fields(fields);
ValueRecord {
span: Span::new(start, end),
kind: ValueKind::Object(fields)
}
},
<start:@L> <value:EnumValue> <end:@R> => {
ValueRecord {
span: Span::new(start, end),
Expand All @@ -239,16 +288,6 @@ ValueRecord: ValueRecord = {
},
}

ObjectField: (values::ids::StringId, Span, ValueId) = {
<name_start:@L> <name:Name> <name_end:@R> ":" <value:Value> => {
(
values::ids::StringId::from_executable_id(name),
Span::new(name_start, name_end),
value
)
}
}

EnumValue: StringId = {
<s:RawIdent> => ast.ident(s),
schema => ast.ident("schema"),
Expand Down
Loading

0 comments on commit 984fbb0

Please sign in to comment.