diff --git a/src/cgis/resolver/indices.py b/src/cgis/resolver/indices.py index d3d5e421..483ed053 100644 --- a/src/cgis/resolver/indices.py +++ b/src/cgis/resolver/indices.py @@ -16,6 +16,8 @@ _PYTHON_SUFFIXES: tuple[str, ...] = (".py",) #: The suffixes the TypeScript extractor handles; `.js`/`.jsx` are not ingested (registry.py). _TYPESCRIPT_SUFFIXES = (".ts", ".tsx") +#: What a declaration file's module FQN ends in: `Calendar.d.ts` -> `Calendar.d` (#507). +_DECLARATION_SUFFIX = ".d" @dataclass(frozen=True) @@ -101,7 +103,7 @@ def resolve_import_target(self, fqn: str, source_file: str | None = None) -> str if fqn in self.nodes: return fqn if source_file is not None and source_file.endswith(_TYPESCRIPT_SUFFIXES): - return self._resolve_workspace_import(fqn) + return _resolve_typescript_import(fqn, self.nodes, self.workspace_packages) if source_file is not None and not source_file.endswith(_PYTHON_SUFFIXES): return None parts = fqn.split(".") @@ -113,30 +115,6 @@ def resolve_import_target(self, fqn: str, source_file: str | None = None) -> str return candidate return None - def _resolve_workspace_import(self, fqn: str) -> str | None: - """Map a TypeScript import of a workspace package onto the module it names. - - `@calcom.lib.hooks.useLocale` becomes `packages.lib.hooks.useLocale` when the - pipeline found a `package.json` named `@calcom/lib` in `packages/lib`. The - package root is tried before `src/`, a common layout for a package's sources. - - The name must match on a segment boundary, so `@x/a` never claims - `@x/a-b`. The first — longest — package that matches decides: falling back - to a shorter one would read `a.b.util` as package `a` when package `a.b` - was meant. And the rewrite happens only onto a node that exists; otherwise - None, leaving the edge visibly unresolved rather than on a plausible name. - """ - for name, directory in self.workspace_packages.items(): - if fqn != name and not fqn.startswith(name + "."): - continue - subpath = fqn[len(name) + 1 :] - for base in (directory, f"{directory}.src"): - candidate = f"{base}.{subpath}" if subpath else base - if candidate in self.nodes: - return candidate - return None - return None - def map_to_node_fqn(self, imported_fqn: str) -> str | None: """Resolve an imported FQN to an actual node in the graph. @@ -319,6 +297,51 @@ def _strips_to_a_node(imported_fqn: str, node_ids: set[str]) -> bool: return any(".".join(parts[i:]) in node_ids for i in range(1, len(parts) - 1)) +def _resolve_typescript_import( + fqn: str, nodes: Mapping[str, Node], packages: Mapping[str, str] +) -> str | None: + """Reconcile a TypeScript import against the node ids, or None to leave it alone. + + Each candidate is tried as written and then as a declaration file: + `packages/types/Calendar.d.ts` is the module `packages.types.Calendar.d`, + while code imports it with no `.d` (#507). That is the order TypeScript + resolves in, so an implementation beside its declaration wins. The node ids + are not changed instead: that would rename every declaration node in + existing graphs and give `foo.ts` and `foo.d.ts` one id. + + The candidates are the target itself — a relative import, already made + absolute by the extractor — and then, for a workspace package, the module + it names (#504). + """ + for candidate in (fqn, *_workspace_candidates(fqn, packages)): + for spelled in (candidate, f"{candidate}{_DECLARATION_SUFFIX}"): + if spelled in nodes: + return spelled + return None + + +def _workspace_candidates(fqn: str, packages: Mapping[str, str]) -> tuple[str, ...]: + """The module FQNs a TypeScript import of a workspace package may name. + + `@calcom.lib.hooks.useLocale` becomes `packages.lib.hooks.useLocale` when the + pipeline found a `package.json` named `@calcom/lib` in `packages/lib`; the + package root is tried before `src/`, a common layout for a package's sources. + + The name must match on a segment boundary, so `@x/a` never claims + `@x/a-b`. The first — longest — package that matches decides: falling back + to a shorter one would read `a.b.util` as package `a` when package `a.b` + was meant. No candidates for a name the map does not hold. + """ + for name, directory in packages.items(): + if fqn != name and not fqn.startswith(name + "."): + continue + subpath = fqn[len(name) + 1 :] + return tuple( + f"{base}.{subpath}" if subpath else base for base in (directory, f"{directory}.src") + ) + return () + + class IndexBuilder: """Builds a SymbolIndex from extracted nodes (Phase 1: indexing). diff --git a/tests/unit/test_resolver_ts_declarations.py b/tests/unit/test_resolver_ts_declarations.py new file mode 100644 index 00000000..94d7ed30 --- /dev/null +++ b/tests/unit/test_resolver_ts_declarations.py @@ -0,0 +1,90 @@ +"""A TypeScript import may name a declaration file, and must reach it (#507). + +`packages/types/Calendar.d.ts` is extracted as the module `packages.types.Calendar.d` +— the extractor strips the last extension only — while code imports it as +`@calcom/types/Calendar` or `./Calendar`, with no `.d`. After #506 mapped +`@calcom/types` to `packages.types`, the candidate `packages.types.Calendar` still +matched nothing, and 249 of cal.com's workspace imports stopped there. + +Node ids are left as they are. Stripping `.d` from them would rename every +declaration node in existing graphs and give `foo.ts` and `foo.d.ts` one id. The +resolver instead tries each candidate as written and then as a declaration, the +order TypeScript itself resolves in: an implementation beside its declaration wins. +""" + +from pathlib import Path + +from cgis.core.models import Edge, EdgeType, Node, NodeNamespace, NodeType +from cgis.extractors.typescript_extractor import TypeScriptExtractor +from cgis.pipeline import IngestionPipeline +from cgis.resolver.engine import ResolverEngine + +PACKAGES = {"@calcom.types": "packages.types"} + + +def _file(node_id: str, file_path: str) -> Node: + return Node( + id=node_id, + type=NodeType.FILE, + name=node_id.rsplit(".", 1)[-1], + file_path=file_path, + start_line=1, + end_line=1, + namespace=NodeNamespace.INTERNAL, + ) + + +def _resolved(nodes: list[Node], source: Node, target: str) -> str: + edge = Edge( + id=f"{source.id}->import:{target}", source=source.id, target=target, type=EdgeType.IMPORTS + ) + resolved, _ = ResolverEngine([*nodes, source], [edge], workspace_packages=PACKAGES).resolve() + (out,) = (e for e in resolved if e.type == EdgeType.IMPORTS) + return out.target + + +PAGE = _file("apps.web.page", "apps/web/page.tsx") + + +def test_a_workspace_import_reaches_a_declaration_module() -> None: + nodes = [_file("packages.types.Calendar.d", "packages/types/Calendar.d.ts")] + assert _resolved(nodes, PAGE, "@calcom.types.Calendar") == "packages.types.Calendar.d" + + +def test_a_relative_import_reaches_a_declaration_module() -> None: + # The extractor has already turned `./types` into `apps.web.types`. + nodes = [_file("apps.web.types.d", "apps/web/types.d.ts")] + assert _resolved(nodes, PAGE, "apps.web.types") == "apps.web.types.d" + + +def test_an_implementation_beside_its_declaration_wins() -> None: + nodes = [ + _file("packages.types.Calendar", "packages/types/Calendar.ts"), + _file("packages.types.Calendar.d", "packages/types/Calendar.d.ts"), + ] + assert _resolved(nodes, PAGE, "@calcom.types.Calendar") == "packages.types.Calendar" + + +def test_a_python_import_is_not_given_a_declaration_suffix() -> None: + seed = _file("tools.seed", "tools/seed.py") + nodes = [_file("tools.config.d", "tools/config.d.ts")] + assert _resolved(nodes, seed, "tools.config") == "tools.config" + + +def test_the_pipeline_resolves_an_import_of_a_real_declaration_file(tmp_path: Path) -> None: + """End to end, so the `.d` in the node id comes from the extractor and not from this test.""" + files = { + "packages/types/package.json": '{"name": "@calcom/types"}', + "packages/types/Calendar.d.ts": "export interface Calendar {\n id: string;\n}\n", + "apps/web/page.ts": ( + 'import type { Calendar } from "@calcom/types/Calendar";\n' + "export function f(c: Calendar) {\n return c.id;\n}\n" + ), + } + for rel, text in files.items(): + path = tmp_path / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + _, _, resolved = IngestionPipeline({".ts": TypeScriptExtractor()}).run(str(tmp_path)) + (edge,) = (e for e in resolved if e.type == EdgeType.IMPORTS and e.source == "apps.web.page") + assert edge.target == "packages.types.Calendar.d"