From 4d684483e7185c64c060eb489d6d5dba058592dd Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Mon, 19 Jan 2026 04:39:05 +0000 Subject: [PATCH] fix(linter/plugins): `report` accept out of range `column` indexes in `loc` (#18199) ESLint's `func-call-spacing` rule sometimes reports errors with `loc` containing `column: -1`. As ESLint doesn't complain, probably other plugins do the same too. Make the checks for validity of `loc` in `report` more relaxed, to support this behavior. --- apps/oxlint/conformance/snapshot.md | 404 +--------------------- apps/oxlint/src-js/package/rule_tester.ts | 20 +- apps/oxlint/src-js/plugins/location.ts | 2 +- apps/oxlint/src-js/plugins/report.ts | 84 ++++- 4 files changed, 104 insertions(+), 406 deletions(-) diff --git a/apps/oxlint/conformance/snapshot.md b/apps/oxlint/conformance/snapshot.md index 6a3f86b848282..b1dbca165a9ea 100644 --- a/apps/oxlint/conformance/snapshot.md +++ b/apps/oxlint/conformance/snapshot.md @@ -7,8 +7,8 @@ | Status | Count | % | | ----------------- | ----- | ------ | | Total rules | 292 | 100.0% | -| Fully passing | 256 | 87.7% | -| Partially passing | 36 | 12.3% | +| Fully passing | 257 | 88.0% | +| Partially passing | 35 | 12.0% | | Fully failing | 0 | 0.0% | | Load errors | 0 | 0.0% | | No tests run | 0 | 0.0% | @@ -18,8 +18,8 @@ | Status | Count | % | | ----------- | ----- | ------ | | Total tests | 33090 | 100.0% | -| Passing | 32703 | 98.8% | -| Failing | 280 | 0.8% | +| Passing | 32717 | 98.9% | +| Failing | 266 | 0.8% | | Skipped | 107 | 0.3% | ## Fully Passing Rules @@ -55,6 +55,7 @@ - `eol-last` (35 tests) - `eqeqeq` (68 tests) - `for-direction` (72 tests) +- `func-call-spacing` (151 tests) - `func-name-matching` (193 tests) - `func-names` (109 tests) - `func-style` (120 tests) @@ -284,7 +285,6 @@ ## Rules with Failures - `comma-dangle` - 265 / 267 (99.3%) -- `func-call-spacing` - 137 / 151 (90.7%) - `id-blacklist` - 128 / 131 (97.7%) - `id-denylist` - 140 / 143 (97.9%) - `no-eval` - 97 / 101 (96.0%) @@ -436,400 +436,6 @@ AssertionError [ERR_ASSERTION]: Should have 2 errors but had 1: [ at apps/oxlint/dist/index.js -### `func-call-spacing` - -Pass: 137 / 151 (90.7%) -Fail: 14 / 151 (9.3%) -Skip: 0 / 151 (0.0%) - -#### func-call-spacing > invalid - -```js -f -(); -``` - -```json -{ - "output": null, - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f (); -``` - -```json -{ - "output": null, - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f
(); -``` - -```json -{ - "output": null, - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f
(); -``` - -```json -{ - "output": null, - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f -(); -``` - -```json -{ - "output": null, - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -import -(source); -``` - -```json -{ - "output": null, - "languageOptions": { - "ecmaVersion": 2020 - }, - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f -(); -``` - -```json -{ - "output": null, - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpectedWhitespace", - "line": 1, - "column": 2, - "endLine": 2, - "endColumn": 0 - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -this.cancelled.add(request) -this.decrement(request) -(0, request.reject)(new api.Cancel()) -``` - -```json -{ - "output": null, - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpectedWhitespace", - "line": 2, - "column": 24, - "endLine": 3, - "endColumn": 0 - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -var a = foo -(function(global) {}(this)); -``` - -```json -{ - "output": null, - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpectedWhitespace", - "line": 1, - "column": 12, - "endLine": 2, - "endColumn": 0 - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -var a = foo -(0, baz()) -``` - -```json -{ - "output": null, - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpectedWhitespace", - "line": 1, - "column": 12, - "endColumn": 0, - "endLine": 2 - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f (); -``` - -```json -{ - "output": null, - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f
(); -``` - -```json -{ - "output": null, - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f
(); -``` - -```json -{ - "output": null, - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - -#### func-call-spacing > invalid - -```js -f -(); -``` - -```json -{ - "output": null, - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpectedWhitespace" - } - ] -} -``` - -RangeError: Invalid column number (column -1 requested). - at getOffsetFromLineColumn (apps/oxlint/dist/lint.js) - at report (apps/oxlint/dist/lint.js) - at Object.report (apps/oxlint/dist/lint.js) - at checkSpacing (apps/oxlint/conformance/submodules/eslint/lib/rules/func-call-spacing.js:146:13) - - ### `id-blacklist` Pass: 128 / 131 (97.7%) diff --git a/apps/oxlint/src-js/package/rule_tester.ts b/apps/oxlint/src-js/package/rule_tester.ts index 8a633aacf4018..d3da5493c3c19 100644 --- a/apps/oxlint/src-js/package/rule_tester.ts +++ b/apps/oxlint/src-js/package/rule_tester.ts @@ -994,8 +994,24 @@ function lint(test: TestCase, plugin: Plugin): Diagnostic[] { const ruleId = `${plugin.meta!.name!}/${Object.keys(plugin.rules)[0]}`; return diagnostics.map((diagnostic) => { - const { line, column } = getLineColumnFromOffset(diagnostic.start), - { line: endLine, column: endColumn } = getLineColumnFromOffset(diagnostic.end); + let line, column, endLine, endColumn; + + // Convert start/end offsets to line/column. + // In conformance build, use original `loc` if one was passed to `report`. + if (!CONFORMANCE || diagnostic.loc == null) { + ({ line, column } = getLineColumnFromOffset(diagnostic.start)); + ({ line: endLine, column: endColumn } = getLineColumnFromOffset(diagnostic.end)); + } else { + const { loc } = diagnostic; + ({ line, column } = loc.start); + if (loc.end != null) { + ({ line: endLine, column: endColumn } = loc.end); + } else { + endLine = line; + endColumn = column; + } + } + const node = getNodeByRangeIndex(diagnostic.start); return { ruleId, diff --git a/apps/oxlint/src-js/plugins/location.ts b/apps/oxlint/src-js/plugins/location.ts index 80f67dba4a986..0f46700f0d254 100644 --- a/apps/oxlint/src-js/plugins/location.ts +++ b/apps/oxlint/src-js/plugins/location.ts @@ -107,7 +107,7 @@ export function initLines(): void { * Debug assert that `lines` and `lineStartIndices` are initialized. * No-op in release build - TSDown will remove this function and all calls to it. */ -function debugAssertLinesIsInitialized(): void { +export function debugAssertLinesIsInitialized(): void { debugAssert(lines.length > 0, "`lines` should be initialized"); debugAssert( lines.length === lineStartIndices.length, diff --git a/apps/oxlint/src-js/plugins/report.ts b/apps/oxlint/src-js/plugins/report.ts index 3666d822c2e0f..e71c95621a503 100644 --- a/apps/oxlint/src-js/plugins/report.ts +++ b/apps/oxlint/src-js/plugins/report.ts @@ -4,8 +4,9 @@ import { filePath } from "./context.ts"; import { getFixes } from "./fix.ts"; -import { getOffsetFromLineColumn } from "./location.ts"; -import { typeAssertIs } from "../utils/asserts.ts"; +import { initLines, lines, lineStartIndices, debugAssertLinesIsInitialized } from "./location.ts"; +import { sourceText } from "./source_code.ts"; +import { debugAssertIsNonNull, typeAssertIs } from "../utils/asserts.ts"; import type { RequireAtLeastOne } from "type-fest"; import type { Fix, FixFn } from "./fix.ts"; @@ -71,6 +72,8 @@ export interface DiagnosticReport { ruleIndex: number; fixes: Fix[] | null; messageId: string | null; + // Only used in conformance tests + loc?: LocationWithOptionalEnd | null; } // Diagnostics array. Reused for every file. @@ -100,6 +103,8 @@ export function report(diagnostic: Diagnostic, ruleDetails: RuleDetails): void { // TODO: Validate `diagnostic` let start: number, end: number, loc: LocationWithOptionalEnd | LineColumn | undefined; + // We need the original location in conformance tests + let conformedLoc: LocationWithOptionalEnd | null = null; if (Object.hasOwn(diagnostic, "loc") && (loc = diagnostic.loc) != null) { // `loc` @@ -113,12 +118,28 @@ export function report(diagnostic: Diagnostic, ruleDetails: RuleDetails): void { if (Object.hasOwn(loc, "start")) { typeAssertIs(loc); - start = getOffsetFromLineColumn(loc.start); - end = loc.end == null ? start : getOffsetFromLineColumn(loc.end); + const { start: startLineCol, end: endLineCol } = loc; + + if (startLineCol === null || typeof startLineCol !== "object") { + throw new TypeError("`loc.start` must be an object"); + } + start = getOffsetFromLineColumn(startLineCol); + + if (endLineCol == null) { + end = start; + } else if (typeof endLineCol === "object") { + end = getOffsetFromLineColumn(endLineCol); + } else { + throw new TypeError("`loc.end` must be an object or null/undefined"); + } + + if (CONFORMANCE) conformedLoc = loc; } else { typeAssertIs(loc); start = getOffsetFromLineColumn(loc); end = start; + + if (CONFORMANCE) conformedLoc = { start: loc, end: null }; } } else { // `node` @@ -159,6 +180,9 @@ export function report(diagnostic: Diagnostic, ruleDetails: RuleDetails): void { ruleIndex: ruleDetails.ruleIndex, fixes: getFixes(diagnostic, ruleDetails), }); + + // We need the original location in conformance tests + if (CONFORMANCE) diagnostics.at(-1)!.loc = conformedLoc; } /** @@ -232,3 +256,55 @@ export function replacePlaceholders(message: string, data: DiagnosticData): stri return value !== undefined ? (value as string) : match; }); } + +/** + * Convert a `{ line, column }` pair into a range index. + * + * Same as `getOffsetFromLineColumn` in `location.ts`, except: + * 1. Does not check that `lineCol` is an object - caller must do that. + * 2. Allows `column` to be less than 0 or greater than the length of the line. + * + * Relaxing the restriction on `column` is required because some rules (e.g. ESLint's `func-call-spacing`) + * produce `column: -1` in some cases. + * + * @param lineCol - A line/column location. + * @returns The character index of the location in the file. + * @throws {TypeError} If `lineCol` is not an object with a integer `line` and `column`. + * @throws {RangeError} If `line` is less than or equal to 0, or greater than the number of lines in the source text. + * @throws {RangeError} If computed offset is out of range of the source text. + */ +function getOffsetFromLineColumn(lineCol: LineColumn): number { + const { line, column } = lineCol; + if ( + typeof line !== "number" || + typeof column !== "number" || + (line | 0) !== line || + (column | 0) !== column + ) { + throw new TypeError("Expected an object with integer `line` and `column` properties"); + } + + // Build `lines` and `lineStartIndices` tables if they haven't been already. + // This also decodes `sourceText` if it wasn't already. + if (lines.length === 0) initLines(); + debugAssertIsNonNull(sourceText); + debugAssertLinesIsInitialized(); + + if (line <= 0 || line > lineStartIndices.length) { + throw new RangeError( + `Line number out of range (line ${line} requested). ` + + `Line numbers should be 1-based, and less than or equal to number of lines in file (${lineStartIndices.length}).`, + ); + } + + const lineOffset = lineStartIndices[line - 1]; + const offset = lineOffset + column; + + // Ensure offset is within bounds. + // Do this here on JS side to prevent a NAPI error when converting to `u32` on Rust side. + if (offset < 0 || offset > sourceText.length) { + throw new RangeError("Line/column pair translates to an out of range offset"); + } + + return offset; +}