From ef2689193a40b8ab3aa4665d3a751996f8fccbf5 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Mon, 5 Mar 2018 19:09:56 -0800 Subject: [PATCH 1/7] RFC: Fix ambiguity with null variable values and default values > This is a **behavioral change** which changes how explicit `null` values interact with variable and argument default values. This also changes a validation rule which makes the rule more strict. There is currently ambiguity and inconsistency in how `null` values are coerced and resolved as part of variable values, default values, and argument values. This inconsistency and ambiguity can allow for `null` values to appear at non-null arguments, which might result in unforseen null-pointer-errors. This appears in three distinct but related issues: **Validation: All Variable Usages are Allowed** The explicit value `null` may be used as a default value for a variable with a nullable type, however this rule asks to treat a variable's type as non-null if it has a default value. Instead this rule should specifically only treat the variable's type as non-null if the default value is not `null`. Additionally, the `AreTypesCompatible` algorithm is underspecificied, which could lead to further misinterpretation of this validation rule. **Coercing Variable Values** `CoerceVariableValues()` allows the explicit `null` value to be used instead of a default value. This can result in a null value flowing to a non-null argument due to the validation rule mentioned above. Instead a default value must be used even when an explicit `null` value is provided. This is also more consistent with the explanation for validation rule "Variable Default Value Is Allowed" Also, how to treat an explicit `null` value is currently underspecified. While an input object explains that a `null` value should result in an explicit `null` value at the input object field, there is no similar explaination for typical scalar input types. Instead, `CoerceVariableValues()` should explicitly handle the `null` value to make it clear a `null` is the resulting value in the `coercedValues` Map. **Coercing Argument Values** The `CoerceArgumentValues()` algorithm is intentionally similar to `CoerceVariableValues()` and suffers from the same inconsistency. Explicit `null` values should not take precedence over default values, and should also be explicitly handled rather than left to underspecified input scalar coercion. --- spec/Section 5 -- Validation.md | 42 ++++++++++++------ spec/Section 6 -- Execution.md | 79 +++++++++++++++++++-------------- 2 files changed, 75 insertions(+), 46 deletions(-) diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md index e77e650dc..3b2725268 100644 --- a/spec/Section 5 -- Validation.md +++ b/spec/Section 5 -- Validation.md @@ -1836,17 +1836,33 @@ an extraneous variable. * For each {operation} in {document} * Let {variableUsages} be all usages transitively included in the {operation} * For each {variableUsage} in {variableUsages} - * Let {variableType} be the type of variable definition in the {operation} - * Let {argumentType} be the type of the argument the variable is passed to. - * Let {hasDefault} be true if the variable definition defines a default. - * AreTypesCompatible({argumentType}, {variableType}, {hasDefault}) must be true - - * AreTypesCompatible({argumentType}, {variableType}, {hasDefault}): - * If {hasDefault} is true, treat the {variableType} as non-null. - * If inner type of {argumentType} and {variableType} are different, return false - * If {argumentType} and {variableType} have different list dimensions, return false - * If any list level of {variableType} is not non-null, and the corresponding level - in {argument} is non-null, the types are not compatible. + * Let {variableName} be the name of {variableUsage} + * Let {variableDefinition} be the {VariableDefinition} in {operation}. + * Let {variableType} be the type of {variableDefinition}. + * Let {defaultValue} be the default value of {variableDefinition}. + * If {variableType} is not a non-null and {defaultValue} is provided and not {null}: + * Let {variableType} be the non-null of {variableType}. + * Let {argumentType} be the type of the argument {variableUsage} is passed to. + * AreTypesCompatible({argumentType}, {variableType}) must be {true}. + +AreTypesCompatible(argumentType, variableType): + * If {argumentType} is a non-null type: + * If {variableType} is a non-null type: + * Let {nullableArgumentType} be the nullable type of {argumentType}. + * Let {nullableVariableType} be the nullable type of {variableType}. + * Return {AreTypesCompatible(nullableArgumentType, nullableVariableType)}. + * Otherwise return {false}. + * If {variableType} is a non-null type: + * Let {nullableVariableType} be the nullable type of {variableType}. + * Return {AreTypesCompatible(argumentType, nullableVariableType)}. + * If {argumentType} is a list type: + * If {variableType} is a list type: + * Let {itemArgumentType} be the item type of {argumentType}. + * Let {itemVariableType} be the item type of {variableType}. + * Return {AreTypesCompatible(itemArgumentType, itemVariableType)}. + * Otherwise return {false}. + * If {variableType} is a list type return {false}. + * Return if {variableType} and {argumentType} are identical. **Explanatory Text** @@ -1890,8 +1906,8 @@ query booleanArgQuery($booleanArg: Boolean) { } ``` -A notable exception is when default arguments are provided. They are, in effect, -treated as non-nulls. +A notable exception is when a default value are provided. They are, in effect, +treated as non-nulls as long as the default value is not {null}. ```graphql example query booleanArgQueryWithDefault($booleanArg: Boolean = true) { diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md index 7ae8706a3..f636b01ea 100644 --- a/spec/Section 6 -- Execution.md +++ b/spec/Section 6 -- Execution.md @@ -82,20 +82,27 @@ CoerceVariableValues(schema, operation, variableValues): * For each {variableDefinition} in {variableDefinitions}: * Let {variableName} be the name of {variableDefinition}. * Let {variableType} be the expected type of {variableDefinition}. + * Assert: {variableType} must be an input type. * Let {defaultValue} be the default value for {variableDefinition}. - * Let {value} be the value provided in {variableValues} for the name {variableName}. - * If {value} does not exist (was not provided in {variableValues}): - * If {defaultValue} exists (including {null}): + * Let {hasValue} be {true} if {variableValues} provides a value for the + name {variableName}. + * Let {value} be the value provided in {variableValues} for the + name {variableName}. + * If {hasValue} is not {true} or if {value} is {null}: + * If {variableType} is a Non-Nullable type, throw a query error. + * Otherwise if {defaultValue} exists (including {null}): * Add an entry to {coercedValues} named {variableName} with the value {defaultValue}. - * Otherwise if {variableType} is a Non-Nullable type, throw a query error. - * Otherwise, continue to the next variable definition. - * Otherwise, if {value} cannot be coerced according to the input coercion - rules of {variableType}, throw a query error. - * Let {coercedValue} be the result of coercing {value} according to the - input coercion rules of {variableType}. - * Add an entry to {coercedValues} named {variableName} with the - value {coercedValue}. + * Otherwise if {hasValue} is {true} and {value} is {null}: + * Add an entry to {coercedValues} named {variableName} with the + value {null}. + * Otherwise, {value} must not be {null}: + * If {value} cannot be coerced according to the input coercion + rules of {variableType}, throw a query error. + * Let {coercedValue} be the result of coercing {value} according to the + input coercion rules of {variableType}. + * Add an entry to {coercedValues} named {variableName} with the + value {coercedValue}. * Return {coercedValues}. Note: This algorithm is very similar to {CoerceArgumentValues()}. @@ -530,8 +537,8 @@ order to correctly produce a value. These arguments are defined by the field in the type system to have a specific input type: Scalars, Enum, Input Object, or List or Non-Null wrapped variations of these three. -At each argument position in a query may be a literal value or a variable to be -provided at runtime. +At each argument position in a query may be a literal {Value} or {Variable} to +be provided at runtime. CoerceArgumentValues(objectType, field, variableValues): * Let {coercedValues} be an empty unordered Map. @@ -543,30 +550,36 @@ CoerceArgumentValues(objectType, field, variableValues): * Let {argumentName} be the name of {argumentDefinition}. * Let {argumentType} be the expected type of {argumentDefinition}. * Let {defaultValue} be the default value for {argumentDefinition}. - * Let {value} be the value provided in {argumentValues} for the name {argumentName}. - * If {value} is a Variable: - * Let {variableName} be the name of Variable {value}. - * Let {variableValue} be the value provided in {variableValues} for the name {variableName}. - * If {variableValue} exists (including {null}): - * Add an entry to {coercedValues} named {argName} with the - value {variableValue}. + * Let {hasValue} be {true} if {argumentValues} provides a value for the + name {argumentName}. + * Let {value} be the value provided in {argumentValues} for the + name {argumentName}. + * If {hasValue} is not {true} or if {value} is {null}: + * If {argumentType} is a Non-Nullable type, throw a field error. * Otherwise, if {defaultValue} exists (including {null}): - * Add an entry to {coercedValues} named {argName} with the + * Add an entry to {coercedValues} named {argumentName} with the value {defaultValue}. + * Otherwise, if {hasValue} is {true} and {value} is {null}: + * Add an entry to {coercedValues} named {argumentName} with the + value {null}. + * Otherwise, if {value} is a {Variable}: + * Let {variableName} be the name of {value}. + * If {variableValues} provides a value for the name {variableName}: + * Let {variableValue} be the value provided in {variableValues} for the + name {variableName}. + * Add an entry to {coercedValues} named {argumentName} with the + value {variableValue}. * Otherwise, if {argumentType} is a Non-Nullable type, throw a field error. - * Otherwise, continue to the next argument definition. - * Otherwise, if {value} does not exist (was not provided in {argumentValues}: - * If {defaultValue} exists (including {null}): - * Add an entry to {coercedValues} named {argName} with the + * Otherwise, if {defaultValue} exists (including {null}): + * Add an entry to {coercedValues} named {argumentName} with the value {defaultValue}. - * Otherwise, if {argumentType} is a Non-Nullable type, throw a field error. - * Otherwise, continue to the next argument definition. - * Otherwise, if {value} cannot be coerced according to the input coercion - rules of {argType}, throw a field error. - * Let {coercedValue} be the result of coercing {value} according to the - input coercion rules of {argType}. - * Add an entry to {coercedValues} named {argName} with the - value {coercedValue}. + * Otherwise, {value} must be a {Value} and not {null}: + * If {value} cannot be coerced according to the input coercion + rules of {argType}, throw a field error. + * Let {coercedValue} be the result of coercing {value} according to the + input coercion rules of {argType}. + * Add an entry to {coercedValues} named {argumentName} with the + value {coercedValue}. * Return {coercedValues}. Note: Variable values are not coerced because they are expected to be coerced From 3dc6d1b9accde911c77768eb22819e5314d966de Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Thu, 29 Mar 2018 20:48:48 -0700 Subject: [PATCH 2/7] Updated based on feedback. This updates this proposal to be a bit broader in scope however much narrower in breaking behavior changes. Mirroring the changes in https://github.com/graphql/graphql-js/pull/1274, this update better defines the difference between a "required" and "non-null" argument / input field as a non-null typed argument / input-field with a default value is no longer required. As such the validation rule which prohibited queries from using non-null variables and default values has been removed. This also adds clarity to the input field validation - this rule has existed in the GraphQL.js reference implementation however was found missing within the spec. This also updates the CoerceVariableValues() and CoerceArgumentValues() algorithms to retain explicit null values overriding a default value (minimizing breaking changes), however critically adding additional protection to CoerceArgumentValues() to explicitly block null values from variables - thus allowing the older pattern of passing a nullable variable into a non-null argument while limiting the problematic case of an explicit null value at runtime. --- spec/Section 5 -- Validation.md | 96 +++++++++++++++------------------ spec/Section 6 -- Execution.md | 74 ++++++++++++------------- 2 files changed, 82 insertions(+), 88 deletions(-) diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md index 3b2725268..a85f60d10 100644 --- a/spec/Section 5 -- Validation.md +++ b/spec/Section 5 -- Validation.md @@ -710,15 +710,16 @@ and invalid. * {arguments} must be the set containing only {argument}. -#### Required Non-Null Arguments +#### Required Arguments * For each Field or Directive in the document. * Let {arguments} be the arguments provided by the Field or Directive. * Let {argumentDefinitions} be the set of argument definitions of that Field or Directive. - * For each {definition} in {argumentDefinitions}: - * Let {type} be the expected type of {definition}. - * If {type} is Non-Null: - * Let {argumentName} be the name of {definition}. + * For each {argumentDefinition} in {argumentDefinitions}: + * Let {type} be the expected type of {argumentDefinition}. + * Let {defaultValue} be the default value of {argumentDefinition}. + * If {type} is Non-Null and {defaultValue} does not exist: + * Let {argumentName} be the name of {argumentDefinition}. * Let {argument} be the argument in {arguments} named {argumentName} * {argument} must exist. * Let {value} be the value of {argument}. @@ -726,9 +727,9 @@ and invalid. **Explanatory Text** -Arguments can be required. If the argument type is non-null the argument is -required and furthermore the explicit value {null} may not be provided. -Otherwise, the argument is optional. +Arguments can be required. If the argument type is non-null and does not have a +default value, the argument is required and furthermore the explicit value +{null} may not be provided. Otherwise, the argument is optional. For example the following are valid: @@ -738,7 +739,7 @@ fragment goodBooleanArg on Arguments { } fragment goodNonNullArg on Arguments { - nonNullBooleanArgField(nonNullBooleanArg: true) + requiredBooleanArgField(requiredBooleanArg: true) } ``` @@ -752,11 +753,11 @@ fragment goodBooleanArgDefault on Arguments { } ``` -but this is not valid on a non-null argument. +but this is not valid on a required argument. ```graphql counter-example fragment missingRequiredArg on Arguments { - nonNullBooleanArgField + requiredBooleanArgField } ``` @@ -764,7 +765,7 @@ Providing the explicit value {null} is also not valid. ```graphql counter-example fragment missingRequiredArg on Arguments { - notNullBooleanArgField(nonNullBooleanArg: null) + requiredBooleanArgField(requiredBooleanArg: null) } ``` @@ -1358,6 +1359,31 @@ For example the following query will not pass validation. ``` +### Input Object Required Fields + +**Formal Specification** + + * For each Input Object in the document. + * Let {fields} be the fields provided by that Input Object. + * Let {fieldDefinitions} be the set of input field definitions of that Input Object. + * For each {fieldDefinition} in {fieldDefinitions}: + * Let {type} be the expected type of {fieldDefinition}. + * Let {defaultValue} be the default value of {fieldDefinition}. + * If {type} is Non-Null and {defaultValue} does not exist: + * Let {fieldName} be the name of {fieldDefinition}. + * Let {field} be the input field in {fields} named {fieldName} + * {field} must exist. + * Let {value} be the value of {field}. + * {value} must not be the {null} literal. + +**Explanatory Text** + +Input object fields can be required. If the input object field type is non-null +and does not have a default value, the input object field is required and +furthermore the explicit value {null} may not be provided. Otherwise, the input +object field is optional. + + ## Directives @@ -1494,44 +1520,6 @@ fragment HouseTrainedFragment { ``` -### Variable Default Value Is Allowed - -**Formal Specification** - - * For every Variable Definition {variableDefinition} in a document - * Let {variableType} be the type of {variableDefinition} - * Let {defaultValue} be the default value of {variableDefinition} - * If {variableType} is Non-null: - * {defaultValue} must be undefined. - -**Explanatory Text** - -Variables defined by operations are allowed to define default values -if the type of that variable is not non-null. - -For example the following query will pass validation. - -```graphql example -query houseTrainedQuery($atOtherHomes: Boolean = true) { - dog { - isHousetrained(atOtherHomes: $atOtherHomes) - } -} -``` - -However if the variable is defined as non-null, default values -are unreachable. Therefore queries such as the following fail -validation - -```graphql counter-example -query houseTrainedQuery($atOtherHomes: Boolean! = true) { - dog { - isHousetrained(atOtherHomes: $atOtherHomes) - } -} -``` - - ### Variables Are Input Types **Formal Specification** @@ -1906,8 +1894,12 @@ query booleanArgQuery($booleanArg: Boolean) { } ``` -A notable exception is when a default value are provided. They are, in effect, -treated as non-nulls as long as the default value is not {null}. +A notable exception is when a default value is provided. Variables which include +a default value may be provided to a non-null argument as long as the default +value is not {null}. + +Note: The value {null} could still be provided to a variable at runtime, and +a non-null argument must produce a field error if provided such a variable. ```graphql example query booleanArgQueryWithDefault($booleanArg: Boolean = true) { diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md index f636b01ea..9783dc4c6 100644 --- a/spec/Section 6 -- Execution.md +++ b/spec/Section 6 -- Execution.md @@ -88,21 +88,22 @@ CoerceVariableValues(schema, operation, variableValues): name {variableName}. * Let {value} be the value provided in {variableValues} for the name {variableName}. - * If {hasValue} is not {true} or if {value} is {null}: - * If {variableType} is a Non-Nullable type, throw a query error. - * Otherwise if {defaultValue} exists (including {null}): - * Add an entry to {coercedValues} named {variableName} with the - value {defaultValue}. - * Otherwise if {hasValue} is {true} and {value} is {null}: + * If {hasValue} is not {true} and {defaultValue} exists (including {null}): + * Add an entry to {coercedValues} named {variableName} with the + value {defaultValue}. + * Otherwise if {variableType} is a Non-Nullable type, and either {hasValue} + is not {true} or {value} is {null}, throw a query error. + * Otherwise if {hasValue} is true: + * If {value} is {null}: * Add an entry to {coercedValues} named {variableName} with the value {null}. - * Otherwise, {value} must not be {null}: - * If {value} cannot be coerced according to the input coercion - rules of {variableType}, throw a query error. - * Let {coercedValue} be the result of coercing {value} according to the - input coercion rules of {variableType}. - * Add an entry to {coercedValues} named {variableName} with the - value {coercedValue}. + * Otherwise: + * If {value} cannot be coerced according to the input coercion + rules of {variableType}, throw a query error. + * Let {coercedValue} be the result of coercing {value} according to the + input coercion rules of {variableType}. + * Add an entry to {coercedValues} named {variableName} with the + value {coercedValue}. * Return {coercedValues}. Note: This algorithm is very similar to {CoerceArgumentValues()}. @@ -552,34 +553,35 @@ CoerceArgumentValues(objectType, field, variableValues): * Let {defaultValue} be the default value for {argumentDefinition}. * Let {hasValue} be {true} if {argumentValues} provides a value for the name {argumentName}. - * Let {value} be the value provided in {argumentValues} for the + * Let {argumentValue} be the value provided in {argumentValues} for the name {argumentName}. - * If {hasValue} is not {true} or if {value} is {null}: - * If {argumentType} is a Non-Nullable type, throw a field error. - * Otherwise, if {defaultValue} exists (including {null}): - * Add an entry to {coercedValues} named {argumentName} with the - value {defaultValue}. - * Otherwise, if {hasValue} is {true} and {value} is {null}: + * If {argumentValue} is a {Variable}: + * Let {variableName} be the name of {argumentValue}. + * Let {hasValue} be {true} if {variableValues} provides a value for the + name {variableName}. + * Let {value} be the value provided in {variableValues} for the + name {variableName}. + * Otherwise: + * Let {value} be {argumentValue}. + * If {hasValue} is not {true} and {defaultValue} exists (including {null}): + * Add an entry to {coercedValues} named {argumentName} with the + value {defaultValue}. + * Otherwise if {argumentType} is a Non-Nullable type, and either {hasValue} + is not {true} or {value} is {null}, throw a field error. + * Otherwise if {hasValue} is true: + * If {value} is {null}: * Add an entry to {coercedValues} named {argumentName} with the value {null}. - * Otherwise, if {value} is a {Variable}: - * Let {variableName} be the name of {value}. - * If {variableValues} provides a value for the name {variableName}: - * Let {variableValue} be the value provided in {variableValues} for the - name {variableName}. + * Otherwise, if {argumentValue} is a {Variable}: * Add an entry to {coercedValues} named {argumentName} with the - value {variableValue}. - * Otherwise, if {argumentType} is a Non-Nullable type, throw a field error. - * Otherwise, if {defaultValue} exists (including {null}): + value {value}. + * Otherwise: + * If {value} cannot be coerced according to the input coercion + rules of {variableType}, throw a field error. + * Let {coercedValue} be the result of coercing {value} according to the + input coercion rules of {variableType}. * Add an entry to {coercedValues} named {argumentName} with the - value {defaultValue}. - * Otherwise, {value} must be a {Value} and not {null}: - * If {value} cannot be coerced according to the input coercion - rules of {argType}, throw a field error. - * Let {coercedValue} be the result of coercing {value} according to the - input coercion rules of {argType}. - * Add an entry to {coercedValues} named {argumentName} with the - value {coercedValue}. + value {coercedValue}. * Return {coercedValues}. Note: Variable values are not coerced because they are expected to be coerced From 2fbeb5db9ab238fe9fc82773ce9d67da4ad8cdea Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Mon, 2 Apr 2018 20:07:20 -0700 Subject: [PATCH 3/7] One step further towards the idealized "from scratch" proposal, this makes it more explicitly clear that changing the effective type of a variable definition is only relevent when supporting legacy clients and suggests that new services should not use this behavior. I like that this balances a clear description of how this rule should work for existing services along with a stricter and therefore safer future path for new services. --- spec/Section 5 -- Validation.md | 86 +++++++++++++++++++-------------- spec/Section 6 -- Execution.md | 3 +- 2 files changed, 51 insertions(+), 38 deletions(-) diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md index a85f60d10..8b29db6d2 100644 --- a/spec/Section 5 -- Validation.md +++ b/spec/Section 5 -- Validation.md @@ -727,9 +727,8 @@ and invalid. **Explanatory Text** -Arguments can be required. If the argument type is non-null and does not have a -default value, the argument is required and furthermore the explicit value -{null} may not be provided. Otherwise, the argument is optional. +Arguments can be required. An argument is required if the argument type is +non-null and does not have a default value. Otherwise, the argument is optional. For example the following are valid: @@ -761,7 +760,8 @@ fragment missingRequiredArg on Arguments { } ``` -Providing the explicit value {null} is also not valid. +Providing the explicit value {null} is also not valid since required arguments +always have a non-null type. ```graphql counter-example fragment missingRequiredArg on Arguments { @@ -1378,10 +1378,10 @@ For example the following query will not pass validation. **Explanatory Text** -Input object fields can be required. If the input object field type is non-null -and does not have a default value, the input object field is required and -furthermore the explicit value {null} may not be provided. Otherwise, the input -object field is optional. +Input object fields may be required. Much like a field may have required +arguments, an input object may have required fields. An input field is required +if it has a non-null type and does not have a default value. Otherwise, the +input object field is optional. ## Directives @@ -1821,15 +1821,12 @@ an extraneous variable. **Formal Specification** - * For each {operation} in {document} - * Let {variableUsages} be all usages transitively included in the {operation} - * For each {variableUsage} in {variableUsages} - * Let {variableName} be the name of {variableUsage} + * For each {operation} in {document}: + * Let {variableUsages} be all usages transitively included in the {operation}. + * For each {variableUsage} in {variableUsages}: + * Let {variableName} be the name of {variableUsage}. * Let {variableDefinition} be the {VariableDefinition} in {operation}. - * Let {variableType} be the type of {variableDefinition}. - * Let {defaultValue} be the default value of {variableDefinition}. - * If {variableType} is not a non-null and {defaultValue} is provided and not {null}: - * Let {variableType} be the non-null of {variableType}. + * Let {variableType} be {EffectiveVariableType(variableDefinition)}. * Let {argumentType} be the type of the argument {variableUsage} is passed to. * AreTypesCompatible({argumentType}, {variableType}) must be {true}. @@ -1839,7 +1836,7 @@ AreTypesCompatible(argumentType, variableType): * Let {nullableArgumentType} be the nullable type of {argumentType}. * Let {nullableVariableType} be the nullable type of {variableType}. * Return {AreTypesCompatible(nullableArgumentType, nullableVariableType)}. - * Otherwise return {false}. + * Otherwise, return {false}. * If {variableType} is a non-null type: * Let {nullableVariableType} be the nullable type of {variableType}. * Return {AreTypesCompatible(argumentType, nullableVariableType)}. @@ -1848,9 +1845,9 @@ AreTypesCompatible(argumentType, variableType): * Let {itemArgumentType} be the item type of {argumentType}. * Let {itemVariableType} be the item type of {variableType}. * Return {AreTypesCompatible(itemArgumentType, itemVariableType)}. - * Otherwise return {false}. - * If {variableType} is a list type return {false}. - * Return if {variableType} and {argumentType} are identical. + * Otherwise, return {false}. + * If {variableType} is a list type, return {false}. + * Return {true} if {variableType} and {argumentType} are identical, otherwise {false}. **Explanatory Text** @@ -1894,21 +1891,6 @@ query booleanArgQuery($booleanArg: Boolean) { } ``` -A notable exception is when a default value is provided. Variables which include -a default value may be provided to a non-null argument as long as the default -value is not {null}. - -Note: The value {null} could still be provided to a variable at runtime, and -a non-null argument must produce a field error if provided such a variable. - -```graphql example -query booleanArgQueryWithDefault($booleanArg: Boolean = true) { - arguments { - nonNullBooleanArgField(nonNullBooleanArg: $booleanArg) - } -} -``` - For list types, the same rules around nullability apply to both outer types and inner types. A nullable list cannot be passed to a non-null list, and a list of nullable values cannot be passed to a list of non-null values. @@ -1933,5 +1915,37 @@ query listToNonNullList($booleanList: [Boolean]) { ``` This would fail validation because a `[T]` cannot be passed to a `[T]!`. - Similarly a `[T]` cannot be passed to a `[T!]`. + +**Supporting legacy operations** + +EffectiveVariableType(variableDefinition): + * Let {variableType} be the expected type of {variableDefinition}. + * If service supports operations written before this specification edition: + * If {variableType} is not a non-null type: + * Let {defaultValue} be the default value of {variableDefinition}. + * If {defaultValue} is provided and not the value {null}: + * Return the non-null of {variableType}. + * Otherwise, return {variableType}. + +A notable exception to type compatibility a GraphQL service may support is +allowing a variable definition with a nullable type and provided default value +to be provided to a non-null position as long as that variable's default value +is also not the value {null}. + +```graphql example +query booleanArgQueryWithDefault($booleanArg: Boolean = true) { + arguments { + nonNullBooleanArgField(nonNullBooleanArg: $booleanArg) + } +} +``` + +Earlier editions of this specification explicitly allowed such operations due to +a slightly different interpretation of default values and {null} values. GraphQL +services accepting operations written before this edition of the specification +may continue to allow such operations, however GraphQL services established +after this edition should not allow such operations. + +Note: The value {null} could still be provided to a such a variable at runtime. +A non-null argument must produce a field error if provided a {null} value. diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md index 9783dc4c6..d2575b625 100644 --- a/spec/Section 6 -- Execution.md +++ b/spec/Section 6 -- Execution.md @@ -561,8 +561,7 @@ CoerceArgumentValues(objectType, field, variableValues): name {variableName}. * Let {value} be the value provided in {variableValues} for the name {variableName}. - * Otherwise: - * Let {value} be {argumentValue}. + * Otherwise, let {value} be {argumentValue}. * If {hasValue} is not {true} and {defaultValue} exists (including {null}): * Add an entry to {coercedValues} named {argumentName} with the value {defaultValue}. From ee6220bc5126f6778a66004de3814162dea75f51 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Thu, 5 Apr 2018 10:54:16 -0700 Subject: [PATCH 4/7] Editing AreTypesCompatible() to avoid trailing "Otherwise return false" statements for easier reading. Functionality is equivalent. --- spec/Section 5 -- Validation.md | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md index 8b29db6d2..49835155c 100644 --- a/spec/Section 5 -- Validation.md +++ b/spec/Section 5 -- Validation.md @@ -1832,21 +1832,19 @@ an extraneous variable. AreTypesCompatible(argumentType, variableType): * If {argumentType} is a non-null type: - * If {variableType} is a non-null type: - * Let {nullableArgumentType} be the nullable type of {argumentType}. - * Let {nullableVariableType} be the nullable type of {variableType}. - * Return {AreTypesCompatible(nullableArgumentType, nullableVariableType)}. - * Otherwise, return {false}. - * If {variableType} is a non-null type: + * If {variableType} is NOT a non-null type, return {false}. + * Let {nullableArgumentType} be the nullable type of {argumentType}. + * Let {nullableVariableType} be the nullable type of {variableType}. + * Return {AreTypesCompatible(nullableArgumentType, nullableVariableType)}. + * Otherwise, if {variableType} is a non-null type: * Let {nullableVariableType} be the nullable type of {variableType}. * Return {AreTypesCompatible(argumentType, nullableVariableType)}. - * If {argumentType} is a list type: - * If {variableType} is a list type: - * Let {itemArgumentType} be the item type of {argumentType}. - * Let {itemVariableType} be the item type of {variableType}. - * Return {AreTypesCompatible(itemArgumentType, itemVariableType)}. - * Otherwise, return {false}. - * If {variableType} is a list type, return {false}. + * Otherwise, if {argumentType} is a list type: + * If {variableType} is NOT a list type, return {false}. + * Let {itemArgumentType} be the item type of {argumentType}. + * Let {itemVariableType} be the item type of {variableType}. + * Return {AreTypesCompatible(itemArgumentType, itemVariableType)}. + * Otherwise, if {variableType} is a list type, return {false}. * Return {true} if {variableType} and {argumentType} are identical, otherwise {false}. **Explanatory Text** From 429bf2be067f997610d529290a18807a9947b74d Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Fri, 6 Apr 2018 12:42:44 -0700 Subject: [PATCH 5/7] Update "All Variable Usages are Allowed" to remove breaking change. Also attempts to improve clarity and formatting and adds an example case. --- spec/Section 5 -- Validation.md | 85 +++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 36 deletions(-) diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md index 49835155c..c911ab0f6 100644 --- a/spec/Section 5 -- Validation.md +++ b/spec/Section 5 -- Validation.md @@ -676,6 +676,7 @@ type Arguments { intArgField(intArg: Int): Int nonNullBooleanArgField(nonNullBooleanArg: Boolean!): Boolean! booleanListArgField(booleanListArg: [Boolean]!): [Boolean] + optionalNonNullBooleanArgField(optionalBooleanArg: Boolean! = false): Boolean! } extend type Query { @@ -1824,28 +1825,41 @@ an extraneous variable. * For each {operation} in {document}: * Let {variableUsages} be all usages transitively included in the {operation}. * For each {variableUsage} in {variableUsages}: - * Let {variableName} be the name of {variableUsage}. - * Let {variableDefinition} be the {VariableDefinition} in {operation}. - * Let {variableType} be {EffectiveVariableType(variableDefinition)}. - * Let {argumentType} be the type of the argument {variableUsage} is passed to. - * AreTypesCompatible({argumentType}, {variableType}) must be {true}. - -AreTypesCompatible(argumentType, variableType): - * If {argumentType} is a non-null type: + * {IsVariableUsageAllowed(variableUsage)} must be {true}. + +IsVariableUsageAllowed(variableUsage): + * Let {variableName} be the name of {variableUsage}. + * Let {variableDefinition} be the {VariableDefinition} in {operation}. + * Let {variableType} be the expected type of {variableDefinition}. + * Let {locationType} be the expected type of the {Argument}, {ObjectField}, + or {ListValue} entry where {variableUsage} is located. + * If {locationType} is a non-null type AND {variableType} is NOT a non-null type: + * Let {hasNonNullVariableDefaultValue} be {true} if a default value exists + for {variableDefinition} and is not the value {null}. + * Let {hasLocationDefaultValue} be {true} if a default value exists for + the {Argument} or {ObjectField} where {variableUsage} is located. + * If {hasNonNullVariableDefaultValue} is NOT {true} AND + {hasLocationDefaultValue} is NOT {true}, return {false}. + * Let {nullableLocationType} be the unwrapped nullable type of {locationType}. + * Return {AreTypesCompatible(variableType, nullableLocationType)}. + * Return {AreTypesCompatible(variableType, locationType)}. + +AreTypesCompatible(variableType, locationType): + * If {locationType} is a non-null type: * If {variableType} is NOT a non-null type, return {false}. - * Let {nullableArgumentType} be the nullable type of {argumentType}. - * Let {nullableVariableType} be the nullable type of {variableType}. - * Return {AreTypesCompatible(nullableArgumentType, nullableVariableType)}. + * Let {nullableLocationType} be the unwrapped nullable type of {locationType}. + * Let {nullableVariableType} be the unwrapped nullable type of {variableType}. + * Return {AreTypesCompatible(nullableVariableType, nullableLocationType)}. * Otherwise, if {variableType} is a non-null type: * Let {nullableVariableType} be the nullable type of {variableType}. - * Return {AreTypesCompatible(argumentType, nullableVariableType)}. - * Otherwise, if {argumentType} is a list type: + * Return {AreTypesCompatible(nullableVariableType, locationType)}. + * Otherwise, if {locationType} is a list type: * If {variableType} is NOT a list type, return {false}. - * Let {itemArgumentType} be the item type of {argumentType}. - * Let {itemVariableType} be the item type of {variableType}. - * Return {AreTypesCompatible(itemArgumentType, itemVariableType)}. + * Let {itemLocationType} be the unwrapped item type of {locationType}. + * Let {itemVariableType} be the unwrapped item type of {variableType}. + * Return {AreTypesCompatible(itemVariableType, itemLocationType)}. * Otherwise, if {variableType} is a list type, return {false}. - * Return {true} if {variableType} and {argumentType} are identical, otherwise {false}. + * Return {true} if {variableType} and {locationType} are identical, otherwise {false}. **Explanatory Text** @@ -1915,21 +1929,21 @@ query listToNonNullList($booleanList: [Boolean]) { This would fail validation because a `[T]` cannot be passed to a `[T]!`. Similarly a `[T]` cannot be passed to a `[T!]`. -**Supporting legacy operations** +**Allowing optional variables when default values exist** -EffectiveVariableType(variableDefinition): - * Let {variableType} be the expected type of {variableDefinition}. - * If service supports operations written before this specification edition: - * If {variableType} is not a non-null type: - * Let {defaultValue} be the default value of {variableDefinition}. - * If {defaultValue} is provided and not the value {null}: - * Return the non-null of {variableType}. - * Otherwise, return {variableType}. - -A notable exception to type compatibility a GraphQL service may support is -allowing a variable definition with a nullable type and provided default value -to be provided to a non-null position as long as that variable's default value -is also not the value {null}. +A notable exception to typical variable type compatibility is allowing a +variable definition with a nullable type to be provided to a non-null location +as long as either that variable or that location provides a default value. + +```graphql example +query booleanArgQueryWithDefault($booleanArg: Boolean) { + arguments { + optionalNonNullBooleanArgField(optionalBooleanArg: $booleanArg) + } +} +``` + +In the example above, an optional variable is allowed to be used in an non-null argument which provides a default value. ```graphql example query booleanArgQueryWithDefault($booleanArg: Boolean = true) { @@ -1939,11 +1953,10 @@ query booleanArgQueryWithDefault($booleanArg: Boolean = true) { } ``` -Earlier editions of this specification explicitly allowed such operations due to -a slightly different interpretation of default values and {null} values. GraphQL -services accepting operations written before this edition of the specification -may continue to allow such operations, however GraphQL services established -after this edition should not allow such operations. +In the example above, a variable provides a default value and can be used in a +non-null argument. This behavior is explicitly supported for compatibility with +earlier editions of this specification. GraphQL authoring tools may wish to +report this is a warning with the suggestion to replace `Boolean` with `Boolean!`. Note: The value {null} could still be provided to a such a variable at runtime. A non-null argument must produce a field error if provided a {null} value. From 314768994d8f853904fd225a1666699b27ed5dbb Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Fri, 6 Apr 2018 16:23:56 -0700 Subject: [PATCH 6/7] Make related changes to input object coercion rules --- spec/Section 3 -- Type System.md | 43 ++++++++++++++++++++------------ spec/Section 5 -- Validation.md | 6 ++--- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/spec/Section 3 -- Type System.md b/spec/Section 3 -- Type System.md index e52cfbbe0..d24580dc7 100644 --- a/spec/Section 3 -- Type System.md +++ b/spec/Section 3 -- Type System.md @@ -1262,25 +1262,36 @@ type of an Object or Interface field. **Input Coercion** The value for an input object should be an input object literal or an unordered -map supplied by a variable, otherwise an error should be thrown. In either -case, the input object literal or unordered map should not contain any entries +map supplied by a variable, otherwise an error must be thrown. In either +case, the input object literal or unordered map must not contain any entries with names not defined by a field of this input object type, otherwise an error -should be thrown. +must be thrown. The result of coercion is an unordered map with an entry for each field both -defined by the input object type and provided with a value. If the value {null} -was provided, an entry in the coerced unordered map must exist for that field. -In other words, there is a semantic difference between the explicitly provided -value {null} versus having not provided a value. - -The value of each entry in the coerced unordered map is the result of input -coercion of the value provided for that field for the type of the field defined -by the input object type - -Any non-nullable field defined by the input object type which does not have -a corresponding entry in the original value, or is represented by a variable -which was not provided a value, or for which the value {null} was provided, an -error should be thrown. +defined by the input object type and for which a value exists. The resulting map +is constructed with the following rules: + +* If no value is provided for a defined input object field and that field + definition provides a default value, the default value should be used. If no + default value is provided and the input object field's type is non-null, an + error should be thrown. Otherwise, if the field is not required, then no entry + is added to the coerced unordered map. + +* If the value {null} was provided for an input object field, and the field's + type is not a non-null type, an entry in the coerced unordered map is given + the value {null}. In other words, there is a semantic difference between the + explicitly provided value {null} versus having not provided a value. + +* If a literal value is provided for an input object field, an entry in the + coerced unordered map is given the result of coercing that value according + to the input coercion rules for the type of that field. + +* If a variable is provided for an input object field, the runtime value of that + variable must be used. If the runtime value is {null} and the field type + is non-null, a field error must be thrown. If no runtime value is provided, + the variable definition's default value should be used. If the variable + definition does not provide a default value, the input object field + definition's default value should be used. Following are examples of input coercion for an input object type with a `String` field `a` and a required (non-null) `Int!` field `b`: diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md index c911ab0f6..3dd60e324 100644 --- a/spec/Section 5 -- Validation.md +++ b/spec/Section 5 -- Validation.md @@ -739,7 +739,7 @@ fragment goodBooleanArg on Arguments { } fragment goodNonNullArg on Arguments { - requiredBooleanArgField(requiredBooleanArg: true) + nonNullBooleanArgField(nonNullBooleanArg: true) } ``` @@ -757,7 +757,7 @@ but this is not valid on a required argument. ```graphql counter-example fragment missingRequiredArg on Arguments { - requiredBooleanArgField + nonNullBooleanArgField } ``` @@ -766,7 +766,7 @@ always have a non-null type. ```graphql counter-example fragment missingRequiredArg on Arguments { - requiredBooleanArgField(requiredBooleanArg: null) + nonNullBooleanArgField(nonNullBooleanArg: null) } ``` From 03197f85a5ee5e18f4dbe90e0037532b100d780f Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Wed, 18 Apr 2018 16:11:07 -0400 Subject: [PATCH 7/7] Final review edits --- spec/Section 5 -- Validation.md | 9 +++++---- spec/Section 6 -- Execution.md | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/spec/Section 5 -- Validation.md b/spec/Section 5 -- Validation.md index 3dd60e324..15c11d4ea 100644 --- a/spec/Section 5 -- Validation.md +++ b/spec/Section 5 -- Validation.md @@ -1825,11 +1825,12 @@ an extraneous variable. * For each {operation} in {document}: * Let {variableUsages} be all usages transitively included in the {operation}. * For each {variableUsage} in {variableUsages}: - * {IsVariableUsageAllowed(variableUsage)} must be {true}. + * Let {variableName} be the name of {variableUsage}. + * Let {variableDefinition} be the {VariableDefinition} named {variableName} + defined within {operation}. + * {IsVariableUsageAllowed(variableDefinition, variableUsage)} must be {true}. -IsVariableUsageAllowed(variableUsage): - * Let {variableName} be the name of {variableUsage}. - * Let {variableDefinition} be the {VariableDefinition} in {operation}. +IsVariableUsageAllowed(variableDefinition, variableUsage): * Let {variableType} be the expected type of {variableDefinition}. * Let {locationType} be the expected type of the {Argument}, {ObjectField}, or {ListValue} entry where {variableUsage} is located. diff --git a/spec/Section 6 -- Execution.md b/spec/Section 6 -- Execution.md index d2575b625..124877bd5 100644 --- a/spec/Section 6 -- Execution.md +++ b/spec/Section 6 -- Execution.md @@ -538,8 +538,8 @@ order to correctly produce a value. These arguments are defined by the field in the type system to have a specific input type: Scalars, Enum, Input Object, or List or Non-Null wrapped variations of these three. -At each argument position in a query may be a literal {Value} or {Variable} to -be provided at runtime. +At each argument position in a query may be a literal {Value}, or a {Variable} +to be provided at runtime. CoerceArgumentValues(objectType, field, variableValues): * Let {coercedValues} be an empty unordered Map.