feat(graph): Lua symbol/call extraction + fix file discovery for whitelist .gitignore#67
feat(graph): Lua symbol/call extraction + fix file discovery for whitelist .gitignore#67SIRTHEO wants to merge 1 commit into
Conversation
… .gitignore Two related graph-engine fixes that together make the code graph work on Lua projects (and unblock any repo using a whitelist-style .gitignore): - getGraphableFiles skipped every directory whose bare name matched .gitignore but was re-included only in trailing-slash form (e.g. `/*` then `!/src/`). The walk never descended, producing an empty graph for ALL languages on such repos. Check the directory form (trailing slash), per gitignore dir semantics. - Lua had no dedicated ast-grep extractor and fell through to the regex fallback, which records `Mod` for `function Mod.parse()`. Add extractFromLua (function_declaration dotted/method/local names, `T.f = function()` assigns, and call sites) and register the @ast-grep/lang-lua grammar, so namespace-table style resolves to precise qualified symbols.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds Lua language support to the code graph and symbol extraction system. The changes add a runtime dependency for the Lua AST grammar, register Lua as a dynamically-loaded language, implement Lua-specific symbol extraction from namespace-table and local function patterns, and refine directory traversal to use trailing slashes in ignore-filter matching. ChangesLua Language Support and Symbol Extraction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks for this, it is a clean and well-scoped contribution, and CI is green across Node 18/20/22. I verified both halves against the engine: Discovery fix: correct and, importantly, complete. I tested our ignore@7.0.5 with the exact /* then !/src/ pattern: ignores("src") is true (the old walk bailed here, hence the empty graph), ignores("src/") is false (your fix descends), and ignores("src/foo.lua") is false, so the files under the re-included directory are actually picked up, not just the directory entered. I also confirmed the search/embedding discovery path (getIndexableFiles, which uses glob(nodir:true) plus a per-file check) is unaffected, so scoping this to the graph walk is right. Lua extractor: faithful to the existing extractors. Good calls on taking the direct child before parameters as the name, requiring the assignment RHS to be directly a function_definition, and filtering keywords on call sites. A few things before merge: Add real Lua tests. tests/unit/graph-symbols.test.ts has a per-language block with concrete assertions for every other extractor. Please add a describe("Lua") that locks in the new behavior: qualified Table.method / T:m() names, local function, the T.f = function() ... end form, and call attribution to the enclosing function. Fix the now-misleading existing test. it("handles Lua via regex fallback", ...) (currently in the "Regex fallback" block) no longer exercises the fallback, since Lua now routes to extractFromLua. It still passes because it only asserts exists, but the name and placement are now wrong. Please move it into the new Lua block and give it real assertions. Minor style: every other dynamically-registered grammar calls parse("x" as unknown as Lang, source). Please match that for parse("lua", ...) for consistency. Not blocking since it compiles. The discovery fix is valuable also on its own. Thanks again. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
2 participants
Summary
Two small graph-engine fixes that together make the code graph work for Lua
projects (and unblock any repo using a whitelist-style
.gitignore).1. Fix: getGraphableFiles skips whitelisted directories
getGraphableFilescallsshouldIgnore(ig, relPath)with the bare relativepath for every entry, directories included. For a repo that ignores everything
and re-includes a folder by its trailing-slash form — e.g.
/*then!/src/—ignores("src")is true whileignores("src/")is false. The walk skips thesrcdirectory and never descends, so the graph is empty for EVERY language onsuch repos. Fix: pass the directory form (trailing slash) for directory entries,
matching gitignore directory semantics. Safe for normal ignores.
2. Feature: dedicated Lua symbol extractor
Lua had no node-kind-specific extractor and fell through to the regex fallback,
which records
Modforfunction Mod.parse(). AddsextractFromLua, walkingthe ast-grep Lua tree:
function T.f()/T:m()/local function f()(name = thedirect child before
parameters),T.f = function() ... endassignments (RHSmust be directly a function), and call sites. Registers
@ast-grep/lang-lua.Namespace-table style now resolves to precise qualified symbols (
Table.method).Testing
npm run build(tsc strict) passes.names + call edges. Same-file callees resolve; cross-file caller resolution is
still bounded by the import-based resolver (Lua has no imports) — out of scope.
Notes
No breaking changes; other languages unaffected. The two changes are independent
but bundled since the Lua graph is only observable once discovery works.
Summary by CodeRabbit
New Features
Bug Fixes