Overload intellisense issue with type of parameters

[Rathoz]

How are you using the lua-language-server?

Visual Studio Code Extension (sumneko.lua)

Which OS are you using?

Windows

What is the issue affecting?

Type Checking

Expected Behaviour

a is string|number

Actual Behaviour

a is string

Reproduction steps

---@overload fun(a: string): string
---@overload fun(a: number): table
function Foo.Bar(a)
	-- `a` is incorrectly assumed to be a `string`
	return a:lower() -- This does not error while it should!
end

Additional Notes

No response

Log File

No response

[tomlau10]

I think I've found the cause of this issue, in the compileFunctionParam() of here:

lua-language-server/script/vm/compiler.lua

Lines 1090 to 1107 in ba8f90e

	local function compileFunctionParam(func, source)
	-- local call ---@type fun(f: fun(x: number));call(function (x) end) --> x -> number
	local funcNode = vm.compileNode(func)
	for n in funcNode:eachObject() do
	if n.type == 'doc.type.function' then
	for index, arg in ipairs(n.args) do
	if func.args[index] == source then
	local argNode = vm.compileNode(arg)
	for an in argNode:eachObject() do
	if an.type ~= 'doc.generic.name' then
	vm.setNode(source, an)
	end
	end
	return true
	end
	end
	end
	end

• It returns true immediately after using the first function type's params' inferred view to set into the function object itself
• Thus the param in the actual function only has the first type defined in the overload, instead of the combined type of all overloads

---

I tried to remove this early break, and it can solve this issue:

local function compileFunctionParam(func, source)
    -- local call ---@type fun(f: fun(x: number));call(function (x) end) --> x -> number
    local funcNode = vm.compileNode(func)
    local found = false
    for n in funcNode:eachObject() do
        ...
        found = true -- replace the original `return true`
        ...
    end
    if found then
        return true
    end

However this fix cause a few new problems...

• When the function is a callargs type (in another word callback) and the type narrow function fail to find the exact signature, this will cause the params in the callback function have combined types, and this is undesirable in many cases.
  • An example that I can think of is in the comment of an already closed issue: Function overloads with function args not narrowing inner function params #2758 (comment)
  • with this suggested patch, the following code will be problematic:

  local e = defines.events
  script.on_event(e.on_ai_command_completed, function (event)
      -- before patch: `event` = `EventData.on_ai_command_completed`
      -- after patch: `event` = `EventData|EventData.on_ai_command_completed`
  end)

  • this is because the type narrow did not perform well in this case, and 2 signatures matches, one with the event as EventData.on_ai_command_completed, the other with type EventData
  • and this patch will combine the 2...
• Another problematic case is in this issue: overload doesn't seem to match correctly #2509
  It actually works partially as of LuaLS v3.10.5. It only fails when pass rvalue as the type, because the base function is inferred as:

  function expect(x: ffi.cdata*, type: "rvalue")
    -> gccjit.LValue*|gccjit.RValue*|gccjit.Type*

  • the reason is the same as described in this issue, because only the first overload param types are used when inferring the function params' type.
  • with this fix, it will make things worse...

  function expect(x: ffi.cdata*, type: "lvalue"|"rvalue"|"type")
    -> gccjit.LValue*|gccjit.RValue*|gccjit.Type*

  • with whichever type value: "lvalue"|"rvalue"|"type", all will match this base function, and no type narrow can be performed 😇

  local lval = expect(data, "lvalue")
    -> gccjit.LValue*|gccjit.RValue*|gccjit.Type*
    -- because "lvalue" now matches the base function signature

PS: I am working on PR #2822 to try to fix that #2509 🤔

[tomlau10]

I just found out that even with my #2838, now although a in Foo.Bar() can be correctly inferred as string|number, a:lower() will not trigger warning 😅 . I think that's because the string.lower actually accepts number as the 1st param:

lua-language-server/meta/template/string.lua

Lines 85 to 88 in d1320ae

	---@param s string|number
	---@return string
	---@nodiscard
	function string.lower(s) end

string.lower indeed accepts string|number as 1st param, just that it doesn't allow number:lower() call style, and it is not addressed in this issue nor in my PR #2838.

Maybe @Rathoz you have to open another issue for that 😂

[Rathoz]

Correct on string.lower() accpect string|number.

number doesn't have any metatable, unlike string. aString:lower() is just syntactic sugar for aString.lower(aString) which thanks the metatable turns into string.lower(aString). But do the same on a number aNumber:lower() is tries to call the function aNumber.lower(aNumber), which doesn't exist.

I can open a new issue on it if you like