feat: analyze JavaScript — discovery, dynamic method idioms, and a loud jelly failure - #86
Merged
Conversation
`SOURCE_EXTS` held only the four TypeScript extensions, so a JavaScript-only project produced an empty symbol table and exited 0 with no warning. Measured on OWASP NodeGoat: 0 modules, 0 edges, an 84-byte analysis.json. Nothing downstream needed changing — `defaultCompilerOptions()` already sets `allowJs`, and Jelly already accepts .js/.mjs/.cjs; both were simply never handed a file. Discovery was the whole gate. - SOURCE_EXTS gains .js/.jsx/.mjs/.cjs - isTestFile's regex covers the same four, so .test.js is skipped like .test.ts - buildSymbolTable warns when discovery finds nothing, instead of succeeding silently On unmodified NodeGoat this now yields 27 modules, and a call graph of 161 union edges with dependencies installed (136 with --no-build). The module key set exactly equals the set of .js files outside node_modules, vendor and test trees; discovery is unaffected by dependency state. sample-app output is byte-identical before and after. Does not model CommonJS `require`/`module.exports` at the module level — imports and exports stay empty on CJS input. Relative `require()` call targets do resolve through the tsc resolver. Closes #84
The union provider degrades to tsc-only when the jelly leg throws, and reported
that at `info` level — which is not printed at default verbosity, so the failure
was entirely silent.
That is tolerable on TypeScript, where the resolver carries the graph. On
JavaScript it is a cliff: measured on OWASP NodeGoat with dependencies installed,
jelly supplies 156 of the 161 union edges, so a silent degradation drops the call
graph by ~81% with no signal. The failure is now reported at error level when
most analyzed modules are JavaScript, and stays at info otherwise.
Also truncate the reason. execFileSync puts the whole command line in
Error.message, which on NodeGoat meant 27 file paths inlined into an error the
user is meant to act on.
The default stays `union`, deliberately: union is a strict superset of jelly on
JS. Measured both with and without dependencies materialized, union - jelly is
the same 5 edges and jelly - union is empty. Three of those five target
`const x = () => {}` callables declared inside a constructor function
(app/routes/session.js:14,138 and app/data/allocations-dao.js:60) that jelly
misses; two are library phantoms, including `needle.get` — NodeGoat's SSRF sink —
which stays tsc-only even once jelly can see node_modules. Jelly is the better
source of external symbols overall (21 to tsc's 2 with deps present), which is
why the two legs are kept complementary rather than one being preferred.
Success path is unchanged: sample-app analysis.json byte-identical to v0.5.0.
Two ways of declaring a method were never materialized as callables, so calls to
them could not resolve: edges are gated to `allSignatures`, which is built from
the symbol table.
• `this.<name> = fn` inside a constructor function — landed in local_variables
• object-literal members (`{ foo(){} }`, `{ foo: function(){} }`) — dropped
Language-neutral, not a JavaScript gap: NodeGoat renamed .js -> .ts yielded the
same 24 callables before this change.
Four sites: `contributorName` names the two new forms (and lets a variable bound
to an object literal contribute its name, so members are homed under it);
`namedBoundary` treats them as callable boundaries; `walkBody`'s dispatch is
replaced by a `callableOf` helper; and `buildStatemented` walks module-level
object literals, which no function body covers.
`resolveCalleeSignature` needed a matching branch — the checker hands these back
as BinaryExpression / PropertyAssignment declarations, which `isCallableDecl`
does not cover, so edges were still dropped after the symbol table was correct.
`buildCallable` falls back to `contributorName` for the display name, which was
otherwise "(anonymous)".
Measured on OWASP NodeGoat (deps installed, -a 2):
callables 24 -> 59 (parser-derived ground truth: 59 nameable)
tsc resolved 28 -> 51
tsc edges 30 -> 53
union edges 161 -> 184
named graph nodes 32 -> 62 (positional share 75% -> 61%)
call-site resolution 11% -> 20%
The DAO method layer now appears in the call graph, which it did not before:
app/routes/allocations.AllocationsHandler.displayAllocations
-> app/data/allocations-dao.AllocationsDAO.getByUserIdAndThreshold
sample-app analysis.json stays byte-identical to v0.5.0 — no signature churn for
code that already resolved.
Closes #85
briefly(): drop the never-varied limit param, one line instead of four. isJavaScriptMajority(): regex instead of a JS_EXTS array duplicating SOURCE_EXTS. Comments cut where they ran longer than the code they explained. 129 -> 113 added lines. No behavior change: 42 tests green, typecheck clean.
- version 0.5.0 -> 0.6.0 in package.json and src/utils/version.ts, in lockstep. ANALYZER_VERSION is the only thing that invalidates a cache (utils/cache.ts:24) — the per-file source hash cannot see that extraction logic moved, and this release extracts more callables from unchanged sources. Verified: a 0.5.0-stamped cache is rejected (27 built, 0 cached) where a matching one is reused (0 built, 27 cached). - CHANGELOG.md, following the codeanalyzer-python house format. The repo had none; the release-announcement task in CLAUDE.md already assumed one existed. - CI on release/0.x, ported from main's ci.yml with the branch filter changed. No automated check had ever run on this line — the tag pipeline, which publishes to PyPI, GitHub Releases and the Homebrew tap, would have been the first.
…uage Node labels carried a TS twin; relationship types carried nothing. A database holding output from more than one analyzer therefore mingled edges — codeanalyzer-python already namespaces all 18 of its relationship types (PY_CALLS, PY_DECLARES, …) while this analyzer emitted bare CALLS/DECLARES. Now per source language, not per analyzer: a .js module is :Module:JSModule and a .ts module is :Module:TSModule, and every relationship type is prefixed. Rules: - a node with `_module` takes that module's language; - nodes with no language of their own — application root, packages, external library symbols — take the analyzer's own TS namespace, since a sibling analyzer emits its own; - an edge takes its source module's language, falling back to its target's, so application->module on a JavaScript project is JS_HAS_MODULE. Implemented at the two hooks RowBuilder already exposed rather than at the 21 edge call sites: `expand` now sees the node's props, and a new `retype` runs in finish(), where both endpoints' props are known. REL_TYPES stays the single source of truth; REL_TYPES_NS derives both namespaces so the catalog and the projection cannot drift. Also updates the hand-written traversals in wipe() and DESCENDANTS, which matched bare types and would have silently deleted nothing. BREAKING: Neo4j schema version 1.1.0 -> 2.0.0. Stored queries must move from `[:CALLS]` to `[:TS_CALLS|JS_CALLS]`; the version change forces a full re-upsert on the next incremental push.
…hip types The container suite is skipped without Docker, so a local run stayed green while CI failed: `MATCH (:Callable)-[:CALLS]->` matches nothing now that edges are namespaced. This is the same migration the CHANGELOG asks consumers to make.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #84. Closes #85. Closes #88.
Two related gaps that together meant JavaScript projects were unanalyzable, plus one fix to stop a silent degradation.
Targets the 0.x line (
release/0.x, offv0.5.0), notmain—python-sdkpinscodeanalyzer-typescript==0.4.3and its model layer is the v1 flatTSApplication, so nothing released on the v2 line reaches a consumer. Planning context: roadmap.1 — Discovery never offered a JavaScript file (#84)
src/syntactic_analysis/discovery.ts:5restricted discovery to.ts/.tsx/.mts/.cts, so a JS-only project emitted an empty symbol table and exited 0 with no warning. On OWASP NodeGoat: 0 modules, an 84-byteanalysis.json.Nothing downstream needed changing —
defaultCompilerOptions()already setallowJsand Jelly already accepts.js/.mjs/.cjs; both were simply never handed a file.SOURCE_EXTS+=.js/.jsx/.mjs/.cjsisTestFileregex covers the same four, so.test.jsis skipped like.test.tsbuildSymbolTablewarns when discovery finds nothing, instead of succeeding silently2 — Two method idioms were never callables (#85)
Edges are gated to
allSignatures, built from the symbol table, so a method that isn't a callable can never be a call target:this.<name> = fninside a constructor function — landed inlocal_variables{ foo(){} },{ foo: function(){} }) — dropped entirelyLanguage-neutral, not a JavaScript gap: NodeGoat renamed
.js→.tsyielded the same 24 callables before this change.Five sites:
contributorNamenames both forms (and lets a variable bound to an object literal contribute its name);namedBoundarytreats them as callable boundaries;walkBody's dispatch becomes acallableOfhelper;buildStatementedwalks module-level object literals, which no function body covers; andresolveCalleeSignaturegains a matching branch — the checker returns these asBinaryExpression/PropertyAssignmentdeclarations, whichisCallableDecldoes not cover, so edges were still dropped after the symbol table was already correct.3 — A jelly failure was silent on JS
The union provider degrades to tsc-only when the jelly leg throws, and reported it at
info, which is not printed at default verbosity. On JS that is a ~81% edge loss with no signal. Now reported at error level when most analyzed modules are JavaScript; the reason is truncated, sinceexecFileSyncinlines the whole command line (27 file paths on NodeGoat).The default stays
uniondeliberately: measured with and withoutnode_modules,union − jellyis the same 5 edges andjelly − unionis empty. Two are library phantoms includingneedle.get, NodeGoat's SSRF sink, which stays tsc-only even once jelly can seenode_modules.Results on unmodified NodeGoat (deps installed,
-a 2)59 callables matches parser-derived ground truth exactly — 115 function-like nodes in source, of which 59 are nameable.
The module key set exactly equals the set of
.jsfiles outsidenode_modules,vendorand test trees (27/27,diffempty).Security-scoped reachability
The DAO method layer now appears in the graph, which it did not before:
So "does user input from
POST /allocationsreach a Mongo query?" is answerable from the output. A machine-checkable edge intomongodbstill does not exist — that needs interprocedural points-to and is tracked in #87, deliberately not faked here.4 — Neo4j labels and relationship types are namespaced per source language (#88)
BREAKING. Node labels carried a
TStwin; relationship types carried nothing at all. A database holding output from more than one analyzer therefore mingled edges —codeanalyzer-pythonalready namespaces all 18 of its relationship types (PY_CALLS,PY_DECLARES, …) while this analyzer emitted bareCALLS/DECLARES.Now namespaced by source language, not by analyzer:
Rules: a node carrying
_moduletakes that module's language; nodes with none of their own (application root, packages, external library symbols) take the analyzer'sTSnamespace, since a sibling analyzer emits its own; an edge takes its source module's language, falling back to its target's — so application→module on a JavaScript project isJS_HAS_MODULE.Implemented at the two hooks
RowBuilderalready exposed rather than at the 21 edge call sites:expandnow receives the node's props, and a newretyperuns infinish()where both endpoints' props are known.REL_TYPESstays the single source of truth andREL_TYPES_NSderives both namespaces, so catalog and projection cannot drift.Also fixes a latent bug this change would otherwise have introduced:
wipe()and bolt'sDESCENDANTSare hand-written traversals over bare relationship types. Once edges were namespaced they matched nothing — the wipe would have silently deleted zero rows and left stale data. Both now derive fromnsAlt().Migration
Neo4j schema version 1.1.0 → 2.0.0. Every stored query must move:
The version change forces a full re-upsert on the next incremental push.
Verification
src/sample-appanalysis.jsonbyte-identical tov0.5.0— no signature churn for code that already resolvedJELLY_BIN=/nonexistent) at default verbosity: loud on NodeGoat, silent onsample-appKnown gaps, not addressed
require/module.exportsmodule-level modeling —imports/exportsstay empty on CJS input. Relativerequire()call targets do resolve.mongodbedge — feat(call-graph): attribute method calls on untyped receivers so DB sinks are reachable #87, needs points-to.