From 8f9bfc2e778dcf867d02b5b28101d04449710234 Mon Sep 17 00:00:00 2001 From: Vitali Avagyan Date: Sat, 3 Oct 2026 05:03:57 +0000 Subject: [PATCH 1/5] fix(parsers): bind JS/TS calls through barrel re-exports to the definition (#2464) --- codebase_rag/constants/ast_js.py | 3 + codebase_rag/constants/ast_nodes.py | 2 + codebase_rag/editing/rename.py | 85 +++ codebase_rag/parsers/call_resolver.py | 33 + codebase_rag/parsers/import_processor.py | 80 ++- codebase_rag/parsers/js_ts/reexports.py | 108 ++++ .../tests/test_ts_barrel_reexport_calls.py | 569 ++++++++++++++++++ docs/architecture/rename.md | 9 + 8 files changed, 888 insertions(+), 1 deletion(-) create mode 100644 codebase_rag/parsers/js_ts/reexports.py create mode 100644 codebase_rag/tests/test_ts_barrel_reexport_calls.py diff --git a/codebase_rag/constants/ast_js.py b/codebase_rag/constants/ast_js.py index d00999d4aa..2229713f8e 100644 --- a/codebase_rag/constants/ast_js.py +++ b/codebase_rag/constants/ast_js.py @@ -44,6 +44,9 @@ JS_THIS_CALL_PREFIX = "this." JS_TS_PARENT_REF_TYPES = (TS_IDENTIFIER, TS_MEMBER_EXPRESSION) +# The `*` of `export * from './m'`. The JavaScript and TypeScript grammars +# spell it as an anonymous token, not Java's named `asterisk` node. +TS_JS_STAR = "*" # JSX element nodes that carry a component name (javascript and tsx grammars # share these); the closing element repeats the name and must not double-emit. TS_JSX_SELF_CLOSING_ELEMENT = "jsx_self_closing_element" diff --git a/codebase_rag/constants/ast_nodes.py b/codebase_rag/constants/ast_nodes.py index 4e21b08be3..6c27896e18 100644 --- a/codebase_rag/constants/ast_nodes.py +++ b/codebase_rag/constants/ast_nodes.py @@ -132,6 +132,8 @@ FIELD_BOUNDS = "bounds" TS_RS_TRAIT_BOUNDS = "trait_bounds" FIELD_VALUE = "value" +FIELD_DECLARATION = "declaration" +FIELD_SOURCE = "source" FIELD_LEFT = "left" FIELD_RIGHT = "right" # A C-style for's post-iteration clause: Java/C++ hold it in an `update` diff --git a/codebase_rag/editing/rename.py b/codebase_rag/editing/rename.py index 34b348faa4..4a2b39c272 100644 --- a/codebase_rag/editing/rename.py +++ b/codebase_rag/editing/rename.py @@ -443,8 +443,93 @@ def _collect(self, qn: str) -> tuple[list[RenameSite], list[str], str, str | Non old_name, patcher, ) + sites = self._js_import_bound(qn, path, sites, old_name, patcher) return sites, unlocatable, old_name, definition["label"] + def _js_import_bound( + self, + qn: str, + path: str, + sites: list[RenameSite], + old_name: str, + patcher: Patcher, + ) -> list[RenameSite]: + """The sites, as the JS/TS imports binding each one let it be renamed. + + The graph binds `mul(8, 9)` to `mul` through a default import, and + `add(1, 2)` through a barrel's `export *`: the token equals the + definition's name either way (issue #2464). A bare use in another + file follows the rename only when its file imports the name by the + definition's own name, so the import is rewritten with it. One whose + file binds a name of its own is left as it is, and one bound through + an import the rename cannot rewrite is a guess: rewriting the use + alone would leave the import naming what no longer exists. + """ + bindings: tuple[set[str], set[str]] | None = None + kept: list[RenameSite] = [] + for site in sites: + if ( + site.kind not in ("call", "reference") + or site.path == path + or get_language_for_extension(Path(site.path).suffix) + not in cs.JS_TS_LANGUAGES + or self._is_member_token(patcher, site) + ): + kept.append(site) + continue + if bindings is None: + bindings = self._js_name_bindings(qn, old_name) + by_name, aliased = bindings + if site.path in by_name: + kept.append(site) + elif site.path not in aliased: + kept.append(site._replace(resolution=cs.EdgeResolution.HEURISTIC.value)) + return kept + + @staticmethod + def _is_member_token(patcher: Patcher, site: RenameSite) -> bool: + # `ns.helper(1)` names the member by its own name whatever binds + # `ns`; only a bare `helper(1)` is bound by an import of that name. + try: + source = patcher.source(site.path) + offset = line_col_to_byte(source, site.line, site.col) + except PatcherError: + return False + return source[:offset].rstrip().endswith(b".") + + def _js_name_bindings(self, qn: str, name: str) -> tuple[set[str], set[str]]: + """(files importing `name` by the definition's own name, files whose + `name` is a binding of their own for the definition).""" + module_qn, _path = self._module_of(qn) + by_name = { + site.path + for site, _module in self._import_sites(qn, name) + if (site.alias or site.imported_name) == name + } + # `import mul from`, `const mul = require(...)` and a barrel's + # `export { default as mul } from` name the binding as the importer + # chose, and so does every module importing that `mul` from such a + # barrel: the definition's rename changes none of them. + aliased: set[str] = set() + pending = [(module_qn, False)] + seen = {module_qn} + while pending: + source, chosen = pending.pop() + for row in graph_query.importers(self.fetch_all, self.project, source): + # The definition's own name is rewritten with its import; a + # barrel's chosen name only passes on under that same name. + if ( + not row["path"] + or row["alias"] != name + or (row["imported_name"] == name) != chosen + ): + continue + aliased.add(row["path"]) + if row["module"] not in seen: + seen.add(row["module"]) + pending.append((row["module"], True)) + return by_name, aliased + def _definition_site( self, qn: str, diff --git a/codebase_rag/parsers/call_resolver.py b/codebase_rag/parsers/call_resolver.py index 92fdfbdac2..5d53f70b71 100644 --- a/codebase_rag/parsers/call_resolver.py +++ b/codebase_rag/parsers/call_resolver.py @@ -2165,6 +2165,34 @@ def _is_super_call(self, call_name: str) -> bool: or call_name.startswith(f"{cs.KEYWORD_SUPER}()") ) + def _try_resolve_js_reexport( + self, + call_name: str, + import_map: dict[str, str], + language: cs.SupportedLanguage | None, + module_qn: str, + ) -> tuple[str, str] | None: + # `import { plus } from "./lib"` maps `plus` to `lib.plus`, where a + # barrel registers nothing: its `export { add as plus } from` names + # the definition, possibly through further barrels. Without this the + # call fell to the name-only fallback, which found no `plus` at all, + # or only guessed the right `add` (issue #2464). + if language not in cs.JS_TS_LANGUAGES or call_name not in import_map: + return None + imported_qn = import_map[call_name] + followed = self.import_processor.follow_js_reexports( + imported_qn, + self.type_inference.module_qn_to_file_path, + self.function_registry, + ) + if followed == imported_qn: + return None + # The chain's end resolves as a direct import of it would, so a + # CommonJS whole-module export there is still reached. + return self._try_resolve_direct_import( + call_name, {call_name: followed}, language, module_qn + ) + def _try_resolve_via_imports( self, call_name: str, @@ -2189,6 +2217,11 @@ def _try_resolve_via_imports( ): return result + if result := self._try_resolve_js_reexport( + call_name, import_map, language, module_qn + ): + return result + if result := self._try_resolve_qualified_call( call_name, import_map, module_qn, local_var_types, language ): diff --git a/codebase_rag/parsers/import_processor.py b/codebase_rag/parsers/import_processor.py index 18e8887dbe..b819df4389 100644 --- a/codebase_rag/parsers/import_processor.py +++ b/codebase_rag/parsers/import_processor.py @@ -51,6 +51,7 @@ discover_js_workspace_packages, resolve_js_workspace_import, ) +from .js_ts.reexports import follow_js_reexports from .lua import utils as lua_utils from .python_source_roots import discover_python_source_roots, resolve_via_source_roots from .rs import utils as rs_utils @@ -394,6 +395,19 @@ def _js_pair_binding(pair: Node) -> tuple[str, str] | None: return None +def _js_default_export_local(statement: Node) -> str | None: + # The local name `export default` publishes: a named function or class + # declaration, or a bare identifier. An anonymous function or any other + # expression is known by no name a binding could hold. + declaration = statement.child_by_field_name(cs.FIELD_DECLARATION) + if declaration is not None: + return safe_decode_text(declaration.child_by_field_name(cs.FIELD_NAME)) + value = statement.child_by_field_name(cs.FIELD_VALUE) + if value is not None and value.type == cs.TS_IDENTIFIER: + return safe_decode_text(value) + return None + + def _load_jsonc(path: Path) -> dict | None: # tsconfig.json is JSONC (comments, trailing commas). Try strict JSON first, # then fall back to stripping comments/trailing commas. The naive strip can @@ -818,6 +832,7 @@ class ImportProcessor: "_deferred_import_edges", "unresolved_specifiers", "unresolved_references", + "js_export_bindings", "_import_sites", "_import_site_owners", "_import_site_writers", @@ -912,6 +927,13 @@ def __init__( # writes it onto the Module node after Pass 3, and matches an added # file against it to find the modules that waited (issue #1568). self.unresolved_references: dict[str, set[str]] = {} + # JS/TS module qn -> {exported name: local qn} for the module's own + # bindings it exports under another name (`export { trim as strip }`, + # `export default upper`). An importer's map holds `.strip`, + # which names nothing registered; the call resolver follows it here + # to the definition (issue #2464). Re-exports with a `from` live in + # import_mapping instead. + self.js_export_bindings: dict[str, dict[str, str]] = {} # Per scope qn: the site props of each bound import name (#1522), # attached to the IMPORTS edge when the deferred edge flushes. self._import_sites: dict[str, dict[str, PropertyDict]] = {} @@ -1262,6 +1284,8 @@ def _clear_module_import_state(self, module_qn: str) -> None: self._cpp_declaration_mappings = { entry for entry in self._cpp_declaration_mappings if entry[0] != module_qn } + # Re-derived from the export statements the re-parse finds. + self.js_export_bindings.pop(module_qn, None) # The Dart prefix maps share the same invariant. Both are written # only when the file HAS prefixed imports, so removing the last one # left the previous parse's entries in place: a stale alias kept @@ -1927,6 +1951,11 @@ def _flush_resolved_import( self.note_unresolved(entry.module_qn, entry.full_name) return 0 module_path = verified + if module_path == entry.module_qn and entry.language in cs.JS_TS_LANGUAGES: + # `export * from "./add"` in `math/index.ts` names `math.add`, + # which reads as the name `add` of the `math` barrel: the barrel + # itself. A module importing itself is no edge (issue #2464). + return 0 self._emit_import_edge(entry, target_label, module_path) logger.debug( ls.IMP_CREATED_RELATIONSHIP, @@ -3659,6 +3688,7 @@ def _parse_js_ts_imports(self, captures: dict, module_qn: str) -> None: elif import_node.type == cs.TS_EXPORT_STATEMENT: self._parse_js_reexport(import_node, module_qn) + self._record_js_export_bindings(import_node, module_qn) def _ts_alias_module_qn(self, import_path: str) -> str | None: # Resolve a tsconfig `paths` alias (`@/util` -> `src/util`) to the @@ -3964,7 +3994,7 @@ def _parse_js_reexport(self, export_node: Node, current_module: str) -> None: return for child in export_node.children: - if child.type == cs.TS_ASTERISK: + if child.type == cs.TS_JS_STAR: wildcard_key = f"*{source_module}" self.import_mapping[current_module][wildcard_key] = source_module self._record_import_site( @@ -4009,6 +4039,54 @@ def _record_js_reexport_specifier( original=original_name, ) + def follow_js_reexports( + self, + qn: str, + module_paths: Mapping[str, Path], + function_registry: FunctionRegistryTrieProtocol, + ) -> str: + """Where a JS/TS import of `qn` leads through the barrels re-exporting + it; see `js_ts.reexports.follow_js_reexports`.""" + return follow_js_reexports( + qn, + self.import_mapping, + self.js_export_bindings, + module_paths, + function_registry, + ) + + def _record_js_export_bindings(self, statement: Node, module_qn: str) -> None: + """Record the local bindings a module exports under another name. + + `export { trim as strip }` and `export default upper` publish a local + binding as `strip` and `default`, names no definition is registered + under, so a call through them reached nothing (issue #2464). A + statement with a `from` names another module's binding, which + `_parse_js_reexport` maps. + """ + if statement.child_by_field_name(cs.FIELD_SOURCE) is not None: + return + exported: dict[str, str] = {} + if any(child.type == cs.TS_EXPORT_DEFAULT for child in statement.children): + if local := _js_default_export_local(statement): + exported[cs.TS_EXPORT_DEFAULT] = local + for clause in statement.named_children: + if clause.type != cs.TS_EXPORT_CLAUSE: + continue + for specifier in clause.named_children: + local = safe_decode_text(specifier.child_by_field_name(cs.FIELD_NAME)) + alias = safe_decode_text(specifier.child_by_field_name(cs.FIELD_ALIAS)) + # An unaliased `export { pad }` already resolves as `.pad`. + if local and alias and alias != local: + exported[alias] = local + if exported: + self.js_export_bindings.setdefault(module_qn, {}).update( + { + name: f"{module_qn}{cs.SEPARATOR_DOT}{local}" + for name, local in exported.items() + } + ) + def _parse_java_imports(self, captures: dict, module_qn: str) -> None: for import_node in captures.get(cs.CAPTURE_IMPORT, []): if import_node.type != cs.TS_IMPORT_DECLARATION: diff --git a/codebase_rag/parsers/js_ts/reexports.py b/codebase_rag/parsers/js_ts/reexports.py new file mode 100644 index 0000000000..e949ca8712 --- /dev/null +++ b/codebase_rag/parsers/js_ts/reexports.py @@ -0,0 +1,108 @@ +"""Follow a JS/TS import through the barrels that re-export it (issue #2464). + +A library exposes its API through barrels (`lib/index.ts`): `export * from`, +`export { add as plus } from`, `export { default as times } from`, or an +import the barrel exports again. An importer's map names the barrel +(`lib.plus`), where nothing is registered; what the barrel binds that name to +is in its own import map (the re-exports with a `from`) and its export +bindings (its own bindings exported under another name). +""" + +from __future__ import annotations + +from collections.abc import Mapping +from pathlib import Path + +from ... import constants as cs +from ...types_defs import FunctionRegistryTrieProtocol + + +def follow_js_reexports( + qn: str, + import_mapping: Mapping[str, Mapping[str, str]], + export_bindings: Mapping[str, Mapping[str, str]], + module_paths: Mapping[str, Path], + function_registry: FunctionRegistryTrieProtocol, +) -> str: + """Where the import of `qn` (`.`) leads through barrels. + + The registered definition the chain of barrels reaches, or the last name + it reached when it ends anywhere else: a name no barrel binds, two star + sources exporting it, a cycle, or `.default` for a default export + known by no name. + """ + return _ReexportWalk( + import_mapping, export_bindings, module_paths, function_registry + ).follow(qn, set()) + + +class _ReexportWalk: + __slots__ = ("_imports", "_exports", "_modules", "_registry") + + def __init__( + self, + import_mapping: Mapping[str, Mapping[str, str]], + export_bindings: Mapping[str, Mapping[str, str]], + module_paths: Mapping[str, Path], + function_registry: FunctionRegistryTrieProtocol, + ) -> None: + self._imports = import_mapping + self._exports = export_bindings + self._modules = module_paths + self._registry = function_registry + + def follow(self, qn: str, seen: set[str]) -> str: + # `seen` is shared with the star branches, so a cycle of barrels ends. + current = qn + while current not in self._registry and current not in seen: + seen.add(current) + following = self._hop(current, seen) + if following is None: + break + current = following + return current + + def _hop(self, qn: str, seen: set[str]) -> str | None: + module_qn, separator, name = qn.rpartition(cs.SEPARATOR_DOT) + barrel = self._barrel(module_qn) if separator else None + if barrel is None: + return None + # The barrel's own `export { local as name }` comes first: a module + # may import a name and export another binding under it. + if (local := self._exports.get(barrel, {}).get(name)) is not None: + return local + bindings = self._imports.get(barrel, {}) + if (bound := bindings.get(name)) is not None: + return bound + # `export *` passes on every name a source exports except `default`. + if name == cs.TS_EXPORT_DEFAULT: + return None + return self._star_source(bindings, name, seen) + + def _barrel(self, module_qn: str) -> str | None: + # A specifier names `lib.ts` when it exists, else `lib/index.ts`. + # Only a JS/TS module's maps hold JS export semantics. + for candidate in ( + module_qn, + f"{module_qn}{cs.SEPARATOR_DOT}{cs.JS_INDEX_STEM}", + ): + path = self._modules.get(candidate) + if path is not None and path.suffix in cs.JS_TS_ALL_EXTENSIONS: + return candidate + return None + + def _star_source( + self, bindings: Mapping[str, str], name: str, seen: set[str] + ) -> str | None: + # Each `export * from` source may export the name, itself possibly + # through further barrels. Two that reach different definitions make + # the name ambiguous, and TypeScript exports neither, so nothing is + # guessed. + reached = { + target + for key, source in bindings.items() + if key.startswith(cs.IMPORTED_NAME_WILDCARD) + and (target := self.follow(f"{source}{cs.SEPARATOR_DOT}{name}", seen)) + in self._registry + } + return reached.pop() if len(reached) == 1 else None diff --git a/codebase_rag/tests/test_ts_barrel_reexport_calls.py b/codebase_rag/tests/test_ts_barrel_reexport_calls.py new file mode 100644 index 0000000000..16977aeeb8 --- /dev/null +++ b/codebase_rag/tests/test_ts_barrel_reexport_calls.py @@ -0,0 +1,569 @@ +"""Issue #2464: a call through a TS/JS barrel's re-export binds exactly. + +Barrel files (`index.ts`) are how TS/JS libraries expose their API. A call +through a renamed re-export (`export { add as plus } from`) or a default +re-export (`export { default as times } from`) got no CALLS edge, because the +importer's map named the barrel (`lib.plus`), where nothing is registered, and +the name-only fallback found no `plus` anywhere. A plain named re-export +(`export { x } from`), `export *` and a direct default import got only a +`heuristic` edge from that fallback, which `cgr rename` refuses to rewrite +through. The re-export itself names the definition, so each of these binds +`exact` when the chain of barrels leads to exactly one definition. +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from codebase_rag import constants as cs +from codebase_rag.editing.rename import RenameRefused, rename +from codebase_rag.graph_updater import GraphUpdater +from codebase_rag.parser_loader import load_parsers +from codebase_rag.tests.conftest import create_and_run_updater +from codebase_rag.tests.test_rename_op import RecordedGraph +from evals.cgr_graph import _StatefulIngestor + +EXACT = cs.EdgeResolution.EXACT.value +CALLS = cs.RelationshipType.CALLS.value + +# The issue's layout, verbatim. +ISSUE_FILES = { + "src/lib/math/add.ts": ( + "export function add(a: number, b: number): number {\n return a + b;\n}\n" + ), + "src/lib/math/mul.ts": ( + "export default function mul(a: number, b: number): number {\n" + " return a * b;\n}\n" + ), + "src/lib/math/index.ts": ( + 'export * from "./add";\nexport { default as times } from "./mul";\n' + ), + "src/lib/index.ts": ( + 'export * as math from "./math";\nexport { add as plus } from "./math/add";\n' + ), + "src/app/main.ts": ( + 'import { add, times } from "../lib/math";\n' + 'import { math, plus } from "../lib";\n' + 'import mul from "../lib/math/mul";\n' + "\n" + "export function run(): number {\n" + " return add(1, 2) + times(2, 3) + math.add(4, 5) + plus(6, 7) + mul(8, 9);\n" + "}\n" + ), +} +ISSUE_ADD = "barrel.src.lib.math.add.add" +ISSUE_MUL = "barrel.src.lib.math.mul.mul" + +# The issue comment's layout: a plain named re-export, and the forms that +# already resolved beside it. +COMMENT_FILES = { + "src/lib/util.ts": ( + "export function formatPrice(cents: number): string {\n" + " return `$${(cents / 100).toFixed(2)}`;\n}\n" + ), + "src/lib/index.ts": 'export { formatPrice } from "./util";\n', + "src/app.ts": ( + 'import { formatPrice } from "./lib";\n\n' + "export function show(): string {\n return formatPrice(7);\n}\n" + ), + "src/alias.ts": ( + 'import { formatPrice as fp } from "./lib";\n\n' + "export function short(): string {\n return fp(1);\n}\n" + ), + "src/cart.ts": ( + 'import { formatPrice } from "./lib/util";\n' + 'import * as u from "./lib/util";\n\n' + "export function total(): string {\n" + " return formatPrice(10) + u.formatPrice(2);\n}\n" + ), +} +FORMAT_PRICE = "shop.src.lib.util.formatPrice" + +# Barrels of barrels: an alias of an alias, a default re-exported twice, and +# a star of a star, each three hops from the definition. +CHAIN_FILES = { + "src/core/scale.ts": ( + "export function scale(n: number): number {\n return n * 2;\n}\n\n" + "export default function shift(n: number): number {\n return n + 1;\n}\n" + ), + "src/core/index.ts": ( + 'export * from "./scale";\nexport { default as nudge } from "./scale";\n' + ), + "src/api/index.ts": ( + 'export { scale as resize, nudge } from "../core";\nexport * from "../core";\n' + ), + "src/index.ts": ( + 'export { resize as grow } from "./api";\nexport { nudge as bump } from "./api";\n' + ), + "src/use.ts": ( + 'import { grow, bump } from ".";\n' + 'import { scale, resize } from "./api";\n\n' + "export function use(): number {\n" + " return grow(1) + bump(2) + scale(3) + resize(4);\n}\n" + ), +} +CHAIN_SCALE = "chain.src.core.scale.scale" +CHAIN_SHIFT = "chain.src.core.scale.shift" + +# A barrel that imports and then exports (no `from` on the export), and a +# module exporting its own definitions under another name. +LOCAL_FILES = { + "src/text/pad.ts": ( + "export function pad(s: string): string {\n return ` ${s}`;\n}\n\n" + "function trim(s: string): string {\n return s.trim();\n}\n\n" + "export { trim as strip };\n\n" + "function upper(s: string): string {\n return s.toUpperCase();\n}\n\n" + "export default upper;\n" + ), + "src/text/index.ts": ( + 'import { pad } from "./pad";\n' + 'import upper, { strip } from "./pad";\n\n' + "export { pad as leftPad, strip, upper as shout };\n" + ), + "src/main.ts": ( + 'import { leftPad, strip, shout } from "./text";\n' + 'import { strip as clean } from "./text/pad";\n\n' + "export function main(): string {\n" + ' return leftPad("a") + strip(" b ") + shout("c") + clean("d");\n}\n' + ), +} +LOCAL_PAD = "local.src.text.pad.pad" +LOCAL_TRIM = "local.src.text.pad.trim" +LOCAL_UPPER = "local.src.text.pad.upper" + +# What must stay unbound: two star sources exporting one name, a default a +# star does not carry, and a cycle of stars. +NEGATIVE_FILES = { + "src/a.ts": ( + "export function dup(): number {\n return 1;\n}\n\n" + "export function onlyA(): number {\n return 2;\n}\n" + ), + "src/b.ts": ( + "export function dup(): number {\n return 3;\n}\n\n" + "export default function bdef(): number {\n return 4;\n}\n" + ), + "src/c.ts": "export function other(): number {\n return 5;\n}\n", + "src/stars/index.ts": 'export * from "../a";\nexport * from "../b";\n', + "src/pick/index.ts": ( + 'export * from "../a";\nexport { other as dup } from "../c";\n' + ), + "src/loop1.ts": 'export * from "./loop2";\n', + "src/loop2.ts": 'export * from "./loop1";\n', + "src/neg.ts": ( + 'import { dup, onlyA } from "./stars";\n' + 'import starDefault from "./stars";\n' + 'import { dup as picked } from "./pick";\n' + 'import { ghost } from "./loop1";\n\n' + "export function neg(): number {\n" + " return dup() + onlyA() + starDefault() + picked() + ghost();\n}\n" + ), +} + + +def _run(root: Path, files: dict[str, str]) -> tuple[GraphUpdater, _StatefulIngestor]: + for rel, text in files.items(): + path = root / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + parsers, queries = load_parsers() + if cs.SupportedLanguage.TS not in parsers: + pytest.skip("typescript parser not available") + store = _StatefulIngestor() + updater = GraphUpdater( + ingestor=store, + repo_path=root, + parsers=parsers, + queries=queries, + project_name=root.name, + ) + updater.run(force=True) + return updater, store + + +# The React barrel shape: a default-exported component re-exported under its +# own name, constructed through the barrel and through a default import. +WIDGET_FILES = { + "src/widget.ts": ( + "export default class Widget {\n draw(): number {\n return 1;\n }\n}\n" + ), + "src/kit/index.ts": 'export { default as Widget } from "../widget";\n', + "src/app.ts": ( + 'import Widget from "./widget";\n\n' + "export function make() {\n return new Widget();\n}\n" + ), + "src/app2.ts": ( + 'import { Widget } from "./kit";\n\n' + "export function make2() {\n return new Widget();\n}\n" + ), +} + + +def _index(root: Path, files: dict[str, str]) -> _StatefulIngestor: + return _run(root, files)[1] + + +def _calls(store: _StatefulIngestor, caller: str) -> dict[int, tuple[str, str]]: + """The CALLS edges out of `caller`, by call column: (target, resolution).""" + out: dict[int, tuple[str, str]] = {} + for edge in store.keyed_edges: + if edge[1] != caller or edge[2] != CALLS: + continue + props = store.props_for(edge) + col = props.get(cs.KEY_COL) + assert isinstance(col, int), props + out[col] = (str(edge[4]), str(props.get(cs.KEY_RESOLUTION))) + return out + + +@pytest.fixture(scope="module") +def issue_store(tmp_path_factory: pytest.TempPathFactory) -> _StatefulIngestor: + return _index(tmp_path_factory.mktemp("issue") / "barrel", ISSUE_FILES) + + +@pytest.fixture(scope="module") +def comment_store(tmp_path_factory: pytest.TempPathFactory) -> _StatefulIngestor: + return _index(tmp_path_factory.mktemp("comment") / "shop", COMMENT_FILES) + + +@pytest.fixture(scope="module") +def chain_store(tmp_path_factory: pytest.TempPathFactory) -> _StatefulIngestor: + return _index(tmp_path_factory.mktemp("chain") / "chain", CHAIN_FILES) + + +@pytest.fixture(scope="module") +def local_store(tmp_path_factory: pytest.TempPathFactory) -> _StatefulIngestor: + return _index(tmp_path_factory.mktemp("local") / "local", LOCAL_FILES) + + +@pytest.fixture(scope="module") +def negative_store(tmp_path_factory: pytest.TempPathFactory) -> _StatefulIngestor: + return _index(tmp_path_factory.mktemp("negative") / "neg", NEGATIVE_FILES) + + +# --- the issue's table ---------------------------------------------------------- + + +@pytest.mark.parametrize( + ("col", "target"), + [ + pytest.param(9, ISSUE_ADD, id="add-through-export-star"), + pytest.param(21, ISSUE_MUL, id="times-through-default-as"), + pytest.param(52, ISSUE_ADD, id="plus-through-named-alias"), + pytest.param(65, ISSUE_MUL, id="mul-direct-default-import"), + ], +) +def test_each_call_through_the_barrel_is_an_exact_call_to_the_definition( + issue_store: _StatefulIngestor, col: int, target: str +) -> None: + calls = _calls(issue_store, "barrel.src.app.main.run") + assert calls.get(col) == (target, EXACT), calls + + +def test_callers_of_the_default_export_include_its_barrel_alias( + issue_store: _StatefulIngestor, +) -> None: + # `cgr graph callers …mul.mul` listed only the direct import (col 65). + calls = _calls(issue_store, "barrel.src.app.main.run") + assert sorted(col for col, (target, _) in calls.items() if target == ISSUE_MUL) == [ + 21, + 65, + ] + + +def test_the_barrel_maps_record_what_each_export_names(tmp_path: Path) -> None: + # `export * from` never reached the import map: the parser compared the + # child with Java's named `asterisk` node, while the JS/TS grammars spell + # `*` as an anonymous token. A module's own bindings exported under + # another name were recorded nowhere. + updater, _store = _run(tmp_path / "maps", {**ISSUE_FILES, **LOCAL_FILES}) + imports = updater.factory.import_processor.import_mapping + assert imports["maps.src.lib.math.index"] == { + "*maps.src.lib.math.add": "maps.src.lib.math.add", + "times": "maps.src.lib.math.mul.default", + } + bindings = updater.factory.import_processor.js_export_bindings + assert bindings["maps.src.lib.math.mul"] == {"default": "maps.src.lib.math.mul.mul"} + assert bindings["maps.src.text.pad"] == { + "strip": "maps.src.text.pad.trim", + "default": "maps.src.text.pad.upper", + } + assert bindings["maps.src.text.index"] == { + "leftPad": "maps.src.text.index.pad", + "shout": "maps.src.text.index.upper", + } + + +# --- the issue comment: a plain named re-export --------------------------------- + + +@pytest.mark.parametrize( + ("caller", "col"), + [ + pytest.param("shop.src.app.show", 9, id="named-reexport"), + pytest.param("shop.src.alias.short", 9, id="aliased-import-of-reexport"), + ], +) +def test_a_named_reexport_binds_exactly( + comment_store: _StatefulIngestor, caller: str, col: int +) -> None: + calls = _calls(comment_store, caller) + assert calls.get(col) == (FORMAT_PRICE, EXACT), calls + + +# --- chains of barrels ----------------------------------------------------------- + + +@pytest.mark.parametrize( + ("col", "target"), + [ + pytest.param(9, CHAIN_SCALE, id="alias-of-alias-three-barrels"), + pytest.param(19, CHAIN_SHIFT, id="default-renamed-twice"), + pytest.param(29, CHAIN_SCALE, id="star-of-star"), + pytest.param(40, CHAIN_SCALE, id="alias-through-star"), + ], +) +def test_a_chain_of_barrels_reaches_the_definition( + chain_store: _StatefulIngestor, col: int, target: str +) -> None: + calls = _calls(chain_store, "chain.src.use.use") + assert calls.get(col) == (target, EXACT), calls + + +# --- exports of a module's own bindings ----------------------------------------- + + +@pytest.mark.parametrize( + ("col", "target"), + [ + pytest.param(9, LOCAL_PAD, id="imported-then-exported-as"), + pytest.param(24, LOCAL_TRIM, id="reexport-of-a-local-alias"), + pytest.param(39, LOCAL_UPPER, id="default-identifier-exported-as"), + pytest.param(52, LOCAL_TRIM, id="local-export-alias-imported-as"), + ], +) +def test_an_export_of_a_local_binding_reaches_the_definition( + local_store: _StatefulIngestor, col: int, target: str +) -> None: + calls = _calls(local_store, "local.src.main.main") + assert calls.get(col) == (target, EXACT), calls + + +# --- negative: what must not change, and what must stay unbound ------------------- + + +def test_the_namespace_reexport_call_stays_exact( + issue_store: _StatefulIngestor, +) -> None: + calls = _calls(issue_store, "barrel.src.app.main.run") + assert calls.get(35) == (ISSUE_ADD, EXACT), calls + + +@pytest.mark.parametrize( + "col", + [ + pytest.param(9, id="direct-named-import"), + pytest.param(27, id="namespace-import-member"), + ], +) +def test_direct_imports_of_the_definition_stay_exact( + comment_store: _StatefulIngestor, col: int +) -> None: + calls = _calls(comment_store, "shop.src.cart.total") + assert calls.get(col) == (FORMAT_PRICE, EXACT), calls + + +def test_a_star_barrel_does_not_import_itself( + issue_store: _StatefulIngestor, +) -> None: + # `export * from "./add"` in `math/index.ts` names `math.add`, which the + # IMPORTS flush reads as the name `add` of the `math` barrel itself. + imports = { + (str(edge[1]), str(edge[4])) + for edge in issue_store.keyed_edges + if edge[2] == cs.RelationshipType.IMPORTS.value + } + assert all(source != target for source, target in imports), imports + + +def test_two_star_sources_exporting_one_name_bind_neither_exactly( + negative_store: _StatefulIngestor, +) -> None: + # TypeScript exports neither of two conflicting star names; picking one + # would be a guess, so the call is not an exact edge to either. + calls = _calls(negative_store, "neg.src.neg.neg") + assert calls.get(9, (None, None))[1] != EXACT, calls + + +def test_a_name_only_one_star_source_exports_still_binds( + negative_store: _StatefulIngestor, +) -> None: + calls = _calls(negative_store, "neg.src.neg.neg") + assert calls.get(17) == ("neg.src.a.onlyA", EXACT), calls + + +def test_a_star_reexport_does_not_carry_the_default( + negative_store: _StatefulIngestor, +) -> None: + # `export *` never re-exports `default`, so the barrel has no default and + # `b`'s default export is not what `starDefault` names. + calls = _calls(negative_store, "neg.src.neg.neg") + assert 27 not in calls, calls + assert "neg.src.b.bdef" not in {target for target, _ in calls.values()}, calls + + +def test_an_explicit_reexport_outranks_a_star( + negative_store: _StatefulIngestor, +) -> None: + calls = _calls(negative_store, "neg.src.neg.neg") + assert calls.get(43) == ("neg.src.c.other", EXACT), calls + + +def test_a_cycle_of_stars_ends_without_an_edge( + negative_store: _StatefulIngestor, +) -> None: + calls = _calls(negative_store, "neg.src.neg.neg") + assert 54 not in calls, calls + + +# --- cgr rename through a barrel (the issue comment) ----------------------------- + + +def _rename_graph(root: Path, files: dict[str, str], mock: MagicMock) -> RecordedGraph: + for rel, text in files.items(): + path = root / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + updater = create_and_run_updater(root, mock) + graph = RecordedGraph(mock, updater.project_name) + graph.root_path = str(root.resolve()) + return graph + + +def _read(root: Path, rel: str) -> str: + return (root / rel).read_text(encoding="utf-8") + + +def test_a_rename_through_a_named_reexport_needs_no_allow_heuristic( + temp_repo: Path, mock_ingestor: MagicMock +) -> None: + # The call through `./lib` was `heuristic`, so the rename refused the + # most common barrel layout unless the caller accepted guesses. + graph = _rename_graph(temp_repo, COMMENT_FILES, mock_ingestor) + report = rename( + temp_repo, + graph.fetch_all, + graph.project, + f"{graph.project}.src.lib.util.formatPrice", + "formatMoney", + ) + assert report.applied, report.message + assert _read(temp_repo, "src/lib/util.ts").startswith( + "export function formatMoney(cents: number)" + ) + assert _read(temp_repo, "src/lib/index.ts") == ( + 'export { formatMoney } from "./util";\n' + ) + assert _read(temp_repo, "src/app.ts") == ( + 'import { formatMoney } from "./lib";\n\n' + "export function show(): string {\n return formatMoney(7);\n}\n" + ) + alias = _read(temp_repo, "src/alias.ts") + assert alias.startswith('import { formatMoney as fp } from "./lib";\n') + assert "return fp(1);" in alias + assert "return formatMoney(10) + u.formatMoney(2);" in _read( + temp_repo, "src/cart.ts" + ) + + +def test_a_default_import_call_keeps_the_importers_own_name( + temp_repo: Path, mock_ingestor: MagicMock +) -> None: + # `import mul from` binds a name the importer chose. Renaming the function + # leaves that import and its `mul(8, 9)` alone; rewriting only the call + # would leave it naming nothing. + graph = _rename_graph(temp_repo, ISSUE_FILES, mock_ingestor) + before = _read(temp_repo, "src/app/main.ts") + report = rename( + temp_repo, + graph.fetch_all, + graph.project, + f"{graph.project}.src.lib.math.mul.mul", + "product", + ) + assert report.applied, report.message + assert _read(temp_repo, "src/lib/math/mul.ts").startswith( + "export default function product(a: number, b: number)" + ) + assert _read(temp_repo, "src/app/main.ts") == before + + +def test_a_call_through_a_star_barrel_still_refuses_without_leave( + temp_repo: Path, mock_ingestor: MagicMock +) -> None: + # `import { add } from "../lib/math"` binds `add` through `export *`, an + # import the rename cannot rewrite: renaming the call alone would leave + # the import naming a function the barrel no longer exports. + graph = _rename_graph(temp_repo, ISSUE_FILES, mock_ingestor) + before = {rel: _read(temp_repo, rel) for rel in ISSUE_FILES} + with pytest.raises(RenameRefused) as refused: + rename( + temp_repo, + graph.fetch_all, + graph.project, + f"{graph.project}.src.lib.math.add.add", + "sum", + ) + assert [(s.path, s.line, s.col, s.resolution) for s in refused.value.ambiguous] == [ + ("src/app/main.ts", 6, 9, cs.EdgeResolution.HEURISTIC.value) + ] + assert {rel: _read(temp_repo, rel) for rel in ISSUE_FILES} == before + + +def test_a_default_reexported_under_its_own_name_keeps_its_importers( + temp_repo: Path, mock_ingestor: MagicMock +) -> None: + # `export { default as Widget } from` names the barrel's export as the + # barrel chose; a consumer of `Widget` binds that name, not the class's, + # so renaming the class leaves every importer as it is. + graph = _rename_graph(temp_repo, WIDGET_FILES, mock_ingestor) + before = {rel: _read(temp_repo, rel) for rel in WIDGET_FILES} + report = rename( + temp_repo, + graph.fetch_all, + graph.project, + f"{graph.project}.src.widget.Widget", + "Gadget", + ) + assert report.applied, report.message + assert _read(temp_repo, "src/widget.ts").startswith("export default class Gadget {") + for rel in ("src/kit/index.ts", "src/app.ts", "src/app2.ts"): + assert _read(temp_repo, rel) == before[rel], rel + + +def test_a_require_binding_keeps_the_importers_own_name( + temp_repo: Path, mock_ingestor: MagicMock +) -> None: + # `const mul = require("./mul")` names the binding as the importer chose, + # as a default import does; the call was rewritten and the binding left. + files = { + "src/mul.js": "module.exports = function mul(a, b) {\n return a * b;\n};\n", + "src/app.js": ( + 'const mul = require("./mul");\n\n' + "function run() {\n return mul(2, 3);\n}\n\n" + "module.exports = { run };\n" + ), + } + graph = _rename_graph(temp_repo, files, mock_ingestor) + report = rename( + temp_repo, + graph.fetch_all, + graph.project, + f"{graph.project}.src.mul.mul", + "product", + ) + assert report.applied, report.message + assert "function product(a, b)" in _read(temp_repo, "src/mul.js") + assert _read(temp_repo, "src/app.js") == files["src/app.js"] diff --git a/docs/architecture/rename.md b/docs/architecture/rename.md index 267e2253ce..09c59cce1c 100644 --- a/docs/architecture/rename.md +++ b/docs/architecture/rename.md @@ -38,6 +38,15 @@ becomes `a.b.assist(x)` and the receiver is untouched. Every edit is a span-preserving [patcher](patchers.md) edit, so formatting elsewhere in the file is not disturbed. +A bare JS/TS call or reference in another file follows the rename only when +that file imports the name by the definition's own name, so the import is +rewritten with it. A file that binds a name of its own keeps both its import +and its uses: `import mul from "./mul"`, `const mul = require("./mul")`, and a +barrel's `export { default as mul } from "./mul"` together with the modules +importing that `mul`. A use reached through an import the rename cannot +rewrite, such as a barrel's `export * from`, is reported as `heuristic` and +refuses like any guessed site (issue #2464). + ## Refusal The graph tags each call edge with how it was resolved (issue #1526). A From 01ebc40fd4295658a75727574f210efe96689dc5 Mon Sep 17 00:00:00 2001 From: Vitali Avagyan Date: Sat, 3 Oct 2026 07:15:11 +0000 Subject: [PATCH 2/5] fix(parsers): follow only a barrel's exports and import its star sources (#2464) --- codebase_rag/constants/cli.py | 9 ++ codebase_rag/editing/rename.py | 58 ++++++++-- codebase_rag/parsers/import_processor.py | 93 ++++++++++------ codebase_rag/parsers/js_ts/reexports.py | 70 +++++++----- .../tests/test_ts_barrel_reexport_calls.py | 104 ++++++++++++++---- docs/architecture/rename.md | 8 +- 6 files changed, 250 insertions(+), 92 deletions(-) diff --git a/codebase_rag/constants/cli.py b/codebase_rag/constants/cli.py index 8a7cc69bf7..42e5272cc5 100644 --- a/codebase_rag/constants/cli.py +++ b/codebase_rag/constants/cli.py @@ -704,6 +704,15 @@ class DiffMarker: "Rename failed its postcondition ({reasons}) and was not rolled back " "({error}); renamed files may remain modified; check the working tree" ) +# A JS/TS use bound through a barrel's `export *`: the rename cannot reach +# the importer's statement through the star, so no leave makes it safe +# (issue #2464). +RENAME_STAR_REEXPORT = ( + "Cannot rename {qn}: {count} site(s) import it through an `export *` in " + "{barrels}, which the rename cannot follow; their imports would keep naming " + "what the barrel no longer exports" +) +RENAME_SITE_STAR_REEXPORT = "through export *" RENAME_ROLLBACK_UNKNOWN = ( "Rename kept: its postcondition failed ({reasons}) and its transaction is " "no longer in the edit history, so whether it was already reversed cannot " diff --git a/codebase_rag/editing/rename.py b/codebase_rag/editing/rename.py index 4a2b39c272..63877fb2d9 100644 --- a/codebase_rag/editing/rename.py +++ b/codebase_rag/editing/rename.py @@ -461,12 +461,15 @@ def _js_import_bound( definition's name either way (issue #2464). A bare use in another file follows the rename only when its file imports the name by the definition's own name, so the import is rewritten with it. One whose - file binds a name of its own is left as it is, and one bound through - an import the rename cannot rewrite is a guess: rewriting the use - alone would leave the import naming what no longer exists. + file binds a name of its own is left as it is. One whose import goes + through an `export *` refuses outright: the rename cannot rewrite that + import, so rewriting the use would leave it naming what the barrel no + longer exports, whatever leave the caller gives. Any other is a guess. """ - bindings: tuple[set[str], set[str]] | None = None + bindings: tuple[set[str], set[str], dict[str, str]] | None = None kept: list[RenameSite] = [] + starred: list[RenameSite] = [] + barrels: set[str] = set() for site in sites: if ( site.kind not in ("call", "reference") @@ -479,11 +482,22 @@ def _js_import_bound( continue if bindings is None: bindings = self._js_name_bindings(qn, old_name) - by_name, aliased = bindings + by_name, aliased, star_bound = bindings if site.path in by_name: kept.append(site) + elif (barrel := star_bound.get(site.path)) is not None: + starred.append(site._replace(resolution=cs.RENAME_SITE_STAR_REEXPORT)) + barrels.add(barrel) elif site.path not in aliased: kept.append(site._replace(resolution=cs.EdgeResolution.HEURISTIC.value)) + if starred: + raise RenameRefused( + cs.RENAME_STAR_REEXPORT.format( + qn=qn, count=len(starred), barrels=", ".join(sorted(barrels)) + ), + starred, + [], + ) return kept @staticmethod @@ -497,9 +511,12 @@ def _is_member_token(patcher: Patcher, site: RenameSite) -> bool: return False return source[:offset].rstrip().endswith(b".") - def _js_name_bindings(self, qn: str, name: str) -> tuple[set[str], set[str]]: + def _js_name_bindings( + self, qn: str, name: str + ) -> tuple[set[str], set[str], dict[str, str]]: """(files importing `name` by the definition's own name, files whose - `name` is a binding of their own for the definition).""" + `name` is a binding of their own for the definition, files importing + it through an `export *` with that barrel's path).""" module_qn, _path = self._module_of(qn) by_name = { site.path @@ -528,7 +545,32 @@ def _js_name_bindings(self, qn: str, name: str) -> tuple[set[str], set[str]]: if row["module"] not in seen: seen.add(row["module"]) pending.append((row["module"], True)) - return by_name, aliased + return by_name, aliased, self._js_star_bound(module_qn, name) + + def _js_star_bound(self, module_qn: str, name: str) -> dict[str, str]: + """Files importing `name` from a barrel that passes it on through + `export *` (from the module, or from a barrel that does), each with + the path of the barrel it imports from.""" + bound: dict[str, str] = {} + pending = [(module_qn, "")] + seen = {module_qn} + while pending: + source, barrel = pending.pop() + for row in graph_query.importers(self.fetch_all, self.project, source): + path = row["path"] + if not path: + continue + # `export * from` records `*` and binds no alias. + if ( + row["imported_name"] == cs.IMPORTED_NAME_WILDCARD + and row["alias"] is None + ): + if row["module"] not in seen: + seen.add(row["module"]) + pending.append((row["module"], path)) + elif barrel and row["alias"] == name and row["imported_name"] == name: + bound.setdefault(path, barrel) + return bound def _definition_site( self, diff --git a/codebase_rag/parsers/import_processor.py b/codebase_rag/parsers/import_processor.py index b819df4389..de2b0960fd 100644 --- a/codebase_rag/parsers/import_processor.py +++ b/codebase_rag/parsers/import_processor.py @@ -51,7 +51,7 @@ discover_js_workspace_packages, resolve_js_workspace_import, ) -from .js_ts.reexports import follow_js_reexports +from .js_ts.reexports import JsExport, follow_js_reexports from .lua import utils as lua_utils from .python_source_roots import discover_python_source_roots, resolve_via_source_roots from .rs import utils as rs_utils @@ -408,6 +408,16 @@ def _js_default_export_local(statement: Node) -> str | None: return None +def _is_js_star_reexport(site: PropertyDict | None) -> bool: + # `export * from` records `*` as its imported name and binds no alias, + # where `import * as ns` binds one. + return ( + site is not None + and site.get(cs.KEY_IMPORTED_NAME) == cs.IMPORTED_NAME_WILDCARD + and cs.KEY_ALIAS not in site + ) + + def _load_jsonc(path: Path) -> dict | None: # tsconfig.json is JSONC (comments, trailing commas). Try strict JSON first, # then fall back to stripping comments/trailing commas. The naive strip can @@ -927,13 +937,13 @@ def __init__( # writes it onto the Module node after Pass 3, and matches an added # file against it to find the modules that waited (issue #1568). self.unresolved_references: dict[str, set[str]] = {} - # JS/TS module qn -> {exported name: local qn} for the module's own - # bindings it exports under another name (`export { trim as strip }`, - # `export default upper`). An importer's map holds `.strip`, - # which names nothing registered; the call resolver follows it here - # to the definition (issue #2464). Re-exports with a `from` live in - # import_mapping instead. - self.js_export_bindings: dict[str, dict[str, str]] = {} + # JS/TS module qn -> {exported name: what it names}, from every + # `export { ... }` (with or without a `from`) and `export default`. + # An importer's map holds `.strip`, which names nothing + # registered; the call resolver follows it through this table, and + # only through it, since a module's own imports are not its exports + # (issue #2464). + self.js_export_bindings: dict[str, dict[str, JsExport]] = {} # Per scope qn: the site props of each bound import name (#1522), # attached to the IMPORTS edge when the deferred edge flushes. self._import_sites: dict[str, dict[str, PropertyDict]] = {} @@ -1951,10 +1961,22 @@ def _flush_resolved_import( self.note_unresolved(entry.module_qn, entry.full_name) return 0 module_path = verified + if entry.language in cs.JS_TS_LANGUAGES and _is_js_star_reexport(entry.site): + # `export * from "./add"` in `math/index.ts` stores the module + # `math.add`, which the resolution above reads as the name `add` + # of the `math` barrel: the barrel itself (issue #2464). + module_path = ( + self._verify_internal_import_target( + entry.full_name, + known_module_paths, + module_aliases, + entry.language, + siblings, + ) + or module_path + ) if module_path == entry.module_qn and entry.language in cs.JS_TS_LANGUAGES: - # `export * from "./add"` in `math/index.ts` names `math.add`, - # which reads as the name `add` of the `math` barrel: the barrel - # itself. A module importing itself is no edge (issue #2464). + # A module importing itself is no edge. return 0 self._emit_import_edge(entry, target_label, module_path) logger.debug( @@ -4056,36 +4078,45 @@ def follow_js_reexports( ) def _record_js_export_bindings(self, statement: Node, module_qn: str) -> None: - """Record the local bindings a module exports under another name. + """Record what each name an `export` statement publishes stands for. `export { trim as strip }` and `export default upper` publish a local - binding as `strip` and `default`, names no definition is registered - under, so a call through them reached nothing (issue #2464). A - statement with a `from` names another module's binding, which - `_parse_js_reexport` maps. + binding under a name no definition is registered under, and + `export { add as plus } from "./add"` another module's export + (issue #2464). Recording only exports keeps a barrel's private import + of `foo` from standing in for the `foo` it exports through + `export *`. """ - if statement.child_by_field_name(cs.FIELD_SOURCE) is not None: - return - exported: dict[str, str] = {} - if any(child.type == cs.TS_EXPORT_DEFAULT for child in statement.children): + reexport = statement.child_by_field_name(cs.FIELD_SOURCE) is not None + exported: dict[str, JsExport] = {} + if not reexport and any( + child.type == cs.TS_EXPORT_DEFAULT for child in statement.children + ): if local := _js_default_export_local(statement): - exported[cs.TS_EXPORT_DEFAULT] = local + exported[cs.TS_EXPORT_DEFAULT] = JsExport( + f"{module_qn}{cs.SEPARATOR_DOT}{local}", local=True + ) + # `_parse_js_reexport` has just mapped this statement's re-exports. + mapped = self.import_mapping.get(module_qn, {}) for clause in statement.named_children: if clause.type != cs.TS_EXPORT_CLAUSE: continue for specifier in clause.named_children: local = safe_decode_text(specifier.child_by_field_name(cs.FIELD_NAME)) - alias = safe_decode_text(specifier.child_by_field_name(cs.FIELD_ALIAS)) - # An unaliased `export { pad }` already resolves as `.pad`. - if local and alias and alias != local: - exported[alias] = local + name = ( + safe_decode_text(specifier.child_by_field_name(cs.FIELD_ALIAS)) + or local + ) + if not local or not name: + continue + if not reexport: + exported[name] = JsExport( + f"{module_qn}{cs.SEPARATOR_DOT}{local}", local=True + ) + elif (target := mapped.get(name)) is not None: + exported[name] = JsExport(target, local=False) if exported: - self.js_export_bindings.setdefault(module_qn, {}).update( - { - name: f"{module_qn}{cs.SEPARATOR_DOT}{local}" - for name, local in exported.items() - } - ) + self.js_export_bindings.setdefault(module_qn, {}).update(exported) def _parse_java_imports(self, captures: dict, module_qn: str) -> None: for import_node in captures.get(cs.CAPTURE_IMPORT, []): diff --git a/codebase_rag/parsers/js_ts/reexports.py b/codebase_rag/parsers/js_ts/reexports.py index e949ca8712..2c00485fd9 100644 --- a/codebase_rag/parsers/js_ts/reexports.py +++ b/codebase_rag/parsers/js_ts/reexports.py @@ -3,36 +3,46 @@ A library exposes its API through barrels (`lib/index.ts`): `export * from`, `export { add as plus } from`, `export { default as times } from`, or an import the barrel exports again. An importer's map names the barrel -(`lib.plus`), where nothing is registered; what the barrel binds that name to -is in its own import map (the re-exports with a `from`) and its export -bindings (its own bindings exported under another name). +(`lib.plus`), where nothing is registered; what the barrel exports under that +name is in its export table and its `export *` sources. Only exports are +followed: a barrel may import `foo` for its own use and export another `foo`. """ from __future__ import annotations from collections.abc import Mapping from pathlib import Path +from typing import NamedTuple from ... import constants as cs from ...types_defs import FunctionRegistryTrieProtocol +class JsExport(NamedTuple): + """What one name a JS/TS module exports stands for.""" + + # `.` for the module's own binding (`local`), or + # `.` for another module's export it re-exports. + target: str + local: bool + + def follow_js_reexports( qn: str, import_mapping: Mapping[str, Mapping[str, str]], - export_bindings: Mapping[str, Mapping[str, str]], + exports: Mapping[str, Mapping[str, JsExport]], module_paths: Mapping[str, Path], function_registry: FunctionRegistryTrieProtocol, ) -> str: """Where the import of `qn` (`.`) leads through barrels. The registered definition the chain of barrels reaches, or the last name - it reached when it ends anywhere else: a name no barrel binds, two star + it reached when it ends anywhere else: a name no barrel exports, two star sources exporting it, a cycle, or `.default` for a default export known by no name. """ return _ReexportWalk( - import_mapping, export_bindings, module_paths, function_registry + import_mapping, exports, module_paths, function_registry ).follow(qn, set()) @@ -42,42 +52,48 @@ class _ReexportWalk: def __init__( self, import_mapping: Mapping[str, Mapping[str, str]], - export_bindings: Mapping[str, Mapping[str, str]], + exports: Mapping[str, Mapping[str, JsExport]], module_paths: Mapping[str, Path], function_registry: FunctionRegistryTrieProtocol, ) -> None: self._imports = import_mapping - self._exports = export_bindings + self._exports = exports self._modules = module_paths self._registry = function_registry - def follow(self, qn: str, seen: set[str]) -> str: - # `seen` is shared with the star branches, so a cycle of barrels ends. - current = qn - while current not in self._registry and current not in seen: - seen.add(current) - following = self._hop(current, seen) - if following is None: + def follow(self, qn: str, seen: set[tuple[str, bool]]) -> str: + # `qn` names a module's export, then possibly one of its own bindings + # (`local`). `seen` is shared with the star branches, so a cycle of + # barrels ends. + current, local = qn, False + while current not in self._registry and (current, local) not in seen: + seen.add((current, local)) + step = self._binding(current) if local else self._export(current, seen) + if step is None: break - current = following + current, local = step return current - def _hop(self, qn: str, seen: set[str]) -> str | None: + def _export(self, qn: str, seen: set[tuple[str, bool]]) -> tuple[str, bool] | None: module_qn, separator, name = qn.rpartition(cs.SEPARATOR_DOT) barrel = self._barrel(module_qn) if separator else None if barrel is None: return None - # The barrel's own `export { local as name }` comes first: a module - # may import a name and export another binding under it. - if (local := self._exports.get(barrel, {}).get(name)) is not None: - return local - bindings = self._imports.get(barrel, {}) - if (bound := bindings.get(name)) is not None: - return bound + if (export := self._exports.get(barrel, {}).get(name)) is not None: + return export.target, export.local # `export *` passes on every name a source exports except `default`. if name == cs.TS_EXPORT_DEFAULT: return None - return self._star_source(bindings, name, seen) + if (star := self._star_source(barrel, name, seen)) is None: + return None + return star, False + + def _binding(self, qn: str) -> tuple[str, bool] | None: + # A binding the module exports but does not define is an import of + # it: `import { pad } from "./pad"; export { pad as leftPad }`. + module_qn, _separator, name = qn.rpartition(cs.SEPARATOR_DOT) + imported = self._imports.get(module_qn, {}).get(name) + return (imported, False) if imported is not None else None def _barrel(self, module_qn: str) -> str | None: # A specifier names `lib.ts` when it exists, else `lib/index.ts`. @@ -92,7 +108,7 @@ def _barrel(self, module_qn: str) -> str | None: return None def _star_source( - self, bindings: Mapping[str, str], name: str, seen: set[str] + self, barrel: str, name: str, seen: set[tuple[str, bool]] ) -> str | None: # Each `export * from` source may export the name, itself possibly # through further barrels. Two that reach different definitions make @@ -100,7 +116,7 @@ def _star_source( # guessed. reached = { target - for key, source in bindings.items() + for key, source in self._imports.get(barrel, {}).items() if key.startswith(cs.IMPORTED_NAME_WILDCARD) and (target := self.follow(f"{source}{cs.SEPARATOR_DOT}{name}", seen)) in self._registry diff --git a/codebase_rag/tests/test_ts_barrel_reexport_calls.py b/codebase_rag/tests/test_ts_barrel_reexport_calls.py index 16977aeeb8..01697b7327 100644 --- a/codebase_rag/tests/test_ts_barrel_reexport_calls.py +++ b/codebase_rag/tests/test_ts_barrel_reexport_calls.py @@ -135,7 +135,8 @@ LOCAL_UPPER = "local.src.text.pad.upper" # What must stay unbound: two star sources exporting one name, a default a -# star does not carry, and a cycle of stars. +# star does not carry, and a cycle of stars. `private/index.ts` imports `dup` +# for its own use and exports another `dup` through `export *`. NEGATIVE_FILES = { "src/a.ts": ( "export function dup(): number {\n return 1;\n}\n\n" @@ -152,13 +153,19 @@ ), "src/loop1.ts": 'export * from "./loop2";\n', "src/loop2.ts": 'export * from "./loop1";\n', + "src/d.ts": "export function dup(): number {\n return 6;\n}\n", + "src/private/index.ts": ( + 'import { dup } from "../a";\n\nexport * from "../d";\n\n' + "export function viaA(): number {\n return dup();\n}\n" + ), "src/neg.ts": ( 'import { dup, onlyA } from "./stars";\n' 'import starDefault from "./stars";\n' 'import { dup as picked } from "./pick";\n' - 'import { ghost } from "./loop1";\n\n' + 'import { ghost } from "./loop1";\n' + 'import { dup as hidden } from "./private";\n\n' "export function neg(): number {\n" - " return dup() + onlyA() + starDefault() + picked() + ghost();\n}\n" + " return dup() + onlyA() + starDefault() + picked() + ghost() + hidden();\n}\n" ), } @@ -284,15 +291,27 @@ def test_the_barrel_maps_record_what_each_export_names(tmp_path: Path) -> None: "*maps.src.lib.math.add": "maps.src.lib.math.add", "times": "maps.src.lib.math.mul.default", } - bindings = updater.factory.import_processor.js_export_bindings - assert bindings["maps.src.lib.math.mul"] == {"default": "maps.src.lib.math.mul.mul"} - assert bindings["maps.src.text.pad"] == { - "strip": "maps.src.text.pad.trim", - "default": "maps.src.text.pad.upper", + # Only what a module exports may be followed through it: a re-export + # names another module's export, a local export (True) one of its own + # bindings. + exports = updater.factory.import_processor.js_export_bindings + assert exports["maps.src.lib.math.index"] == { + "times": ("maps.src.lib.math.mul.default", False) + } + assert exports["maps.src.lib.index"] == { + "plus": ("maps.src.lib.math.add.add", False) + } + assert exports["maps.src.lib.math.mul"] == { + "default": ("maps.src.lib.math.mul.mul", True) + } + assert exports["maps.src.text.pad"] == { + "strip": ("maps.src.text.pad.trim", True), + "default": ("maps.src.text.pad.upper", True), } - assert bindings["maps.src.text.index"] == { - "leftPad": "maps.src.text.index.pad", - "shout": "maps.src.text.index.upper", + assert exports["maps.src.text.index"] == { + "leftPad": ("maps.src.text.index.pad", True), + "strip": ("maps.src.text.index.strip", True), + "shout": ("maps.src.text.index.upper", True), } @@ -375,17 +394,36 @@ def test_direct_imports_of_the_definition_stay_exact( assert calls.get(col) == (FORMAT_PRICE, EXACT), calls -def test_a_star_barrel_does_not_import_itself( - issue_store: _StatefulIngestor, -) -> None: - # `export * from "./add"` in `math/index.ts` names `math.add`, which the - # IMPORTS flush reads as the name `add` of the `math` barrel itself. - imports = { +def _imports(store: _StatefulIngestor) -> set[tuple[str, str]]: + return { (str(edge[1]), str(edge[4])) - for edge in issue_store.keyed_edges + for edge in store.keyed_edges if edge[2] == cs.RelationshipType.IMPORTS.value } - assert all(source != target for source, target in imports), imports + + +def test_a_star_barrel_imports_the_module_it_reexports( + issue_store: _StatefulIngestor, chain_store: _StatefulIngestor +) -> None: + # `export * from "./add"` in `math/index.ts` names the module `math.add`, + # which the IMPORTS flush read as the name `add` of the `math` barrel: + # the barrel itself, so the edge to `add.ts` never existed. + assert ("barrel.src.lib.math.index", "barrel.src.lib.math.add") in _imports( + issue_store + ) + # A star of a directory barrel, and one into a sibling file. + assert { + ("chain.src.api.index", "chain.src.core.index"), + ("chain.src.core.index", "chain.src.core.scale"), + } <= _imports(chain_store) + + +def test_a_star_barrel_does_not_import_itself( + issue_store: _StatefulIngestor, chain_store: _StatefulIngestor +) -> None: + for store in (issue_store, chain_store): + imports = _imports(store) + assert all(source != target for source, target in imports), imports def test_two_star_sources_exporting_one_name_bind_neither_exactly( @@ -414,6 +452,22 @@ def test_a_star_reexport_does_not_carry_the_default( assert "neg.src.b.bdef" not in {target for target, _ in calls.values()}, calls +def test_a_barrels_private_import_does_not_stand_in_for_its_export( + negative_store: _StatefulIngestor, +) -> None: + # `private/index.ts` imports `dup` from `a` for its own use and exports + # `d`'s `dup` through `export *`; the consumer's `dup` is `d`'s. + calls = _calls(negative_store, "neg.src.neg.neg") + assert calls.get(64) == ("neg.src.d.dup", EXACT), calls + + +def test_the_barrels_own_call_keeps_its_private_import( + negative_store: _StatefulIngestor, +) -> None: + calls = _calls(negative_store, "neg.src.private.index.viaA") + assert calls == {9: ("neg.src.a.dup", EXACT)}, calls + + def test_an_explicit_reexport_outranks_a_star( negative_store: _StatefulIngestor, ) -> None: @@ -500,12 +554,14 @@ def test_a_default_import_call_keeps_the_importers_own_name( assert _read(temp_repo, "src/app/main.ts") == before -def test_a_call_through_a_star_barrel_still_refuses_without_leave( - temp_repo: Path, mock_ingestor: MagicMock +@pytest.mark.parametrize("allow_heuristic", [False, True]) +def test_a_call_through_a_star_barrel_refuses_even_with_allow_heuristic( + temp_repo: Path, mock_ingestor: MagicMock, allow_heuristic: bool ) -> None: # `import { add } from "../lib/math"` binds `add` through `export *`, an # import the rename cannot rewrite: renaming the call alone would leave - # the import naming a function the barrel no longer exports. + # the import naming a function the barrel no longer exports, so no + # leave to guess makes the rename safe. graph = _rename_graph(temp_repo, ISSUE_FILES, mock_ingestor) before = {rel: _read(temp_repo, rel) for rel in ISSUE_FILES} with pytest.raises(RenameRefused) as refused: @@ -515,9 +571,11 @@ def test_a_call_through_a_star_barrel_still_refuses_without_leave( graph.project, f"{graph.project}.src.lib.math.add.add", "sum", + allow_heuristic=allow_heuristic, ) + assert "src/lib/math/index.ts" in str(refused.value) assert [(s.path, s.line, s.col, s.resolution) for s in refused.value.ambiguous] == [ - ("src/app/main.ts", 6, 9, cs.EdgeResolution.HEURISTIC.value) + ("src/app/main.ts", 6, 9, cs.RENAME_SITE_STAR_REEXPORT) ] assert {rel: _read(temp_repo, rel) for rel in ISSUE_FILES} == before diff --git a/docs/architecture/rename.md b/docs/architecture/rename.md index 09c59cce1c..02e75a8ae6 100644 --- a/docs/architecture/rename.md +++ b/docs/architecture/rename.md @@ -43,9 +43,11 @@ that file imports the name by the definition's own name, so the import is rewritten with it. A file that binds a name of its own keeps both its import and its uses: `import mul from "./mul"`, `const mul = require("./mul")`, and a barrel's `export { default as mul } from "./mul"` together with the modules -importing that `mul`. A use reached through an import the rename cannot -rewrite, such as a barrel's `export * from`, is reported as `heuristic` and -refuses like any guessed site (issue #2464). +importing that `mul`. A use whose import goes through a barrel's +`export * from` refuses even with `--allow-heuristic`, naming the barrel: the +rename cannot rewrite that import, so rewriting the use would leave it naming +what the barrel no longer exports. Any other use with no such import is +reported as `heuristic` and refuses like a guessed site (issue #2464). ## Refusal From 940aee5db96f973e86e95b112b391bb649ed2d1b Mon Sep 17 00:00:00 2001 From: Vitali Avagyan Date: Sat, 3 Oct 2026 07:33:21 +0000 Subject: [PATCH 3/5] fix(rename): refuse a JS/TS use reached through a star after named re-exports (#2464) --- codebase_rag/editing/rename.py | 29 +++++++--- .../tests/test_ts_barrel_reexport_calls.py | 55 +++++++++++++++++++ 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/codebase_rag/editing/rename.py b/codebase_rag/editing/rename.py index 63877fb2d9..668de828f3 100644 --- a/codebase_rag/editing/rename.py +++ b/codebase_rag/editing/rename.py @@ -548,12 +548,17 @@ def _js_name_bindings( return by_name, aliased, self._js_star_bound(module_qn, name) def _js_star_bound(self, module_qn: str, name: str) -> dict[str, str]: - """Files importing `name` from a barrel that passes it on through - `export *` (from the module, or from a barrel that does), each with - the path of the barrel it imports from.""" + """Files importing `name` through a chain of re-exports that passes + an `export *`, each with the path of the first such barrel. + + The chain may name the definition before the star + (`export { foo } from "../a"` in `b`, then `export * from "../b"` in + `c`): the import walk rewrites `b`, but not the consumer importing + `foo` from `c`. + """ bound: dict[str, str] = {} pending = [(module_qn, "")] - seen = {module_qn} + seen = {(module_qn, False)} while pending: source, barrel = pending.pop() for row in graph_query.importers(self.fetch_all, self.project, source): @@ -565,11 +570,17 @@ def _js_star_bound(self, module_qn: str, name: str) -> dict[str, str]: row["imported_name"] == cs.IMPORTED_NAME_WILDCARD and row["alias"] is None ): - if row["module"] not in seen: - seen.add(row["module"]) - pending.append((row["module"], path)) - elif barrel and row["alias"] == name and row["imported_name"] == name: - bound.setdefault(path, barrel) + following = barrel or path + elif row["alias"] == name and row["imported_name"] == name: + # A consumer, or a barrel passing the name on by name. + following = barrel + if barrel: + bound.setdefault(path, barrel) + else: + continue + if (row["module"], bool(following)) not in seen: + seen.add((row["module"], bool(following))) + pending.append((row["module"], following)) return bound def _definition_site( diff --git a/codebase_rag/tests/test_ts_barrel_reexport_calls.py b/codebase_rag/tests/test_ts_barrel_reexport_calls.py index 01697b7327..8d5bf374b2 100644 --- a/codebase_rag/tests/test_ts_barrel_reexport_calls.py +++ b/codebase_rag/tests/test_ts_barrel_reexport_calls.py @@ -625,3 +625,58 @@ def test_a_require_binding_keeps_the_importers_own_name( assert report.applied, report.message assert "function product(a, b)" in _read(temp_repo, "src/mul.js") assert _read(temp_repo, "src/app.js") == files["src/app.js"] + + +def _chain_files(c_barrel: str) -> dict[str, str]: + # `foo` passes from `a` through the named re-export in `b` and then + # through `c`, which either re-exports it by name or with `export *`. + return { + "src/a.ts": "export function foo(): number {\n return 1;\n}\n", + "src/b/index.ts": 'export { foo } from "../a";\n', + "src/c/index.ts": c_barrel, + "src/app.ts": ( + 'import { foo } from "./c";\n\n' + "export function run(): number {\n return foo();\n}\n" + ), + } + + +def test_a_star_after_a_named_reexport_refuses_even_with_allow_heuristic( + temp_repo: Path, mock_ingestor: MagicMock +) -> None: + # The star is one hop past the named re-export `b`: the rename rewrites + # `b`'s `export { foo }` and the call, but `app.ts` imports `foo` from + # `c`, through `export *`, and that import is out of its reach. + files = _chain_files('export * from "../b";\n') + graph = _rename_graph(temp_repo, files, mock_ingestor) + with pytest.raises(RenameRefused) as refused: + rename( + temp_repo, + graph.fetch_all, + graph.project, + f"{graph.project}.src.a.foo", + "bar", + allow_heuristic=True, + ) + assert "src/c/index.ts" in str(refused.value) + sites = [(s.path, s.resolution) for s in refused.value.ambiguous] + assert sites == [("src/app.ts", cs.RENAME_SITE_STAR_REEXPORT)] + assert {rel: _read(temp_repo, rel) for rel in files} == files + + +def test_a_chain_of_named_reexports_renames_without_leave( + temp_repo: Path, mock_ingestor: MagicMock +) -> None: + # With no star on the way, every hop names `foo`, so each re-export and + # the consumer's import are rewritten with the definition. + files = _chain_files('export { foo } from "../b";\n') + graph = _rename_graph(temp_repo, files, mock_ingestor) + report = rename( + temp_repo, graph.fetch_all, graph.project, f"{graph.project}.src.a.foo", "bar" + ) + assert report.applied, report.message + assert _read(temp_repo, "src/b/index.ts") == 'export { bar } from "../a";\n' + assert _read(temp_repo, "src/c/index.ts") == 'export { bar } from "../b";\n' + app = _read(temp_repo, "src/app.ts") + assert app.startswith('import { bar } from "./c";\n') + assert "return bar();" in app From a8a6f0b3b9ce58f582d4f60941ef186cf2448a82 Mon Sep 17 00:00:00 2001 From: Vitali Avagyan Date: Sat, 3 Oct 2026 12:20:01 +0000 Subject: [PATCH 4/5] test(dart): give the reparse probe the JS export-binding slot this PR's reset clears (#2464) --- codebase_rag/tests/test_dart_getter_reads.py | 1 + 1 file changed, 1 insertion(+) diff --git a/codebase_rag/tests/test_dart_getter_reads.py b/codebase_rag/tests/test_dart_getter_reads.py index 57669c81da..7a48866e06 100644 --- a/codebase_rag/tests/test_dart_getter_reads.py +++ b/codebase_rag/tests/test_dart_getter_reads.py @@ -818,6 +818,7 @@ def _retract_import_sites(self, module_qn: str) -> None: processor._cpp_declaration_mappings = set() processor.csharp_static_imports = {} processor.csharp_global_static_imports = {} + processor.js_export_bindings = {} processor._clear_module_import_state("m") From 269f1a6c541958f1b1e31258f88b87e790d4ae0c Mon Sep 17 00:00:00 2001 From: Vitali Avagyan Date: Sat, 3 Oct 2026 19:45:17 +0000 Subject: [PATCH 5/5] refactor(rename,imports): extract the star-walk step and export-specifier loop to clear Sonar S3776 (#2464) --- codebase_rag/editing/rename.py | 42 ++++--- codebase_rag/parsers/import_processor.py | 37 +++--- .../tests/test_ts_barrel_reexport_calls.py | 110 +++++++++++++++++- 3 files changed, 155 insertions(+), 34 deletions(-) diff --git a/codebase_rag/editing/rename.py b/codebase_rag/editing/rename.py index 668de828f3..23cc5082bf 100644 --- a/codebase_rag/editing/rename.py +++ b/codebase_rag/editing/rename.py @@ -562,27 +562,35 @@ def _js_star_bound(self, module_qn: str, name: str) -> dict[str, str]: while pending: source, barrel = pending.pop() for row in graph_query.importers(self.fetch_all, self.project, source): - path = row["path"] - if not path: + following = self._js_star_following(row, name, barrel, bound) + if following is None: continue - # `export * from` records `*` and binds no alias. - if ( - row["imported_name"] == cs.IMPORTED_NAME_WILDCARD - and row["alias"] is None - ): - following = barrel or path - elif row["alias"] == name and row["imported_name"] == name: - # A consumer, or a barrel passing the name on by name. - following = barrel - if barrel: - bound.setdefault(path, barrel) - else: - continue - if (row["module"], bool(following)) not in seen: - seen.add((row["module"], bool(following))) + step = (row["module"], bool(following)) + if step not in seen: + seen.add(step) pending.append((row["module"], following)) return bound + @staticmethod + def _js_star_following( + row: graph_query.ImporterRow, name: str, barrel: str, bound: dict[str, str] + ) -> str | None: + """The first `export *` barrel on the chain once it passes `row`, empty + while there is none, or None when `row` does not pass `name` on. A + file binding `name` past such a barrel is recorded in `bound`.""" + path = row["path"] + if not path: + return None + # `export * from` records `*` and binds no alias. + if row["imported_name"] == cs.IMPORTED_NAME_WILDCARD and row["alias"] is None: + return barrel or path + if row["alias"] != name or row["imported_name"] != name: + return None + # A consumer, or a barrel passing the name on by name. + if barrel: + bound.setdefault(path, barrel) + return barrel + def _definition_site( self, qn: str, diff --git a/codebase_rag/parsers/import_processor.py b/codebase_rag/parsers/import_processor.py index 92014d4cf3..1250d0783c 100644 --- a/codebase_rag/parsers/import_processor.py +++ b/codebase_rag/parsers/import_processor.py @@ -408,6 +408,21 @@ def _js_default_export_local(statement: Node) -> str | None: return None +def _js_export_specifiers(statement: Node) -> Iterator[tuple[str, str]]: + # The (local, exported) names of each `export { local as exported }` + # specifier; a comment inside the braces is no specifier. + for clause in statement.named_children: + if clause.type != cs.TS_EXPORT_CLAUSE: + continue + for specifier in clause.named_children: + local = safe_decode_text(specifier.child_by_field_name(cs.FIELD_NAME)) + name = ( + safe_decode_text(specifier.child_by_field_name(cs.FIELD_ALIAS)) or local + ) + if local and name: + yield local, name + + def _is_js_star_reexport(site: PropertyDict | None) -> bool: # `export * from` records `*` as its imported name and binds no alias, # where `import * as ns` binds one. @@ -4124,23 +4139,13 @@ def _record_js_export_bindings(self, statement: Node, module_qn: str) -> None: ) # `_parse_js_reexport` has just mapped this statement's re-exports. mapped = self.import_mapping.get(module_qn, {}) - for clause in statement.named_children: - if clause.type != cs.TS_EXPORT_CLAUSE: - continue - for specifier in clause.named_children: - local = safe_decode_text(specifier.child_by_field_name(cs.FIELD_NAME)) - name = ( - safe_decode_text(specifier.child_by_field_name(cs.FIELD_ALIAS)) - or local + for local, name in _js_export_specifiers(statement): + if not reexport: + exported[name] = JsExport( + f"{module_qn}{cs.SEPARATOR_DOT}{local}", local=True ) - if not local or not name: - continue - if not reexport: - exported[name] = JsExport( - f"{module_qn}{cs.SEPARATOR_DOT}{local}", local=True - ) - elif (target := mapped.get(name)) is not None: - exported[name] = JsExport(target, local=False) + elif (target := mapped.get(name)) is not None: + exported[name] = JsExport(target, local=False) if exported: self.js_export_bindings.setdefault(module_qn, {}).update(exported) diff --git a/codebase_rag/tests/test_ts_barrel_reexport_calls.py b/codebase_rag/tests/test_ts_barrel_reexport_calls.py index 8d5bf374b2..38664dff49 100644 --- a/codebase_rag/tests/test_ts_barrel_reexport_calls.py +++ b/codebase_rag/tests/test_ts_barrel_reexport_calls.py @@ -19,7 +19,8 @@ import pytest from codebase_rag import constants as cs -from codebase_rag.editing.rename import RenameRefused, rename +from codebase_rag import graph_query +from codebase_rag.editing.rename import Renamer, RenameRefused, rename from codebase_rag.graph_updater import GraphUpdater from codebase_rag.parser_loader import load_parsers from codebase_rag.tests.conftest import create_and_run_updater @@ -315,6 +316,33 @@ def test_the_barrel_maps_record_what_each_export_names(tmp_path: Path) -> None: } +# Export statements that publish no name a binding could follow: an anonymous +# default, a default expression, and a re-export whose source names no +# module. A comment inside a clause is no specifier either. +UNNAMED_EXPORT_FILES = { + "src/anon.ts": "export default function () {\n return 1;\n}\n", + "src/expr.ts": "const base = 40;\n\nexport default base + 2;\n", + "src/blank.ts": 'export { gone } from "";\n', + "src/listed.ts": ( + "function first(): number {\n return 1;\n}\n\n" + "function last(): number {\n return 2;\n}\n\n" + "export { first, /* middle was removed */ last as final };\n" + ), +} + + +def test_an_export_naming_no_binding_records_nothing(tmp_path: Path) -> None: + updater, _store = _run(tmp_path / "unnamed", UNNAMED_EXPORT_FILES) + exports = updater.factory.import_processor.js_export_bindings + assert "unnamed.src.anon" not in exports, exports + assert "unnamed.src.expr" not in exports, exports + assert "unnamed.src.blank" not in exports, exports + assert exports["unnamed.src.listed"] == { + "first": ("unnamed.src.listed.first", True), + "final": ("unnamed.src.listed.last", True), + } + + # --- the issue comment: a plain named re-export --------------------------------- @@ -680,3 +708,83 @@ def test_a_chain_of_named_reexports_renames_without_leave( app = _read(temp_repo, "src/app.ts") assert app.startswith('import { bar } from "./c";\n') assert "return bar();" in app + + +# --- the star walk behind the refusal -------------------------------------------- + + +def _importer( + module: str, path: str | None, alias: str | None, imported_name: str | None +) -> graph_query.ImporterRow: + return graph_query.ImporterRow( + module=module, + path=path, + line=1, + col=0, + end_line=1, + end_col=1, + alias=alias, + imported_name=imported_name, + ) + + +STAR = cs.IMPORTED_NAME_WILDCARD + +# Who imports each module, and how. `p.a` defines `foo`; `b` and `c` star it +# and `d` stars both, so the walk reaches `d` twice past a star. `direct` +# imports `foo` by name before `e` stars it. A row with no path, and one +# naming another export, lead nowhere even past a star. +STAR_WALK_IMPORTERS = { + "p.a": [ + _importer("p.b", "src/b.ts", None, STAR), + _importer("p.c", "src/c.ts", None, STAR), + _importer("p.direct", "src/direct.ts", "foo", "foo"), + ], + "p.b": [ + _importer("p.d", "src/d.ts", None, STAR), + _importer("p.wrong", "src/wrong.ts", "bar", "bar"), + ], + "p.c": [ + _importer("p.d", "src/d.ts", None, STAR), + _importer("p.ghost", None, "foo", "foo"), + _importer("p.near", "src/near.ts", "foo", "foo"), + ], + "p.d": [_importer("p.app", "src/app.ts", "foo", "foo")], + "p.direct": [_importer("p.e", "src/e.ts", None, STAR)], + "p.e": [_importer("p.late", "src/late.ts", "foo", "foo")], + "p.ghost": [_importer("p.haunted", "src/haunted.ts", "foo", "foo")], + "p.wrong": [_importer("p.astray", "src/astray.ts", "foo", "foo")], +} + + +def test_the_star_walk_binds_each_consumer_to_its_first_star_barrel( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + asked: list[str] = [] + + def importers( + _fetch_all: object, _project: str, source: str + ) -> list[graph_query.ImporterRow]: + asked.append(source) + return STAR_WALK_IMPORTERS.get(source, []) + + monkeypatch.setattr(graph_query, "importers", importers) + bound = Renamer(tmp_path, MagicMock(), "p")._js_star_bound("p.a", "foo") + assert bound == { + "src/near.ts": "src/c.ts", + "src/app.ts": "src/c.ts", + "src/late.ts": "src/e.ts", + } + # Each module is walked once past a star; the pathless and the unrelated + # rows are not followed at all. + assert sorted(asked) == [ + "p.a", + "p.app", + "p.b", + "p.c", + "p.d", + "p.direct", + "p.e", + "p.late", + "p.near", + ]