From 5505a86121ba3c760752379524d15acfd8dcc734 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Sat, 4 Oct 2025 15:47:47 +0000 Subject: [PATCH] feat(linter/plugins): include `range` field in AST (#14321) Add `range` field to AST nodes, using the support for `range` in raw transfer added in #14319. Use `range` field rather than `start` + `end` in nodes passed to `context.report` and fixer methods - for closer alignment with ESLint. --- apps/oxlint/scripts/build.js | 2 +- apps/oxlint/src-js/index.ts | 3 +- apps/oxlint/src-js/plugins/context.ts | 13 ++- apps/oxlint/src-js/plugins/fix.ts | 12 ++- apps/oxlint/src-js/plugins/source_code.ts | 2 +- apps/oxlint/src-js/plugins/types.ts | 38 ++++---- .../fixtures/context_properties/plugin.ts | 4 +- .../oxlint/test/fixtures/createOnce/plugin.ts | 4 +- .../test/fixtures/definePlugin/plugin.ts | 6 +- .../definePlugin_and_defineRule/plugin.ts | 5 +- .../oxlint/test/fixtures/defineRule/plugin.ts | 5 +- .../test/fixtures/estree/output.snap.md | 88 ++++++++++++++++++- apps/oxlint/test/fixtures/estree/plugin.ts | 12 +++ apps/oxlint/test/fixtures/fixes/plugin.ts | 12 +-- .../oxlint/test/fixtures/sourceCode/plugin.ts | 4 +- .../fixtures/sourceCode_late_access/plugin.ts | 4 +- .../plugin.ts | 4 +- .../fixtures/utf16_offsets/output.snap.md | 27 +++++- .../test/fixtures/utf16_offsets/plugin.ts | 12 ++- 19 files changed, 205 insertions(+), 52 deletions(-) diff --git a/apps/oxlint/scripts/build.js b/apps/oxlint/scripts/build.js index 271b432164e7c..f6ab7d93b17f1 100755 --- a/apps/oxlint/scripts/build.js +++ b/apps/oxlint/scripts/build.js @@ -32,7 +32,7 @@ const parserFilePaths = [ 'generated/lazy/types.js', 'generated/lazy/walk.js', */ - 'generated/deserialize/ts.js', + 'generated/deserialize/ts_range.js', 'generated/visit/keys.js', 'generated/visit/types.js', 'generated/visit/visitor.d.ts', diff --git a/apps/oxlint/src-js/index.ts b/apps/oxlint/src-js/index.ts index 458921b5180df..c50130a5b8188 100644 --- a/apps/oxlint/src-js/index.ts +++ b/apps/oxlint/src-js/index.ts @@ -3,7 +3,7 @@ import type { CreateOnceRule, Plugin, Rule } from './plugins/load.ts'; import type { BeforeHook, Visitor, VisitorWithHooks } from './plugins/types.ts'; export type { Context, Diagnostic, DiagnosticBase, DiagnosticWithLoc, DiagnosticWithNode } from './plugins/context.ts'; -export type { Fix, Fixer, FixFn, Range } from './plugins/fix.ts'; +export type { Fix, Fixer, FixFn } from './plugins/fix.ts'; export type { CreateOnceRule, CreateRule, Plugin, Rule } from './plugins/load.ts'; export type { Definition, @@ -23,6 +23,7 @@ export type { Location, Node, NodeOrToken, + Range, RuleMeta, Token, Visitor, diff --git a/apps/oxlint/src-js/plugins/context.ts b/apps/oxlint/src-js/plugins/context.ts index c8461c646871c..a3737afd4cfa9 100644 --- a/apps/oxlint/src-js/plugins/context.ts +++ b/apps/oxlint/src-js/plugins/context.ts @@ -155,14 +155,23 @@ export class Context { const { node } = diagnostic as DiagnosticWithNode; if (node == null) throw new TypeError('Either `node` or `loc` is required'); if (typeof node !== 'object') throw new TypeError('`node` must be an object'); - ({ start, end } = node); + + // ESLint uses `loc` here instead of `range`. + // We can't do that because AST nodes don't have `loc` property yet. In any case, `range` is preferable, + // as otherwise we have to convert `loc` to `range` which is expensive at present. + // TODO: Revisit this once we have `loc` support in AST, and a fast translation table to convert `loc` to `range`. + const { range } = node; + if (range === null || typeof range !== 'object') throw new TypeError('`node.range` must be present'); + start = range[0]; + end = range[1]; + // Do type validation checks here, to ensure no error in serialization / deserialization. // Range validation happens on Rust side. if ( typeof start !== 'number' || typeof end !== 'number' || start < 0 || end < 0 || (start | 0) !== start || (end | 0) !== end ) { - throw new TypeError('`node.start` and `node.end` must be non-negative integers'); + throw new TypeError('`node.range[0]` and `node.range[1]` must be non-negative integers'); } } diff --git a/apps/oxlint/src-js/plugins/fix.ts b/apps/oxlint/src-js/plugins/fix.ts index bdded6355cf8d..6c9af21eef964 100644 --- a/apps/oxlint/src-js/plugins/fix.ts +++ b/apps/oxlint/src-js/plugins/fix.ts @@ -1,7 +1,7 @@ import { assertIs } from './utils.js'; import type { Diagnostic, InternalContext } from './context.ts'; -import type { NodeOrToken } from './types.ts'; +import type { NodeOrToken, Range } from './types.ts'; const { prototype: ArrayPrototype, from: ArrayFrom } = Array, { getPrototypeOf, hasOwn, prototype: ObjectPrototype } = Object, @@ -18,15 +18,13 @@ export type FixFn = ( // Type of a fix, as returned by `fix` function. export type Fix = { range: Range; text: string }; -export type Range = [number, number]; - // Fixer, passed as argument to `fix` function passed to `Context#report()`. // // Fixer is stateless, so reuse a single object for all fixes. // Freeze the object to prevent user mutating it. const FIXER = Object.freeze({ insertTextBefore(nodeOrToken: NodeOrToken, text: string): Fix { - const { start } = nodeOrToken; + const start = nodeOrToken.range[0]; return { range: [start, start], text }; }, insertTextBeforeRange(range: Range, text: string): Fix { @@ -34,7 +32,7 @@ const FIXER = Object.freeze({ return { range: [start, start], text }; }, insertTextAfter(nodeOrToken: NodeOrToken, text: string): Fix { - const { end } = nodeOrToken; + const end = nodeOrToken.range[1]; return { range: [end, end], text }; }, insertTextAfterRange(range: Range, text: string): Fix { @@ -42,13 +40,13 @@ const FIXER = Object.freeze({ return { range: [end, end], text }; }, remove(nodeOrToken: NodeOrToken): Fix { - return { range: [nodeOrToken.start, nodeOrToken.end], text: '' }; + return { range: nodeOrToken.range, text: '' }; }, removeRange(range: Range): Fix { return { range, text: '' }; }, replaceText(nodeOrToken: NodeOrToken, text: string): Fix { - return { range: [nodeOrToken.start, nodeOrToken.end], text }; + return { range: nodeOrToken.range, text }; }, replaceTextRange(range: Range, text: string): Fix { return { range, text }; diff --git a/apps/oxlint/src-js/plugins/source_code.ts b/apps/oxlint/src-js/plugins/source_code.ts index d16951c32d776..e71032792df6a 100644 --- a/apps/oxlint/src-js/plugins/source_code.ts +++ b/apps/oxlint/src-js/plugins/source_code.ts @@ -6,7 +6,7 @@ import { // @ts-expect-error } from '../generated/constants.js'; // @ts-expect-error we need to generate `.d.ts` file for this module -import { deserializeProgramOnly } from '../../dist/generated/deserialize/ts.js'; +import { deserializeProgramOnly } from '../../dist/generated/deserialize/ts_range.js'; import type { Program } from '@oxc-project/types'; import type { Scope, ScopeManager, Variable } from './scope.ts'; diff --git a/apps/oxlint/src-js/plugins/types.ts b/apps/oxlint/src-js/plugins/types.ts index 3d0a279f9e2cb..8ed49e7360605 100644 --- a/apps/oxlint/src-js/plugins/types.ts +++ b/apps/oxlint/src-js/plugins/types.ts @@ -25,11 +25,32 @@ export interface VisitorWithHooks extends Visitor { // Visit function for a specific AST node type. export type VisitFn = (node: Node) => void; -// Internal interface for any type which has `start` and `end` properties. -// We'll add `range` and `loc` properties to this later. +// Internal interface for any type which has location properties. interface Spanned { start: number; end: number; + // This property should not be optional - all AST nodes do have a `range` field. + // But ESTree types have `range` field as optional, so to allow AST nodes to be passed + // to methods which take `Node`, we have to make it optional here too. + // TODO: Fix this + range?: Range; + loc?: Location; +} + +// Range of source offsets. +export type Range = [number, number]; + +// Source code location. +export interface Location { + start: LineColumn; + end: LineColumn; +} + +// Line number + column number pair. +// `line` is 1-indexed, `column` is 0-indexed. +export interface LineColumn { + line: number; + column: number; } // AST node type. @@ -50,19 +71,6 @@ export interface Comment extends Spanned { value: string; } -// Source code location. -export interface Location { - start: LineColumn; - end: LineColumn; -} - -// Line number + column number pair. -// `line` is 1-indexed, `column` is 0-indexed. -export interface LineColumn { - line: number; - column: number; -} - // Element of compiled visitor array. // * `VisitFn | null` for leaf nodes. // * `EnterExit | null` for non-leaf nodes. diff --git a/apps/oxlint/test/fixtures/context_properties/plugin.ts b/apps/oxlint/test/fixtures/context_properties/plugin.ts index 022584b4d2ab2..feddd7dcf3eee 100644 --- a/apps/oxlint/test/fixtures/context_properties/plugin.ts +++ b/apps/oxlint/test/fixtures/context_properties/plugin.ts @@ -1,8 +1,8 @@ import { sep } from 'node:path'; -import type { Plugin, Rule } from '../../../dist/index.js'; +import type { Node, Plugin, Rule } from '../../../dist/index.js'; -const SPAN = { start: 0, end: 0 }; +const SPAN: Node = { start: 0, end: 0, range: [0, 0] }; const DIR_PATH_LEN = import.meta.dirname.length + 1; diff --git a/apps/oxlint/test/fixtures/createOnce/plugin.ts b/apps/oxlint/test/fixtures/createOnce/plugin.ts index 9ca8b6a7aa43f..a855c53f4ea08 100644 --- a/apps/oxlint/test/fixtures/createOnce/plugin.ts +++ b/apps/oxlint/test/fixtures/createOnce/plugin.ts @@ -1,8 +1,8 @@ import { sep } from 'node:path'; -import type { Plugin, Rule } from '../../../dist/index.js'; +import type { Node, Plugin, Rule } from '../../../dist/index.js'; -const SPAN = { start: 0, end: 0 }; +const SPAN: Node = { start: 0, end: 0, range: [0, 0] }; const DIR_PATH_LEN = import.meta.dirname.length + 1; diff --git a/apps/oxlint/test/fixtures/definePlugin/plugin.ts b/apps/oxlint/test/fixtures/definePlugin/plugin.ts index 8fa63efefe9af..b8d92de77a37a 100644 --- a/apps/oxlint/test/fixtures/definePlugin/plugin.ts +++ b/apps/oxlint/test/fixtures/definePlugin/plugin.ts @@ -1,11 +1,13 @@ import { sep } from 'node:path'; import { definePlugin } from '../../../dist/index.js'; -import type { Rule } from '../../../dist/index.js'; + +import type { Node, Rule } from '../../../dist/index.js'; // `loc` is required for ESLint -const SPAN = { +const SPAN: Node = { start: 0, end: 0, + range: [0, 0], loc: { start: { line: 0, column: 0 }, end: { line: 0, column: 0 }, diff --git a/apps/oxlint/test/fixtures/definePlugin_and_defineRule/plugin.ts b/apps/oxlint/test/fixtures/definePlugin_and_defineRule/plugin.ts index 312824e2697ff..1aaa346d6723c 100644 --- a/apps/oxlint/test/fixtures/definePlugin_and_defineRule/plugin.ts +++ b/apps/oxlint/test/fixtures/definePlugin_and_defineRule/plugin.ts @@ -1,10 +1,13 @@ import { sep } from 'node:path'; import { definePlugin, defineRule } from '../../../dist/index.js'; +import type { Node } from '../../../dist/index.js'; + // `loc` is required for ESLint -const SPAN = { +const SPAN: Node = { start: 0, end: 0, + range: [0, 0], loc: { start: { line: 0, column: 0 }, end: { line: 0, column: 0 }, diff --git a/apps/oxlint/test/fixtures/defineRule/plugin.ts b/apps/oxlint/test/fixtures/defineRule/plugin.ts index c1a41e5b52c56..f559efedf29b7 100644 --- a/apps/oxlint/test/fixtures/defineRule/plugin.ts +++ b/apps/oxlint/test/fixtures/defineRule/plugin.ts @@ -1,10 +1,13 @@ import { sep } from 'node:path'; import { defineRule } from '../../../dist/index.js'; +import type { Node } from '../../../dist/index.js'; + // `loc` is required for ESLint -const SPAN = { +const SPAN: Node = { start: 0, end: 0, + range: [0, 0], loc: { start: { line: 0, column: 0 }, end: { line: 0, column: 0 }, diff --git a/apps/oxlint/test/fixtures/estree/output.snap.md b/apps/oxlint/test/fixtures/estree/output.snap.md index 3be8b92eac657..e015720d7e793 100644 --- a/apps/oxlint/test/fixtures/estree/output.snap.md +++ b/apps/oxlint/test/fixtures/estree/output.snap.md @@ -47,7 +47,93 @@ 14 | `-> type U = (((((string)) | ((number))))); `---- -Found 0 warnings and 1 error. + x estree-check(check): program: + | start/end: [59,265] + | range: [59,265] + ,-[files/index.ts:5:1] + 4 | // All `Identifier`s + 5 | ,-> let a = { x: y }; + 6 | | + 7 | | // No `ParenthesizedExpression`s in AST + 8 | | const b = (x * ((('str' + ((123)))))); + 9 | | + 10 | | // TS syntax + 11 | | type T = string; + 12 | | + 13 | | // No `TSParenthesizedType`s in AST + 14 | `-> type U = (((((string)) | ((number))))); + `---- + + x estree-check(check): ident "a": + | start/end: [63,64] + | range: [63,64] + ,-[files/index.ts:5:5] + 4 | // All `Identifier`s + 5 | let a = { x: y }; + : ^ + 6 | + `---- + + x estree-check(check): ident "x": + | start/end: [69,70] + | range: [69,70] + ,-[files/index.ts:5:11] + 4 | // All `Identifier`s + 5 | let a = { x: y }; + : ^ + 6 | + `---- + + x estree-check(check): ident "y": + | start/end: [72,73] + | range: [72,73] + ,-[files/index.ts:5:14] + 4 | // All `Identifier`s + 5 | let a = { x: y }; + : ^ + 6 | + `---- + + x estree-check(check): ident "b": + | start/end: [124,125] + | range: [124,125] + ,-[files/index.ts:8:7] + 7 | // No `ParenthesizedExpression`s in AST + 8 | const b = (x * ((('str' + ((123)))))); + : ^ + 9 | + `---- + + x estree-check(check): ident "x": + | start/end: [129,130] + | range: [129,130] + ,-[files/index.ts:8:12] + 7 | // No `ParenthesizedExpression`s in AST + 8 | const b = (x * ((('str' + ((123)))))); + : ^ + 9 | + `---- + + x estree-check(check): ident "T": + | start/end: [176,177] + | range: [176,177] + ,-[files/index.ts:11:6] + 10 | // TS syntax + 11 | type T = string; + : ^ + 12 | + `---- + + x estree-check(check): ident "U": + | start/end: [230,231] + | range: [230,231] + ,-[files/index.ts:14:6] + 13 | // No `TSParenthesizedType`s in AST + 14 | type U = (((((string)) | ((number))))); + : ^ + `---- + +Found 0 warnings and 9 errors. Finished in Xms on 1 file using X threads. ``` diff --git a/apps/oxlint/test/fixtures/estree/plugin.ts b/apps/oxlint/test/fixtures/estree/plugin.ts index f8c3281b75a93..1748622af1f0e 100644 --- a/apps/oxlint/test/fixtures/estree/plugin.ts +++ b/apps/oxlint/test/fixtures/estree/plugin.ts @@ -13,6 +13,12 @@ const plugin: Plugin = { const visits: string[] = []; return { Program(program) { + context.report({ + message: 'program:\n' + + `start/end: [${program.start},${program.end}]\n` + + `range: [${program.range}]`, + node: program, + }); visits.push(program.type); }, VariableDeclaration(decl) { @@ -26,6 +32,12 @@ const plugin: Plugin = { visits.push(`${decl.type}: (init: ${decl.init.type})`); }, Identifier(ident) { + context.report({ + message: `ident "${ident.name}":\n` + + `start/end: [${ident.start},${ident.end}]\n` + + `range: [${ident.range}]`, + node: ident, + }); visits.push(`${ident.type}: ${ident.name}`); }, ObjectExpression(expr) { diff --git a/apps/oxlint/test/fixtures/fixes/plugin.ts b/apps/oxlint/test/fixtures/fixes/plugin.ts index 07c7a4d9172f7..733e31cf39184 100644 --- a/apps/oxlint/test/fixtures/fixes/plugin.ts +++ b/apps/oxlint/test/fixtures/fixes/plugin.ts @@ -1,4 +1,4 @@ -import type { Diagnostic, Plugin, Range } from '../../../dist/index.js'; +import type { Diagnostic, Node, Plugin } from '../../../dist/index.js'; const plugin: Plugin = { meta: { @@ -87,7 +87,8 @@ const plugin: Plugin = { // Fixes can be in any order return [ fixer.insertTextAfter(node, 'ty'), - fixer.replaceText(node, 'mp'), + // Test that any object with `range` property works + fixer.replaceText({ range: [node.start, node.end] } as Node, 'mp'), fixer.insertTextBefore(node, 'nu'), ]; }, @@ -98,7 +99,7 @@ const plugin: Plugin = { node, fix(fixer) { // Fixes can be in any order - const range: Range = [node.start, node.end]; + const { range } = node; return [ fixer.replaceTextRange(range, 'er'), fixer.insertTextAfterRange(range, 'mouse'), @@ -114,7 +115,8 @@ const plugin: Plugin = { *fix(fixer) { yield fixer.insertTextBefore(node, 'gra'); yield fixer.replaceText(node, 'nu'); - yield fixer.insertTextAfter(node, 'lar'); + // Test that any object with `range` property works + yield fixer.insertTextAfter({ range: [node.start, node.end] } as Node, 'lar'); }, }); case 'j': @@ -124,7 +126,7 @@ const plugin: Plugin = { // `fix` can be a generator function *fix(fixer) { // Fixes can be in any order - const range: Range = [node.start, node.end]; + const { range } = node; yield fixer.insertTextAfterRange(range, 'bunga'); yield fixer.replaceTextRange(range, 'a'); yield fixer.insertTextBeforeRange(range, 'cow'); diff --git a/apps/oxlint/test/fixtures/sourceCode/plugin.ts b/apps/oxlint/test/fixtures/sourceCode/plugin.ts index 56d48872e3d2f..4e4ca206f3af0 100644 --- a/apps/oxlint/test/fixtures/sourceCode/plugin.ts +++ b/apps/oxlint/test/fixtures/sourceCode/plugin.ts @@ -1,9 +1,9 @@ import assert from 'node:assert'; import type { Program } from '@oxc-project/types'; -import type { Plugin, Rule } from '../../../dist/index.js'; +import type { Node, Plugin, Rule } from '../../../dist/index.js'; -const SPAN = { start: 0, end: 0 }; +const SPAN: Node = { start: 0, end: 0, range: [0, 0] }; const createRule: Rule = { create(context) { diff --git a/apps/oxlint/test/fixtures/sourceCode_late_access/plugin.ts b/apps/oxlint/test/fixtures/sourceCode_late_access/plugin.ts index 69bc6ae16006f..3a77c8f85f3d1 100644 --- a/apps/oxlint/test/fixtures/sourceCode_late_access/plugin.ts +++ b/apps/oxlint/test/fixtures/sourceCode_late_access/plugin.ts @@ -1,9 +1,9 @@ import assert from 'node:assert'; import type { Program } from '@oxc-project/types'; -import type { Plugin, Rule } from '../../../dist/index.js'; +import type { Node, Plugin, Rule } from '../../../dist/index.js'; -const SPAN = { start: 0, end: 0 }; +const SPAN: Node = { start: 0, end: 0, range: [0, 0] }; // Purpose of this test fixture is to ensure that AST is not deserialized twice // if `context.sourceCode.ast` is accessed during AST traversal. diff --git a/apps/oxlint/test/fixtures/sourceCode_late_access_after_only/plugin.ts b/apps/oxlint/test/fixtures/sourceCode_late_access_after_only/plugin.ts index 118c539a55285..d6748071a6dee 100644 --- a/apps/oxlint/test/fixtures/sourceCode_late_access_after_only/plugin.ts +++ b/apps/oxlint/test/fixtures/sourceCode_late_access_after_only/plugin.ts @@ -1,6 +1,6 @@ -import type { Plugin, Rule } from '../../../dist/index.js'; +import type { Node, Plugin, Rule } from '../../../dist/index.js'; -const SPAN = { start: 0, end: 0 }; +const SPAN: Node = { start: 0, end: 0, range: [0, 0] }; // Purpose of this test fixture is to ensure that source text and AST are available in `after` hook // via `context.sourceCode` when the AST is not traversed diff --git a/apps/oxlint/test/fixtures/utf16_offsets/output.snap.md b/apps/oxlint/test/fixtures/utf16_offsets/output.snap.md index e38e1bc48f7dc..6a268312cbba4 100644 --- a/apps/oxlint/test/fixtures/utf16_offsets/output.snap.md +++ b/apps/oxlint/test/fixtures/utf16_offsets/output.snap.md @@ -11,13 +11,28 @@ `---- help: Remove the debugger statement - x utf16-plugin(no-debugger): Debugger at 0-9 + x utf16-plugin(no-debugger): debugger: + | start/end: [0,9] + | range: [0,9] ,-[files/index.js:1:1] 1 | debugger; : ^^^^^^^^^ 2 | // £ `---- + x utf16-plugin(no-debugger): program: + | start/end: [0,47] + | range: [0,47] + ,-[files/index.js:1:1] + 1 | ,-> debugger; + 2 | | // £ + 3 | | debugger; + 4 | | // 🤨 + 5 | | { + 6 | | debugger; + 7 | `-> } + `---- + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\eslint(no-debugger)]8;;\: `debugger` statement is not allowed ,-[files/index.js:3:1] 2 | // £ @@ -27,7 +42,9 @@ `---- help: Remove the debugger statement - x utf16-plugin(no-debugger): Debugger at 15-24 + x utf16-plugin(no-debugger): debugger: + | start/end: [15,24] + | range: [15,24] ,-[files/index.js:3:1] 2 | // £ 3 | debugger; @@ -44,7 +61,9 @@ `---- help: Remove the debugger statement - x utf16-plugin(no-debugger): Debugger at 35-44 + x utf16-plugin(no-debugger): debugger: + | start/end: [35,44] + | range: [35,44] ,-[files/index.js:6:3] 5 | { 6 | debugger; @@ -52,7 +71,7 @@ 7 | } `---- -Found 3 warnings and 3 errors. +Found 3 warnings and 4 errors. Finished in Xms on 1 file using X threads. ``` diff --git a/apps/oxlint/test/fixtures/utf16_offsets/plugin.ts b/apps/oxlint/test/fixtures/utf16_offsets/plugin.ts index c064988b3e7c1..3f14fa28e8b35 100644 --- a/apps/oxlint/test/fixtures/utf16_offsets/plugin.ts +++ b/apps/oxlint/test/fixtures/utf16_offsets/plugin.ts @@ -8,9 +8,19 @@ const plugin: Plugin = { 'no-debugger': { create(context) { return { + Program(program) { + context.report({ + message: 'program:\n' + + `start/end: [${program.start},${program.end}]\n` + + `range: [${program.range}]`, + node: program, + }); + }, DebuggerStatement(debuggerStatement) { context.report({ - message: `Debugger at ${debuggerStatement.start}-${debuggerStatement.end}`, + message: 'debugger:\n' + + `start/end: [${debuggerStatement.start},${debuggerStatement.end}]\n` + + `range: [${debuggerStatement.range}]`, node: debuggerStatement, }); },