reject null variable values in non-null list items and input fields - #4507

Ye old nullability on variables - this has long been a challenge around is a value present but null versus null because its absent.

I am not sure this is correct or that we can tell here if a value is present and was set explicitly to null versus being null

We may have to use graphql.execution.CoercedVariables#containsKey to distinguish

I would like @andimarek to comment on this PR

fair concern, but i don't think the distinction changes the outcome at either site.

externalValueToInternalValueForVariables only puts a key into CoercedVariables when the variable was supplied (explicit null included) or has a default, so get() returning null here means either an explicit null or an absent variable with no default. for a non-null target both are invalid: graphql-js valueFromAST rejects a missing variable in a non-null list item the same way it rejects an explicit null. the one place absent has to behave differently is an input field with a default, and that branch already goes through containsKey (hasValue) and runs before the line i changed, so an absent variable still picks up the field default.

i ran the combinations against this branch to check:

query($v: String = "x") { list(arg: [$v]) }            no variables  -> [x]
query($v: String = "x") { list(arg: [$v]) }            {"v": null}   -> Coerced Null value for NonNull type 'String!'
query($v: String) { nullableItems(arg: [$v]) }          no variables  -> [null]  (unchanged)
query($v: String) { nullableItems(arg: [$v]) }          {"v": null}   -> [null]  (unchanged)
query($v: String) { obj(arg: {a: $v}) }   a: String! = "d"   no variables -> [a:d]
query($v: String) { obj(arg: {a: $v}) }   a: String! = "d"   {"v": null}  -> Field 'a' has coerced Null value for NonNull type 'String!'

an absent variable without a default can only reach the list item check with validation bypassed (a nullable variable with no default isn't allowed in a [String!] position, and a non-null variable fails variable coercion first), and rejecting it there still matches graphql-js. happy to switch the list item check to containsKey if you'd rather see the absent case spelled out, but the result would be the same.