From bef875343e3f7cc2dd855e10d314a728e614835f Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Mon, 22 Sep 2025 11:16:33 +0000 Subject: [PATCH] feat(linter/plugins): ESTree-compatible AST for JS plugins (#13942) Use eager deserialization instead of lazy deserialization in linter plugins, so AST custom rules receive is ESTree-compatible. Unfortunately this reduces performance significantly, but we'll bring back lazy deserialization later on once we have it producing an ESTree-compatible AST. --- apps/oxlint/scripts/build.js | 7 ++ apps/oxlint/src-js/plugins/lint.ts | 17 ++++ apps/oxlint/src-js/plugins/types.ts | 5 ++ apps/oxlint/src-js/plugins/visitor.ts | 10 ++- .../test/__snapshots__/e2e.test.ts.snap | 52 +++++++++++- apps/oxlint/test/compile-visitor.test.ts | 7 ++ apps/oxlint/test/e2e.test.ts | 6 ++ .../.oxlintrc.json | 2 +- .../test_plugin/index.js | 11 +-- .../test/fixtures/estree/.oxlintrc.json | 10 +++ apps/oxlint/test/fixtures/estree/index.ts | 11 +++ .../test/fixtures/estree/test_plugin/index.js | 80 +++++++++++++++++++ apps/oxlint/tsconfig.json | 3 +- 13 files changed, 209 insertions(+), 12 deletions(-) create mode 100644 apps/oxlint/test/fixtures/estree/.oxlintrc.json create mode 100644 apps/oxlint/test/fixtures/estree/index.ts create mode 100644 apps/oxlint/test/fixtures/estree/test_plugin/index.js diff --git a/apps/oxlint/scripts/build.js b/apps/oxlint/scripts/build.js index 7691151e15ead..622330dffb092 100755 --- a/apps/oxlint/scripts/build.js +++ b/apps/oxlint/scripts/build.js @@ -24,11 +24,18 @@ execSync('pnpm tsdown', { stdio: 'inherit', cwd: oxlintDirPath }); console.log('Copying files from parser...'); const parserFilePaths = [ + // Lazy implementation + /* 'src-js/raw-transfer/lazy-common.mjs', 'src-js/raw-transfer/node-array.mjs', 'generated/lazy/constructors.mjs', 'generated/lazy/types.mjs', 'generated/lazy/walk.mjs', + */ + 'generated/deserialize/ts.mjs', + 'generated/visit/types.mjs', + 'generated/visit/visitor.d.ts', + 'generated/visit/walk.mjs', ]; for (const parserFilePath of parserFilePaths) { diff --git a/apps/oxlint/src-js/plugins/lint.ts b/apps/oxlint/src-js/plugins/lint.ts index c9bd596de3342..25c7cb126cdce 100644 --- a/apps/oxlint/src-js/plugins/lint.ts +++ b/apps/oxlint/src-js/plugins/lint.ts @@ -9,10 +9,18 @@ import { registeredRules } from './load.js'; import { assertIs } from './utils.js'; import { addVisitorToCompiled, compiledVisitor, finalizeCompiledVisitor, initCompiledVisitor } from './visitor.js'; +// Lazy implementation +/* // @ts-expect-error we need to generate `.d.ts` file for this module. import { TOKEN } from '../../dist/src-js/raw-transfer/lazy-common.mjs'; // @ts-expect-error we need to generate `.d.ts` file for this module. import { walkProgram } from '../../dist/generated/lazy/walk.mjs'; +*/ + +// @ts-expect-error we need to generate `.d.ts` file for this module +import { deserializeProgramOnly } from '../../dist/generated/deserialize/ts.mjs'; +// @ts-expect-error we need to generate `.d.ts` file for this module +import { walkProgram } from '../../dist/generated/visit/walk.mjs'; // Buffer with typed array views of itself stored as properties interface BufferWithArrays extends Uint8Array { @@ -80,6 +88,14 @@ export function lintFile(filePath: string, bufferId: number, buffer: Uint8Array sourceByteLen = uint32[(programPos + SOURCE_LEN_OFFSET) >> 2]; const sourceText = textDecoder.decode(buffer.subarray(0, sourceByteLen)); + + // `preserveParens` argument is `false`, to match ESLint. + // ESLint does not include `ParenthesizedExpression` nodes in its AST. + const program = deserializeProgramOnly(buffer, sourceText, sourceByteLen, false); + walkProgram(program, compiledVisitor); + + // Lazy implementation + /* const sourceIsAscii = sourceText.length === sourceByteLen; const ast = { buffer, @@ -91,6 +107,7 @@ export function lintFile(filePath: string, bufferId: number, buffer: Uint8Array }; walkProgram(programPos, ast, compiledVisitor); + */ } // Send diagnostics back to Rust diff --git a/apps/oxlint/src-js/plugins/types.ts b/apps/oxlint/src-js/plugins/types.ts index 7f6b80e6a17df..87eeb83174f0c 100644 --- a/apps/oxlint/src-js/plugins/types.ts +++ b/apps/oxlint/src-js/plugins/types.ts @@ -1,7 +1,12 @@ +// Lazy implementation +/* // Visitor object returned by a `Rule`'s `create` function. export interface Visitor { [key: string]: VisitFn; } +*/ + +export type { VisitorObject as Visitor } from '../../dist/generated/visit/visitor.d.ts'; // Visit function for a specific AST node type. export type VisitFn = (node: Node) => void; diff --git a/apps/oxlint/src-js/plugins/visitor.ts b/apps/oxlint/src-js/plugins/visitor.ts index 2a7b5471c191e..3b83ed4bafca8 100644 --- a/apps/oxlint/src-js/plugins/visitor.ts +++ b/apps/oxlint/src-js/plugins/visitor.ts @@ -72,9 +72,15 @@ // for objects created by user code in visitors. If ephemeral user-created objects all fit in new space, // it will avoid full GC runs, which should greatly improve performance. +// Lazy implementation +/* // TODO(camc314): we need to generate `.d.ts` file for this module. -// @ts-expect-error import { LEAF_NODE_TYPES_COUNT, NODE_TYPE_IDS_MAP, NODE_TYPES_COUNT } from '../../dist/generated/lazy/types.mjs'; +*/ + +// TODO(camc314): we need to generate `.d.ts` file for this module. +// @ts-expect-error +import { LEAF_NODE_TYPES_COUNT, NODE_TYPE_IDS_MAP, NODE_TYPES_COUNT } from '../../dist/generated/visit/types.mjs'; import { assertIs } from './utils.js'; import type { CompiledVisitorEntry, EnterExit, Node, VisitFn, Visitor } from './types.ts'; @@ -214,7 +220,7 @@ export function addVisitorToCompiled(visitor: Visitor): void { for (let i = 0; i < keysLen; i++) { let name = keys[i]; - const visitFn = visitor[name]; + const visitFn = (visitor as { [key: string]: VisitFn })[name]; if (typeof visitFn !== 'function') { throw new TypeError(`'${name}' property of visitor object is not a function`); } diff --git a/apps/oxlint/test/__snapshots__/e2e.test.ts.snap b/apps/oxlint/test/__snapshots__/e2e.test.ts.snap index 33224d92319b2..1991b12ba05d2 100644 --- a/apps/oxlint/test/__snapshots__/e2e.test.ts.snap +++ b/apps/oxlint/test/__snapshots__/e2e.test.ts.snap @@ -444,6 +444,56 @@ Found 1 warning and 6 errors. Finished in Xms on 1 file using X threads." `; +exports[`oxlint CLI > should receive ESTree-compatible AST 1`] = ` +" + x estree-check(check): Visited nodes: + | * Program + | * VariableDeclaration: let + | * VariableDeclarator: (init: ObjectExpression) + | * Identifier: a + | * ObjectExpression + | * Identifier: x + | * Identifier: y + | * VariableDeclaration:exit: let + | * VariableDeclaration: const + | * VariableDeclarator: (init: BinaryExpression) + | * Identifier: b + | * BinaryExpression: * (right: BinaryExpression) + | * Identifier: x + | * BinaryExpression: + (right: Literal) + | * Literal: str + | * Literal: 123 + | * VariableDeclaration:exit: const + | * TSTypeAliasDeclaration: (typeAnnotation: TSStringKeyword) + | * Identifier: T + | * TSStringKeyword + | * TSTypeAliasDeclaration:exit: (typeAnnotation: TSStringKeyword) + | * TSTypeAliasDeclaration: (typeAnnotation: TSUnionType) + | * Identifier: U + | * TSUnionType: (types: TSStringKeyword, TSNumberKeyword) + | * TSStringKeyword + | * TSNumberKeyword + | * TSUnionType:exit: (types: TSStringKeyword, TSNumberKeyword) + | * TSTypeAliasDeclaration:exit: (typeAnnotation: TSUnionType) + | * Program:exit + ,-[index.ts:2:1] + 1 | // All \`Identifier\`s + 2 | ,-> let a = { x: y }; + 3 | | + 4 | | // No \`ParenthesizedExpression\`s in AST + 5 | | const b = (x * ((('str' + ((123)))))); + 6 | | + 7 | | // TS syntax + 8 | | type T = string; + 9 | | + 10 | | // No \`TSParenthesizedType\`s in AST + 11 | \`-> type U = (((((string)) | ((number))))); + \`---- + +Found 0 warnings and 1 error. +Finished in Xms on 1 file using X threads." +`; + exports[`oxlint CLI > should receive data via \`context\` 1`] = ` " x context-plugin(log-context): id: context-plugin/log-context @@ -612,7 +662,7 @@ exports[`oxlint CLI > should work with multiple rules 1`] = ` \`---- help: Remove the debugger statement - x basic-custom-plugin(no-ident-references-named-foo): Unexpected Identifier Reference named foo + x basic-custom-plugin(no-identifiers-named-foo): Unexpected Identifier named foo ,-[index.js:3:1] 2 | 3 | foo; diff --git a/apps/oxlint/test/compile-visitor.test.ts b/apps/oxlint/test/compile-visitor.test.ts index 8918f3c334e82..4e20a73532a71 100644 --- a/apps/oxlint/test/compile-visitor.test.ts +++ b/apps/oxlint/test/compile-visitor.test.ts @@ -1,7 +1,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +// Lazy implementation +/* // TODO(camc314): we need to generate `.d.ts` file for this module. // @ts-expect-error import { NODE_TYPE_IDS_MAP } from '../dist/generated/lazy/types.mjs'; +*/ +// TODO(camc314): we need to generate `.d.ts` file for this module +// @ts-expect-error +import { NODE_TYPE_IDS_MAP } from '../dist/generated/visit/types.mjs'; import { addVisitorToCompiled, compiledVisitor, @@ -30,6 +36,7 @@ describe('compile visitor', () => { }); it('throws if unknown visitor key', () => { + // @ts-expect-error expect(() => addVisitorToCompiled({ Foo() {} })) .toThrow(new Error("Unknown node type 'Foo' in visitor object")); }); diff --git a/apps/oxlint/test/e2e.test.ts b/apps/oxlint/test/e2e.test.ts index 8fa7df2400244..718dcba1f9502 100644 --- a/apps/oxlint/test/e2e.test.ts +++ b/apps/oxlint/test/e2e.test.ts @@ -136,6 +136,12 @@ describe('oxlint CLI', () => { expect(normalizeOutput(stdout)).toMatchSnapshot(); }); + it('should receive ESTree-compatible AST', async () => { + const { stdout, exitCode } = await runOxlint('test/fixtures/estree'); + expect(exitCode).toBe(1); + expect(normalizeOutput(stdout)).toMatchSnapshot(); + }); + it('should receive data via `context`', async () => { const { stdout, exitCode } = await runOxlint('test/fixtures/context_properties'); expect(exitCode).toBe(1); diff --git a/apps/oxlint/test/fixtures/basic_custom_plugin_multiple_rules/.oxlintrc.json b/apps/oxlint/test/fixtures/basic_custom_plugin_multiple_rules/.oxlintrc.json index b90b01941ee3b..8dcbc19f0211e 100644 --- a/apps/oxlint/test/fixtures/basic_custom_plugin_multiple_rules/.oxlintrc.json +++ b/apps/oxlint/test/fixtures/basic_custom_plugin_multiple_rules/.oxlintrc.json @@ -3,7 +3,7 @@ "rules": { "basic-custom-plugin/no-debugger": "error", "basic-custom-plugin/no-debugger-2": "error", - "basic-custom-plugin/no-ident-references-named-foo": "error" + "basic-custom-plugin/no-identifiers-named-foo": "error" }, "ignorePatterns": ["test_plugin/**"] } diff --git a/apps/oxlint/test/fixtures/basic_custom_plugin_multiple_rules/test_plugin/index.js b/apps/oxlint/test/fixtures/basic_custom_plugin_multiple_rules/test_plugin/index.js index dcb1d566fa178..736a42838a635 100644 --- a/apps/oxlint/test/fixtures/basic_custom_plugin_multiple_rules/test_plugin/index.js +++ b/apps/oxlint/test/fixtures/basic_custom_plugin_multiple_rules/test_plugin/index.js @@ -27,15 +27,12 @@ export default { }; }, }, - "no-ident-references-named-foo": { + "no-identifiers-named-foo": { create(context) { return { - IdentifierReference(identifierReference) { - if (identifierReference.name == "foo") { - context.report({ - message: "Unexpected Identifier Reference named foo", - node: identifierReference, - }); + Identifier(ident) { + if (ident.name == "foo") { + context.report({ message: "Unexpected Identifier named foo", node: ident}); } }, }; diff --git a/apps/oxlint/test/fixtures/estree/.oxlintrc.json b/apps/oxlint/test/fixtures/estree/.oxlintrc.json new file mode 100644 index 0000000000000..77a0b689b4c27 --- /dev/null +++ b/apps/oxlint/test/fixtures/estree/.oxlintrc.json @@ -0,0 +1,10 @@ +{ + "plugins": ["./test_plugin"], + "categories": { + "correctness": "off" + }, + "rules": { + "estree-check/check": "error" + }, + "ignorePatterns": ["test_plugin/**"] +} diff --git a/apps/oxlint/test/fixtures/estree/index.ts b/apps/oxlint/test/fixtures/estree/index.ts new file mode 100644 index 0000000000000..1f48a09774e1d --- /dev/null +++ b/apps/oxlint/test/fixtures/estree/index.ts @@ -0,0 +1,11 @@ +// All `Identifier`s +let a = { x: y }; + +// No `ParenthesizedExpression`s in AST +const b = (x * ((('str' + ((123)))))); + +// TS syntax +type T = string; + +// No `TSParenthesizedType`s in AST +type U = (((((string)) | ((number))))); diff --git a/apps/oxlint/test/fixtures/estree/test_plugin/index.js b/apps/oxlint/test/fixtures/estree/test_plugin/index.js new file mode 100644 index 0000000000000..6256289ee6ab0 --- /dev/null +++ b/apps/oxlint/test/fixtures/estree/test_plugin/index.js @@ -0,0 +1,80 @@ +export default { + meta: { + name: "estree-check", + }, + rules: { + check: { + create(context) { + // Note: Collect visits in an array instead of `context.report` in each visitor function, + // to ensure visitation happens in right order. + // Diagnostics may be output in different order from the order they're created in. + const visits = []; + return { + Program(program) { + visits.push(program.type); + }, + VariableDeclaration(decl) { + visits.push(`${decl.type}: ${decl.kind}`); + }, + 'VariableDeclaration:exit'(decl) { + visits.push(`${decl.type}:exit: ${decl.kind}`); + }, + VariableDeclarator(decl) { + // `init` should not be `ParenthesizedExpression` + visits.push(`${decl.type}: (init: ${decl.init.type})`); + }, + Identifier(ident) { + visits.push(`${ident.type}: ${ident.name}`); + }, + ObjectExpression(expr) { + visits.push(expr.type); + }, + ParenthesizedExpression(paren) { + // Should not be visited - no `ParenthesizedExpression`s in AST in ESLint + visits.push(paren.type); + }, + BinaryExpression(expr) { + // `right` should not be `ParenthesizedExpression` + visits.push(`${expr.type}: ${expr.operator} (right: ${expr.right.type})`); + }, + Literal(lit) { + visits.push(`${lit.type}: ${lit.value}`); + }, + TSTypeAliasDeclaration(decl) { + // `typeAnnotation` should not be `TSParenthesizedType` + visits.push(`${decl.type}: (typeAnnotation: ${decl.typeAnnotation.type})`); + }, + 'TSTypeAliasDeclaration:exit'(decl) { + // `typeAnnotation` should not be `TSParenthesizedType` + visits.push(`${decl.type}:exit: (typeAnnotation: ${decl.typeAnnotation.type})`); + }, + TSStringKeyword(keyword) { + visits.push(keyword.type); + }, + TSParenthesizedType(paren) { + // Should not be visited - no `TSParenthesizedType`s in AST in TS-ESLint + visits.push(paren.type); + }, + TSUnionType(union) { + // `types` should not be `TSParenthesizedType` + visits.push(`${union.type}: (types: ${union.types.map(t => t.type).join(', ')})`); + }, + 'TSUnionType:exit'(union) { + // `types` should not be `TSParenthesizedType` + visits.push(`${union.type}:exit: (types: ${union.types.map(t => t.type).join(', ')})`); + }, + TSNumberKeyword(keyword) { + visits.push(keyword.type); + }, + 'Program:exit'(program) { + visits.push(`${program.type}:exit`); + context.report({ + message: `Visited nodes:\n* ${visits.join('\n* ')}`, + node: program, + }); + }, + }; + }, + }, + }, +}; diff --git a/apps/oxlint/tsconfig.json b/apps/oxlint/tsconfig.json index f1a7450d93b5b..cdadd6c52599f 100644 --- a/apps/oxlint/tsconfig.json +++ b/apps/oxlint/tsconfig.json @@ -9,6 +9,7 @@ }, "exclude": [ "node_modules", - "fixtures" + "fixtures", + "test/fixtures" ] }