VM.OnCompileFunctionParam doesn't function as documented

[Wild-W]

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?

Plugins

Expected Behaviour

Defining the global VM.OnCompileFunctionParam in a plugin should create an intermediary step in compiling function parameters.

Actual Behaviour

Attempt to index nil 'VM' and when setting VM manually, nothing happens.

Reproduction steps

1. Create plugin.lua
2. Set runtime.plugin to the plugin.lua directory
3. Insert this code into plugin.lua

local vm = require "vm"
local log = require "log"

log.info("Loaded test plugin!") -- Appears in log

function OnCompileFunctionParam(next, func, param)
    log.info("Inside OnCompileFunctionParam!") -- Does not appear in log
    -- Set type to number (test)
    vm.setNode(param, vm.declareGlobal('type', 'number'))
    return true
end

-- VM does not exist, so I tried declaring it globally
VM = VM or {}
VM.OnCompileFunctionParam = OnCompileFunctionParam

-- Also does not work when returned
return {
    VM = VM,
    OnCompileFunctionParam = OnCompileFunctionParam
}

4. Restart LLS and observe that the plugin loads by verifying in the logs but does not set the parameters of any functions to numbers.

Additional Notes

Image demonstrating that the parameters are not numbers.

If this is a bug, I would be happy to look into it myself if the maintainers have their hands full with other issues. Let me know if I should.

If this is not a bug and in some way, it's actually my own user error, then perhaps some changes should be made to the documentation to better reflect the intended functionality of OnCompileFunctionParam. On that note, there are actually a few more issues with the documentation for OnCompileFunctionParam that should probably be addressed, since the code snippet used as example appears to be copy-pasted from the tests and does not define many of the variables it references. require is also misspelt as reuqire on the snippet's first line.

Log File

file_c%3A_smbx%20lls%20plugin.log

[Wild-W]

I found the problem, though with the way the original code for this system was written, it's not clear what the plan was in the first place.

plugin.lua, line 63:

function m.getVmPlugin(uri)
    local scp = scope.getScope(uri)
    ---@type pluginInterfaces
    local interfaces = scp:get('pluginInterfaces')
    if not interfaces then
        return
    end
    return interfaces.VM
end

getVmPlugin assumes that scp:get('pluginInterfaces') returns a pluginInterfaces, but it actually returns a pluginInterfaces[]. Replacing return interfaces.VM with return interfaces[1].VM solves the issue, but only if a maximum of one plugin is loaded.

To support multiple plugins, compiler.lua, line 1389 will need to be reworked, because it only seems to assume there is one plugin to check from and the next function will always be the default compileFunctionParam.

local hasDocArg = vmPlugin and vmPlugin.OnCompileFunctionParam(compileFunctionParam, func, source)
            or compileFunctionParam(func, source)

This code would likely need to become an iterative algorithm that loops over each call to OnCompileFunctionParam until one returns true.

I propose something like this:

-- compiler.lua
    if source.parent.type == 'funcargs' and not hasMarkDoc and not hasMarkParam then
        local func = source.parent.parent
        local vmPlugins = plugin.getVmPlugin(guide.getUri(source))
        if vmPlugins then
            local hasDocArg = false
            for _, vmPlugin in ipairs(vmPlugins) do
                hasDocArg = vmPlugin.VM.OnCompileFunctionParam(compileFunctionParam, func, source)
                if hasDocArg then break end
            end
            if not hasDocArg then
                vm.setNode(source, vm.declareGlobal('type', 'any'))
            end
        end
    end

-- plugin.lua
function m.getVmPlugin(uri)
    local scp = scope.getScope(uri)
    ---@type pluginInterface
    local interfaces = scp:get('pluginInterfaces')
    if not interfaces then
        return
    end
    return interfaces -- changed from `return interfaces.VM`
end

Of course the names of several functions and types should be changed accordingly.

This piece of code will also become redundant, which in turn will safely allow us to delete the createMethodGroup function and pluginConfigs table.

        for key, config in pairs(pluginConfigs) do
            interfaces[key] = createMethodGroup(interfaces, key, config)
        end

In case the changes here aren't to the maintainers' likings, I'll hold off on opening a pull request until I get the go ahead.

Thanks for your time.

[CppCXY]

You can directly make a PR