fix(dialogs): wait for the confirmation before deleting or removing the key #465

Merged
maximus merged 2 commits from issue-421-confirm-dialog into main 2026-10-18 18:31:43 +00:00
Showing only changes of commit ff9656d015 - Show all commits

View file

@ -8,10 +8,15 @@
*
* It reads the non-test sources of `src/`, comments removed, as `i18n-mock-guard.test.ts` does,
* and fails on:
* 1. a call to `confirm` or `alert`, bare or through `window`, `globalThis` or `self`
* (a member call such as `deps.confirm(` is not the native one);
* 1. a native `confirm` or `alert`: called bare or through `window`, `globalThis` or `self`
* (a member call such as `deps.confirm(` is not the native one), with `?.(` too, read by
* index (`window["confirm"]`) or destructured from one of those objects;
* 2. a call to `confirmAction(` or to an injected `confirm(` (`deps.confirm(`,
* `ui.confirm(`) that is not directly preceded by `await`.
* `ui.confirm(`) that is not directly preceded by `await`;
* 3. any other mention of `confirmAction`: an alias (`const c = confirmAction`) or a renamed
* import would call it out of reach of rule 2 (#465 review). Allowed besides the awaited
* call: its declaration, a plain named import, and `confirm: confirmAction`, the value of
* an injected `confirm` whose calls rule 2 checks.
* `confirmAction` itself calls the plugin's `ask`, so `src/services/confirmService.ts` is
* scanned like any other file. Keywords rather than a parse: the samples below check that each
* rule flags what it targets and lets the right form through.
@ -23,12 +28,22 @@ import { relative, resolve, sep } from "path";
const SRC = resolve(import.meta.dirname, "..");
const NATIVE_CALL = /(?:\b(?:window|globalThis|self)\s*\.\s*|(?<![.\w$]))(?:confirm|alert)\s*\(/;
const NATIVE_CALL = /(?:\b(?:window|globalThis|self)\s*\.\s*|(?<![.\w$]))(?:confirm|alert)\s*(?:\?\.\s*)?\(/;
const NATIVE_INDEX = /\b(?:window|globalThis|self)\s*(?:\?\.\s*)?\[\s*["'`](?:confirm|alert)["'`]\s*\]/;
const NATIVE_DESTRUCTURED = /\{[^}]*\b(?:confirm|alert)\b[^}]*\}\s*=\s*(?:window|globalThis|self)\b/;
// `confirmAction(` or `<receiver>.confirm(`, the receiver captured to skip the native ones.
const CONFIRM_CALL = /\b(?:confirmAction|(\w+)\s*\.\s*confirm)\s*\(/g;
const NATIVE_RECEIVER = /^(?:window|globalThis|self)$/;
const AWAITED = /\bawait\s+$/;
const DECLARED = /\bfunction\s+$/;
// Mentions of `confirmAction` that rule 3 allows; any other one is an offence.
const CONFIRM_ACTION = /\bconfirmAction\b/g;
const ALLOWED_MENTION = [
/\bawait\s+confirmAction\s*\(/g,
/\bfunction\s+confirmAction\s*\(/g,
/^\s*import\s*\{\s*confirmAction\s*\}\s*from\s/g,
/\bconfirm\s*:\s*confirmAction\s*[,}]/g,
];
/** The source without its comments, line breaks kept so that line numbers stay right. */
function withoutComments(source: string): string {
@ -40,12 +55,16 @@ function withoutComments(source: string): string {
/** The offences of one line: a native call, or a confirmation not awaited. */
function lineOffences(line: string): string[] {
const found: string[] = [];
if (NATIVE_CALL.test(line)) found.push("native confirm/alert");
if (NATIVE_CALL.test(line) || NATIVE_INDEX.test(line) || NATIVE_DESTRUCTURED.test(line)) {
found.push("native confirm/alert");
}
for (const match of line.matchAll(CONFIRM_CALL)) {
if (match[1] !== undefined && NATIVE_RECEIVER.test(match[1])) continue;
const before = line.slice(0, match.index);
if (!AWAITED.test(before) && !DECLARED.test(before)) found.push("confirmation not awaited");
}
const allowed = ALLOWED_MENTION.reduce((n, rule) => n + [...line.matchAll(rule)].length, 0);
if ([...line.matchAll(CONFIRM_ACTION)].length > allowed) found.push("confirmAction out of reach");
return found;
}
@ -78,6 +97,11 @@ describe("native dialog guard (#421)", () => {
"globalThis.confirm(message);",
"self . alert(message);",
"alert(message);",
"if (!window[\"confirm\"](message)) return;",
"globalThis['alert'](message);",
"if (!confirm?.(message)) return;",
"const { confirm } = window;",
"const { alert: show, confirm: ask } = globalThis;",
])("flags the native call in %s", (line) => {
expect(lineOffences(line)).toContain("native confirm/alert");
});
@ -88,7 +112,17 @@ describe("native dialog guard (#421)", () => {
"if (!deps.confirm(deps.t(key))) return;",
"const ok = ui.confirm(message);",
])("flags the confirmation not awaited in %s", (line) => {
expect(lineOffences(line)).toEqual(["confirmation not awaited"]);
expect(lineOffences(line)).toContain("confirmation not awaited");
});
it.each([
"const c = confirmAction;",
"import { confirmAction as ask } from \"../services/confirmService\";",
"const handlers = { remove: confirmAction };",
"items.map(confirmAction);",
"if (await confirmAction(a) && check(confirmAction)) {",
])("flags confirmAction out of reach in %s", (line) => {
expect(lineOffences(line)).toContain("confirmAction out of reach");
});
it.each([
@ -97,9 +131,11 @@ describe("native dialog guard (#421)", () => {
"if (!(await deps.confirm(deps.t(key)))) return;",
"const ok = await confirmAction(message);",
"onClick={() => void removeKey({ confirm: confirmAction, t })}",
"import { confirmAction } from \"../../services/confirmService\";",
"export async function confirmAction(message: string): Promise<boolean> {",
"if ((await confirmAction(a)) && (await confirmAction(b))) {",
"deps.confirm.mockClear();",
"const confirmed = await ask(message, { kind: \"warning\" });",
"export async function confirmAction(message: string): Promise<boolean> {",
"t(\"common.confirm\")",
])("lets %s through", (line) => {
expect(lineOffences(line)).toEqual([]);