Allow directives on variable definitions. Closes sangria-graphql#396

sangria-graphql-org · Oct 6, 2018 · 65ffe26 · 65ffe26
1 parent aa020cd
commit 65ffe26
Show file tree

Hide file tree

Showing 19 changed files with 100 additions and 36 deletions.
diff --git a/src/main/scala/sangria/ast/QueryAst.scala b/src/main/scala/sangria/ast/QueryAst.scala
@@ -198,8 +198,9 @@ case class VariableDefinition(
   name: String,
   tpe: Type,
   defaultValue: Option[Value],
+  directives: Vector[Directive] = Vector.empty,
   comments: Vector[Comment] = Vector.empty,
-  location: Option[AstLocation] = None) extends AstNode with WithComments
+  location: Option[AstLocation] = None) extends AstNode with WithComments with WithDirectives
 
 sealed trait Type extends AstNode {
   def namedType: NamedType = {
@@ -652,10 +653,11 @@ object AstVisitor {
             trailingComments.foreach(s ⇒ loop(s))
             breakOrSkip(onLeave(n))
           }
-        case n @ VariableDefinition(_, tpe, default, comment, _) ⇒
+        case n @ VariableDefinition(_, tpe, default, dirs, comment, _) ⇒
           if (breakOrSkip(onEnter(n))) {
             loop(tpe)
             default.foreach(d ⇒ loop(d))
+            dirs.foreach(d ⇒ loop(d))
             comment.foreach(s ⇒ loop(s))
             breakOrSkip(onLeave(n))
           }

diff --git a/src/main/scala/sangria/execution/batch/BatchExecutor.scala b/src/main/scala/sangria/execution/batch/BatchExecutor.scala
@@ -392,7 +392,7 @@ object BatchExecutor {
             if (violations.nonEmpty)
               inferenceViolations ++= violations
             else
-              newVariableDefs += ast.VariableDefinition(ud, firstAstType, None, Vector(ast.Comment("Inferred variable")))
+              newVariableDefs += ast.VariableDefinition(ud, firstAstType, None, Vector.empty, Vector(ast.Comment("Inferred variable")))
           }
 
           if (newVariableDefs.nonEmpty && inferenceViolations.isEmpty)

diff --git a/src/main/scala/sangria/introspection/package.scala b/src/main/scala/sangria/introspection/package.scala
@@ -65,6 +65,8 @@ package object introspection {
         description = Some("Location adjacent to a fragment spread.")),
       EnumValue("INLINE_FRAGMENT", value = DirectiveLocation.InlineFragment,
         description = Some("Location adjacent to an inline fragment.")),
+      EnumValue("VARIABLE_DEFINITION", value = DirectiveLocation.VariableDefinition,
+        description = Some("Location adjacent to a variable definition.")),
 
       EnumValue("SCHEMA", value = DirectiveLocation.Schema,
         description = Some("Location adjacent to a schema definition.")),

diff --git a/src/main/scala/sangria/macros/AstLiftable.scala b/src/main/scala/sangria/macros/AstLiftable.scala
@@ -41,8 +41,8 @@ trait AstLiftable {
   }
 
   implicit def liftVarDef: Liftable[VariableDefinition] = Liftable {
-    case VariableDefinition(n, t, d, c, p) ⇒
-      q"_root_.sangria.ast.VariableDefinition($n, $t, $d, $c, $p)"
+    case VariableDefinition(n, t, d, dirs, c, p) ⇒
+      q"_root_.sangria.ast.VariableDefinition($n, $t, $d, $dirs, $c, $p)"
   }
 
   implicit def liftInpValDef: Liftable[InputValueDefinition] = Liftable {

diff --git a/src/main/scala/sangria/parser/QueryParser.scala b/src/main/scala/sangria/parser/QueryParser.scala
@@ -354,7 +354,8 @@ trait TypeSystemDefinitions { this: Parser with Tokens with Ignored with Directi
     wsCapture("ENUM_VALUE") |
     wsCapture("ENUM") |
     wsCapture("INPUT_OBJECT") |
-    wsCapture("INPUT_FIELD_DEFINITION")
+    wsCapture("INPUT_FIELD_DEFINITION") |
+    wsCapture("VARIABLE_DEFINITION")
   }
 
   def SchemaDefinition = rule {
@@ -395,8 +396,8 @@ trait Operations extends PositionTracking { this: Parser with Tokens with Ignore
 
   def VariableDefinitions = rule { wsNoComment('(') ~ VariableDefinition.+ ~ wsNoComment(')') ~> (_.toVector)}
 
-  def VariableDefinition = rule { Comments ~ trackPos ~ Variable ~ ws(':') ~ Type ~ DefaultValue.? ~>
-      ((comment, location, name, tpe, defaultValue) ⇒ ast.VariableDefinition(name, tpe, defaultValue, comment, location)) }
+  def VariableDefinition = rule { Comments ~ trackPos ~ Variable ~ ws(':') ~ Type ~ DefaultValue.? ~ (DirectivesConst.? ~> (_ getOrElse Vector.empty)) ~>
+      ((comment, location, name, tpe, defaultValue, dirs) ⇒ ast.VariableDefinition(name, tpe, defaultValue, dirs, comment, location)) }
 
   def Variable = rule { Ignored.* ~ '$' ~ NameStrict }
 

diff --git a/src/main/scala/sangria/renderer/QueryRenderer.scala b/src/main/scala/sangria/renderer/QueryRenderer.scala
@@ -353,11 +353,12 @@ object QueryRenderer {
           renderDirs(dirs, config, indent) +
           renderSelections(sels, fd, indent, config)
 
-      case vd @ VariableDefinition(name, tpe, defaultValue, _, _) ⇒
+      case vd @ VariableDefinition(name, tpe, defaultValue, dirs, _, _) ⇒
         renderComment(vd, prev, indent, config) +
           indent.str + "$" + name + ":" + config.separator +
           renderNode(tpe, config, indent.zero) +
-          (defaultValue map (v ⇒ config.separator + "=" + config.separator + renderNode(v, config, indent.zero)) getOrElse "")
+          (defaultValue map (v ⇒ config.separator + "=" + config.separator + renderNode(v, config, indent.zero)) getOrElse "") +
+          renderDirs(dirs, config, indent, frontSep = true)
 
       case NotNullType(ofType, _) ⇒
         renderNode(ofType, config, indent.zero) + "!"

diff --git a/src/main/scala/sangria/schema/Schema.scala b/src/main/scala/sangria/schema/Schema.scala
@@ -698,6 +698,7 @@ object DirectiveLocation extends Enumeration {
   val Schema = Value
   val Subscription = Value
   val Union = Value
+  val VariableDefinition = Value
 
   def fromString(location: String): DirectiveLocation.Value = location match {
     case "QUERY" ⇒ Query
@@ -707,6 +708,7 @@ object DirectiveLocation extends Enumeration {
     case "FRAGMENT_DEFINITION" ⇒ FragmentDefinition
     case "FRAGMENT_SPREAD" ⇒ FragmentSpread
     case "INLINE_FRAGMENT" ⇒ InlineFragment
+    case "VARIABLE_DEFINITION" ⇒ VariableDefinition
 
     case "SCHEMA" ⇒ Schema
     case "SCALAR" ⇒ Scalar
@@ -729,6 +731,7 @@ object DirectiveLocation extends Enumeration {
     case FragmentDefinition ⇒ "FRAGMENT_DEFINITION"
     case FragmentSpread ⇒ "FRAGMENT_SPREAD"
     case InlineFragment ⇒ "INLINE_FRAGMENT"
+    case VariableDefinition ⇒ "VARIABLE_DEFINITION"
 
     case Schema ⇒ "SCHEMA"
     case Scalar ⇒ "SCALAR"

diff --git a/src/main/scala/sangria/validation/rules/KnownDirectives.scala b/src/main/scala/sangria/validation/rules/KnownDirectives.scala
@@ -50,6 +50,7 @@ object KnownDirectives {
     case _: ast.FragmentDefinition ⇒ Some(DirectiveLocation.FragmentDefinition → "fragment definition")
     case _: ast.FragmentSpread ⇒ Some(DirectiveLocation.FragmentSpread → "fragment spread")
     case _: ast.InlineFragment ⇒ Some(DirectiveLocation.InlineFragment → "inline fragment")
+    case _: ast.VariableDefinition ⇒ Some(DirectiveLocation.VariableDefinition → "variable definition")
 
     case _: ast.SchemaDefinition ⇒ Some(DirectiveLocation.Schema → "schema definition")
     case _: ast.SchemaExtensionDefinition ⇒ Some(DirectiveLocation.Schema → "schema extension definition")

diff --git a/src/main/scala/sangria/validation/rules/UniqueVariableNames.scala b/src/main/scala/sangria/validation/rules/UniqueVariableNames.scala
@@ -21,7 +21,7 @@ class UniqueVariableNames extends ValidationRule {
         knownVariableNames.clear()
         AstVisitorCommand.RightContinue
 
-      case ast.VariableDefinition(name, _, _, _, pos) ⇒
+      case ast.VariableDefinition(name, _, _, _, _, pos) ⇒
         knownVariableNames get name match {
           case Some(otherPos) ⇒
             Left(Vector(DuplicateVariableViolation(name, ctx.sourceMapper, otherPos ++ pos.toList)))

diff --git a/src/main/scala/sangria/validation/rules/VariablesAreInputTypes.scala b/src/main/scala/sangria/validation/rules/VariablesAreInputTypes.scala
@@ -15,7 +15,7 @@ import sangria.validation._
 class VariablesAreInputTypes extends ValidationRule {
   override def visitor(ctx: ValidationContext) = new AstValidatingVisitor {
     override val onEnter: ValidationVisit = {
-      case ast.VariableDefinition(name, tpe, _, _, pos) ⇒
+      case ast.VariableDefinition(name, tpe, _, _, _, pos) ⇒
         ctx.schema.getInputType(tpe) match {
           case Some(_) ⇒ AstVisitorCommand.RightContinue
           case None ⇒ Left(Vector(

diff --git a/src/test/resources/scenarios/validation/KnownDirectives.yaml b/src/test/resources/scenarios/validation/KnownDirectives.yaml
@@ -93,8 +93,8 @@ tests:
   - name: with well placed directives
     given:
       query: |-
-        query Foo @onQuery {
-          name @include(if: true)
+        query Foo($var: Boolean @onVariableDefinition) @onQuery {
+          name @include(if: $var)
           ...Frag @include(if: true)
           skippedField @skip(if: true)
           ...SkippedFrag @skip(if: true)
@@ -111,8 +111,8 @@ tests:
   - name: with misplaced directives
     given:
       query: |-
-        query Foo @include(if: true) {
-          name @onQuery
+        query Foo($var: Boolean @onField) @include(if: true) {
+          name @onQuery @include(if: $var)
           ...Frag @onQuery
         }
 
@@ -123,14 +123,21 @@ tests:
       validate:
         - KnownDirectives
     then:
-      - error-count: 4
+      - error-count: 5
+      - error-code: misplacedDirective
+        args:
+          directiveName: onField
+          location: VARIABLE_DEFINITION
+        loc:
+          line: 1
+          column: 25
       - error-code: misplacedDirective
         args:
           directiveName: include
           location: QUERY
         loc:
           line: 1
-          column: 11
+          column: 35
       - error-code: misplacedDirective
         args:
           directiveName: onQuery

diff --git a/src/test/resources/scenarios/validation/validation.schema.graphql b/src/test/resources/scenarios/validation/validation.schema.graphql
@@ -106,6 +106,7 @@ directive @onField on FIELD
 directive @onFragmentDefinition on FRAGMENT_DEFINITION
 directive @onFragmentSpread on FRAGMENT_SPREAD
 directive @onInlineFragment on INLINE_FRAGMENT
+directive @onVariableDefinition on VARIABLE_DEFINITION
 directive @onSchema on SCHEMA
 directive @onScalar on SCALAR
 directive @onObject on OBJECT

diff --git a/src/test/scala/sangria/introspection/IntrospectionSpec.scala b/src/test/scala/sangria/introspection/IntrospectionSpec.scala
@@ -158,6 +158,11 @@ class IntrospectionSpec extends WordSpec with Matchers with FutureResultSupport
                     "description" → "Location adjacent to an inline fragment.",
                     "isDeprecated" → false,
                     "deprecationReason" → null),
+                  Map(
+                    "name" → "VARIABLE_DEFINITION",
+                    "description" → "Location adjacent to a variable definition.",
+                    "isDeprecated" → false,
+                    "deprecationReason" → null),
                   Map(
                     "name" → "SCHEMA",
                     "description" → "Location adjacent to a schema definition.",

diff --git a/src/test/scala/sangria/macros/LiteralMacroSpec.scala b/src/test/scala/sangria/macros/LiteralMacroSpec.scala
@@ -51,13 +51,15 @@ class LiteralMacroSpec extends WordSpec with Matchers {
                   NamedType("Int", None),
                   Some(BigDecimalValue(1.23, Vector.empty, None)),
                   Vector.empty,
+                  Vector.empty,
                   None
                 ),
                 VariableDefinition(
                   "anotherVar",
                   NamedType("Int", None),
                   Some(BigIntValue(123, Vector.empty, None)),
                   Vector.empty,
+                  Vector.empty,
                   None
                 )),
               Vector(
@@ -300,13 +302,15 @@ class LiteralMacroSpec extends WordSpec with Matchers {
                   NamedType("ComplexType", None),
                   None,
                   Vector.empty,
+                  Vector.empty,
                   None
                 ),
                 VariableDefinition(
                   "site",
                   NamedType("Site", None),
                   Some(EnumValue("MOBILE", Vector.empty, None)),
                   Vector.empty,
+                  Vector.empty,
                   None
                 )),
               Vector.empty,
@@ -441,6 +445,7 @@ class LiteralMacroSpec extends WordSpec with Matchers {
                   NamedType("StoryLikeSubscribeInput", None),
                   None,
                   Vector.empty,
+                  Vector.empty,
                   None
                 )),
               Vector.empty,