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.

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

Detecting unused "variable values" in input? #615

Closed
mfn opened this issue Jan 23, 2020 · 3 comments
Closed

Detecting unused "variable values" in input? #615

mfn opened this issue Jan 23, 2020 · 3 comments

Comments

@mfn
Copy link
Contributor

mfn commented Jan 23, 2020

I recently had a bug in an application where I provided a variable value as input which wasn't consumed though => it was a typo and the (real) variable in question was nullable, thus there was no immediate GraphQL error.

Is there a way to detect "unused" variable values?

To give an example, this test:

    public function testAllowsNullableInputsToBeSetToAValueInAVariable() : void
    {
        $doc      = '
      query SetsNullable($value: String) {
        fieldWithNullableStringInput(input: $value)
      }
        ';
        $result   = $this->executeQuery($doc, ['value' => 'a']);
        $expected = ['data' => ['fieldWithNullableStringInput' => '"a"']];
        self::assertEquals($expected, $result->toArray());
    }

if I modify it:

diff --git a/tests/Executor/VariablesTest.php b/tests/Executor/VariablesTest.php
index a2d698b..74c71f7 100644
--- a/tests/Executor/VariablesTest.php
+++ b/tests/Executor/VariablesTest.php
@@ -551,7 +551,7 @@ class VariablesTest extends TestCase
         fieldWithNullableStringInput(input: $value)
       }
         ';
-        $result   = $this->executeQuery($doc, ['value' => 'a']);
+        $result   = $this->executeQuery($doc, ['value' => 'a', 'foo' => 'bar']);
         $expected = ['data' => ['fieldWithNullableStringInput' => '"a"']];
         self::assertEquals($expected, $result->toArray());
     }

then there's no error/warning/etc that foo is not being used.

thanks!

@vladar
Copy link
Member

vladar commented Feb 26, 2020

Good question. It makes sense to add as a feature request I guess. Technically shouldn't be that hard to implement in the userland too. You just need to parse your query and then check the list of variable names in AST against your array keys.

Feel free to submit a PR if you decided to tackle it!

@mfn
Copy link
Contributor Author

mfn commented Feb 26, 2020

You just need to parse your query and then check the list of variable names in AST against your array keys.

Ugh. Feels a bit out of my league TBH 😄

But thanks for acknowledging the usefulness. Not sure if I can cook up something though.

@vladar
Copy link
Member

vladar commented Jun 8, 2020

It is not that hard. Use something like this as a reference:

use GraphQL\Language\Parser;
use GraphQL\Language\OperationDefinitionNode;

$document = Praser::parse($queryString);
foreach ($document->definitions as $definition) {
  if ($definition instanceof OperationDefinitionNode) {
    // here is a list of all query variables which you can match against your $variables:
    $definition->variableDefinitions;
  }
}

$result = GraphQL::executeQuery(
    $schema,
    $document // just pass the document in place of $queryString
);

I am closing this for now but feel free to re-open if you want to work on this utility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants