Skip to content

Fix type inference for and/or idioms and circular dependency in tracer#3391

Open
RomanSpector wants to merge 2 commits intoLuaLS:masterfrom
RomanSpector:fix/type-inference-and-or-idiom
Open

Fix type inference for and/or idioms and circular dependency in tracer#3391
RomanSpector wants to merge 2 commits intoLuaLS:masterfrom
RomanSpector:fix/type-inference-and-or-idiom

Conversation

@RomanSpector
Copy link
Copy Markdown

Summary

  • Fix type narrowing for x == nil and "default" or x pattern — the and handler in the tracer now propagates outNode when the right side is a truthy literal, so the or branch correctly infers the variable as non-nil
  • Fix circular dependency in calcNode for assignments inside if/for blocks — a _compilingAssigns guard prevents incomplete types and stale marks from propagating when the if-handler's getNode(lastAssign) triggers recompilation of an already-compiling setlocal
  • Filter no-unknown false positives when a node only contains a variable object whose base declaration has a known type

Closes #2236, #2374, #2494

Examples

Ternary idiom x == nil and "default" or x

---@param a string?
local function test(a)
    a = a == nil and "test" or a
    print(a) -- Before: unknown/string|nil, After: string ✓
end

Assignments inside if blocks

---@param a string?
local function test(a)
    if true then
        a = a or "test"
        print(a) -- Before: unknown, After: string ✓
    end
end

Variable in for loop

local test = 1
for i = 1, 100 do
    test = test + 1
    if test > 2 then
        test = test + 2 -- Before: unknown, After: integer ✓
    end
end

Test plan

  • a = a == nil and "test" or aa inferred as string, not string|nil or unknown
  • Same pattern inside if true then ... end — no regression
  • a = a or "test" inside if then block — a inferred as string
  • Variable reassigned inside for loop (test = test + 1) — no unknown degradation
  • if a and b then / elseif chains — no regression in type narrowing

- Fix type narrowing for `x == nil and "default" or x` pattern: the `and`
  handler in the tracer now propagates outNode when the right side is a
  truthy literal, so the `or` branch correctly infers the variable as non-nil
- Fix circular dependency in calcNode for assignments inside if/for blocks:
  a _compilingAssigns guard prevents incomplete types and stale marks from
  propagating when the if-handler's getNode(lastAssign) triggers recompilation
  of an already-compiling setlocal
- Filter no-unknown false positives when a node only contains a variable
  object whose base declaration has a known type

Closes LuaLS#2236, LuaLS#2374, LuaLS#2494
Copy link
Copy Markdown

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

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 addresses false positive "unknown" diagnostics and type degradation caused by circular dependencies. It introduces a guard in the assignment compilation process and allows falling back to base declarations when a type is not immediately known. Furthermore, it improves type inference for logical "and" expressions by propagating narrowed nodes when the right-hand side is a truthy literal. A suggestion was made to refine the truthiness check in tracer.lua by utilizing vm.compileNode and alwaysTruthy() for a more robust implementation.

Comment on lines +644 to +656
-- 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
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 current check for a truthy right-hand side of an and expression is limited to literals. This could be made more robust and general by using vm.compileNode and the existing alwaysTruthy() method on the resulting node. This would handle not just literals but any expression that can be determined to be always truthy, such as constants or function calls returning known truthy values.

            -- When the right side of `and` is a truthy expression, 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 node2 = vm.compileNode(action[2])
            if node2:alwaysTruthy() then
                outNode = outNode1
            end

@RomanSpector
Copy link
Copy Markdown
Author

The current check for a truthy right-hand side of an and expression is limited to literals. This could be made more robust and general by using vm.compileNode and the existing alwaysTruthy() method on the resulting node.

We intentionally avoid calling vm.compileNode inside the tracer walk — it can trigger cascading compilation and circular dependencies (we encountered this during development). The AST literal check is safe and covers the most common use case (x == nil and "default" or x where the default is a literal).

@RomanSpector RomanSpector force-pushed the fix/type-inference-and-or-idiom branch from 2276dc2 to eb1135e Compare March 27, 2026 21:27
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.

Unknown type when dividing two numbers (2)

1 participant