Add source_location to Dart extraction - #3379
Conversation
extractors/dart.py hardcoded source_location: None at every node and edge construction site, so a Dart graph could cite zero lines no matter how many declarations it had (0% across three real repos, ~25k unciteable nodes in one of them). Threads a line number through add_node/add_edge from each match's own offset, anchored on the capture group rather than the whole match: a pattern like the class declaration's leading ^\s* otherwise reports the line too early when blank lines precede the declaration. The comment stripping pass also needed a fix first: it deleted a comment token outright, so a multi-line comment shifted every later offset (and therefore every later line number) up by the comment's own height. Blanking it to the same number of newlines instead keeps src_clean's line count in step with the original file. Sourceless reference stub nodes (an inherited class, a mixin, an import target, and similar names the file only references rather than defines) keep source_location: None, same as before, since there is no single line in THIS file for a symbol that lives elsewhere. Every node this file actually defines, and every edge attributed to a real declaration site, now carries its real line. Fixes Graphify-Labs#3365.
|
@safishamsi opened this against #3365 (Dart nodes had zero source_location, 0% across the reporter's real repos). Verified the fix against their exact repro and the multi-line-comment trap they flagged, plus checked every add_node/add_edge call site in the file for coverage. Happy to address any feedback. |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Records source line numbers on Dart nodes and edges, populating source_location as L<n> (previously always None) by threading an optional line through add_node and add_edge and computing it via a new _line_at helper. Fixes line-number drift (#3365) by having _comment_replace preserve a comment's newline count instead of deleting it, and by anchoring class declarations on the name capture group so leading whitespace no longer reports the line too early. Applies these line numbers across class/typedef definitions, inheritance, mixins, interfaces, annotations, and Bloc/Riverpod reference edges.
Worth a look
- Bloc handler line offsets are wrong when class_body is trimmed relative to brace_pos —
graphify/extractors/dart.py:293· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 31 functions depend on the 30 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_dart()— 11 callers, 7 callees
Verification — 31 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 31 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_dart.
The verifier did not have enough to check extract\_dart, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 more finding(s) on lines outside this diff (see the check run).
|
Looked into the bot finding on the bloc_event line offset (dart.py:293). class_body = src_clean[brace_pos:end_pos] is a plain slice, no reindexing, so class_body[k] is always src_clean[brace_pos + k] and _line_at(brace_pos + em.start(1)) should be correct by construction. Stress tested directly: multiple handlers on different lines, a leading comment, brace characters inside string literals before and inside the class body, and a second class following an unrelated first one with its own brace-in-string. Every case reported the true source line exactly. Could not reproduce the claimed issue, so treating it as a false positive rather than changing anything speculatively. Happy to revisit with a concrete repro if one turns up. (Separately noticed, unrelated to this finding and predating this PR: a .add() call inside a method can get counted twice, once by the class level scan and once by the method level scan, since their bodies overlap. Not touching that here since it's out of scope for #3365.) |
What
extractors/dart.pyhardcodedsource_location: Noneat every node and edge construction site, so a Dart graph could cite zero lines no matter how many declarations it had. Threads a real line number throughadd_node/add_edgefrom each match's own offset instead.The trap (verified, matches the issue)
Two details had to be right together, or line numbers would be silently wrong rather than missing:
^\s*, som.start()sits before any preceding blank lines and reports the line too early.return ""), so a multi-line comment shifted every later offset up by the comment's own height. Fixed to blank to the same number of newlines instead.Verified both against the reporter's own repro:
Scope
Sourceless reference stub nodes (an inherited class, a mixin, an import target — anything the file only references rather than defines, already marked
source_file=None) keepsource_location: None. There's no single line in this file for a symbol that lives elsewhere. Every node this file actually defines, and every edge attributed to a real declaration site, now carries its real line — verified this covers everyadd_node/add_edgecall site in the file (classes, mixins, extensions, typedefs, variables, methods, annotations, imports/exports, and all the framework-specific reference patterns).Fixes #3365.