Save nullness information in the AST

[MangelMaxime]

I am only working on JS/TS code for now, which explains why others target fails in the CI.

With the nullable information we have trouble with TypeScript compiler type inference:

let length (s: string | null) =
    match s with
    | null -> 0
    | s -> s.Length

compiles to

export function length(s: Nullable<string>): int32 {
    if (equals(s, defaultOf())) {
        return 0;
    }
    else {
        const s_1: string = s;
        return s_1.length | 0;
    }
}

But TS compiler generated an error on const s_1: string = s; because of Type 'Nullable<string>' is not assignable to type 'string'.

I think what we would like to do is, detect match/if operation against null and there the check for null inline directly or using a dedicated helpers function to make TypeScript understand that we check the null | undefined case.

Here we are defaulting to the default equality which works but don't gives enough information to TypeScript

[MangelMaxime]

This PR is actually surfacing issues with our TypeScript generation when using Nullable type:

let nullableTestWorking (s: Nullable<Test>) =
    // Works because we have generate a type guard s != null
    if s.HasValue then s.Value.Length else 0

let nullableTestFailing (s: Nullable<Test>) =
    // Fails because we don't have a type guard s != null
    s.Value.Length

export function nullableTestWorking(s: Nullable<Test>): int32 {
    if (s != null) {
        return value(s).Length | 0;
    }
    else {
        return 0;
    }
}

export function nullableTestFailing(s: Nullable<Test>): int32 {
    return value(s).Length | 0;
}

[MangelMaxime]

I am facing a wall here, because it feels like when we generate the type information for

let copyOfStruct: int32 = value(s)

we don't have the information that we are coming from a Nullable type. Do we need to propagate an additional information somewhere to say, here we come from nullable don't do a standard cast but instead force it using XX as int ?

@ncave do you have any ideas?

[ncave]

@MangelMaxime One would think that defensive struct copy would not be applicable to (nullable) reference types, but I'm not sure.

[MangelMaxime]

One would think that defensive struct copy would not be applicable to (nullable) reference types, but I'm not sure.

Perhaps indeed.

But this is a similar issue for the first snippet:

export function length(s: Nullable<string>): int32 {
    if (equals(s, defaultOf())) {
        return 0;
    }
    else {
        const s_1: string = s;
        return s_1.length | 0;
    }
}

Should be written like as

export function length(s: Nullable<string>): int32 {
    if (s == null) {
        return 0;
    }
    else {
        const s_1: string = s;
        return s_1.length | 0;
    }
}

I found a way to replace defaultOf() with null for nullable. Now the idea would be to detect calls to equals against to rewrite it to a direct equality so TS is happy.

But, I am not sure if there are others places where doing such things would break the code. I am just trying to prototype something fixing one problem at a time to see what happens 😇

[MangelMaxime]

Hello @ncave

I think this is the main missing feature in order to make Fable 5 stable, do you have an idea how to fix it?

In order to release Fable 5 I see the following options:

1. We release Fable 5, without Nullable information in the AST. This means we will probably need a Fable 6 version
2. We release Fable 5, with Nullable information in the AST as proposed in this PR without being sure if this is good enough/usable.

Nullable should work for JavaScript already because everything is happening in the runtime, the issue is more for typed languages especially TypeScript which want a specific way of writing the code for its nullable check to happen.

My current issue, is that we have a lot of fixes in Fable 5, that could be useful to Fable users. And some of them, don't want to use an alpha/beta version of Fable by security.

One of the solution is to make Fable 5 stable happens, even with a smaller feature set (see above).

Or to back port fixes to Fable 4, using git cherry-pick. I started doing that, it doesn't seems half bad but means maintaining 2 branches at the same time. And they are not exactly the same because I can't/don't want to back port F# 9 features, latest FCS version, etc.

What do you think we should do?

[ncave]

@MangelMaxime We could, technically, not make any changes to the Fable.AST, and just wrap nullable types in a Nullable as a marker, and then handle wrapped nullable types appropriately in the printer.

I.e. instead of Fable.Nullable typ, use makeRuntimeType [ typ ] Types.nullable to represent it as Nullable<T>.

But honestly, either way works.

[ncave]

This has been merged into and superseded by #4159.