Skip to content

Commit e69b9ff

Browse files
committed
Drive prewarm bindings off command argDefs, not an allowlist
The prewarm walker previously fed `getCommandsUntilLine` a hardcoded list of `["load", "set", "switch"]`, silently skipping every other binding-producing command. Hovering `$voting` after `install $voting voting:0.1.0`, `$myDao` after `new-dao`, `$myToken` after `new-token`, or any variable defined by `deploy`, `sign`, `for` returned "no info"; helpers nested inside a `connect myDao { ... }` block whose body the cursor wasn't in never reached the helper cache either. Replaces the allowlist with a new `getAllCommandsUntilLine(line)` traversal that visits every command in DFS pre-order regardless of block nesting. Per-command behaviour is now driven entirely by the node itself: `argDef.type === "variable"` seeds the USER binding, custom argDef types with a `resolve()` hook (`module`, `dao`, ...) get invoked, and any `eventCaptures` / `errorCaptures` on the node seed placeholder bindings for their slot variables (and `boolVar`), mirroring the runtime's `applyDestructure` `$`-prefixing rule. New binding-producing commands therefore participate in prewarm with no allowlist edits. Hover suppresses the noisy `**Variable** $foo = $foo` rendering when the binding value is the placeholder name (i.e. the command can't predict its runtime output yet), so `$myContract` from `deploy` shows as `**Variable** $myContract` rather than restating itself. Adds regression coverage for the `deploy`-style variable, the `-> Evt(...) [$a $b]` event capture, and the `-!>` / `-?!>` error capture (both destructure and boolVar shapes). Made-with: Cursor
1 parent 166e32b commit e69b9ff

4 files changed

Lines changed: 205 additions & 37 deletions

File tree

packages/core/src/EvmlAST.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,41 @@ export class EvmlAST implements AST {
3838
);
3939
}
4040

41+
/**
42+
* Walk every command in the AST up to (and including) `line`,
43+
* descending into every block expression regardless of whether it
44+
* encloses `line`. Returns a flat list in DFS pre-order (parent
45+
* before its block's children). Used by the prewarm walker so that
46+
* hover sees bindings produced by any command — `set`, `deploy`,
47+
* `new-dao`, `install`, `for`, `sign`, command-with-event/error
48+
* captures, ... — without having to enumerate command names by hand.
49+
*/
50+
getAllCommandsUntilLine(line: number): CommandExpressionNode[] {
51+
return this.#getAllCommandsUntilLine(this.body, line);
52+
}
53+
54+
#getAllCommandsUntilLine(
55+
body: CommandExpressionNode[],
56+
line: number,
57+
): CommandExpressionNode[] {
58+
const out: CommandExpressionNode[] = [];
59+
for (const c of body) {
60+
if (c.loc?.start.line && c.loc.start.line > line) continue;
61+
out.push(c);
62+
for (const arg of c.args) {
63+
if (arg.type === NodeType.BlockExpression) {
64+
out.push(
65+
...this.#getAllCommandsUntilLine(
66+
(arg as BlockExpressionNode).body,
67+
line,
68+
),
69+
);
70+
}
71+
}
72+
}
73+
return out;
74+
}
75+
4176
#getCommandAtLine(
4277
line: number,
4378
body: CommandExpressionNode[],

packages/core/src/hover/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,20 @@ export async function getHoverInfo(
360360
);
361361
const formatted = formatVariableValue(value, scriptLines);
362362

363+
// Commands like `deploy`, `new-dao`, `new-token`, `install`, `sign`
364+
// and `for` produce a runtime value the prewarm walker can't
365+
// predict; it seeds the binding with the variable's own name as a
366+
// placeholder so subsequent hovers know the symbol is defined.
367+
// Treat that as "no resolvable value yet" rather than rendering
368+
// `**Variable** $voting = $voting`.
369+
const isPlaceholder = formatted === token.value;
370+
363371
// Single-line card: `**Variable** $name = value`. The address
364372
// card, if any, is appended as a separate section so the renderer
365373
// draws the green divider between the two — no need for a verbose
366374
// "(variable)" tag or "(defined on line N)" hint.
367375
const labelCard =
368-
formatted != null
376+
formatted != null && !isPlaceholder
369377
? `**Variable** ${token.value} = ${formatted}`
370378
: `**Variable** ${token.value}`;
371379
const sections: string[] = [labelCard];

packages/core/src/scriptWalk.ts

Lines changed: 99 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import type {
44
CompletionContext,
55
DestructurePatternNode,
66
DestructureSlot,
7+
ErrorCaptureNode,
8+
EventCaptureNode,
79
HelperResolver,
810
ICommand,
911
ModuleBinding,
@@ -262,8 +264,64 @@ function seedDestructureSlots(
262264
// Walk
263265
// ---------------------------------------------------------------------------
264266

267+
/** Seed every `EventCapture` / `ErrorCapture` slot on a command as a
268+
* USER binding. The walker can't predict the runtime value (logs and
269+
* reverts only exist at execution time), so the bindings hold a
270+
* placeholder name that the hover renderer suppresses — what matters
271+
* is that the variable *exists* in the prewarm snapshot, so hovering
272+
* `$err` later renders `**Variable** $err` instead of "no info".
273+
*
274+
* Note: capture slots are parsed WITHOUT the `$` prefix (the runtime
275+
* prefixes them when calling `applyDestructure`). We mirror that here
276+
* so the hover lookup `bindings.getBindingValue("$err", USER)` finds
277+
* the placeholder. */
278+
function seedCapturePlaceholders(
279+
slots: DestructureSlot[],
280+
bindings: BindingsManager,
281+
): void {
282+
for (const s of slots) {
283+
if (typeof s === "string") {
284+
const key = `$${s}`;
285+
try {
286+
bindings.setBinding(key, key, USER);
287+
} catch {
288+
// binding already exists
289+
}
290+
} else if (Array.isArray(s)) {
291+
seedCapturePlaceholders(s, bindings);
292+
}
293+
}
294+
}
295+
296+
function seedCaptureBindings(
297+
c: CommandExpressionNode,
298+
bindings: BindingsManager,
299+
): void {
300+
const captures: Array<EventCaptureNode | ErrorCaptureNode> = [
301+
...(c.eventCaptures ?? []),
302+
...(c.errorCaptures ?? []),
303+
];
304+
for (const cap of captures) {
305+
seedCapturePlaceholders(cap.captures, bindings);
306+
if ("boolVar" in cap && cap.boolVar) {
307+
try {
308+
bindings.setBinding(`$${cap.boolVar}`, `$${cap.boolVar}`, USER);
309+
} catch {
310+
// binding already exists
311+
}
312+
}
313+
}
314+
}
315+
265316
/** Walk a list of fully-typed command nodes and resolve any bindings they
266-
* produce (variable → USER, custom type with resolve → arbitrary bindings).
317+
* produce (variable → USER, custom type with resolve → arbitrary bindings,
318+
* event/error captures → USER placeholders).
319+
*
320+
* Per-command behaviour is driven by the command's own argDefs (looking
321+
* for `type: "variable"` or a custom type with a `resolve()` hook) and
322+
* by any `eventCaptures` / `errorCaptures` attached to the node — never
323+
* by command-name allowlists. New binding-producing commands therefore
324+
* participate in prewarm automatically.
267325
*
268326
* When `history` is provided, each `set`-style variable assignment is also
269327
* appended to it as `{ line, value }`. Hover uses this to render the value
@@ -286,6 +344,12 @@ export async function walkCommandsForBindings(
286344
}
287345
const commandModule = c.module ?? parentModule;
288346

347+
// Even when the command isn't registered (e.g. an aragonos command
348+
// before `load aragonos` has run, or a typo) the user can still
349+
// attach event/error captures to it — seed those so hover finds
350+
// the symbols even when the command resolution fails.
351+
seedCaptureBindings(c, bindings);
352+
289353
const command = await resolveCommandNode(c, bindings, commandModule);
290354
if (!command) continue;
291355

@@ -414,16 +478,24 @@ export interface WalkResult {
414478
}
415479

416480
/**
417-
* Parse `script` and walk all `load` / `set` / `switch` commands up to and
418-
* including `upToLine`. Helper-function results are written into
419-
* `moduleCache`'s CACHE space (using the same key shape as completions),
420-
* which is what makes hover's address card visible for `@ens(...)` etc.
481+
* Parse `script` and walk every command up to (and including) `upToLine`,
482+
* producing the bindings + variable history hover needs to render rich
483+
* cards. Helper-function results are also written into `moduleCache`'s
484+
* CACHE space (using the same key shape as completions), which is what
485+
* makes hover's address card visible for `@ens(...)` etc.
421486
*
422-
* `upToLine` is 1-indexed (matches `Position.line`). Pass `Infinity` to walk
423-
* the entire script.
487+
* Per-command behaviour is purely argDef-driven (see
488+
* `walkCommandsForBindings`): commands with `type: "variable"` argDefs
489+
* seed USER bindings, custom-typed argDefs with a `resolve()` hook get
490+
* invoked, and any `eventCaptures` / `errorCaptures` on a node seed
491+
* placeholder bindings for the captured slots. This means new
492+
* binding-producing commands work automatically — no allowlist edits.
493+
*
494+
* `upToLine` is 1-indexed (matches `Position.line`). Pass `Infinity` to
495+
* walk the entire script.
424496
*
425497
* Returns the per-walk USER bindings and the effective chainId after any
426-
* `switch` commands. Failures during parsing or RPC lookup are swallowed —
498+
* `switch` commands. Failures during parsing or RPC lookup are swallowed —
427499
* the function always resolves so callers can use it as a best-effort
428500
* pre-warm step.
429501
*/
@@ -456,24 +528,18 @@ export async function walkScript(
456528
}
457529
}
458530

459-
const commandNodes: CommandExpressionNode[] = (
460-
ast?.getCommandsUntilLine(upToLine, ["load", "set", "switch"]) ?? []
461-
).filter((c: CommandExpressionNode) => {
462-
const itHasCommandsBlock = hasCommandsBlock(c);
463-
const loc = c.loc;
464-
if (
465-
!itHasCommandsBlock ||
466-
(itHasCommandsBlock &&
467-
loc &&
468-
upToLine >= loc.start.line &&
469-
upToLine <= loc.end.line)
470-
) {
471-
return true;
472-
}
473-
return false;
474-
});
475-
476-
// Apply switch commands to find the effective client/chain.
531+
// Walk EVERY command up to `upToLine`, including those inside blocks
532+
// that don't enclose the cursor (e.g. a `connect myDao { ... }`
533+
// block whose body installs apps the rest of the script references).
534+
// The per-command logic in `walkCommandsForBindings` already decides
535+
// what to do based on each command's argDefs and capture nodes — no
536+
// command-name allowlist required.
537+
const commandNodes: CommandExpressionNode[] =
538+
ast?.getAllCommandsUntilLine(upToLine) ?? [];
539+
540+
// Apply switch commands to find the effective client/chain. `switch`
541+
// is the only command that mutates *which* chain the rest of the
542+
// walk runs against, so it gets a dedicated pre-pass.
477543
let effectiveClient = client;
478544
for (const c of commandNodes) {
479545
const switchedChainId = resolveSwitchChainId(c, bindings);
@@ -510,16 +576,13 @@ export async function walkScript(
510576
variableHistory,
511577
);
512578

513-
// Second pass: populate the helper cache for *all* commands the user
514-
// might hover over, not just the ones that produce bindings. Without
515-
// this, `print @token(DAI)` and similar helpers used as direct args
516-
// to non-binding commands never reach the cache and the hover card
517-
// can't show their resolved address. Helpers already evaluated
518-
// during `walkCommandsForBindings` are cache hits and re-add no
519-
// network cost.
520-
const allCommandNodes: CommandExpressionNode[] =
521-
ast?.getCommandsUntilLine(upToLine) ?? [];
522-
await prewarmHelperExpressions(allCommandNodes, resolveNode);
579+
// Second pass: populate the helper cache for *every* command the
580+
// user might hover over, including helpers inside non-binding
581+
// commands (`print @token(DAI)`) and helpers nested deep inside
582+
// `connect myDao { ... }` blocks. Helpers already evaluated during
583+
// `walkCommandsForBindings` are cache hits and re-add no network
584+
// cost.
585+
await prewarmHelperExpressions(commandNodes, resolveNode);
523586

524587
return { bindings, chainId, client: effectiveClient, variableHistory };
525588
}

packages/core/test/integration/hover.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,68 @@ describe("Core > hover", () => {
115115
expect(c).to.match(/\*\*EOA\*\*|\*\*Contract\*\*/);
116116
});
117117

118+
it("seeds variable bindings for any command with a `type: variable` argDef", async () => {
119+
// Regression: previously the prewarm walker only visited
120+
// `load`/`set`/`switch` commands, so variables created by
121+
// commands like `deploy`, `new-dao`, `new-token`, `install`,
122+
// `sign` and `for` were never seeded — hovering them returned
123+
// null. The walker now visits every command and trusts each
124+
// command's own argDefs to decide what to bind, so any
125+
// `type: "variable"` argDef on any command participates in
126+
// prewarm automatically.
127+
const script = "deploy $myContract Foo.sol\nprint $myContract";
128+
const evm = ctx.createEvm();
129+
await evm.prewarm(script);
130+
const result = await evm.getHoverInfo(script, { line: 2, col: 8 });
131+
expect(result).to.not.be.null;
132+
const c = result!.contents.join("\n");
133+
expect(c).to.include("**Variable**");
134+
expect(c).to.include("$myContract");
135+
// Placeholder bindings (value === name) shouldn't render as
136+
// `$myContract = $myContract` — that's noise.
137+
expect(c).to.not.include("= $myContract");
138+
});
139+
140+
it("seeds variable bindings for event-capture slots", async () => {
141+
// The `-> Withdrawn(uint, address) [$amount $to]` syntax
142+
// attaches an `EventCaptureNode` to a command. The walker has to
143+
// descend into `c.eventCaptures` and seed each slot as a USER
144+
// placeholder so hover finds `$amount` even though no command
145+
// argDef declares it.
146+
const script =
147+
"exec $contract withdraw() -> Withdrawn(uint,address) [$amount $to]\nprint $amount";
148+
const evm = ctx.createEvm();
149+
await evm.prewarm(script);
150+
const result = await evm.getHoverInfo(script, { line: 2, col: 8 });
151+
expect(result).to.not.be.null;
152+
const c = result!.contents.join("\n");
153+
expect(c).to.include("**Variable**");
154+
expect(c).to.include("$amount");
155+
expect(c).to.not.include("= $amount");
156+
});
157+
158+
it("seeds variable bindings for error-capture slots and bool flags", async () => {
159+
// `-!> Err(uint) [$shortfall]` (required) and `-?!> Err $e`
160+
// (optional, boolean) both produce USER bindings the walker
161+
// must seed so hover finds them. We exercise both shapes in one
162+
// script — the boolVar variant goes through a different code
163+
// path than the destructure-slot variant.
164+
const script = [
165+
"exec $contract transfer(uint) 100 -!> InsufficientBalance(uint) [$shortfall]",
166+
"exec $contract approve(address,uint) $spender 1 -?!> Unauthorized() $authErr",
167+
"print $shortfall",
168+
"print $authErr",
169+
].join("\n");
170+
const evm = ctx.createEvm();
171+
await evm.prewarm(script);
172+
const shortfall = await evm.getHoverInfo(script, { line: 3, col: 8 });
173+
const authErr = await evm.getHoverInfo(script, { line: 4, col: 8 });
174+
expect(shortfall).to.not.be.null;
175+
expect(shortfall!.contents.join("\n")).to.include("$shortfall");
176+
expect(authErr).to.not.be.null;
177+
expect(authErr!.contents.join("\n")).to.include("$authErr");
178+
});
179+
118180
it("appends the address card under @token(...) when used as a direct arg to a non-binding command", async () => {
119181
// Regression: previously the prewarm walker only visited
120182
// `load`/`set`/`switch` commands, so helpers used as direct args to

0 commit comments

Comments
 (0)