From fb3e7e36a3473360756aa6d99e3a1e49d3135ff5 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue, 23 Sep 2025 23:40:33 +0000 Subject: [PATCH] fix(linter/plugins): `defineRule` accept visitor with no `before` / `after` hooks (#14060) Fix `defineRule`. Previously it would throw an error if either `before` or `after` hooks was missing. Make it accept that without error. --- apps/oxlint/src-js/index.ts | 12 +- .../test/__snapshots__/e2e.test.ts.snap | 200 +++++++++++++++++- .../__snapshots__/eslint-compat.test.ts.snap | 34 ++- .../test/fixtures/createOnce/.oxlintrc.json | 5 +- .../fixtures/createOnce/test_plugin/index.js | 50 +++++ .../test/fixtures/define/.oxlintrc.json | 5 +- .../test/fixtures/define/eslint.config.js | 3 + .../test/fixtures/define/test_plugin/index.js | 61 ++++++ 8 files changed, 363 insertions(+), 7 deletions(-) diff --git a/apps/oxlint/src-js/index.ts b/apps/oxlint/src-js/index.ts index de8c61267eec0..14c24e4d25b3c 100644 --- a/apps/oxlint/src-js/index.ts +++ b/apps/oxlint/src-js/index.ts @@ -27,10 +27,18 @@ export function defineRule(rule: Rule): Rule { report: { value: dummyReport, enumerable: true, configurable: true }, }); - const { before: beforeHook, after: afterHook, ...visitor } = rule.createOnce(context as Context); + let { before: beforeHook, after: afterHook, ...visitor } = rule.createOnce(context as Context); + + if (beforeHook === void 0) { + beforeHook = null; + } else if (beforeHook !== null && typeof beforeHook !== 'function') { + throw new Error('`before` property of visitor must be a function if defined'); + } // Add `after` hook to `Program:exit` visit fn - if (afterHook !== null) { + if (afterHook != null) { + if (typeof afterHook !== 'function') throw new Error('`after` property of visitor must be a function if defined'); + const programExit = visitor['Program:exit']; visitor['Program:exit'] = programExit ? (node) => { diff --git a/apps/oxlint/test/__snapshots__/e2e.test.ts.snap b/apps/oxlint/test/__snapshots__/e2e.test.ts.snap index 717a4880346fb..31c7cec0d21bf 100644 --- a/apps/oxlint/test/__snapshots__/e2e.test.ts.snap +++ b/apps/oxlint/test/__snapshots__/e2e.test.ts.snap @@ -640,6 +640,18 @@ Finished in Xms on 1 file using X threads." exports[`oxlint CLI > should support \`createOnce\` 1`] = ` " + x create-once-plugin(after-only): after hook: filename: files/1.js + ,-[files/1.js:1:1] + 1 | let x; + : ^ + \`---- + + x create-once-plugin(after-only): after hook: id: create-once-plugin/after-only + ,-[files/1.js:1:1] + 1 | let x; + : ^ + \`---- + x create-once-plugin(always-run): createOnce: filename: Cannot access \`context.filename\` in \`createOnce\` ,-[files/1.js:1:1] 1 | let x; @@ -700,6 +712,18 @@ exports[`oxlint CLI > should support \`createOnce\` 1`] = ` : ^ \`---- + x create-once-plugin(before-only): before hook: filename: files/1.js + ,-[files/1.js:1:1] + 1 | let x; + : ^ + \`---- + + x create-once-plugin(before-only): before hook: id: create-once-plugin/before-only + ,-[files/1.js:1:1] + 1 | let x; + : ^ + \`---- + x create-once-plugin(skip-run): before hook: filename: files/1.js ,-[files/1.js:1:1] 1 | let x; @@ -712,12 +736,42 @@ exports[`oxlint CLI > should support \`createOnce\` 1`] = ` : ^ \`---- + x create-once-plugin(after-only): ident visit fn "x": filename: files/1.js + ,-[files/1.js:1:5] + 1 | let x; + : ^ + \`---- + x create-once-plugin(always-run): ident visit fn "x": filename: files/1.js ,-[files/1.js:1:5] 1 | let x; : ^ \`---- + x create-once-plugin(before-only): ident visit fn "x": filename: files/1.js + ,-[files/1.js:1:5] + 1 | let x; + : ^ + \`---- + + x create-once-plugin(no-hooks): ident visit fn "x": filename: files/1.js + ,-[files/1.js:1:5] + 1 | let x; + : ^ + \`---- + + x create-once-plugin(after-only): after hook: filename: files/2.js + ,-[files/2.js:1:1] + 1 | let y; + : ^ + \`---- + + x create-once-plugin(after-only): after hook: id: create-once-plugin/after-only + ,-[files/2.js:1:1] + 1 | let y; + : ^ + \`---- + x create-once-plugin(always-run): createOnce: filename: Cannot access \`context.filename\` in \`createOnce\` ,-[files/2.js:1:1] 1 | let y; @@ -778,6 +832,18 @@ exports[`oxlint CLI > should support \`createOnce\` 1`] = ` : ^ \`---- + x create-once-plugin(before-only): before hook: filename: files/2.js + ,-[files/2.js:1:1] + 1 | let y; + : ^ + \`---- + + x create-once-plugin(before-only): before hook: id: create-once-plugin/before-only + ,-[files/2.js:1:1] + 1 | let y; + : ^ + \`---- + x create-once-plugin(skip-run): before hook: filename: files/2.js ,-[files/2.js:1:1] 1 | let y; @@ -790,13 +856,31 @@ exports[`oxlint CLI > should support \`createOnce\` 1`] = ` : ^ \`---- + x create-once-plugin(after-only): ident visit fn "y": filename: files/2.js + ,-[files/2.js:1:5] + 1 | let y; + : ^ + \`---- + x create-once-plugin(always-run): ident visit fn "y": filename: files/2.js ,-[files/2.js:1:5] 1 | let y; : ^ \`---- -Found 0 warnings and 26 errors. + x create-once-plugin(before-only): ident visit fn "y": filename: files/2.js + ,-[files/2.js:1:5] + 1 | let y; + : ^ + \`---- + + x create-once-plugin(no-hooks): ident visit fn "y": filename: files/2.js + ,-[files/2.js:1:5] + 1 | let y; + : ^ + \`---- + +Found 0 warnings and 40 errors. Finished in Xms on 2 files using X threads." `; @@ -825,6 +909,20 @@ exports[`oxlint CLI > should support \`defineRule\` + \`definePlugin\` 1`] = ` : ^ \`---- + x define-rule-plugin(create-once-after-only): after hook: + | filename: files/1.js + ,-[files/1.js:1:1] + 1 | let a, b; + : ^ + \`---- + + x define-rule-plugin(create-once-before-only): before hook: + | filename: files/1.js + ,-[files/1.js:1:1] + 1 | let a, b; + : ^ + \`---- + x define-rule-plugin(create): ident visit fn "a": | filename: files/1.js ,-[files/1.js:1:5] @@ -840,6 +938,27 @@ exports[`oxlint CLI > should support \`defineRule\` + \`definePlugin\` 1`] = ` : ^ \`---- + x define-rule-plugin(create-once-after-only): ident visit fn "a": + | filename: files/1.js + ,-[files/1.js:1:5] + 1 | let a, b; + : ^ + \`---- + + x define-rule-plugin(create-once-before-only): ident visit fn "a": + | filename: files/1.js + ,-[files/1.js:1:5] + 1 | let a, b; + : ^ + \`---- + + x define-rule-plugin(create-once-no-hooks): ident visit fn "a": + | filename: files/1.js + ,-[files/1.js:1:5] + 1 | let a, b; + : ^ + \`---- + x define-rule-plugin(create): ident visit fn "b": | filename: files/1.js ,-[files/1.js:1:8] @@ -855,6 +974,27 @@ exports[`oxlint CLI > should support \`defineRule\` + \`definePlugin\` 1`] = ` : ^ \`---- + x define-rule-plugin(create-once-after-only): ident visit fn "b": + | filename: files/1.js + ,-[files/1.js:1:8] + 1 | let a, b; + : ^ + \`---- + + x define-rule-plugin(create-once-before-only): ident visit fn "b": + | filename: files/1.js + ,-[files/1.js:1:8] + 1 | let a, b; + : ^ + \`---- + + x define-rule-plugin(create-once-no-hooks): ident visit fn "b": + | filename: files/1.js + ,-[files/1.js:1:8] + 1 | let a, b; + : ^ + \`---- + x define-rule-plugin(create): create body: | this === rule: true ,-[files/2.js:1:1] @@ -878,6 +1018,20 @@ exports[`oxlint CLI > should support \`defineRule\` + \`definePlugin\` 1`] = ` : ^ \`---- + x define-rule-plugin(create-once-after-only): after hook: + | filename: files/2.js + ,-[files/2.js:1:1] + 1 | let c, d; + : ^ + \`---- + + x define-rule-plugin(create-once-before-only): before hook: + | filename: files/2.js + ,-[files/2.js:1:1] + 1 | let c, d; + : ^ + \`---- + x define-rule-plugin(create): ident visit fn "c": | filename: files/2.js ,-[files/2.js:1:5] @@ -893,6 +1047,27 @@ exports[`oxlint CLI > should support \`defineRule\` + \`definePlugin\` 1`] = ` : ^ \`---- + x define-rule-plugin(create-once-after-only): ident visit fn "c": + | filename: files/2.js + ,-[files/2.js:1:5] + 1 | let c, d; + : ^ + \`---- + + x define-rule-plugin(create-once-before-only): ident visit fn "c": + | filename: files/2.js + ,-[files/2.js:1:5] + 1 | let c, d; + : ^ + \`---- + + x define-rule-plugin(create-once-no-hooks): ident visit fn "c": + | filename: files/2.js + ,-[files/2.js:1:5] + 1 | let c, d; + : ^ + \`---- + x define-rule-plugin(create): ident visit fn "d": | filename: files/2.js ,-[files/2.js:1:8] @@ -908,7 +1083,28 @@ exports[`oxlint CLI > should support \`defineRule\` + \`definePlugin\` 1`] = ` : ^ \`---- -Found 0 warnings and 14 errors. + x define-rule-plugin(create-once-after-only): ident visit fn "d": + | filename: files/2.js + ,-[files/2.js:1:8] + 1 | let c, d; + : ^ + \`---- + + x define-rule-plugin(create-once-before-only): ident visit fn "d": + | filename: files/2.js + ,-[files/2.js:1:8] + 1 | let c, d; + : ^ + \`---- + + x define-rule-plugin(create-once-no-hooks): ident visit fn "d": + | filename: files/2.js + ,-[files/2.js:1:8] + 1 | let c, d; + : ^ + \`---- + +Found 0 warnings and 30 errors. Finished in Xms on 2 files using X threads." `; diff --git a/apps/oxlint/test/__snapshots__/eslint-compat.test.ts.snap b/apps/oxlint/test/__snapshots__/eslint-compat.test.ts.snap index 01b458c78c287..8db71305b0e83 100644 --- a/apps/oxlint/test/__snapshots__/eslint-compat.test.ts.snap +++ b/apps/oxlint/test/__snapshots__/eslint-compat.test.ts.snap @@ -8,19 +8,35 @@ this === rule: true define-rule-plugin/create 0:1 error before hook: this === rule: true filename: files/1.js define-rule-plugin/create-once + 0:1 error before hook: +filename: files/1.js define-rule-plugin/create-once-before-only 0:1 error after hook: identNum: 2 filename: files/1.js define-rule-plugin/create-once + 0:1 error after hook: +filename: files/1.js define-rule-plugin/create-once-after-only 1:5 error ident visit fn "a": filename: files/1.js define-rule-plugin/create 1:5 error ident visit fn "a": identNum: 1 filename: files/1.js define-rule-plugin/create-once + 1:5 error ident visit fn "a": +filename: files/1.js define-rule-plugin/create-once-before-only + 1:5 error ident visit fn "a": +filename: files/1.js define-rule-plugin/create-once-after-only + 1:5 error ident visit fn "a": +filename: files/1.js define-rule-plugin/create-once-no-hooks 1:8 error ident visit fn "b": filename: files/1.js define-rule-plugin/create 1:8 error ident visit fn "b": identNum: 2 filename: files/1.js define-rule-plugin/create-once + 1:8 error ident visit fn "b": +filename: files/1.js define-rule-plugin/create-once-before-only + 1:8 error ident visit fn "b": +filename: files/1.js define-rule-plugin/create-once-after-only + 1:8 error ident visit fn "b": +filename: files/1.js define-rule-plugin/create-once-no-hooks /apps/oxlint/test/fixtures/define/files/2.js 0:1 error create body: @@ -28,20 +44,36 @@ this === rule: true define-rule-plugin/create 0:1 error before hook: this === rule: true filename: files/2.js define-rule-plugin/create-once + 0:1 error before hook: +filename: files/2.js define-rule-plugin/create-once-before-only 0:1 error after hook: identNum: 2 filename: files/2.js define-rule-plugin/create-once + 0:1 error after hook: +filename: files/2.js define-rule-plugin/create-once-after-only 1:5 error ident visit fn "c": filename: files/2.js define-rule-plugin/create 1:5 error ident visit fn "c": identNum: 1 filename: files/2.js define-rule-plugin/create-once + 1:5 error ident visit fn "c": +filename: files/2.js define-rule-plugin/create-once-before-only + 1:5 error ident visit fn "c": +filename: files/2.js define-rule-plugin/create-once-after-only + 1:5 error ident visit fn "c": +filename: files/2.js define-rule-plugin/create-once-no-hooks 1:8 error ident visit fn "d": filename: files/2.js define-rule-plugin/create 1:8 error ident visit fn "d": identNum: 2 filename: files/2.js define-rule-plugin/create-once + 1:8 error ident visit fn "d": +filename: files/2.js define-rule-plugin/create-once-before-only + 1:8 error ident visit fn "d": +filename: files/2.js define-rule-plugin/create-once-after-only + 1:8 error ident visit fn "d": +filename: files/2.js define-rule-plugin/create-once-no-hooks -✖ 14 problems (14 errors, 0 warnings) +✖ 30 problems (30 errors, 0 warnings) " `; diff --git a/apps/oxlint/test/fixtures/createOnce/.oxlintrc.json b/apps/oxlint/test/fixtures/createOnce/.oxlintrc.json index 420f65a46c1e6..ca3cd35d81c63 100644 --- a/apps/oxlint/test/fixtures/createOnce/.oxlintrc.json +++ b/apps/oxlint/test/fixtures/createOnce/.oxlintrc.json @@ -3,7 +3,10 @@ "categories": {"correctness": "off"}, "rules": { "create-once-plugin/always-run": "error", - "create-once-plugin/skip-run": "error" + "create-once-plugin/skip-run": "error", + "create-once-plugin/before-only": "error", + "create-once-plugin/after-only": "error", + "create-once-plugin/no-hooks": "error" }, "ignorePatterns": ["test_plugin/**"] } diff --git a/apps/oxlint/test/fixtures/createOnce/test_plugin/index.js b/apps/oxlint/test/fixtures/createOnce/test_plugin/index.js index 66e5216572a47..e7ec78678b30f 100644 --- a/apps/oxlint/test/fixtures/createOnce/test_plugin/index.js +++ b/apps/oxlint/test/fixtures/createOnce/test_plugin/index.js @@ -64,6 +64,53 @@ const skipRunRule = { }, }; +const beforeOnlyRule = { + createOnce(context) { + return { + before() { + context.report({ message: `before hook: id: ${context.id}`, node: SPAN }); + context.report({ message: `before hook: filename: ${relativePath(context.filename)}`, node: SPAN }); + }, + Identifier(node) { + context.report({ + message: `ident visit fn "${node.name}": filename: ${relativePath(context.filename)}`, + node, + }); + }, + }; + }, +}; + +const afterOnlyRule = { + createOnce(context) { + return { + Identifier(node) { + context.report({ + message: `ident visit fn "${node.name}": filename: ${relativePath(context.filename)}`, + node, + }); + }, + after() { + context.report({ message: `after hook: id: ${context.id}`, node: SPAN }); + context.report({ message: `after hook: filename: ${relativePath(context.filename)}`, node: SPAN }); + }, + }; + }, +}; + +const noHooksRule = { + createOnce(context) { + return { + Identifier(node) { + context.report({ + message: `ident visit fn "${node.name}": filename: ${relativePath(context.filename)}`, + node, + }); + }, + }; + }, +}; + export default { meta: { name: "create-once-plugin", @@ -71,6 +118,9 @@ export default { rules: { "always-run": alwaysRunRule, "skip-run": skipRunRule, + "before-only": beforeOnlyRule, + "after-only": afterOnlyRule, + "no-hooks": noHooksRule, }, }; diff --git a/apps/oxlint/test/fixtures/define/.oxlintrc.json b/apps/oxlint/test/fixtures/define/.oxlintrc.json index 3f2263bf1c12b..70da84b641ce4 100644 --- a/apps/oxlint/test/fixtures/define/.oxlintrc.json +++ b/apps/oxlint/test/fixtures/define/.oxlintrc.json @@ -3,7 +3,10 @@ "categories": {"correctness": "off"}, "rules": { "define-rule-plugin/create": "error", - "define-rule-plugin/create-once": "error" + "define-rule-plugin/create-once": "error", + "define-rule-plugin/create-once-before-only": "error", + "define-rule-plugin/create-once-after-only": "error", + "define-rule-plugin/create-once-no-hooks": "error" }, "ignorePatterns": ["test_plugin/**", "eslint.config.js"] } diff --git a/apps/oxlint/test/fixtures/define/eslint.config.js b/apps/oxlint/test/fixtures/define/eslint.config.js index 7f82b35c9d687..a3898246ffd6f 100644 --- a/apps/oxlint/test/fixtures/define/eslint.config.js +++ b/apps/oxlint/test/fixtures/define/eslint.config.js @@ -9,6 +9,9 @@ export default [ rules: { "define-rule-plugin/create": "error", "define-rule-plugin/create-once": "error", + "define-rule-plugin/create-once-before-only": "error", + "define-rule-plugin/create-once-after-only": "error", + "define-rule-plugin/create-once-no-hooks": "error", }, }, ]; diff --git a/apps/oxlint/test/fixtures/define/test_plugin/index.js b/apps/oxlint/test/fixtures/define/test_plugin/index.js index 524eedbb14bd1..68878f0c14a6e 100644 --- a/apps/oxlint/test/fixtures/define/test_plugin/index.js +++ b/apps/oxlint/test/fixtures/define/test_plugin/index.js @@ -100,6 +100,64 @@ const createOnceRule = defineRule({ }, }); +// These 3 rules test that `createOnce` without `before` and `after` hooks works correctly. + +const createOnceBeforeOnlyRule = defineRule({ + createOnce(context) { + return { + before() { + context.report({ + message: 'before hook:\n' + + `filename: ${relativePath(context.filename)}`, + node: SPAN, + }); + }, + Identifier(node) { + context.report({ + message: `ident visit fn "${node.name}":\n` + + `filename: ${relativePath(context.filename)}`, + node: { ...SPAN, ...node }, + }); + }, + }; + }, +}); + +const createOnceAfterOnlyRule = defineRule({ + createOnce(context) { + return { + Identifier(node) { + context.report({ + message: `ident visit fn "${node.name}":\n` + + `filename: ${relativePath(context.filename)}`, + node: { ...SPAN, ...node }, + }); + }, + after() { + context.report({ + message: 'after hook:\n' + + `filename: ${relativePath(context.filename)}`, + node: SPAN, + }); + }, + }; + }, +}); + +const createOnceNoHooksRule = defineRule({ + createOnce(context) { + return { + Identifier(node) { + context.report({ + message: `ident visit fn "${node.name}":\n` + + `filename: ${relativePath(context.filename)}`, + node: { ...SPAN, ...node }, + }); + }, + }; + }, +}); + export default definePlugin({ meta: { name: "define-rule-plugin", @@ -107,5 +165,8 @@ export default definePlugin({ rules: { create: createRule, "create-once": createOnceRule, + "create-once-before-only": createOnceBeforeOnlyRule, + "create-once-after-only": createOnceAfterOnlyRule, + "create-once-no-hooks": createOnceNoHooksRule, }, });