From 1cd4f5f1e561789d2e85315fd480d994719ee41c Mon Sep 17 00:00:00 2001 From: kzoeps Date: Wed, 7 Oct 2026 13:52:13 +0600 Subject: [PATCH 1/3] tooling: add JavaScript and Lua maintainability checks --- api/eslint.config.js | 37 + api/tests/fixtures/activities.js | 3 + api/tests/fixtures/actor-follows.js | 3 + api/tests/fixtures/bad-dates.js | 6 + api/tests/fixtures/bad-location-dates.js | 5 +- api/tests/fixtures/records.js | 4 + .../tooling/eslint-maintainability.test.js | 68 ++ api/tests/unit/tooling/lint-lua.test.js | 99 ++- .../unit/tooling/lua-maintainability.test.js | 209 ++++++ api/tooling/installer.js | 3 +- api/tooling/js-maintainability.md | 81 ++ api/tooling/lint-lua.js | 76 +- api/tooling/lua-maintainability.js | 693 ++++++++++++++++++ api/tooling/lua-maintainability.md | 31 + api/tooling/run-http-tests.js | 64 +- api/tooling/seed.js | 25 +- api/tsconfig.json | 7 +- 17 files changed, 1389 insertions(+), 25 deletions(-) create mode 100644 api/tests/unit/tooling/eslint-maintainability.test.js create mode 100644 api/tests/unit/tooling/lua-maintainability.test.js create mode 100644 api/tooling/js-maintainability.md create mode 100644 api/tooling/lua-maintainability.js create mode 100644 api/tooling/lua-maintainability.md diff --git a/api/eslint.config.js b/api/eslint.config.js index 5bb9113..01e046c 100644 --- a/api/eslint.config.js +++ b/api/eslint.config.js @@ -20,6 +20,43 @@ export default [ "no-new-func": "error", "no-console": "error", "no-implicit-coercion": "error", + "no-restricted-imports": ["error", { + paths: [ + { + name: "node:child_process", + importNames: ["exec", "execSync"], + message: "Use spawn or spawnSync with an argument array instead of shell-based command execution.", + }, + { + name: "child_process", + importNames: ["exec", "execSync"], + message: "Use spawn or spawnSync with an argument array instead of shell-based command execution.", + }, + ], + }], + "no-restricted-properties": ["error", { + object: "process", + property: "exit", + message: "Set process.exitCode so pending cleanup and asynchronous work can finish.", + }], + "no-restricted-syntax": ["error", + { + selector: "CallExpression[callee.name=/^(spawn|spawnSync)$/] > ObjectExpression.arguments > Property[key.name='shell'][value.value=true]", + message: "Do not enable shell execution for subprocess calls; pass an executable and argument array.", + }, + { + selector: "CallExpression[callee.name=/^(spawn|spawnSync)$/] > ObjectExpression.arguments > Property[key.value='shell'][value.value=true]", + message: "Do not enable shell execution for subprocess calls; pass an executable and argument array.", + }, + { + selector: "CallExpression[callee.property.name=/^(spawn|spawnSync)$/] > ObjectExpression.arguments > Property[key.name='shell'][value.value=true]", + message: "Do not enable shell execution for subprocess calls; pass an executable and argument array.", + }, + { + selector: "CallExpression[callee.property.name=/^(spawn|spawnSync)$/] > ObjectExpression.arguments > Property[key.value='shell'][value.value=true]", + message: "Do not enable shell execution for subprocess calls; pass an executable and argument array.", + }, + ], "no-param-reassign": ["error", { props: true }], "no-shadow": "error", "no-var": "error", diff --git a/api/tests/fixtures/activities.js b/api/tests/fixtures/activities.js index 98f1548..38087f3 100644 --- a/api/tests/fixtures/activities.js +++ b/api/tests/fixtures/activities.js @@ -8,6 +8,9 @@ const authorDid = 'did:plc:gggggggggggggggggggggggg'; const contributorDid = 'did:plc:hhhhhhhhhhhhhhhhhhhhhhhh'; const indexedAt = '2025-01-02T03:04:05.000Z'; +/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record }} SeedRow */ + +/** @param {string} collection @param {string} rkey @param {Record} fields @param {string} did @returns {Promise} */ async function row(collection, rkey, fields, did) { const record = { $type: collection, ...fields }; const uri = `at://${did}/${collection}/${rkey}`; diff --git a/api/tests/fixtures/actor-follows.js b/api/tests/fixtures/actor-follows.js index f63c717..cc62b11 100644 --- a/api/tests/fixtures/actor-follows.js +++ b/api/tests/fixtures/actor-follows.js @@ -6,6 +6,8 @@ const profileCollection = 'app.certified.actor.profile'; const organizationCollection = 'app.certified.actor.organization'; const indexedAt = '2025-01-02T03:04:05.000Z'; +/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record }} SeedRow */ + export const actorFollowDids = { publisher: 'did:plc:aaaaaaaaaaaaaaaaaaaaaaaa', otherPublisher: 'did:plc:bbbbbbbbbbbbbbbbbbbbbbbb', @@ -15,6 +17,7 @@ export const actorFollowDids = { curator: 'did:plc:ffffffffffffffffffffffff', }; +/** @param {string} collection @param {string} rkey @param {Record} fields @param {string} did @returns {Promise} */ async function row(collection, rkey, fields, did) { const record = { $type: collection, ...fields }; const uri = `at://${did}/${collection}/${rkey}`; diff --git a/api/tests/fixtures/bad-dates.js b/api/tests/fixtures/bad-dates.js index daefbdd..6e435d2 100644 --- a/api/tests/fixtures/bad-dates.js +++ b/api/tests/fixtures/bad-dates.js @@ -3,6 +3,10 @@ import { encode } from '@atcute/cbor'; import * as CID from '@atcute/cid'; +/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record }} SeedRow */ +/** @typedef {Omit & { indexedAt: string | null; storedAt: string }} DateFixtureRow */ + +/** @type {[string, string | number | string[] | null | undefined, string | null][]} */ const cases = [ ['badmissing', undefined, '2025-01-02T03:04:05.123456Z'], ['badnull', null, '2025-01-02T03:04:05.123456Z'], @@ -20,6 +24,7 @@ const cases = [ ['goodearlier', '2025-01-01T00:00:00Z', '2025-01-08T00:00:00Z'], ]; +/** @param {SeedRow} base @param {{ did: string; decorateRecord?: (record: Record, rkey: string) => Record }} options @returns {Promise} */ export async function makeDateCaseRows(base, { did, decorateRecord = (record) => record }) { return Promise.all(cases.map(async ([rkey, createdAt, indexedAt]) => { const record = decorateRecord({ ...base.record }, rkey); @@ -33,6 +38,7 @@ export async function makeDateCaseRows(base, { did, decorateRecord = (record) => })); } +/** @param {DateFixtureRow[]} rows @param {{ disposableTestTarget?: boolean }} [options] @returns {{ sql: string; params: unknown[] }[]} */ export function badDateSeedSql(rows, { disposableTestTarget = false } = {}) { if (disposableTestTarget !== true) throw new Error('Bad-date seeding requires explicit disposable-test target opt-in'); return rows.map((row) => ({ diff --git a/api/tests/fixtures/bad-location-dates.js b/api/tests/fixtures/bad-location-dates.js index 52d830b..2d531bc 100644 --- a/api/tests/fixtures/bad-location-dates.js +++ b/api/tests/fixtures/bad-location-dates.js @@ -1,7 +1,10 @@ import { makeDateCaseRows } from './bad-dates.js'; import { locationRecords } from './records.js'; -export const badDateLocations = await makeDateCaseRows(locationRecords[0], { +const [locationRecord] = locationRecords; +if (!locationRecord) throw new Error('Bad-date fixture generation requires a location record in records.js'); + +export const badDateLocations = await makeDateCaseRows(locationRecord, { did: 'did:plc:baddatefixturesexamplexx', decorateRecord: (record, rkey) => ({ ...record, locationType: 'date-test', name: `Date test ${rkey}`, description: 'Synthetic timestamp fixture', diff --git a/api/tests/fixtures/records.js b/api/tests/fixtures/records.js index 0b242cf..4686970 100644 --- a/api/tests/fixtures/records.js +++ b/api/tests/fixtures/records.js @@ -9,6 +9,9 @@ const collections = { const did = 'did:plc:abcdefghijklmnopqrstuvwx'; const indexedAt = '2025-01-02T03:04:05.000Z'; +/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record }} SeedRow */ + +/** @param {string} collection @param {string} rkey @param {Record} fields @param {string} [recordDid] @returns {Promise} */ async function row(collection, rkey, fields, recordDid = did) { const record = { $type: collection, ...fields }; const uri = `at://${recordDid}/${collection}/${rkey}`; @@ -84,6 +87,7 @@ export const locationRecords = await Promise.all([ }, 'did:web:organization-only.example'), ]); +/** @param {SeedRow[]} rows @param {{ disposableTestTarget?: boolean }} [options] @returns {{ sql: string; params: unknown[] }[]} */ export function seedSql(rows, { disposableTestTarget = false } = {}) { if (disposableTestTarget !== true) throw new Error('Seeding requires explicit disposable-test target opt-in'); if (!Array.isArray(rows) || rows.length === 0) throw new Error('Seeding requires at least one fixture row; supply a nonempty rows array'); diff --git a/api/tests/unit/tooling/eslint-maintainability.test.js b/api/tests/unit/tooling/eslint-maintainability.test.js new file mode 100644 index 0000000..2d6137c --- /dev/null +++ b/api/tests/unit/tooling/eslint-maintainability.test.js @@ -0,0 +1,68 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { ESLint } from 'eslint'; +import { fileURLToPath } from 'node:url'; + +const apiRoot = fileURLToPath(new URL('../../../', import.meta.url)); +const eslint = new ESLint({ cwd: apiRoot }); + +async function lint(source) { + const [result] = await eslint.lintText(source, { filePath: 'tooling/maintainability-fixture.js' }); + return result.messages; +} + +function ruleIds(messages) { + return messages.map(({ ruleId }) => ruleId); +} + +test('rejects named synchronous child_process APIs from either Node module spelling', async () => { + for (const specifier of ['node:child_process', 'child_process']) { + for (const imported of ['exec', 'execSync']) { + const messages = await lint(`import { ${imported} as run } from '${specifier}'; run('tool');`); + assert.deepEqual(ruleIds(messages), ['no-restricted-imports'], `${specifier} must not provide ${imported}`); + } + } +}); + +test('rejects namespace imports from either child_process module spelling', async () => { + for (const specifier of ['node:child_process', 'child_process']) { + const messages = await lint(`import * as childProcess from '${specifier}'; childProcess.spawn('tool', []);`); + assert.deepEqual(ruleIds(messages), ['no-restricted-imports'], `${specifier} namespace imports are intentionally restricted`); + } +}); + +test('rejects static shell:true keys on subprocess calls but allows unrelated shell options', async () => { + const messages = await lint([ + "import { spawn, spawnSync } from 'node:child_process';", + "spawn('tool', [], { shell: true });", + "spawn('tool', [], { 'shell': true });", + "spawn('tool', [], { ['shell']: true });", + "spawnSync('tool', [], { shell: true });", + "spawnSync('tool', [], { 'shell': true });", + "spawnSync('tool', [], { ['shell']: true });", + 'const subprocess = { spawn: (...args) => args };', + "subprocess.spawn('tool', [], { shell: true });", + "subprocess.spawn('tool', [], { 'shell': true });", + "subprocess.spawn('tool', [], { ['shell']: true });", + '({ shell: true });', + ].join('\n')); + + assert.deepEqual(ruleIds(messages), [ + 'no-restricted-syntax', 'no-restricted-syntax', 'no-restricted-syntax', + 'no-restricted-syntax', 'no-restricted-syntax', 'no-restricted-syntax', + 'no-restricted-syntax', 'no-restricted-syntax', 'no-restricted-syntax', + ]); + + const allowed = await lint([ + "import { spawn } from 'node:child_process';", + "spawn('tool', [], { shell: false });", + '({ shell: true });', + ].join('\n')); + assert.deepEqual(ruleIds(allowed), []); +}); + +test('rejects direct and static computed process.exit while allowing process.exitCode', async () => { + assert.deepEqual(ruleIds(await lint('process.exit(1);')), ['no-restricted-properties']); + assert.deepEqual(ruleIds(await lint("process['exit'](1);")), ['no-restricted-properties']); + assert.deepEqual(ruleIds(await lint('process.exitCode = 1;')), []); +}); diff --git a/api/tests/unit/tooling/lint-lua.test.js b/api/tests/unit/tooling/lint-lua.test.js index ed53ba1..7322c3b 100644 --- a/api/tests/unit/tooling/lint-lua.test.js +++ b/api/tests/unit/tooling/lint-lua.test.js @@ -1,8 +1,9 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; +import { access, mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; +import { buildLuaBundles } from '../../../tooling/lua-bundles.js'; import { lintLua } from '../../../tooling/lint-lua.js'; async function withTempRoot(run) { @@ -17,17 +18,40 @@ async function withTempRoot(run) { async function declareLuaHandler(root, { manifestPath = 'modules/demo/manifest.json', outputPath = '../../lua/endpoints/getExample.lua', + sharedSourcePaths = [], + lexiconParameters, } = {}) { const moduleFile = path.join(root, 'modules', 'demo', 'manifest.json'); + const sourceFile = path.join(root, 'lua', 'src', 'getExample.lua'); await mkdir(path.dirname(moduleFile), { recursive: true }); + await mkdir(path.dirname(sourceFile), { recursive: true }); + try { + await access(sourceFile); + } catch (error) { + if (error.code !== 'ENOENT') throw error; + await writeFile(sourceFile, 'function handle() end\n'); + } await writeFile(path.join(root, 'manifest.json'), JSON.stringify({ modules: [manifestPath] })); - await writeFile(moduleFile, JSON.stringify({ assets: [{ + const assets = []; + if (lexiconParameters) { + const lexiconFile = path.join(root, 'lexicons', 'getExample.json'); + await mkdir(path.dirname(lexiconFile), { recursive: true }); + await writeFile(lexiconFile, JSON.stringify({ + lexicon: 1, + id: 'org.example.getExample', + defs: { main: { type: 'query', parameters: { type: 'params', properties: Object.fromEntries(lexiconParameters.map((name) => [name, { type: 'string' }])) } } }, + })); + assets.push({ kind: 'lexicon', id: 'org.example.getExample', path: '../../lexicons/getExample.json' }); + } + assets.push({ kind: 'script', id: 'xrpc.query:org.example.getExample', path: outputPath, sourcePath: '../../lua/src/getExample.lua', + sharedSourcePaths, config: { script_type: 'lua' }, - }] })); + }); + await writeFile(moduleFile, JSON.stringify({ assets })); } test('skips Lua lint when the package has no Lua files', async () => { @@ -141,12 +165,75 @@ test('runs Luacheck only against generated handlers declared by the module manif assert.deepEqual(result, { status: 0 }); assert.equal(command, 'luacheck'); - assert.deepEqual(args, ['--config', '.luacheckrc', 'lua/endpoints/getExample.lua']); + assert.deepEqual(args, ['--config', '.luacheckrc', '--no-color', 'lua/endpoints/getExample.lua']); assert.equal(options.cwd, root); - assert.equal(options.stdio, 'inherit'); + assert.equal(options.encoding, 'utf8'); + assert.equal(options.stdio, 'pipe'); + }); +}); + +test('maps generated Luacheck locations back to authored Lua sources', async () => { + await withTempRoot(async (root) => { + const source = path.join(root, 'lua', 'src', 'getExample.lua'); + const sharedSource = path.join(root, 'lua', 'shared', 'helper.lua'); + const endpoint = path.join(root, 'lua', 'endpoints', 'getExample.lua'); + await mkdir(path.dirname(source), { recursive: true }); + await mkdir(path.dirname(sharedSource), { recursive: true }); + await mkdir(path.dirname(endpoint), { recursive: true }); + await writeFile(source, 'function handle()\n missing_helper()\nend\n'); + await writeFile(sharedSource, 'local function helper()\n return true\nend\n'); + await declareLuaHandler(root, { sharedSourcePaths: ['../../lua/shared/helper.lua'] }); + await buildLuaBundles(root); + const output = []; + + const result = await lintLua({ + root, + log: (message) => output.push(message), + spawn: () => ({ status: 1, stdout: "lua/endpoints/getExample.lua:6:3: undefined variable 'missing_helper'\n", stderr: '' }), + }); + + assert.equal(result.status, 1); + assert.ok(output.some((message) => message.includes("lua/src/getExample.lua:2:3: undefined variable 'missing_helper'"))); + assert.ok(output.some((message) => message.includes('in xrpc.query:org.example.getExample bundle at lua/endpoints/getExample.lua:6'))); }); }); +test('fails on parameter mismatches but keeps SQL interpolation diagnostics advisory', async () => { + const cases = [ + { + source: 'function handle()\n keys_only(params, { authors = true })\nend\n', + status: 1, + finding: /differs from its Lexicon; missing: uri; extra: authors/, + }, + { + source: 'function handle()\n keys_only(params, { uri = true })\n db.raw("SELECT * FROM records WHERE uri = " .. params.uri, {})\nend\n', + status: 0, + finding: /db.raw SQL text.*directly references params/, + }, + ]; + for (const scenario of cases) { + await withTempRoot(async (root) => { + const source = path.join(root, 'lua', 'src', 'getExample.lua'); + const endpoint = path.join(root, 'lua', 'endpoints', 'getExample.lua'); + await mkdir(path.dirname(source), { recursive: true }); + await mkdir(path.dirname(endpoint), { recursive: true }); + await writeFile(source, scenario.source); + await writeFile(endpoint, 'function handle() end\n'); + await declareLuaHandler(root, { lexiconParameters: ['uri'] }); + const output = []; + + const result = await lintLua({ + root, + log: (message) => output.push(message), + spawn: () => ({ status: 0, stdout: '', stderr: '' }), + }); + + assert.equal(result.status, scenario.status); + assert.ok(output.some((message) => scenario.finding.test(message))); + }); + } +}); + test('uses the default user-local LuaRocks binary when it is not on PATH', async () => { await withTempRoot(async (root) => { const home = path.join(root, 'home'); @@ -172,7 +259,7 @@ test('uses the default user-local LuaRocks binary when it is not on PATH', async assert.deepEqual(result, { status: 0 }); assert.deepEqual(commands.map(({ command }) => command), ['luacheck', localLuacheck]); - assert.deepEqual(commands[1].args, ['--config', '.luacheckrc', 'lua/endpoints/getExample.lua']); + assert.deepEqual(commands[1].args, ['--config', '.luacheckrc', '--no-color', 'lua/endpoints/getExample.lua']); }); }); diff --git a/api/tests/unit/tooling/lua-maintainability.test.js b/api/tests/unit/tooling/lua-maintainability.test.js new file mode 100644 index 0000000..b8d17e2 --- /dev/null +++ b/api/tests/unit/tooling/lua-maintainability.test.js @@ -0,0 +1,209 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { checkLuaMaintainability, checkLuaParameterContracts } from '../../../tooling/lua-maintainability.js'; + +async function withPackage(run, { source, parameters, sharedSourcePaths = [] }) { + const root = await mkdtemp(path.join(os.tmpdir(), 'hypercerts-lua-maintainability-')); + try { + await mkdir(path.join(root, 'modules', 'demo'), { recursive: true }); + await mkdir(path.join(root, 'lua', 'src'), { recursive: true }); + await mkdir(path.join(root, 'lexicons'), { recursive: true }); + await writeFile(path.join(root, 'manifest.json'), JSON.stringify({ modules: ['modules/demo/manifest.json'] })); + await writeFile(path.join(root, 'modules/demo/manifest.json'), JSON.stringify({ assets: [ + { kind: 'lexicon', id: 'org.example.listThings', path: '../../lexicons/listThings.json' }, + { + kind: 'script', + id: 'xrpc.query:org.example.listThings', + path: '../../lua/endpoints/listThings.lua', + sourcePath: '../../lua/src/listThings.lua', + sharedSourcePaths, + config: { script_type: 'lua' }, + }, + ] })); + await writeFile(path.join(root, 'lexicons/listThings.json'), JSON.stringify({ + lexicon: 1, + id: 'org.example.listThings', + defs: { main: { type: 'query', parameters: { type: 'params', properties: Object.fromEntries(parameters.map((name) => [name, { type: 'string' }])) } } }, + })); + await writeFile(path.join(root, 'lua/src/listThings.lua'), source); + await run(root); + } finally { + await rm(root, { recursive: true, force: true }); + } +} + +test('parameter contracts report both missing and extra literal allow-list keys', async () => { + await withPackage(async (root) => { + const diagnostics = await checkLuaParameterContracts({ root }); + const mismatch = diagnostics.find(({ code }) => code === 'parameter-contract-mismatch'); + + assert.deepEqual(mismatch?.missing, ['cursor']); + assert.deepEqual(mismatch?.extra, ['unexpected']); + }, { + parameters: ['authors', 'cursor'], + source: 'function handle()\n keys_only(params, { authors = true, unexpected = true })\nend\n', + }); +}); + +test('matching literal parameter names pass, and Lua comments or strings are not code', async () => { + await withPackage(async (root) => { + const diagnostics = await checkLuaParameterContracts({ root }); + assert.deepEqual(diagnostics, []); + }, { + parameters: ['authors', 'cursor'], + source: '-- keys_only(params, { commented = true })\nfunction handle()\n local note = "keys_only(params, { stringValue = true })"\n keys_only(params, { authors = true, cursor = true })\nend\n', + }); +}); + +test('qualified custom keys_only methods are unverified, not parameter allow-lists', async () => { + for (const source of [ + 'function handle()\n custom.keys_only(params, { authors = true })\nend\n', + 'function handle()\n custom:keys_only(params, { authors = true })\nend\n', + ]) { + await withPackage(async (root) => { + const diagnostics = await checkLuaParameterContracts({ root }); + assert.equal(diagnostics.length, 1); + assert.equal(diagnostics[0].status, 'unverified'); + assert.equal(diagnostics[0].code, 'parameter-contract-unverified'); + }, { parameters: ['uri'], source }); + } +}); + +test('dynamic allow-lists are reported as unverified instead of blocking lint', async () => { + await withPackage(async (root) => { + const diagnostics = await checkLuaParameterContracts({ root }); + assert.equal(diagnostics.length, 1); + assert.equal(diagnostics[0].status, 'unverified'); + assert.match(diagnostics[0].message, /dynamic or/); + }, { + parameters: ['authors'], + source: 'function handle()\n keys_only(params, make_allowed_keys())\nend\n', + }); +}); + +test('named local allow-lists are unverified unless visibility and immutability are clear', async () => { + const unsupportedSources = [ + 'local function unrelated()\n local allowed = { authors = true }\nend\nfunction handle()\n keys_only(params, allowed)\nend\n', + 'do\n local allowed = { authors = true }\nend\nfunction handle()\n keys_only(params, allowed)\nend\n', + 'function handle()\n local allowed = { uri = true }\n do\n local allowed = { authors = true }\n keys_only(params, allowed)\n end\nend\n', + 'function handle()\n local allowed = { uri = true }\n allowed.uri = false\n keys_only(params, allowed)\nend\n', + 'function handle()\n local allowed = { uri = true }\n local alias = allowed\n alias.uri = false\n keys_only(params, allowed)\nend\n', + ]; + for (const source of unsupportedSources) { + await withPackage(async (root) => { + const diagnostics = await checkLuaParameterContracts({ root }); + assert.equal(diagnostics.length, 1); + assert.equal(diagnostics[0].status, 'unverified'); + assert.equal(diagnostics[0].code, 'parameter-contract-unverified'); + }, { parameters: ['uri'], source }); + } + await withPackage(async (root) => { + assert.deepEqual(await checkLuaParameterContracts({ root }), []); + }, { + parameters: ['uri'], + source: 'function handle()\n local allowed = { uri = true }\n keys_only(params, allowed)\nend\n', + }); +}); + +test('bundle diagnostics identify a missing shared helper but accept an included one', async () => { + for (const sharedSourcePaths of [[], ['../../lua/shared/helper.lua']]) { + await withPackage(async (root) => { + const helperPath = path.join(root, 'lua/shared/helper.lua'); + await mkdir(path.dirname(helperPath), { recursive: true }); + await writeFile(helperPath, 'local function helper() return true end\n'); + const modulePath = path.join(root, 'modules/demo/manifest.json'); + const module = JSON.parse(await readFile(modulePath, 'utf8')); + module.assets.push({ + kind: 'script', + id: 'xrpc.query:org.example.helperCatalog', + path: '../../lua/endpoints/helperCatalog.lua', + sourcePath: '../../lua/src/helperCatalog.lua', + sharedSourcePaths: ['../../lua/shared/helper.lua'], + config: { script_type: 'lua' }, + }); + await writeFile(modulePath, JSON.stringify(module)); + await writeFile(path.join(root, 'lua/src/listThings.lua'), 'function handle() return helper() end\n'); + await writeFile(path.join(root, 'lua/src/helperCatalog.lua'), 'function handle() return true end\n'); + const diagnostics = await checkLuaMaintainability({ root }); + const helperFindings = diagnostics.diagnostics.filter(({ code, endpoint }) => (code.startsWith('helper-') || code === 'missing-helper-dependency') && endpoint === 'xrpc.query:org.example.listThings'); + const misses = helperFindings.filter(({ code }) => code === 'missing-helper-dependency'); + assert.equal(misses.length, sharedSourcePaths.length === 0 ? 1 : 0, JSON.stringify(diagnostics.diagnostics)); + if (misses.length) assert.match(misses[0].message, /lua\/shared\/helper.lua/); + if (sharedSourcePaths.length > 0) assert.deepEqual(helperFindings, []); + }, { + parameters: ['authors'], + source: 'function handle() return helper() end\n', + sharedSourcePaths, + }); + } +}); + +test('helper candidates are not resolved when later in the bundle or block-local', async () => { + await withPackage(async (root) => { + const earlyPath = path.join(root, 'lua/shared/early.lua'); + const latePath = path.join(root, 'lua/shared/later.lua'); + await mkdir(path.dirname(earlyPath), { recursive: true }); + await writeFile(earlyPath, 'local function use_helper() return helper() end\n'); + await writeFile(latePath, 'local function helper() return true end\n'); + await writeFile(path.join(root, 'lua/src/listThings.lua'), 'function handle() return use_helper() end\n'); + const { diagnostics } = await checkLuaMaintainability({ root }); + const findings = diagnostics.filter(({ endpoint, code }) => endpoint === 'xrpc.query:org.example.listThings' && (code.startsWith('helper-') || code === 'missing-helper-dependency')); + assert.equal(findings.length, 1); + assert.equal(findings[0].code, 'helper-availability-unverified'); + assert.equal(findings[0].status, 'unverified'); + }, { + parameters: ['authors'], + source: 'function handle() return use_helper() end\n', + sharedSourcePaths: ['../../lua/shared/early.lua', '../../lua/shared/later.lua'], + }); + + await withPackage(async (root) => { + const hiddenPath = path.join(root, 'lua/shared/hidden.lua'); + await mkdir(path.dirname(hiddenPath), { recursive: true }); + await writeFile(hiddenPath, 'if true then\n local label = "end"\n local function helper() return true end\nend\n'); + const modulePath = path.join(root, 'modules/demo/manifest.json'); + const module = JSON.parse(await readFile(modulePath, 'utf8')); + module.assets.push({ + kind: 'script', + id: 'xrpc.query:org.example.helperCatalog', + path: '../../lua/endpoints/helperCatalog.lua', + sourcePath: '../../lua/src/helperCatalog.lua', + sharedSourcePaths: ['../../lua/shared/hidden.lua'], + config: { script_type: 'lua' }, + }); + await writeFile(modulePath, JSON.stringify(module)); + await writeFile(path.join(root, 'lua/src/helperCatalog.lua'), 'function handle() return true end\n'); + await writeFile(path.join(root, 'lua/src/listThings.lua'), 'function handle() return helper() end\n'); + const { diagnostics } = await checkLuaMaintainability({ root }); + const findings = diagnostics.filter(({ endpoint, code }) => endpoint === 'xrpc.query:org.example.listThings' && (code.startsWith('helper-') || code === 'missing-helper-dependency')); + assert.equal(findings.length, 1); + assert.equal(findings[0].code, 'helper-availability-unverified'); + assert.equal(findings[0].status, 'unverified'); + }, { + parameters: ['authors'], + source: 'function handle() return helper() end\n', + }); +}); + +test('SQL interpolation warnings catch direct request values but ignore bound SQL, structure, comments, and strings', async () => { + await withPackage(async (root) => { + const { diagnostics } = await checkLuaMaintainability({ root }); + const warnings = diagnostics.filter(({ code }) => code === 'possible-sql-interpolation'); + assert.equal(warnings.length, 1); + assert.match(warnings[0].message, /directly references params, cursor/); + }, { + parameters: ['authors'], + source: [ + '-- db.raw("SELECT " .. params.comment)', + 'local note = "db.raw SQL text mentions params.cursor"', + 'function handle()', + ' db.raw("SELECT * FROM records WHERE uri = $1", { params.uri })', + ' db.raw("SELECT * FROM " .. table_name, values)', + ' db.raw("SELECT * FROM records WHERE uri = " .. params.uri .. cursor, {})', + 'end', + ].join('\n'), + }); +}); diff --git a/api/tooling/installer.js b/api/tooling/installer.js index 891435e..a47f369 100644 --- a/api/tooling/installer.js +++ b/api/tooling/installer.js @@ -264,8 +264,7 @@ async function writeAssets(states, client, onProgress) { const changed = []; const total = states.filter(({ state }) => state !== 'unchanged').length; let progress = 0; - for (let i = 0; i < states.length; i++) { - const { asset, state } = states[i]; + for (const [i, { asset, state }] of states.entries()) { if (state === 'unchanged') continue; progress++; onProgress?.(`[install ${progress}/${total}] ${asset.id}`); diff --git a/api/tooling/js-maintainability.md b/api/tooling/js-maintainability.md new file mode 100644 index 0000000..3d42fb4 --- /dev/null +++ b/api/tooling/js-maintainability.md @@ -0,0 +1,81 @@ +# JavaScript tooling safeguards + +The API package uses ESLint restrictions and strict TypeScript checking to keep +its JavaScript tooling predictable. These are static safeguards; accompanying +type annotations preserve behavior for valid installer, seed, and HTTP-test +inputs. + +## ESLint restrictions + +The package ESLint config applies these rules to JavaScript files under `api/`: + +- Named `exec` and `execSync` imports from `node:child_process` and + `child_process` are rejected, including locally renamed imports. Namespace + imports from either module are intentionally rejected wholesale; use a named + safe import such as `{ spawn }` or `{ spawnSync }`. +- `shell: true` is rejected when a bare `shell` key, quoted key, or + string-literal computed key such as `['shell']` appears in an object literal + passed directly as an argument to a recognized `spawn` or `spawnSync` call, + including member calls such as `childProcess.spawn(...)`. Unrelated objects + may use a `shell` property; default shell behavior and explicit + `shell: false` remain allowed. +- `process.exit(...)` is rejected. Set `process.exitCode` so pending cleanup + and asynchronous work can finish. + +These static rules do not perform data-flow analysis. Namespace imports are +rejected wholesale, but a default import of either child-process module can +bypass the named `exec`/`execSync` restriction; use named imports for allowed +APIs. CommonJS `require` and computed/dynamic module loading are also not +tracked. The subprocess selector does not follow a +named `spawn` import renamed to another identifier, copied function references, +template/expression shell keys, inline object spreads, computed method names, +or options stored in a variable or alias. Because it keys on call names, it can +also match an unrelated bare `spawn(...)` function or an unrelated object's +`.spawn(...)` method. The `process.exit` restriction covers direct access and +literal property access such as `process['exit']`; it makes no general guarantee +for computed expressions and does not track aliases. Keep those limitations in +mind during review. + +## Type checking + +`api/tsconfig.json` enables `checkJs`, `strict`, and `noUncheckedIndexedAccess` +for `tooling/installer.js`, `tooling/run-http-tests.js`, and `tooling/seed.js`. +`lint-lua.js` is outside this iteration's selected typecheck scope. Installer +progress iteration uses `states.entries()` so the state is +known to exist without an unchecked array-index assertion. Because `seed.js` +imports the fixture builders, their row shapes and callback signatures also +have JSDoc types; these annotations do not change fixture data or runtime +behavior. + +## Validation + +Run from the repository root: + +```sh +pnpm --dir api run lint:js +pnpm --dir api run typecheck +pnpm --dir api run test:unit +``` + +Focused checks for these safeguards and their owners are: + +```sh +cd api +pnpm exec eslint --max-warnings=0 --report-unused-disable-directives \ + eslint.config.js tooling/installer.js tooling/run-http-tests.js tooling/seed.js \ + tests/unit/tooling/eslint-maintainability.test.js +node --test tests/unit/tooling/eslint-maintainability.test.js \ + tests/unit/tooling/installer.test.js \ + tests/unit/tooling/installer-core.test.js \ + tests/unit/tooling/seed.test.js +pnpm run typecheck +``` + +The HTTP runtime suite is not required for these static checks and launches +Docker-managed services; use the separate procedure in `CONTRIBUTING.md` only +when that runtime validation is explicitly intended. + +The aggregate typecheck follows `seed.js` imports into the fixture modules. +Those imported fixture modules are included in the checked boundary so row +shapes and the date-fixture decorator callback remain explicit; the ordinary +fixture values and generated SQL are unchanged. diff --git a/api/tooling/lint-lua.js b/api/tooling/lint-lua.js index 45868f6..801b4e2 100644 --- a/api/tooling/lint-lua.js +++ b/api/tooling/lint-lua.js @@ -4,6 +4,7 @@ import { spawnSync } from 'node:child_process'; import { homedir } from 'node:os'; import path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; +import { checkLuaMaintainability } from './lua-maintainability.js'; const defaultRoot = fileURLToPath(new URL('../', import.meta.url)); @@ -79,6 +80,68 @@ async function declaredLuaOutputs(root) { return outputs; } +async function declaredLuaSourceMaps(root) { + const rootManifest = JSON.parse(await readFile(path.join(root, 'manifest.json'), 'utf8')); + const sourceMaps = new Map(); + for (const modulePath of rootManifest.modules) { + const moduleFile = resolveInside(root, root, modulePath, 'Module manifest path'); + const module = JSON.parse(await readFile(moduleFile, 'utf8')); + for (const asset of module.assets) { + if (asset.kind !== 'script' || asset.config?.script_type !== 'lua') continue; + const shared = asset.sharedSourcePaths + ?? (asset.sharedSourcePath ? [asset.sharedSourcePath] : []); + if (!Array.isArray(shared)) throw new Error(`${asset.id ?? 'Lua handler'} must declare sharedSourcePaths as an array`); + const declarations = [ + ...shared.map((source) => ({ path: source, kind: 'shared' })), + { path: asset.sourcePath, kind: 'handler' }, + ]; + const ranges = []; + let nextLine = 1; + for (const declaration of declarations) { + if (!declaration.path) throw new Error(`${asset.id ?? 'Lua handler'} has no authored ${declaration.kind} source for bundle diagnostics`); + const sourceFile = resolveInside(root, path.dirname(moduleFile), declaration.path, `${asset.id ?? 'Lua handler'} ${declaration.kind} source`); + let source; + try { + source = await readFile(sourceFile, 'utf8'); + } catch (error) { + throw new Error(`${asset.id ?? 'Lua handler'}: cannot map generated bundle to ${path.relative(root, sourceFile)}: ${error.message}`, { cause: error }); + } + const trimmed = source.trimEnd(); + const lineCount = trimmed.length === 0 ? 1 : (trimmed.match(/\n/g) ?? []).length + 1; + ranges.push({ + startLine: nextLine, + endLine: nextLine + lineCount - 1, + sourceFile: path.relative(root, sourceFile), + }); + nextLine += lineCount + 1; + } + const outputFile = resolveInside(root, path.dirname(moduleFile), asset.path, `Lua handler ${asset.id ?? '(unnamed)'} output path`); + sourceMaps.set(path.resolve(outputFile), { endpoint: asset.id ?? '(unnamed Lua handler)', ranges }); + } + } + return sourceMaps; +} + +function mapLuacheckOutput(output, root, sourceMaps) { + return output.replace(/^(.+):(\d+):(\d+): (.+)$/gm, (line, reportedPath, lineNumber, column, message) => { + const outputFile = path.resolve(root, reportedPath); + const sourceMap = sourceMaps.get(outputFile); + if (!sourceMap) return line; + const generatedLine = Number(lineNumber); + const range = sourceMap.ranges.find(({ startLine, endLine }) => generatedLine >= startLine && generatedLine <= endLine); + if (!range) return line; + const sourceLine = generatedLine - range.startLine + 1; + const generatedPath = path.relative(root, outputFile); + return `${range.sourceFile}:${sourceLine}:${column}: ${message} (in ${sourceMap.endpoint} bundle at ${generatedPath}:${generatedLine})`; + }); +} + +function emitLintOutput(output, log, stream) { + if (!output) return; + if (log === console.log) stream.write(output); + else log(output); +} + export async function lintLua({ root = defaultRoot, home = homedir(), spawn = spawnSync, log = console.log } = {}) { const allFiles = await luaFiles(path.join(root, 'lua')); if (allFiles.length === 0) { @@ -103,12 +166,18 @@ export async function lintLua({ root = defaultRoot, home = homedir(), spawn = sp } } + const maintainability = await checkLuaMaintainability({ root }); + for (const diagnostic of maintainability.diagnostics) log(`[Lua ${diagnostic.status}] ${diagnostic.message}`); + log(`Lua parameter contracts: ${maintainability.stats.parameterContractsVerified} verified, ${maintainability.stats.parameterContractsUnverified} unverified.`); + + const sourceMaps = await declaredLuaSourceMaps(root); const args = [ '--config', '.luacheckrc', + '--no-color', ...endpointFiles.map((file) => path.relative(root, file)), ]; - const options = { cwd: root, stdio: 'inherit' }; + const options = { cwd: root, encoding: 'utf8', stdio: 'pipe' }; let result = spawn('luacheck', args, options); if (result.error?.code === 'ENOENT') { const localLuacheck = path.join(home, '.luarocks', 'bin', 'luacheck'); @@ -123,7 +192,10 @@ export async function lintLua({ root = defaultRoot, home = homedir(), spawn = sp throw new Error('Luacheck is missing. Install it with `luarocks --lua-version=5.4 --local install luacheck 1.2.0`.'); } if (result.error) throw result.error; - return { status: result.status ?? 1 }; + emitLintOutput(mapLuacheckOutput(String(result.stdout ?? ''), root, sourceMaps), log, process.stdout); + emitLintOutput(mapLuacheckOutput(String(result.stderr ?? ''), root, sourceMaps), log, process.stderr); + const maintainabilityFailed = maintainability.diagnostics.some(({ status }) => status === 'error'); + return { status: Math.max(result.status ?? 1, maintainabilityFailed ? 1 : 0) }; } if (process.argv[1] && import.meta.url === pathToFileURL(path.resolve(process.argv[1])).href) { diff --git a/api/tooling/lua-maintainability.js b/api/tooling/lua-maintainability.js new file mode 100644 index 0000000..71bd220 --- /dev/null +++ b/api/tooling/lua-maintainability.js @@ -0,0 +1,693 @@ +import { readFile } from 'node:fs/promises'; +import path from 'node:path'; + +function resolveInside(root, base, relativePath, description) { + if (typeof relativePath !== 'string' || !relativePath.trim()) { + throw new Error(`${description} must be a nonempty path in the API manifests`); + } + const resolved = path.resolve(base, relativePath); + const relative = path.relative(root, resolved); + if (relative === '..' || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) { + throw new Error(`${description} resolves outside the API package: ${relativePath}`); + } + return resolved; +} + +async function readJson(file, description) { + try { + return JSON.parse(await readFile(file, 'utf8')); + } catch (error) { + throw new Error(`Cannot read ${description} at ${file}: ${error.message}`, { cause: error }); + } +} + +async function loadRegisteredApi(root) { + const packageRoot = path.resolve(root); + const manifestFile = path.join(packageRoot, 'manifest.json'); + const rootManifest = await readJson(manifestFile, 'root API manifest'); + if (!Array.isArray(rootManifest.modules)) throw new Error(`Root API manifest ${manifestFile} must declare a modules array`); + + const modules = []; + const handlers = []; + const lexicons = new Map(); + for (const modulePath of rootManifest.modules) { + const moduleFile = resolveInside(packageRoot, packageRoot, modulePath, 'Module manifest path'); + const module = await readJson(moduleFile, 'module manifest'); + if (!Array.isArray(module.assets)) throw new Error(`Lua module manifest ${moduleFile} must declare an assets array`); + const entry = { file: moduleFile, assets: module.assets }; + modules.push(entry); + for (const asset of module.assets) { + if (asset.kind === 'lexicon' && typeof asset.id === 'string') { + const records = lexicons.get(asset.id) ?? []; + records.push({ asset, moduleFile }); + lexicons.set(asset.id, records); + } + if (asset.kind === 'script' && asset.config?.script_type === 'lua') { + handlers.push({ asset, moduleFile }); + } + } + } + return { packageRoot, modules, handlers, lexicons }; +} + +function declaredSharedSources(handler) { + const declaration = handler.sharedSourcePaths + ?? (handler.sharedSourcePath ? [handler.sharedSourcePath] : []); + if (!Array.isArray(declaration) || declaration.some((source) => typeof source !== 'string' || !source.trim())) { + throw new Error(`${handler.id ?? '(unnamed Lua handler)'} must declare sharedSourcePaths as nonempty paths`); + } + return declaration; +} + +async function authoredSources(api, handlerRecord) { + const { asset, moduleFile } = handlerRecord; + const moduleDirectory = path.dirname(moduleFile); + const endpoint = asset.id ?? '(unnamed Lua handler)'; + const declarations = [ + ...declaredSharedSources(asset).map((source) => ({ path: source, kind: 'shared' })), + { path: asset.sourcePath, kind: 'handler' }, + ]; + const sources = []; + const diagnostics = []; + + for (const declaration of declarations) { + if (!declaration.path) { + diagnostics.push({ + status: 'error', + code: 'lua-source-missing', + endpoint, + message: `${endpoint}: no authored ${declaration.kind} source is declared in ${path.relative(api.packageRoot, moduleFile)}; add ${declaration.kind === 'handler' ? 'sourcePath' : 'sharedSourcePaths'} to the module manifest.`, + }); + continue; + } + const file = resolveInside(api.packageRoot, moduleDirectory, declaration.path, `${endpoint} ${declaration.kind} source`); + const relativeFile = path.relative(api.packageRoot, file); + let source; + try { + source = await readFile(file, 'utf8'); + } catch (error) { + if (error.code !== 'ENOENT') throw error; + diagnostics.push({ + status: 'error', + code: 'lua-source-missing', + endpoint, + file: relativeFile, + message: `${endpoint}: declared ${declaration.kind} Lua source ${relativeFile} is missing (from ${path.relative(api.packageRoot, moduleFile)}); restore it or correct the manifest source path.`, + }); + continue; + } + sources.push({ file, relativeFile, kind: declaration.kind, source }); + } + return { sources, diagnostics }; +} + +function longBracketEnd(source, start) { + const open = source.slice(start).match(/^\[(=*)\[/); + if (!open) return null; + const close = `]${open[1]}]`; + const end = source.indexOf(close, start + open[0].length); + return end < 0 ? source.length : end + close.length; +} + +function tokenizeLua(source) { + const tokens = []; + let index = 0; + let line = 1; + while (index < source.length) { + const char = source[index]; + if (/\s/.test(char)) { + if (char === '\n') line += 1; + index += 1; + continue; + } + if (source.startsWith('--', index)) { + const commentStart = index + 2; + const longEnd = longBracketEnd(source, commentStart); + if (longEnd !== null) { + line += (source.slice(index, longEnd).match(/\n/g) ?? []).length; + index = longEnd; + } else { + const newline = source.indexOf('\n', index); + index = newline < 0 ? source.length : newline; + } + continue; + } + if (char === '"' || char === "'") { + const startLine = line; + const quote = char; + index += 1; + let value = ''; + let staticValue = true; + while (index < source.length && source[index] !== quote) { + if (source[index] === '\n') line += 1; + if (source[index] === '\\') { + staticValue = false; + index += Math.min(2, source.length - index); + } else { + value += source[index]; + index += 1; + } + } + if (source[index] === quote) index += 1; + tokens.push({ type: 'string', value: staticValue ? value : null, line: startLine }); + continue; + } + const longEnd = char === '[' ? longBracketEnd(source, index) : null; + if (longEnd !== null) { + const raw = source.slice(index, longEnd); + const startLine = line; + line += (raw.match(/\n/g) ?? []).length; + tokens.push({ type: 'string', value: null, line: startLine }); + index = longEnd; + continue; + } + const identifier = source.slice(index).match(/^[A-Za-z_][A-Za-z0-9_]*/); + if (identifier) { + tokens.push({ type: 'identifier', value: identifier[0], line }); + index += identifier[0].length; + continue; + } + const number = source.slice(index).match(/^\d+(?:\.\d+)?/); + if (number) { + tokens.push({ type: 'number', value: number[0], line }); + index += number[0].length; + continue; + } + const operator = ['...', '..', '==', '~=', '<=', '>=', '//', '::'].find((candidate) => source.startsWith(candidate, index)); + const value = operator ?? char; + tokens.push({ type: 'symbol', value, line }); + index += value.length; + } + return tokens; +} + +function matchingClose(tokens, start, open, close) { + let depth = 0; + for (let index = start; index < tokens.length; index += 1) { + if (tokens[index].value === open) depth += 1; + else if (tokens[index].value === close) { + depth -= 1; + if (depth === 0) return index; + } + } + return -1; +} + +function splitTopLevel(tokens, separator) { + const parts = []; + let start = 0; + const stack = []; + const closing = { '(': ')', '{': '}', '[': ']' }; + for (let index = 0; index < tokens.length; index += 1) { + const value = tokens[index].value; + if (closing[value]) stack.push(closing[value]); + else if (stack.at(-1) === value) stack.pop(); + else if (value === separator && stack.length === 0) { + parts.push(tokens.slice(start, index)); + start = index + 1; + } + } + parts.push(tokens.slice(start)); + return parts; +} + +function literalTableKeys(tokens) { + if (tokens[0]?.value !== '{' || matchingClose(tokens, 0, '{', '}') !== tokens.length - 1) { + return { status: 'unverified', reason: 'the keys_only allow-list is dynamic or is not a literal table' }; + } + const keys = []; + const fields = splitTopLevel(tokens.slice(1, -1), ',').filter((field) => field.length > 0); + for (const field of fields) { + let key; + let valueIndex; + if (field.length === 3 && field[0].type === 'identifier' && field[1].value === '=') { + key = field[0].value; + valueIndex = 2; + } else if (field.length === 5 && field[0].value === '[' && field[1].type === 'string' + && field[1].value !== null && field[2].value === ']' && field[3].value === '=') { + key = field[1].value; + valueIndex = 4; + } else { + return { status: 'unverified', reason: 'the keys_only table contains a computed or unsupported field' }; + } + if (field[valueIndex].value !== 'true' || field[valueIndex].type !== 'identifier') { + return { status: 'unverified', reason: 'the keys_only table does not use literal true values' }; + } + keys.push(key); + } + return { status: 'verified', keys }; +} + +const LOCAL_BINDING_BOUNDARIES = new Set(['function', 'end', 'if', 'for', 'while', 'do', 'repeat', 'until', 'else', 'elseif']); + +function localLiteralTable(tokens, name, callIndex) { + const declarations = []; + for (let index = 0; index + 3 < callIndex; index += 1) { + if (tokens[index].value !== 'local' || tokens[index + 1].value !== name || tokens[index + 2].value !== '=') continue; + const start = index + 3; + if (tokens[start]?.value !== '{') return null; + const end = matchingClose(tokens, start, '{', '}'); + if (end < 0 || end >= callIndex) return null; + declarations.push({ index, end, expression: tokens.slice(start, end + 1) }); + } + if (declarations.length !== 1) return null; + const declaration = declarations[0]; + for (let index = declaration.end + 1; index < callIndex; index += 1) { + if (LOCAL_BINDING_BOUNDARIES.has(tokens[index].value)) return null; + if (tokens[index].value === name) return null; + } + return declaration.expression; +} + +const PARAMETER_ALLOWLIST_CALLS = new Set(['keys_only', 'collection_keys_only', 'collection_items_keys_only']); + +function keysOnlyCalls(tokens) { + const calls = []; + for (let index = 0; index + 1 < tokens.length; index += 1) { + const name = tokens[index].value; + const previous = tokens[index - 1]?.value; + const isFunctionDeclaration = previous === 'function' + || ((previous === '.' || previous === ':') && tokens[index - 3]?.value === 'function'); + if (tokens[index].type !== 'identifier' || !PARAMETER_ALLOWLIST_CALLS.has(name) + || tokens[index + 1].value !== '(' || isFunctionDeclaration) continue; + const close = matchingClose(tokens, index + 1, '(', ')'); + if (close < 0) continue; + const args = splitTopLevel(tokens.slice(index + 2, close), ','); + if (args.length < 2) continue; + calls.push({ index, name, args, line: tokens[index].line, qualified: previous === '.' || previous === ':' }); + } + return calls; +} + +function inspectParameterSources(sources) { + const calls = sources.flatMap((source) => { + const tokens = tokenizeLua(source.source); + return keysOnlyCalls(tokens).map((call) => ({ ...call, tokens, source })); + }); + const qualifiedCall = calls.find(({ qualified }) => qualified); + if (qualifiedCall) { + return { + status: 'unverified', + reason: 'qualified keys_only methods are unsupported and are not treated as query allow-list checks', + file: qualifiedCall.source.relativeFile, + line: qualifiedCall.line, + }; + } + const queryCalls = calls.filter(({ args }) => args[0]?.length === 1 + && ['params', 'request_params'].includes(args[0][0].value)); + if (calls.length > 0 && queryCalls.length === 0) { + return { status: 'unverified', reason: 'keys_only does not receive a recognizable params or request_params value in this bundle' }; + } + const candidates = queryCalls; + if (candidates.length !== 1) { + return { + status: 'unverified', + reason: candidates.length === 0 + ? 'no recognizable keys_only allow-list appears in the ordered handler bundle; validation may be delegated or use another pattern' + : 'the ordered handler bundle contains multiple keys_only calls, so the query-specific allow-list is ambiguous', + }; + } + const call = candidates[0]; + const [values, expression] = call.args; + if (values.length !== 1 || values[0].type !== 'identifier') { + return { status: 'unverified', reason: 'the keys_only values argument is dynamic or unsupported', file: call.source.relativeFile, line: call.line }; + } + let allowList = expression; + if (expression.length === 1 && expression[0].type === 'identifier') { + allowList = localLiteralTable(call.tokens, expression[0].value, call.index); + } + if (!allowList) return { status: 'unverified', reason: 'the keys_only allow-list is dynamic or unsupported', file: call.source.relativeFile, line: call.line }; + return { ...literalTableKeys(allowList), file: call.source.relativeFile, line: call.line }; +} + +function queryParameters(lexicon) { + const main = lexicon?.defs?.main; + if (main?.type !== 'query') return { status: 'unverified', reason: 'the registered Lexicon is not a recognizable query' }; + const parameters = main.parameters; + if (parameters === undefined) return { status: 'verified', names: [] }; + if (!parameters || parameters.type !== 'params' || !parameters.properties || typeof parameters.properties !== 'object' || Array.isArray(parameters.properties)) { + return { status: 'unverified', reason: 'the Lexicon parameter schema is not a recognizable params object' }; + } + return { status: 'verified', names: Object.keys(parameters.properties).sort() }; +} + +function unverifiedParameter(id, file, reason, line) { + return { + status: 'unverified', + code: 'parameter-contract-unverified', + endpoint: id, + file, + line, + message: `${id}: parameter contract unverified${file ? ` in ${file}${line ? `:${line}` : ''}` : ''}: ${reason}`, + }; +} + +function lexiconPath(api, records, id) { + if (records.length !== 1) return null; + const { asset, moduleFile } = records[0]; + if (asset.path) return resolveInside(api.packageRoot, path.dirname(moduleFile), asset.path, `Lexicon ${id} path`); + if (asset.packagePath) return resolveInside(api.packageRoot, api.packageRoot, asset.packagePath, `Lexicon ${id} package path`); + return null; +} + +function parameterDiagnosticsForHandler(api, handlerRecord, sources) { + const { asset } = handlerRecord; + if (typeof asset.id !== 'string' || !asset.id.startsWith('xrpc.query:')) return { diagnostics: [], verified: false, query: false }; + const id = asset.id.slice('xrpc.query:'.length); + const records = api.lexicons.get(id) ?? []; + if (records.length !== 1) { + return { + diagnostics: [unverifiedParameter(asset.id, asset.sourcePath, records.length ? 'multiple registered Lexicon assets use this query id' : 'no Lexicon asset with this id appears in the root manifest registered modules')], + verified: false, + query: true, + }; + } + const file = lexiconPath(api, records, id); + if (!file) return { diagnostics: [unverifiedParameter(asset.id, asset.sourcePath, 'the registered Lexicon asset has no path')], verified: false, query: true }; + return { id, file, sources, query: true }; +} + +async function compareParameterContracts(api, handlerRecord, sources) { + const prep = parameterDiagnosticsForHandler(api, handlerRecord, sources); + if (prep.diagnostics) return { diagnostics: prep.diagnostics, verified: prep.verified, query: prep.query }; + const { asset } = handlerRecord; + const lexicon = await readJson(prep.file, `registered Lexicon ${prep.id}`); + const contract = queryParameters(lexicon); + if (contract.status !== 'verified') { + return { diagnostics: [unverifiedParameter(asset.id, asset.sourcePath, contract.reason)], verified: false, query: true }; + } + const inspection = inspectParameterSources(sources); + if (inspection.status !== 'verified') { + return { + diagnostics: [unverifiedParameter(asset.id, inspection.file ?? asset.sourcePath, inspection.reason, inspection.line)], + verified: false, + query: true, + }; + } + const actual = [...new Set(inspection.keys)].sort(); + const missing = contract.names.filter((name) => !actual.includes(name)); + const extra = actual.filter((name) => !contract.names.includes(name)); + if (!missing.length && !extra.length) return { diagnostics: [], verified: true, query: true }; + const file = inspection.file; + return { + diagnostics: [{ + status: 'error', + code: 'parameter-contract-mismatch', + endpoint: asset.id, + file, + line: inspection.line, + expected: contract.names, + actual, + missing, + extra, + message: `${asset.id}: Lua parameter allow-list in ${file}:${inspection.line} differs from its Lexicon${missing.length ? `; missing: ${missing.join(', ')}` : ''}${extra.length ? `; extra: ${extra.join(', ')}` : ''}`, + }], + verified: true, + query: true, + }; +} + +const LUA_GLOBALS = new Set([ + '_G', '_VERSION', 'assert', 'collectgarbage', 'coroutine', 'debug', 'dofile', 'error', 'getmetatable', 'io', 'ipairs', + 'load', 'loadfile', 'math', 'next', 'os', 'package', 'pairs', 'pcall', 'print', 'rawequal', 'rawget', 'rawlen', 'rawset', + 'select', 'setmetatable', 'string', 'table', 'tonumber', 'tostring', 'type', 'utf8', 'warn', 'xpcall', +]); + +async function luacheckGlobals(root) { + const globals = new Set(LUA_GLOBALS); + let source; + try { + source = await readFile(path.join(root, '.luacheckrc'), 'utf8'); + } catch (error) { + if (error.code === 'ENOENT') return globals; + throw error; + } + const tokens = tokenizeLua(source); + for (let index = 0; index < tokens.length - 2; index += 1) { + if (!['globals', 'read_globals'].includes(tokens[index].value) || tokens[index + 1].value !== '=' || tokens[index + 2].value !== '{') continue; + const close = matchingClose(tokens, index + 2, '{', '}'); + if (close < 0) continue; + for (const token of tokens.slice(index + 3, close)) { + if (token.type === 'string' && token.value !== null) globals.add(token.value); + } + } + return globals; +} + +function functionDeclarations(tokens) { + const declarations = []; + const blocks = []; + let loopsAwaitingDo = 0; + for (let index = 0; index < tokens.length; index += 1) { + if (tokens[index].type !== 'identifier') continue; + const value = tokens[index].value; + if (value === 'function') { + if (tokens[index + 1]?.type === 'identifier' && tokens[index + 2]?.value === '(') { + declarations.push({ + name: tokens[index + 1].value, + line: tokens[index].line, + index, + topLevel: blocks.length === 0, + local: tokens[index - 1]?.value === 'local', + }); + } + blocks.push('function'); + } else if (value === 'if') { + blocks.push('if'); + } else if (value === 'for' || value === 'while') { + blocks.push('loop'); + loopsAwaitingDo += 1; + } else if (value === 'do') { + if (loopsAwaitingDo > 0) loopsAwaitingDo -= 1; + else blocks.push('do'); + } else if (value === 'repeat') { + blocks.push('repeat'); + } else if (value === 'end') { + if (blocks.length > 0 && blocks.at(-1) !== 'repeat') blocks.pop(); + } else if (value === 'until' && blocks.at(-1) === 'repeat') { + blocks.pop(); + } + } + return declarations; +} + +function functionCalls(tokens) { + const calls = []; + for (let index = 0; index + 1 < tokens.length; index += 1) { + if (tokens[index].type !== 'identifier' || tokens[index].value === 'function' || tokens[index + 1].value !== '(' + || tokens[index - 1]?.value === 'function' || tokens[index - 1]?.value === '.' || tokens[index - 1]?.value === ':') continue; + calls.push({ name: tokens[index].value, line: tokens[index].line, index }); + } + return calls; +} + +function helperCatalog(sources) { + const catalog = new Map(); + for (const source of sources) { + const tokens = tokenizeLua(source.source); + for (const declaration of functionDeclarations(tokens)) { + const definitions = catalog.get(declaration.name) ?? []; + definitions.push({ ...declaration, file: source.file, relativeFile: source.relativeFile }); + catalog.set(declaration.name, definitions); + } + } + return catalog; +} + +function helperAvailabilityDiagnostic(endpoint, source, call, definitions) { + const candidates = [...new Set(definitions.map(({ relativeFile }) => relativeFile))].sort(); + return { + status: 'unverified', + code: 'helper-availability-unverified', + endpoint, + file: source.relativeFile, + line: call.line, + message: `${endpoint}: cannot prove that shared helper ${call.name} at ${source.relativeFile}:${call.line} resolves to a top-level local declared earlier in this ordered bundle${candidates.length ? `; same-name declarations exist in ${candidates.join(', ')}` : ''}. Luacheck remains authoritative for undefined globals.`, + }; +} + +function missingHelperDiagnostics(endpoint, sources, catalog, globals) { + const inBundle = new Map(); + const tokenized = sources.map((source, sourceIndex) => ({ source, sourceIndex, tokens: tokenizeLua(source.source) })); + for (const item of tokenized) { + for (const declaration of functionDeclarations(item.tokens)) { + const definitions = inBundle.get(declaration.name) ?? []; + definitions.push({ ...declaration, sourceIndex: item.sourceIndex, file: item.source.file, relativeFile: item.source.relativeFile }); + inBundle.set(declaration.name, definitions); + } + } + + const diagnostics = []; + const reported = new Set(); + for (const { source, sourceIndex, tokens } of tokenized) { + for (const call of functionCalls(tokens)) { + if (globals.has(call.name)) continue; + const visible = (inBundle.get(call.name) ?? []).some((definition) => definition.local && definition.topLevel + && (definition.sourceIndex < sourceIndex || (definition.sourceIndex === sourceIndex && definition.index < call.index))); + if (visible) continue; + const candidates = catalog.get(call.name) ?? []; + if (candidates.length === 0) continue; + + const key = `${source.file}:${call.line}:${call.name}`; + if (reported.has(key)) continue; + reported.add(key); + const bundleDeclarations = candidates.filter(({ file }) => tokenized.some(({ source: included }) => included.file === file)); + if (bundleDeclarations.length > 0) { + diagnostics.push(helperAvailabilityDiagnostic(endpoint, source, call, bundleDeclarations)); + continue; + } + + if (candidates.length === 1 && candidates[0].local && candidates[0].topLevel) { + diagnostics.push({ + status: 'warning', + code: 'missing-helper-dependency', + endpoint, + file: source.relativeFile, + line: call.line, + message: `${endpoint}: possible missing shared dependency for ${call.name} at ${source.relativeFile}:${call.line}; the only registered top-level local declaration is in ${candidates[0].relativeFile}. Add it to sharedSourcePaths if intended. Luacheck remains authoritative for undefined globals.`, + }); + } else { + diagnostics.push(helperAvailabilityDiagnostic(endpoint, source, call, candidates)); + } + } + } + return diagnostics; +} + +function rawSqlExpressions(tokens) { + const expressions = []; + for (let index = 0; index + 3 < tokens.length; index += 1) { + const isDbRaw = tokens[index].value === 'db' && tokens[index + 1].value === '.' + && tokens[index + 2].value === 'raw' && tokens[index + 3].value === '('; + if (isDbRaw) { + const close = matchingClose(tokens, index + 3, '(', ')'); + if (close >= 0) { + const args = splitTopLevel(tokens.slice(index + 4, close), ','); + if (args[0]?.length) expressions.push({ expression: args[0], line: tokens[index].line }); + } + } + + if (tokens[index].value !== 'pcall' || tokens[index + 1].value !== '(') continue; + const close = matchingClose(tokens, index + 1, '(', ')'); + if (close < 0) continue; + const args = splitTopLevel(tokens.slice(index + 2, close), ','); + if (args[0]?.length === 3 && args[0][0].value === 'db' && args[0][1].value === '.' && args[0][2].value === 'raw' + && args[1]?.length) expressions.push({ expression: args[1], line: tokens[index].line }); + } + return expressions; +} + +function requestValueReferences(expression) { + const references = new Set(); + for (const token of expression) { + if (token.type !== 'identifier') continue; + if (token.value === 'params' || token.value === 'request_params') references.add(token.value); + if (token.value === 'cursor') references.add(token.value); + } + return [...references]; +} + +function sqlInterpolationDiagnostics(endpoint, sources) { + const diagnostics = []; + const reported = new Set(); + for (const source of sources) { + const tokens = tokenizeLua(source.source); + for (const { expression, line } of rawSqlExpressions(tokens)) { + const references = requestValueReferences(expression); + if (!references.length) continue; + const key = `${source.file}:${line}:${references.join(',')}`; + if (reported.has(key)) continue; + reported.add(key); + diagnostics.push({ + status: 'warning', + code: 'possible-sql-interpolation', + endpoint, + file: source.relativeFile, + line, + message: `${endpoint}: db.raw SQL text at ${source.relativeFile}:${line} directly references ${references.join(', ')}; bind request values instead. This advisory is not a taint analysis.`, + }); + } + } + return diagnostics; +} + +async function parameterCheck(api, handlersAndSources) { + const diagnostics = []; + let queryCount = 0; + let verifiedCount = 0; + let unverifiedCount = 0; + for (const item of handlersAndSources) { + const result = await compareParameterContracts(api, item.handler, item.sources); + diagnostics.push(...result.diagnostics); + if (!result.query) continue; + queryCount += 1; + if (result.verified) verifiedCount += 1; + else unverifiedCount += 1; + } + return { diagnostics, queryCount, verifiedCount, unverifiedCount }; +} + +async function loadHandlerSources(api) { + const loaded = []; + const diagnostics = []; + for (const handler of api.handlers) { + const result = await authoredSources(api, handler); + diagnostics.push(...result.diagnostics); + loaded.push({ handler, sources: result.sources }); + } + return { loaded, diagnostics }; +} + +async function allDeclaredSharedSources(api) { + const sourceDeclarations = new Map(); + for (const { asset, moduleFile } of api.handlers) { + for (const sourcePath of declaredSharedSources(asset)) { + const file = resolveInside(api.packageRoot, path.dirname(moduleFile), sourcePath, `${asset.id ?? 'Lua handler'} shared source`); + sourceDeclarations.set(file, path.relative(api.packageRoot, file)); + } + } + const sources = []; + for (const [file, relativeFile] of sourceDeclarations) { + try { + sources.push({ file, relativeFile, source: await readFile(file, 'utf8') }); + } catch (error) { + if (error.code !== 'ENOENT') throw error; + } + } + return sources; +} + +export async function checkLuaParameterContracts({ root }) { + const api = await loadRegisteredApi(root); + const { loaded } = await loadHandlerSources(api); + const result = await parameterCheck(api, loaded); + return result.diagnostics; +} + +export async function checkLuaMaintainability({ root }) { + const api = await loadRegisteredApi(root); + const { loaded, diagnostics: sourceDiagnostics } = await loadHandlerSources(api); + const parameters = await parameterCheck(api, loaded); + const catalog = helperCatalog(await allDeclaredSharedSources(api)); + const globals = await luacheckGlobals(api.packageRoot); + const helperDiagnostics = []; + const sqlDiagnostics = []; + for (const { handler, sources } of loaded) { + const endpoint = handler.asset.id ?? '(unnamed Lua handler)'; + helperDiagnostics.push(...missingHelperDiagnostics(endpoint, sources, catalog, globals)); + sqlDiagnostics.push(...sqlInterpolationDiagnostics(endpoint, sources)); + } + const diagnostics = [...sourceDiagnostics, ...parameters.diagnostics, ...helperDiagnostics, ...sqlDiagnostics]; + return { + diagnostics, + stats: { + registeredQueries: parameters.queryCount, + parameterContractsVerified: parameters.verifiedCount, + parameterContractsUnverified: parameters.unverifiedCount, + parameterMismatches: diagnostics.filter(({ code }) => code === 'parameter-contract-mismatch').length, + helperWarnings: helperDiagnostics.filter(({ status }) => status === 'warning').length, + helperAvailabilityUnverified: helperDiagnostics.filter(({ status }) => status === 'unverified').length, + sqlWarnings: sqlDiagnostics.length, + }, + }; +} diff --git a/api/tooling/lua-maintainability.md b/api/tooling/lua-maintainability.md new file mode 100644 index 0000000..9f24d76 --- /dev/null +++ b/api/tooling/lua-maintainability.md @@ -0,0 +1,31 @@ +# Lua maintainability checks + +Run the integrated check with: + +```sh +pnpm lint:lua +``` + +`pnpm check` includes this command through `pnpm lint`. The checker reads only modules listed in `api/manifest.json` and their manifest-declared assets and Lua sources. It does not scan standalone Lexicon files that are not registered by those modules. Generated endpoint bundles remain the input to Luacheck; diagnostics with a generated-bundle location are mapped back to the ordered `sharedSourcePaths` and handler `sourcePath` that produced that line. Adding `lint-lua.js` to the TypeScript `checkJs` file set is deferred; this change does not extend that configuration. + +## Findings and severity + +- **Error:** a recognized literal `keys_only` allow-list differs from its registered query Lexicon parameter names, or a declared authored Lua source is missing. Parameter mismatches list both missing and extra names. Errors make `lint:lua` fail. +- **Unverified (nonblocking):** the checker cannot safely compare a query contract, such as when validation is delegated through an unsupported wrapper or uses dynamic/computed allow-list construction, or cannot prove a helper's local visibility/order. These are reported and are not counted as verified. +- **Warning (nonblocking):** a call appears to refer to a shared helper whose definition exists in another registered shared source but is absent from this handler's ordered bundle, or a direct request/cursor value appears in a `db.raw` SQL-text argument. Existing Luacheck findings retain their normal exit status. + +The command prints a parameter-contract count split into verified and unverified registered queries. A verified mismatch is counted as verified and is separately reported as an error. The current manifest snapshot has 55 registered queries: 47 verified, 8 unverified, and 0 mismatches; helper diagnostics are 0 missing-dependency warnings and 1 availability-unverified result; SQL warnings are 0. These counts describe this checkout and will change with registered modules or source patterns. + +## Coverage and limitations + +The parameter comparison recognizes simple unqualified `keys_only(params, { name = true })` and `keys_only(request_params, { name = true })` calls, plus the explicit `collection_keys_only` and `collection_items_keys_only` wrappers, when their literal allow-list is uniquely identifiable in the declared source bundle. Dot- or colon-qualified custom methods such as `custom.keys_only(...)` are unsupported and reported as unverified. A named local literal table is resolved only when its declaration and call are in the same simple block, it is not reassigned, shadowed, or otherwise referenced before the check. Out-of-scope bindings, aliases, block transitions, dynamic flags, computed keys, multiple candidate calls, and unsupported validation patterns are unverified rather than inferred as matches. + +Bundle helper diagnostics are deliberately narrow. A call is considered resolved only when a top-level `local function` declaration appears earlier in the same ordered shared-source-plus-handler bundle. Later declarations, block-local candidates, non-local declarations, or ambiguous candidates are reported as availability-unverified; a possible missing-dependency warning is emitted only for a unique top-level local declaration found in another registered shared source. Name matches alone never prove availability. Only identifier tokens affect the block walk; string and comment contents do not. This is not a full Lua scope engine; generated-bundle Luacheck remains authoritative for undefined globals. Source attribution follows the current builder composition order (shared sources, then handler) and blank-line separators; diagnostics that do not match Luacheck's plain `file:line:column` form are left unchanged. + +SQL detection is advisory, not taint analysis or a SQL-injection proof. It checks only obvious direct references to `params`, `request_params`, or a variable named `cursor` in the SQL-text argument of `db.raw(...)` and `pcall(db.raw, ...)`. Values supplied separately as SQL bind arguments are allowed, as is structural SQL concatenation that does not directly reference those names. The detector does not follow SQL stored in locals, aliases, wrapper/helper calls, `string.format`, table-built fragments, or values renamed through arbitrary transformations; it can also warn on a structural identifier named `cursor`. Review SQL construction and binding manually when data flow is indirect. + +Focused offline tests: + +```sh +node --test tests/unit/tooling/lua-maintainability.test.js tests/unit/tooling/lint-lua.test.js +``` diff --git a/api/tooling/run-http-tests.js b/api/tooling/run-http-tests.js index b912fd3..482c1ae 100644 --- a/api/tooling/run-http-tests.js +++ b/api/tooling/run-http-tests.js @@ -15,6 +15,15 @@ const fixtureRoot = path.join(httpTestRoot, 'fixtures'); const services = new Set(['postgres', 'happyview']); const proxyBlock = 'http://127.0.0.1:9'; +/** @typedef {{ cwd?: string; env?: NodeJS.ProcessEnv; input?: string; maxBuffer?: number; timeoutMs?: number; label?: string }} SpawnOptions */ +/** @typedef {import('node:child_process').SpawnSyncReturns} SpawnResult */ +/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record }} SeedRow */ +/** @typedef {(args: string[]) => string} ComposeRunner */ +/** @typedef {{ services?: Record }} ComposeConfig */ +/** @typedef {{ ownsProject: boolean; retainCredentials: boolean; failure: unknown }} LifecycleState */ +/** @typedef {{ psqlPath: string; suites: string[]; httpSeedRows: SeedRow[]; dockerEnv: NodeJS.ProcessEnv; dockerContext: string; projectName: string; postgresUser: string; postgresPassword: string; postgresDatabase: string; adminToken: string; composeEnv: NodeJS.ProcessEnv; tempRoot: string; envFile: string; compose: ComposeRunner; lifecycleState: LifecycleState }} RunContext */ + +/** @param {string} command @param {string[]} args @param {SpawnOptions} [options] @returns {SpawnResult} */ function spawn(command, args, { cwd = apiRoot, env = process.env, @@ -24,7 +33,8 @@ function spawn(command, args, { label = `${command} ${args[0] ?? ''}`, } = {}) { const result = spawnSync(command, args, { cwd, env, input, encoding: 'utf8', maxBuffer, timeout: timeoutMs }); - if (result.error?.code === 'ETIMEDOUT') { + const errorCode = result.error && 'code' in result.error ? result.error.code : undefined; + if (errorCode === 'ETIMEDOUT') { if (result.stdout) process.stderr.write(result.stdout); if (result.stderr) process.stderr.write(result.stderr); throw new Error(`${label} timed out after ${timeoutMs}ms; the subprocess was terminated. Check task-owned Docker logs and local service health before retrying.`, { cause: result.error }); @@ -33,6 +43,7 @@ function spawn(command, args, { return result; } +/** @param {string} command @param {string[]} args @param {SpawnOptions} [options] @returns {string} */ function run(command, args, options = {}) { const result = spawn(command, args, options); if (result.stdout) process.stdout.write(result.stdout); @@ -43,6 +54,7 @@ function run(command, args, options = {}) { return result.stdout; } +/** @param {string} command @param {string[]} args @param {SpawnOptions} [options] @returns {string} */ function runQuiet(command, args, options = {}) { const result = spawn(command, args, options); if (result.status !== 0) { @@ -52,6 +64,7 @@ function runQuiet(command, args, options = {}) { return result.stdout.trim(); } +/** @returns {string} */ function requirePsqlPath() { const executable = process.env.PSQL_PATH; if (!executable || !path.isAbsolute(executable)) { @@ -61,23 +74,26 @@ function requirePsqlPath() { if (!statSync(executable).isFile()) throw new Error('not a regular file'); accessSync(executable, constants.X_OK); } catch (error) { - throw new Error(`PSQL_PATH must point to an existing executable file (${executable}): ${error.message}`, { cause: error }); + const detail = error instanceof Error ? error.message : String(error); + throw new Error(`PSQL_PATH must point to an existing executable file (${executable}): ${detail}`, { cause: error }); } return executable; } +/** @param {import('node:fs').Dirent} left @param {import('node:fs').Dirent} right @returns {number} */ function compareEntryNames(left, right) { if (left.name < right.name) return -1; if (left.name > right.name) return 1; return 0; } +/** @param {string} directory @param {string} suffix @returns {Promise} */ async function findFiles(directory, suffix) { let entries; try { entries = await readdir(directory, { withFileTypes: true }); } catch (error) { - if (error?.code === 'ENOENT') return []; + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') return []; throw error; } entries.sort(compareEntryNames); @@ -94,9 +110,12 @@ async function findFiles(directory, suffix) { return fileGroups.flat(); } +/** @param {string[]} fixtureModules @returns {Promise} */ async function loadFixtureRows(fixtureModules) { + /** @type {SeedRow[]} */ const rows = []; await fixtureModules.reduce((previous, file) => previous.then(async () => { + /** @type {{ seedRows?: SeedRow[] }} */ const fixture = await import(pathToFileURL(file).href); if (!Array.isArray(fixture.seedRows) || fixture.seedRows.length === 0) { throw new Error(`${path.relative(apiRoot, file)} must export a nonempty seedRows array.`); @@ -107,6 +126,7 @@ async function loadFixtureRows(fixtureModules) { return rows; } +/** @returns {NodeJS.ProcessEnv} */ function dockerEnvironment() { const env = { ...process.env }; delete env.DOCKER_HOST; @@ -116,6 +136,7 @@ function dockerEnvironment() { return env; } +/** @param {NodeJS.ProcessEnv} env @returns {string} */ function assertLocalDocker(env) { const context = runQuiet('docker', ['context', 'show'], { env }); const endpoint = runQuiet('docker', ['context', 'inspect', '--format', '{{.Endpoints.docker.Host}}', context], { env }); @@ -125,6 +146,7 @@ function assertLocalDocker(env) { return `${context} (${endpoint})`; } +/** @param {ComposeConfig} config @returns {string[]} */ function imagesFromComposeConfig(config) { return [...services].map((service) => { const image = config.services?.[service]?.image; @@ -135,6 +157,7 @@ function imagesFromComposeConfig(config) { }); } +/** @param {string[]} images @param {NodeJS.ProcessEnv} env @param {string} context */ function assertImagesCached(images, env, context) { for (const image of images) { const result = spawn('docker', ['image', 'inspect', image], { env, label: `Docker image cache check for ${image}` }); @@ -149,6 +172,7 @@ function assertImagesCached(images, env, context) { } } +/** @param {NodeJS.ProcessEnv} env @returns {NodeJS.ProcessEnv} */ function localProxyEnvironment(env) { return { ...env, @@ -163,6 +187,7 @@ function localProxyEnvironment(env) { }; } +/** @param {string} projectName @param {string} envFile @param {string[]} args @returns {string[]} */ function composeArgs(projectName, envFile, args) { return [ 'compose', @@ -174,12 +199,14 @@ function composeArgs(projectName, envFile, args) { ]; } +/** @param {ComposeRunner} compose @param {string} service @param {number} targetPort @param {NodeJS.ProcessEnv} dockerEnv @returns {string} */ function publishedPort(compose, service, targetPort, dockerEnv) { const ids = compose(['ps', '--all', '--quiet', service]).split(/\r?\n/).filter(Boolean); - if (ids.length !== 1) throw new Error(`Expected one task-owned ${service} container, found ${ids.length}.`); + const [containerId] = ids; + if (ids.length !== 1 || !containerId) throw new Error(`Expected one task-owned ${service} container, found ${ids.length}.`); const portKey = `${targetPort}/tcp`; - const rawBindings = runQuiet('docker', ['inspect', '--format', `{{json (index .NetworkSettings.Ports "${portKey}")}}`, ids[0]], { env: dockerEnv }); + const rawBindings = runQuiet('docker', ['inspect', '--format', `{{json (index .NetworkSettings.Ports "${portKey}")}}`, containerId], { env: dockerEnv }); let bindings; try { bindings = JSON.parse(rawBindings); @@ -198,7 +225,9 @@ function publishedPort(compose, service, targetPort, dockerEnv) { return String(port); } +/** @param {{ database: string; user: string; password: string; port: string; psqlPath: string }} options @returns {NodeJS.ProcessEnv} */ function seedEnvironment({ database, user, password, port, psqlPath }) { + /** @type {NodeJS.ProcessEnv} */ const env = { ...process.env, HAPPYVIEW_DISPOSABLE_TEST_TARGET: 'YES', @@ -216,14 +245,17 @@ function seedEnvironment({ database, user, password, port, psqlPath }) { return env; } +/** @param {string} psqlPath @param {NodeJS.ProcessEnv} env @param {string} input */ function psql(psqlPath, env, input) { run(psqlPath, ['--no-psqlrc', '--quiet', '--set', 'ON_ERROR_STOP=1', ...psqlTargetArgs(env)], { env, input }); } +/** @param {string} value @returns {string} */ function sqlString(value) { return `'${value.replaceAll("'", "''")}'`; } +/** @param {string} psqlPath @param {NodeJS.ProcessEnv} env @param {string} token */ function bootstrapAdmin(psqlPath, env, token) { const now = new Date().toISOString(); const userId = randomUUID(); @@ -256,6 +288,7 @@ COMMIT; psql(psqlPath, env, sql); } +/** @param {string} baseUrl @param {number} [timeoutMs] @returns {Promise} */ async function waitForHealth(baseUrl, timeoutMs = 30_000) { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { @@ -270,6 +303,7 @@ async function waitForHealth(baseUrl, timeoutMs = 30_000) { throw new Error(`Task-owned HappyView did not become healthy at ${baseUrl}/health within ${timeoutMs}ms.`); } +/** @param {string[]} files @param {NodeJS.ProcessEnv} env */ function runHttpCases(files, env) { const relativeFiles = files.map((file) => path.relative(apiRoot, file)); const result = spawn(process.execPath, ['--test', '--test-reporter=tap', ...relativeFiles], { @@ -290,14 +324,17 @@ function runHttpCases(files, env) { process.stdout.write(`Verified ${tests[1]} executed HTTP cases (${passed[1]} passed) across ${files.length} suite files.\n`); } +/** @param {ComposeRunner} compose @param {string} projectName @param {string} envFile @param {NodeJS.ProcessEnv} dockerEnv @param {NodeJS.ProcessEnv} composeEnv */ function teardownOwnedProject(compose, projectName, envFile, dockerEnv, composeEnv) { const ids = compose(['ps', '--all', '--quiet']).split(/\r?\n/).filter(Boolean); for (const id of ids) { const rawLabels = runQuiet('docker', ['inspect', '--format', '{{json .Config.Labels}}', id], { env: dockerEnv }); - const labels = JSON.parse(rawLabels); + const labels = /** @type {Record} */ (JSON.parse(rawLabels)); const configFiles = (labels['com.docker.compose.project.config_files'] ?? '').split(',').map((file) => path.resolve(file)); + const service = labels['com.docker.compose.service']; if (labels['com.docker.compose.project'] !== projectName - || !services.has(labels['com.docker.compose.service']) + || typeof service !== 'string' + || !services.has(service) || !configFiles.includes(path.resolve(composeFile))) { throw new Error(`Refusing teardown: container ${id} is not owned by this task's Compose project and file.`); } @@ -305,6 +342,7 @@ function teardownOwnedProject(compose, projectName, envFile, dockerEnv, composeE run('docker', composeArgs(projectName, envFile, ['down', '--volumes', '--timeout', '10']), { env: composeEnv }); } +/** @returns {Promise} */ async function prepareRun() { const psqlPath = requirePsqlPath(); const suites = await findFiles(httpTestRoot, '.http.test.js'); @@ -346,6 +384,7 @@ async function prepareRun() { '', ].join('\n'), { mode: 0o600 }); + /** @type {ComposeRunner} */ const compose = (args) => runQuiet('docker', composeArgs(projectName, envFile, args), { env: composeEnv }); return { psqlPath, @@ -366,13 +405,16 @@ async function prepareRun() { }; } +/** @param {RunContext} context */ async function validateTaskProject(context) { const { lifecycleState } = context; + /** @type {ComposeConfig} */ let composeConfig; try { composeConfig = JSON.parse(context.compose(['config', '--format', 'json'])); } catch (cause) { - throw new Error(`Could not read the task Compose configuration before starting services: ${cause.message}`, { cause }); + const detail = cause instanceof Error ? cause.message : String(cause); + throw new Error(`Could not read the task Compose configuration before starting services: ${detail}`, { cause }); } assertImagesCached(imagesFromComposeConfig(composeConfig), context.dockerEnv, context.dockerContext); if (context.compose(['ps', '--all', '--quiet'])) throw new Error(`Refusing to reuse non-empty Compose project ${context.projectName}.`); @@ -386,6 +428,7 @@ async function validateTaskProject(context) { lifecycleState.ownsProject = true; } +/** @param {RunContext} context */ function startTaskServices(context) { process.stdout.write(`Starting disposable local HTTP test project ${context.projectName} (cached images only).\n`); run('docker', composeArgs(context.projectName, context.envFile, ['up', '--pull', 'never', '--detach', '--wait', '--wait-timeout', '180']), { @@ -395,6 +438,7 @@ function startTaskServices(context) { }); } +/** @param {RunContext} context @returns {Promise} */ async function installAndSeedApi(context) { const postgresPort = publishedPort(context.compose, 'postgres', 5432, context.dockerEnv); const happyviewPort = publishedPort(context.compose, 'happyview', 3000, context.dockerEnv); @@ -428,6 +472,7 @@ async function installAndSeedApi(context) { return baseUrl; } +/** @param {RunContext} context @param {string} baseUrl */ function runHttpSuites(context, baseUrl) { const testEnv = { ...localProxyEnvironment(process.env), @@ -437,6 +482,7 @@ function runHttpSuites(context, baseUrl) { runHttpCases(context.suites, testEnv); } +/** @param {RunContext} context */ async function runHttpLifecycle(context) { await validateTaskProject(context); startTaskServices(context); @@ -444,6 +490,7 @@ async function runHttpLifecycle(context) { runHttpSuites(context, baseUrl); } +/** @param {unknown} error @param {RunContext} context */ function reportRunFailure(error, context) { process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); if (!context.lifecycleState.ownsProject) return; @@ -456,6 +503,7 @@ function reportRunFailure(error, context) { } } +/** @param {RunContext} context */ async function cleanupRun(context) { const { lifecycleState } = context; if (lifecycleState.ownsProject) { diff --git a/api/tooling/seed.js b/api/tooling/seed.js index 95517df..5f3c941 100644 --- a/api/tooling/seed.js +++ b/api/tooling/seed.js @@ -8,33 +8,43 @@ import { activityFixtureRows } from '../tests/fixtures/activities.js'; import { badDateSeedSql } from '../tests/fixtures/bad-dates.js'; import { badDateLocations } from '../tests/fixtures/bad-location-dates.js'; +/** @typedef {{ uri: string; did: string; collection: string; rkey: string; cid: string; indexedAt: string; record: Record }} SeedRow */ +/** @typedef {Omit & { indexedAt: string | null; storedAt: string }} DateFixtureRow */ + +/** @param {NodeJS.ProcessEnv} env @returns {{ host: string; port: number; database: string; user: string | undefined }} */ function disposableTarget(env) { if (env.HAPPYVIEW_DISPOSABLE_TEST_TARGET !== 'YES') throw new Error('Set HAPPYVIEW_DISPOSABLE_TEST_TARGET=YES only after confirming this is a disposable test database'); if (env.PGHOSTADDR || env.PGSERVICE || env.PGSERVICEFILE) throw new Error('PGHOSTADDR, PGSERVICE, and PGSERVICEFILE are not allowed; they can override the loopback seed target'); if (!env.PGDATABASE || !/^\w[\w-]*$/.test(env.PGDATABASE) || !/(^|[_-])test([_-]|$)/i.test(env.PGDATABASE)) { throw new Error('PGDATABASE must be a simple database name containing a test marker (for example happyview_test), not a URI or conninfo string'); } - if (!['localhost', '127.0.0.1', '::1'].includes(env.PGHOST)) throw new Error('Set PGHOST explicitly to localhost, 127.0.0.1, or ::1'); + const host = env.PGHOST; + if (typeof host !== 'string' || !['localhost', '127.0.0.1', '::1'].includes(host)) throw new Error('Set PGHOST explicitly to localhost, 127.0.0.1, or ::1'); const port = env.PGPORT === undefined ? 5432 : Number(env.PGPORT); if (!Number.isInteger(port) || port < 1 || port > 65535) throw new Error('PGPORT must be an integer from 1 to 65535'); - return { host: env.PGHOST, port, database: env.PGDATABASE, user: env.PGUSER }; + return { host, port, database: env.PGDATABASE, user: env.PGUSER }; } +/** @param {unknown} value @returns {string} */ function quote(value) { if (value === null) return 'NULL'; const escapedBackslash = String.raw`\\`; return `E'${String(value).replaceAll('\\', escapedBackslash).replaceAll("'", "''")}'`; } +/** @param {{ sql: string; params: unknown[] }[]} statements @param {string} [types] @returns {string} */ function sqlInput(statements, types = 'text, text, text, text, jsonb, text, text') { + const [firstStatement] = statements; + if (!firstStatement) throw new Error('Seed SQL requires at least one statement; provide a nonempty fixture row set'); return [ String.raw`\set ON_ERROR_STOP on`, - `PREPARE happyview_fixture (${types}) AS ${statements[0].sql};`, + `PREPARE happyview_fixture (${types}) AS ${firstStatement.sql};`, ...statements.map(({ params }) => `EXECUTE happyview_fixture(${params.map(quote).join(', ')});`), 'DEALLOCATE happyview_fixture;', ].join('\n'); } +/** @param {NodeJS.ProcessEnv} [env] @param {SeedRow[]} [rows] @returns {string} */ export function buildSeedInput(env = process.env, rows = [ ...locationRecords, ...profileRecords, ...organizationRecords, ...actorFollowRecords, ...actorFollowProfileRecords, ...actorFollowOrganizationRecords, @@ -44,12 +54,14 @@ export function buildSeedInput(env = process.env, rows = [ return sqlInput(seedSql(rows, { disposableTestTarget: true })); } +/** @param {NodeJS.ProcessEnv} [env] @param {DateFixtureRow[]} [rows] @returns {string} */ export function buildBadDateSeedInput(env = process.env, rows = badDateLocations) { disposableTarget(env); if (!Array.isArray(rows) || rows.length === 0) throw new Error('Seeding requires at least one fixture row; supply a nonempty rows array'); return sqlInput(badDateSeedSql(rows, { disposableTestTarget: true }), 'text, text, text, text, jsonb, text, text, text'); } +/** @param {NodeJS.ProcessEnv} env @returns {string} */ function configuredPsqlPath(env) { const executable = env.PSQL_PATH; if (!executable || !path.isAbsolute(executable)) { @@ -59,11 +71,13 @@ function configuredPsqlPath(env) { if (!statSync(executable).isFile()) throw new Error('not a regular file'); accessSync(executable, constants.X_OK); } catch (error) { - throw new Error(`PSQL_PATH must point to an existing executable file (${executable}): ${error.message}`, { cause: error }); + const detail = error instanceof Error ? error.message : String(error); + throw new Error(`PSQL_PATH must point to an existing executable file (${executable}): ${detail}`, { cause: error }); } return executable; } +/** @param {NodeJS.ProcessEnv} [env] @returns {string[]} */ export function psqlTargetArgs(env = process.env) { const target = disposableTarget(env); const args = ['--host', target.host, '--port', String(target.port), '--dbname', target.database]; @@ -80,6 +94,7 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) const input = options.length === 1 ? buildBadDateSeedInput() : buildSeedInput(); const args = ['--no-psqlrc', '--set', 'ON_ERROR_STOP=1', ...psqlTargetArgs()]; const executable = configuredPsqlPath(process.env); + /** @type {NodeJS.ProcessEnv} */ const env = { ...process.env }; delete env.PGHOSTADDR; delete env.PGSERVICE; @@ -88,7 +103,7 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) if (result.error) throw result.error; if (result.status !== 0) throw new Error(`psql failed with exit status ${result.status}`); } catch (error) { - console.error(error.message); + console.error(error instanceof Error ? error.message : String(error)); process.exitCode = 1; } } diff --git a/api/tsconfig.json b/api/tsconfig.json index f96aaf3..3df7f7d 100644 --- a/api/tsconfig.json +++ b/api/tsconfig.json @@ -4,11 +4,16 @@ "checkJs": true, "noEmit": true, "strict": true, + "noUncheckedIndexedAccess": true, "skipLibCheck": true, "target": "ES2022", "module": "NodeNext", "moduleResolution": "NodeNext", "types": ["node"] }, - "include": ["tooling/installer.js"] + "include": [ + "tooling/installer.js", + "tooling/run-http-tests.js", + "tooling/seed.js" + ] } From 045ef78ae0f3991f52412b1d70f8eab5b5650a66 Mon Sep 17 00:00:00 2001 From: kzoeps Date: Wed, 7 Oct 2026 14:12:45 +0600 Subject: [PATCH 2/3] tooling: address Lua maintainability analysis findings --- .../unit/tooling/lua-maintainability.test.js | 92 ++++ api/tooling/lint-lua.js | 145 ++++-- api/tooling/lua-maintainability.js | 472 +++++++++++------- 3 files changed, 466 insertions(+), 243 deletions(-) diff --git a/api/tests/unit/tooling/lua-maintainability.test.js b/api/tests/unit/tooling/lua-maintainability.test.js index b8d17e2..fd0d8a5 100644 --- a/api/tests/unit/tooling/lua-maintainability.test.js +++ b/api/tests/unit/tooling/lua-maintainability.test.js @@ -48,6 +48,53 @@ test('parameter contracts report both missing and extra literal allow-list keys' }); }); +test('parameter mismatch names retain UTF-16 code-unit ordering', async () => { + await withPackage(async (root) => { + const mismatch = (await checkLuaParameterContracts({ root }))[0]; + + assert.deepEqual(mismatch.expected, ['𐀀', '']); + assert.deepEqual(mismatch.actual, ['unexpected', '𐀀', '']); + }, { + parameters: ['', '𐀀'], + source: 'function handle()\n keys_only(params, { [""] = true, ["𐀀"] = true, unexpected = true })\nend\n', + }); +}); + +test('helper candidate paths retain UTF-16 code-unit ordering in diagnostics', async () => { + await withPackage(async (root) => { + const codePointPath = 'lua/shared/𐀀.lua'; + const privateUsePath = 'lua/shared/.lua'; + for (const file of [codePointPath, privateUsePath]) { + const fullPath = path.join(root, file); + await mkdir(path.dirname(fullPath), { recursive: true }); + await writeFile(fullPath, 'local function helper() return true end\n'); + } + await writeFile(path.join(root, 'lua/src/listThings.lua'), 'function handle() return helper() end\n'); + + const modulePath = path.join(root, 'modules/demo/manifest.json'); + const module = JSON.parse(await readFile(modulePath, 'utf8')); + module.assets.push({ + kind: 'script', + id: 'xrpc.query:org.example.helperCatalog', + path: '../../lua/endpoints/helperCatalog.lua', + sourcePath: '../../lua/src/helperCatalog.lua', + sharedSourcePaths: [`../../${privateUsePath}`, `../../${codePointPath}`], + config: { script_type: 'lua' }, + }); + await writeFile(modulePath, JSON.stringify(module)); + await writeFile(path.join(root, 'lua/src/helperCatalog.lua'), 'function handle() return true end\n'); + + const { diagnostics } = await checkLuaMaintainability({ root }); + const finding = diagnostics.find(({ code, endpoint }) => code === 'helper-availability-unverified' + && endpoint === 'xrpc.query:org.example.listThings'); + assert.ok(finding); + assert.match(finding.message, /same-name declarations exist in lua\/shared\/𐀀\.lua, lua\/shared\/\.lua/); + }, { + parameters: ['authors'], + source: 'function handle() return helper() end\n', + }); +}); + test('matching literal parameter names pass, and Lua comments or strings are not code', async () => { await withPackage(async (root) => { const diagnostics = await checkLuaParameterContracts({ root }); @@ -58,6 +105,51 @@ test('matching literal parameter names pass, and Lua comments or strings are not }); }); +test('concurrent source reads retain handler and source declaration order', async () => { + await withPackage(async (root) => { + const modulePath = path.join(root, 'modules/demo/manifest.json'); + const module = JSON.parse(await readFile(modulePath, 'utf8')); + const firstHandler = module.assets.find(({ kind, id }) => kind === 'script' && id === 'xrpc.query:org.example.listThings'); + firstHandler.sourcePath = '../../lua/src/missingFirst.lua'; + module.assets.push({ + kind: 'script', + id: 'xrpc.query:org.example.secondThings', + path: '../../lua/endpoints/secondThings.lua', + sourcePath: '../../lua/src/missingSecond.lua', + sharedSourcePaths: [], + config: { script_type: 'lua' }, + }); + await writeFile(modulePath, JSON.stringify(module)); + + const { diagnostics } = await checkLuaMaintainability({ root }); + assert.deepEqual(diagnostics.filter(({ code }) => code === 'lua-source-missing').map(({ file }) => file), [ + 'lua/shared/first.lua', + 'lua/shared/second.lua', + 'lua/src/missingFirst.lua', + 'lua/src/missingSecond.lua', + ]); + }, { + parameters: ['authors'], + source: 'function handle() end\n', + sharedSourcePaths: ['../../lua/shared/first.lua', '../../lua/shared/second.lua'], + }); +}); + +test('module reads report the first manifest-order error when later reads fail sooner', async () => { + await withPackage(async (root) => { + await writeFile(path.join(root, 'manifest.json'), JSON.stringify({ modules: [ + 'modules/first.json', + '../outside.json', + ] })); + await writeFile(path.join(root, 'modules/first.json'), JSON.stringify({ assets: 'not an array' })); + + await assert.rejects( + checkLuaParameterContracts({ root }), + /Lua module manifest .* must declare an assets array/, + ); + }, { parameters: ['authors'], source: 'function handle() end\n' }); +}); + test('qualified custom keys_only methods are unverified, not parameter allow-lists', async () => { for (const source of [ 'function handle()\n custom.keys_only(params, { authors = true })\nend\n', diff --git a/api/tooling/lint-lua.js b/api/tooling/lint-lua.js index 801b4e2..d435d89 100644 --- a/api/tooling/lint-lua.js +++ b/api/tooling/lint-lua.js @@ -8,6 +8,19 @@ import { checkLuaMaintainability } from './lua-maintainability.js'; const defaultRoot = fileURLToPath(new URL('../', import.meta.url)); +async function mapConcurrentInOrder(items, mapItem) { + const results = await Promise.all(items.map(async (item) => { + try { + return { ok: true, value: await mapItem(item) }; + } catch (error) { + return { ok: false, error }; + } + })); + const firstFailure = results.find(({ ok }) => !ok); + if (firstFailure) throw firstFailure.error; + return results.map(({ value }) => value); +} + function resolveInside(root, base, relativePath, description) { if (typeof relativePath !== 'string' || !relativePath.trim()) { throw new Error(`${description} must be a nonempty path in the API manifests`); @@ -80,46 +93,58 @@ async function declaredLuaOutputs(root) { return outputs; } +async function readSourceMapDeclaration(root, moduleFile, asset, declaration) { + if (!declaration.path) throw new Error(`${asset.id ?? 'Lua handler'} has no authored ${declaration.kind} source for bundle diagnostics`); + const sourceFile = resolveInside(root, path.dirname(moduleFile), declaration.path, `${asset.id ?? 'Lua handler'} ${declaration.kind} source`); + try { + const source = await readFile(sourceFile, 'utf8'); + return { sourceFile, source }; + } catch (error) { + throw new Error(`${asset.id ?? 'Lua handler'}: cannot map generated bundle to ${path.relative(root, sourceFile)}: ${error.message}`, { cause: error }); + } +} + +async function sourceMapForAsset(root, moduleFile, asset) { + if (asset.kind !== 'script' || asset.config?.script_type !== 'lua') return null; + const shared = asset.sharedSourcePaths + ?? (asset.sharedSourcePath ? [asset.sharedSourcePath] : []); + if (!Array.isArray(shared)) throw new Error(`${asset.id ?? 'Lua handler'} must declare sharedSourcePaths as an array`); + const declarations = [ + ...shared.map((source) => ({ path: source, kind: 'shared' })), + { path: asset.sourcePath, kind: 'handler' }, + ]; + const loadedSources = await mapConcurrentInOrder(declarations, (declaration) => ( + readSourceMapDeclaration(root, moduleFile, asset, declaration) + )); + const ranges = []; + let nextLine = 1; + for (const { sourceFile, source } of loadedSources) { + const trimmed = source.trimEnd(); + const lineCount = trimmed.length === 0 ? 1 : (trimmed.match(/\n/g) ?? []).length + 1; + ranges.push({ + startLine: nextLine, + endLine: nextLine + lineCount - 1, + sourceFile: path.relative(root, sourceFile), + }); + nextLine += lineCount + 1; + } + const outputFile = resolveInside(root, path.dirname(moduleFile), asset.path, `Lua handler ${asset.id ?? '(unnamed)'} output path`); + return [path.resolve(outputFile), { endpoint: asset.id ?? '(unnamed Lua handler)', ranges }]; +} + +async function sourceMapsForModule(root, modulePath) { + const moduleFile = resolveInside(root, root, modulePath, 'Module manifest path'); + const module = JSON.parse(await readFile(moduleFile, 'utf8')); + const maps = await mapConcurrentInOrder(module.assets, (asset) => sourceMapForAsset(root, moduleFile, asset)); + return maps.filter((sourceMap) => sourceMap !== null); +} + async function declaredLuaSourceMaps(root) { const rootManifest = JSON.parse(await readFile(path.join(root, 'manifest.json'), 'utf8')); - const sourceMaps = new Map(); - for (const modulePath of rootManifest.modules) { - const moduleFile = resolveInside(root, root, modulePath, 'Module manifest path'); - const module = JSON.parse(await readFile(moduleFile, 'utf8')); - for (const asset of module.assets) { - if (asset.kind !== 'script' || asset.config?.script_type !== 'lua') continue; - const shared = asset.sharedSourcePaths - ?? (asset.sharedSourcePath ? [asset.sharedSourcePath] : []); - if (!Array.isArray(shared)) throw new Error(`${asset.id ?? 'Lua handler'} must declare sharedSourcePaths as an array`); - const declarations = [ - ...shared.map((source) => ({ path: source, kind: 'shared' })), - { path: asset.sourcePath, kind: 'handler' }, - ]; - const ranges = []; - let nextLine = 1; - for (const declaration of declarations) { - if (!declaration.path) throw new Error(`${asset.id ?? 'Lua handler'} has no authored ${declaration.kind} source for bundle diagnostics`); - const sourceFile = resolveInside(root, path.dirname(moduleFile), declaration.path, `${asset.id ?? 'Lua handler'} ${declaration.kind} source`); - let source; - try { - source = await readFile(sourceFile, 'utf8'); - } catch (error) { - throw new Error(`${asset.id ?? 'Lua handler'}: cannot map generated bundle to ${path.relative(root, sourceFile)}: ${error.message}`, { cause: error }); - } - const trimmed = source.trimEnd(); - const lineCount = trimmed.length === 0 ? 1 : (trimmed.match(/\n/g) ?? []).length + 1; - ranges.push({ - startLine: nextLine, - endLine: nextLine + lineCount - 1, - sourceFile: path.relative(root, sourceFile), - }); - nextLine += lineCount + 1; - } - const outputFile = resolveInside(root, path.dirname(moduleFile), asset.path, `Lua handler ${asset.id ?? '(unnamed)'} output path`); - sourceMaps.set(path.resolve(outputFile), { endpoint: asset.id ?? '(unnamed Lua handler)', ranges }); - } - } - return sourceMaps; + const moduleMaps = await mapConcurrentInOrder(rootManifest.modules, (modulePath) => ( + sourceMapsForModule(root, modulePath) + )); + return new Map(moduleMaps.flat()); } function mapLuacheckOutput(output, root, sourceMaps) { @@ -142,20 +167,18 @@ function emitLintOutput(output, log, stream) { else log(output); } -export async function lintLua({ root = defaultRoot, home = homedir(), spawn = spawnSync, log = console.log } = {}) { +async function declaredEndpointFiles(root, log) { const allFiles = await luaFiles(path.join(root, 'lua')); if (allFiles.length === 0) { log('No Lua files in this branch; skipping Luacheck.'); - return { skipped: true }; + return null; } - const endpointFiles = await declaredLuaOutputs(root); if (endpointFiles.length === 0) { log('No Lua endpoint handlers declared in this branch; skipping Luacheck.'); - return { skipped: true }; + return null; } - - for (const file of endpointFiles) { + await mapConcurrentInOrder(endpointFiles, async (file) => { try { await access(file); } catch (error) { @@ -164,20 +187,16 @@ export async function lintLua({ root = defaultRoot, home = homedir(), spawn = sp } throw error; } - } + }); + return endpointFiles; +} - const maintainability = await checkLuaMaintainability({ root }); +function logMaintainabilityDiagnostics(maintainability, log) { for (const diagnostic of maintainability.diagnostics) log(`[Lua ${diagnostic.status}] ${diagnostic.message}`); log(`Lua parameter contracts: ${maintainability.stats.parameterContractsVerified} verified, ${maintainability.stats.parameterContractsUnverified} unverified.`); +} - const sourceMaps = await declaredLuaSourceMaps(root); - const args = [ - '--config', - '.luacheckrc', - '--no-color', - ...endpointFiles.map((file) => path.relative(root, file)), - ]; - const options = { cwd: root, encoding: 'utf8', stdio: 'pipe' }; +async function runLuacheck(args, options, home, spawn) { let result = spawn('luacheck', args, options); if (result.error?.code === 'ENOENT') { const localLuacheck = path.join(home, '.luarocks', 'bin', 'luacheck'); @@ -192,6 +211,24 @@ export async function lintLua({ root = defaultRoot, home = homedir(), spawn = sp throw new Error('Luacheck is missing. Install it with `luarocks --lua-version=5.4 --local install luacheck 1.2.0`.'); } if (result.error) throw result.error; + return result; +} + +export async function lintLua({ root = defaultRoot, home = homedir(), spawn = spawnSync, log = console.log } = {}) { + const endpointFiles = await declaredEndpointFiles(root, log); + if (!endpointFiles) return { skipped: true }; + + const maintainability = await checkLuaMaintainability({ root }); + logMaintainabilityDiagnostics(maintainability, log); + const sourceMaps = await declaredLuaSourceMaps(root); + const args = [ + '--config', + '.luacheckrc', + '--no-color', + ...endpointFiles.map((file) => path.relative(root, file)), + ]; + const options = { cwd: root, encoding: 'utf8', stdio: 'pipe' }; + const result = await runLuacheck(args, options, home, spawn); emitLintOutput(mapLuacheckOutput(String(result.stdout ?? ''), root, sourceMaps), log, process.stdout); emitLintOutput(mapLuacheckOutput(String(result.stderr ?? ''), root, sourceMaps), log, process.stderr); const maintainabilityFailed = maintainability.diagnostics.some(({ status }) => status === 'error'); diff --git a/api/tooling/lua-maintainability.js b/api/tooling/lua-maintainability.js index 71bd220..987fdb9 100644 --- a/api/tooling/lua-maintainability.js +++ b/api/tooling/lua-maintainability.js @@ -1,6 +1,23 @@ import { readFile } from 'node:fs/promises'; import path from 'node:path'; +function compareCodeUnits(left, right) { + return left < right ? -1 : left > right ? 1 : 0; +} + +async function mapConcurrentInOrder(items, mapItem) { + const results = await Promise.all(items.map(async (item, index) => { + try { + return { ok: true, value: await mapItem(item, index) }; + } catch (error) { + return { ok: false, error }; + } + })); + const firstFailure = results.find(({ ok }) => !ok); + if (firstFailure) throw firstFailure.error; + return results.map(({ value }) => value); +} + function resolveInside(root, base, relativePath, description) { if (typeof relativePath !== 'string' || !relativePath.trim()) { throw new Error(`${description} must be a nonempty path in the API manifests`); @@ -27,15 +44,17 @@ async function loadRegisteredApi(root) { const rootManifest = await readJson(manifestFile, 'root API manifest'); if (!Array.isArray(rootManifest.modules)) throw new Error(`Root API manifest ${manifestFile} must declare a modules array`); - const modules = []; - const handlers = []; - const lexicons = new Map(); - for (const modulePath of rootManifest.modules) { + const manifestEntries = await mapConcurrentInOrder(rootManifest.modules, async (modulePath) => { const moduleFile = resolveInside(packageRoot, packageRoot, modulePath, 'Module manifest path'); const module = await readJson(moduleFile, 'module manifest'); if (!Array.isArray(module.assets)) throw new Error(`Lua module manifest ${moduleFile} must declare an assets array`); - const entry = { file: moduleFile, assets: module.assets }; - modules.push(entry); + return { moduleFile, module }; + }); + const modules = []; + const handlers = []; + const lexicons = new Map(); + for (const { moduleFile, module } of manifestEntries) { + modules.push({ file: moduleFile, assets: module.assets }); for (const asset of module.assets) { if (asset.kind === 'lexicon' && typeof asset.id === 'string') { const records = lexicons.get(asset.id) ?? []; @@ -67,38 +86,39 @@ async function authoredSources(api, handlerRecord) { ...declaredSharedSources(asset).map((source) => ({ path: source, kind: 'shared' })), { path: asset.sourcePath, kind: 'handler' }, ]; - const sources = []; - const diagnostics = []; - - for (const declaration of declarations) { + const results = await mapConcurrentInOrder(declarations, async (declaration) => { if (!declaration.path) { - diagnostics.push({ - status: 'error', - code: 'lua-source-missing', - endpoint, - message: `${endpoint}: no authored ${declaration.kind} source is declared in ${path.relative(api.packageRoot, moduleFile)}; add ${declaration.kind === 'handler' ? 'sourcePath' : 'sharedSourcePaths'} to the module manifest.`, - }); - continue; + return { + diagnostic: { + status: 'error', + code: 'lua-source-missing', + endpoint, + message: `${endpoint}: no authored ${declaration.kind} source is declared in ${path.relative(api.packageRoot, moduleFile)}; add ${declaration.kind === 'handler' ? 'sourcePath' : 'sharedSourcePaths'} to the module manifest.`, + }, + }; } const file = resolveInside(api.packageRoot, moduleDirectory, declaration.path, `${endpoint} ${declaration.kind} source`); const relativeFile = path.relative(api.packageRoot, file); - let source; try { - source = await readFile(file, 'utf8'); + const source = await readFile(file, 'utf8'); + return { source: { file, relativeFile, kind: declaration.kind, source } }; } catch (error) { if (error.code !== 'ENOENT') throw error; - diagnostics.push({ - status: 'error', - code: 'lua-source-missing', - endpoint, - file: relativeFile, - message: `${endpoint}: declared ${declaration.kind} Lua source ${relativeFile} is missing (from ${path.relative(api.packageRoot, moduleFile)}); restore it or correct the manifest source path.`, - }); - continue; + return { + diagnostic: { + status: 'error', + code: 'lua-source-missing', + endpoint, + file: relativeFile, + message: `${endpoint}: declared ${declaration.kind} Lua source ${relativeFile} is missing (from ${path.relative(api.packageRoot, moduleFile)}); restore it or correct the manifest source path.`, + }, + }; } - sources.push({ file, relativeFile, kind: declaration.kind, source }); - } - return { sources, diagnostics }; + }); + return { + sources: results.flatMap(({ source }) => source ? [source] : []), + diagnostics: results.flatMap(({ diagnostic }) => diagnostic ? [diagnostic] : []), + }; } function longBracketEnd(source, start) { @@ -109,74 +129,112 @@ function longBracketEnd(source, start) { return end < 0 ? source.length : end + close.length; } +function countLineBreaks(source) { + return (source.match(/\n/g) ?? []).length; +} + +function consumeLuaWhitespace(source, index, line) { + if (!/\s/.test(source[index])) return null; + return { index: index + 1, line: line + (source[index] === '\n' ? 1 : 0) }; +} + +function consumeLuaComment(source, index, line) { + if (!source.startsWith('--', index)) return null; + const longEnd = longBracketEnd(source, index + 2); + if (longEnd !== null) { + const comment = source.slice(index, longEnd); + return { index: longEnd, line: line + countLineBreaks(comment) }; + } + const newline = source.indexOf('\n', index); + return { index: newline < 0 ? source.length : newline, line }; +} + +function consumeQuotedLuaString(source, index, line, tokens) { + const startLine = line; + const quote = source[index]; + let value = ''; + let staticValue = true; + let nextIndex = index + 1; + let nextLine = line; + while (nextIndex < source.length && source[nextIndex] !== quote) { + if (source[nextIndex] === '\n') nextLine += 1; + if (source[nextIndex] === '\\') { + staticValue = false; + nextIndex += Math.min(2, source.length - nextIndex); + } else { + value += source[nextIndex]; + nextIndex += 1; + } + } + if (source[nextIndex] === quote) nextIndex += 1; + tokens.push({ type: 'string', value: staticValue ? value : null, line: startLine }); + return { index: nextIndex, line: nextLine }; +} + +function consumeLongLuaString(source, index, line, tokens) { + const longEnd = source[index] === '[' ? longBracketEnd(source, index) : null; + if (longEnd === null) return null; + const raw = source.slice(index, longEnd); + tokens.push({ type: 'string', value: null, line }); + return { index: longEnd, line: line + countLineBreaks(raw) }; +} + +function consumeLuaWord(source, index, line, tokens, pattern, type) { + const match = source.slice(index).match(pattern); + if (!match) return null; + tokens.push({ type, value: match[0], line }); + return index + match[0].length; +} + +function consumeLuaSymbol(source, index, line, tokens) { + const operator = ['...', '..', '==', '~=', '<=', '>=', '//', '::'] + .find((candidate) => source.startsWith(candidate, index)); + const value = operator ?? source[index]; + tokens.push({ type: 'symbol', value, line }); + return index + value.length; +} + function tokenizeLua(source) { const tokens = []; let index = 0; let line = 1; while (index < source.length) { - const char = source[index]; - if (/\s/.test(char)) { - if (char === '\n') line += 1; - index += 1; + const whitespace = consumeLuaWhitespace(source, index, line); + if (whitespace !== null) { + index = whitespace.index; + line = whitespace.line; continue; } - if (source.startsWith('--', index)) { - const commentStart = index + 2; - const longEnd = longBracketEnd(source, commentStart); - if (longEnd !== null) { - line += (source.slice(index, longEnd).match(/\n/g) ?? []).length; - index = longEnd; - } else { - const newline = source.indexOf('\n', index); - index = newline < 0 ? source.length : newline; - } + const comment = consumeLuaComment(source, index, line); + if (comment !== null) { + index = comment.index; + line = comment.line; continue; } + const char = source[index]; if (char === '"' || char === "'") { - const startLine = line; - const quote = char; - index += 1; - let value = ''; - let staticValue = true; - while (index < source.length && source[index] !== quote) { - if (source[index] === '\n') line += 1; - if (source[index] === '\\') { - staticValue = false; - index += Math.min(2, source.length - index); - } else { - value += source[index]; - index += 1; - } - } - if (source[index] === quote) index += 1; - tokens.push({ type: 'string', value: staticValue ? value : null, line: startLine }); + const string = consumeQuotedLuaString(source, index, line, tokens); + index = string.index; + line = string.line; continue; } - const longEnd = char === '[' ? longBracketEnd(source, index) : null; - if (longEnd !== null) { - const raw = source.slice(index, longEnd); - const startLine = line; - line += (raw.match(/\n/g) ?? []).length; - tokens.push({ type: 'string', value: null, line: startLine }); - index = longEnd; + const longString = consumeLongLuaString(source, index, line, tokens); + if (longString !== null) { + index = longString.index; + line = longString.line; continue; } - const identifier = source.slice(index).match(/^[A-Za-z_][A-Za-z0-9_]*/); - if (identifier) { - tokens.push({ type: 'identifier', value: identifier[0], line }); - index += identifier[0].length; + const identifierEnd = consumeLuaWord(source, index, line, tokens, /^[A-Za-z_]\w*/, 'identifier'); + if (identifierEnd !== null) { + index = identifierEnd; continue; } - const number = source.slice(index).match(/^\d+(?:\.\d+)?/); - if (number) { - tokens.push({ type: 'number', value: number[0], line }); - index += number[0].length; + const numberEnd = consumeLuaWord(source, index, line, tokens, /^\d+(?:\.\d+)?/, 'number'); + if (numberEnd !== null) { + index = numberEnd; continue; } - const operator = ['...', '..', '==', '~=', '<=', '>=', '//', '::'].find((candidate) => source.startsWith(candidate, index)); - const value = operator ?? char; - tokens.push({ type: 'symbol', value, line }); - index += value.length; + index = consumeLuaSymbol(source, index, line, tokens); } return tokens; } @@ -325,20 +383,25 @@ function queryParameters(lexicon) { if (main?.type !== 'query') return { status: 'unverified', reason: 'the registered Lexicon is not a recognizable query' }; const parameters = main.parameters; if (parameters === undefined) return { status: 'verified', names: [] }; - if (!parameters || parameters.type !== 'params' || !parameters.properties || typeof parameters.properties !== 'object' || Array.isArray(parameters.properties)) { + if (parameters?.type !== 'params' || !parameters.properties || typeof parameters.properties !== 'object' || Array.isArray(parameters.properties)) { return { status: 'unverified', reason: 'the Lexicon parameter schema is not a recognizable params object' }; } - return { status: 'verified', names: Object.keys(parameters.properties).sort() }; + return { status: 'verified', names: Object.keys(parameters.properties).sort(compareCodeUnits) }; } function unverifiedParameter(id, file, reason, line) { + let location = ''; + if (file) { + location = ` in ${file}`; + if (line) location += `:${line}`; + } return { status: 'unverified', code: 'parameter-contract-unverified', endpoint: id, file, line, - message: `${id}: parameter contract unverified${file ? ` in ${file}${line ? `:${line}` : ''}` : ''}: ${reason}`, + message: `${id}: parameter contract unverified${location}: ${reason}`, }; } @@ -384,11 +447,15 @@ async function compareParameterContracts(api, handlerRecord, sources) { query: true, }; } - const actual = [...new Set(inspection.keys)].sort(); + const actual = [...new Set(inspection.keys)].sort(compareCodeUnits); const missing = contract.names.filter((name) => !actual.includes(name)); const extra = actual.filter((name) => !contract.names.includes(name)); if (!missing.length && !extra.length) return { diagnostics: [], verified: true, query: true }; const file = inspection.file; + const mismatchDetails = []; + if (missing.length) mismatchDetails.push(`missing: ${missing.join(', ')}`); + if (extra.length) mismatchDetails.push(`extra: ${extra.join(', ')}`); + const mismatchSuffix = mismatchDetails.length > 0 ? `; ${mismatchDetails.join('; ')}` : ''; return { diagnostics: [{ status: 'error', @@ -400,7 +467,7 @@ async function compareParameterContracts(api, handlerRecord, sources) { actual, missing, extra, - message: `${asset.id}: Lua parameter allow-list in ${file}:${inspection.line} differs from its Lexicon${missing.length ? `; missing: ${missing.join(', ')}` : ''}${extra.length ? `; extra: ${extra.join(', ')}` : ''}`, + message: `${asset.id}: Lua parameter allow-list in ${file}:${inspection.line} differs from its Lexicon${mismatchSuffix}`, }], verified: true, query: true, @@ -434,39 +501,48 @@ async function luacheckGlobals(root) { return globals; } +function addFunctionDeclaration(tokens, index, blocks, declarations) { + if (tokens[index + 1]?.type !== 'identifier' || tokens[index + 2]?.value !== '(') return; + declarations.push({ + name: tokens[index + 1].value, + line: tokens[index].line, + index, + topLevel: blocks.length === 0, + local: tokens[index - 1]?.value === 'local', + }); +} + +function updateFunctionBlocks(value, blocks, loopsAwaitingDo) { + if (value === 'function' || value === 'if' || value === 'repeat') { + blocks.push(value); + return loopsAwaitingDo; + } + if (value === 'for' || value === 'while') { + blocks.push('loop'); + return loopsAwaitingDo + 1; + } + if (value === 'do') { + if (loopsAwaitingDo > 0) return loopsAwaitingDo - 1; + blocks.push('do'); + return loopsAwaitingDo; + } + if (value === 'end' && blocks.length > 0 && blocks.at(-1) !== 'repeat') { + blocks.pop(); + return loopsAwaitingDo; + } + if (value === 'until' && blocks.at(-1) === 'repeat') blocks.pop(); + return loopsAwaitingDo; +} + function functionDeclarations(tokens) { const declarations = []; const blocks = []; let loopsAwaitingDo = 0; for (let index = 0; index < tokens.length; index += 1) { if (tokens[index].type !== 'identifier') continue; - const value = tokens[index].value; - if (value === 'function') { - if (tokens[index + 1]?.type === 'identifier' && tokens[index + 2]?.value === '(') { - declarations.push({ - name: tokens[index + 1].value, - line: tokens[index].line, - index, - topLevel: blocks.length === 0, - local: tokens[index - 1]?.value === 'local', - }); - } - blocks.push('function'); - } else if (value === 'if') { - blocks.push('if'); - } else if (value === 'for' || value === 'while') { - blocks.push('loop'); - loopsAwaitingDo += 1; - } else if (value === 'do') { - if (loopsAwaitingDo > 0) loopsAwaitingDo -= 1; - else blocks.push('do'); - } else if (value === 'repeat') { - blocks.push('repeat'); - } else if (value === 'end') { - if (blocks.length > 0 && blocks.at(-1) !== 'repeat') blocks.pop(); - } else if (value === 'until' && blocks.at(-1) === 'repeat') { - blocks.pop(); - } + const { value } = tokens[index]; + if (value === 'function') addFunctionDeclaration(tokens, index, blocks, declarations); + loopsAwaitingDo = updateFunctionBlocks(value, blocks, loopsAwaitingDo); } return declarations; } @@ -495,84 +571,103 @@ function helperCatalog(sources) { } function helperAvailabilityDiagnostic(endpoint, source, call, definitions) { - const candidates = [...new Set(definitions.map(({ relativeFile }) => relativeFile))].sort(); + const candidates = [...new Set(definitions.map(({ relativeFile }) => relativeFile))].sort(compareCodeUnits); + const candidateDetails = candidates.length > 0 + ? `; same-name declarations exist in ${candidates.join(', ')}` + : ''; return { status: 'unverified', code: 'helper-availability-unverified', endpoint, file: source.relativeFile, line: call.line, - message: `${endpoint}: cannot prove that shared helper ${call.name} at ${source.relativeFile}:${call.line} resolves to a top-level local declared earlier in this ordered bundle${candidates.length ? `; same-name declarations exist in ${candidates.join(', ')}` : ''}. Luacheck remains authoritative for undefined globals.`, + message: `${endpoint}: cannot prove that shared helper ${call.name} at ${source.relativeFile}:${call.line} resolves to a top-level local declared earlier in this ordered bundle${candidateDetails}. Luacheck remains authoritative for undefined globals.`, }; } -function missingHelperDiagnostics(endpoint, sources, catalog, globals) { - const inBundle = new Map(); - const tokenized = sources.map((source, sourceIndex) => ({ source, sourceIndex, tokens: tokenizeLua(source.source) })); - for (const item of tokenized) { - for (const declaration of functionDeclarations(item.tokens)) { - const definitions = inBundle.get(declaration.name) ?? []; - definitions.push({ ...declaration, sourceIndex: item.sourceIndex, file: item.source.file, relativeFile: item.source.relativeFile }); - inBundle.set(declaration.name, definitions); +function bundleFunctionDeclarations(tokenized) { + const declarations = new Map(); + for (const { source, sourceIndex, tokens } of tokenized) { + for (const declaration of functionDeclarations(tokens)) { + const definitions = declarations.get(declaration.name) ?? []; + definitions.push({ ...declaration, sourceIndex, file: source.file, relativeFile: source.relativeFile }); + declarations.set(declaration.name, definitions); } } + return declarations; +} + +function helperIsVisible(declarations, name, sourceIndex, callIndex) { + return (declarations.get(name) ?? []).some((definition) => definition.local && definition.topLevel + && (definition.sourceIndex < sourceIndex || (definition.sourceIndex === sourceIndex && definition.index < callIndex))); +} + +function missingHelperDiagnostic(endpoint, source, call, bundleFiles, catalog) { + const candidates = catalog.get(call.name) ?? []; + if (candidates.length === 0) return null; + const bundled = candidates.filter(({ file }) => bundleFiles.has(file)); + if (bundled.length > 0) return helperAvailabilityDiagnostic(endpoint, source, call, bundled); + if (candidates.length !== 1 || !candidates[0].local || !candidates[0].topLevel) { + return helperAvailabilityDiagnostic(endpoint, source, call, candidates); + } + return { + status: 'warning', + code: 'missing-helper-dependency', + endpoint, + file: source.relativeFile, + line: call.line, + message: `${endpoint}: possible missing shared dependency for ${call.name} at ${source.relativeFile}:${call.line}; the only registered top-level local declaration is in ${candidates[0].relativeFile}. Add it to sharedSourcePaths if intended. Luacheck remains authoritative for undefined globals.`, + }; +} +function missingHelperDiagnostics(endpoint, sources, catalog, globals) { + const tokenized = sources.map((source, sourceIndex) => ({ source, sourceIndex, tokens: tokenizeLua(source.source) })); + const declarations = bundleFunctionDeclarations(tokenized); + const bundleFiles = new Set(tokenized.map(({ source }) => source.file)); const diagnostics = []; const reported = new Set(); for (const { source, sourceIndex, tokens } of tokenized) { for (const call of functionCalls(tokens)) { - if (globals.has(call.name)) continue; - const visible = (inBundle.get(call.name) ?? []).some((definition) => definition.local && definition.topLevel - && (definition.sourceIndex < sourceIndex || (definition.sourceIndex === sourceIndex && definition.index < call.index))); - if (visible) continue; - const candidates = catalog.get(call.name) ?? []; - if (candidates.length === 0) continue; - + if (globals.has(call.name) || helperIsVisible(declarations, call.name, sourceIndex, call.index)) continue; + const diagnostic = missingHelperDiagnostic(endpoint, source, call, bundleFiles, catalog); + if (!diagnostic) continue; const key = `${source.file}:${call.line}:${call.name}`; if (reported.has(key)) continue; reported.add(key); - const bundleDeclarations = candidates.filter(({ file }) => tokenized.some(({ source: included }) => included.file === file)); - if (bundleDeclarations.length > 0) { - diagnostics.push(helperAvailabilityDiagnostic(endpoint, source, call, bundleDeclarations)); - continue; - } - - if (candidates.length === 1 && candidates[0].local && candidates[0].topLevel) { - diagnostics.push({ - status: 'warning', - code: 'missing-helper-dependency', - endpoint, - file: source.relativeFile, - line: call.line, - message: `${endpoint}: possible missing shared dependency for ${call.name} at ${source.relativeFile}:${call.line}; the only registered top-level local declaration is in ${candidates[0].relativeFile}. Add it to sharedSourcePaths if intended. Luacheck remains authoritative for undefined globals.`, - }); - } else { - diagnostics.push(helperAvailabilityDiagnostic(endpoint, source, call, candidates)); - } + diagnostics.push(diagnostic); } } return diagnostics; } +function callArguments(tokens, openIndex) { + const close = matchingClose(tokens, openIndex, '(', ')'); + return close < 0 ? null : splitTopLevel(tokens.slice(openIndex + 1, close), ','); +} + +function directDbRawExpression(tokens, index) { + const isDbRaw = tokens[index].value === 'db' && tokens[index + 1].value === '.' + && tokens[index + 2].value === 'raw' && tokens[index + 3].value === '('; + if (!isDbRaw) return null; + const args = callArguments(tokens, index + 3); + return args?.[0]?.length ? { expression: args[0], line: tokens[index].line } : null; +} + +function protectedDbRawExpression(tokens, index) { + if (tokens[index].value !== 'pcall' || tokens[index + 1].value !== '(') return null; + const args = callArguments(tokens, index + 1); + if (args?.[0]?.length !== 3 || args[0][0].value !== 'db' || args[0][1].value !== '.' + || args[0][2].value !== 'raw' || !args[1]?.length) return null; + return { expression: args[1], line: tokens[index].line }; +} + function rawSqlExpressions(tokens) { const expressions = []; for (let index = 0; index + 3 < tokens.length; index += 1) { - const isDbRaw = tokens[index].value === 'db' && tokens[index + 1].value === '.' - && tokens[index + 2].value === 'raw' && tokens[index + 3].value === '('; - if (isDbRaw) { - const close = matchingClose(tokens, index + 3, '(', ')'); - if (close >= 0) { - const args = splitTopLevel(tokens.slice(index + 4, close), ','); - if (args[0]?.length) expressions.push({ expression: args[0], line: tokens[index].line }); - } - } - - if (tokens[index].value !== 'pcall' || tokens[index + 1].value !== '(') continue; - const close = matchingClose(tokens, index + 1, '(', ')'); - if (close < 0) continue; - const args = splitTopLevel(tokens.slice(index + 2, close), ','); - if (args[0]?.length === 3 && args[0][0].value === 'db' && args[0][1].value === '.' && args[0][2].value === 'raw' - && args[1]?.length) expressions.push({ expression: args[1], line: tokens[index].line }); + const directExpression = directDbRawExpression(tokens, index); + if (directExpression) expressions.push(directExpression); + const protectedExpression = protectedDbRawExpression(tokens, index); + if (protectedExpression) expressions.push(protectedExpression); } return expressions; } @@ -612,30 +707,28 @@ function sqlInterpolationDiagnostics(endpoint, sources) { } async function parameterCheck(api, handlersAndSources) { - const diagnostics = []; - let queryCount = 0; - let verifiedCount = 0; - let unverifiedCount = 0; - for (const item of handlersAndSources) { - const result = await compareParameterContracts(api, item.handler, item.sources); - diagnostics.push(...result.diagnostics); - if (!result.query) continue; - queryCount += 1; - if (result.verified) verifiedCount += 1; - else unverifiedCount += 1; - } - return { diagnostics, queryCount, verifiedCount, unverifiedCount }; + const results = await mapConcurrentInOrder(handlersAndSources, ({ handler, sources }) => ( + compareParameterContracts(api, handler, sources) + )); + const queries = results.filter(({ query }) => query); + const verifiedCount = queries.filter(({ verified }) => verified).length; + return { + diagnostics: results.flatMap(({ diagnostics }) => diagnostics), + queryCount: queries.length, + verifiedCount, + unverifiedCount: queries.length - verifiedCount, + }; } async function loadHandlerSources(api) { - const loaded = []; - const diagnostics = []; - for (const handler of api.handlers) { - const result = await authoredSources(api, handler); - diagnostics.push(...result.diagnostics); - loaded.push({ handler, sources: result.sources }); - } - return { loaded, diagnostics }; + const results = await mapConcurrentInOrder(api.handlers, async (handler) => ({ + handler, + ...await authoredSources(api, handler), + })); + return { + loaded: results.map(({ handler, sources }) => ({ handler, sources })), + diagnostics: results.flatMap(({ diagnostics }) => diagnostics), + }; } async function allDeclaredSharedSources(api) { @@ -646,15 +739,16 @@ async function allDeclaredSharedSources(api) { sourceDeclarations.set(file, path.relative(api.packageRoot, file)); } } - const sources = []; - for (const [file, relativeFile] of sourceDeclarations) { + const declarations = [...sourceDeclarations]; + const sources = await mapConcurrentInOrder(declarations, async ([file, relativeFile]) => { try { - sources.push({ file, relativeFile, source: await readFile(file, 'utf8') }); + return { file, relativeFile, source: await readFile(file, 'utf8') }; } catch (error) { if (error.code !== 'ENOENT') throw error; + return null; } - } - return sources; + }); + return sources.filter((source) => source !== null); } export async function checkLuaParameterContracts({ root }) { From 76da41d8804c8ca7147ab00e18684630fc728f53 Mon Sep 17 00:00:00 2001 From: kzoeps Date: Wed, 7 Oct 2026 14:16:25 +0600 Subject: [PATCH 3/3] tooling: simplify deterministic string comparator --- api/tooling/lua-maintainability.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/tooling/lua-maintainability.js b/api/tooling/lua-maintainability.js index 987fdb9..9a3f696 100644 --- a/api/tooling/lua-maintainability.js +++ b/api/tooling/lua-maintainability.js @@ -2,7 +2,9 @@ import { readFile } from 'node:fs/promises'; import path from 'node:path'; function compareCodeUnits(left, right) { - return left < right ? -1 : left > right ? 1 : 0; + if (left < right) return -1; + if (left > right) return 1; + return 0; } async function mapConcurrentInOrder(items, mapItem) {