Skip to content

feat(graph): index Hacklang (.hack/.hhi) via a native tree-sitter-hack grammar - #348

Open
mmissey wants to merge 3 commits into
trailhq:mainfrom
mmissey:feat/hack-native-grammar
Open

feat(graph): index Hacklang (.hack/.hhi) via a native tree-sitter-hack grammar#348
mmissey wants to merge 3 commits into
trailhq:mainfrom
mmissey:feat/hack-native-grammar

Conversation

@mmissey

@mmissey mmissey commented Sep 10, 2026

Copy link
Copy Markdown

What

Adds first-class Hacklang (.hack / .hhi) support to the code graph, using a native tree-sitter-hack grammar with its own node-kind mapping (higher fidelity than reusing the PHP grammar).

The grammar is distributed as a published npm package, @mmissey/tree-sitter-hacklang, aliased to the bare tree-sitter-hack import — mirroring the existing tree-sitter-r@davisvaughan/tree-sitter-r alias pattern. No vendored parser blob; nothing new to build locally.

"tree-sitter-hack": "npm:@mmissey/tree-sitter-hacklang@^0.1.1"

Why a fork of the grammar

slackhq/tree-sitter-hack (MIT, © 2020 Antonio de Jesus Ochoa Solano; archived) ships a working parser.c/scanner.c but is built with the legacy nan bindings, which tree-sitter@0.21+ rejects at runtime as an "Invalid language object" (0.21 validates a napi_type_tag on the language External that nan doesn't set). The fork re-binds it with node-addon-api so it satisfies 0.21, and ships prebuilt N-API binaries for linux-x64, darwin-arm64, darwin-x64, and win32-x64 (via prebuildify + node-gyp-build), so consumers need no C toolchain. Falls back to a source build only where no prebuild matches. LICENSE is preserved verbatim.

Changes

  • src/graph/extract.ts — add hack to the Language union, extensions (.hack, .hhi), kind/call-type maps, and grammar registration; Hack-specific heritage (extends_clause/implements_clause as direct children), callee resolution (selection_expression/scoped_identifier), imports (use_statement/use_clause), and trait_use_clause → implements.
  • src/graph/bindings.tshandleHack for typed parameters and $x = new Foo() receiver binding, plus Hack defName handling.
  • src/graph/resolve.ts — extend the classTraits map to .hack/.hhi so $this->traitMethod() resolves.
  • package.json — depend on @mmissey/tree-sitter-hacklang via the npm alias (no vendored grammar).
  • test/graph-hack.test.ts — 6 extraction tests (classes/methods/interface/trait/enum/function, call+extends+implements edges, trait use + typed-parameter receiver binding, new constructor binding, enum class, .hhi indexing).

Verification

  • npm install resolves @mmissey/tree-sitter-hacklang@0.1.1 from npm with the prebuilt binary; npm run build (tsc) passes.
  • test/graph-hack.test.ts — 6/6 pass. PHP suite unaffected.
  • Smoke on ~275 real-world .hack files: 0 throws, receiver types recovered on ~19% of call edges.

Open question for maintainers

Happy to adjust the distribution approach — e.g. transfer/co-own the grammar package under a Trail-owned scope, or vendor it — whichever you prefer. Opening as a draft to get direction on that before finalizing.


Supersedes #331 (closed).

Marc Missey and others added 3 commits September 9, 2026 16:27
…ute C)

Add depth-tier extraction for Hack (`.hack`/`.hhi`) using its own vendored
tree-sitter grammar and a dedicated node-kind mapping — higher fidelity than
reusing the PHP grammar, and no `<?hh` shim (tagless Hack parses natively).

- vendor/tree-sitter-hack: upstream Hack grammar sources, re-bound with
  node-addon-api so the language object passes tree-sitter@0.21's type-tag
  check. build/ and node_modules/ are git-ignored (rebuilt on npm install).
- extract.ts: `hack` language, `.hack`/`.hhi` extensions, HACK_KINDS (incl.
  `enum_class_declaration`), call/heritage/import handling for Hack's node
  shapes (single call_expression, direct extends/implements clauses,
  trait_use_clause, use_clause).
- bindings.ts: typed-parameter and `new`-assignment receiver-type bindings.
- resolve.ts: trait-method resolution now spans .hack/.hhi, not just .php.
- test/graph-hack.test.ts: nodes, edges, trait use, param binding, enum class.

Smoke-tested on 275 real webapp .hack files: 0 extraction throws, 1.1%
file-only (all genuinely symbol-free), receiver types on ~19% of call edges.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…of vendoring

Replace the `file:vendor/tree-sitter-hack` dependency (an ~11MB committed
parser.c plus a node-addon-api rebind) with an npm alias:

    "tree-sitter-hack": "npm:@mmissey/tree-sitter-hacklang@^0.1.0"

This mirrors the existing `tree-sitter-r` -> `@davisvaughan/tree-sitter-r`
alias, so the `import Hack from "tree-sitter-hack"` in extract.ts is unchanged.
@mmissey/tree-sitter-hacklang is the same grammar re-bound with node-addon-api
for tree-sitter@0.21+, now published with prebuilt binaries (no C toolchain
needed on supported platforms). The whole vendor/ tree is removed.

Hack extraction tests: 6/6 pass against the published package.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
0.1.1 is the first release shipping prebuilt binaries for all four
platforms (linux-x64, darwin-arm64, darwin-x64, win32-x64), so installs
no longer need a local C toolchain on any of them. Pins the lockfile to
0.1.1 and updates the @lavamoat/allow-scripts key to match.

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

🌱 graft blast radius

1 area changed → 3 areas can be affected. 5 dependent symbols, depth 2.
Tests: Graph Resolution has tests the diff did not touch.
Tag: 贾旭卿 — 4 of 4 areas · @anirudhkumar-nanonets — Pull Request Review · Matt Strayer — Graph Resolution

flowchart TB
  A0(("Graph Building<br/>2 symbols"))
  A1(("Pull Request Review<br/>2 symbols"))
  A2(("Graph Representation<br/>1 symbol"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1,A2 reached;
Loading
Can be affected Symbols Nearest hop Reached from
Graph Building 2 src/graph/build.ts:L151-L410 buildGraph — calls, depth 1 Graph Resolution
Pull Request Review 2 src/app/brain-build.ts:L101-L228 buildRepoIntoBrain — calls, depth 2 Graph Resolution
Graph Representation 1 src/engine.ts:L91-L101 graph — calls, depth 2 Graph Resolution
Who knows this code — 5 people across 4 areas
Area Who knows it
Graph Resolution · changed Matt Strayer — 1 commit, last 14d ago · 贾旭卿 — 1 commit, last 14d ago
Graph Building · affected @tpoignonec — 1 commit, last 14d ago · 贾旭卿 — 1 commit, last 14d ago
Pull Request Review · affected @anirudhkumar-nanonets — 3 commits, last today · 贾旭卿 — 1 commit, last 14d ago
Graph Representation · affected @Frankie-Xu — 1 commit, last 10d ago · 贾旭卿 — 1 commit, last 14d ago

Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no @ has no GitHub handle in its commit email — tag them by hand, or add a .mailmap entry. A suggestion from history, not a CODEOWNERS rule.

All 5 dependent symbols, grouped by area

Graph Building — 2 symbols in 2 files

  • src/graph/build.ts:L151-L410 — buildGraph (calls, depth 1)
  • src/graph/refresh.ts:L150-L227 — ensureFreshGraph (calls, depth 2)

Pull Request Review — 2 symbols in 2 files

  • src/app/brain-build.ts:L101-L228 — buildRepoIntoBrain (calls, depth 2)
  • src/app/review.ts:L45-L99 — reviewPullRequest (calls, depth 2)

Graph Representation — 1 symbol in 1 file

  • src/engine.ts:L91-L101 — graph (calls, depth 2)
Test signal per changed area — 1 ⚠

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • Graph Resolution — 1 of 16 reached · 3 test files reach it, none changed here
    • not reached: defName, resolveRecvType, visit, hackConstructedName, hackTrailingIdent, hackTypeName, handleHack, walk, …7 more
35 test suites also reference this code

38 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.

  • test/ask-index.test.ts
  • test/ask.test.ts
  • test/container-extract.test.ts
  • test/context-only-dir.test.ts
  • test/context.test.ts
  • test/covers.test.ts
  • test/generic-extract.test.ts
  • test/graph-cross-language.test.ts
  • test/graph-go.test.ts
  • test/graph-incremental.test.ts
  • test/graph-invariants.test.ts
  • test/graph-java.test.ts
  • test/graph-languages.test.ts
  • test/graph-php.test.ts
  • test/graph-posix-paths.test.ts
  • test/graph-python.test.ts
  • test/graph-r-classes.test.ts
  • test/graph-r-phase3.test.ts
  • test/graph-r-phase4.test.ts
  • test/graph-r-phase5.test.ts
  • …15 more

⚠️ 2 changed files not in the graph (package-lock.json, package.json) — no parser claims the extension, or the index predates the file.

graft blast · refs/graft/base...HEAD · depth 2 · 6 changed files

Open the interactive graph → — click an area to see the code that changed, and the line that reaches it.

github-actions Bot added a commit that referenced this pull request Sep 10, 2026
@mmissey
mmissey marked this pull request as ready for review September 10, 2026 15:37
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