Przeglądaj źródła

fix(steps): three things the real repos caught

Validating the reading against four real servers turned up three defects, all
in the walk and all visible in both readings:

- **A hop's span is only a hop's span when the call is the one we asked for.**
  An inline Express handler's edges carry the ROUTE's line — where the only
  call is `router.post('/users/login', async (req, res) => {` — so the read
  span covered the whole registration and every call in the handler counted as
  written inside it, and so as running first. express-realworld's login drew
  its 200 before the `login()` that produces it. A read that does not find the
  call it was asked for is now a bare position: no span, no `inside`.

- **A name-match the call as written disproves.** `crypto.createHash('sha256')
  .update(…)` in a Nest service kept only `update` in the index and matched it
  to the caller's own `AuthService.update` — and the login endpoint then read
  as though it updated the user, four extra replies and a session delete
  included. In this family a method of your own class is written `this.x(…)`,
  so a receiver that is not `this` proves the guess wrong; the call leaves the
  index instead. The endpoint goes from 15 steps to 6, all of them real.

- **A value with no calls of its own is lent the file's.** The gate counted any
  edge, and `const signIn = validatedAction(schema, async (data) => { … })`
  holds one plain `references` edge to its schema — so the whole server action
  went unlent and its picture had one call out of nine. Only edges the walk
  follows as behaviour count now, and next-saas-starter's `signIn` reads whole:
  the lookup, two early returns, `Promise.all` of session and activity log, and
  the redirect to /dashboard or the checkout session.

Also: an `elif` whose body raises does not mean the arm it is written in always
raises — FastAPI's `if not user: raise … elif not user.is_active: raise …` was
ending its own arm.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REFyW9hmNrxhwN5wxRoAkC
Colby McHenry 1 tydzień temu
rodzic
commit
676030314a

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

@@ -296,6 +296,20 @@ export async function save() {
     expect(one[0]!.branch).not.toBe(two[0]!.branch);
   });
 
+  it('does not call an arm an exit because a later elif raises', async () => {
+    const src = `
+def handler(user):
+    if not user:
+        raise HTTPException(400)
+    elif not user.is_active:
+        raise HTTPException(400)
+    go(user)
+`;
+    // The `elif` arm raises; the arm it is written in runs on to `go(user)`.
+    const after = await guardsInSource(src, 'python', lineOf(src, 'go(user)'), 4);
+    expect(after.map((g) => g.armExit ?? null)).toEqual(after.map(() => null));
+  });
+
   it('reads a Swift guard as an exit', async () => {
     const src = `
 func load() {

+ 2 - 2
src/graph/branch-guards.ts

@@ -1212,7 +1212,8 @@ function branchKey(node: SyntaxNode): string {
  * 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.
+ * is the wrapper, not the block. A CONDITIONAL wrapper is not one of them: an
+ * `elif` whose body raises does not mean the arm it is in always raises.
  */
 const BLOCKISH: ReadonlySet<string> = new Set([
   'statement_block',
@@ -1222,7 +1223,6 @@ const BLOCKISH: ReadonlySet<string> = new Set([
   'compound_statement',
   'control_structure_body',
   'else_clause',
-  'elif_clause',
   'else_statement',
   'catch_clause',
   'catch_block',

+ 42 - 5
src/ui-server/api/steps.ts

@@ -397,12 +397,20 @@ export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLS
     const line = at.line ?? caller.startLine;
     const column = at.column ?? 0;
     const read = at.line ? await callAt(caller, { ...at, ...(callee ? { callee } : {}) }) : null;
+    // The read must be THIS call. An inline handler's edges carry the ROUTE's
+    // line (`router.post('/users/login', async (req, res) => {`), where the
+    // only call is the registration itself — and taking its span would make
+    // every call in the handler read as written inside it, and so as running
+    // first. A miss is a bare position: no span to nest by, no `within`.
+    const last = (n: string) => n.replace(/\([^()]*\)/g, '').split(/[.:]/).pop() ?? n;
+    const usable = read !== null && (!callee || last(read.callee) === last(callee));
+    if (!usable) return { file: caller.filePath, line, column, end: { line, column }, within: null };
     return {
       file: caller.filePath,
-      line: read?.span?.start.line ?? line,
-      column: read?.span?.start.column ?? column,
-      end: read?.span?.end ?? { line, column },
-      within: read?.within ?? null,
+      line: read.span?.start.line ?? line,
+      column: read.span?.start.column ?? column,
+      end: read.span?.end ?? { line, column },
+      within: read.within ?? null,
     };
   };
   const pointHop = (caller: Node, at: { line?: number; column?: number }): HopSite => {
@@ -478,6 +486,19 @@ export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLS
     return members[0] ?? null;
   };
 
+  /**
+   * A name-match that the call as written disproves: the target is a method of
+   * the CALLER's own class, but the call names a receiver that is not `this` —
+   * and in the JS family a method of your own class cannot be called any other
+   * way. Only there: `self.` is a convention elsewhere, not a rule.
+   */
+  const ownMethodWithoutReceiver = (caller: Node, target: Node, written: string): boolean => {
+    if (!JS_FAMILY.has(caller.language) || target.kind !== 'method') return false;
+    if (/^(?:this|self)[.?]/.test(written)) return false;
+    const container = (q: string) => q.replace(/[.:]+[^.:]*$/, '');
+    return caller.filePath === target.filePath && container(caller.qualifiedName) === container(target.qualifiedName);
+  };
+
   /** A method the walk cannot enter (an interface's, an ORM's) on a repository-shaped container. */
   const repositoryMethod = (target: Node): boolean => {
     if (target.kind !== 'method' && target.kind !== 'function') return false;
@@ -892,7 +913,15 @@ export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLS
       // lands on the constant, and the walk goes on into what the handler does.
       for (const fold of frontier) {
         const value = fold.node.kind === 'constant' || fold.node.kind === 'variable';
-        if ((fold.node.kind !== 'component' && !value) || (bySource.get(fold.node.id)?.length ?? 0) > 0) continue;
+        // Edges the walk would follow as BEHAVIOUR. `const signIn =
+        // validatedAction(schema, async (data) => { … })` holds a plain
+        // `references` edge to its schema and nothing else: counting that as
+        // "it has edges of its own" left the whole handler body unlent, and the
+        // picture showed one call out of nine.
+        const behaviour = (bySource.get(fold.node.id) ?? []).filter(
+          (e) => e.kind !== 'references' || (e.metadata as Record<string, unknown> | undefined)?.fnRef === true
+        ).length;
+        if ((fold.node.kind !== 'component' && !value) || behaviour > 0) continue;
         for (const e of fileScopeEdgesWithin(cg, fold.node, fileScopeRefs, value)) {
           const list = bySource.get(fold.node.id) ?? [];
           list.push({ ...e, source: fold.node.id });
@@ -1025,6 +1054,14 @@ export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLS
                   if (real && real.id !== target.id) {
                     target = real;
                     retargeted = true;
+                  } else if (real === null && ownMethodWithoutReceiver(fold.node, target, written.callee)) {
+                    // `crypto.createHash('sha256').update(…)` in a method of a
+                    // class that happens to have an `update`: the index kept
+                    // only `update` and matched it by name. In this family a
+                    // method of your own class is written `this.update(…)`, so
+                    // a receiver that is not `this` proves the guess wrong. The
+                    // call leaves the index — say nothing rather than the wrong thing.
+                    continue;
                   }
                 }
               }