fix(js): capture function-valued object properties like shorthand methods - #2947
fix(js): capture function-valued object properties like shorthand methods#2947rajanpanth wants to merge 2 commits into
Conversation
…hods
The shorthand { m(){} } is a method_definition and lands in the generic
function branch, but { m: () => {} } and { m: function(){} } parse as
pair nodes and were skipped, so the arrow and function-expression
spellings of the same API surface vanished from the graph. The pair
branch mirrors the function branch exactly: same walk position, same
scoping, same body tracking, so scoping baselines are unchanged.
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 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Captures function-valued object-literal properties ({ m: () => {} } and { m: function(){} }) in the JS/TS _extract_generic walk, mirroring the existing method_definition shorthand branch — same node/edge shape, scoping, and body tracking so calls inside the property resolve. Only plain property_identifier keys are handled; computed and string keys remain skipped. Adds tests/test_js_object_property_functions.py covering spelling parity, body-call resolution, and the const-object scoping baseline.
Worth a look
- New pair-based method branch changes graph output for const-bound object literals —
graphify/extractors/engine.py:4038· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Parenthesized function-valued object properties are skipped —
graphify/extractors/engine.py:4044· 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 — 623 functions depend on the 212 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_extract_generic()— 18 callers, 24 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_js()— 82 callers, 3 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_julia()— 16 callers, 7 callees - new:
extract_cpp()— 27 callers, 3 callees - new:
extract_vue()— 10 callers, 6 callees - new:
walk()— 1 callers, 56 callees - …and 8 more — each is listed as a finding
Verification — 623 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: 563 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, 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
· 16 more finding(s) on lines outside this diff (see the check run).
Review advisory: { m: (() => {}) } wraps the function in a
parenthesized_expression, which the pair branch skipped.
d14928b to
50cc537
Compare
|
Both advisories addressed. Parenthesized values: real gap, fixed in 50cc537. Const-bound object literals: no output change there, and the test suite locks it. |
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
Adds a pair-node branch to _extract_generic in graphify/extractors/engine.py so JS/TS function-valued object properties ({ m: () => {} }, { m: function(){} }, including parenthesized forms) emit the same node/edge shape and body tracking as shorthand method_definition members. Restricts capture to plain identifier keys with the existing #1899 normalize guard, leaving computed/string keys and const-object scoping unchanged. Adds tests/test_js_object_property_functions.py covering the three spellings, mixed objects, call-argument parity, in-body call resolution, and the skip/scoping baselines.
Worth a look
- Pair-function branch ignores parent scoping context, emitting members for nested/const object literals —
graphify/extractors/engine.py:4059· Escalate · high- 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 — 624 functions depend on the 213 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_extract_generic()— 18 callers, 24 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_js()— 82 callers, 3 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_julia()— 16 callers, 7 callees - new:
extract_cpp()— 27 callers, 3 callees - new:
extract_vue()— 10 callers, 6 callees - new:
walk()— 1 callers, 56 callees - …and 8 more — each is listed as a finding
Verification — 624 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: 564 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, 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
· 16 more finding(s) on lines outside this diff (see the check run).
Two spellings of the same exported API produce different graphs:
The shorthand parses as a
method_definition, which the generic function branch captures. The arrow and function-expression forms parse aspairnodes, which nothing handled, so those symbols vanished. Same class of gap as the factory object API fix (#2745): common style, silently missing surface.{ handler: async () => {...} }route tables and options objects are the everyday casualties.Fix
A
pairwhose value is a function type is intercepted at the same walk position as the function branch and mirrors it exactly: same naming, same class-vs-file scoping, samecallable_def_nidsandlocal_bound_namesregistration, samefunction_bodiestracking so calls made inside the property resolve. Only plain identifier keys are named; computed and string keys stay out of the graph as before.Because it sits in the same walk with the same guards, the scoping envelope is byte-for-byte the shorthand envelope:
module.exports = { m }m()m()register({ m })m()m()const api = { m }apiapiapi(unchanged)Verification
constscoping baseline, computed/string keys still skipped). With the engine change reverted, 6 of 7 fail.callsedges from inside an arrow property resolve:module.exports = { run: () => helper() }emitsrun() -> helper().