Skip to content

Commit 3e117c6

Browse files
overlookmoteloverlookmotel
committed
feat(linter/plugins): add defineRule API (#13945)
Add `defineRule` method for defining rules. This is the 2nd part of the "alternative API" mentioned in #9905 (comment). ```js import { defineRule } from 'oxlint'; export default defineRule({ meta: { /* ... */ }, create(context) { // Rule implementation } }); ``` or, using the alternative `createOnce` API: ```js export default defineRule({ meta: { /* ... */ }, createOnce(context) { // Rule implementation } }); ``` When using `createOnce`, `defineRule` creates a `create` method and adds it to the rule object. So the rule remains ESLint-compatible, while getting a potential speed boost when the rule is used with Oxlint. This PR includes a test that runs an Oxlint plugin that uses `defineRule` + `createOnce` in ESLint. Not done yet: We need to export TS type defs, so the type checking benefit of `defineRule` is realized. I'll do that in a later PR. --- `defineRule` is intended to be used when defining JS plugins, so we should move it into a separate NPM package (`oxlint-tools`?), so that someone publishing a plugin to NPM doesn't need their plugin to have a dependency on `oxlint` itself, which includes large binaries. But for now, for simplicity, just include it in `oxlint` package.
1 parent a018756 commit 3e117c6

File tree

15 files changed

+791
-2
lines changed

15 files changed

+791
-2
lines changed

‎apps/oxlint/package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{
22
"name": "oxlint",
33
"version": "1.16.0",
4+
"main": "dist/index.js",
45
"bin": "dist/cli.js",
56
"type": "module",
67
"scripts": {
@@ -34,6 +35,7 @@
3435
"dist"
3536
],
3637
"devDependencies": {
38+
"eslint": "^9.36.0",
3739
"execa": "^9.6.0",
3840
"tsdown": "0.15.1",
3941
"typescript": "catalog:",

‎apps/oxlint/src-js/index.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import type { Context } from './plugins/context.ts';
2+
import type { Rule } from './plugins/load.ts';
3+
4+
const { defineProperty, getPrototypeOf, setPrototypeOf } = Object;
5+
6+
const dummyOptions: unknown[] = [],
7+
dummyReport = () => {};
8+
9+
// Define a rule.
10+
// If rule has `createOnce` method, add an ESLint-compatible `create` method which delegates to `createOnce`.
11+
export function defineRule(rule: Rule): Rule {
12+
if (!('createOnce' in rule)) return rule;
13+
if ('create' in rule) throw new Error('Rules must define only `create` or `createOnce` methods, not both');
14+
15+
// Run `createOnce` with empty context object.
16+
// Really, `context` should be an instance of `Context`, which would throw error on accessing e.g. `id`
17+
// in body of `createOnce`. But any such bugs should have been caught when testing the rule in Oxlint,
18+
// so should be OK to take this shortcut.
19+
const context = Object.create(null, {
20+
id: { value: '', enumerable: true, configurable: true },
21+
options: { value: dummyOptions, enumerable: true, configurable: true },
22+
report: { value: dummyReport, enumerable: true, configurable: true },
23+
});
24+
25+
const { before: beforeHook, after: afterHook, ...visitor } = rule.createOnce(context as Context);
26+
27+
// Add `after` hook to `Program:exit` visit fn
28+
if (afterHook !== null) {
29+
const programExit = visitor['Program:exit'];
30+
visitor['Program:exit'] = programExit
31+
? (node) => {
32+
programExit(node);
33+
afterHook();
34+
}
35+
: (_node) => afterHook();
36+
}
37+
38+
// Create `create` function
39+
rule.create = (eslintContext) => {
40+
// Copy properties from ESLint's context object to `context`.
41+
// ESLint's context object is an object of form `{ id, options, report }`, with all other properties
42+
// and methods on another object which is its prototype.
43+
defineProperty(context, 'id', { value: eslintContext.id });
44+
defineProperty(context, 'options', { value: eslintContext.options });
45+
defineProperty(context, 'report', { value: eslintContext.report });
46+
setPrototypeOf(context, getPrototypeOf(eslintContext));
47+
48+
// If `before` hook returns `false`, skip rest of traversal by returning an empty object as visitor
49+
if (beforeHook !== null) {
50+
const shouldRun = beforeHook();
51+
if (shouldRun === false) return {};
52+
}
53+
54+
// Return same visitor each time
55+
return visitor;
56+
};
57+
58+
return rule;
59+
}

‎apps/oxlint/src-js/plugins/load.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ interface Plugin {
1616
// Linter rule.
1717
// `Rule` can have either `create` method, or `createOnce` method.
1818
// If `createOnce` method is present, `create` is ignored.
19-
type Rule = CreateRule | CreateOnceRule;
19+
export type Rule = CreateRule | CreateOnceRule;
2020

2121
interface CreateRule {
2222
create: (context: Context) => Visitor;

‎apps/oxlint/test/__snapshots__/e2e.test.ts.snap‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,118 @@ Found 0 warnings and 26 errors.
800800
Finished in Xms on 2 files using X threads."
801801
`;
802802
803+
exports[`oxlint CLI > should support \`defineRule\` 1`] = `
804+
"
805+
x define-rule-plugin(create): create body:
806+
| this === rule: true
807+
,-[files/1.js:1:1]
808+
1 | let a, b;
809+
: ^
810+
\`----
811+
812+
x define-rule-plugin(create-once): after hook:
813+
| identNum: 2
814+
| filename: files/1.js
815+
,-[files/1.js:1:1]
816+
1 | let a, b;
817+
: ^
818+
\`----
819+
820+
x define-rule-plugin(create-once): before hook:
821+
| this === rule: true
822+
| filename: files/1.js
823+
,-[files/1.js:1:1]
824+
1 | let a, b;
825+
: ^
826+
\`----
827+
828+
x define-rule-plugin(create): ident visit fn "a":
829+
| filename: files/1.js
830+
,-[files/1.js:1:5]
831+
1 | let a, b;
832+
: ^
833+
\`----
834+
835+
x define-rule-plugin(create-once): ident visit fn "a":
836+
| identNum: 1
837+
| filename: files/1.js
838+
,-[files/1.js:1:5]
839+
1 | let a, b;
840+
: ^
841+
\`----
842+
843+
x define-rule-plugin(create): ident visit fn "b":
844+
| filename: files/1.js
845+
,-[files/1.js:1:8]
846+
1 | let a, b;
847+
: ^
848+
\`----
849+
850+
x define-rule-plugin(create-once): ident visit fn "b":
851+
| identNum: 2
852+
| filename: files/1.js
853+
,-[files/1.js:1:8]
854+
1 | let a, b;
855+
: ^
856+
\`----
857+
858+
x define-rule-plugin(create): create body:
859+
| this === rule: true
860+
,-[files/2.js:1:1]
861+
1 | let c, d;
862+
: ^
863+
\`----
864+
865+
x define-rule-plugin(create-once): after hook:
866+
| identNum: 2
867+
| filename: files/2.js
868+
,-[files/2.js:1:1]
869+
1 | let c, d;
870+
: ^
871+
\`----
872+
873+
x define-rule-plugin(create-once): before hook:
874+
| this === rule: true
875+
| filename: files/2.js
876+
,-[files/2.js:1:1]
877+
1 | let c, d;
878+
: ^
879+
\`----
880+
881+
x define-rule-plugin(create): ident visit fn "c":
882+
| filename: files/2.js
883+
,-[files/2.js:1:5]
884+
1 | let c, d;
885+
: ^
886+
\`----
887+
888+
x define-rule-plugin(create-once): ident visit fn "c":
889+
| identNum: 1
890+
| filename: files/2.js
891+
,-[files/2.js:1:5]
892+
1 | let c, d;
893+
: ^
894+
\`----
895+
896+
x define-rule-plugin(create): ident visit fn "d":
897+
| filename: files/2.js
898+
,-[files/2.js:1:8]
899+
1 | let c, d;
900+
: ^
901+
\`----
902+
903+
x define-rule-plugin(create-once): ident visit fn "d":
904+
| identNum: 2
905+
| filename: files/2.js
906+
,-[files/2.js:1:8]
907+
1 | let c, d;
908+
: ^
909+
\`----
910+
911+
Found 0 warnings and 14 errors.
912+
Finished in Xms on 2 files using X threads."
913+
`;
914+
803915
exports[`oxlint CLI > should work with multiple rules 1`] = `
804916
"
805917
x basic-custom-plugin(no-debugger): Unexpected Debugger Statement

‎apps/oxlint/test/__snapshots__/eslint-compat.test.ts.snap‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
2+
3+
exports[`ESLint compatibility > \`defineRule\` should work 1`] = `
4+
"
5+
<root>/apps/oxlint/test/fixtures/defineRule/files/1.js
6+
0:1 error create body:
7+
this === rule: true testPlugin/create
8+
0:1 error before hook:
9+
this === rule: true
10+
filename: files/1.js testPlugin/create-once
11+
0:1 error after hook:
12+
identNum: 2
13+
filename: files/1.js testPlugin/create-once
14+
1:5 error ident visit fn "a":
15+
filename: files/1.js testPlugin/create
16+
1:5 error ident visit fn "a":
17+
identNum: 1
18+
filename: files/1.js testPlugin/create-once
19+
1:8 error ident visit fn "b":
20+
filename: files/1.js testPlugin/create
21+
1:8 error ident visit fn "b":
22+
identNum: 2
23+
filename: files/1.js testPlugin/create-once
24+
25+
<root>/apps/oxlint/test/fixtures/defineRule/files/2.js
26+
0:1 error create body:
27+
this === rule: true testPlugin/create
28+
0:1 error before hook:
29+
this === rule: true
30+
filename: files/2.js testPlugin/create-once
31+
0:1 error after hook:
32+
identNum: 2
33+
filename: files/2.js testPlugin/create-once
34+
1:5 error ident visit fn "c":
35+
filename: files/2.js testPlugin/create
36+
1:5 error ident visit fn "c":
37+
identNum: 1
38+
filename: files/2.js testPlugin/create-once
39+
1:8 error ident visit fn "d":
40+
filename: files/2.js testPlugin/create
41+
1:8 error ident visit fn "d":
42+
identNum: 2
43+
filename: files/2.js testPlugin/create-once
44+
45+
✖ 14 problems (14 errors, 0 warnings)
46+
"
47+
`;

‎apps/oxlint/test/e2e.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,12 @@ describe('oxlint CLI', () => {
154154
expect(normalizeOutput(stdout)).toMatchSnapshot();
155155
});
156156

157+
it('should support `defineRule`', async () => {
158+
const { stdout, exitCode } = await runOxlint('test/fixtures/defineRule');
159+
expect(exitCode).toBe(1);
160+
expect(normalizeOutput(stdout)).toMatchSnapshot();
161+
});
162+
157163
it('should have UTF-16 spans in AST', async () => {
158164
const { stdout, exitCode } = await runOxlint('test/fixtures/utf16_offsets');
159165
expect(exitCode).toBe(1);

‎apps/oxlint/test/eslint-compat.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { dirname, join as pathJoin } from 'node:path';
2+
3+
import { describe, expect, it } from 'vitest';
4+
5+
import { execa } from 'execa';
6+
7+
const PACKAGE_ROOT_PATH = dirname(import.meta.dirname);
8+
const REPO_ROOT_PATH = pathJoin(PACKAGE_ROOT_PATH, '../..');
9+
10+
async function runEslint(cwd: string, args: string[] = []) {
11+
return await execa('pnpx', ['eslint', ...args], {
12+
cwd: pathJoin(PACKAGE_ROOT_PATH, cwd),
13+
reject: false,
14+
});
15+
}
16+
17+
function normalizeOutput(output: string): string {
18+
const lines = output.split('\n');
19+
for (let i = lines.length - 1; i >= 0; i--) {
20+
const line = lines[i];
21+
if (line.startsWith(REPO_ROOT_PATH)) lines[i] = `<root>${line.slice(REPO_ROOT_PATH.length)}`;
22+
}
23+
return lines.join('\n');
24+
}
25+
26+
describe('ESLint compatibility', () => {
27+
it('`defineRule` should work', async () => {
28+
const { stdout, exitCode } = await runEslint('test/fixtures/defineRule');
29+
expect(exitCode).toBe(1);
30+
expect(normalizeOutput(stdout)).toMatchSnapshot();
31+
});
32+
});

‎apps/oxlint/test/fixtures/defineRule/.oxlintrc.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"plugins": ["./test_plugin"],
3+
"categories": {"correctness": "off"},
4+
"rules": {
5+
"define-rule-plugin/create": "error",
6+
"define-rule-plugin/create-once": "error"
7+
},
8+
"ignorePatterns": ["test_plugin/**", "eslint.config.js"]
9+
}

‎apps/oxlint/test/fixtures/defineRule/eslint.config.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import testPlugin from './test_plugin/index.js';
2+
3+
export default [
4+
{
5+
files: ["files/*.js"],
6+
plugins: {
7+
testPlugin,
8+
},
9+
rules: {
10+
"testPlugin/create": "error",
11+
"testPlugin/create-once": "error",
12+
},
13+
},
14+
];

‎apps/oxlint/test/fixtures/defineRule/files/1.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
let a, b;

0 commit comments

Comments
 (0)