Prechádzať zdrojové kódy

feat(steps): guards say which decision they belong to, and how an arm leaves

A joined `when` string cannot tell an `if` from its `else`: two sites read as
opposite conditions, and nothing says they are the two arms of ONE decision.
The reading a rail needs is the structure, so each guard now carries it:

- `branch` — where the branching construct starts (`line:column`). Both arms of
  an `if`, every case of a `switch`, an early exit and the code it guards share
  it; two `try`/`catch` blocks in one function no longer collapse into one.
- `armExit` — how the arm the site is in leaves, when it always does (`return`,
  `throw`, or `exit` for a `panic` / `exit()` the rules count but no keyword
  names), read from the arm's last statement.
- `exit` — for an early exit, how the arm that was NOT taken leaves.

`SiteReader.guards()` returns the array; `when` is now `guardLabel` over it, so
a caller that wants both pays for one read. Nothing else changes: `guardLabel`
ignores the new fields and every existing label is byte-identical.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REFyW9hmNrxhwN5wxRoAkC
Colby McHenry 1 týždeň pred
rodič
commit
482690b62d

+ 108 - 0
__tests__/branch-guards.test.ts

@@ -201,6 +201,114 @@ func f() {
   });
 });
 
+describe('branch guards: the arms of one decision', () => {
+  /** The guards at the site, unjoined. */
+  async function guardsAt(src: string, needle: string, language: 'tsx' | 'typescript' | 'swift' = 'tsx') {
+    const line = lineOf(src, needle);
+    const column = src.split('\n')[line - 1]!.indexOf(needle);
+    return guardsInSource(src, language, line, column);
+  }
+
+  const ifElse = `
+export async function authUser(req, res) {
+  const user = await User.findOne({ email })
+  if (user && (await user.matchPassword(password))) {
+    res.json({ token: generateToken(user._id) })
+  } else {
+    res.status(401)
+    throw new Error('Invalid email or password')
+  }
+}`;
+
+  it('gives an if and its else the same branch, with negated flipped', async () => {
+    const yes = await guardsAt(ifElse, 'res.json');
+    const no = await guardsAt(ifElse, 'res.status');
+    expect(yes).toHaveLength(1);
+    expect(no).toHaveLength(1);
+    expect(yes[0]!.text).toBe(no[0]!.text);
+    expect(yes[0]!.negated).toBe(false);
+    expect(no[0]!.negated).toBe(true);
+    // The identity of the FORK, not of the arm: both arms of one `if`.
+    expect(yes[0]!.branch).toBe(no[0]!.branch);
+    expect(yes[0]!.branch).toMatch(/^\d+:\d+$/);
+    // The else arm ends by throwing; the then arm runs on.
+    expect(no[0]!.armExit).toBe('throw');
+    expect(yes[0]!.armExit).toBeUndefined();
+  });
+
+  const earlyExit = `
+export async function createReview(req, res) {
+  const product = await Product.findById(req.params.id)
+  if (!product) {
+    res.status(404)
+    throw new Error('Product not found')
+  }
+  await product.save()
+}`;
+
+  it('gives an early exit and the code it guards the same branch', async () => {
+    const inside = await guardsAt(earlyExit, 'res.status');
+    const after = await guardsAt(earlyExit, 'product.save');
+    expect(inside).toHaveLength(1);
+    expect(after).toHaveLength(1);
+    expect(inside[0]!.branch).toBe(after[0]!.branch);
+    expect(inside[0]!.negated).toBe(false);
+    expect(after[0]!.negated).toBe(true);
+    // The arm NOT taken throws — what the rail draws as the fork's terminal.
+    expect(after[0]!.form).toBe('guard');
+    expect(after[0]!.exit).toBe('throw');
+    expect(inside[0]!.armExit).toBe('throw');
+  });
+
+  const switched = `
+export function route(kind) {
+  switch (kind) {
+    case 'a':
+      first()
+      break
+    case 'b':
+      second()
+      break
+    default:
+      other()
+  }
+}`;
+
+  it('gives every case of one switch the same branch', async () => {
+    const a = await guardsAt(switched, 'first()');
+    const b = await guardsAt(switched, 'second()');
+    const d = await guardsAt(switched, 'other()');
+    expect(a[0]!.branch).toBe(b[0]!.branch);
+    expect(a[0]!.branch).toBe(d[0]!.branch);
+    expect([a[0]!.text, b[0]!.text, d[0]!.text]).toEqual(['kind === \'a\'', 'kind === \'b\'', 'kind: default']);
+  });
+
+  it('gives two try/catch blocks branches of their own', async () => {
+    const src = `
+export async function save() {
+  try { await a() } catch (e) { first(e) }
+  try { await b() } catch (e) { second(e) }
+}`;
+    const one = await guardsAt(src, 'first(e)');
+    const two = await guardsAt(src, 'second(e)');
+    expect(one[0]!.text).toBe('on error');
+    expect(two[0]!.text).toBe('on error');
+    expect(one[0]!.branch).not.toBe(two[0]!.branch);
+  });
+
+  it('reads a Swift guard as an exit', async () => {
+    const src = `
+func load() {
+  guard let user = current else { return }
+  fetch(user)
+}`;
+    const after = await guardsAt(src, 'fetch(user)', 'swift');
+    expect(after[0]!.form).toBe('guard');
+    expect(after[0]!.exit).toBe('return');
+    expect(after[0]!.branch).toMatch(/^\d+:\d+$/);
+  });
+});
+
 describe('branch guards: unsupported', () => {
   it('reports no guards for a language without rules', async () => {
     expect(supportsBranchGuards('ruby')).toBe(false);

+ 126 - 23
src/graph/branch-guards.ts

@@ -40,6 +40,9 @@ import { getParser, loadGrammarsForLanguages } from '../extraction/grammars';
 
 export type GuardForm = 'if' | 'else' | 'ternary' | 'case' | 'guard' | 'and' | 'or' | 'catch';
 
+/** How an arm leaves the flow: back to the caller, or by an error. */
+export type GuardExit = 'return' | 'throw' | 'exit';
+
 export interface BranchGuard {
   /** The condition's source, whitespace-collapsed, outer parens dropped, capped in length. */
   text: string;
@@ -48,6 +51,20 @@ export interface BranchGuard {
   form: GuardForm;
   /** Line of the condition (1-based). */
   line: number;
+  /**
+   * Where the branching construct starts, `line:column` (1-based line, 0-based
+   * column) — the identity of the FORK rather than of the arm: the `if` an
+   * `if` guard and its `else` guard both come from, the `switch` every one of
+   * its cases comes from, the `try` a `catch` belongs to. Two guards with the
+   * same `branch` are arms of one decision, which is what a reader of the code
+   * in its own order needs and a joined condition string cannot say.
+   * '' when the walk could not place it.
+   */
+  branch: string;
+  /** How the arm the site is IN leaves, when it always does — `return`, `throw`. */
+  armExit?: GuardExit;
+  /** For an early exit (`form: 'guard'`), how the arm that was NOT taken leaves. */
+  exit?: GuardExit;
 }
 
 /** Longest condition text kept before it is cut with an ellipsis. */
@@ -967,7 +984,17 @@ export function guardsInTree(
       node = parent;
       continue;
     }
+    // Anything `enclosing` pushed came from THIS construct, and the arm the
+    // site is in is the child the walk came up through: stamp both, unless the
+    // rule already named a different branch (a `switch` case's is the switch).
+    const before = found.length;
     rules.enclosing(parent, node, found);
+    for (let i = before; i < found.length; i++) {
+      const g = found[i]!;
+      if (!g.branch) g.branch = branchKey(parent);
+      const arm = exitKind(node);
+      if (arm && !g.armExit) g.armExit = arm;
+    }
     if (rules.blocks.has(parent.type)) rules.earlyExits(parent, node, found);
     node = parent;
   }
@@ -1041,10 +1068,82 @@ function condText(node: SyntaxNode | null | undefined): string {
   return text.length > MAX_TEXT ? text.slice(0, MAX_TEXT - 1) + '…' : text;
 }
 
-function guard(form: GuardForm, cond: SyntaxNode | null | undefined, negated: boolean, text?: string): BranchGuard | null {
+/**
+ * One guard. `branch` names the construct the arm belongs to: by default the
+ * node the walk is climbing THROUGH (an `if`, a ternary, a `catch`), which is
+ * right whenever the construct is the site's parent — and overridden where it
+ * is not, by a `switch` case (whose parent is the case) and by an early exit
+ * (whose branch is the `if` that returned, several statements back).
+ */
+function guard(
+  form: GuardForm,
+  cond: SyntaxNode | null | undefined,
+  negated: boolean,
+  text?: string,
+  branch?: SyntaxNode | null,
+  exit?: GuardExit | null
+): BranchGuard | null {
   const t = text ?? condText(cond);
   if (!t) return null;
-  return { text: t, negated, form, line: (cond ?? null) ? cond!.startPosition.row + 1 : 0 };
+  return {
+    text: t,
+    negated,
+    form,
+    line: (cond ?? null) ? cond!.startPosition.row + 1 : 0,
+    branch: branch ? branchKey(branch) : '',
+    ...(exit ? { exit } : {}),
+  };
+}
+
+/** A branching construct's identity: where it starts, `line:column`. */
+function branchKey(node: SyntaxNode): string {
+  return `${node.startPosition.row + 1}:${node.startPosition.column}`;
+}
+
+/**
+ * Containers whose LAST statement decides how the whole thing leaves: a block,
+ * and the clause wrappers a grammar puts an arm's block inside (`else_clause`,
+ * `except_clause`) — the walk climbs through those, so the node it hands back
+ * is the wrapper, not the block.
+ */
+const BLOCKISH: ReadonlySet<string> = new Set([
+  'statement_block',
+  'block',
+  'statements',
+  'function_body',
+  'compound_statement',
+  'control_structure_body',
+  'else_clause',
+  'elif_clause',
+  'else_statement',
+  'catch_clause',
+  'catch_block',
+  'except_clause',
+  'finally_clause',
+]);
+
+/**
+ * How a statement — or a block, by its last statement — leaves: `throw` for a
+ * raised error, `return` for a return / break / continue, `exit` for a
+ * language's other way out (Go's `panic`, C's `exit`) that the language rules
+ * count as an exit but no keyword names. Null when it does not always leave.
+ */
+function exitKind(node: SyntaxNode | null | undefined): GuardExit | null {
+  if (!node) return null;
+  const t = node.type;
+  if (t === 'throw_statement' || t === 'raise_statement' || t === 'throw_expression') return 'throw';
+  if (/^(?:return|break|continue|goto|yield)_statement$/.test(t)) return 'return';
+  // Swift's `control_transfer_statement` and Kotlin's `jump_expression` say
+  // which in their first word.
+  if (t === 'control_transfer_statement' || t === 'jump_expression') return /^\s*throw\b/.test(node.text) ? 'throw' : 'return';
+  if (BLOCKISH.has(t)) return exitKind(lastNamed(node));
+  return null;
+}
+
+/** {@link exitKind}, or `exit` when the language's rules call it an exit and no keyword names it. */
+function exitKindOr(node: SyntaxNode | null | undefined, exits: boolean): GuardExit | null {
+  if (!exits) return null;
+  return exitKind(node) ?? 'exit';
 }
 
 function push(out: BranchGuard[], g: BranchGuard | null): void {
@@ -1125,10 +1224,10 @@ const JS: Rules = {
         const body = parent.parent; // switch_body
         const stmt = body?.parent; // switch_statement
         const subject = condText(stmt?.childForFieldName('value'));
-        if (parent.type === 'switch_default') push(out, guard('case', stmt?.childForFieldName('value'), false, subject ? `${subject}: default` : 'default'));
+        if (parent.type === 'switch_default') push(out, guard('case', stmt?.childForFieldName('value'), false, subject ? `${subject}: default` : 'default', stmt));
         else {
           const value = condText(parent.childForFieldName('value'));
-          push(out, guard('case', parent.childForFieldName('value'), false, subject ? `${subject} === ${value}` : value));
+          push(out, guard('case', parent.childForFieldName('value'), false, subject ? `${subject} === ${value}` : value, stmt));
         }
         return;
       }
@@ -1156,8 +1255,9 @@ const JS: Rules = {
     for (let i = before.length - 1; i >= 0; i--) {
       const s = before[i]!;
       if (s.type !== 'if_statement' || s.childForFieldName('alternative')) continue;
-      if (!jsAlwaysExits(s.childForFieldName('consequence'))) continue;
-      push(out, guard('guard', s.childForFieldName('condition'), true));
+      const body = s.childForFieldName('consequence');
+      if (!jsAlwaysExits(body)) continue;
+      push(out, guard('guard', s.childForFieldName('condition'), true, undefined, s, exitKindOr(body, true)));
     }
   },
 };
@@ -1243,7 +1343,7 @@ const SWIFT: Rules = {
         const isDefault = parent.children.some((n) => n.type === 'default_keyword');
         const value = pattern ? condText(pattern) : '';
         const text = isDefault ? (subject ? `${subject}: default` : 'default') : subject ? `${subject} == ${value}` : value;
-        push(out, guard('case', pattern ?? stmt?.childForFieldName('expr'), false, text));
+        push(out, guard('case', pattern ?? stmt?.childForFieldName('expr'), false, text, stmt));
         return;
       }
       case 'catch_block':
@@ -1260,12 +1360,13 @@ const SWIFT: Rules = {
       const s = before[i]!;
       if (s.type === 'guard_statement') {
         const c = swiftConditions(s);
-        push(out, guard('guard', c.node, false, c.text));
+        const body = s.namedChildren.find((n) => n.type === 'statements') ?? null;
+        push(out, guard('guard', c.node, false, c.text, s, exitKindOr(body, true)));
       } else if (s.type === 'if_statement' && !s.children.some((n) => n.type === 'else')) {
         const body = s.namedChildren.find((n) => n.type === 'statements') ?? null;
         if (!swiftAlwaysExits(body)) continue;
         const c = swiftConditions(s);
-        push(out, guard('guard', c.node, true, c.text));
+        push(out, guard('guard', c.node, true, c.text, s, exitKindOr(body, true)));
       }
     }
   },
@@ -1301,9 +1402,10 @@ function guardsBefore(
   for (let i = before.length - 1; i >= 0; i--) {
     const s = before[i]!;
     if (!isIf(s) || hasElse(s)) continue;
-    if (!alwaysExits(body(s))) continue;
+    const arm = body(s);
+    if (!alwaysExits(arm)) continue;
     const c = condition(s);
-    push(out, guard('guard', c.node, true, c.text));
+    push(out, guard('guard', c.node, true, c.text, s, exitKindOr(arm, true)));
   }
 }
 
@@ -1368,7 +1470,7 @@ const PYTHON: Rules = {
           const value = pattern ? condText(pattern) : '';
           const isDefault = value === '_' || value === '';
           const text = isDefault ? (subject ? `${subject}: default` : 'default') : subject ? `${subject} == ${value}` : value;
-          push(out, guard('case', pattern ?? null, false, text));
+          push(out, guard('case', pattern ?? null, false, text, stmt));
         }
         return;
       }
@@ -1457,7 +1559,7 @@ const JAVA: Rules = {
         const stmt = parent.parent?.parent;
         const subject = condText(stmt?.childForFieldName('condition'));
         const labels = namedChildren(parent).filter((n) => n.type === 'switch_label');
-        push(out, guard('case', labels[0] ?? null, false, javaCaseText(labels, subject)));
+        push(out, guard('case', labels[0] ?? null, false, javaCaseText(labels, subject), stmt));
         return;
       }
       case 'binary_expression': {
@@ -1535,10 +1637,10 @@ const KOTLIN: Rules = {
         const when = parent.parent;
         const subject = condText(namedChildren(when!).find((n) => n.type === 'when_subject')).replace(/^\((.*)\)$/, '$1');
         const conds = namedChildren(parent).filter((n) => n.type === 'when_condition');
-        if (conds.length === 0) push(out, guard('case', null, false, subject ? `${subject}: else` : 'else'));
+        if (conds.length === 0) push(out, guard('case', null, false, subject ? `${subject}: else` : 'else', when));
         else {
           const value = conds.map((c) => condText(c)).join(', ');
-          push(out, guard('case', conds[0]!, false, subject ? `${subject} == ${value}` : value));
+          push(out, guard('case', conds[0]!, false, subject ? `${subject} == ${value}` : value, when));
         }
         return;
       }
@@ -1627,16 +1729,17 @@ const CSHARP: Rules = {
         const labels = namedChildren(parent).filter(isLabel);
         const value = labels.map((l) => condText(l)).filter(Boolean).join(', ');
         const text = value === '' ? (subject ? `${subject}: default` : 'default') : subject ? `${subject} == ${value}` : value;
-        push(out, guard('case', labels[0] ?? null, false, text));
+        push(out, guard('case', labels[0] ?? null, false, text, stmt));
         return;
       }
       case 'switch_expression_arm': {
         if (!isField(parent, 'expression', child)) return;
-        const subject = condText(parent.parent?.childForFieldName('value'));
+        const stmt = parent.parent;
+        const subject = condText(stmt?.childForFieldName('value'));
         const pattern = parent.childForFieldName('pattern');
         const value = condText(pattern);
         const text = value === '_' || value === '' ? (subject ? `${subject}: default` : 'default') : subject ? `${subject} == ${value}` : value;
-        push(out, guard('case', pattern, false, text));
+        push(out, guard('case', pattern, false, text, stmt));
         return;
       }
       case 'binary_expression': {
@@ -1707,14 +1810,14 @@ const GO: Rules = {
         const stmt = parent.parent;
         const subject = condText(stmt?.childForFieldName('value'));
         const v = condText(value);
-        if (parent.type === 'communication_case') push(out, guard('case', value, false, v));
-        else push(out, guard('case', value, false, subject ? `${subject} == ${v}` : v));
+        if (parent.type === 'communication_case') push(out, guard('case', value, false, v, stmt));
+        else push(out, guard('case', value, false, subject ? `${subject} == ${v}` : v, stmt));
         return;
       }
       case 'default_case': {
         const stmt = parent.parent;
         const subject = condText(stmt?.childForFieldName('value'));
-        push(out, guard('case', stmt?.childForFieldName('value'), false, subject ? `${subject}: default` : 'default'));
+        push(out, guard('case', stmt?.childForFieldName('value'), false, subject ? `${subject}: default` : 'default', stmt));
         return;
       }
       case 'binary_expression': {
@@ -1792,10 +1895,10 @@ const C: Rules = {
         if (value && child.id === value.id) return;
         const stmt = parent.parent?.parent;
         const subject = condText(stmt?.childForFieldName('condition'));
-        if (!value) push(out, guard('case', stmt?.childForFieldName('condition'), false, subject ? `${subject}: default` : 'default'));
+        if (!value) push(out, guard('case', stmt?.childForFieldName('condition'), false, subject ? `${subject}: default` : 'default', stmt));
         else {
           const v = condText(value);
-          push(out, guard('case', value, false, subject ? `${subject} == ${v}` : v));
+          push(out, guard('case', value, false, subject ? `${subject} == ${v}` : v, stmt));
         }
         return;
       }

+ 23 - 7
src/ui-server/api/when.ts

@@ -23,6 +23,7 @@ import {
   siteKey,
   supportsBranchGuards,
   triggersForFile,
+  type BranchGuard,
   type CallSiteText,
   type DefinitionDecorators,
   type SiteTrigger,
@@ -98,6 +99,13 @@ export async function annotateWhen(cg: CodeGraph, projectRoot: string, batches:
 export interface SiteReader {
   /** The conditions the site runs under, joined; '' when unconditional or unreadable. */
   when(caller: { filePath: string; language: Language }, site: { line?: number; column?: number }): Promise<string>;
+  /**
+   * The same conditions, outermost first, unjoined — each with the branching
+   * construct it belongs to, so two sites can be told to be the two arms of
+   * ONE `if` rather than two conditions that happen to read as opposites.
+   * What {@link SiteReader.when} joins; empty when unconditional or unreadable.
+   */
+  guards(caller: { filePath: string; language: Language }, site: { line?: number; column?: number }): Promise<BranchGuard[]>;
   /** What the site passes, abbreviated (`'userEmail', values.email`); null when unreadable. '' for an empty list. */
   args(caller: { filePath: string; language: Language }, site: { line?: number; column?: number }): Promise<string | null>;
   /** What fires the site — the JSX prop, `on*` option or runs-later call it is written under; null when nothing binds it. */
@@ -149,15 +157,23 @@ export function createSiteReader(cg: CodeGraph, projectRoot: string, maxSites =
     }
     return file;
   };
+  // Named rather than a method, because `createWhenReader` hands `when` out
+  // detached: it must not depend on `this`.
+  const guards = async (
+    caller: { filePath: string; language: Language },
+    site: { line?: number; column?: number }
+  ): Promise<BranchGuard[]> => {
+    if (!site.line || sites >= maxSites || !supportsBranchGuards(caller.language)) return [];
+    const file = resolve(caller);
+    if (!file) return [];
+    sites++;
+    const key = { line: site.line, column: typeof site.column === 'number' ? site.column : null };
+    return (await guardsForFile(file.abs, file.language, [key])).get(siteKey(key)) ?? [];
+  };
   return {
+    guards,
     async when(caller, site) {
-      if (!site.line || sites >= maxSites || !supportsBranchGuards(caller.language)) return '';
-      const file = resolve(caller);
-      if (!file) return '';
-      sites++;
-      const key = { line: site.line, column: typeof site.column === 'number' ? site.column : null };
-      const g = (await guardsForFile(file.abs, file.language, [key])).get(siteKey(key));
-      return g ? guardLabel(g) : '';
+      return guardLabel(await guards(caller, site));
     },
     async args(caller, site) {
       if (!site.line || sites >= maxSites || !supportsBranchGuards(caller.language)) return null;