Problem reported where null is not supported by a provided type after SDK update

[dsyme]

See "Problem 1" in this thread: https://github.com/fsprojects/FSharp.TypeProviders.SDK/issues/164#issuecomment-340908311

1. Clone the repo https://github.com/fsprojects/SQLProvider and git checkout tp-sdk-problem
2. Run build.cmd to restore package. It will fail to unit tests...
3. Open in Visual Studio: SQLProvider.sln and build. Works.
4. Problem 1: From Solution Explorer, open file doc\core\crud.fsx and observe that isNull foundEmployee is underlined by compiler, saying it cannot be null. But it can, as there is no searched ID in the database. You can run the file in FSI and observe that foundEmployee is null.

[Thorium]

I think there is already a test case that demonstrates this failure:

There is currently a test provider: /examples/ComboProvider And test for that: /examples/ComboProvider.Tests

Combo-provider has a method that returns a class:

type SomeRuntimeHelper() = 
    static member Help() = "help"

This is a class and not a struct (even if there is only a static method), right? So null should be allowed. This class is exposed from a method StaticMethod which has default isNonNullable which is false, so again, null should be allowed. But the null check fails.

A little fsx script demonstrating the problem:

#I @"examples/ComboProvider/bin/Debug/netstandard2.0"
#r "ComboProvider.dll"

// works, creates a myObj (which is null):
let myObj = ComboProvider.Provided.MyType.StaticMethod() 

// This is the regression, previously such check was allowed:
let checkNull = ComboProvider.Provided.MyType.StaticMethod() = null 
// produces now: error FS0043:
// The type 'ProviderImplementation.SomeRuntimeHelper' does not have 'null' as a proper value

[dsyme]

@Thorium This is expected - the validity of null as a value is determined on a type-by-type basis. Types defined in F# files are, by default, non-nullable. So SomeRuntmeHelper is a non-nullable type. The fact that the type provider returns a null value for something of this type would be considered a bug (i.e. invariant violation) in the type provider.

I'm pretty sure this worked the same way in previous versions of the TPSDK. If you need the helper type to be nullable, then put [<AllowNullLiteral>] on the type.

It may be that your case is a little more complicated and the returned type is an erased type, rather than a helper type defined a library. If that's the case please let me know and I'll look into it further

[dsyme]

@Thorium I added more testing for these cases in https://github.com/fsprojects/FSharp.TypeProviders.SDK/pull/193 and things seem to be working as expected. Please do reopen and provide more info if you're still having trouble, thanks

[Thorium]

It is an erased type, and it did work before. The sad thing is that it still is null at runtime (as in the script above), now I just cannot do the null-check anymore, so the code will fail. I tried to copy the functionality of mkAllowNullLiteralCustomAttributeData and add the custom attribute to the type, but didn't help.

[dsyme]

It is an erased type, and it did work before.

I don't understand this yet - all erased types support null, and always have done.

[dsyme]

I'll reopen until we've nailed what's going on. Could you put together two type provider samples showing exactly what worked before/after? thanks

[Thorium]

Is there a boxing removed from an erased type? I think if I would box the erased types manually, it could work.

[dsyme]

Is there a boxing removed from an erased type? I think if I would box the erased types manually, it could work.

I think really need to see a before ("this used to work") and after ("look,it no longer works") examples to understand what's changed and what to do about it, thanks. To my knowledge there has been no specific change

[Thorium]

My simplified code:

type TypeProviderConnection = SqlDataProvider<...>

/// DataContext is a erased type:
type DataContext = TypeProviderConnection.dataContext

let mutable internal dbContext = Unchecked.defaultof<DataContext>

// Example scenario, web-server lost the DB-connection because of Azure maintenance break:
let ``reinit a broken database-connection``() =
    if dbContext = null then
        dbContext <- TypeProviderConnection.GetDataContext()
    ()

Using the latest nuget, branch https://github.com/fsprojects/SQLProvider/tree/old-sdk "this used to work", using ProvidedTypes.fs commit e0436b11faed9e7938b3c90d874a535f94311b87 this code compiles.

Using the latest master from: https://github.com/fsprojects/SQLProvider "look,it no longer works" using ProvidedTypes.fs commit 5c8b7963f58ead4b85eb2ee4424d524009f763f5 fails on compilation. But I, as a user of the library, can get the code working by boxing manually:

    ...
    if (box dbContext) = null then
    ...

Ok, I think this is not a critical bug as there is the workaround to box manually. But it clearly shows that at some point the type-provier SDK backward compatibility has been broken on erased types and I wonder what actually changed, and will it affect to some other things also.

[dsyme]

My simplified code...

If possible I'd appreciate an exact step-by-step sequence I can reproduce on my machine - where there are no ... to be filled in, even if it means I have to get a database to use. From what I see, dbContext of type TypeProviderConnection.dataContext should indeed support null.

[dsyme]

@Thorium OK, I've worked out the problem. At all the places where you construct a ProvidedTypeDefinition you must explicitly use isErased=true here:

    let serviceType = ProvidedTypeDefinition( "dataContext", None, isErased=true)

instead of this:

    let serviceType = ProvidedTypeDefinition( "dataContext", None, true)

If you don't, you are implicitly setting the nonNullable argument of the constructor. The order of arguments has changed. Instead of changing the API back I think we should just change the SQLProvider code, in the name of API stability

Thanks for sticking with me on this! We found it in the end

[Thorium]

Ok, thanks, one thing that confused me more was that ProvidedTypes.fs is actually setting that only to false and never true, as booleans by default should be false, right?

   if nonNullable then yield mkAllowNullLiteralCustomAttributeData false

[dsyme]

@Thorium If you track it through the true argument you were providing does flow through to that conditional, e.g. https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/b6d5c949e46c34f051e2adf88170e1a2d20cbfd9/src/ProvidedTypes.fs#L1336 then https://github.com/fsprojects/FSharp.TypeProviders.SDK/blob/b6d5c949e46c34f051e2adf88170e1a2d20cbfd9/src/ProvidedTypes.fs#L1328

[Thorium]

Working, thanks! This can be closed.

[dsyme]

@Thorium great! :)