Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type checker widens annotated variables returned by functions to be nilable after equality comparisons #1517

Open
Ukendio opened this issue Nov 10, 2024 · 1 comment
Labels
bug Something isn't working new solver This issue is specific to the new solver.

Comments

@Ukendio
Copy link

Ukendio commented Nov 10, 2024

Variables explicitly declared as non-nilable types are incorrectly inferred as nilable after being assigned from function returns, particularly when involved in equality comparisons (==). This leads to unexpected type errors when accessing properties or reassigning these variables within conditional blocks, despite their explicit non-nilable annotations.

Minimal code example

type MyType = {
    data: any
}

local function createMyType(): MyType
    local obj = { data = {} }
    return obj
end

local function testTypeInference()
    local a: MyType = createMyType()
    local b: MyType = createMyType()

    if a == b then
        -- 'b' is inferred as nilable here
        local c: MyType = b  -- TypeError: Type 'MyType?' could not be converted into 'MyType'; type MyType?[0] (nil) is not a subtype of MyType (MyType)
        local value = b.data -- TypeError: Value of type 'MyType?' could be nil
    end
end

Expected Behaviour:

Variables declared as non-nilable and assigned from functions that return non-nilable types should remain non-nullable throughout their scope.

The type checker should not infer b as nilable simply because it was assigned from a function return and involved in an equality comparison.

Actual Behaviour:

The type checker infers b as nilable (MyType?) within the if block, leading to type errors when accessing its properties or assigning it to another non-nilable variable.

Note I am not quite sure whether to describe the issue as the type being widened or that the nilability is not being propagated into the conditional blocks following equality comparisons.

@Ukendio Ukendio added the bug Something isn't working label Nov 10, 2024
@aatxe aatxe added the new solver This issue is specific to the new solver. label Nov 10, 2024
@aatxe
Copy link
Collaborator

aatxe commented Nov 10, 2024

The equality comparison is the thing applying the refinement here that makes it potentially nil. This addition exists because our equality check errors if there are no common inhabitants, but we wanted to allow checks against nil anyway because of indexers lying about the result. We treat function call results as potentially an indexer result since we cannot tell from the function's type.

There's sort of two potential ways forward here, but it might make sense for us to do both: (1) the additional possible nil should be limited strictly to just the equality comparison anyway, and (2) we could limit that additional or nil further to only be added when the comparison is against nil in particular.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working new solver This issue is specific to the new solver.
Development

No branches or pull requests

2 participants