diff --git a/changelog.md b/changelog.md index c9375a4cd..f24ab2346 100644 --- a/changelog.md +++ b/changelog.md @@ -2,6 +2,7 @@ ## Unreleased +* `FIX` Reapplied the type inference fix for `and`/`or` idioms and circular dependency in tracer (reverted in 3.18.2). The tracer now drops caches populated while an assignment was still being compiled, so if-blocks that reassign a variable no longer lose the assignment's type in subsequent code [#2236](https://github.com/LuaLS/lua-language-server/issues/2236) [#2374](https://github.com/LuaLS/lua-language-server/issues/2374) [#2494](https://github.com/LuaLS/lua-language-server/issues/2494) * `NEW` Support type inference for `@field` and `@type` function declarations in method overrides [#3367](https://github.com/LuaLS/lua-language-server/issues/3367) * `FIX` Deduplicate documentation bindings for parameters * `FIX` Correct `math.type` meta return annotation to use `nil` instead of the string literal `'nil'` diff --git a/script/core/diagnostics/no-unknown.lua b/script/core/diagnostics/no-unknown.lua index e706931ad..51f5858fa 100644 --- a/script/core/diagnostics/no-unknown.lua +++ b/script/core/diagnostics/no-unknown.lua @@ -26,11 +26,29 @@ return function (uri, callback) guide.eachSourceTypes(ast.ast, types, function (source) await.delay() if vm.getInfer(source):view(uri) == 'unknown' then - callback { - start = source.start, - finish = source.finish, - message = lang.script('DIAG_UNKNOWN'), - } + -- When a node only contains a 'variable' object whose base + -- declaration has a known type, this is a false positive caused + -- by circular dependency during compilation, not a true unknown. + local dominated = false + local node = vm.getNode(source) + if node then + for n in node:eachObject() do + if n.type == 'variable' and n.base and n.base.value then + local baseView = vm.getInfer(n.base):view(uri) + if baseView ~= 'unknown' then + dominated = true + break + end + end + end + end + if not dominated then + callback { + start = source.start, + finish = source.finish, + message = lang.script('DIAG_UNKNOWN'), + } + end end end) end diff --git a/script/vm/tracer.lua b/script/vm/tracer.lua index cc6d10e59..3317d8ed1 100644 --- a/script/vm/tracer.lua +++ b/script/vm/tracer.lua @@ -638,9 +638,22 @@ local lookIntoChild = util.switch() tracer:lookIntoChild(action[2], topNode) return topNode, outNode end - if action.op.type == 'and' then - topNode = tracer:lookIntoChild(action[1], topNode, topNode:copy()) - topNode = tracer:lookIntoChild(action[2], topNode, topNode:copy()) + if action.op.type == 'and' then + local topNode1, outNode1 = tracer:lookIntoChild(action[1], topNode, topNode:copy()) + topNode = tracer:lookIntoChild(action[2], topNode1, topNode1:copy()) + -- When the right side of `and` is a truthy literal (string, number, + -- true, table, function), the `and` can only be false when the left + -- side is false. Propagate the narrowed outNode so that patterns + -- like `x == nil and "default" or x` correctly infer x as non-nil. + local tp2 = action[2].type + if tp2 == 'string' + or tp2 == 'number' + or tp2 == 'integer' + or tp2 == 'table' + or tp2 == 'function' + or (tp2 == 'boolean' and action[2][1] == true) then + outNode = outNode1 + end elseif action.op.type == 'or' then outNode = outNode or topNode:copy() local topNode1, outNode1 = tracer:lookIntoChild(action[1], topNode, outNode) @@ -844,7 +857,54 @@ function mt:calcNode(source) return end if self.assignMap[source] then + -- Guard against circular dependency: when this setlocal is already + -- being compiled on this coroutine (e.g. if-handler's getNode + -- triggers calcNode for a setlocal whose value is currently being + -- compiled), skip lookIntoBlock to avoid propagating incomplete + -- types and setting marks that would prevent later correct + -- processing. The outer call purges everything cached meanwhile. + local co = coroutine.running() + local compiling = self._compilingAssigns and self._compilingAssigns[co] + if compiling and compiling[source] then + self._reentered = (self._reentered or 0) + 1 + self.nodes[source] = vm.compileNode(source) + return + end + if not self._compilingAssigns then + -- weak keys so entries of dead coroutines can be collected + self._compilingAssigns = setmetatable({}, { __mode = 'k' }) + end + if not compiling then + compiling = {} + self._compilingAssigns[co] = compiling + end + compiling[source] = true + local reentered = self._reentered or 0 local node = vm.compileNode(source) + compiling[source] = nil + if (self._reentered or 0) ~= reentered then + -- A re-entrant calcNode happened while this assign was being + -- compiled: nodes cached since then may be based on its + -- incomplete type (e.g. an if-branch merged without this + -- assignment's effect, losing it for all code after the block). + -- Drop the caches so later lookups recompute with the final type. + self.nodes = {} + self.mark = {} + end + -- When the compiled node has no known type (only contains a 'variable' + -- due to circular dependency), fall back to the variable's base + -- declaration node. This prevents incomplete nodes from propagating + -- through control flow analysis (e.g. if-blocks inside for-loops), + -- which would otherwise cause the type to degrade to 'unknown'. + if not node:hasKnownType() + and self.mode == 'local' + and self.source.type == 'variable' + and self.source.base then + local baseNode = vm.compileNode(self.source.base) + if baseNode:hasKnownType() then + node = baseNode + end + end self.nodes[source] = node local parentBlock = guide.getParentBlock(source) if parentBlock then diff --git a/test/type_inference/init.lua b/test/type_inference/init.lua index b45a1a4dd..b3b9d2dec 100644 --- a/test/type_inference/init.lua +++ b/test/type_inference/init.lua @@ -45,5 +45,6 @@ function TEST(wanted) end require 'type_inference.common' +require 'type_inference.reassign_narrow' require 'type_inference.param_match' require 'type_inference.field_override' diff --git a/test/type_inference/reassign_narrow.lua b/test/type_inference/reassign_narrow.lua new file mode 100644 index 000000000..b43f51eb6 --- /dev/null +++ b/test/type_inference/reassign_narrow.lua @@ -0,0 +1,98 @@ +-- Narrowing across if-blocks that reassign the variable, when an earlier +-- query position inside the branch is compiled first (as happens in a real +-- editor session). Regression tests for the revert in 3.18.2: the circular +-- dependency guard must not lose the branch assignment's type in code after +-- the block. +local files = require 'files' +local guide = require 'parser.guide' +local catch = require 'catch' +local vm = require 'vm' + +local function getSource(pos) + local state = files.getState(TESTURI) + if not state then + return + end + local result + guide.eachSourceContain(state.ast, pos, function (source) + if source.type == 'local' + or source.type == 'getlocal' + or source.type == 'setlocal' + or source.type == 'setglobal' + or source.type == 'getglobal' + or source.type == 'field' + or source.type == 'method' + or source.type == 'function' + or source.type == 'table' + or source.type == 'doc.type.name' then + result = source + end + end) + return result +end + +-- Queries `` first and `` second against the same file text, +-- so caches populated by the first query are visible to the second. +local function TEST2(wanted1, wanted2) + return function (script) + local newScript, catched = catch(script, '?!') + files.setText(TESTURI, newScript) + local source1 = getSource(catched['?'][1][1]) + assert(source1) + local result1 = vm.getInfer(source1):view(TESTURI) + assert(wanted1 == result1, ('Query 1 failed! Wanted: %s Got: %s'):format(wanted1, result1)) + local source2 = getSource(catched['!'][1][1]) + assert(source2) + local result2 = vm.getInfer(source2):view(TESTURI) + assert(wanted2 == result2, ('Query 2 failed! Wanted: %s Got: %s'):format(wanted2, result2)) + files.remove(TESTURI) + end +end + +TEST2('string', 'string?') [[ +---@type integer? +local x + +if x then + x = x .. 's' + print() +end + +print() +]] + +TEST2('integer', 'integer?') [[ +---@type integer? +local x + +if x then + x = x + 1 + print() +end + +print() +]] + +TEST2('string', 'string?') [[ +---@type integer? +local x + +if x then + x = tostring(x) + print() +end + +print() +]] + +TEST2('string', 'string|integer') [[ +---@type integer? +local x + +if not x then + x = 's' .. tostring(x) + print() +end + +print() +]]