diff --git a/scripts/verify/rea-numeric-limits.mjs b/scripts/verify/rea-numeric-limits.mjs index ff7dcc95e6..f189bd0602 100644 --- a/scripts/verify/rea-numeric-limits.mjs +++ b/scripts/verify/rea-numeric-limits.mjs @@ -109,6 +109,53 @@ const boundLimitName = (node, bindings) => { return undefined; }; +const isPropertyAccess = (node) => + ts.isPropertyAccessExpression(node) || + ts.isPropertyAccessChain?.(node) === true; + +const isElementAccess = (node) => + ts.isElementAccessExpression(node) || + ts.isElementAccessChain?.(node) === true; + +/** + * Number(), parseInt(), and schema parse() read the value they are given. + * The accessed property or element is that read, through parentheses and + * other wrappers, so a suffixless name is still classified. + */ +const rememberNumericArgument = (discovered, path, node, bindings) => { + const expression = unwrapExpression(node); + if (isPropertyAccess(expression)) + remember(discovered, path, expression.name.text, expression, true); + else if (isElementAccess(expression)) + remember( + discovered, + path, + boundLimitName(expression.argumentExpression, bindings), + expression, + true, + ); + else + remember( + discovered, + path, + boundLimitName(expression, bindings), + expression, + true, + ); + ts.forEachChild(expression, (child) => + rememberNumericArgument(discovered, path, child, bindings), + ); +}; + +/** + * Generated source assigns a limit (`REA_NAME = ...`). A name mentioned in + * template prose is not a producer, matching an ordinary string. + */ +const assignedTemplateLimits = (chunk) => + [...chunk.matchAll(/(?:^|[^A-Za-z0-9_])(REA_[A-Z0-9_]+)\s*=(?!=)/gu)].map( + (match) => match[1], + ); + export const isNumericLimit = (name) => REQUIRED_RUNTIME_LIMITS.includes(name) || /(?:_BYTES|_MS|_MIB|_SECONDS|_POLLS|_JOBS|_ELEMENTS|_COUNT|_SESSIONS|_COMMANDS)$/u.test( @@ -131,7 +178,12 @@ const numericSchemaText = (node) => node.getText(), ); -const numericParse = (node) => { +const isParseAccess = (expression) => + isPropertyAccess(expression) && + (expression.name.text === "safeParse" || expression.name.text === "parse"); + +/** Number() and a numeric schema read their argument. Other parsers do not. */ +const parsesNumericArgument = (node) => { if (!ts.isCallExpression(node)) return false; const expression = unwrapExpression(node.expression); if ( @@ -141,10 +193,13 @@ const numericParse = (node) => { expression.text === "parseFloat") ) return true; - return ( - ts.isPropertyAccessExpression(expression) && - (expression.name.text === "safeParse" || expression.name.text === "parse") - ); + return isParseAccess(expression) && numericSchemaText(expression.expression); +}; + +const numericParse = (node) => { + if (!ts.isCallExpression(node)) return false; + const expression = unwrapExpression(node.expression); + return parsesNumericArgument(node) || isParseAccess(expression); }; const readLifecycle = (node) => { @@ -172,7 +227,9 @@ const remember = (discovered, path, name, node, force = false) => { * Producers are syntax, not text. A read is a property or element access, * including a const string used as the element, a string argument to a call, * an object-binding key (rename and default included), an `in` check, or an - * object key that a spread can copy. Comments and array literals are not reads. + * object key that a spread can copy. A numeric parser also reads a property or + * element access inside its argument. Comments, array literals, and template + * prose are not reads. A template is a producer only when it assigns the name. */ export const discoverNumericLimitUses = (files) => { const discovered = new Map(); @@ -201,15 +258,24 @@ export const discoverNumericLimitUses = (files) => { node, ); if (ts.isCallExpression(node) || ts.isNewExpression(node)) { - const force = numericParse(node); - for (const argument of node.arguments ?? []) - remember( - discovered, - relativePath, - boundLimitName(argument, bindings), - node, - force, - ); + const parsesNumber = parsesNumericArgument(node); + for (const argument of node.arguments ?? []) { + if (parsesNumber) + rememberNumericArgument( + discovered, + relativePath, + argument, + bindings, + ); + else + remember( + discovered, + relativePath, + boundLimitName(argument, bindings), + node, + numericParse(node), + ); + } } if ( ts.isBinaryExpression(node) && @@ -273,7 +339,7 @@ export const discoverNumericLimitUses = (files) => { ...node.templateSpans.map((span) => span.literal.text), ]; for (const chunk of chunks) - for (const token of chunk.match(/REA_[A-Z0-9_]+/gu) ?? []) + for (const token of assignedTemplateLimits(chunk)) remember(discovered, relativePath, token, node); } ts.forEachChild(node, visit); diff --git a/tests/evaluation/numericLimitCheck.test.ts b/tests/evaluation/numericLimitCheck.test.ts index d133d0f07a..be43f5a408 100644 --- a/tests/evaluation/numericLimitCheck.test.ts +++ b/tests/evaluation/numericLimitCheck.test.ts @@ -92,16 +92,73 @@ it("records a constructor read separately from a per-call read", () => { }); }); -it("treats a count parsed from the environment as numeric without a suffix", () => { +it("treats a count parsed from a property access as numeric without a suffix", () => { const name = "REA_ARCHIVE_ENTRY_LIMIT"; expect(isNumericLimit(name)).toBe(false); const discovered = discoverNumericLimitUses([ { path: "src/config/archiveLimit.ts", - source: `const limit = Number(process.env.${name});\nconst schema = { ${name}: z.number().int() };`, + source: `const limit = Number(process.env.${name});\nconst indexed = parseInt(environment["${name}"], 10);`, }, ]); - expect(discovered.get(name)?.path).toBe("src/config/archiveLimit.ts"); + expect(discovered.get(name)).toEqual({ + path: "src/config/archiveLimit.ts", + read: "call", + }); + expect( + discoverNumericLimitUses([ + { + path: "src/config/indexedArchiveLimit.ts", + source: `const indexed = Number(environment["${name}"]);`, + }, + ]).get(name), + ).toEqual({ path: "src/config/indexedArchiveLimit.ts", read: "call" }); + expect( + discoverNumericLimitUses([ + { + path: "src/config/schemaArchiveLimit.ts", + source: `const indexed = z.number().int().parse(process.env.${name});`, + }, + ]).get(name), + ).toEqual({ path: "src/config/schemaArchiveLimit.ts", read: "call" }); + expect( + discoverNumericLimitUses([ + { + path: "src/application/SetupHost.ts", + source: + "const requested = analysisProviderSelectorSchema.safeParse(environment.REA_ANALYSIS_PROVIDER);", + }, + ]).has("REA_ANALYSIS_PROVIDER"), + ).toBe(false); +}); + +it("does not let diagnostic template prose stand in for a removed producer", () => { + const name = "REA_NEW_MAX_BYTES"; + const prose = discoverNumericLimitUses([ + { + path: "src/config/mcpResponseBudget.ts", + source: `const guidance = \`The effective ${name} value is \${String(effective)} bytes. A change requires restarting.\`;`, + }, + { + path: "src/gdb/GdbSessionManager.ts", + source: `return \`This release has no runtime override; ${name} does not raise it.\`;`, + }, + { + path: "src/config/comparedLimit.ts", + source: `const same = \`${name} === \${limit}\`;`, + }, + ]); + expect(prose.has(name)).toBe(false); + const assigned = discoverNumericLimitUses([ + { + path: "src/hopper/BridgeLauncher.ts", + source: `const line = \`${name} = \${timeout / 1_000}\`;`, + }, + ]); + expect(assigned.get(name)).toEqual({ + path: "src/hopper/BridgeLauncher.ts", + read: "call", + }); }); it("does not invent a limit from a spread of the environment object", () => {