Overloaded methods do not properly return a generic bound to the self parameter

[GrandpaScout]

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

Given the following code:

---@class A
local A = {}

---@generic S
---@param self S
---@param str string
---@return S
function A:method1(str) end

---@generic S
---@param self S
---@param num1 number
---@param num2? number
---@param num3? number
---@return S
function A:method1(num1, num2, num3) end

---@class B: A
local B = {}

local x = B:method1("str")
local y = B:method1(1)
local z = B:method1(1, 2, 3)

The following should be true:

local x = B:method1("str")   --> local x: B
local y = B:method1(1)       --> local y: B
local z = B:method1(1, 2, 3) --> local z: B

All three local variables should be the B type as that is what generic S would be during this method call.

Actual Behaviour

The following is given instead by LuaLS:

local x = B:method1("str")   --> local x: unknown
local y = B:method1(1)       --> local y: unknown
local z = B:method1(1, 2, 3) --> local z: B

From my testing, it seems that given any amount of overloads and a method call the following seems to happen. Whether this is actually what happens in LuaLS is a different story as I could not figure out why this happens.

• Change the method call's parameters to be all anys
  B:method1("str") --> B:method1(any)
• Check if that new call matches more than one overload.
  B:method1(any) matches A:method1(str: string)
B:method1(any) matches A:method1(num1: number, num2?: number, num3?: number)
• If multiple overloads were matched, return unknown instead.

Reproduction steps

1. Paste the following code into a lua file and open it:
   (Alternatively, use the test cases file found in the "Additional Notes" section)

   ---@class A
   local A = {}
   
   ---@generic S
   ---@param self S
   ---@param str string
   ---@return S
   function A:method1(str) end
   
   ---@generic S
   ---@param self S
   ---@param num1 number
   ---@param num2? number
   ---@param num3? number
   ---@return S
   function A:method1(num1, num2, num3) end
   
   ---@class B: A
   local B = {}
   
   local x = B:method1("str")
   local y = B:method1(1)
   local z = B:method1(1, 2, 3)

2. Hover over locals x, y, and z.
3. Notice that locals x and y have the wrong type while local z is fine.

Additional Notes

This method of returning self is being done because the self keyword does not actually resolve to the class being used for the method call and always seems to be class that created the method instead.

This error started appearing in 3.10.1 and has not been fixed yet as of 3.13.6.

(The testing was done before 3.13.6 was released and I have personally tested 3.13.6 and confirm that it is still the same as 3.13.5)

The following file was used to get the information for the above chart.
testcases.txt

Reverting the changes made by #2898 and #2765 appears to fix this issue (while also re-introducing the bugs those PRs fixed.)

I have searched for other issues related to this, but only found mostly unrelated issues. However, i did find one comment on issue #723 that mentions this exact bug and the PR following it (#2898) seemed to only fix half of the issue. (As shown by version 3.12.0 correcting more than half of the test cases in the test case file.)

Log File

file_d%3A__test.log

[tomlau10]

Hi, I am the author of #2898 and #2765 (though I am not author of LuaLS).

This is a very detailed bug report 👍
I will try to take a look when I have time.
But I cannot promise when / or even if possible I can come up a fix with it 🙁 .

---

It's been a while since I wrote those fix PRs.
I can give more details on it in case you want to do further debugging yourself.

PR #2765

This tries to fix #2758, where the infer on functions with overloads are inconsistent.

• After quite some debugging, I believe the server is keeping outdated function node cache.
• So the fix is to reset those nodes to allow recomputing.
• This is released in 3.10.0

run time error in #2776

• However the fix at that time caused a run time
• sumneko (author of LuaLS) fixed it in commit 8ecec08
• and this "fix" is released in 3.10.1

The regression fix

• As you have already found out, that I then found a regression issue while discussing in [Feature request] generics in overload #723
• I came up with the fix in fix: regression related to type narrow and generic since v3.10.1 #2898, but I was unsure how that issue occur
• And quite unfortunately, this seemed to only fix half of the issue 🤦‍♂

Seems we need to find out why the run time error fix in 8ecec08 (3.10.1) is problematic 🤔

[tomlau10]

known facts

• 3.10.0 is correct
• PR#2765 is released in 3.10.0
  • => PR#2765 itself has no problem
  • i.e. unrelated to PR#2765
• from the provided additional note, this issue is introduced in 3.10.1
• the only major logic change in 3.10.1 is the run time error fix in 8ecec08
  diff: 3.10.0...3.10.1
  • => the change there must be the culprit

---

the overly complicated fix in 8ecec08

8ecec08 is the runtime error fix to #2776.
To be honest, now I don't know why @sumneko comes up with such a complicated fix 🙈
(I don't have enough knowledge in LuaLS's codebase at that time)

I reinvestigated #2776, the root cause seems to be that:

• if an arg node in call.args is already compiled by the time of removing unmatched function signature
  • this compiled arg node must exist afterwards
  • i.e. simply clearing it by vm.removeNode() will lead to possible index nil value error in that callstack / callframe
• So the simplest fix would be just recompiling it immediately after removing 🤔

the fix

• replace the logic here:
  

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

  Lines 643 to 660 in 64c7084

  	if call.args then
  	-- clear existing node caches of args to allow recomputation with the type narrowed call
  	for _, arg in ipairs(call.args) do
  	if vm.getNode(arg) then
  	vm.setNode(arg, vm.createNode(), true)
  	end
  	end
  	for n in newNode:eachObject() do
  	if n.type == 'function'
  	or n.type == 'doc.type.function' then
  	for i, arg in ipairs(call.args) do
  	if vm.getNode(arg) and n.args[i] then
  	vm.setNode(arg, vm.compileNode(n.args[i]))
  	end
  	end
  	end
  	end
  	end

• with:

        if call.args then
            -- recompile existing node caches of args to allow recomputation with the type narrowed call
            for _, arg in ipairs(call.args) do
                if vm.getNode(arg) then
                    vm.removeNode(arg)
                    vm.compileNode(arg)
                end
            end
        end

This seems passed all existing tests, as well as the reproduction snippet in your example 😄
I will try to open a PR soon.

[GrandpaScout]

Gonna be honest, I might have confused 2765 with another PR, that's my bad. I did actually look into what was causing this but LuaLS is a complex beast and I couldn't find anything that would lead to the solution, so I'm glad you found something. Thanks for looking into this so quickly.