fix(extractor): gate member_expression callback args on callee allowlist (#974)

* fix(extractor): gate member_expression callback args on callee allowlist

PR #947's extractCallbackReferenceCalls emitted a dynamic call edge for every
identifier or member_expression argument of every call expression. That was
correct for real callbacks (router.use, promise.then, items.map) but produced
false positives for plain property reads passed as data, e.g.
`store.set(user.id, user)` — the `user.id` arg wrongly resolved to
`User.id@types.ts`, dropping TS resolution precision from 100% → 93.8% and
tripping the regression guard.

Fix: gate member_expression args on a CALLBACK_ACCEPTING_CALLEES allowlist
covering router/middleware, promises, array methods, event emitters, and
scheduling APIs. Identifier args remain unchanged. Adds positive and
negative tests; updates the 3.9.4 resolution benchmark to reflect
restored precision.

Fixes #971

Impact: 2 functions changed, 7 affected

* fix(extractor): require string-literal path for HTTP-verb callback gating (#974)

Addresses Greptile review feedback on PR #974:

- HTTP-verb callees (get/post/put/delete/patch/options/head/all) double as
  Map/cache/repository method names. Require a string-literal first argument
  (Express route path) for member-expr args to be emitted as dynamic calls,
  so `cache.get(user.id)` and `repo.put(record.key, value)` no longer leak
  `id`/`key` as false-positive dynamic calls while `router.get('/path', h)`
  still works.
- Document that optional-chaining callees (`obj?.on(handlers.fn)`) are
  handled transparently: tree-sitter-javascript/typescript represent them
  as `member_expression` with an `optional_chain` child, so the existing
  extraction returns the property name correctly. Add a regression test.
- Tests: three new cases in `tests/parsers/javascript.test.ts`:
  - negative: `cache.get(user.id)`, `repo.put(record.key, ...)`, `map.delete(entry.id)`
  - positive: `router.get('/path', auth.check)`, `app.post(\`/api\`, handlers.create)`
  - optional-chaining: `emitter?.on('tick', handlers.fn)` still emits

All JS parser, regression-guard, and TS/JS resolution benchmarks stay green
(TS precision 1.0, JS precision 1.0).

Impact: 2 functions changed, 7 affected

Author: carlos-alm

generated/benchmarks/BUILD-BENCHMARKS.md

@@ -1134,12 +1134,12 @@ pre-parse that previously added ~388ms on native builds.
         }
       },
       "typescript": {
-        "precision": 0.938,
+        "precision": 1,
         "recall": 0.75,
         "truePositives": 15,
-        "falsePositives": 1,
+        "falsePositives": 0,
         "falseNegatives": 5,
-        "totalResolved": 16,
+        "totalResolved": 15,
         "totalExpected": 20,
         "byMode": {
           "same-file": {

src/extractors/javascript.ts

@@ -1277,16 +1277,159 @@ function extractSubscriptCallInfo(fn: TreeSitterNode, callNode: TreeSitterNode):
   return null;
 }
 
+/**
+ * Callee names that idiomatically accept callback references. Used to gate
+ * member_expression args in {@link extractCallbackReferenceCalls}: arguments
+ * like `user.id` are only emitted as dynamic callback calls when the callee
+ * is a known callback-accepting API (router/middleware, promises, array
+ * methods, event emitters, scheduling APIs). This avoids false positives
+ * from plain property reads passed as data, e.g. `store.set(user.id, user)`.
+ *
+ * Identifier args (e.g. `router.use(handleToken)`) are always emitted — the
+ * collateral damage of dropping them is larger than the FP risk, since plain
+ * identifier data args rarely collide with real function names.
+ */
+const CALLBACK_ACCEPTING_CALLEES: ReadonlySet<string> = new Set([
+  // Express / router / middleware
+  'use',
+  'get',
+  'post',
+  'put',
+  'delete',
+  'patch',
+  'options',
+  'head',
+  'all',
+  // Promises
+  'then',
+  'catch',
+  'finally',
+  // Array iteration / reduction
+  'map',
+  'filter',
+  'forEach',
+  'find',
+  'findIndex',
+  'findLast',
+  'findLastIndex',
+  'some',
+  'every',
+  'reduce',
+  'reduceRight',
+  'flatMap',
+  'sort',
+  // Event emitters / DOM
+  'on',
+  'once',
+  'off',
+  'addListener',
+  'removeListener',
+  'addEventListener',
+  'removeEventListener',
+  'subscribe',
+  'unsubscribe',
+  // Scheduling / plain function callbacks
+  'setTimeout',
+  'setInterval',
+  'setImmediate',
+  'queueMicrotask',
+  'requestAnimationFrame',
+  'requestIdleCallback',
+  'nextTick',
+  // Commander / yargs / hooks
+  'action',
+  'command',
+]);
+
+/**
+ * HTTP-verb callees that double as Map/cache/repository method names (`get`,
+ * `post`, `put`, `delete`, `patch`, `options`, `head`, `all`). Express/router
+ * invocations always take a string-literal route path as the first argument
+ * (`app.get('/path', handler)`), whereas Map-like APIs pass values/keys
+ * (`cache.get(user.id)`). Requiring a string-literal first arg keeps real
+ * route handlers covered while dropping the Map/cache false-positive surface.
+ *
+ * `use` and `all` without a path are legitimate middleware registrations, so
+ * `use` is intentionally excluded here — it stays in the general allowlist.
+ */
+const HTTP_VERB_CALLEES: ReadonlySet<string> = new Set([
+  'get',
+  'post',
+  'put',
+  'delete',
+  'patch',
+  'options',
+  'head',
+  'all',
+]);
+
+/**
+ * Extract the callee's final name (function identifier or member expression
+ * property) for callback-eligibility filtering. Returns null if the callee
+ * shape is not analyzable (e.g. computed subscripts, IIFEs).
+ *
+ * Optional-chaining (`obj?.method(...)`) is handled transparently: in both
+ * tree-sitter-javascript and tree-sitter-typescript grammars `obj?.method` is
+ * still a `member_expression` (the `?.` appears as an `optional_chain` child),
+ * so the property extraction below returns `method` as expected.
+ */
+function extractCalleeName(callNode: TreeSitterNode): string | null {
+  const fn = callNode.childForFieldName('function');
+  if (!fn) return null;
+  if (fn.type === 'identifier') return fn.text;
+  if (fn.type === 'member_expression') {
+    const prop = fn.childForFieldName('property');
+    return prop ? prop.text : null;
+  }
+  return null;
+}
+
+/**
+ * True iff the first argument of an arguments node is a string literal.
+ * Used to distinguish Express/router route handlers (`app.get('/path', h)`)
+ * from Map/cache APIs that reuse the same verb names (`cache.get(user.id)`).
+ */
+function firstArgIsStringLiteral(argsNode: TreeSitterNode): boolean {
+  for (let i = 0; i < argsNode.childCount; i++) {
+    const child = argsNode.child(i);
+    if (!child) continue;
+    // Skip parens and commas; the first non-punctuation child is the first arg.
+    if (child.type === '(' || child.type === ',' || child.type === ')') continue;
+    return child.type === 'string' || child.type === 'template_string';
+  }
+  return false;
+}
+
 /**
  * Extract Call entries for named function references passed as arguments.
  * e.g. `router.use(handleToken, checkAuth)` yields calls to handleToken and checkAuth.
  * `app.use(auth.validate)` yields a call to validate with receiver auth.
  * Skips literals, objects, arrays, anonymous functions, and call expressions (already handled).
+ *
+ * To avoid false positives where plain property reads are passed as data
+ * (e.g. `store.set(user.id, user)` — `user.id` is a value, not a callback),
+ * member_expression args are only emitted when the callee is in
+ * {@link CALLBACK_ACCEPTING_CALLEES}. Identifier args are always emitted.
+ *
+ * HTTP-verb callees (`get`, `post`, `put`, `delete`, `patch`, `options`,
+ * `head`, `all`) double as Map/cache/repository method names, so their
+ * member-expr args are only emitted when the first argument is a string
+ * literal route path — matching Express/router shape and skipping
+ * `cache.get(user.id)`-style calls.
  */
 function extractCallbackReferenceCalls(callNode: TreeSitterNode): Call[] {
   const args = callNode.childForFieldName('arguments') || findChild(callNode, 'arguments');
   if (!args) return [];
 
+  const calleeName = extractCalleeName(callNode);
+  let memberExprArgsAllowed = calleeName !== null && CALLBACK_ACCEPTING_CALLEES.has(calleeName);
+  if (memberExprArgsAllowed && calleeName !== null && HTTP_VERB_CALLEES.has(calleeName)) {
+    // HTTP verbs require a string-literal route path to be treated as a
+    // callback-accepting API; otherwise `cache.get(user.id)` etc. would
+    // still emit `id` as a dynamic call.
+    memberExprArgsAllowed = firstArgIsStringLiteral(args);
+  }
+
   const result: Call[] = [];
   const callLine = callNode.startPosition.row + 1;
 
@@ -1296,7 +1439,7 @@ function extractCallbackReferenceCalls(callNode: TreeSitterNode): Call[] {
 
     if (child.type === 'identifier') {
       result.push({ name: child.text, line: callLine, dynamic: true });
-    } else if (child.type === 'member_expression') {
+    } else if (child.type === 'member_expression' && memberExprArgsAllowed) {
       const prop = child.childForFieldName('property');
       const obj = child.childForFieldName('object');
       if (prop) {

tests/parsers/javascript.test.ts

@@ -321,6 +321,66 @@ describe('JavaScript parser', () => {
       expect(dynamicCalls).toHaveLength(0);
     });
 
+    it('does not treat member_expression args as callbacks for non-allowlisted callees', () => {
+      // `store.set(user.id, user)` — `user.id` is a property read passed as a
+      // value (map key), NOT a callback. Only allowlisted callees (use, then,
+      // map, addEventListener, etc.) get member_expression args emitted as
+      // dynamic calls. See issue #971.
+      const symbols = parseJS(`store.set(user.id, user);`);
+      const dynamicMemberCalls = symbols.calls.filter((c) => c.dynamic && c.name === 'id');
+      expect(dynamicMemberCalls).toHaveLength(0);
+    });
+
+    it('still emits member_expression args for allowlisted callees (regression guard)', () => {
+      // Positive companion to the test above: `app.use(auth.validate)` and
+      // `promise.then(handlers.onSuccess)` must still produce dynamic calls,
+      // because `use` and `then` are callback-accepting APIs.
+      const useSymbols = parseJS(`app.use(auth.validate);`);
+      expect(useSymbols.calls).toContainEqual(
+        expect.objectContaining({ name: 'validate', receiver: 'auth', dynamic: true }),
+      );
+      const thenSymbols = parseJS(`promise.then(handlers.onSuccess);`);
+      expect(thenSymbols.calls).toContainEqual(
+        expect.objectContaining({ name: 'onSuccess', receiver: 'handlers', dynamic: true }),
+      );
+    });
+
+    it('does not treat cache/Map .get/.put as callback-accepting (HTTP-verb guard)', () => {
+      // `cache.get(user.id)` shares the verb name `get` with Express routes,
+      // but has no string-literal route path first arg — so member-expr args
+      // must not be emitted as dynamic calls. Same for `repo.put`, `map.delete`.
+      const cacheSymbols = parseJS(`cache.get(user.id);`);
+      expect(cacheSymbols.calls.filter((c) => c.dynamic && c.name === 'id')).toHaveLength(0);
+      const repoSymbols = parseJS(`repo.put(record.key, value);`);
+      expect(repoSymbols.calls.filter((c) => c.dynamic && c.name === 'key')).toHaveLength(0);
+      const mapSymbols = parseJS(`map.delete(entry.id);`);
+      expect(mapSymbols.calls.filter((c) => c.dynamic && c.name === 'id')).toHaveLength(0);
+    });
+
+    it('still emits member-expr args for Express HTTP routes with string path', () => {
+      // Positive regression guard: HTTP-verb calls with a string-literal
+      // first arg (Express route signature) must still emit member-expr args.
+      const routerSymbols = parseJS(`router.get('/users/:id', auth.check);`);
+      expect(routerSymbols.calls).toContainEqual(
+        expect.objectContaining({ name: 'check', receiver: 'auth', dynamic: true }),
+      );
+      const templateSymbols = parseJS('app.post(`/api`, handlers.create);');
+      expect(templateSymbols.calls).toContainEqual(
+        expect.objectContaining({ name: 'create', receiver: 'handlers', dynamic: true }),
+      );
+    });
+
+    it('handles optional-chaining callees in allowlist (obj?.on)', () => {
+      // `obj?.on(event, handler.fn)` — tree-sitter-javascript/typescript
+      // represent `obj?.on` as a `member_expression` with an `optional_chain`
+      // child, so `extractCalleeName` still returns `on` and the allowlist
+      // gate works. Guards against a previously-flagged false-negative class.
+      const symbols = parseJS(`emitter?.on('tick', handlers.fn);`);
+      expect(symbols.calls).toContainEqual(
+        expect.objectContaining({ name: 'fn', receiver: 'handlers', dynamic: true }),
+      );
+    });
+
     it('extracts callback in plain function calls like setTimeout', () => {
       const symbols = parseJS(`setTimeout(tick, 1000);`);
       expect(symbols.calls).toContainEqual(