Skip to content

Commit

Permalink
RFC: Fix ambiguity with null variable values and default values (#418)
Browse files Browse the repository at this point in the history 
This change corresponds to a spec proposal (#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:

* 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 (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.
  • Loading branch information
@leebyron
leebyron committed Apr 18, 2018
1 parent 2b2467a commit 0356f0c
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 129 deletions.
43 changes: 27 additions & 16 deletions spec/Section 3 -- Type System.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1263,25 +1263,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`:
Expand Down
184 changes: 109 additions & 75 deletions spec/Section 5 -- Validation.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -710,25 +711,25 @@ 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}.
* {value} must not be the {null} literal.

**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. 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:

Expand All @@ -752,19 +753,20 @@ 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
}
```

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 {
notNullBooleanArgField(nonNullBooleanArg: null)
nonNullBooleanArgField(nonNullBooleanArg: null)
}
```

Expand Down Expand Up @@ -1358,6 +1360,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 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


Expand Down Expand Up @@ -1494,44 +1521,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**
Expand Down Expand Up @@ -1833,20 +1822,45 @@ 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 {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.
* 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} named {variableName}
defined within {operation}.
* {IsVariableUsageAllowed(variableDefinition, variableUsage)} must be {true}.

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.
* 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 {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(nullableVariableType, locationType)}.
* Otherwise, if {locationType} is a list type:
* If {variableType} is NOT a list type, return {false}.
* 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 {locationType} are identical, otherwise {false}.

**Explanatory Text**

Expand Down Expand Up @@ -1890,17 +1904,6 @@ query booleanArgQuery($booleanArg: Boolean) {
}
```

A notable exception is when default arguments are provided. They are, in effect,
treated as non-nulls.

```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.
Expand All @@ -1925,5 +1928,36 @@ 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!]`.

**Allowing optional variables when default values exist**

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) {
arguments {
nonNullBooleanArgField(nonNullBooleanArg: $booleanArg)
}
}
```

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.
Loading

0 comments on commit 0356f0c

Please sign in to comment.