fix: reapply tracer circular dependency fix without breaking if-block narrowing

changelog.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 <!-- Add all new changes here. They will be moved under a version at release -->
+* `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'`

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

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

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'

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(<?x?>)
+end
+
+print(<!x!>)
+]]
+
+TEST2('integer', 'integer?') [[
+---@type integer?
+local x
+
+if x then
+    x = x + 1
+    print(<?x?>)
+end
+
+print(<!x!>)
+]]
+
+TEST2('string', 'string?') [[
+---@type integer?
+local x
+
+if x then
+    x = tostring(x)
+    print(<?x?>)
+end
+
+print(<!x!>)
+]]
+
+TEST2('string', 'string|integer') [[
+---@type integer?
+local x
+
+if not x then
+    x = 's' .. tostring(x)
+    print(<?x?>)
+end
+
+print(<!x!>)
+]]