Compare commits
1 Commits
18ad93be45
...
c8d758f4eb
| Author | SHA1 | Date | |
|---|---|---|---|
| c8d758f4eb |
23
TODO.md
23
TODO.md
@@ -67,16 +67,23 @@ but the review is broader than any of them.
|
|||||||
the build when esbuild's own metafile reports `src/shared/state.js` as an
|
the build when esbuild's own metafile reports `src/shared/state.js` as an
|
||||||
input of either background bundle — the resolution the shipped bundle was
|
input of either background bundle — the resolution the shipped bundle was
|
||||||
actually built from, so no specifier syntax and no resolution rule can slip
|
actually built from, so no specifier syntax and no resolution rule can slip
|
||||||
past it, and `make build` runs in CI. The assertion itself is pinned by
|
past it, and `make build` runs in CI. A bundled entry point under
|
||||||
`tests/buildForbiddenInputs.test.js`, including both ways its table can rot: a
|
`src/background/` with no line in the table fails the build too, so a second
|
||||||
key no bundled entry point matched, and a forbidden module this build bundled
|
worker entry point is protected by default rather than only if whoever adds it
|
||||||
nowhere. Its bound is that it is keyed by path, so a COPY of the singleton at
|
knows the table exists. The assertion itself is pinned by
|
||||||
another path is outside it; that is recorded where the table lives
|
`tests/buildForbiddenInputs.test.js`, including every way its table can rot: a
|
||||||
|
key no bundled entry point matched, a forbidden module this build bundled
|
||||||
|
nowhere, and an entry that lists no modules (which would otherwise empty the
|
||||||
|
lint rule's forbidden set as well, and is refused at require time). Its bound
|
||||||
|
is that it is keyed by path, so a COPY of the singleton at another path is
|
||||||
|
outside it — loud for three of the five defects and silent for the other two;
|
||||||
|
the bounds are recorded in full where the table lives
|
||||||
(`script/lib/forbiddenBundleInputs.js`). An ESLint rule that walks the require
|
(`script/lib/forbiddenBundleInputs.js`). An ESLint rule that walks the require
|
||||||
graph textually gives the same answer in the editor, before a full bundle; it
|
graph textually gives the same answer in the editor, before a full bundle; it
|
||||||
reads the same table, and it is fast feedback rather than the guarantee, with
|
reads the same table, and it is fast feedback rather than the guarantee. The
|
||||||
the shapes it is known to catch — and the two it is known to miss — pinned by
|
shapes it catches are pinned by `tests/backgroundStateLintRule.test.js`, and
|
||||||
`tests/backgroundStateLintRule.test.js`. Reading an unloaded singleton now
|
so are the two it misses — a computed specifier and a symlink — as asserted
|
||||||
|
non-reports, which the build fails on. Reading an unloaded singleton now
|
||||||
throws `StateNotLoadedError` instead of serving defaults. The
|
throws `StateNotLoadedError` instead of serving defaults. The
|
||||||
`chrome.storage.local` stubs in eight test files aliased instead of
|
`chrome.storage.local` stubs in eight test files aliased instead of
|
||||||
structured-cloning, which could let an assertion pass on a build that never
|
structured-cloning, which could let an assertion pass on a build that never
|
||||||
|
|||||||
46
build.js
46
build.js
@@ -4,7 +4,11 @@ const crypto = require("crypto");
|
|||||||
const { execSync } = require("child_process");
|
const { execSync } = require("child_process");
|
||||||
const esbuild = require("esbuild");
|
const esbuild = require("esbuild");
|
||||||
const { resolveVersion } = require("./script/lib/version");
|
const { resolveVersion } = require("./script/lib/version");
|
||||||
const { FORBIDDEN_INPUTS } = require("./script/lib/forbiddenBundleInputs");
|
const {
|
||||||
|
BACKGROUND_ENTRY_PREFIX,
|
||||||
|
FORBIDDEN_INPUTS,
|
||||||
|
assertTableWellFormed,
|
||||||
|
} = require("./script/lib/forbiddenBundleInputs");
|
||||||
|
|
||||||
const DIST = path.join(__dirname, "dist");
|
const DIST = path.join(__dirname, "dist");
|
||||||
const DIST_CHROME = path.join(DIST, "chrome");
|
const DIST_CHROME = path.join(DIST, "chrome");
|
||||||
@@ -24,7 +28,9 @@ const AUDITED_MODULE = "src/shared/constants.js";
|
|||||||
// This is the authoritative check, and it is here rather than in the linter
|
// This is the authoritative check, and it is here rather than in the linter
|
||||||
// because it consults the resolution esbuild actually performed. Any specifier
|
// because it consults the resolution esbuild actually performed. Any specifier
|
||||||
// syntax, any hop, any resolution rule that puts the module in the bundle fails
|
// syntax, any hop, any resolution rule that puts the module in the bundle fails
|
||||||
// the build, whether or not a text matcher would have recognized it.
|
// the build, whether or not a text matcher would have recognized it. A
|
||||||
|
// background entry point the table does not name fails as well, so a second
|
||||||
|
// worker is protected by default rather than by someone remembering this file.
|
||||||
// Dockerfile:42 runs `make build`, so it is enforced in CI.
|
// Dockerfile:42 runs `make build`, so it is enforced in CI.
|
||||||
|
|
||||||
// The build receipt: every file this build emits, with its sha256 and whether
|
// The build receipt: every file this build emits, with its sha256 and whether
|
||||||
@@ -126,6 +132,12 @@ function recordBundledInputs(metafile, record) {
|
|||||||
// Fail the build when an entry point's bundle contains a module it is
|
// Fail the build when an entry point's bundle contains a module it is
|
||||||
// prohibited from reaching. The inputs come from esbuild's metafile, so this is
|
// prohibited from reaching. The inputs come from esbuild's metafile, so this is
|
||||||
// the resolution the shipped bundle was built from and not a guess at it.
|
// the resolution the shipped bundle was built from and not a guess at it.
|
||||||
|
//
|
||||||
|
// A background entry point with no line in the table fails here too. The five
|
||||||
|
// defects this exists to prevent were accidents, and so is adding a second
|
||||||
|
// worker entry point without knowing that a table somewhere needs a line: the
|
||||||
|
// protection has to be the default for that directory rather than something
|
||||||
|
// the next author must opt into.
|
||||||
function assertNoForbiddenInputs(
|
function assertNoForbiddenInputs(
|
||||||
entryPoint,
|
entryPoint,
|
||||||
outfile,
|
outfile,
|
||||||
@@ -135,8 +147,18 @@ function assertNoForbiddenInputs(
|
|||||||
) {
|
) {
|
||||||
const entry = repoRelative(entryPoint);
|
const entry = repoRelative(entryPoint);
|
||||||
const forbidden = table[entry];
|
const forbidden = table[entry];
|
||||||
if (!forbidden) return;
|
if (!forbidden) {
|
||||||
record.entriesChecked.add(entry);
|
if (!entry.startsWith(BACKGROUND_ENTRY_PREFIX)) return;
|
||||||
|
throw new Error(
|
||||||
|
`${entry} is a background entry point with no line in ` +
|
||||||
|
`FORBIDDEN_INPUTS, so nothing stops its bundle from ` +
|
||||||
|
`containing the shared state singleton. Add it to ` +
|
||||||
|
`script/lib/forbiddenBundleInputs.js. The MV3 worker never ` +
|
||||||
|
`populates that singleton, so reading it serves ` +
|
||||||
|
`DEFAULT_STATE; use getState()/updateState() from ` +
|
||||||
|
`src/background/state.js instead.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
const out = repoRelative(outfile);
|
const out = repoRelative(outfile);
|
||||||
const entryOutput = Object.entries(metafile.outputs).find(
|
const entryOutput = Object.entries(metafile.outputs).find(
|
||||||
@@ -149,6 +171,15 @@ function assertNoForbiddenInputs(
|
|||||||
Object.keys(entryOutput[1].inputs).map(repoRelative),
|
Object.keys(entryOutput[1].inputs).map(repoRelative),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Recorded only once the bundle's inputs are actually in hand. Marking the
|
||||||
|
// entry checked any earlier — as this did — means an early return above
|
||||||
|
// satisfies assertForbiddenTableCovered() with a bundle nobody examined,
|
||||||
|
// and the coverage half cannot tell that from a real check. The lookup
|
||||||
|
// above is the fragile step: repoRelative() resolves against process.cwd()
|
||||||
|
// while esbuild's output keys are cwd-relative, so a change to where the
|
||||||
|
// build runs from could miss.
|
||||||
|
record.entriesChecked.add(entry);
|
||||||
|
|
||||||
for (const module of forbidden) {
|
for (const module of forbidden) {
|
||||||
if (!inputs.has(module)) continue;
|
if (!inputs.has(module)) continue;
|
||||||
const chain = importChain(metafile, entryPoint, module);
|
const chain = importChain(metafile, entryPoint, module);
|
||||||
@@ -180,7 +211,14 @@ function assertNoForbiddenInputs(
|
|||||||
// rewrites this persistence layer, and a rename that quietly disarmed the
|
// rewrites this persistence layer, and a rename that quietly disarmed the
|
||||||
// guarantee would put the singleton back within reach of the worker with every
|
// guarantee would put the singleton back within reach of the worker with every
|
||||||
// check in the repo still green.
|
// check in the repo still green.
|
||||||
|
//
|
||||||
|
// The third way — an entry that lists no modules at all — is refused where the
|
||||||
|
// table is defined, at require time, because that one also empties the ESLint
|
||||||
|
// rule's forbidden set and so has to fail before either layer runs. It is
|
||||||
|
// re-checked here so the build's own half does not depend on the table having
|
||||||
|
// been loaded from that file.
|
||||||
function assertForbiddenTableCovered(record, table = FORBIDDEN_INPUTS) {
|
function assertForbiddenTableCovered(record, table = FORBIDDEN_INPUTS) {
|
||||||
|
assertTableWellFormed(table);
|
||||||
for (const [entry, modules] of Object.entries(table)) {
|
for (const [entry, modules] of Object.entries(table)) {
|
||||||
if (!record.entriesChecked.has(entry)) {
|
if (!record.entriesChecked.has(entry)) {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
|
|||||||
@@ -9,6 +9,9 @@
|
|||||||
const js = require("@eslint/js");
|
const js = require("@eslint/js");
|
||||||
const globals = require("globals");
|
const globals = require("globals");
|
||||||
const backgroundState = require("./script/lib/eslint/noStateSingletonInBackground");
|
const backgroundState = require("./script/lib/eslint/noStateSingletonInBackground");
|
||||||
|
const {
|
||||||
|
BACKGROUND_ENTRY_PREFIX,
|
||||||
|
} = require("./script/lib/forbiddenBundleInputs");
|
||||||
|
|
||||||
// The extension APIs. MV3 Chrome exposes `chrome`; Firefox exposes both, and
|
// The extension APIs. MV3 Chrome exposes `chrome`; Firefox exposes both, and
|
||||||
// the code feature-detects between them.
|
// the code feature-detects between them.
|
||||||
@@ -95,8 +98,12 @@ module.exports = [
|
|||||||
// It is not the guarantee: build.js asserts the same prohibition against
|
// It is not the guarantee: build.js asserts the same prohibition against
|
||||||
// esbuild's own metafile, from the shared table in
|
// esbuild's own metafile, from the shared table in
|
||||||
// script/lib/forbiddenBundleInputs.js. This is the early report.
|
// script/lib/forbiddenBundleInputs.js. This is the early report.
|
||||||
|
//
|
||||||
|
// The glob comes from that same file, because build.js uses the prefix to
|
||||||
|
// decide which entry points must be listed in the table at all: the two
|
||||||
|
// layers must not disagree about which files are "the background".
|
||||||
{
|
{
|
||||||
files: ["src/background/**/*.js"],
|
files: [`${BACKGROUND_ENTRY_PREFIX}**/*.js`],
|
||||||
plugins: { background: backgroundState },
|
plugins: { background: backgroundState },
|
||||||
languageOptions: {
|
languageOptions: {
|
||||||
...commonjs,
|
...commonjs,
|
||||||
|
|||||||
@@ -46,7 +46,9 @@
|
|||||||
// - a symlink to the module — esbuild reports the real path and fails the
|
// - a symlink to the module — esbuild reports the real path and fails the
|
||||||
// build; this rule resolves the link's own path and sees a different file.
|
// build; this rule resolves the link's own path and sees a different file.
|
||||||
//
|
//
|
||||||
// They are listed as known divergences, not as things that cannot happen. A
|
// Both are pinned as non-reports in tests/backgroundStateLintRule.test.js, so
|
||||||
|
// this list is a measured description of the rule rather than a claim about
|
||||||
|
// it. They are known divergences, not things that cannot happen. A
|
||||||
// matcher will keep diverging from a bundler; that is why the guarantee is the
|
// matcher will keep diverging from a bundler; that is why the guarantee is the
|
||||||
// build's and this rule is not widened again to chase them.
|
// build's and this rule is not widened again to chase them.
|
||||||
|
|
||||||
|
|||||||
@@ -29,26 +29,95 @@
|
|||||||
// (`import("../shared/" + variable)`), and on a symlink to the module
|
// (`import("../shared/" + variable)`), and on a symlink to the module
|
||||||
// (esbuild reports the real path): each is `make build` exit 2.
|
// (esbuild reports the real path): each is `make build` exit 2.
|
||||||
//
|
//
|
||||||
|
// - Every background entry point, whether or not anyone remembered to list
|
||||||
|
// it. A bundled entry point under BACKGROUND_ENTRY_PREFIX with no line in
|
||||||
|
// this table fails the build (assertNoForbiddenInputs()), so adding a
|
||||||
|
// second worker entry point is protected by default rather than protected
|
||||||
|
// only if the person adding it knew about this file. Measured: bundling
|
||||||
|
// src/background/worker2.js with no line here is `make build` exit 2.
|
||||||
|
//
|
||||||
// - NOT covered: a COPY of a listed module at another path. The table is
|
// - NOT covered: a COPY of a listed module at another path. The table is
|
||||||
// keyed by path, so `cp src/shared/state.js src/shared/stateCopy.js` plus
|
// keyed by path, so `cp src/shared/state.js src/shared/stateCopy.js` plus
|
||||||
// a background require of the copy is `make build` exit 0 and `make lint`
|
// a background require of the copy is `make build` exit 0 and `make lint`
|
||||||
// exit 0 (measured). That is deliberate rather than an oversight: a copy
|
// exit 0 (measured). The copy carries the singleton's own guard, so
|
||||||
// carries the singleton's own guard, so a background read of an unloaded
|
// defects 1-3 of https://git.eeqj.de/sneak/AutistMask/issues/324 — a read
|
||||||
// field throws StateNotLoadedError instead of being served DEFAULT_STATE
|
// of a field nothing loaded — become a loud StateNotLoadedError instead of
|
||||||
// — loud, which is the opposite of the failure this table exists to
|
// a silent DEFAULT_STATE. Defects 4 and 5 do NOT: a copy also carries
|
||||||
// prevent. A newly WRITTEN singleton would carry no such backstop, and
|
// loadState(), and a stale read several awaits after a load, or a load
|
||||||
// nothing mechanical catches that one.
|
// detaching the objects an in-flight handler is mutating, are silent over
|
||||||
|
// a LOADED singleton whether it is the original or a copy. So the residual
|
||||||
|
// is wider than "it fails loudly". A newly WRITTEN singleton has no
|
||||||
|
// backstop at all.
|
||||||
//
|
//
|
||||||
// - The table protects the entry points it names. A second background-side
|
// - NOT covered: a background-behaving entry point outside
|
||||||
// entry point added later needs its own line here; the ESLint rule's
|
// BACKGROUND_ENTRY_PREFIX. The default protection above is keyed on that
|
||||||
// src/background/** glob would cover it, the build assertion would not.
|
// directory, which is also what eslint.config.js scopes the rule to, so a
|
||||||
|
// worker entry point placed somewhere else is covered by neither layer and
|
||||||
|
// needs its own line here.
|
||||||
//
|
//
|
||||||
// Both halves are checked for rot at the end of a build: a key no bundled
|
// The ESLint rule's bounds are its own and are narrower: it matches specifiers
|
||||||
// entry point matched, and a listed module this build bundled nowhere, each
|
// textually, so a computed specifier and a symlink to a listed module are
|
||||||
// fail the build rather than passing vacuously
|
// reported by the build and not by the rule. Both are pinned as non-reports in
|
||||||
// (assertForbiddenTableCovered(), pinned by tests/buildForbiddenInputs.test.js).
|
// tests/backgroundStateLintRule.test.js and are `make build` exit 2 (measured).
|
||||||
|
// A second background entry point reached by one of those two shapes is
|
||||||
|
// therefore caught by the build and not by the rule — which is the same
|
||||||
|
// division of labour as everywhere else here, not an extra hole.
|
||||||
|
//
|
||||||
|
// Every way the table itself can rot is a failure rather than a quiet pass:
|
||||||
|
//
|
||||||
|
// - a KEY no bundled entry point matched, and a listed MODULE this build
|
||||||
|
// bundled nowhere: assertForbiddenTableCovered(), at the end of a build;
|
||||||
|
// - an entry that lists NO modules, and a table with no entries at all:
|
||||||
|
// assertTableWellFormed() below, at require time — so it fails the build
|
||||||
|
// and the lint run alike, because the rule reads the same values and an
|
||||||
|
// empty list leaves it with nothing to look for.
|
||||||
|
//
|
||||||
|
// All of it is pinned by tests/buildForbiddenInputs.test.js.
|
||||||
|
|
||||||
|
// What counts as a background entry point, and therefore must be listed above.
|
||||||
|
// The build has no other notion of one: entry points are the paths handed to
|
||||||
|
// bundle(), and this prefix is the narrowest rule that names the worker's
|
||||||
|
// directory. eslint.config.js scopes the lint rule with the same prefix, from
|
||||||
|
// this constant, so the two layers cannot disagree about what "background"
|
||||||
|
// means.
|
||||||
|
const BACKGROUND_ENTRY_PREFIX = "src/background/";
|
||||||
|
|
||||||
const FORBIDDEN_INPUTS = {
|
const FORBIDDEN_INPUTS = {
|
||||||
"src/background/index.js": ["src/shared/state.js"],
|
"src/background/index.js": ["src/shared/state.js"],
|
||||||
};
|
};
|
||||||
|
|
||||||
module.exports = { FORBIDDEN_INPUTS };
|
// Refuse a table that cannot prohibit anything. An entry whose module list is
|
||||||
|
// empty passes every check in both layers while enforcing nothing: the build
|
||||||
|
// finds no module to look for and records the entry as checked, and the rule's
|
||||||
|
// forbidden set — Object.values(...).flat() — comes back empty, so a plain
|
||||||
|
// `require("../shared/state")` in the worker is green everywhere. That is a
|
||||||
|
// one-character edit, so it fails here, where the table is defined and both
|
||||||
|
// layers must load it, rather than in either layer's own checks.
|
||||||
|
function assertTableWellFormed(table) {
|
||||||
|
const entries = Object.entries(table);
|
||||||
|
if (entries.length === 0) {
|
||||||
|
throw new Error(
|
||||||
|
"FORBIDDEN_INPUTS is empty, so nothing is prohibited anywhere. " +
|
||||||
|
"Removing the last entry disables the guarantee behind " +
|
||||||
|
"https://git.eeqj.de/sneak/AutistMask/issues/324.",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
for (const [entry, modules] of entries) {
|
||||||
|
if (!Array.isArray(modules) || modules.length === 0) {
|
||||||
|
throw new Error(
|
||||||
|
`FORBIDDEN_INPUTS["${entry}"] lists no modules, so it ` +
|
||||||
|
`prohibits nothing while still looking enforced. Give it ` +
|
||||||
|
`the modules that entry point may not reach, or remove ` +
|
||||||
|
`the entry.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
assertTableWellFormed(FORBIDDEN_INPUTS);
|
||||||
|
|
||||||
|
module.exports = {
|
||||||
|
BACKGROUND_ENTRY_PREFIX,
|
||||||
|
FORBIDDEN_INPUTS,
|
||||||
|
assertTableWellFormed,
|
||||||
|
};
|
||||||
|
|||||||
@@ -57,7 +57,11 @@ async function updateStateOnce(mutate) {
|
|||||||
// place; it may be async, but it must not do anything slow — the window
|
// place; it may be async, but it must not do anything slow — the window
|
||||||
// between the read and the write is the window in which another context's
|
// between the read and the write is the window in which another context's
|
||||||
// write is lost, and keeping it to one storage round trip is what makes a
|
// write is lost, and keeping it to one storage round trip is what makes a
|
||||||
// whole-record write safe here.
|
// whole-record write safe here. Concretely: the write is the WHOLE record, so
|
||||||
|
// a popup write that lands inside that window is reverted, in every field, by
|
||||||
|
// the record this turn read before it. That is accepted because the window is
|
||||||
|
// one round trip long and the popup is not writing while the worker is;
|
||||||
|
// widening it is what would make it a real hazard.
|
||||||
//
|
//
|
||||||
// `mutate` must also not call updateState() itself, directly or through
|
// `mutate` must also not call updateState() itself, directly or through
|
||||||
// anything it awaits: the queue is strictly serial, so the inner turn waits on
|
// anything it awaits: the queue is strictly serial, so the inner turn waits on
|
||||||
|
|||||||
@@ -19,12 +19,16 @@
|
|||||||
// backtick, the dynamic import and the `from` clause; its successor missed a
|
// backtick, the dynamic import and the `from` clause; its successor missed a
|
||||||
// comment inside the call and a directory resolved through package.json `main`.
|
// comment inside the call and a directory resolved through package.json `main`.
|
||||||
//
|
//
|
||||||
// NOT covered, and pinned nowhere below because the rule genuinely does not
|
// Two shapes the rule does NOT report are pinned below as non-reports, in
|
||||||
// report it: a computed specifier such as `require("../shared/" + "state")`.
|
// "the divergences from the build's answer": a computed specifier such as
|
||||||
// esbuild constant-folds that and puts state.js in the bundle — measured,
|
// `require("../shared/" + "state")`, which esbuild constant-folds, and a
|
||||||
// `make lint` exit 0 and `make build` exit 2 — so it is a known divergence
|
// symlink to the module, whose real path esbuild reports. Both put state.js in
|
||||||
// that the build catches, not a shape that cannot occur. Same for a symlink to
|
// the shipped background bundle and both are `make build` exit 2 with
|
||||||
// the module.
|
// `make lint` exit 0 (measured). Pinning them as non-reports is what makes the
|
||||||
|
// rule's stated bounds a measured description rather than a claim: if either
|
||||||
|
// starts being reported, or the matcher is widened until one is, a test says
|
||||||
|
// so. Their catch is the build's, and is pinned in
|
||||||
|
// tests/buildForbiddenInputs.test.js against the metafile that catches it.
|
||||||
|
|
||||||
const fs = require("fs");
|
const fs = require("fs");
|
||||||
const os = require("os");
|
const os = require("os");
|
||||||
@@ -216,6 +220,33 @@ describe("reachability, not just the direct specifier", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// These two are holes in the rule, and they are pinned as holes on purpose:
|
||||||
|
// the build catches both, the rule is fast feedback in front of it, and a
|
||||||
|
// written-down bound that nothing measures is how the previous three rounds of
|
||||||
|
// this change ended up with claims that were false.
|
||||||
|
describe("the divergences from the build's answer", () => {
|
||||||
|
test("a computed specifier is not reported (esbuild folds it; the build fails)", () => {
|
||||||
|
const root = fixture({
|
||||||
|
"src/background/index.js":
|
||||||
|
'globalThis.__probe = require("../shared/" + "state").state;\n',
|
||||||
|
});
|
||||||
|
expect(lintBackground(root)).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a symlink to the module is not reported (esbuild reports the real path)", () => {
|
||||||
|
const root = fixture({
|
||||||
|
"src/background/index.js":
|
||||||
|
'const { state } = require("../shared/stateLink");\n' +
|
||||||
|
"module.exports = { state };\n",
|
||||||
|
});
|
||||||
|
fs.symlinkSync(
|
||||||
|
path.join(root, "src/shared/state.js"),
|
||||||
|
path.join(root, "src/shared/stateLink.js"),
|
||||||
|
);
|
||||||
|
expect(lintBackground(root)).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("what the rule must not report", () => {
|
describe("what the rule must not report", () => {
|
||||||
test("a background file that reaches only its own state layer", () => {
|
test("a background file that reaches only its own state layer", () => {
|
||||||
const root = fixture({
|
const root = fixture({
|
||||||
|
|||||||
@@ -9,11 +9,19 @@
|
|||||||
// the worker. Five defects, one destroyed wallet, and the whole argument for
|
// the worker. Five defects, one destroyed wallet, and the whole argument for
|
||||||
// the scoped loud-read guard rest on this assertion, so it is pinned here.
|
// the scoped loud-read guard rest on this assertion, so it is pinned here.
|
||||||
//
|
//
|
||||||
// The subject is build.js's exported helpers, driven against SYNTHETIC
|
// The subject is build.js's exported helpers plus the table's own
|
||||||
// metafiles in esbuild's shape. Nothing here shells out to a build or writes
|
// well-formedness check, driven against SYNTHETIC metafiles in esbuild's
|
||||||
// dist/: the assertion's job is to read a metafile correctly, and a metafile is
|
// shape. Nothing here shells out to a build or writes dist/: the assertion's
|
||||||
// data. That the real shapes reach it is the build's own business and is
|
// job is to read a metafile correctly, and a metafile is data. That the real
|
||||||
// measured in the PR that introduced it.
|
// shapes reach it is the build's own business and is measured in the PR that
|
||||||
|
// introduced it.
|
||||||
|
//
|
||||||
|
// Every vacuous pass this guarantee has been found to have is pinned below,
|
||||||
|
// because each one was a way for `make check`, `make lint` and `make build` to
|
||||||
|
// be green over a background bundle containing the singleton: a stale key, a
|
||||||
|
// stale module, an entry that lists no modules, an entry recorded as checked
|
||||||
|
// before its bundle was in hand, and a background entry point nobody added to
|
||||||
|
// the table.
|
||||||
//
|
//
|
||||||
// Paths are absolute on the way in, because the helpers normalize whatever
|
// Paths are absolute on the way in, because the helpers normalize whatever
|
||||||
// esbuild gave them to repo-relative and this file should not depend on the
|
// esbuild gave them to repo-relative and this file should not depend on the
|
||||||
@@ -28,6 +36,11 @@ const {
|
|||||||
assertNoForbiddenInputs,
|
assertNoForbiddenInputs,
|
||||||
assertForbiddenTableCovered,
|
assertForbiddenTableCovered,
|
||||||
} = require("../build");
|
} = require("../build");
|
||||||
|
const {
|
||||||
|
BACKGROUND_ENTRY_PREFIX,
|
||||||
|
FORBIDDEN_INPUTS,
|
||||||
|
assertTableWellFormed,
|
||||||
|
} = require("../script/lib/forbiddenBundleInputs");
|
||||||
|
|
||||||
const ROOT = path.resolve(__dirname, "..");
|
const ROOT = path.resolve(__dirname, "..");
|
||||||
const abs = (p) => path.join(ROOT, p);
|
const abs = (p) => path.join(ROOT, p);
|
||||||
@@ -36,6 +49,10 @@ const ENTRY = "src/background/index.js";
|
|||||||
const OUT = "dist/chrome/src/background/index.js";
|
const OUT = "dist/chrome/src/background/index.js";
|
||||||
const STATE = "src/shared/state.js";
|
const STATE = "src/shared/state.js";
|
||||||
const HOP = "src/shared/chainSwitchFields.js";
|
const HOP = "src/shared/chainSwitchFields.js";
|
||||||
|
const SECOND = "src/background/worker2.js";
|
||||||
|
const SECOND_OUT = "dist/chrome/src/background/worker2.js";
|
||||||
|
const POPUP = "src/popup/index.js";
|
||||||
|
const POPUP_OUT = "dist/chrome/src/popup/index.js";
|
||||||
|
|
||||||
// The real table's shape: entry point -> modules its bundle may not contain.
|
// The real table's shape: entry point -> modules its bundle may not contain.
|
||||||
const TABLE = { [ENTRY]: [STATE] };
|
const TABLE = { [ENTRY]: [STATE] };
|
||||||
@@ -113,6 +130,114 @@ describe("assertNoForbiddenInputs()", () => {
|
|||||||
`${OUT} bundles ${STATE}, which ${ENTRY} must not reach.`,
|
`${OUT} bundles ${STATE}, which ${ENTRY} must not reach.`,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The lookup this covers is the fragile step in the whole assertion:
|
||||||
|
// repoRelative() resolves against process.cwd() and esbuild's output keys
|
||||||
|
// are cwd-relative, so a change to where the build runs from, or to
|
||||||
|
// outfile vs outdir, makes it miss. It must be loud, and the entry must
|
||||||
|
// NOT already be marked checked when it does — otherwise a later edit
|
||||||
|
// turning this throw into an early return leaves both halves of the
|
||||||
|
// guarantee satisfied by a bundle nothing looked at.
|
||||||
|
test("an output esbuild did not report fails, and records nothing as checked", () => {
|
||||||
|
const mf = metafile({
|
||||||
|
outputs: { "dist/chrome/src/background/renamed.js": [ENTRY] },
|
||||||
|
imports: { [ENTRY]: [] },
|
||||||
|
});
|
||||||
|
const record = newForbiddenRecord();
|
||||||
|
recordBundledInputs(mf, record);
|
||||||
|
|
||||||
|
expect(() =>
|
||||||
|
assertNoForbiddenInputs(abs(ENTRY), abs(OUT), mf, record, TABLE),
|
||||||
|
).toThrow(`esbuild reported no metafile output for ${OUT}`);
|
||||||
|
expect([...record.entriesChecked]).toEqual([]);
|
||||||
|
|
||||||
|
// So even if that throw became `return`, the coverage half catches it.
|
||||||
|
record.bundledInputs.add(STATE);
|
||||||
|
expect(() => assertForbiddenTableCovered(record, TABLE)).toThrow(
|
||||||
|
`${ENTRY} is listed in FORBIDDEN_INPUTS but was not bundled`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// Finding from the fifth review of this change: adding a second worker entry
|
||||||
|
// point is exactly the accident this guarantee exists for, and the person
|
||||||
|
// adding one has no reason to know a table elsewhere needs a line. So the
|
||||||
|
// default for the background directory is protected, not unprotected.
|
||||||
|
describe("background entry points are protected by default", () => {
|
||||||
|
test("a bundled background entry point with no line in the table fails", () => {
|
||||||
|
const mf = metafile({
|
||||||
|
outputs: { [SECOND_OUT]: [SECOND, STATE] },
|
||||||
|
imports: { [SECOND]: [STATE] },
|
||||||
|
});
|
||||||
|
const record = newForbiddenRecord();
|
||||||
|
recordBundledInputs(mf, record);
|
||||||
|
|
||||||
|
expect(() =>
|
||||||
|
assertNoForbiddenInputs(
|
||||||
|
abs(SECOND),
|
||||||
|
abs(SECOND_OUT),
|
||||||
|
mf,
|
||||||
|
record,
|
||||||
|
TABLE,
|
||||||
|
),
|
||||||
|
).toThrow(
|
||||||
|
`${SECOND} is a background entry point with no line in ` +
|
||||||
|
`FORBIDDEN_INPUTS`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("an entry point outside the background directory needs no line", () => {
|
||||||
|
// The popup legitimately bundles the singleton; that is its model.
|
||||||
|
const mf = metafile({
|
||||||
|
outputs: { [POPUP_OUT]: [POPUP, STATE] },
|
||||||
|
imports: { [POPUP]: [STATE] },
|
||||||
|
});
|
||||||
|
const record = newForbiddenRecord();
|
||||||
|
recordBundledInputs(mf, record);
|
||||||
|
|
||||||
|
expect(() =>
|
||||||
|
assertNoForbiddenInputs(
|
||||||
|
abs(POPUP),
|
||||||
|
abs(POPUP_OUT),
|
||||||
|
mf,
|
||||||
|
record,
|
||||||
|
TABLE,
|
||||||
|
),
|
||||||
|
).not.toThrow();
|
||||||
|
expect([...record.entriesChecked]).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("the prefix is the one the lint rule is scoped to", () => {
|
||||||
|
expect(BACKGROUND_ENTRY_PREFIX).toBe("src/background/");
|
||||||
|
expect(SECOND.startsWith(BACKGROUND_ENTRY_PREFIX)).toBe(true);
|
||||||
|
expect(POPUP.startsWith(BACKGROUND_ENTRY_PREFIX)).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("recordBundledInputs()", () => {
|
||||||
|
// The sole data source for the module half of the anti-rot check, and
|
||||||
|
// every other case here hand-seeds what it produces. Driven for real,
|
||||||
|
// across two outputs, and then handed straight to the check that reads it.
|
||||||
|
test("records every input of every output, satisfying the module half", () => {
|
||||||
|
const mf = metafile({
|
||||||
|
outputs: {
|
||||||
|
[OUT]: [ENTRY, HOP],
|
||||||
|
[POPUP_OUT]: [POPUP, STATE],
|
||||||
|
},
|
||||||
|
imports: { [ENTRY]: [HOP], [POPUP]: [STATE] },
|
||||||
|
});
|
||||||
|
const record = newForbiddenRecord();
|
||||||
|
recordBundledInputs(mf, record);
|
||||||
|
|
||||||
|
expect([...record.bundledInputs].sort()).toEqual(
|
||||||
|
[ENTRY, HOP, POPUP, STATE].sort(),
|
||||||
|
);
|
||||||
|
|
||||||
|
// Nothing hand-seeded: the covered check passes on what the recorder
|
||||||
|
// actually collected, so a recorder that collects nothing fails here.
|
||||||
|
assertNoForbiddenInputs(abs(ENTRY), abs(OUT), mf, record, TABLE);
|
||||||
|
expect(() => assertForbiddenTableCovered(record, TABLE)).not.toThrow();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("importChain()", () => {
|
describe("importChain()", () => {
|
||||||
@@ -157,13 +282,20 @@ describe("assertForbiddenTableCovered()", () => {
|
|||||||
expect(() => assertForbiddenTableCovered(record, TABLE)).not.toThrow();
|
expect(() => assertForbiddenTableCovered(record, TABLE)).not.toThrow();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The build this drives bundles the popup and no background entry point,
|
||||||
|
// because a stale key AND a bundled background entry point is now the
|
||||||
|
// stronger failure above — the entry point that is there but unlisted is
|
||||||
|
// reported by name, before the end of the build. This case is the rot that
|
||||||
|
// is left after that one: a key naming something this build never bundled.
|
||||||
test("a key no bundled entry point matched fails", () => {
|
test("a key no bundled entry point matched fails", () => {
|
||||||
const mf = metafile({
|
const mf = metafile({
|
||||||
outputs: { [OUT]: [ENTRY] },
|
outputs: { [POPUP_OUT]: [POPUP] },
|
||||||
imports: { [ENTRY]: [] },
|
imports: { [POPUP]: [] },
|
||||||
});
|
});
|
||||||
const stale = { "src/background/renamed.js": [STATE] };
|
const stale = { "src/background/renamed.js": [STATE] };
|
||||||
const record = check(mf, stale);
|
const record = newForbiddenRecord();
|
||||||
|
recordBundledInputs(mf, record);
|
||||||
|
assertNoForbiddenInputs(abs(POPUP), abs(POPUP_OUT), mf, record, stale);
|
||||||
record.bundledInputs.add(STATE);
|
record.bundledInputs.add(STATE);
|
||||||
|
|
||||||
expect(() => assertForbiddenTableCovered(record, stale)).toThrow(
|
expect(() => assertForbiddenTableCovered(record, stale)).toThrow(
|
||||||
@@ -189,4 +321,46 @@ describe("assertForbiddenTableCovered()", () => {
|
|||||||
` ${ENTRY}, but this build bundled it nowhere`,
|
` ${ENTRY}, but this build bundled it nowhere`,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The third rot, and the worst of the three: an entry with an empty list
|
||||||
|
// is checked, is bundled, names no module that could be missing, and
|
||||||
|
// prohibits nothing — in BOTH layers at once, since the rule's forbidden
|
||||||
|
// set is Object.values(table).flat().
|
||||||
|
test("an entry that lists no modules fails", () => {
|
||||||
|
const mf = metafile({
|
||||||
|
outputs: { [OUT]: [ENTRY, STATE] },
|
||||||
|
imports: { [ENTRY]: [STATE] },
|
||||||
|
});
|
||||||
|
const empty = { [ENTRY]: [] };
|
||||||
|
const record = newForbiddenRecord();
|
||||||
|
recordBundledInputs(mf, record);
|
||||||
|
assertNoForbiddenInputs(abs(ENTRY), abs(OUT), mf, record, empty);
|
||||||
|
|
||||||
|
expect(() => assertForbiddenTableCovered(record, empty)).toThrow(
|
||||||
|
`FORBIDDEN_INPUTS["${ENTRY}"] lists no modules`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("assertTableWellFormed()", () => {
|
||||||
|
// Runs at require time on the shipped table, in script/lib/
|
||||||
|
// forbiddenBundleInputs.js, so an empty list fails the lint run as well as
|
||||||
|
// the build — the build's own re-check cannot help a layer that never
|
||||||
|
// reaches the build.
|
||||||
|
test("the shipped table is well formed", () => {
|
||||||
|
expect(() => assertTableWellFormed(FORBIDDEN_INPUTS)).not.toThrow();
|
||||||
|
expect(Object.entries(FORBIDDEN_INPUTS).length).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("an entry that lists no modules fails, naming the entry", () => {
|
||||||
|
expect(() => assertTableWellFormed({ [ENTRY]: [] })).toThrow(
|
||||||
|
`FORBIDDEN_INPUTS["${ENTRY}"] lists no modules`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a table with no entries at all fails", () => {
|
||||||
|
expect(() => assertTableWellFormed({})).toThrow(
|
||||||
|
"FORBIDDEN_INPUTS is empty",
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user