Merge refreshed PR #3457: preserve current source ancestry and new regression cases

This commit is contained in:
haelyra
2026-10-10 18:13:55 -04:00
2 changed files with 50 additions and 0 deletions
+7
View File
@@ -102,3 +102,10 @@ Third review follow-up. Review of that head found four more problems, each repro
Two first-round findings had stayed open and still reproduced. A member call such as `records.require('./label')` matched the call pattern, because the dot satisfies a word boundary, so a self-contained object method failed every projection. And the dynamic-argument pattern allowed only one level of nested parentheses, so `require(path.join(__dirname, getName()))` produced no warning while `require(getName())` did. A seventh RED added 3 cases, all failing. GREEN skips calls preceded by a dot, including after whitespace or optional chaining, and finds each call's argument by balanced-paren scanning over the string-free skeleton. Rescanning the 802 files changed 6 results, each a newly reported nested dynamic argument in a docker eval check or a test, none of them in a carrier. The real registry is unchanged.
Focused verification: 22 carrier cases, 12 compiler cases, 57 closure cases, 3 CI cases and 2 publish-surface cases pass with zero failures or skips. ESLint passes on the validator and the new test. The validator is CI-only and stays out of the packed surface, matching `scripts/ci/validate-context-profiles.js`.
Fourth review follow-up. Three open review findings still let a carrier with a missing dependency pass the gate, each reproduced against the real registry first. Treating every `}` as a statement boundary, the third follow-up's fix, also caught object literals, so in `const ratio = {} / 2; require('../x')` the division was read as a regex that swallowed the `require`; a member named `of` (`a.of / 2`) did the same through the keyword list. The member-call guard treated the last dot of a spread as member access, so `[...require('../x')]` was skipped. And a literal first argument counted only when the call closed right after it, so `require('../x', null)` was demoted to a dynamic warning, which never fails. An eighth RED added 14 cases, 11 failing; the three controls (regex after a function body, a class body and a nested block) already passed. GREEN records whether each `{` opens a block or an object literal from the token before it, and only a block's `}` lets a regex follow. It matches keywords only when no member dot precedes them, ignores the dot of a spread, and resolves a call's first argument up to its first top-level comma, so `import('./data.json', { with: { type: 'json' } })` is an exact-path import, not a dynamic warning. As a side effect, `import(path.join(__dirname, …))`, which used to be skipped silently, now warns as dynamic, because `import()` takes a URL, not a path. Rescanning all 802 repository JavaScript files against the previous head changed no result. The real registry is unchanged: 35/35 resolved, zero failures or warnings. 71 closure cases pass.
Scope. The gate checks static, relative module specifiers in planned JavaScript files, and nothing else. It does not see `require.resolve`, an aliased or `createRequire`-made `require`, files a script spawns or reads at run time, or `.sh` and `.py` scripts. It does not model `.node` addons or a directory's `package.json` `main`. Each of those can still ship a carrier that fails at load time.
October 10 maintainer reconciliation: the refreshed source head `f7bc750326a40bca31fa4ff4ef7337c7b480de4c` rebases the four-file gate onto October 9 main and contributes fourteen further regression cases. Its refreshed source ancestry and those cases are retained. The maintained TypeScript parser supersedes the further partial-lexer repair, while preserving the existing parser, malformed-source, native module.require and ESM URL fixtures. The historical lexer verification above remains dated author evidence.
@@ -230,6 +230,21 @@ const SCANNER_CASES = [
['a template import() is still a dependency', 'import(`./templated`);', ['./templated']],
['member calls named require or import',
"r.require('./label'); obj?.import('./x'); obj . require('./y'); obj.\nimport('./z'); require('./real');", ['./real']],
['division after an object literal', "const ratio = {} / 2; require('../after-object');", ['../after-object']],
['division after a nested object literal', "const v = { a: { b: 1 } } / 2, q = \"/\"; require('./after-nested-object');",
['./after-nested-object']],
['division after an object literal inside an interpolation',
"const s = `${ {} / 2 }`, q = \"/\"; require('./after-interpolated-object');", ['./after-interpolated-object']],
['regex after a function body', "function f() {} /\"/.test(s); require('./after-function');", ['./after-function']],
['regex after a class body', "class A {} /\"/.test(s); require('./after-class-body');", ['./after-class-body']],
['regex after a nested block', "if (a) { if (b) {} } /\"/.test(s); require('./after-nested-block');",
['./after-nested-block']],
['division after a member named of', "const n = a.of / 2, q = \"/\"; require('./after-member-of');",
['./after-member-of']],
['spread of a require', "const all = [...require('../spread-array')]; f(... require('./spread-arg'));",
['../spread-array', './spread-arg']],
['a literal require with extra arguments', "require('../extra-arg', null); require(path.join(__dirname, 'j.js'), 0);",
['../extra-arg', './j.js']],
];
test('scanner: computed arguments nested more than one level deep still warn', () => {
@@ -310,3 +325,31 @@ for (const specifier of ['./lib%2fhelper.js', './lib%5chelper.js', './invalid%zz
assert.ok(result.failures.every(failure => failure.reason.startsWith('invalid-url-')));
}));
}
test('scanner: an import() with an options argument is an exact-path import, not a dynamic warning', () => {
const { specifiers, imports, dynamic } = validator().extractReferences(
"import('./data.json', { with: { type: 'json' } }); require('./plain', null);");
assert.deepEqual(specifiers.sort(), ['./data.json', './plain']);
assert.deepEqual(imports, ['./data.json']);
assert.deepEqual(dynamic, []);
});
test('scanner: a computed first argument still warns when more arguments follow', () => {
assert.deepEqual(validator().extractReferences("require(name, null); require('./a' + b, c);").dynamic.sort(),
["'./a' + b", 'name']);
});
// Each of these shapes once passed the gate with its dependency missing.
for (const [label, line] of [
['division after an object literal', "const ratio = {} / 2; require('../../shared/helper');"],
['a spread require', "module.exports = [...require('../../shared/helper')];"],
['a require with an extra argument', "module.exports = require('../../shared/helper', null);"],
]) {
test(`an unplanned require hidden by ${label} still fails`, () => withFixture(root => {
plantSkillScript(root, `'use strict';\n\n${line}\n`);
const result = validator().validate(root);
assert.equal(result.status, 'failure');
assert.ok(specifiersOf(result.failures).includes('../../shared/helper'), JSON.stringify(result.failures));
assert.ok(result.failures.every(failure => failure.reason === 'unplanned-target'));
}));
}