Function overloads with function args not narrowing inner function params #2758

Activity

1. justarandomgeek commented on Jul 14, 2024

   

   justarandomgeek

   on Jul 14, 2024

   More actions

   The third parameter, filters, is irrelevant to the bug.

   well, it kind of is but it's mostly a repeat of the same thing as the second: it has narrower types when properly matched, or doesn't exist at all on some overloads.

   I'd also note that our defines.events enum (like all of our enums) is a little odd specifically to make it an opaque enum - the actual numeric values are not supposed to be visible anywhere, so they're masked away to just their types here for the overload resolution to see. I suspect this is involved somehow, but i'm not sure why it would cause any trouble.
2. tomlau10 commented on Jul 15, 2024

   

   tomlau10

   on Jul 15, 2024

   Last edited by tomlau10

   Contributor

   More actions

   I think I've been able to reproduce this issue more reliably.

   Setup

   First create a workspace with the following files:

   • .luarc.json
     this enables the inlay function param hint for better understanding what variable type that LuaLS is currently inferred.

   {
       "hint.enable": true
   }

   • meta.lua
     a minimized definition file modified from the provided gist
     each EventData sub class has a field of its name, with this we can know if the event type is correctly inferred

   ---@meta _
   
   defines = {}
   
   script = {
   ---@param event (defines.events)|(string)|(((defines.events)|(string))[])
   ---@param handler (fun(event:EventData))|(nil)
   ---@overload fun(event:string, handler:fun(event:EventData.CustomInputEvent))
   ---@overload fun(event:defines.events.on_ai_command_completed, handler:fun(event:EventData.on_ai_command_completed))
   ---@overload fun(event:defines.events.on_area_cloned, handler:fun(event:EventData.on_area_cloned))
   ---@overload fun(event:integer, handler:fun(event:EventData))
   on_event = function(event, handler) end
   }
   
   ---@enum defines.events
   defines.events={
   on_ai_command_completed=#{} --[[@as defines.events.on_ai_command_completed]],
   on_area_cloned=#{} --[[@as defines.events.on_area_cloned]],
   }
   
   ---@class EventData
   ---@field name defines.events
   ---@field tick integer
   ---@field mod_name? string
   
   ---@class EventData.CustomInputEvent:EventData
   ---@field CustomInputEvent true
   
   ---@class EventData.on_ai_command_completed:EventData
   ---@field on_ai_command_completed true
   
   ---@class EventData.on_area_cloned:EventData
   ---@field on_area_cloned true

   • test.lua
     yes, the event name is missing a d, but this just sets up the reproduction environment 🙂

   local e = defines.events
   script.on_event(e.on_ai_command_complete, function (event)
   end)

   Reproduction steps

   • Now with the above setup, e.on_ai_command_complete is actually undefined
     • so it is of unknown type
     • and from the hover preview of handler, it matches all function definitions and selected EventData as param type
     • somewhat expected ✅
   • Then we type the missing d => e.on_ai_command_completed
     • now it will be correctly displayed as defines.events.on_ai_command_completed
     • but the handler still matches all function definitions, type narrowing seems doesn't triggered ❌
   • ‼️ At this stage, any modification of the file seems to retrigger the type narrowing
     For example, just adding a trailing newline at the bottom
   • Save the file and reload vscode window now (use the Reload window cmd)
     • at file load, the handler type narrowing is correct
     • ‼️ But then, by just adding trailing a new line, the type narrow will stop working ... 🙄

   Thoughts

   • I'm not very familiar with LuaLS's codebase, but just by skimming through the files, the logic of finding matched functions call seems to reside in vm.getExactMatchedFunctions
     

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

     Line 359 in ddc96bd

     function vm.getExactMatchedFunctions(func, args)

   • This returns an array of matched function signature (?), and its only caller is the matchCall() from here
     

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

     Lines 546 to 560 in ddc96bd

     	local function matchCall(source)
     	local call = source.parent
     	if not call
     	or call.type ~= 'call'
     	or call.node ~= source then
     	return
     	end
     	local myNode = vm.getNode(source)
     	if not myNode then
     	return
     	end
     	local funcs = vm.getExactMatchedFunctions(source, call.args)
     	if not funcs then
     	return
     	end

   • I tried to add a debug print there print("matchCall", #funcs)
     It correctly shows matched number when I type / delete the d in e.on_ai_command_completed
     • after I typed d to make it e.on_ai_command_completed => matchCall 2
     • then I deleted the d to revert it to e.on_ai_command_complete => matchCall 4
   • So this vm.getExactMatchedFunctions logic seems working correctly, and will trigger on every file change.
     Just that inferred view is not updated correctly 😕
3. tomlau10 commented on Jul 16, 2024

   

   tomlau10

   on Jul 16, 2024

   Contributor

   More actions

   Some updates and findings on this issue:

   But then, by just adding trailing a new line, the type narrow will stop working ...

   By adding or removing trailing new lines on the above reproduction file that I provided, this issue randomly triggers 😕
   Each time I add/remove newline, it may correct itself, or trigger the bug.

   As far as I know, the inferred view of a source object is based on its node's view. And in order to change / add a view to a node, vm.setNode(src, node) is called. So I added some debugging print log / debug.traceback() there when the source object is matched.

   ---

   Some interesting findings:

   • When the type narrowing is working, all the stacktraces start with:

   	script/core/semantic-tokens.lua:893: in local 'callback'
   	script/parser/guide.lua:710: in function 'parser.guide.eachSourceBetween'
   	script/core/semantic-tokens.lua:888: in function 'core.semantic-tokens'
   	script/provider/provider.lua:1116: in function <script/provider/provider.lua:1104>
   	[C]: in function 'xpcall'
   	script/proto/proto.lua:200: in function <script/proto/proto.lua:175>
   

   and the node object will have 3 views in the end [function, doc.type.function*2]

   • On the other hand when the bug triggers, all the stacktraces start with:

   	script/core/diagnostics/init.lua:119: in upvalue 'check'
   	script/core/diagnostics/init.lua:181: in function 'core.diagnostics'
   	[C]: in function 'xpcall'
   	script/provider/diagnostic.lua:337: in function 'provider.diagnostic.doDiagnostic'
   	[C]: in function 'xpcall'
   	script/provider/diagnostic.lua:463: in function <script/provider/diagnostic.lua:457>
   

   and the node object will have 6 views in the end [function, doc.type.function*5]

   ---

   Each time the file is edited, the file will be re-parsed, and the diagnostics will be rechecked. And the above suggests that if a node recompilation is triggered by diagnostic, the final inferred view will be incorrect 😳 .

   • I then tried to delay the trigger of diagnostic by 1s here:

     lua-language-server/script/provider/diagnostic.lua

     Lines 457 to 464 in f221eb3

     	await.call(function ()
     	await.setID('diag:' .. uri)
     	repeat
     	await.sleep(0.1)
     	until not m.isPaused()
     	xpcall(m.doDiagnostic, log.error, uri)
     	end)
     	end

     
     changed to =>

   await.call(function ()
       await.sleep(1) --<< sleep 1 second
       await.setID('diag:' .. uri)

   🎉 The triggering chance of this random bug is greatly reduced (to <1% on each edit)

   ---

   This is of course not a proper fix, but at worst may be a way to "mitigate" the effect of this bug. 🤔
4. tomlau10 commented on Jul 17, 2024

   

   tomlau10

   on Jul 17, 2024

   Contributor

   More actions

   Good news 🎉 I have found the root cause of this issue. The logic doesn't clear node caches of call.args after doing the function type narrowing, thus their views are not recomputed based on the type narrowed function.

   It's the call order of vm.compileNode() which matters. I guess in the problematic call order, the arg nodes are keeping the outdated cache of parent which has all overload signatures.

   To fix this, the logic has to clear call.args node caches after setting the type narrowed node in matchCall():
   

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

   Lines 573 to 577 in f221eb3

   	if needRemove then
   	local newNode = myNode:copy()
   	newNode:removeNode(needRemove)
   	newNode.originNode = myNode
   	vm.setNode(source, newNode, true)

   
   insert codes below at line 577 =>

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

   ---

   I have tested this against the reproduction case above, and it solves the problem 🎉 Both the random bug triggered when adding / removing trailing newlines, as well as just after completed the typing e.on_ai_command_completed. I will soon open a PR for this 😄

   Meanwhile can you guys test it out to confirm if it can solve the issue? Say by just modifying the source code of your installed extension locally. @CodeGreen0386 @justarandomgeek

   🎉React with 🎉2Reacted by PennyJim and _CodeGreen
5. sumneko

   closed this as completedin #2765on Jul 22, 2024