diff --git a/graphify/extractors/dart.py b/graphify/extractors/dart.py index acbe19583..d4ab84ecf 100644 --- a/graphify/extractors/dart.py +++ b/graphify/extractors/dart.py @@ -5,6 +5,7 @@ from pathlib import Path from graphify.extractors.base import _file_stem, _make_id +from graphify.extractors.resolution import _resolve_dart_import_target def extract_dart(path: Path) -> dict: @@ -500,17 +501,21 @@ def _find_matching_brace(text: str, start_pos: int) -> int: add_edge(nid, route_nid, "navigates", context="route_object") # 6. Imports and Exports - for m in re.finditer(r"""^\s*import\s+['"]([^'"]+)['"]""", src_clean, re.MULTILINE): - pkg = m.group(1) - tgt_nid = _make_id(pkg) - add_node(tgt_nid, pkg, source_file=None) - add_edge(file_nid, tgt_nid, "imports") - - for m in re.finditer(r"""^\s*export\s+['"]([^'"]+)['"]""", src_clean, re.MULTILINE): - pkg = m.group(1) - tgt_nid = _make_id(pkg) - add_node(tgt_nid, pkg, source_file=None) - add_edge(file_nid, tgt_nid, "exports") + for kind, pattern in (("imports", r"""^\s*import\s+['"]([^'"]+)['"]"""), + ("exports", r"""^\s*export\s+['"]([^'"]+)['"]""")): + for m in re.finditer(pattern, src_clean, re.MULTILINE): + pkg = m.group(1) + # Resolve the URI so the edge target is the id that file's own node + # carries. Without this every Dart import edge pointed at a bare string + # node with source_file=None, so reverse traversal ("who imports this + # file", `affected`) was blind on Dart while Python/TS resolved (#2329). + resolved = _resolve_dart_import_target(pkg, str(path)) + if resolved is not None: + add_edge(file_nid, _make_id(str(resolved)), kind, context="import") + else: + tgt_nid = _make_id(pkg) + add_node(tgt_nid, pkg, source_file=None) + add_edge(file_nid, tgt_nid, kind) # 7. Generic Invocations / Type Lookups (Universal Dependency Lookup) # Matches any method call with type parameters: methodName() or object.methodName() diff --git a/graphify/extractors/resolution.py b/graphify/extractors/resolution.py index b5edd264a..85f8246ae 100644 --- a/graphify/extractors/resolution.py +++ b/graphify/extractors/resolution.py @@ -565,6 +565,202 @@ def _resolve_c_include_path(raw: str, str_path: str) -> "Path | None": return candidate return None +_DART_PUBSPEC_NAME_RE = re.compile(r"""^name:\s*['"]?([A-Za-z_]\w*)['"]?\s*(?:#.*)?$""", re.MULTILINE) + +_DART_WORKSPACE_RE = re.compile(r"^workspace:\s*(?:#.*)?$", re.MULTILINE) + +_DART_PACKAGE_INDEX_CACHE: "dict[str, tuple[tuple[str, str], ...]]" = {} + +_DART_MAX_PARENTS = 40 + + +def _dart_pubspec_name(pubspec: Path) -> "str | None": + """Read the `name:` of a pubspec.yaml without a YAML dependency.""" + try: + text = pubspec.read_text(encoding="utf-8", errors="replace") + except OSError: + return None + m = _DART_PUBSPEC_NAME_RE.search(text) + return m.group(1) if m else None + + +def _dart_workspace_members(pubspec: Path) -> "list[Path]": + """Members listed under a pub-workspace `workspace:` block (#pub workspaces). + + Only the top-level block is read; the entries are directory paths relative to + the pubspec's own directory: + + workspace: + - apps/client_app + - packages/pricing_catalog + """ + try: + text = pubspec.read_text(encoding="utf-8", errors="replace") + except OSError: + return [] + m = _DART_WORKSPACE_RE.search(text) + if m is None: + return [] + members: list[Path] = [] + for line in text[m.end():].splitlines(): + if not line.strip(): + continue + entry = re.match(r"^\s+-\s+(?:['\"])?([^'\"#\s]+)(?:['\"])?\s*(?:#.*)?$", line) + if entry is None: + break # block ended (a new top-level key, or a non-list line) + members.append(pubspec.parent / entry.group(1)) + return members + + +def _dart_package_index(start_dir: str) -> "tuple[tuple[str, str], ...]": + """Map every reachable Dart package name to its package root directory. + + Walks up from `start_dir` recording each `pubspec.yaml` it passes, and at each + one also records the members of a pub `workspace:` block, so an import of a + sibling package in a monorepo resolves as well as the file's own package. + + Cached per starting directory; the result is a tuple of (name, root) pairs so + it stays immutable across callers. + """ + cached = _DART_PACKAGE_INDEX_CACHE.get(start_dir) + if cached is not None: + return cached + + found: dict[str, str] = {} + try: + here = Path(start_dir).resolve() + except OSError: + here = Path(start_dir) + for parent in [here, *here.parents][:_DART_MAX_PARENTS]: + pubspec = parent / "pubspec.yaml" + if not pubspec.is_file(): + continue + name = _dart_pubspec_name(pubspec) + if name: + found.setdefault(name, str(parent)) + for member in _dart_workspace_members(pubspec): + member_spec = member / "pubspec.yaml" + if not member_spec.is_file(): + continue + member_name = _dart_pubspec_name(member_spec) + if member_name: + found.setdefault(member_name, str(member)) + + result = tuple(sorted(found.items())) + _DART_PACKAGE_INDEX_CACHE[start_dir] = result + return result + + +def _dart_lib_root(start_dir: Path) -> "Path | None": + """The `lib/` of the package owning `start_dir`, if it is inside one. + + A directory only counts when its parent holds a pubspec.yaml, so a `lib/` + belonging to something else (an asset tree, a vendored copy) is not mistaken + for a package root. + """ + try: + here = start_dir.resolve() + except OSError: + return None + for parent in [here, *here.parents][:_DART_MAX_PARENTS]: + if parent.name == "lib" and (parent.parent / "pubspec.yaml").is_file(): + return parent + return None + + +def _resolve_dart_package_uri(raw: str, start_dir: Path) -> "Path | None": + """Resolve `package:/` to `/lib/`.""" + pkg, _, sub = raw[len("package:"):].partition("/") + if not pkg or not sub: + return None + for name, root in _dart_package_index(str(start_dir)): + if name != pkg: + continue + try: + candidate = (Path(root) / "lib" / sub).resolve() + except OSError: + return None + return candidate if candidate.is_file() else None + return None + + +def _resolve_dart_relative_uri(raw: str, start_dir: Path) -> "Path | None": + """Resolve a relative Dart URI, clamping `..` at the package root. + + A relative URI is resolved against the importing library's OWN uri, and for + anything under `lib/` that uri is `package:/...`, not a file path. The + difference is not cosmetic: RFC 3986 drops `..` segments that would escape the + base, so from `package:app/core/data/x.dart` an import of + `../../../features/y.dart` CLAMPS at the package root and resolves to + `package:app/features/y.dart`, while the same join over the file path escapes + `lib/` and lands on a file that does not exist. Dart accepts the first — this + is live, compiling code in real repos — so resolving on the filesystem alone + reports a false miss on exactly those imports. + + Outside `lib/` (bin/, test/, tool/, a loose script) the base really is a file + uri, so a plain join is the correct semantics there. + """ + lib_root = _dart_lib_root(start_dir) + if lib_root is None: + try: + candidate = (start_dir / raw).resolve() + except OSError: + return None + return candidate if candidate.is_file() else None + + try: + segments = list(start_dir.resolve().relative_to(lib_root).parts) + except ValueError: + segments = [] + for part in raw.split("/"): + if part in ("", "."): + continue + if part == "..": + if segments: + segments.pop() + continue # already at the package root: clamp, as `package:` does + segments.append(part) + candidate = lib_root.joinpath(*segments) + return candidate if candidate.is_file() else None + + +def _resolve_dart_uri(raw: str, start_dir: Path) -> "Path | None": + """Resolve a non-SDK Dart URI against `start_dir`. + + Split out of `_resolve_dart_import_target` so that entry point stays a thin + dispatcher; see its docstring for the URI forms handled here. + """ + if raw.startswith("package:"): + return _resolve_dart_package_uri(raw, start_dir) + if raw.startswith(("http:", "https:", "asset:")): + return None + return _resolve_dart_relative_uri(raw, start_dir) + + +def _resolve_dart_import_target(raw: str, str_path: str) -> "Path | None": + """Resolve a Dart `import`/`export`/`part` URI to a real file on disk. + + Dart resolution is fully deterministic — there is no extension guessing, no + index file, and no alias config, so only three forms exist: + + 'dart:async' SDK, always external -> None + '../cell/cell_widget.dart' relative to the importing file + 'package:app/x/y.dart' `lib/x/y.dart` of package `app` + + Returns the resolved path, or None when the URI names something outside the + scanned corpus (the SDK, a pub dependency, a dangling path) — mirroring + `_resolve_c_include_path`, so the caller keeps its existing external-node + behaviour untouched. + """ + if not raw or raw.startswith("dart:"): + return None + try: + start_dir = Path(str_path).parent + except Exception: + return None + return _resolve_dart_uri(raw, start_dir) + + def _resolve_lua_import_target(raw_module: str, str_path: str) -> str: """Resolve a Lua require() module name to a node id. diff --git a/tests/test_dart.py b/tests/test_dart.py index 094a6f5e3..46594e2f4 100644 --- a/tests/test_dart.py +++ b/tests/test_dart.py @@ -5,6 +5,10 @@ from pathlib import Path from graphify.extract import extract_dart, _make_id, _file_stem +from graphify.extractors.resolution import ( + _DART_PACKAGE_INDEX_CACHE, + _resolve_dart_import_target, +) class TestDart(unittest.TestCase): @@ -636,5 +640,142 @@ class ChildClass extends Bloc, State> {} self.assertEqual(nav_edge["target"], "route_home_id_123_type_auth") + +class TestDartImportResolution(unittest.TestCase): + """Dart import/export URIs must land on the id of the file they name (#2329). + + Before this, every Dart import edge targeted a bare string node with + source_file=None, so reverse traversal ("who imports this file", `affected`) + returned nothing on Dart while Python and TypeScript resolved. + """ + + def setUp(self): + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name).resolve() + _DART_PACKAGE_INDEX_CACHE.clear() + + def tearDown(self): + _DART_PACKAGE_INDEX_CACHE.clear() + self.temp_dir.cleanup() + + def _pkg(self, name, at=".", workspace=None): + d = (self.root / at).resolve() + (d / "lib").mkdir(parents=True, exist_ok=True) + body = "name: {}\n".format(name) + if workspace: + body += "workspace:\n" + "".join(" - {}\n".format(m) for m in workspace) + (d / "pubspec.yaml").write_text(body) + return d + + def _write(self, pkg_dir, rel, text=""): + f = pkg_dir / "lib" / rel + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(textwrap.dedent(text)) + return f + + def _import_targets(self, path): + out = extract_dart(path) + return { + e["target"] + for e in out["edges"] + if e["relation"] in ("imports", "exports") + } + + def test_relative_import_targets_the_real_file_node(self): + pkg = self._pkg("app") + target = self._write(pkg, "core/data/auth_repository.dart", "class AuthRepo {}") + importer = self._write( + pkg, "features/auth/login.dart", + "import '../../core/data/auth_repository.dart';", + ) + self.assertIn(_make_id(str(target)), self._import_targets(importer)) + + def test_package_import_of_own_package_resolves_through_pubspec_name(self): + pkg = self._pkg("app") + target = self._write(pkg, "config/theme.dart", "class Theme {}") + importer = self._write( + pkg, "features/home.dart", "import 'package:app/config/theme.dart';" + ) + self.assertIn(_make_id(str(target)), self._import_targets(importer)) + + def test_excess_parent_segments_clamp_at_the_package_root(self): + """A relative uri is resolved against the library's own `package:` uri, and + RFC 3986 drops `..` segments that would escape the base. So from + `package:app/core/data/x.dart` an import of `../../../features/y.dart` + resolves to `package:app/features/y.dart` — real, compiling Dart that a + plain filesystem join reports as a miss (it escapes `lib/`).""" + pkg = self._pkg("app") + target = self._write(pkg, "features/auth/auth_repository.dart", "class AuthRepo {}") + importer = self._write( + pkg, "core/data/prefs.dart", + "import '../../../features/auth/auth_repository.dart';", + ) + self.assertIn(_make_id(str(target)), self._import_targets(importer)) + + def test_export_resolves_like_import(self): + pkg = self._pkg("app") + target = self._write(pkg, "models/user.dart", "class User {}") + importer = self._write(pkg, "models.dart", "export 'models/user.dart';") + self.assertIn(_make_id(str(target)), self._import_targets(importer)) + + def test_workspace_sibling_package_resolves(self): + self._pkg("root", workspace=["apps/app", "packages/shared"]) + app = self._pkg("app", at="apps/app") + shared = self._pkg("shared", at="packages/shared") + target = self._write(shared, "pricing.dart", "class Pricing {}") + importer = self._write( + app, "main.dart", "import 'package:shared/pricing.dart';" + ) + self.assertIn(_make_id(str(target)), self._import_targets(importer)) + + def test_sdk_and_third_party_imports_stay_external(self): + pkg = self._pkg("app") + importer = self._write( + pkg, "main.dart", + """\ + import 'dart:async'; + import 'package:flutter/material.dart'; + import 'package:app/missing.dart'; + """, + ) + targets = self._import_targets(importer) + for raw in ("dart:async", "package:flutter/material.dart", "package:app/missing.dart"): + self.assertIn(_make_id(raw), targets, raw) + + def test_outside_lib_does_not_clamp(self): + """In bin/ or tool/ the library's base really is a file uri, so an escaping + `..` is a genuine miss and must stay external rather than silently + clamping onto an unrelated file.""" + pkg = self._pkg("app") + self._write(pkg, "helper.dart", "class Helper {}") + (pkg / "bin").mkdir(exist_ok=True) + script = pkg / "bin" / "run.dart" + script.write_text("import '../../../lib/helper.dart';") + self.assertIn(_make_id("../../../lib/helper.dart"), self._import_targets(script)) + + def test_dot_segments_in_the_importing_files_own_path_are_normalized(self): + """The importing file's own path may carry `.`/`..` segments (a scan rooted + through one, a symlinked corpus). Both the `lib/` root and the start + directory are resolved before the relative walk, so those segments never + shift the result or the clamping.""" + pkg = self._pkg("app") + target = self._write(pkg, "features/auth/repo.dart", "class Repo {}") + self._write(pkg, "core/data/prefs.dart", "") + (pkg / "lib" / "core" / "zzz").mkdir(parents=True, exist_ok=True) + noisy = str(pkg / "lib" / "core" / "zzz" / ".." / "data" / "prefs.dart") + clean = str(pkg / "lib" / "core" / "data" / "prefs.dart") + for path in (clean, noisy): + for raw in ("../../features/auth/repo.dart", # exact + "../../../features/auth/repo.dart"): # one too many: clamps + _DART_PACKAGE_INDEX_CACHE.clear() + self.assertEqual(_resolve_dart_import_target(raw, path), target, (path, raw)) + + def test_resolver_returns_none_for_unresolvable_uris(self): + pkg = self._pkg("app") + f = self._write(pkg, "main.dart", "") + for raw in ("", "dart:core", "package:other/x.dart", "package:app", "./gone.dart"): + self.assertIsNone(_resolve_dart_import_target(raw, str(f)), raw) + + if __name__ == "__main__": unittest.main()