Skip to content

Dofile inference#3431

Closed
NathanSnail wants to merge 8 commits into
LuaLS:masterfrom
NathanSnail:dofile_inference
Closed

Dofile inference#3431
NathanSnail wants to merge 8 commits into
LuaLS:masterfrom
NathanSnail:dofile_inference

Conversation

@NathanSnail

Copy link
Copy Markdown
Contributor

This adds type inference for dofile similar to how require works. Also adds a new setting Lua.workspace.dofileRoots to control what dofile considers a root. The doc generator seemingly hasn't been run in a long time which is why the doc changes are so big, potentially merge from chore: changelog if you don't want the docs changes.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for type inference of dofile and adds a new Lua.workspace.dofileRoots configuration setting to specify additional directories to search. The changes span documentation updates, template configuration, core resolution logic, and localization files. Feedback on the implementation highlights potential runtime crashes: first, when handling relative paths in Lua.workspace.dofileRoots without resolving them to absolute paths; second, due to missing type checks on configuration values before iterating with ipairs; and third, when a dofile target returns fewer values than expected, resulting in a nil node compilation. Additionally, translations for the new configuration setting should be provided across the various localization files.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +429 to +444
---@type string[]
local roots = config.get(sourceUri, 'Lua.workspace.dofileRoots')
local results = {}

local function addResult(absPath)
local uris = m.findUrisByFilePath(absPath)
for _, uri in ipairs(uris) do
results[#results+1] = uri
end
end
resultCache[path] = results

for _, root in ipairs(roots) do
local rootUri = furi.encode(root)
addResult(m.getAbsolutePath(rootUri, path))
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The root path retrieved from Lua.workspace.dofileRoots can be a relative path. Passing it directly to furi.encode(root) without resolving it to an absolute path first will result in an invalid URI and failure to locate the file. Resolving it using m.getAbsolutePath(sourceUri or m.rootUri, root) resolves this issue. Additionally, a check is added to ensure roots is a table before iterating to prevent runtime crashes if the configuration is nil or invalid.

    ---@type string[]
    local roots = config.get(sourceUri, 'Lua.workspace.dofileRoots')
    local results = {}

    local function addResult(absPath)
        local uris = m.findUrisByFilePath(absPath)
        for _, uri in ipairs(uris) do
            results[#results+1] = uri
        end
    end

    if type(roots) == 'table' then
        for _, root in ipairs(roots) do
            local apath = m.getAbsolutePath(sourceUri or m.rootUri, root)
            if apath then
                local rootUri = furi.encode(apath)
                addResult(m.getAbsolutePath(rootUri, path))
            end
        end
    end

Comment on lines +232 to +239
for _, path in ipairs(config.get(scp.uri, 'Lua.workspace.dofileRoots')) do
local apath = m.getAbsolutePath(scp.uri, path)
if apath then
local norm = files.normalize(apath)
librarys[norm] = true
dofileRootsSet[norm] = true
end
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The configuration value returned by config.get can be nil or a non-table value if misconfigured. Directly passing it to ipairs will throw a runtime error. Adding a type check prevents this.

    local dofileRoots = config.get(scp.uri, 'Lua.workspace.dofileRoots')
    if type(dofileRoots) == 'table' then
        for _, path in ipairs(dofileRoots) do
            local apath = m.getAbsolutePath(scp.uri, path)
            if apath then
                local norm = files.normalize(apath)
                librarys[norm] = true
                dofileRootsSet[norm] = true
            end
        end
    end

Comment thread script/vm/compiler.lua
Comment on lines +2268 to +2274
local state = files.getState(uri)
local returns = state and state.ast and state.ast.returns and state.ast.returns[1]
if not returns then
return
end
vm.setNode(source, vm.compileNode(returns[index]))
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the target file returns fewer values than expected (e.g., return 1337 but called as local a, b = dofile(...)), returns[index] will be nil. Calling vm.compileNode(nil) can lead to unexpected behavior or runtime errors. Adding a guard check for expr prevents this.

            local state = files.getState(uri)
            local returns = state and state.ast and state.ast.returns and state.ast.returns[1]
            if not returns then
                return
            end
            local expr = returns[index]
            if not expr then
                return
            end
            vm.setNode(source, vm.compileNode(expr))
            return

Comment thread locale/zh-cn/setting.lua
Comment on lines +135 to +136
config.workspace.dofileRoots = -- TODO: need translate!
"In addition to the current workspace, which directories `dofile` will treat as a possible root. The files in these directories will be loaded immediately."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Provide the Simplified Chinese translation for the new Lua.workspace.dofileRoots setting.

config.workspace.dofileRoots          =
"除了当前工作区以外,哪些目录将被 `dofile` 视为可能的根目录。这些目录中的文件将被立即加载。"

Comment thread locale/zh-tw/setting.lua
Comment on lines +135 to +136
config.workspace.dofileRoots = -- TODO: need translate!
"In addition to the current workspace, which directories `dofile` will treat as a possible root. The files in these directories will be loaded immediately."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Provide the Traditional Chinese translation for the new Lua.workspace.dofileRoots setting.

config.workspace.dofileRoots          =
"除了目前工作區以外,哪些目錄將被 `dofile` 視為可能的根目錄。這些目錄中的檔案將被立即載入。"

Comment thread locale/ja-jp/setting.lua
Comment on lines +136 to +137
config.workspace.dofileRoots = -- TODO: need translate!
"In addition to the current workspace, which directories `dofile` will treat as a possible root. The files in these directories will be loaded immediately."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Provide the Japanese translation for the new Lua.workspace.dofileRoots setting.

config.workspace.dofileRoots          =
"現在のワークスペースに加えて、`dofile` が可能なルートとして扱うディレクトリ。これらのディレクトリ内のファイルはすぐに読み込まれます。"

Comment thread locale/pt-br/setting.lua
Comment on lines +136 to +137
config.workspace.dofileRoots = -- TODO: need translate!
"In addition to the current workspace, which directories `dofile` will treat as a possible root. The files in these directories will be loaded immediately."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Provide the Brazilian Portuguese translation for the new Lua.workspace.dofileRoots setting.

config.workspace.dofileRoots          =
"Além do espaço de trabalho atual, quais diretórios o `dofile` tratará como uma possível raiz. Os arquivos nesses diretórios serão carregados imediatamente。"

Comment thread locale/es-419/setting.lua
Comment on lines +137 to +138
config.workspace.dofileRoots = -- TODO: need translate!
"In addition to the current workspace, which directories `dofile` will treat as a possible root. The files in these directories will be loaded immediately."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Provide the Latin American Spanish translation for the new Lua.workspace.dofileRoots setting.

config.workspace.dofileRoots          =
"Además del espacio de trabajo actual, qué directorios tratará `dofile` como una posible raíz. Los archivos en estos directorios se cargarán inmediatamente。"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant