From 05dde88db22fec94b403d3e99526607a6ba7bd32 Mon Sep 17 00:00:00 2001 From: Miles Sabin Date: Wed, 13 Dec 2023 13:04:32 +0000 Subject: [PATCH] Added option to disable unused variable/fragment validation --- modules/core/src/main/scala/compiler.scala | 21 +++++---- modules/core/src/main/scala/mapping.scala | 8 ++-- .../test/scala/compiler/FragmentSuite.scala | 47 ++++++++++++++++++- .../test/scala/compiler/VariablesSuite.scala | 32 +++++++++++++ 4 files changed, 95 insertions(+), 13 deletions(-) diff --git a/modules/core/src/main/scala/compiler.scala b/modules/core/src/main/scala/compiler.scala index 3bb5c3a8..111378e4 100644 --- a/modules/core/src/main/scala/compiler.scala +++ b/modules/core/src/main/scala/compiler.scala @@ -215,10 +215,10 @@ class QueryCompiler(parser: QueryParser, schema: Schema, phases: List[Phase]) { * * GraphQL errors and warnings are accumulated in the result. */ - def compile(text: String, name: Option[String] = None, untypedVars: Option[Json] = None, introspectionLevel: IntrospectionLevel = Full, env: Env = Env.empty): Result[Operation] = + def compile(text: String, name: Option[String] = None, untypedVars: Option[Json] = None, introspectionLevel: IntrospectionLevel = Full, reportUnused: Boolean = true, env: Env = Env.empty): Result[Operation] = parser.parseText(text).flatMap { case (ops, frags) => for { - _ <- Result.fromProblems(validateVariablesAndFragments(ops, frags)) + _ <- Result.fromProblems(validateVariablesAndFragments(ops, frags, reportUnused)) ops0 <- ops.traverse(op => compileOperation(op, untypedVars, frags, introspectionLevel, env).map(op0 => (op.name, op0))) res <- (ops0, name) match { case (List((_, op)), None) => @@ -323,7 +323,7 @@ class QueryCompiler(parser: QueryParser, schema: Schema, phases: List[Phase]) { loop(tpe, false) } - def validateVariablesAndFragments(ops: List[UntypedOperation], frags: List[UntypedFragment]): List[Problem] = { + def validateVariablesAndFragments(ops: List[UntypedOperation], frags: List[UntypedFragment], reportUnused: Boolean): List[Problem] = { val (uniqueFrags, duplicateFrags) = frags.map(_.name).foldLeft((Set.empty[String], Set.empty[String])) { case ((unique, duplicate), nme) => if (unique.contains(nme)) (unique, duplicate + nme) @@ -439,10 +439,13 @@ class QueryCompiler(parser: QueryParser, schema: Schema, phases: List[Phase]) { val varProblems = if (qv == pendingVars) Nil else { - val undefined = qv.diff(pendingVars) - val unused = pendingVars.diff(qv) - val undefinedProblems = undefined.toList.map(nme => Problem(s"Variable '$nme' is undefined")) - val unusedProblems = unused.toList.map(nme => Problem(s"Variable '$nme' is unused")) + val undefinedProblems = + qv.diff(pendingVars).toList.map(nme => Problem(s"Variable '$nme' is undefined")) + + val unusedProblems = + if (!reportUnused) Nil + else pendingVars.diff(qv).toList.map(nme => Problem(s"Variable '$nme' is unused")) + undefinedProblems ++ unusedProblems } @@ -464,7 +467,9 @@ class QueryCompiler(parser: QueryParser, schema: Schema, phases: List[Phase]) { (acc ++ problems, pendingFrags0) } - val unreferencedFragProblems = unreferencedFrags.toList.map(nme => Problem(s"Fragment '$nme' is unused")) + val unreferencedFragProblems = + if (!reportUnused) Nil + else unreferencedFrags.toList.map(nme => Problem(s"Fragment '$nme' is unused")) opProblems ++ unreferencedFragProblems } diff --git a/modules/core/src/main/scala/mapping.scala b/modules/core/src/main/scala/mapping.scala index b9d35239..36ddc94e 100644 --- a/modules/core/src/main/scala/mapping.scala +++ b/modules/core/src/main/scala/mapping.scala @@ -47,10 +47,10 @@ abstract class Mapping[F[_]] { * * Yields a JSON response containing the result of the query or mutation. */ - def compileAndRun(text: String, name: Option[String] = None, untypedVars: Option[Json] = None, introspectionLevel: IntrospectionLevel = Full, env: Env = Env.empty)( + def compileAndRun(text: String, name: Option[String] = None, untypedVars: Option[Json] = None, introspectionLevel: IntrospectionLevel = Full, reportUnused: Boolean = true, env: Env = Env.empty)( implicit sc: Compiler[F,F] ): F[Json] = - compileAndRunSubscription(text, name, untypedVars, introspectionLevel, env).compile.toList.flatMap { + compileAndRunSubscription(text, name, untypedVars, introspectionLevel, reportUnused, env).compile.toList.flatMap { case List(j) => j.pure[F] case Nil => M.raiseError(new IllegalStateException("Result stream was empty.")) case js => M.raiseError(new IllegalStateException(s"Result stream contained ${js.length} results; expected exactly one.")) @@ -61,8 +61,8 @@ abstract class Mapping[F[_]] { * * Yields a stream of JSON responses containing the results of the subscription. */ - def compileAndRunSubscription(text: String, name: Option[String] = None, untypedVars: Option[Json] = None, introspectionLevel: IntrospectionLevel = Full, env: Env = Env.empty): Stream[F,Json] = { - val compiled = compiler.compile(text, name, untypedVars, introspectionLevel, env) + def compileAndRunSubscription(text: String, name: Option[String] = None, untypedVars: Option[Json] = None, introspectionLevel: IntrospectionLevel = Full, reportUnused: Boolean = true, env: Env = Env.empty): Stream[F,Json] = { + val compiled = compiler.compile(text, name, untypedVars, introspectionLevel, reportUnused, env) Stream.eval(compiled.pure[F]).flatMap(_.flatTraverse(op => interpreter.run(op.query, op.rootTpe, env))).evalMap(mkResponse) } diff --git a/modules/core/src/test/scala/compiler/FragmentSuite.scala b/modules/core/src/test/scala/compiler/FragmentSuite.scala index 698b2b8c..d781eb44 100644 --- a/modules/core/src/test/scala/compiler/FragmentSuite.scala +++ b/modules/core/src/test/scala/compiler/FragmentSuite.scala @@ -817,7 +817,7 @@ final class FragmentSuite extends CatsEffectSuite { assertIO(res, expected) } - test("fragment used") { + test("fragment unused (1)") { val query = """ query withFragments { user(id: 1) { @@ -851,6 +851,51 @@ final class FragmentSuite extends CatsEffectSuite { assertIO(res, expected) } + test("fragment unused (2)") { + val query = """ + query withFragments { + user(id: 1) { + friends { + id + name + profilePic + } + } + } + + fragment friendFields on User { + id + name + profilePic + } + """ + + val expected = json""" + { + "data" : { + "user" : { + "friends" : [ + { + "id" : "2", + "name" : "Bob", + "profilePic" : "B" + }, + { + "id" : "3", + "name" : "Carol", + "profilePic" : "C" + } + ] + } + } + } + """ + + val res = FragmentMapping.compileAndRun(query, reportUnused = false) + + assertIO(res, expected) + } + test("fragment duplication") { val query = """ query withFragments { diff --git a/modules/core/src/test/scala/compiler/VariablesSuite.scala b/modules/core/src/test/scala/compiler/VariablesSuite.scala index c8e09bbc..178b27cd 100644 --- a/modules/core/src/test/scala/compiler/VariablesSuite.scala +++ b/modules/core/src/test/scala/compiler/VariablesSuite.scala @@ -511,6 +511,36 @@ final class VariablesSuite extends CatsEffectSuite { assertEquals(compiled, expected) } + + test("variable unused (2)") { + val query = """ + query getZuckProfile($devicePicSize: Int) { + user(id: 4) { + id + name + } + } + """ + + val compiled = VariablesMapping.compiler.compile(query, reportUnused = false) + println(compiled) + + val expected = + Operation( + UntypedSelect("user", None, List(Binding("id", IDValue("4"))), Nil, + Group( + List( + UntypedSelect("id", None, Nil, Nil, Empty), + UntypedSelect("name", None, Nil, Nil, Empty) + ) + ) + ), + VariablesMapping.QueryType, + Nil + ) + + assertEquals(compiled, Result.success(expected)) + } } object VariablesMapping extends TestMapping { @@ -544,5 +574,7 @@ object VariablesMapping extends TestMapping { scalar BigDecimal """ + val QueryType = schema.ref("Query").dealias + override val selectElaborator = PreserveArgsElaborator }