Dofile inference#3431
Conversation
There was a problem hiding this comment.
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.
| ---@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 | ||
|
|
There was a problem hiding this comment.
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
| 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 |
There was a problem hiding this comment.
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
| 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 |
There was a problem hiding this comment.
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
| 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." |
| 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." |
| 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." |
| 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." |
There was a problem hiding this comment.
| 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." |
There was a problem hiding this comment.
This adds type inference for
dofilesimilar to howrequireworks. Also adds a new settingLua.workspace.dofileRootsto control whatdofileconsiders 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 fromchore: changelogif you don't want the docs changes.