Skip to content

fix(lua): port eval/computed-key dynamic-call detection to WASM extractor#2043

Open
carlos-alm wants to merge 1 commit into
fix/issue-1907-scripts-token-benchmark-ts-perf-dynamicfrom
fix/issue-1909-lua-wasm-extractor-missing-eval-computed-key
Open

fix(lua): port eval/computed-key dynamic-call detection to WASM extractor#2043
carlos-alm wants to merge 1 commit into
fix/issue-1907-scripts-token-benchmark-ts-perf-dynamicfrom
fix/issue-1909-lua-wasm-extractor-missing-eval-computed-key

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

The native Rust Lua extractor (crates/codegraph-core/src/extractors/lua.rs, handle_lua_function_call) already detects two dynamic-call patterns that the WASM/TS Lua extractor (src/extractors/lua.ts, handleLuaFunctionCall) never implemented, despite a prior commit (f9e4e20d, #1656) claiming to mirror it:

  1. load(...) / loadstring(...) / dofile(...) — dynamic code execution, classified dynamicKind: 'eval'.
  2. t[k]() (bracket_index_expression call target) — classified dynamicKind: 'computed-key', or resolved directly (as a plain call) when k is a string literal.

This ports both patterns into src/extractors/lua.ts's handleLuaFunctionCall, matching the Call shape (dynamic/dynamicKind/keyExpr/receiver) established by the other TS extractors (e.g. src/extractors/javascript.ts).

  • Bracket-index key lookup uses childForFieldName('field') (confirmed against both the npm @tree-sitter-grammars/tree-sitter-lua grammar used by WASM and the tree-sitter-lua crate used by native — both declare field('field', $.expression) for bracket_index_expression), which is simpler than native's manual child-scan for the same node and produces identical results.
  • No changes needed in build-edges.ts / incremental.ts / build_edges.rs: eval and computed-key are already generic entries in FLAG_ONLY_DYNAMIC_KINDS, handled uniformly across all languages.
  • Added Rust unit test coverage for this logic in lua.rs, which had none since it was introduced in feat(dynamic-calls): Phase 6 Rust parity — add GetMethod/Invoke/apply detection to Rust extractors (C#, Swift, Elixir, Lua) #1656.

Verification

  • npx vitest run tests/parsers/lua.test.ts — 21/21 passed (7 new tests for eval/computed-key)
  • cargo test --package codegraph-core --lib lua:: — 16/16 passed (6 new tests mirroring the TS coverage)
  • npm test — full suite: 253 files / 4013 tests passed, 0 failed
  • npm run lint — clean (no new issues)
  • Native addon rebuilt locally (napi build --platform --release), codesigned, and verified directly: built a small Lua fixture exercising all patterns (load, loadstring, dofile, t["handler"](), t['handler'](), t[k](), handlers[eventName .. "Handler"]()) and confirmed --engine wasm and --engine native now produce byte-identical codegraph roles --dynamic --json output (1 eval + 1 computed-key sink edge each) and identical fn-impact resolution for the literal-key case.

Filed #2042 (Lua's literal-key bracket-index call resolves silently instead of tagging computed-literal like JS does for the equivalent pattern) as a separate out-of-scope design question — both engines already agree with each other on this behavior, so it isn't a parity bug, just a possible taxonomy gap worth deciding on its own.

Closes #1909

Stacked on #1907 (base branch fix/issue-1907-scripts-token-benchmark-ts-perf-dynamic) — only the Lua extractor/test diff is this issue's change.

…ctor

The native Rust Lua extractor already detected load()/loadstring()/dofile()
as eval and t[k]() bracket-index calls as computed-key (or resolved directly
for a string-literal key), but src/extractors/lua.ts never had the
equivalent logic despite a prior commit claiming parity — so `--engine wasm`
and `--engine native` diverged on any Lua file using these patterns.

Ports handleLuaFunctionCall's eval/bracket-index handling from lua.rs into
lua.ts, matching the Call shape (dynamic/dynamicKind/keyExpr/receiver) used
by the other TS extractors. No changes to build-edges.ts/incremental.ts are
needed since eval/computed-key are already generic FLAG_ONLY_DYNAMIC_KINDS.

Also adds Rust unit test coverage for this logic, which had none.

Impact: 1 functions changed, 2 affected
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR ports two dynamic-call detection patterns from the native Rust Lua extractor (lua.rs) into the WASM/TS Lua extractor (lua.ts), closing a parity gap that existed since #1656: load/loadstring/dofile calls are now classified as dynamicKind: 'eval', and t[k]() bracket-index calls are classified as dynamicKind: 'computed-key' (or resolved directly when the key is a string literal).

  • TS extractor (src/extractors/lua.ts): adds the eval guard before the existing require check, and a new bracket_index_expression branch inside the call-name dispatch; uses childForFieldName('field') for the key — confirmed against the @tree-sitter-grammars/tree-sitter-lua grammar which declares field('field', $.expression) for that node.
  • Rust tests (crates/codegraph-core/src/extractors/lua.rs): adds six unit tests for the existing (untested) eval/computed-key logic in the native extractor.
  • TS tests (tests/parsers/lua.test.ts): adds seven Vitest cases covering all new paths, including the string-literal key resolution and expression-key computed-key classification.

Confidence Score: 4/5

Safe to merge; the change is a faithful port of already-shipping Rust logic with no downstream schema changes required.

The TS implementation correctly mirrors the Rust extractor's dispatch logic and the childForFieldName('field') field name is confirmed by the grammar. The one gap worth noting — loadfile absent from the eval set — is identical in both extractors and pre-dates this PR, so it is not a regression introduced here. Test coverage for all new paths is thorough on both sides of the native/WASM split.

src/extractors/lua.ts — the loadfile omission from the eval guard is worth a follow-up in both this file and its Rust counterpart.

Important Files Changed

Filename Overview
src/extractors/lua.ts Adds eval detection for load/loadstring/dofile and bracket-index (tk) call handling; loadfile is absent from eval set (mirrors Rust, minor gap).
crates/codegraph-core/src/extractors/lua.rs Adds six Rust unit tests covering eval/computed-key detection that was missing since #1656; no logic changes to the extractor itself.
tests/parsers/lua.test.ts Adds seven TS vitest cases covering all new detection paths including string-literal key resolution and expression-key computed-key classification.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    FC[function_call node] --> NN{nameNode type?}

    NN -->|identifier: load / loadstring / dofile| EVAL[Push eval dynamic call\nname: '<dynamic:eval>'\ndynamicKind: 'eval']
    NN -->|identifier: require| REQ[Handle as import]
    NN -->|method_index_expression| MIE[call.name = method\ncall.receiver = table]
    NN -->|dot_index_expression| DIE[call.name = field\ncall.receiver = table]
    NN -->|bracket_index_expression| BIE{key type?}
    NN -->|other identifier| PLAIN[call.name = nameNode.text]

    BIE -->|string / string_literal| STRKEY[call.name = stripped key\ncall.receiver = table\nresolves directly]
    BIE -->|variable / expression| DYNKEY[Push computed-key dynamic call\nname: '<dynamic:computed-key>'\ndynamicKind: 'computed-key'\nkeyExpr = key.text]
    BIE -->|no key found| DROP[Silently dropped\ncall.name stays empty]

    STRKEY --> GUARD{call.name truthy?}
    MIE --> GUARD
    DIE --> GUARD
    PLAIN --> GUARD
    GUARD -->|yes| PUSH[ctx.calls.push call]
    GUARD -->|no| SKIP[Skip]

    style EVAL fill:#f9a,stroke:#c00
    style DYNKEY fill:#f9a,stroke:#c00
    style STRKEY fill:#afa,stroke:#090
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    FC[function_call node] --> NN{nameNode type?}

    NN -->|identifier: load / loadstring / dofile| EVAL[Push eval dynamic call\nname: '<dynamic:eval>'\ndynamicKind: 'eval']
    NN -->|identifier: require| REQ[Handle as import]
    NN -->|method_index_expression| MIE[call.name = method\ncall.receiver = table]
    NN -->|dot_index_expression| DIE[call.name = field\ncall.receiver = table]
    NN -->|bracket_index_expression| BIE{key type?}
    NN -->|other identifier| PLAIN[call.name = nameNode.text]

    BIE -->|string / string_literal| STRKEY[call.name = stripped key\ncall.receiver = table\nresolves directly]
    BIE -->|variable / expression| DYNKEY[Push computed-key dynamic call\nname: '<dynamic:computed-key>'\ndynamicKind: 'computed-key'\nkeyExpr = key.text]
    BIE -->|no key found| DROP[Silently dropped\ncall.name stays empty]

    STRKEY --> GUARD{call.name truthy?}
    MIE --> GUARD
    DIE --> GUARD
    PLAIN --> GUARD
    GUARD -->|yes| PUSH[ctx.calls.push call]
    GUARD -->|no| SKIP[Skip]

    style EVAL fill:#f9a,stroke:#c00
    style DYNKEY fill:#f9a,stroke:#c00
    style STRKEY fill:#afa,stroke:#090
Loading

Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(lua): port eval/computed-key dynamic..." | Re-trigger Greptile

Comment thread src/extractors/lua.ts
Comment on lines +266 to +274
if (ident === 'load' || ident === 'loadstring' || ident === 'dofile') {
ctx.calls.push({
name: '<dynamic:eval>',
line: node.startPosition.row + 1,
dynamic: true,
dynamicKind: 'eval',
});
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 loadfile omitted from eval detection

loadfile is listed in LUA_BUILTIN_GLOBALS on line 30 alongside load, loadstring, and dofile, and it performs the same dynamic-code-loading operation (it compiles a Lua file and returns a chunk function). The eval guard currently covers load, loadstring, and dofile but not loadfile, so loadfile("script.lua")() — and any call graph that passes through a loadfile result — will not be flagged as a dynamic eval site. This mirrors the native Rust extractor's equivalent omission, so no parity divergence is introduced here, but the shared gap is worth addressing in both extractors.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

1 functions changed2 callers affected across 1 files

  • handleLuaFunctionCall in src/extractors/lua.ts:257 (2 transitive callers)

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