diff --git a/plugins/axiomcode/skills/axiomcode/scripts/ax_registration.py b/plugins/axiomcode/skills/axiomcode/scripts/ax_registration.py index 79ac61e3a..cf31d10a2 100644 --- a/plugins/axiomcode/skills/axiomcode/scripts/ax_registration.py +++ b/plugins/axiomcode/skills/axiomcode/scripts/ax_registration.py @@ -82,28 +82,7 @@ def registrations(q, site_file=None): # the cap and joins two unrelated tests. The DEPENDENT row is still true and still printed — a test that mounts a # handler does depend on it — so only the key is withheld. tf = {f for (f,) in q("SELECT DISTINCT file FROM symbols WHERE is_test = 1 AND file IS NOT NULL")} if _has(q, 'symbols') else set() - out = {} - verbs = {} # (file, line) -> {HTTP verb: first column}, for the wording below - for name, site_kind, fp, a, b, col in q("""SELECT callee_name, kind, file_path, start_line, end_line, end_column FROM call_sites - WHERE callee_name IS NOT NULL AND start_line > 0"""): - f = sf(fp); b = b or a - if b < a or b - a > MAX_SITE_SPAN: - continue - if site_kind == 'DECORATOR_CALL': - continue # a decoration is reported as a decoration, not as a registration - short = (name or '').split('.')[-1] - paths = [v for l in range(a, b + 1) for v in lits.get((f, l), ()) if isinstance(v, str) and v.startswith('/')] - if short.lower() in ROUTE_VERB and paths: - kind, key = 'route', ('' if f in tf else paths[0]) - why = f'registered as a {short.upper()} route "{paths[0]}" here — the router calls it, no call site does' - else: - continue # no evidence that this call does anything with a declaration named here - for l in range(a, b + 1): - # a route beats a plain callback on the same line: `router.use('/x', wrap(handler))` is a route site - if (f, l) not in out or kind == 'route': - out[(f, l)] = (kind, key, why) - if short.lower() in HTTP_VERB and (l == b or a == b): - vs = verbs.setdefault((f, l), {}); vs[short.upper()] = min(vs.get(short.upper(), col or 0), col or 0) + out, verbs, _own = _route_links(q, sf, lits, tf) # `router.route('/').get(h)` is two route-shaped calls on one line, and `route` said nothing about the method: the # line's HTTP verbs name it, in the order written. A chain registering two (`.get(a).post(b)`) names both, because # a line is all a reference records and it cannot say which argument list the handler sat in @@ -115,6 +94,83 @@ def registrations(q, site_file=None): return sorted((ref_at.get((f, l), ''), f, l, k, key, w) for (f, l), (k, key, w) in out.items()) +def route_site_lines(q, site_file=None): + """{call_site_id: line} — the line a CHAINED route link's own part is written on, for each link after the first. + + A call site's start line is where its receiver starts, so the engine places the handler of `.post('/c', h3)` in + `router.get('/a', h1)\\n.get('/b', h2)\\n.post('/c', h3)` on the chain's first line, where the registration rows say + GET "/a". The `calls` fact and the SQL rows place an edge at one of these sites on this line instead, the line + `registrations()` labels with the link's own verb and path. Only route links move; every other site keeps its line. + """ + if not _has(q, 'call_sites'): + return {} + lits = {} + if _has(q, 'literals'): + for v, f, l in q("SELECT value, file, line FROM literals WHERE line > 0 AND value IS NOT NULL"): + lits.setdefault((f, l), []).append(v) + return _route_links(q, site_file or (lambda x: x), lits, set())[2] + + +def _route_links(q, sf, lits, tf): + """({(file, line): (kind, key, why)}, {(file, line): {HTTP verb: column}}, {call_site_id: own line}) for every + route registration site. + + A CHAINED REGISTRATION `router.get('/a', h1).get('/b', h2).post('/c', h3)` is three call sites that all START + where `router` starts: each one's span covers its receiver, i.e. every call before it in the chain. Reading a + site's verb and path over its whole span gave each link the first link's path, and whichever link was read last + claimed every line. A link's OWN part begins where its receiver ends, so the chain is walked innermost first and + each link takes the first path its own part holds that an earlier link has not taken; a link with no path of its + own (`router.route('/p').get(h).put(h2)`) keeps its receiver's, which is what that API means. + """ + sites = [] + for sid, name, site_kind, fp, a, ac, b, bc in q("""SELECT id, callee_name, kind, file_path, start_line, start_column, + end_line, end_column FROM call_sites + WHERE callee_name IS NOT NULL AND start_line > 0"""): + b = b or a + if b < a or b - a > MAX_SITE_SPAN: + continue + if site_kind == 'DECORATOR_CALL': + continue # a decoration is reported as a decoration, not as a registration + sites.append((sf(fp), a, ac, b, bc, (name or '').split('.')[-1], sid)) + chains = {} + for s in sites: + # without a start column two calls on one line cannot be told apart from a chain: each is its own chain + chains.setdefault((s[0], s[1], s[2]) if s[2] is not None else s, []).append(s) + out, owned, own_line = {}, set(), {} + verbs = {} # (file, line) -> {HTTP verb: first column}, for registrations' wording + for links in chains.values(): + links.sort(key=lambda s: (s[3], s[4] or 0)) + taken, prev_end, prev_path = set(), None, None + for f, a, _ac, b, col, short, sid in links: + lo = a if prev_end is None else prev_end + own = [(l, i, v) for l in range(lo, b + 1) for i, v in enumerate(lits.get((f, l), ())) + if isinstance(v, str) and v.startswith('/') and (l, i) not in taken] + if own: + l0, i0, path = own[0] + taken.add((l0, i0)) + else: + # no path of its own: the link's own part starts on the line after its receiver's, when it has one + l0, path = (min(lo + 1, b) if prev_end is not None else a), prev_path + chained, prev_end, prev_path = prev_end is not None, b, path + if short.lower() not in ROUTE_VERB or path is None: + continue # no evidence that this call does anything with a declaration named here + if chained: + own_line[sid] = l0 + kind, key = 'route', ('' if f in tf else path) + why = f'registered as a {short.upper()} route "{path}" here — the router calls it, no call site does' + # a line shared by two links belongs to the verb whose own part starts on it, else to the outer link: on a + # chain written on one line nothing tells the links apart, and the line keeps the last verb it always had. + # `route(p)` registers no handler, so it never holds a line against the verbs chained onto it + owns = short.lower() != 'route' and (not chained or l0 > lo) + for l in range(lo, b + 1): + if (f, l) not in owned: + out[(f, l)] = (kind, key, why) + if owns and l == l0: owned.add((f, l)) + if short.lower() in HTTP_VERB and (l == b or lo == b): + vs = verbs.setdefault((f, l), {}); vs[short.upper()] = min(vs.get(short.upper(), col or 0), col or 0) + return out, verbs, own_line + + # A CONST HOLDING A WRAPPED HANDLER — `const h = catchAsync(async (req, res) => …)`, then `router.get('/a', h)` — is a # field, not a method, so it has no call edge of its own to carry the route wording. What both backends join instead # (`const_route` in dl/impact.dl, graph_sql.direct_for_field) is two facts read from the source at the call sites the diff --git a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact index eac4612c6..4dbf8455c 100755 --- a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact +++ b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-impact @@ -736,7 +736,7 @@ class Impact: return [ln for ln in range(s['line'], min(s['end_line'], len(L)) + 1) if pat.search(L[ln - 1])] # ── facts: the graph, exported once (reused while graph.sqlite is unchanged) ──────────────────────────────── - IMPACT_VERSION = '38' # 38: reg_key_fact drops a decoration string with a space in it (a description, not a key); 37: via_base / via_site, a caller that reaches a declaration through a base it is override-equivalent to (#1542), and injected_bean, the bean an injection point was wired to (#1384); 36: handoff_at, route_arg, callable_const, init_wrapper, init_alias, returns_fn — a const holding a wrapped handler registered at a route; 35: 0.1.5's 33 (#1598, the defines edges the path export links) joined 0.1.6's 33, two different fact sets under one number; 33 (0.1.6): calls carries the tier "stub" for a call inside a mock's stub or verification, reg_verb / lit_verb join a route by its HTTP method, and a handler's route joins its type's prefix; 32: cert_tier's why is worded per tier (an event_dispatch row says it sends the request or event), and the route facts #1633 changed (#1510), which merged without a bump; 31: event_dispatch edges (a published event reaches its listeners, #1391) and the pytest fixture_injection reading (#1631) change impact's facts; 30: registers, a bean another class's annotation registers (#1396, #1414); 29: the edges it links from the path export changed (#1402), and a cache written before it must not survive; 28: reexport_from, the file an `export *` line re-exports; 27: framework, the engine's framework_edge (#1509); 24: the test* naming convention requires a test class as owner (#1181); 23: owner/member disambiguated by file, two classes of one name no longer merging (#1188); 22: lex_parent, the innermost declaration enclosing each one by span (#1183); 21: cert_tier, the tier -> certainty table the call rules join on (#1131); 20: faccess, the engine's resolved field accesses (#1071); 3: decl_file facts (the import-time test-file rule); 14: the registration-key + IMPACT_VERSION = '39' # 39: a chained route link's `calls` row and `registration` label sit on the link's own line, with its own verb and path; 38: reg_key_fact drops a decoration string with a space in it (a description, not a key); 37: via_base / via_site, a caller that reaches a declaration through a base it is override-equivalent to (#1542), and injected_bean, the bean an injection point was wired to (#1384); 36: handoff_at, route_arg, callable_const, init_wrapper, init_alias, returns_fn — a const holding a wrapped handler registered at a route; 35: 0.1.5's 33 (#1598, the defines edges the path export links) joined 0.1.6's 33, two different fact sets under one number; 33 (0.1.6): calls carries the tier "stub" for a call inside a mock's stub or verification, reg_verb / lit_verb join a route by its HTTP method, and a handler's route joins its type's prefix; 32: cert_tier's why is worded per tier (an event_dispatch row says it sends the request or event), and the route facts #1633 changed (#1510), which merged without a bump; 31: event_dispatch edges (a published event reaches its listeners, #1391) and the pytest fixture_injection reading (#1631) change impact's facts; 30: registers, a bean another class's annotation registers (#1396, #1414); 29: the edges it links from the path export changed (#1402), and a cache written before it must not survive; 28: reexport_from, the file an `export *` line re-exports; 27: framework, the engine's framework_edge (#1509); 24: the test* naming convention requires a test class as owner (#1181); 23: owner/member disambiguated by file, two classes of one name no longer merging (#1188); 22: lex_parent, the innermost declaration enclosing each one by span (#1183); 21: cert_tier, the tier -> certainty table the call rules join on (#1131); 20: faccess, the engine's resolved field accesses (#1071); 3: decl_file facts (the import-time test-file rule); 14: the registration-key # layer; 15: the registration facts (two 14s landed independently, which is exactly the collision this # guards); 16: regsite folded into ax_registration's reg_key_fact; 20: implements_pair (#1011); 17/18: the tagged-template test registrar # (it.each`…`) and its table span @@ -912,7 +912,9 @@ class Impact: for t in {r[0] for r in g.q("SELECT DISTINCT tier FROM call_edges")} | {ax_edges.STUB_TIER}})) # a call written inside a mock's stub or verification keeps its row and loses its tier: "stub" (ax_edges.stub_sites) stubs = ax_edges.stub_sites(lambda s, p: g.q(s, *p)) if g.has('call_sites') else set() - W('calls', [(r['caller_id'], r['callee_method_id'], ax_edges.STUB_TIER if r['call_site_id'] in stubs else r['tier'], g.site_file(r['file_path']) if r['file_path'] else '', r['start_line'] or 0) + # a chained route link's edge sits on the line of its own verb and path, where `registration` labels it + link_line = ax_registration.route_site_lines(g.q, g.site_file) + W('calls', [(r['caller_id'], r['callee_method_id'], ax_edges.STUB_TIER if r['call_site_id'] in stubs else r['tier'], g.site_file(r['file_path']) if r['file_path'] else '', link_line.get(r['call_site_id'], r['start_line'] or 0)) for r in g.q("SELECT e.call_site_id, e.caller_id, e.callee_method_id, e.tier, s.file_path, s.start_line FROM call_edges e LEFT JOIN call_sites s ON s.id = e.call_site_id WHERE e.callee_provenance = 'client' AND e.callee_method_id IS NOT NULL")]) # a hand-off on EVERY line its call site spans: a chained registration (`router\n .route('/')\n .post(auth(), ctrl.h)`) # records its edges on the statement's first line, while the handler is named on a later one diff --git a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path index 1a9ebafe0..35ad5c3a6 100755 --- a/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path +++ b/plugins/axiomcode/skills/axiomcode/scripts/axiomcode-path @@ -1220,17 +1220,25 @@ def closure(g, sel, upstream, limit=40, depth=40): # which is not the line that makes the call, and a reader checking the hop wants the second. For a # direct caller that line is knowable exactly, so print it; beyond one hop there is no single site. qmark = ','.join('?' * len(ids)) + # a chained route link (`router.get('/a', h1).post('/b', h2)`) is written on its own line, not its receiver's + link_line = {} + if any(d == 1 for _, d in np_[:limit]): + try: + import ax_registration + link_line = ax_registration.route_site_lines(g.q, g.site_file) + except Exception: + pass for m, d in np_[:limit]: at = '' if d == 1: - r = g.q("SELECT s.file_path f, s.start_line ln FROM call_edges e JOIN call_sites s ON s.id = e.call_site_id " - f"WHERE e.caller_id = ? AND e.callee_method_id IN ({qmark}) AND s.start_line IS NOT NULL " - "ORDER BY s.start_line LIMIT 1", m, *ids) + r = g.q("SELECT s.id sid, s.file_path f, s.start_line ln FROM call_edges e JOIN call_sites s ON s.id = e.call_site_id " + f"WHERE e.caller_id = ? AND e.callee_method_id IN ({qmark}) AND s.start_line IS NOT NULL", m, *ids) if r: - sf = g.site_file(r[0]['f']) + ln, f = min((link_line.get(x['sid'], x['ln']), x['f'] or '') for x in r) + sf = g.site_file(f) # the call is nearly always written in the caller's own file, and repeating a 90-character # path to say so pushes the name off the line. Same file, just the line. - at = f" — calls it at {':' if sf == (g.sym[m]['file'] or '') else sf + ':'}{r[0]['ln']}".replace('at ::', 'at :') + at = f" — calls it at {':' if sf == (g.sym[m]['file'] or '') else sf + ':'}{ln}".replace('at ::', 'at :') print(f" {d:2} hop(s) {g.disp(m)} {g.loc(m)}{at}") if len(np_) > limit: print(f" … +{len(np_) - limit} production (--limit N)") if nt: print(f" +{len(nt)} test caller(s) within 2 hop(s) — `axiomcode test-impact` names them and the command that runs them") diff --git a/plugins/axiomcode/skills/axiomcode/scripts/graph_sql.py b/plugins/axiomcode/skills/axiomcode/scripts/graph_sql.py index bb3285a33..d0c59fde7 100644 --- a/plugins/axiomcode/skills/axiomcode/scripts/graph_sql.py +++ b/plugins/axiomcode/skills/axiomcode/scripts/graph_sql.py @@ -1107,11 +1107,14 @@ def direct_for_method(q, ids, code=None, rel=None, at=None, lines=None, via=None # the lowest. Leaving the order to the table printed a different site (254 against 257) for the same caller. # a site inside a mock's stub or verification carries the tier "stub", exactly as the rules' `calls` fact does stubs = ax_edges.stub_sites(lambda s_, p_: q(s_, *p_)) - raw = [(c, m, ax_edges.STUB_TIER if sid in stubs else t, f, l) for sid, c, m, t, f, l in q( + # a chained route link's edge sits on the line of its own verb and path, as in the `calls` fact + link_line = ax_registration.route_site_lines(q, rel) + raw = [(c, m, ax_edges.STUB_TIER if sid in stubs else t, f, link_line.get(sid, l)) for sid, c, m, t, f, l in q( f"""SELECT e.call_site_id, e.caller_id, e.callee_method_id, e.tier, s.file_path, s.start_line FROM call_edges e LEFT JOIN call_sites s ON s.id=e.call_site_id WHERE e.callee_method_id IN ({ph}) AND e.callee_provenance='client' ORDER BY s.start_line""", *ids)] + if link_line: raw.sort(key=lambda r: r[4] or 0) sites = [(c, t, f, l) for c, _m, t, f, l in raw] bean_callers, why_of = _bean_call(q, ids, sites) # a call written against a base this declaration is override-equivalent to (#1542): its row replaces the plain one diff --git a/tests/cases/javascript/chained-route-verbs/case.json b/tests/cases/javascript/chained-route-verbs/case.json new file mode 100644 index 000000000..6d0850b51 --- /dev/null +++ b/tests/cases/javascript/chained-route-verbs/case.json @@ -0,0 +1,37 @@ +{"lang": "javascript", "src": "src", + "checks": [ + {"why": "each link of a chained registration `router.get(p1, a).get(p2, b).post(p3, c)` spans its receiver, so every link used to be read with the first link's verb and path: newPost said GET \"/posts\" at the chain's first line", + "run": ["impact", "newPost", "--kind", "method"], + "want": ["registered as a GET route \"/posts/new\"", "src/posts.js:15"], + "avoid": ["GET route \"/posts\" "]}, + {"why": "the last link of the chain is a POST, written on its own line", + "run": ["impact", "createPost", "--kind", "method"], + "want": ["registered as a POST route \"/posts\"", "src/posts.js:16"], + "avoid": ["GET route"]}, + {"why": "CONTROL: the chain's first link said its own verb and path before and still does", + "run": ["impact", "listPosts", "--kind", "method"], + "want": ["registered as a GET route \"/posts\"", "src/posts.js:14"], + "avoid": ["POST route", "/posts/new"]}, + {"why": "`router.route(p)` written over several lines: its verbs have no path of their own, so each keeps route()'s path under its own verb, on its own line, and route() itself is never the label", + "run": ["impact", "showPost", "--kind", "method"], + "want": ["registered as a GET route \"/posts/:id\"", "src/posts.js:18"], + "avoid": ["ROUTE route"]}, + {"why": "…and the chain's other verb", + "run": ["impact", "updatePost", "--kind", "method"], + "want": ["registered as a PUT route \"/posts/:id\"", "src/posts.js:19"], + "avoid": ["ROUTE route"]}, + {"why": "CONTROL: a plain, unchained registration is unchanged", + "run": ["impact", "listTags", "--kind", "method"], + "want": ["registered as a GET route \"/tags\"", "src/posts.js:20"]}, + {"why": "CONTROL: a chain on ONE line cannot be split by line, so its line names every verb on it with route()'s path, as it did before", + "run": ["impact", "renameTag", "--kind", "method"], + "want": ["registered as a GET/PUT route \"/tags/:id\"", "src/posts.js:21"], + "avoid": ["ROUTE route"]}, + {"why": "`path` names the line a direct caller makes the call on: a chained link's own line, not the chain's first", + "run": ["path", "*", "createPost"], + "want": ["calls it at :16"], + "avoid": ["calls it at :14"]}, + {"why": "CONTROL: `path` for the chain's first link still names the first line", + "run": ["path", "*", "listPosts"], + "want": ["calls it at :14"]} + ]} diff --git a/tests/cases/javascript/chained-route-verbs/src/posts.js b/tests/cases/javascript/chained-route-verbs/src/posts.js new file mode 100644 index 000000000..154cf29e7 --- /dev/null +++ b/tests/cases/javascript/chained-route-verbs/src/posts.js @@ -0,0 +1,23 @@ +'use strict'; +const express = require('express'); + +function listPosts(req, res) { res.json([]); } +function newPost(req, res) { res.send('form'); } +function createPost(req, res) { res.status(201).end(); } +function showPost(req, res) { res.json({}); } +function updatePost(req, res) { res.end(); } +function listTags(req, res) { res.json([]); } +function showTag(req, res) { res.json({}); } +function renameTag(req, res) { res.end(); } + +const router = express.Router(); +router.get('/posts', listPosts) + .get('/posts/new', newPost) + .post('/posts', createPost); +router.route('/posts/:id') + .get(showPost) + .put(updatePost); +router.get('/tags', listTags); +router.route('/tags/:id').get(showTag).put(renameTag); + +module.exports = router;