Ignore Missing Ignore/Skip Directives

Currently unused variables are ignored for the purposes of calculating complexity.
However, because graphql-js's `getVariableValues` method returns either a map of coersed
values, OR an array of errors, if directives are present in conjuection with
unused variables then strange errors will be raised.

In particular errors will state that directive variables were not present
when they may well have been.

This change adds fallback measures to ensure that complexity will stil be
calculated if unused variables are present alonside directives.

Fixes slicknode#69

Author: Vincent Smith

src/QueryComplexity.ts

@@ -173,6 +173,12 @@ export default class QueryComplexity {
       this.options.variables ?? {}
     ).coerced;
 
+    // If there are no coerced variable values it is because one or all of those variables were
+    // absent. In this case fallback to what was provided and ignore defaults.
+    if (!this.variableValues) {
+      this.variableValues = this.options.variables;
+    }
+
     switch (operation.operation) {
       case 'query':
         this.complexity += this.nodeComplexity(
@@ -261,28 +267,36 @@ export default class QueryComplexity {
 
             for (const directive of childNode.directives ?? []) {
               const directiveName = directive.name.value;
-              switch (directiveName) {
-                case 'include': {
-                  const values = getDirectiveValues(
-                    this.includeDirectiveDef,
-                    childNode,
-                    this.variableValues || {}
-                  );
-                  if (typeof values.if === 'boolean') {
-                    includeNode = values.if;
+              try {
+                switch (directiveName) {
+                  case 'include': {
+                    const values = getDirectiveValues(
+                      this.includeDirectiveDef,
+                      childNode,
+                      this.variableValues || {}
+                    );
+                    if (typeof values.if === 'boolean') {
+                      includeNode = values.if;
+                    }
+                    break;
                   }
-                  break;
-                }
-                case 'skip': {
-                  const values = getDirectiveValues(
-                    this.skipDirectiveDef,
-                    childNode,
-                    this.variableValues || {}
-                  );
-                  if (typeof values.if === 'boolean') {
-                    skipNode = values.if;
+                  case 'skip': {
+                    const values = getDirectiveValues(
+                      this.skipDirectiveDef,
+                      childNode,
+                      this.variableValues || {}
+                    );
+                    if (typeof values.if === 'boolean') {
+                      skipNode = values.if;
+                    }
+                    break;
                   }
-                  break;
+                }
+              } catch (error) {
+                // Ignore GraphQLErrors in favor of falling back to the default values for
+                // includeNode and skipNode
+                if (!(error instanceof GraphQLError)) {
+                  throw error;
                 }
               }
             }

src/__tests__/QueryComplexity-test.ts

@@ -271,6 +271,93 @@ describe('QueryComplexity analysis', () => {
     expect(visitor.complexity).to.equal(10);
   });
 
+  it('should ignore unused variables with include', () => {
+    const ast = parse(`
+      query ($unusedVar: ID!, $shouldInclude: Boolean! = false) {
+        variableScalar(count: 100) @include(if: $shouldInclude)
+      }
+    `);
+
+    const context = new CompatibleValidationContext(schema, ast, typeInfo);
+    const visitor = new ComplexityVisitor(context, {
+      maximumComplexity: 100,
+      estimators: [simpleEstimator({ defaultComplexity: 10 })],
+      variables: { shouldInclude: true },
+    });
+
+    visit(ast, visitWithTypeInfo(typeInfo, visitor));
+    expect(visitor.complexity).to.equal(10);
+  });
+
+  it('should ignore unused variables with skip', () => {
+    const ast = parse(`
+      query ($unusedVar: ID!, $shouldSkip: Boolean! = false) {
+        variableScalar(count: 100) @skip(if: $shouldSkip)
+      }
+    `);
+
+    const context = new CompatibleValidationContext(schema, ast, typeInfo);
+    const visitor = new ComplexityVisitor(context, {
+      maximumComplexity: 100,
+      estimators: [simpleEstimator({ defaultComplexity: 10 })],
+      variables: { shouldSkip: false },
+    });
+
+    visit(ast, visitWithTypeInfo(typeInfo, visitor));
+    expect(visitor.complexity).to.equal(10);
+  });
+
+  it('should ignore unused variables with unused include', () => {
+    const ast = parse(`
+      query ($unusedVar: ID!, $shouldInclude: Boolean! = false) {
+        variableScalar(count: 100) @include(if: $shouldInclude)
+      }
+    `);
+
+    const context = new CompatibleValidationContext(schema, ast, typeInfo);
+    const visitor = new ComplexityVisitor(context, {
+      maximumComplexity: 100,
+      estimators: [simpleEstimator({ defaultComplexity: 10 })],
+    });
+
+    visit(ast, visitWithTypeInfo(typeInfo, visitor));
+    expect(visitor.complexity).to.equal(10);
+  });
+
+  it('should ignore unused variables with unused skip', () => {
+    const ast = parse(`
+      query ($unusedVar: ID!, $shouldSkip: Boolean! = false) {
+        variableScalar(count: 100) @skip(if: $shouldSkip)
+      }
+    `);
+
+    const context = new CompatibleValidationContext(schema, ast, typeInfo);
+    const visitor = new ComplexityVisitor(context, {
+      maximumComplexity: 100,
+      estimators: [simpleEstimator({ defaultComplexity: 10 })],
+    });
+
+    visit(ast, visitWithTypeInfo(typeInfo, visitor));
+    expect(visitor.complexity).to.equal(10);
+  });
+
+  it('should ignore multiple unused variables', () => {
+    const ast = parse(`
+      query ($unusedVar: ID!, $anotherUnusedVar: Boolean!) {
+        variableScalar(count: 100)
+      }
+    `);
+
+    const context = new CompatibleValidationContext(schema, ast, typeInfo);
+    const visitor = new ComplexityVisitor(context, {
+      maximumComplexity: 100,
+      estimators: [simpleEstimator({ defaultComplexity: 10 })],
+    });
+
+    visit(ast, visitWithTypeInfo(typeInfo, visitor));
+    expect(visitor.complexity).to.equal(10);
+  });
+
   it('should ignore unknown field', () => {
     const ast = parse(`
       query {
@@ -639,7 +726,7 @@ describe('QueryComplexity analysis', () => {
           ...on Union {
             ...on Item {
               complexScalar1: complexScalar
-            } 
+            }
           }
           ...on SecondItem {
             scalar
@@ -678,7 +765,7 @@ describe('QueryComplexity analysis', () => {
       fragment F on Union {
         ...on Item {
           complexScalar1: complexScalar
-        } 
+        }
       }
     `);
 
@@ -759,7 +846,7 @@ describe('QueryComplexity analysis', () => {
           }
         }
       }
-      
+
       fragment F on Query {
         complexScalar
         ...on Query {