From 34c64bf8a1a67bd4d25d6d60c1fc72da191b4b13 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 10 Nov 2026 20:01:40 +0100 Subject: [PATCH] feat(extract): record JS/TS receiver binding facts for imports and AMD The same-module guard for #1128 needs to know, from the caller's own source, whether a member call's receiver is bound to another module. Two facts were missing: - A JS/TS import statement that binds no local name (side-effect `import './x'`, a re-export, a bare or destructured `require('x')`, `require('x').y`) still records the path's last segment as its local_name, so `require('node:stream').Readable` made every `stream` parameter look import-bound. CBMImport.local_name_synthesized marks those stand-ins. A lazy `x = require('x')` and `const x = require('x').member` do bind that very name and stay unmarked. local_name itself is unchanged, so IMPORTS edges and the import map are untouched. - AMD `define([...], function (dep) {...})` / `require([...], fn)` bind each factory parameter to the dependency at the same position. That binding is local to the factory and is not an import (no IMPORTS edge), so the unified walk carries it like the Python parameter bindings: the loader call notes its factory, the factory's frame binds the dependency parameters, the frame's pop unwinds them. The CommonJS-wrapper pseudo dependencies (`exports`, `module`, `require`) stay unbound. O(1) per node, no ancestor walk. The JS member-call branch reads it into CBMCall.receiver_is_amd_dependency. Nothing reads either fact yet; the graph is unchanged by this commit. Signed-off-by: Martin Vogel --- internal/cbm/cbm.h | 10 ++ internal/cbm/extract_calls.c | 12 +++ internal/cbm/extract_imports.c | 44 ++++++++- internal/cbm/extract_unified.c | 166 ++++++++++++++++++++++++++++++++ internal/cbm/extract_unified.h | 14 +++ tests/test_extraction_imports.c | 122 +++++++++++++++++++++++ 6 files changed, 364 insertions(+), 4 deletions(-) diff --git a/internal/cbm/cbm.h b/internal/cbm/cbm.h index 86586dd15..396bf9906 100644 --- a/internal/cbm/cbm.h +++ b/internal/cbm/cbm.h @@ -376,6 +376,12 @@ typedef struct { // parameter of an enclosing function, so it cannot be the // module-level foo. Python only today. Read by the // weak-local-binding guard. Default false. + bool receiver_is_amd_dependency; // JS/TS member call x.foo() whose receiver root x is a + // parameter the factory of an enclosing AMD + // define([...], fn) / require([...], fn) binds to a + // dependency (#1128). Not an import (no IMPORTS + // edge): read by the same-module bound-receiver + // guard. Default false. bool receiver_is_self_attribute; // Python member call whose receiver is an attribute // chain rooted at self/cls but not self/cls itself // (self.compiler.apply_converters()). An object the @@ -393,6 +399,10 @@ typedef enum { } CBMImportKind; typedef struct { + /* JS/TS: the statement binds no local name (side-effect `import './x'`, + * re-export, bare `require('x').y`), so local_name is the path's last segment, + * not a binding the file can call through (#1128). Default false. */ + bool local_name_synthesized; const char *local_name; // local alias or name const char *module_path; // resolved module path / QN const char *member; // ES named import: the exported name the local binds (`get` in diff --git a/internal/cbm/extract_calls.c b/internal/cbm/extract_calls.c index 2f512e5c7..ba6c0471b 100644 --- a/internal/cbm/extract_calls.c +++ b/internal/cbm/extract_calls.c @@ -4418,6 +4418,16 @@ static CBMInvocationDescriptor describe_emitted_primary_call( return descriptor; } +/* #1128: the node that roots a JS/TS member receiver (`a` in `a.b.c()`). The + * walk is over this one callee's own member chain, so it stays linear in the + * file; a chain rooted in a call result stops at that call. */ +static TSNode js_receiver_root(TSNode obj) { + while (!ts_node_is_null(obj) && strcmp(ts_node_type(obj), "member_expression") == 0) { + obj = ts_node_child_by_field_name(obj, TS_FIELD("object")); + } + return obj; +} + CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec *spec, WalkState *state) { CBMInvocationDescriptor invocation = {0}; @@ -4489,6 +4499,8 @@ CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBML const char *ok = ts_node_type(obj); if (strcmp(ok, "this") != 0 && strcmp(ok, "super") != 0) { call.is_method = true; + call.receiver_is_amd_dependency = + cbm_walk_amd_dependency_binds(ctx, state, js_receiver_root(obj)); } } } diff --git a/internal/cbm/extract_imports.c b/internal/cbm/extract_imports.c index 576bdb8f1..bc18dfd46 100644 --- a/internal/cbm/extract_imports.c +++ b/internal/cbm/extract_imports.c @@ -941,12 +941,38 @@ static bool process_es_import_statement(CBMExtractCtx *ctx, TSNode node) { } } if (!found) { - CBMImport imp = {.local_name = path_last(a, path), .module_path = path}; + /* A side-effect import (`import './polyfills'`) binds no name. */ + CBMImport imp = { + .local_name = path_last(a, path), .module_path = path, .local_name_synthesized = true}; cbm_imports_push(&ctx->result->imports, a, imp); } return true; } +/* The identifier a require call is bound to when no declarator holds the call + * itself: a lazy assignment `winston = require('winston')`, or a declarator + * over one member `const dns = require('dns').promises`. NULL otherwise. */ +static const char *commonjs_require_bound_name(CBMExtractCtx *ctx, TSNode parent) { + if (ts_node_is_null(parent)) { + return NULL; + } + TSNode name = {0}; + const char *kind = ts_node_type(parent); + if (strcmp(kind, "assignment_expression") == 0) { + name = ts_node_child_by_field_name(parent, TS_FIELD("left")); + } else if (strcmp(kind, "member_expression") == 0) { + TSNode declarator = ts_node_parent(parent); + if (!ts_node_is_null(declarator) && + strcmp(ts_node_type(declarator), "variable_declarator") == 0) { + name = ts_node_child_by_field_name(declarator, TS_FIELD("name")); + } + } + if (ts_node_is_null(name) || strcmp(ts_node_type(name), "identifier") != 0) { + return NULL; + } + return cbm_node_text(ctx->arena, name, ctx->source); +} + /* Handle CommonJS `require("path")` call_expression nodes. The local name * is derived from the enclosing variable_declarator / assignment when * possible (so `const foo = require('./foo')` emits local_name="foo"), @@ -1004,11 +1030,19 @@ static bool process_commonjs_require(CBMExtractCtx *ctx, TSNode call) { local_name = cbm_node_text(a, name_node, ctx->source); } } - if (!local_name) { + /* No declarator binds the module (`require('x').y`, a destructuring, a + * lazy assignment or a bare side-effect require): the path's last segment + * stands in. It is still a real binding when the statement binds exactly + * that name (#1128). */ + bool synthesized = local_name == NULL; + if (synthesized) { local_name = path_last(a, path); + const char *bound = commonjs_require_bound_name(ctx, parent); + synthesized = !(bound && local_name && strcmp(bound, local_name) == 0); } - CBMImport imp = {.local_name = local_name, .module_path = path}; + CBMImport imp = { + .local_name = local_name, .module_path = path, .local_name_synthesized = synthesized}; cbm_imports_push(&ctx->result->imports, a, imp); return true; } @@ -1035,8 +1069,10 @@ static void walk_es_imports(CBMExtractCtx *ctx, TSNode root) { if (!ts_node_is_null(src)) { char *path = strip_quotes(ctx->arena, cbm_node_text(ctx->arena, src, ctx->source)); if (path && path[0]) { + /* A re-export binds no local name in this file. */ CBMImport imp = {.local_name = path_last(ctx->arena, path), - .module_path = path}; + .module_path = path, + .local_name_synthesized = true}; cbm_imports_push(&ctx->result->imports, ctx->arena, imp); } } diff --git a/internal/cbm/extract_unified.c b/internal/cbm/extract_unified.c index 96e6ac865..482abca83 100644 --- a/internal/cbm/extract_unified.c +++ b/internal/cbm/extract_unified.c @@ -306,6 +306,171 @@ static void py_bind_scope_parameters(CBMExtractCtx *ctx, TSNode node, WalkState } } +/* ── AMD dependency bindings (#1128) ────────────────────────────────────── + * `define(['axios', './util'], function (axios, util) {...})` and + * `require([...], function (...) {...})` bind each factory parameter to the + * module at the same position of the dependency array. That binding is local + * to the factory and is not an import of the file (it mints no IMPORTS edge), + * so it rides the walk like the Python parameters above: the loader call notes + * the factory it passes, and when the walk opens that factory's frame its + * dependency parameters are bound into the same per-frame map and unwound with + * the frame. A JS/TS walk binds nothing else there. O(1) per node and no + * ancestor walk (the CBMWalkScope note says why that would be quadratic). */ +enum { AMD_LOADER_MAX_ARGS = 3 }; /* define(id?, dependencies?, factory) */ + +static bool amd_js_walk(CBMLanguage lang) { + return lang == CBM_LANG_JAVASCRIPT || lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX || + lang == CBM_LANG_ARKTS; +} + +static bool amd_node_text_is(const CBMExtractCtx *ctx, TSNode node, const char *text) { + uint32_t start = ts_node_start_byte(node); + uint32_t end = ts_node_end_byte(node); + size_t len = strlen(text); + return end >= start && (size_t)(end - start) == len && end <= (uint32_t)ctx->source_len && + memcmp(ctx->source + start, text, len) == 0; +} + +static bool amd_factory_kind(const char *kind) { + return strcmp(kind, "function_expression") == 0 || strcmp(kind, "function") == 0 || + strcmp(kind, "arrow_function") == 0; +} + +/* At a loader call `define(...)` / `require(...)` / `requirejs(...)` with a + * dependency array followed by an inline factory, note that factory. */ +static void amd_note_factory(const CBMExtractCtx *ctx, TSNode node, WalkState *state) { + if (strcmp(ts_node_type(node), "call_expression") != 0) { + return; + } + TSNode fn = ts_node_child_by_field_name(node, TS_FIELD("function")); + if (ts_node_is_null(fn) || strcmp(ts_node_type(fn), "identifier") != 0 || + !(amd_node_text_is(ctx, fn, "define") || amd_node_text_is(ctx, fn, "require") || + amd_node_text_is(ctx, fn, "requirejs"))) { + return; + } + TSNode args = ts_node_child_by_field_name(node, TS_FIELD("arguments")); + uint32_t count = ts_node_is_null(args) ? 0 : ts_node_named_child_count(args); + if (count == 0 || count > AMD_LOADER_MAX_ARGS) { + return; /* not the loader's signature */ + } + TSNode deps = {0}; + for (uint32_t i = 0; i < count; i++) { + TSNode arg = ts_node_named_child(args, i); + const char *kind = ts_node_type(arg); + if (strcmp(kind, "array") == 0) { + deps = arg; + } else if (amd_factory_kind(kind) && !ts_node_is_null(deps)) { + state->amd_factory_start_byte = ts_node_start_byte(arg); + state->amd_factory_end_byte = ts_node_end_byte(arg); + state->amd_dependencies = deps; + return; + } + } +} + +/* `require`, `exports` and `module` are the CommonJS-wrapper pseudo + * dependencies: `exports` and `module` are THIS module, so their parameters + * stay unbound. */ +static bool amd_pseudo_dependency(const CBMExtractCtx *ctx, TSNode dep) { + static const char *const pseudo[] = {"'require'", "\"require\"", "'exports'", + "\"exports\"", "'module'", "\"module\""}; + for (size_t i = 0; i < sizeof(pseudo) / sizeof(pseudo[0]); i++) { + if (amd_node_text_is(ctx, dep, pseudo[i])) { + return true; + } + } + return false; +} + +/* The identifier a JS/TS factory parameter binds, or NULL for a destructuring + * or rest pattern (those bind no dependency module as a whole). */ +static const char *amd_parameter_name(CBMExtractCtx *ctx, TSNode param) { + const char *kind = ts_node_type(param); + TSNode ident = param; + if (strcmp(kind, "required_parameter") == 0 || strcmp(kind, "optional_parameter") == 0) { + ident = ts_node_child_by_field_name(param, TS_FIELD("pattern")); + } else if (strcmp(kind, "assignment_pattern") == 0) { + ident = ts_node_child_by_field_name(param, TS_FIELD("left")); + } + if (ts_node_is_null(ident) || strcmp(ts_node_type(ident), "identifier") != 0) { + return NULL; + } + return cbm_node_text(ctx->arena, ident, ctx->source); +} + +/* Step a cursor to the next named, non-comment sibling; false at the end. The + * first call must pass `first` = true to enter the children. Cursor steps are + * O(1), so pairing a dependency list with a parameter list stays linear. */ +static bool amd_next_item(TSTreeCursor *cursor, bool first) { + bool moved = + first ? ts_tree_cursor_goto_first_child(cursor) : ts_tree_cursor_goto_next_sibling(cursor); + while (moved) { + TSNode node = ts_tree_cursor_current_node(cursor); + if (ts_node_is_named(node) && strcmp(ts_node_type(node), "comment") != 0) { + return true; + } + moved = ts_tree_cursor_goto_next_sibling(cursor); + } + return false; +} + +/* When the walk opens the noted factory's frame (pushed at `depth`), bind each + * parameter whose dependency is a module string. */ +static void amd_bind_factory_parameters(CBMExtractCtx *ctx, TSNode node, WalkState *state, + uint32_t depth) { + if (state->amd_factory_end_byte == 0 || + ts_node_start_byte(node) != state->amd_factory_start_byte || + ts_node_end_byte(node) != state->amd_factory_end_byte || + !amd_factory_kind(ts_node_type(node))) { + return; + } + TSNode deps = state->amd_dependencies; + state->amd_factory_start_byte = 0; + state->amd_factory_end_byte = 0; + state->amd_dependencies = (TSNode){0}; + /* Bind only into the factory's own frame, or the bindings would outlive it. */ + if (state->scope_top == 0 || state->scopes[state->scope_top - SKIP_ONE].depth != depth) { + return; + } + TSNode single = ts_node_child_by_field_name(node, TS_FIELD("parameter")); /* `dep => ...` */ + TSNode params = ts_node_child_by_field_name(node, TS_FIELD("parameters")); + if (ts_node_is_null(single) && ts_node_is_null(params)) { + return; + } + TSTreeCursor dep_cursor = ts_tree_cursor_new(deps); + TSTreeCursor param_cursor = ts_tree_cursor_new(ts_node_is_null(single) ? params : node); + bool have_dep = amd_next_item(&dep_cursor, true); + bool have_param = ts_node_is_null(single) ? amd_next_item(¶m_cursor, true) : true; + while (have_dep && have_param) { + TSNode dep = ts_tree_cursor_current_node(&dep_cursor); + TSNode param = + ts_node_is_null(single) ? ts_tree_cursor_current_node(¶m_cursor) : single; + if (strcmp(ts_node_type(dep), "string") == 0 && !amd_pseudo_dependency(ctx, dep)) { + py_param_bind(state, amd_parameter_name(ctx, param)); + } + have_dep = amd_next_item(&dep_cursor, false); + have_param = ts_node_is_null(single) && amd_next_item(¶m_cursor, false); + } + ts_tree_cursor_delete(&dep_cursor); + ts_tree_cursor_delete(¶m_cursor); +} + +static void amd_track_bindings(CBMExtractCtx *ctx, TSNode node, WalkState *state, uint32_t depth) { + if (!amd_js_walk(ctx->language)) { + return; + } + amd_bind_factory_parameters(ctx, node, state, depth); + amd_note_factory(ctx, node, state); +} + +bool cbm_walk_amd_dependency_binds(CBMExtractCtx *ctx, const WalkState *state, TSNode ident) { + if (!ctx || !state || !amd_js_walk(state->language) || state->py_param_stack_count == 0 || + ts_node_is_null(ident) || strcmp(ts_node_type(ident), "identifier") != 0) { + return false; + } + return cbm_walk_python_param_is_bound(state, cbm_node_text(ctx->arena, ident, ctx->source)); +} + static bool push_scope(WalkState *state, uint8_t kind, uint32_t depth, const char *qn) { if (!ensure_scope_capacity(state)) { return false; @@ -2564,6 +2729,7 @@ static void push_boundary_scopes(CBMExtractCtx *ctx, TSNode node, const CBMLangS push_lexical_boundary(node, state, depth); py_bind_scope_parameters(ctx, node, state); + amd_track_bindings(ctx, node, state, depth); push_call_scope(state, depth, invocation); if (is_actual_import_boundary(ctx, node, spec) && !is_export_of_declaration(node)) { push_scope(state, SCOPE_IMPORT, depth, NULL); diff --git a/internal/cbm/extract_unified.h b/internal/cbm/extract_unified.h index f80237229..2414b6433 100644 --- a/internal/cbm/extract_unified.h +++ b/internal/cbm/extract_unified.h @@ -198,6 +198,14 @@ typedef struct { /* Allocation failure: stop tracking and answer "not bound" forever after, * which can only cost a suppression, never a true edge. */ bool py_param_tracking_failed; + /* #1128: the factory the AMD define/require call visited last passes (its + * exact span; end 0 = none) and that call's dependency array. The walk + * reaches the factory right after the array, and when it opens the + * factory's frame the dependency parameters are bound into the parameter + * map above, which a JS/TS walk uses for nothing else. */ + uint32_t amd_factory_start_byte; + uint32_t amd_factory_end_byte; + TSNode amd_dependencies; CBMLanguage language; @@ -209,6 +217,12 @@ typedef struct { * caller can only ever lose a suppression, never a true edge. */ bool cbm_walk_python_param_is_bound(const WalkState *state, const char *name); +/* #1128: is identifier `ident` a parameter that the factory of an enclosing AMD + * define([...], fn) / require([...], fn) binds to a dependency module? JS/TS + * walks only. O(1); false on any failure, so a caller can only lose a + * suppression, never a true edge. */ +bool cbm_walk_amd_dependency_binds(CBMExtractCtx *ctx, const WalkState *state, TSNode ident); + // Per-node handler prototypes. Each is called once per node during the // unified cursor walk, replacing the old recursive walk_* functions. CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec *spec, diff --git a/tests/test_extraction_imports.c b/tests/test_extraction_imports.c index 1b1e93d6c..35f0501e9 100644 --- a/tests/test_extraction_imports.c +++ b/tests/test_extraction_imports.c @@ -1310,6 +1310,124 @@ TEST(es_named_import_member_survives_compaction) { PASS(); } +/* #1128: a JS/TS import whose statement binds no local name keeps the path's + * last segment as local_name (the import map still keys on it) but says so, so + * the same-module bound-receiver guard does not read `require('node:stream')` + * as binding every `stream` parameter. A lazy `x = require('x')` and + * `const x = require('x').member` do bind that very name. */ +static const CBMImport *imp_by_path(const CBMFileResult *r, const char *module_path) { + for (int i = 0; i < r->imports.count; i++) { + if (r->imports.items[i].module_path && + strcmp(r->imports.items[i].module_path, module_path) == 0) { + return &r->imports.items[i]; + } + } + return NULL; +} + +TEST(js_import_local_name_synthesized) { + CBMFileResult *r = do_extract("import axios from 'axios';\n" + "import * as fs from 'node:fs';\n" + "import { client as http } from './client';\n" + "import './polyfills';\n" + "export * from './reexported';\n" + "const db = require('./database');\n" + "let winston;\n" + "winston = require('winston');\n" + "let store;\n" + "store = require('./kv-store');\n" + "const dns = require('dns').promises;\n" + "const { join } = require('path');\n" + "require('./side-effect');\n" + "const R = require('node:stream').Readable;\n", + CBM_LANG_JAVASCRIPT, "synth.js"); + ASSERT_NOT_NULL(r); + static const struct { + const char *path; + bool synthesized; + } cases[] = { + {"axios", false}, {"node:fs", false}, {"./client", false}, + {"./polyfills", true}, {"./reexported", true}, {"./database", false}, + {"winston", false}, {"./kv-store", true}, {"dns", false}, + {"path", true}, {"./side-effect", true}, {"node:stream", true}, + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + const CBMImport *imp = imp_by_path(r, cases[i].path); + if (!imp || imp->local_name_synthesized != cases[i].synthesized) { + printf(" FAIL [%s]: %s\n", cases[i].path, imp ? "flag" : "missing"); + } + ASSERT_NOT_NULL(imp); + ASSERT_EQ(imp->local_name_synthesized, cases[i].synthesized); + } + /* The stand-in names themselves are unchanged (the import map keys on them). */ + ASSERT_STR_EQ(imp_by_path(r, "./side-effect")->local_name, "side-effect"); + ASSERT_STR_EQ(imp_by_path(r, "./kv-store")->local_name, "kv-store"); + cbm_free_result(r); + PASS(); +} + +/* #1128: an AMD factory binds each parameter to the dependency at the same + * position (`exports`/`module`/`require` are the module itself or the loader), + * for the factory's body only. The fact rides the call (no IMPORTS edge). */ +static const CBMCall *call_by_callee(const CBMFileResult *r, const char *callee) { + for (int i = 0; i < r->calls.count; i++) { + if (r->calls.items[i].callee_name && strcmp(r->calls.items[i].callee_name, callee) == 0) { + return &r->calls.items[i]; + } + } + return NULL; +} + +TEST(js_amd_dependency_receiver_flag) { + CBMFileResult *r = do_extract( + "define('m', ['api', 'exports', 'alerts'], function (api, exports, alerts) {\n" + " api.get(1);\n" + " api.client.load(2);\n" + " exports.put(3);\n" + " alerts.alert(4);\n" + " other.post(5);\n" + "});\n" + "require(['hooks'], (hooks) => { hooks.fire(6); });\n" + "require(['runner'], h => { h.run(7); });\n" + "api.after(8);\n" + "define(function (require) { const util = require('util'); util.format(9); });\n", + CBM_LANG_JAVASCRIPT, "amd.js"); + ASSERT_NOT_NULL(r); + static const struct { + const char *callee; + bool amd; + } cases[] = { + {"api.get", true}, {"api.client.load", true}, {"exports.put", false}, + {"alerts.alert", true}, {"other.post", false}, {"hooks.fire", true}, + {"h.run", true}, {"api.after", false}, {"util.format", false}, + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + const CBMCall *call = call_by_callee(r, cases[i].callee); + if (!call || call->receiver_is_amd_dependency != cases[i].amd) { + printf(" FAIL [%s]: %s\n", cases[i].callee, call ? "flag" : "missing"); + } + ASSERT_NOT_NULL(call); + ASSERT_EQ(call->receiver_is_amd_dependency, cases[i].amd); + ASSERT_TRUE(call->is_method); + } + /* AMD dependencies mint no import (and so no IMPORTS edge). */ + ASSERT_NULL(imp_by_path(r, "api")); + ASSERT_NULL(imp_by_path(r, "hooks")); + cbm_free_result(r); + + /* TypeScript parameter shape (`api: Api`). */ + CBMFileResult *t = do_extract("define(['api'], function (api: Api) {\n" + " api.get(1);\n" + "});\n", + CBM_LANG_TYPESCRIPT, "amd.ts"); + ASSERT_NOT_NULL(t); + const CBMCall *typed = call_by_callee(t, "api.get"); + ASSERT_NOT_NULL(typed); + ASSERT_TRUE(typed->receiver_is_amd_dependency); + cbm_free_result(t); + PASS(); +} + /* ═══════════════════════════════════════════════════════════════════════════ * SUITE declaration — do NOT register in test_main.c; the main workstream will * ═══════════════════════════════════════════════════════════════════════════ */ @@ -1353,4 +1471,8 @@ SUITE(extraction_imports) { /* ES named-import member (call resolution spells calls through it) */ RUN_TEST(es_named_import_member_survives_compaction); + + /* #1128 binding facts for the same-module bound-receiver guard */ + RUN_TEST(js_import_local_name_synthesized); + RUN_TEST(js_amd_dependency_receiver_flag); }