-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
fix(dart): resolve import/export uris to file nodes (#2329) #2570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v8
Are you sure you want to change the base?
Changes from all commits
66fe1ab
fb3ba1c
3cbc858
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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: | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
fans out to 7 callees (efferent coupling); 9 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
fans out to 7 callees (efferent coupling); 9 callers depend on it (afferent coupling). Grounded coupling-delta finding (deterministic), not an LLM guess. |
||||||||||
|
|
@@ -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: | ||||||||||
|
Comment on lines
+514
to
+515
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. export/import edge target uses resolved file path id, not the file node's own id scheme — agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Graphify suggests a fix:
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Measured this rather than argue it, since the finding is flagged unverified — it doesn't reproduce, and I'd rather not change the id scheme on a hunch. I applied the suggestion verbatim ( Keeping
I also probed the one case where the two could genuinely diverge, since I call Happy to switch if there's a scenario I haven't constructed — I just couldn't produce one where the current form is the weaker of the two. |
||||||||||
| 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<Type>() or object.methodName<Type>() | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract_dart()fans out to 7 callees (efferent coupling); 9 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged, but I don't think there's a fix here — the new callee is the change.
extract_dartpreviously synthesized its import targets inline (_make_id(pkg)on the raw uri string), which is precisely why they never reached a file node. Resolving them means calling a resolver, so the +1 efferent edge is the feature rather than a side effect. The alternative — inlining the resolution back intoextract_dart— would keep the coupling number flat while making the function considerably larger and duplicating logic thatresolution.pyalready owns for JS/TS, C and Lua.For what it's worth, this is the same shape
extract_objcalready has: it calls_resolve_c_include_pathfrom the same module for exactly the same reason.Happy to restructure if you'd rather the call site sit somewhere else.