Skip to content

Commit

Permalink
Add experimental support for parsing variable definitions in fragments.
Browse files Browse the repository at this point in the history
Closes #313
  • Loading branch information
OlegIlyenko committed Feb 7, 2018
1 parent a7550d3 commit 8992744
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 15 deletions.
4 changes: 3 additions & 1 deletion src/main/scala/sangria/ast/QueryAst.scala
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ case class FragmentDefinition(
typeCondition: NamedType,
directives: Vector[Directive],
selections: Vector[Selection],
variables: Vector[VariableDefinition] = Vector.empty,
comments: Vector[Comment] = Vector.empty,
trailingComments: Vector[Comment] = Vector.empty,
position: Option[Position] = None) extends Definition with ConditionalFragment with WithDirectives with SelectionContainer {
Expand Down Expand Up @@ -607,11 +608,12 @@ object AstVisitor {
trailingComments.foreach(s loop(s))
breakOrSkip(onLeave(n))
}
case n @ FragmentDefinition(_, cond, dirs, sels, comment, trailingComments, _)
case n @ FragmentDefinition(_, cond, dirs, sels, vars, comment, trailingComments, _)
if (breakOrSkip(onEnter(n))) {
loop(cond)
dirs.foreach(d loop(d))
sels.foreach(s loop(s))
vars.foreach(s loop(s))
comment.foreach(s loop(s))
trailingComments.foreach(s loop(s))
breakOrSkip(onLeave(n))
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/sangria/macros/AstLiftable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ trait AstLiftable {
implicit def liftDefinition[T <: Definition]: Liftable[T] = Liftable {
case OperationDefinition(o, n, v, d, s, c, tc, p)
q"_root_.sangria.ast.OperationDefinition($o, $n, $v, $d, $s, $c, $tc, $p)"
case FragmentDefinition(n, t, d, s, c, tc, p)
q"_root_.sangria.ast.FragmentDefinition($n, $t, $d, $s, $c, $tc, $p)"
case FragmentDefinition(n, t, d, s, v, c, tc, p)
q"_root_.sangria.ast.FragmentDefinition($n, $t, $d, $s, $v, $c, $tc, $p)"

case DirectiveDefinition(n, a, l, desc, c, p)
q"_root_.sangria.ast.DirectiveDefinition($n, $a, $l, $desc, $c, $p)"
Expand Down
21 changes: 14 additions & 7 deletions src/main/scala/sangria/parser/QueryParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,8 @@ trait Operations extends PositionTracking { this: Parser with Tokens with Ignore

trait Fragments { this: Parser with Tokens with Ignored with Directives with Types with Operations

def experimentalFragmentVariables: Boolean

def FragmentSpread = rule { Comments ~ trackPos ~ Ellipsis ~ FragmentName ~ (Directives.? ~> (_ getOrElse Vector.empty)) ~>
((comment, pos, name, dirs) ast.FragmentSpread(name, dirs, comment, Some(pos))) }

Expand All @@ -413,8 +415,12 @@ trait Fragments { this: Parser with Tokens with Ignored with Directives with Typ

def Fragment = rule { Keyword("fragment") }

def FragmentDefinition = rule { Comments ~ trackPos ~ Fragment ~ FragmentName ~ TypeCondition ~ (Directives.? ~> (_ getOrElse Vector.empty)) ~ SelectionSet ~>
((comment, pos, name, typeCondition, dirs, sels) ast.FragmentDefinition(name, typeCondition, dirs, sels._1, comment, sels._2, Some(pos))) }
def FragmentDefinition = rule { Comments ~ trackPos ~ Fragment ~ FragmentName ~ ExperimentalFragmentVariables ~ TypeCondition ~ (Directives.? ~> (_ getOrElse Vector.empty)) ~ SelectionSet ~>
((comment, pos, name, vars, typeCondition, dirs, sels) ast.FragmentDefinition(name, typeCondition, dirs, sels._1, vars, comment, sels._2, Some(pos))) }

def ExperimentalFragmentVariables = rule {
test(experimentalFragmentVariables) ~ VariableDefinitions.? ~> (_ getOrElse Vector.empty) | push(Vector.empty)
}

def FragmentName = rule { !on ~ Name }

Expand Down Expand Up @@ -497,7 +503,7 @@ trait Types { this: Parser with Tokens with Ignored ⇒
}
}

class QueryParser private (val input: ParserInput, val legacyImplementsInterface: Boolean = false, val legacyEmptyFields: Boolean = false)
class QueryParser private (val input: ParserInput, val legacyImplementsInterface: Boolean = false, val legacyEmptyFields: Boolean = false, val experimentalFragmentVariables: Boolean = false)
extends Parser with Tokens with Ignored with Document with Operations with Fragments with Values with Directives with Types with TypeSystemDefinitions

object QueryParser {
Expand All @@ -506,13 +512,14 @@ object QueryParser {
@deprecated("Use new syntax: `type Foo implements Bar & Baz`", "1.4.0")
legacyImplementsInterface: Boolean = false,
@deprecated("Use new syntax: `type Foo` intead of legacy `type Foo {}`", "1.4.0")
legacyEmptyFields: Boolean = false
legacyEmptyFields: Boolean = false,
experimentalFragmentVariables: Boolean = false
)(implicit scheme: DeliveryScheme[ast.Document]): scheme.Result = {
parse(ParserInput(input), legacyImplementsInterface, legacyEmptyFields)(scheme)
parse(ParserInput(input), legacyImplementsInterface, legacyEmptyFields, experimentalFragmentVariables)(scheme)
}

def parse(input: ParserInput, legacyImplementsInterface: Boolean, legacyEmptyFields: Boolean)(implicit scheme: DeliveryScheme[ast.Document]): scheme.Result = {
val parser = new QueryParser(input, legacyImplementsInterface, legacyEmptyFields)
def parse(input: ParserInput, legacyImplementsInterface: Boolean, legacyEmptyFields: Boolean, experimentalFragmentVariables: Boolean)(implicit scheme: DeliveryScheme[ast.Document]): scheme.Result = {
val parser = new QueryParser(input, legacyImplementsInterface, legacyEmptyFields, experimentalFragmentVariables)

parser.Document.run() match {
case Success(res)
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/sangria/renderer/QueryRenderer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,9 @@ object QueryRenderer {
renderDirs(dirs, config, indent) +
renderSelections(sels, op, indent, config)

case fd @ FragmentDefinition(name, typeCondition, dirs, sels, _, _, _)
case fd @ FragmentDefinition(name, typeCondition, dirs, sels, vars, _, _, _)
renderComment(fd, prev, indent, config) +
indent.str + "fragment" + config.mandatorySeparator + name + config.mandatorySeparator + "on" +
indent.str + "fragment" + config.mandatorySeparator + name + renderVarDefs(vars, indent, config, withSep = false) + config.mandatorySeparator + "on" +
config.mandatorySeparator + typeCondition.name + config.separator +
renderDirs(dirs, config, indent) +
renderSelections(sels, fd, indent, config)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class FragmentsOnCompositeType extends ValidationRule {
case _
AstVisitorCommand.RightContinue
}
case ast.FragmentDefinition(name, cond, _, _, _, _, pos)
case ast.FragmentDefinition(name, cond, _, _, _, _, _, pos)
ctx.typeInfo.tpe match {
case Some(tpe) if !tpe.isInstanceOf[CompositeType[_]]
Left(Vector(FragmentOnNonCompositeErrorViolation(name, cond.name, ctx.sourceMapper, cond.position.toList)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class NoFragmentCycles extends ValidationRule {
}

override val onEnter: ValidationVisit = {
case fragmentDef @ ast.FragmentDefinition(fragmentName, _, _, _, _, _, _)
case fragmentDef @ ast.FragmentDefinition(fragmentName, _, _, _, _, _, _, _)
if (visitedFrags.contains(fragmentName)) AstVisitorCommand.RightSkip
else {
val errors = detectCycleRecursive(fragmentDef)
Expand Down
2 changes: 2 additions & 0 deletions src/test/scala/sangria/macros/LiteralMacroSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ class LiteralMacroSpec extends WordSpec with Matchers {
Some(Position(502, 19, 13))
)),
Vector.empty,
Vector.empty,
Vector(
Comment(" field in fragment!", Some(Position(506, 19, 17)))),
Some(Position(455, 18, 11))
Expand Down Expand Up @@ -611,6 +612,7 @@ class LiteralMacroSpec extends WordSpec with Matchers {
)),
Vector.empty,
Vector.empty,
Vector.empty,
Some(Position(1273, 45, 11))
),
OperationDefinition(
Expand Down
55 changes: 54 additions & 1 deletion src/test/scala/sangria/parser/QueryParserSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ class QueryParserSpec extends WordSpec with Matchers with StringMatchers {
Some(Position(356, 18, 3))
)),
Vector.empty,
Vector.empty,
Vector(
Comment(" field in fragment!", Some(Position(360, 18, 7)))),
Some(Position(319, 17, 1))
Expand Down Expand Up @@ -534,6 +535,7 @@ class QueryParserSpec extends WordSpec with Matchers with StringMatchers {
)),
Vector.empty,
Vector.empty,
Vector.empty,
Some(Position(872, 44, 1))
),
OperationDefinition(
Expand Down Expand Up @@ -744,7 +746,7 @@ class QueryParserSpec extends WordSpec with Matchers with StringMatchers {
""")

error.formattedError should equal (
"""Invalid input 'T', expected TypeCondition (line 3, column 30):
"""Invalid input 'T', expected ExperimentalFragmentVariables or TypeCondition (line 3, column 30):
| fragment MissingOn Type
| ^""".stripMargin) (after being strippedOfCarriageReturns)
}
Expand Down Expand Up @@ -1401,6 +1403,7 @@ class QueryParserSpec extends WordSpec with Matchers with StringMatchers {
Comment(" comment 177", Some(Position(4783, 347, 5)))),
Some(Position(4454, 321, 3))
)),
Vector.empty,
Vector(
Comment(" comment 146", Some(Position(4228, 300, 3))),
Comment(" comment 147", Some(Position(4242, 301, 1)))),
Expand Down Expand Up @@ -1505,6 +1508,7 @@ class QueryParserSpec extends WordSpec with Matchers with StringMatchers {
)),
Vector.empty,
Vector.empty,
Vector.empty,
Some(Position(168, 8, 1))
)),
Vector.empty,
Expand All @@ -1514,6 +1518,55 @@ class QueryParserSpec extends WordSpec with Matchers with StringMatchers {

QueryParser.parse(query) map (_.withoutSourceMapper) should be (Success(expected))
}

"Experimental: allows parsing fragment defined variables" in {
val queryStr = "fragment a($v: Boolean = false) on t { f(v: $v) }"

QueryParser.parse(queryStr).isFailure should be (true)

val Success(query) = QueryParser.parse(queryStr, experimentalFragmentVariables = true)

query.withoutSourceMapper should be (
Document(
Vector(
FragmentDefinition(
"a",
NamedType("t", Some(Position(35, 1, 36))),
Vector.empty,
Vector(
Field(
None,
"f",
Vector(
Argument(
"v",
VariableValue("v", Vector.empty, Some(Position(44, 1, 45))),
Vector.empty,
Some(Position(41, 1, 42))
)),
Vector.empty,
Vector.empty,
Vector.empty,
Vector.empty,
Some(Position(39, 1, 40))
)),
Vector(
VariableDefinition(
"v",
NamedType("Boolean", Some(Position(15, 1, 16))),
Some(BooleanValue(false, Vector.empty, Some(Position(25, 1, 26)))),
Vector.empty,
Some(Position(11, 1, 12))
)),
Vector.empty,
Vector.empty,
Some(Position(0, 1, 1))
)),
Vector.empty,
Some(Position(0, 1, 1)),
None
))
}
}

"Ast" should {
Expand Down
18 changes: 18 additions & 0 deletions src/test/scala/sangria/renderer/QueryRendererSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,24 @@ class QueryRendererSpec extends WordSpec with Matchers with StringMatchers {

prettyRendered should equal (FileUtil loadQuery "block-string-rendered.graphql") (after being strippedOfCarriageReturns)
}

"Experimental: correctly prints fragment defined variables " in {
val query =
"""
fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
id
}
"""

QueryParser.parse(query).isFailure should be (true)

val Success(ast) = QueryParser.parse(query, experimentalFragmentVariables = true)

ast.renderPretty should equal (
"""fragment Foo($a: ComplexType, $b: Boolean = false) on TestType {
| id
|}""".stripMargin) (after being strippedOfCarriageReturns)
}
}

"rendering schema definition" should {
Expand Down

0 comments on commit 8992744

Please sign in to comment.