Merge pull request #1774 from AxiomCodeAI/loop/ja-is1758

javascript: a request handing over a call, an await or a const is not a route registration

Author: swapnilpaliwal-sd

plugins/axiomcode/skills/axiomcode/scripts/ax_registration.py

@@ -156,8 +156,10 @@ def _route_links(q, sf, lits, tf):
                 mine = [(s, l) for s, _t, l in args if s]
                 own = [(l, i, v) for l, i, v in own if (v, l) in mine][:1] or [(l, -1, s) for s, l in mine[:1]]
                 if handlers is None:
-                    handlers = _handler_names(q)
-                if short.lower() in HTTP_VERB and not any(_hands_over(t, handlers, f, read(f)) for s, t, _l in args if not s):
+                    handlers = _Handlers(q, read, sf)
+                hands = _any3(_hands_over(t, handlers, f, read(f)) for s, t, _l in args if not s) \
+                    if short.lower() in HTTP_VERB else True
+                if hands is False or (hands is None and _used_as_promise(read(f), a, _ac, b, col)):
                     prev_end, prev_path = b, None
                     continue               # a request (a client, a Map, Headers): nothing is registered here
             if own:
@@ -279,50 +281,118 @@ def _own_args(L, a, ac, b, bc):
                  'enum_member', 'property', 'local'}
 
 
-def _handler_names(q):
-    """name -> {kind: {file}} of every declaration, for telling a handler handed over by name from the request's data"""
-    names = {}
-    if _has(q, 'symbols'):
-        for n, k, f in q("SELECT name, kind, file FROM symbols WHERE name IS NOT NULL"):
-            names.setdefault(n, {}).setdefault(k or '', set()).add(f)
-    return names
-
-
-def _names_handler(t, names, f, L):
+class _Handlers:
+    """What a route call's argument is checked against: `names` (name -> {kind: {file}} of every declaration), the
+    names of callables the engine says return a function, and — read once per const, from its initializer — whether a
+    const holds a function (`const h = catchAsync(…)`) or data (`const opts = { headers: {} }`)."""
+
+    def __init__(self, q, read, sf):
+        self.q, self.read, self.sf, self.names, self.consts, self.held = q, read, sf, {}, {}, {}
+        if _has(q, 'symbols'):
+            for n, k, f, l in q("SELECT name, kind, file, line FROM symbols WHERE name IS NOT NULL"):
+                self.names.setdefault(n, {}).setdefault(k or '', set()).add(f)
+                if k == 'const' and l: self.consts.setdefault(n, set()).add((f, l))
+        self.returns_fn = set()
+        if _has(q, 'symbols'):
+            for w, _m in returned_functions(q):
+                self.returns_fn.update(n for (n,) in q("SELECT name FROM symbols WHERE method_id = ? AND name IS NOT NULL", w))
+
+    def holds_fn(self, n, files=None):
+        """a const `n` (declared in one of `files`, or anywhere) holds a function; kept when no declaration of it can be
+        read, as before its initializer was looked at"""
+        decls = sorted((f, l) for f, l in self.consts.get(n, ()) if files is None or f in files)
+        readable = [(f, l, n) for f, l in decls if self.read(f)]
+        if not readable:
+            return True
+        key = tuple(readable)
+        if key not in self.held:
+            # a module a const requires may export the handler itself: `const h = require('./h'); app.get('/x', h)`
+            required = any(re.search(rf'(?<![\w$.]){re.escape(n)}\s*=\s*require\s*\(', (self.read(f) or [''])[l - 1])
+                           for f, l, _n in readable if 0 < l <= len(self.read(f)))
+            fns, _w, _a = const_values(self.q, self.read, readable, self.sf)
+            self.held[key] = required or bool(fns)
+        return self.held[key]
+
+
+def _names_handler(t, h, f, L):
     """a written name `h` or `a.b.h` is a handler: `h` a function or method; a member of a module this file imports
-    (`users.signup`, `exports.signup = …` declares nothing); `this.h` a property of the class; or a const (a wrapped
-    handler, `const h = catchAsync(…)`) this file declares or imports. The same name as a parameter, a local, a field
-    of a type or some other module's `data` (`api.post('/discussions', data)`) is the request's data."""
+    (`users.signup`, `exports.signup = …` declares nothing); `this.h` a property of the class; or a const holding a
+    function (a wrapped handler, `const h = catchAsync(…)`) this file declares or imports. The same name as a parameter,
+    a local, a field of a type, some other module's `data` (`api.post('/discussions', data)`) or a const holding data
+    (`const opts = { headers: {} }`, `cfg.options`) is the request's data."""
     parts = re.split(r'\s*\??\.\s*', t)
-    kinds = names.get(parts[-1], {})
+    kinds = h.names.get(parts[-1], {})
     if any(k not in _NOT_CALLABLE for k in kinds):
         return True
     here = parts[0]
+    # a call site's file is relative to the indexed source root and a declaration's to the repository (`--src src`:
+    # `requests.js` and `src/requests.js`), so "this file" is the declaration's file ending in the site's
+    mine = lambda fs: {x for x in fs if f and (x == f or x.endswith('/' + f))}
     if len(parts) > 1 and here == 'this':
-        return any(f in fs for k, fs in kinds.items() if k in ('const', 'field', 'property'))
+        return any(mine(fs) for k, fs in kinds.items() if k in ('const', 'field', 'property'))
     n = re.escape(here)
     imported = bool(re.search(r'\bimport\b[^;]*?\b%s\b[^;]*?\bfrom\b|\b%s\b[^=;\n]*=\s*require\s*\(' % (n, n),
                               '\n'.join(L or ())))
     if len(parts) > 1:
-        return imported
-    return f in kinds.get('const', ()) or (imported and 'const' in kinds)
+        return imported and ('const' not in kinds or h.holds_fn(parts[-1]))
+    if mine(kinds.get('const', ())):
+        return h.holds_fn(parts[-1], mine(kinds['const']))
+    return imported and 'const' in kinds and h.holds_fn(parts[-1])
+
+
+def _any3(vs):
+    """True if one is, else None if one is unknown, else False"""
+    vs = list(vs)
+    return True if True in vs else (None if None in vs else False)
 
 
-def _hands_over(t, names, f=None, L=None):
-    """the argument text can be something a router calls: a function, a name declared as a handler, a call (a wrapper or
-    a middleware factory), an array of those. A string, a number, an object, `new X()` or a name that is not one (a
-    parameter, a local: the request's data) is not."""
+def _hands_over(t, h, f=None, L=None):
+    """True when the argument text can be something a router calls: a function, a name declared as a handler, a call
+    that makes one (a wrapper handed a function or handler, a callable the engine says returns a function), an array of
+    those. False for the request's data: a string, a number, an object, `new X()`, an `await`, a name that is not a
+    handler (a parameter, a local, a const holding data), or a call of this repository's own callable handed only data
+    (`buildConfig()`: the engine read its body and saw no function returned). None for a library call handed only data:
+    `JSON.stringify(x)` and `swaggerUi.setup(specs)` read alike, and the call's own use tells them apart."""
     t = t.strip()
-    if not t or t[0] in '\'"`{' or t[0].isdigit() or re.match(r'(new|true|false|null|undefined)\b', t):
+    if not t or t[0] in '\'"`{' or t[0].isdigit() or re.match(r'(new|await|true|false|null|undefined)\b', t):
         return False
     if t.startswith('...') or _fn_literal(t):
         return True
     if t.startswith('[') and t.endswith(']'):
         b = _blank(t)
-        return any(_hands_over(t[lo:hi], names, f, L) for lo, hi in _split_args(b, 1, len(b) - 1))
+        return _any3(_hands_over(t[lo:hi], h, f, L) for lo, hi in _split_args(b, 1, len(b) - 1))
     if _CHAIN.fullmatch(t):
-        return _names_handler(t, names, f, L)
-    return True                           # a call, a conditional, an `await`: not read further, and kept
+        return _names_handler(t, h, f, L)
+    m = re.match(rf'({_IDENT}(?:\s*\??\.\s*{_IDENT})*)\s*\(', t)
+    if m:
+        # a call: `wrap(async (req, res) => …)`, `asyncHandler(listItems)`, curried `wrap(opts)(fn)` or a factory the
+        # engine says returns a function hands one over
+        callee = re.split(r'\s*\??\.\s*', m.group(1))[-1]
+        if callee in h.returns_fn:
+            return True
+        b, k = _blank(t), m.end() - 1
+        while k < len(t) and b[k] == '(':
+            e = _match(b, k)
+            if _any3(_hands_over(t[lo:hi], h, f, L) for lo, hi in _split_args(b, k + 1, e - 1)):
+                return True
+            k = e
+            while k < len(t) and t[k] in ' \t': k += 1
+        # only a bare `f(…)` is this repository's own by its name: `swaggerUi.setup(specs)` is the library's `setup`,
+        # whatever else of that name the repository declares
+        own = '.' not in m.group(1) and any(k_ not in _NOT_CALLABLE for k_ in h.names.get(callee, {}))
+        return False if own else None
+    return True                           # a conditional, an expression not read further: kept
+
+
+def _used_as_promise(L, a, ac, b, bc):
+    """the call's result is awaited or chained with `.then` / `.catch` / `.finally`: a request's promise — a router's
+    registration returns the router, never awaited or thenned"""
+    if not L or not ac or not bc or b > len(L) or a < 1:
+        return False
+    if re.search(r'\bawait\s*$', L[a - 1][:ac - 1]):
+        return True
+    rest = '\n'.join([L[b - 1][bc - 1:]] + L[b:b + 2])
+    return bool(re.match(r'\s*\??\.\s*(then|catch|finally)\s*\(', rest))
 
 
 # A CONST HOLDING A WRAPPED HANDLER — `const h = catchAsync(async (req, res) => …)`, then `router.get('/a', h)` — is a

tests/cases/javascript/http-client-is-not-a-route/case.json

@@ -34,5 +34,43 @@
   {"why": "`api.patch(`/users/profile`, data)` hands over a parameter that shares its name with another module's const: data, not a handler",
    "run": ["impact", "readProfile", "--kind", "method"],
    "want": ["updateProfile", "src/api.js:5"],
-   "avoid": ["route"]}
+   "avoid": ["route"]},
+  {"why": "`axios.get('/api/f1', buildConfig())` hands over a call building data, not a wrapper handed a function",
+   "run": ["impact", "renderA", "--kind", "method"],
+   "want": ["f1", "src/requests.js:14"],
+   "avoid": ["route"]},
+  {"why": "`axios.get('/api/f2', opts)` hands over a const this file declares, and it holds an object, not a function",
+   "run": ["impact", "renderB", "--kind", "method"],
+   "want": ["f2", "src/requests.js:15"],
+   "avoid": ["route"]},
+  {"why": "`axios.post('/api/f3', JSON.stringify(x))` hands over a library call handed only data",
+   "run": ["impact", "renderC", "--kind", "method"],
+   "want": ["f3", "src/requests.js:16"],
+   "avoid": ["route"]},
+  {"why": "`axios.post('/api/f8', await serialize(x))` hands over an awaited value",
+   "run": ["impact", "renderH", "--kind", "method"],
+   "want": ["f8", "src/requests.js:17"],
+   "avoid": ["route"]},
+  {"why": "`axios.get('/api/f9', cfg.options)` hands over an imported module's const that holds an object",
+   "run": ["impact", "renderI", "--kind", "method"],
+   "want": ["f9", "src/requests.js:18"],
+   "avoid": ["route"]},
+  {"why": "CONTROL: a wrapper call handed an inline async handler is still a GET route",
+   "run": ["impact", "showW", "--kind", "method"],
+   "want": ["registered as a GET route \"/w\"", "src/wrapped.js:15"]},
+  {"why": "CONTROL: a wrapper call handed a declared handler is still a GET route",
+   "run": ["impact", "listR", "--kind", "method"],
+   "want": ["registered as a GET route \"/r\"", "src/wrapped.js:16"]},
+  {"why": "CONTROL: a factory the engine says returns a function is still a GET route",
+   "run": ["impact", "makeHandler", "--kind", "method"],
+   "want": ["registered as a GET route \"/m\"", "src/wrapped.js:17"]},
+  {"why": "CONTROL: a const holding a wrapped handler is still a GET route",
+   "run": ["impact", "guarded"],
+   "want": ["registered as a GET route \"/g\"", "src/wrapped.js:18"]},
+  {"why": "CONTROL: a const that requires a module exporting the handler is still a GET route",
+   "run": ["impact", "showItem"],
+   "want": ["registered as a GET route \"/s\"", "src/wrapped.js:19"]},
+  {"why": "CONTROL: a library factory handed only data (`swaggerUi.setup(specs, …)`) reads like `JSON.stringify(x)`, but nothing awaits or thens this call: still a GET route",
+   "run": ["impact", "logDocs", "--kind", "method"],
+   "want": ["registered as a GET route \"/docs\"", "src/wrapped.js:20"]}
  ]}

tests/cases/javascript/http-client-is-not-a-route/src/cfg.js

@@ -0,0 +1 @@
+export const options = { timeout: 1 };

tests/cases/javascript/http-client-is-not-a-route/src/lib/setup.js

@@ -0,0 +1,6 @@
+'use strict';
+
+// the repository's own `setup`: `swaggerUi.setup(…)` is still the library's
+function setup() { return {}; }
+
+module.exports = { setup };

tests/cases/javascript/http-client-is-not-a-route/src/lib/show.js

@@ -0,0 +1,3 @@
+'use strict';
+
+module.exports = function showItem(req, res) { res.json({}); };

tests/cases/javascript/http-client-is-not-a-route/src/requests.js

@@ -0,0 +1,18 @@
+import axios from 'axios';
+import * as cfg from './cfg';
+
+function buildConfig() { return {}; }
+async function serialize(x) { return x; }
+const opts = { headers: {} };
+
+export function renderA(r) { return r; }
+export function renderB(r) { return r; }
+export function renderC(r) { return r; }
+export function renderH(r) { return r; }
+export function renderI(r) { return r; }
+
+export function f1() { return axios.get('/api/f1', buildConfig()).then(renderA); }
+export function f2() { return axios.get('/api/f2', opts).then(renderB); }
+export function f3(x) { return axios.post('/api/f3', JSON.stringify(x)).then(renderC); }
+export async function f8(x) { return axios.post('/api/f8', await serialize(x)).then(renderH); }
+export function f9() { return axios.get('/api/f9', cfg.options).then(renderI); }

tests/cases/javascript/http-client-is-not-a-route/src/wrapped.js

@@ -0,0 +1,22 @@
+'use strict';
+const express = require('express');
+const swaggerUi = require('swagger-ui-express');
+const showItem = require('./lib/show');
+
+function wrap(fn) { return (req, res, next) => fn(req, res).catch(next); }
+function makeHandler() { return (req, res) => res.end(); }
+function showW(req, res) { return res.json([]); }
+function listR(req, res) { return res.json([]); }
+function logDocs(ui) { return ui; }
+const guarded = wrap(async (req, res) => listR(req, res));
+const specs = { openapi: '3.0.0' };
+
+const app = express();
+app.get('/w', wrap(async (req, res) => showW(req, res)));
+app.get('/r', wrap(listR));
+app.get('/m', makeHandler());
+app.get('/g', guarded);
+app.get('/s', showItem);
+app.get('/docs', swaggerUi.setup(specs, { onComplete: logDocs }));
+
+module.exports = app;