SPEC/BUG: Ambiguity with null variable values and default values (#1274)

This change corresponds to a spec proposal (graphql/graphql-spec#418) which solves an ambiguity in how variable values and default values behave with explicit null values, and changes validation rules to better define the behavior of default values. Otherwise, this ambiguity can allows for null values to appear in non-null argument values, which may result in unforeseen null-pointer-errors.

In summary this change includes:

* **BREAKING:** Removal of `VariablesDefaultValueAllowed` validation rule. All variables may now specify a default value.

* Change to `VariablesInAllowedPosition` rule to explicitly not allow a `null` default value when flowing into a non-null argument, and now allows optional (nullable) variables in non-null arguments that provide default values.

* Changes to `ProvidedRequiredArguments` rule (**BREAKING:** renamed from `ProvidedNonNullArguments`) to no longer require values to be provided to non-null arguments which provide a default value.

* Changes to `getVariableValues()` and `getArgumentValues()` to ensure a `null` value never flows into a non-null argument.

* Changes to `valueFromAST()` to ensure `null` variable values do not flow into non-null types.

* Adds to the `TypeInfo` API to allow referencing the expected default value at a given AST position.

Author: leebyron

src/execution/__tests__/nonnull-test.js

@@ -486,4 +486,199 @@ describe('Execute: handles non-nullable types', () => {
       ],
     },
   );
+
+  describe('Handles non-null argument', () => {
+    const schemaWithNonNullArg = new GraphQLSchema({
+      query: new GraphQLObjectType({
+        name: 'Query',
+        fields: {
+          withNonNullArg: {
+            type: GraphQLString,
+            args: {
+              cannotBeNull: {
+                type: GraphQLNonNull(GraphQLString),
+              },
+            },
+            resolve: async (_, args) => {
+              if (typeof args.cannotBeNull === 'string') {
+                return 'Passed: ' + args.cannotBeNull;
+              }
+            },
+          },
+        },
+      }),
+    });
+
+    it('succeeds when passed non-null literal value', async () => {
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query {
+            withNonNullArg (cannotBeNull: "literal value")
+          }
+        `),
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: 'Passed: literal value',
+        },
+      });
+    });
+
+    it('succeeds when passed non-null variable value', async () => {
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query ($testVar: String!) {
+            withNonNullArg (cannotBeNull: $testVar)
+          }
+        `),
+        variableValues: {
+          testVar: 'variable value',
+        },
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: 'Passed: variable value',
+        },
+      });
+    });
+
+    it('succeeds when missing variable has default value', async () => {
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query ($testVar: String = "default value") {
+            withNonNullArg (cannotBeNull: $testVar)
+          }
+        `),
+        variableValues: {
+          // Intentionally missing variable
+        },
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: 'Passed: default value',
+        },
+      });
+    });
+
+    it('field error when missing non-null arg', async () => {
+      // Note: validation should identify this issue first (missing args rule)
+      // however execution should still protect against this.
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query {
+            withNonNullArg
+          }
+        `),
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: null,
+        },
+        errors: [
+          {
+            message:
+              'Argument "cannotBeNull" of required type "String!" was not provided.',
+            locations: [{ line: 3, column: 13 }],
+            path: ['withNonNullArg'],
+          },
+        ],
+      });
+    });
+
+    it('field error when non-null arg provided null', async () => {
+      // Note: validation should identify this issue first (values of correct
+      // type rule) however execution should still protect against this.
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query {
+            withNonNullArg(cannotBeNull: null)
+          }
+        `),
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: null,
+        },
+        errors: [
+          {
+            message:
+              'Argument "cannotBeNull" of non-null type "String!" must ' +
+              'not be null.',
+            locations: [{ line: 3, column: 42 }],
+            path: ['withNonNullArg'],
+          },
+        ],
+      });
+    });
+
+    it('field error when non-null arg not provided variable value', async () => {
+      // Note: validation should identify this issue first (variables in allowed
+      // position rule) however execution should still protect against this.
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query ($testVar: String) {
+            withNonNullArg(cannotBeNull: $testVar)
+          }
+        `),
+        variableValues: {
+          // Intentionally missing variable
+        },
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: null,
+        },
+        errors: [
+          {
+            message:
+              'Argument "cannotBeNull" of required type "String!" was ' +
+              'provided the variable "$testVar" which was not provided a ' +
+              'runtime value.',
+            locations: [{ line: 3, column: 42 }],
+            path: ['withNonNullArg'],
+          },
+        ],
+      });
+    });
+
+    it('field error when non-null arg provided variable with explicit null value', async () => {
+      const result = await execute({
+        schema: schemaWithNonNullArg,
+        document: parse(`
+          query ($testVar: String = "default value") {
+            withNonNullArg (cannotBeNull: $testVar)
+          }
+        `),
+        variableValues: {
+          testVar: null,
+        },
+      });
+
+      expect(result).to.deep.equal({
+        data: {
+          withNonNullArg: null,
+        },
+        errors: [
+          {
+            message:
+              'Argument "cannotBeNull" of non-null type "String!" must not be null.',
+            locations: [{ line: 3, column: 43 }],
+            path: ['withNonNullArg'],
+          },
+        ],
+      });
+    });
+  });
 });

src/execution/__tests__/variables-test.js

@@ -84,6 +84,10 @@ const TestType = new GraphQLObjectType({
       type: GraphQLString,
       defaultValue: 'Hello World',
     }),
+    fieldWithNonNullableStringInputAndDefaultArgumentValue: fieldWithInputArg({
+      type: GraphQLNonNull(GraphQLString),
+      defaultValue: 'Hello World',
+    }),
     fieldWithNestedInputObject: fieldWithInputArg({
       type: TestNestedInputObject,
       defaultValue: 'Hello World',
@@ -222,6 +226,40 @@ describe('Execute: Handles inputs', () => {
         });
       });
 
+      it('uses undefined when variable not provided', () => {
+        const result = executeQuery(
+          `
+          query q($input: String) {
+            fieldWithNullableStringInput(input: $input)
+          }`,
+          {
+            // Intentionally missing variable values.
+          },
+        );
+
+        expect(result).to.deep.equal({
+          data: {
+            fieldWithNullableStringInput: null,
+          },
+        });
+      });
+
+      it('uses null when variable provided explicit null value', () => {
+        const result = executeQuery(
+          `
+          query q($input: String) {
+            fieldWithNullableStringInput(input: $input)
+          }`,
+          { input: null },
+        );
+
+        expect(result).to.deep.equal({
+          data: {
+            fieldWithNullableStringInput: 'null',
+          },
+        });
+      });
+
       it('uses default value when not provided', () => {
         const result = executeQuery(`
           query ($input: TestInputObject = {a: "foo", b: ["bar"], c: "baz"}) {
@@ -236,6 +274,55 @@ describe('Execute: Handles inputs', () => {
         });
       });
 
+      it('does not use default value when provided', () => {
+        const result = executeQuery(
+          `query q($input: String = "Default value") {
+            fieldWithNullableStringInput(input: $input)
+          }`,
+          { input: 'Variable value' },
+        );
+
+        expect(result).to.deep.equal({
+          data: {
+            fieldWithNullableStringInput: "'Variable value'",
+          },
+        });
+      });
+
+      it('uses explicit null value instead of default value', () => {
+        const result = executeQuery(
+          `
+          query q($input: String = "Default value") {
+            fieldWithNullableStringInput(input: $input)
+          }`,
+          { input: null },
+        );
+
+        expect(result).to.deep.equal({
+          data: {
+            fieldWithNullableStringInput: 'null',
+          },
+        });
+      });
+
+      it('uses null default value when not provided', () => {
+        const result = executeQuery(
+          `
+          query q($input: String = null) {
+            fieldWithNullableStringInput(input: $input)
+          }`,
+          {
+            // Intentionally missing variable values.
+          },
+        );
+
+        expect(result).to.deep.equal({
+          data: {
+            fieldWithNullableStringInput: 'null',
+          },
+        });
+      });
+
       it('properly parses single value to list', () => {
         const params = { input: { a: 'foo', b: 'bar', c: 'baz' } };
         const result = executeQuery(doc, params);
@@ -485,8 +572,7 @@ describe('Execute: Handles inputs', () => {
         errors: [
           {
             message:
-              'Variable "$value" got invalid value null; ' +
-              'Expected non-nullable type String! not to be null.',
+              'Variable "$value" of non-null type "String!" must not be null.',
             locations: [{ line: 2, column: 16 }],
           },
         ],
@@ -644,8 +730,7 @@ describe('Execute: Handles inputs', () => {
         errors: [
           {
             message:
-              'Variable "$input" got invalid value null; ' +
-              'Expected non-nullable type [String]! not to be null.',
+              'Variable "$input" of non-null type "[String]!" must not be null.',
             locations: [{ line: 2, column: 16 }],
           },
         ],
@@ -728,8 +813,7 @@ describe('Execute: Handles inputs', () => {
         errors: [
           {
             message:
-              'Variable "$input" got invalid value null; ' +
-              'Expected non-nullable type [String!]! not to be null.',
+              'Variable "$input" of non-null type "[String!]!" must not be null.',
             locations: [{ line: 2, column: 16 }],
           },
         ],
@@ -853,5 +937,20 @@ describe('Execute: Handles inputs', () => {
         ],
       });
     });
+
+    it('when no runtime value is provided to a non-null argument', () => {
+      const result = executeQuery(`
+        query optionalVariable($optional: String) {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue(input: $optional)
+        }
+      `);
+
+      expect(result).to.deep.equal({
+        data: {
+          fieldWithNonNullableStringInputAndDefaultArgumentValue:
+            "'Hello World'",
+        },
+      });
+    });
   });
 });