Check if input object CLR property type does not match the scalar's CLR type declared in GraphQL object definition

[xperiandri]

No description provided.

[xperiandri]

When we declare scalar with a validator like this

type AddressLine1 = ValidString<Phantom.Address.Line1>

module Address =
    open ValidString
    open Validus.Operators

    let createLine1 : Validator<string, AddressLine1 voption> =
        (allowEmpty ?=> (Check.String.lessThanLen 1000 <+> validateStringCharacters)) *|* ValueOption.map ValidString

let Line1Type : ScalarDefinition<string, AddressLine1 voption> = Define.ValidStringScalar<AddressLine1>("AddressLine1", createLine1, "Address line 1")

it appears that the type of scalar becomes voption<AddressLine1>
and then if we declare it Nullable

Define.Input("line1", Nullable Address.Line1Type)

it is double wrapped like option<voption<AddressLine1>>
The unwrapping itself happes on execution well but in order to perform check here I need to try unwrapping it 2 times

[xperiandri]

@valbers what do you think?

[valbers]

@valbers what do you think?

It looks overall good. Please just take a look at my comments.

[valbers]

If you're sure it will never be null, then:

Suggested change

	let expectedOutputType = outputType.GenericTypeArguments.FirstOrDefault()
	let expectedOutputType = outputType.GenericTypeArguments.First()

Otherwise, I think it's a better idea to actively throw an exception in case of Default:

if expectedOutputType = null
then
   failwith "Expected output type was null. This is a bug in FSharp.Data.GraphQL."

[xperiandri]

No, my point was that the expectedOutputType value is used only in if clauses that a relevant for Option and ValueOption types

[valbers]

Ok. Just keep in mind that by doing this you create a logical dependency in the lines of code and someday someone might introduce a bug by not considering your initial intention with Option and ValueOption.

[xperiandri]

I have no quick idea how to improve that 🙂

[valbers]

Suggested change

	let result = from.IsAssignableTo ``to`` || checkCollections from ``to``
	if result then result
	else
	if from.FullName.StartsWith OptionTypeName || from.FullName.StartsWith ValueOptionTypeName then
	let from = from.GetGenericArguments()[0]
	from.IsAssignableTo ``to`` || checkCollections from ``to``
	else result
	from.IsAssignableTo ``to`` || checkCollections from ``to`` ||
	((from.FullName.StartsWith OptionTypeName || from.FullName.StartsWith ValueOptionTypeName) && ((from.GetGenericArguments()[0]).IsAssignableTo ``to`` || checkCollections from ``to``))

also notice how the else result in your code is superfluous, since result is always false at that point.

[xperiandri]

It is hard to read

[valbers]

ok, but did you also read the part:

also notice how the else result in your code is superfluous, since result is always false at that point.

?

[xperiandri]

Yep but do you see wrong in that?