luau-lang / luau

A fast, small, safe, gradually typed embeddable scripting language derived from Lua
https://luau.org
MIT License
3.98k stars 373 forks source link

Variable's type sometimes unions with `nil` due to `if` statement with new type solver's strict mode #1269

Open Ketasaja opened 4 months ago

Ketasaja commented 4 months ago

Tentative conditions, for a variable v of type t:

  1. Before the if statement, v is last assigned to either the result of a function call, or a table's value where the table's keys' type is not string.
  2. The if condition includes a function call that directly passes v.
  3. Within the if statement, v is not reassigned to anything before its first use.
--!strict
local function f(): number
    return 0
end

local function g(v: number): true
    return true
end

do
    local i = f()

    if g(i) then
        i += 1 -- TypeError: Type 'number?' could not be converted into 'number'; type number?[0] (nil) is not a subtype of number (number)
    end
end

do
    local j = f()

    if g(j + 0) then
        j += 1
    end
end

do
    local k = f() + 0

    if g(k) then
        k += 1
    end
end

do
    local l = f()

    if g(l) then
        l = 0
        l += 1
    end
end

do
    local x: {[string]: number} = {
        v = 0,
    }

    local m = x.v

    if g(m) then
        m += 1
    end
end

do
    local y = {
        0,
    }

    local n = y[1]

    if g(n) then
        n += 1 -- TypeError: Type 'number?' could not be converted into 'number'; type number?[0] (nil) is not a subtype of number (number)
    end
end

Tested on 0.627, with no errors in non-strict mode.

aatxe commented 4 months ago

Here is some context about why this is happening.

There's currently an affordance that indexing a table with an indexer returns a non-nil type. This is present because (at some point) we believed users would find it too frustrating to have to check for nil on every index, and we did not have the type refinement and typestate machinery to necessarily support inferring it automatically.

For example,

local x = { 1, 2, 3 }
if x[4] then
    fib(x[5])
end

In this case, it's not really clear what behavior is possible with the systems in place that doesn't lead to issues (either DX issues or unsoundness issues). This applies as well to function returns because you can return the results from indexing:

function getFourth<T>(tbl: {T}): T
    return tbl[4]
end

The current design point we're at for the new solver here is that indexers (and functions, as a result) can return nil without the type reflecting that, but we want people to be able to still write guards which require them to be able to compare to nil, and so, in conditional contexts, the type of function results and indexing results include implicit nil possibilities.

This lets you still write a check like...

if x[4] ~= nil then
    use(x[4])
end

without requiring refinements to be able to support a refinement per-individual-index and so forth.

All that context being said, it seems like the bug here is that it, for whatever reason, is continuing into the ensuing scope which is unexpected. The type shouldn't be changed, only temporarily viewed as containing nil in a conditional context.