mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-16 13:15:36 +02:00
fix(carriers): close the object-literal, spread and extra-argument false negatives
- Record whether each `{` opens a block or an object literal from the
token before it; only a block's `}` lets a regex follow, so `{} / 2`
divides again while `if (ok) {} /re/` stays a regex.
- Match regex-preceding keywords only when no member dot precedes them,
so `a.of / 2` divides.
- Exempt the last dot of a spread from the member-call guard, so
`...require('./x')` is a module load.
- Resolve a call's first argument up to its first top-level comma, so
`require('./x', null)` and `import('./x.json', { with })` name `./x`.
`import(path.join(__dirname, ...))`, previously skipped silently, now
warns as dynamic.
Rescanning all 802 repository JavaScript files against the previous head
changes no result. The real registry is unchanged: 35/35 resolved, zero
failures or warnings. The design doc records the fix and states the gate's
scope: static relative specifiers in planned JavaScript only.
This commit is contained in:
montjeffrey
@@ -102,3 +102,7 @@ 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.
|
||||
|
||||
@@ -26,23 +26,29 @@ const BUILTINS = new Set(builtinModules);
|
||||
const SCRIPT_EXTENSIONS = new Set(['.js', '.cjs', '.mjs']);
|
||||
const MARK = '\u0000';
|
||||
const SLOT = `${MARK}(\\d+)${MARK}`;
|
||||
// `obj.require(`, `obj?.import(` and `obj . require(` are method calls, not module loads.
|
||||
const NOT_MEMBER = '(?<!\\.\\s*)';
|
||||
const CALL_PATTERN = new RegExp(`${NOT_MEMBER}\\b(require|import)\\s*\\(\\s*${SLOT}\\s*\\)`, 'g');
|
||||
// `obj.require(`, `obj?.import(` and `obj . require(` are method calls, not
|
||||
// module loads; the last dot of a spread (`...require(`) is not member access.
|
||||
const NOT_MEMBER = '(?<!(?<!\\.)\\.\\s*)';
|
||||
const FROM_PATTERN = new RegExp(`\\bfrom\\s*${SLOT}`, 'g');
|
||||
const SIDE_EFFECT_PATTERN = new RegExp(`\\bimport\\s*${SLOT}`, 'g');
|
||||
const DIRNAME_JOIN_PATTERN = new RegExp(
|
||||
`${NOT_MEMBER}\\brequire\\s*\\(\\s*path\\.join\\s*\\(\\s*__dirname\\s*((?:,\\s*${SLOT}\\s*)+)\\)\\s*\\)`, 'g');
|
||||
const CALL_OPEN_PATTERN = new RegExp(`${NOT_MEMBER}\\b(?:require|import)\\s*\\(`, 'g');
|
||||
const STATIC_JOIN_PATTERN = new RegExp(`^path\\.join\\s*\\(\\s*__dirname\\s*(?:,\\s*${SLOT}\\s*)+\\)$`);
|
||||
const CALL_OPEN_PATTERN = new RegExp(`${NOT_MEMBER}\\b(require|import)\\s*\\(`, 'g');
|
||||
const LITERAL_PATTERN = new RegExp(`^${SLOT}$`);
|
||||
const STATIC_JOIN_PATTERN = new RegExp(`^path\\.join\\s*\\(\\s*__dirname\\s*((?:,\\s*${SLOT}\\s*)+)\\)$`);
|
||||
const SLOT_PATTERN = new RegExp(SLOT, 'g');
|
||||
// Closes an interpolation; no keyword pattern can match across it.
|
||||
const CLOSE = `${MARK}${MARK}`;
|
||||
const ESCAPE_PATTERN = /\\(?:u\{([0-9a-fA-F]+)\}|u([0-9a-fA-F]{4})|x([0-9a-fA-F]{2})|(\r\n|[\s\S]))/g;
|
||||
const SIMPLE_ESCAPES = Object.freeze({ n: '\n', r: '\r', t: '\t', b: '\b', f: '\f', v: '\v', 0: '\0' });
|
||||
const LINE_CONTINUATIONS = new Set(['\n', '\r', '\r\n', '\u2028', '\u2029']);
|
||||
// `}` ends a block, so a slash after it starts a statement: a regex.
|
||||
const REGEX_PRECEDES = /(?:[([{},;:=!&|?+\-*/%~^<>]|\b(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$/;
|
||||
// A keyword, not a member of that name: `a.of / 2` divides.
|
||||
const KEYWORD = `${NOT_MEMBER}(?<![\\w$])`;
|
||||
// `}` is absent: whether a slash after it divides depends on the brace (see opensBlock).
|
||||
const REGEX_PRECEDES = new RegExp(
|
||||
`(?:[([{,;:=!&|?+\\-*/%~^<>]|${KEYWORD}(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$`);
|
||||
// After these a `{` opens an object literal; after any other word it opens a block.
|
||||
const OBJECT_KEYWORD = new RegExp(
|
||||
`${KEYWORD}(?:return|typeof|case|in|of|new|delete|void|instanceof|yield|await)$`);
|
||||
const CASE_LABEL = new RegExp(`${KEYWORD}(?:case\\b[^;{}]*|default\\s*):$`);
|
||||
const CONDITION_KEYWORD = /(?:^|[^\w$])(if|while|for|with)$/;
|
||||
|
||||
/** Cook a raw string or template chunk the way the JavaScript parser would. */
|
||||
@@ -73,6 +79,21 @@ function opensCondition(code) {
|
||||
return Boolean(match) && !code.slice(0, code.length - match[1].length).trimEnd().endsWith('.');
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a `{` after code opens a block (a slash after its `}` starts a
|
||||
* regex) or an object literal (a slash after its `}` divides). `blocks` holds
|
||||
* the kind of every enclosing brace, innermost last.
|
||||
*/
|
||||
function opensBlock(code, blocks) {
|
||||
if (!code || code.endsWith('=>')) return true;
|
||||
const last = code[code.length - 1];
|
||||
if (last === ')' || last === ';' || last === '}') return true;
|
||||
if (last === '{') return blocks[blocks.length - 1] !== false;
|
||||
if (last === ':') return blocks[blocks.length - 1] !== false && CASE_LABEL.test(code);
|
||||
if (/[\w$]/.test(last)) return !OBJECT_KEYWORD.test(code);
|
||||
return false;
|
||||
}
|
||||
|
||||
function readRegexLiteral(source, start) {
|
||||
let index = start + 1;
|
||||
let inClass = false;
|
||||
@@ -102,6 +123,10 @@ function scanSource(rawSource) {
|
||||
// last one closed did: `/` after `if (...)` starts a regex, not division.
|
||||
const parens = [];
|
||||
let closedCondition = false;
|
||||
// The same for braces: whether each open one began a block, and whether the
|
||||
// last one closed did: `/` after `{}` divides, after `if (x) {}` it does not.
|
||||
const blocks = [];
|
||||
let closedBlock = false;
|
||||
let code = '';
|
||||
let index = 0;
|
||||
// Slots cut from template literals; `from` and `import` need a quoted string.
|
||||
@@ -141,7 +166,9 @@ function scanSource(rawSource) {
|
||||
index = index === -1 ? source.length : index + 2;
|
||||
continue;
|
||||
}
|
||||
if (char === '/' && (REGEX_PRECEDES.test(code.trimEnd()) || (closedCondition && code.trimEnd().endsWith(')')))) {
|
||||
if (char === '/' && (REGEX_PRECEDES.test(code.trimEnd())
|
||||
|| (closedCondition && code.trimEnd().endsWith(')'))
|
||||
|| (closedBlock && code.trimEnd().endsWith('}')))) {
|
||||
index = readRegexLiteral(source, index);
|
||||
code += ' ';
|
||||
continue;
|
||||
@@ -168,6 +195,8 @@ function scanSource(rawSource) {
|
||||
}
|
||||
if (char === '(') parens.push(opensCondition(code.trimEnd()));
|
||||
else if (char === ')') closedCondition = parens.pop() === true;
|
||||
else if (char === '{') blocks.push(opensBlock(code.trimEnd(), blocks));
|
||||
else if (char === '}') closedBlock = blocks.pop() !== false;
|
||||
code += char;
|
||||
index += 1;
|
||||
}
|
||||
@@ -185,9 +214,11 @@ function collect(pattern, code, handler) {
|
||||
}
|
||||
|
||||
/**
|
||||
* The argument text of every `require(...)`/`import(...)` call, matched to
|
||||
* its closing paren at any nesting depth. Strings are already slots, so a
|
||||
* paren inside one cannot unbalance the count.
|
||||
* The first argument of every `require(...)`/`import(...)` call, matched to
|
||||
* its closing paren at any nesting depth and cut at the first top-level
|
||||
* comma: Node resolves only the first argument, so `require('./x', null)` and
|
||||
* `import('./x.json', { with })` still name `./x`. Strings are already slots,
|
||||
* so a paren or comma inside one cannot unbalance the count.
|
||||
*/
|
||||
function callArguments(code) {
|
||||
const found = [];
|
||||
@@ -195,12 +226,15 @@ function callArguments(code) {
|
||||
const start = match.index + match[0].length;
|
||||
let depth = 1;
|
||||
let cursor = start;
|
||||
let end = null;
|
||||
while (cursor < code.length && depth > 0) {
|
||||
if (code[cursor] === '(') depth += 1;
|
||||
else if (code[cursor] === ')') depth -= 1;
|
||||
const char = code[cursor];
|
||||
if (char === '(' || char === '[' || char === '{') depth += 1;
|
||||
else if (char === ')' || char === ']' || char === '}') depth -= 1;
|
||||
else if (char === ',' && depth === 1 && end === null) end = cursor;
|
||||
cursor += 1;
|
||||
}
|
||||
if (depth === 0) found.push(code.slice(start, cursor - 1).trim());
|
||||
if (depth === 0) found.push({ keyword: match[1], argument: code.slice(start, end ?? cursor - 1).trim() });
|
||||
});
|
||||
return found;
|
||||
}
|
||||
@@ -230,7 +264,6 @@ function extractReferences(rawSource) {
|
||||
packages.add(value.startsWith('@') ? value.split('/').slice(0, 2).join('/') : value.split('/')[0]);
|
||||
}
|
||||
};
|
||||
collect(CALL_PATTERN, code, match => record(literals[Number(match[2])], match[1] === 'import'));
|
||||
// Static `from` and side-effect `import` take only a quoted string, so a
|
||||
// template there (a tag named `from`, say) is not an import.
|
||||
for (const pattern of [FROM_PATTERN, SIDE_EFFECT_PATTERN]) {
|
||||
@@ -238,14 +271,19 @@ function extractReferences(rawSource) {
|
||||
if (!templates.has(Number(match[1]))) record(literals[Number(match[1])], true);
|
||||
});
|
||||
}
|
||||
collect(DIRNAME_JOIN_PATTERN, code, match => {
|
||||
const segments = [];
|
||||
collect(SLOT_PATTERN, match[1], slotMatch => segments.push(literals[Number(slotMatch[1])]));
|
||||
specifiers.add(`./${segments.join('/')}`);
|
||||
});
|
||||
for (const argument of callArguments(code)) {
|
||||
if (!argument || new RegExp(`^${SLOT}$`).test(argument) || STATIC_JOIN_PATTERN.test(argument)) continue;
|
||||
dynamic.add(renderExpression(argument, literals));
|
||||
for (const { keyword, argument } of callArguments(code)) {
|
||||
if (!argument) continue;
|
||||
const literal = LITERAL_PATTERN.exec(argument);
|
||||
const join = keyword === 'require' && STATIC_JOIN_PATTERN.exec(argument);
|
||||
if (literal) {
|
||||
record(literals[Number(literal[1])], keyword === 'import');
|
||||
} else if (join) {
|
||||
const segments = [];
|
||||
collect(SLOT_PATTERN, join[1], slotMatch => segments.push(literals[Number(slotMatch[1])]));
|
||||
specifiers.add(`./${segments.join('/')}`);
|
||||
} else {
|
||||
dynamic.add(renderExpression(argument, literals));
|
||||
}
|
||||
}
|
||||
return { specifiers: [...specifiers], imports: [...imports], packages: [...packages], dynamic: [...dynamic] };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user