diff --git a/apps/oxlint/conformance/snapshots/eslint.md b/apps/oxlint/conformance/snapshots/eslint.md index 55c838ceee267..cc47246c9ceb8 100644 --- a/apps/oxlint/conformance/snapshots/eslint.md +++ b/apps/oxlint/conformance/snapshots/eslint.md @@ -7,8 +7,8 @@ | Status | Count | % | | ----------------- | ----- | ------ | | Total rules | 292 | 100.0% | -| Fully passing | 286 | 97.9% | -| Partially passing | 6 | 2.1% | +| Fully passing | 287 | 98.3% | +| Partially passing | 5 | 1.7% | | Fully failing | 0 | 0.0% | | Load errors | 0 | 0.0% | | No tests run | 0 | 0.0% | @@ -18,8 +18,8 @@ | Status | Count | % | | ----------- | ----- | ------ | | Total tests | 33159 | 100.0% | -| Passing | 32869 | 99.1% | -| Failing | 12 | 0.0% | +| Passing | 32871 | 99.1% | +| Failing | 10 | 0.0% | | Skipped | 278 | 0.8% | ## Fully Passing Rules @@ -303,6 +303,7 @@ - `symbol-description` (8 tests) - `template-curly-spacing` (57 tests) - `template-tag-spacing` (63 tests) +- `unicode-bom` (7 tests) - `use-isnan` (214 tests) - `valid-typeof` (54 tests) - `vars-on-top` (61 tests) @@ -318,7 +319,6 @@ - `no-unused-labels` - 23 / 26 (88.5%) - `object-curly-newline` - 143 / 144 (99.3%) - `space-in-parens` - 137 / 139 (98.6%) -- `unicode-bom` - 5 / 7 (71.4%) ## Rules with Failures Detail @@ -771,64 +771,3 @@ AssertionError [ERR_ASSERTION]: Output is incorrect at apps/oxlint/dist/plugins-dev.js at it (apps/oxlint/conformance/src/capture.ts:123:5) - -### `unicode-bom` - -Pass: 5 / 7 (71.4%) -Fail: 2 / 7 (28.6%) -Skip: 0 / 7 (0.0%) - -#### unicode-bom > invalid - -```js - var a = 123; -``` - -```json -{ - "output": " var a = 123;", - "errors": [ - { - "messageId": "unexpected", - "line": 1, - "column": 1 - } - ] -} -``` - -Error: Failed to apply fixes - at assertInvalidTestCasePasses (apps/oxlint/dist/plugins-dev.js) - at runInvalidTestCase (apps/oxlint/dist/plugins-dev.js) - at apps/oxlint/dist/plugins-dev.js - at it (apps/oxlint/conformance/src/capture.ts:123:5) - - -#### unicode-bom > invalid - -```js - var a = 123; -``` - -```json -{ - "output": " var a = 123;", - "options": [ - "never" - ], - "errors": [ - { - "messageId": "unexpected", - "line": 1, - "column": 1 - } - ] -} -``` - -Error: Failed to apply fixes - at assertInvalidTestCasePasses (apps/oxlint/dist/plugins-dev.js) - at runInvalidTestCase (apps/oxlint/dist/plugins-dev.js) - at apps/oxlint/dist/plugins-dev.js - at it (apps/oxlint/conformance/src/capture.ts:123:5) - diff --git a/apps/oxlint/src-js/bindings.d.ts b/apps/oxlint/src-js/bindings.d.ts index 6a0fae10f29a7..7b607c4e2316a 100644 --- a/apps/oxlint/src-js/bindings.d.ts +++ b/apps/oxlint/src-js/bindings.d.ts @@ -34,6 +34,9 @@ export declare const enum Severity { * one group per diagnostic which provides fixes. * Each inner array should have length of 1 at minimum. * + * If source text starts with a BOM, `JSFix`es must have offsets relative to the start + * of the source text *without* the BOM. + * * Each group's fixes are merged, then all merged fixes are applied to `source_text`. * * Fix ranges are converted from UTF-16 code units to UTF-8 bytes. diff --git a/apps/oxlint/src-js/plugins/fix.ts b/apps/oxlint/src-js/plugins/fix.ts index 794620a1f60d7..b68d3ca1db44f 100644 --- a/apps/oxlint/src-js/plugins/fix.ts +++ b/apps/oxlint/src-js/plugins/fix.ts @@ -18,6 +18,12 @@ export type FixFn = ( /** * Fix, as returned by `fix` function. + * + * `range` offsets are relative to start of the source text. + * When the file has a BOM, they are relative to the start of the source text *without* the BOM. + * + * To represent a position *before* a BOM, -1 is used to mean "before the BOM". + * ESLint's `unicode-bom` rule produces a fix `{ range: [-1, 0], text: "" }` to remove a BOM. */ export interface Fix { range: Range; @@ -26,10 +32,19 @@ export interface Fix { /** * Fix, in form sent to Rust. + * + * Offsets have 1 added to them, so that -1 in `Fix`'s `range` is represented as 0 in `FixReport`. + * This means that both `startPlusOne` and `endPlusOne` are valid `u32`s, even if original range elements were -1. + * + * `Fix { range: [0, 10], text: "xxx" }` -> `FixReport { startPlusOne: 1, endPlusOne: 11, text: "xxx" }`. + * `Fix { range: [-1, 0], text: "" }` -> `FixReport { startPlusOne: 0, endPlusOne: 1, text: "" }`. + * + * Range offsets can be -1 when the file has a BOM, and the fix starts before the BOM. + * ESLint's `unicode-bom` rule produces a fix `{ range: [-1, 0], text: "" }` to remove a BOM. */ export interface FixReport { - start: number; - end: number; + startPlusOne: number; + endPlusOne: number; text: string; } @@ -223,8 +238,11 @@ function validateAndConvertFix(fix: Fix): FixReport { const start = range[0], end = range[1]; if (typeof start === "number" && typeof end === "number") { - // Converting `text` to string follows ESLint, which does that implicitly - return { start, end, text: String(text) }; + // Add 1 to `start` and `end`, to allow original offsets to be -1 (meaning "before the BOM"), + // and `FixReport`'s `startPlusOne` and `endPlusOne` are still valid `u32`s on Rust side. + // + // Converting `text` to string follows ESLint, which does that implicitly. + return { startPlusOne: start + 1, endPlusOne: end + 1, text: String(text) }; } } diff --git a/apps/oxlint/src/js_plugins/fix.rs b/apps/oxlint/src/js_plugins/fix.rs index 4c50e10f8691b..0b9c257b6b219 100644 --- a/apps/oxlint/src/js_plugins/fix.rs +++ b/apps/oxlint/src/js_plugins/fix.rs @@ -4,6 +4,10 @@ use oxc_ast_visit::utf8_to_utf16::Utf8ToUtf16; use oxc_diagnostics::OxcDiagnostic; use oxc_linter::{Fixer, JsFix, Message, PossibleFixes, convert_and_merge_js_fixes}; +const BOM: &str = "\u{feff}"; +#[expect(clippy::cast_possible_truncation)] +const BOM_LEN: u32 = BOM.len() as u32; + /// Apply fixes to source text and return the fixed code. /// /// - `source_text` is the original source code. @@ -11,6 +15,9 @@ use oxc_linter::{Fixer, JsFix, Message, PossibleFixes, convert_and_merge_js_fixe /// one group per diagnostic which provides fixes. /// Each inner array should have length of 1 at minimum. /// +/// If source text starts with a BOM, `JSFix`es must have offsets relative to the start +/// of the source text *without* the BOM. +/// /// Each group's fixes are merged, then all merged fixes are applied to `source_text`. /// /// Fix ranges are converted from UTF-16 code units to UTF-8 bytes. @@ -20,14 +27,20 @@ pub fn apply_fixes(source_text: String, fixes_json: String) -> Option { // Deserialize fixes JSON let fix_groups: Vec> = serde_json::from_str(&fixes_json).ok()?; - // Create `Utf8ToUtf16` converter - let span_converter = Utf8ToUtf16::new(&source_text); + // Create `Utf8ToUtf16` converter. + // If file has a BOM, trim it off start of the source text before creating the converter. + let has_bom = source_text.starts_with(BOM); + let span_converter = if has_bom { + Utf8ToUtf16::new_with_offset(&source_text[BOM_LEN as usize..], BOM_LEN) + } else { + Utf8ToUtf16::new(&source_text) + }; // Merge fix groups into a single fix per group let messages = fix_groups .into_iter() .map(|group| { - convert_and_merge_js_fixes(group, &source_text, &span_converter) + convert_and_merge_js_fixes(group, &source_text, &span_converter, has_bom) .ok() .map(|fix| Message::new(OxcDiagnostic::error(""), PossibleFixes::Single(fix))) }) diff --git a/apps/oxlint/test/fixtures/fixes/files/bom_remove.js b/apps/oxlint/test/fixtures/fixes/files/bom_remove.js new file mode 100644 index 0000000000000..f3fb123651c4f --- /dev/null +++ b/apps/oxlint/test/fixtures/fixes/files/bom_remove.js @@ -0,0 +1,2 @@ +a = c; +d = b \ No newline at end of file diff --git a/apps/oxlint/test/fixtures/fixes/files/bom_remove2.js b/apps/oxlint/test/fixtures/fixes/files/bom_remove2.js new file mode 100644 index 0000000000000..f3fb123651c4f --- /dev/null +++ b/apps/oxlint/test/fixtures/fixes/files/bom_remove2.js @@ -0,0 +1,2 @@ +a = c; +d = b \ No newline at end of file diff --git a/apps/oxlint/test/fixtures/fixes/fix.snap.md b/apps/oxlint/test/fixtures/fixes/fix.snap.md index 820dcd5fd3be3..ec71dc8d94e5f 100644 --- a/apps/oxlint/test/fixtures/fixes/fix.snap.md +++ b/apps/oxlint/test/fixtures/fixes/fix.snap.md @@ -5,7 +5,7 @@ ``` x Error running JS plugin. | File path: /files/range_end_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 116 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 129 x Plugin `fixes-plugin/fixes` returned invalid fixes. | File path: /files/range_end_out_of_bounds.js @@ -17,7 +17,7 @@ x Error running JS plugin. | File path: /files/range_end_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 124 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 138 x Plugin `fixes-plugin/fixes` returned invalid fixes. | File path: /files/range_start_after_end.js @@ -29,11 +29,11 @@ x Error running JS plugin. | File path: /files/range_start_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 110 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 116 x Error running JS plugin. | File path: /files/range_start_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 118 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 125 x fixes-plugin(fixes): end out of bounds ,-[files/range_end_out_of_bounds.js:1:5] @@ -60,7 +60,7 @@ `---- Found 0 warnings and 12 errors. -Finished in Xms on 10 files with 1 rules using X threads. +Finished in Xms on 12 files with 1 rules using X threads. ``` # stderr @@ -80,6 +80,18 @@ rage = abacus rage = abacus ``` +# File altered: files/bom_remove.js +``` +daddy = magic; +damned = abacus +``` + +# File altered: files/bom_remove2.js +``` +daddy = magic; +damned = abacus +``` + # File altered: files/index.js ``` diff --git a/apps/oxlint/test/fixtures/fixes/output.snap.md b/apps/oxlint/test/fixtures/fixes/output.snap.md index fb2b0498fe975..15a4b86054a7d 100644 --- a/apps/oxlint/test/fixtures/fixes/output.snap.md +++ b/apps/oxlint/test/fixtures/fixes/output.snap.md @@ -5,7 +5,7 @@ ``` x Error running JS plugin. | File path: /files/range_end_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 116 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 129 x Plugin `fixes-plugin/fixes` returned invalid fixes. | File path: /files/range_end_out_of_bounds.js @@ -17,7 +17,7 @@ x Error running JS plugin. | File path: /files/range_end_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 124 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 138 x Plugin `fixes-plugin/fixes` returned invalid fixes. | File path: /files/range_start_after_end.js @@ -29,11 +29,11 @@ x Error running JS plugin. | File path: /files/range_start_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 110 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 116 x Error running JS plugin. | File path: /files/range_start_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 118 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 125 x fixes-plugin(fixes): Replace "a" with "daddy" ,-[files/bom.js:1:4] @@ -91,6 +91,74 @@ : ^ `---- + x fixes-plugin(fixes): Replace "a" with "daddy" + ,-[files/bom_remove.js:1:4] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x fixes-plugin(fixes): Remove BOM + ,-[files/bom_remove.js:1:4] + 1 | ,-> a = c; + 2 | `-> d = b + `---- + + x fixes-plugin(fixes): Prefix "c" with "magi" + ,-[files/bom_remove.js:1:8] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x fixes-plugin(fixes): Prefix "d" with "damne" + ,-[files/bom_remove.js:2:1] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x fixes-plugin(fixes): Replace "b" with "abacus" + ,-[files/bom_remove.js:2:5] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x fixes-plugin(fixes): Replace "a" with "daddy" + ,-[files/bom_remove2.js:1:4] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x fixes-plugin(fixes): Remove BOM multiple + ,-[files/bom_remove2.js:1:4] + 1 | ,-> a = c; + 2 | `-> d = b + `---- + + x fixes-plugin(fixes): Prefix "c" with "magi" + ,-[files/bom_remove2.js:1:8] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x fixes-plugin(fixes): Prefix "d" with "damne" + ,-[files/bom_remove2.js:2:1] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x fixes-plugin(fixes): Replace "b" with "abacus" + ,-[files/bom_remove2.js:2:5] + 1 | a = c; + 2 | d = b + : ^ + `---- + x fixes-plugin(fixes): Remove debugger statement ,-[files/index.js:1:1] 1 | debugger; @@ -237,8 +305,8 @@ : ^ `---- -Found 0 warnings and 36 errors. -Finished in Xms on 10 files with 1 rules using X threads. +Found 0 warnings and 46 errors. +Finished in Xms on 12 files with 1 rules using X threads. ``` # stderr diff --git a/apps/oxlint/test/fixtures/fixes/plugin.ts b/apps/oxlint/test/fixtures/fixes/plugin.ts index 5b76a454f3999..44b7c0fc2dfe4 100644 --- a/apps/oxlint/test/fixtures/fixes/plugin.ts +++ b/apps/oxlint/test/fixtures/fixes/plugin.ts @@ -21,6 +21,29 @@ const rule: Rule = { let debuggerCount = 0; return { + Program(node) { + // Remove BOM in `bom_remove.js` and `bom_remove2.js`. + // ESLint's `unicode-bom` rule returns fixes with `range: [-1, 0]` to remove BOM. + // Check behavior is correct for both `fix` returning a single fix and returning multiple fixes. + if (filename === "bom_remove.js") { + context.report({ + message: "Remove BOM", + node, + fix(fixer) { + return fixer.removeRange([-1, 0]); + }, + }); + } else if (filename === "bom_remove2.js") { + context.report({ + message: "Remove BOM multiple", + node, + fix(fixer) { + return [fixer.removeRange([0, 0]), fixer.removeRange([-1, 0])]; + }, + }); + } + }, + DebuggerStatement(node) { debuggerCount++; diff --git a/apps/oxlint/test/fixtures/suggestions/files/bom_remove.js b/apps/oxlint/test/fixtures/suggestions/files/bom_remove.js new file mode 100644 index 0000000000000..f3fb123651c4f --- /dev/null +++ b/apps/oxlint/test/fixtures/suggestions/files/bom_remove.js @@ -0,0 +1,2 @@ +a = c; +d = b \ No newline at end of file diff --git a/apps/oxlint/test/fixtures/suggestions/files/bom_remove2.js b/apps/oxlint/test/fixtures/suggestions/files/bom_remove2.js new file mode 100644 index 0000000000000..f3fb123651c4f --- /dev/null +++ b/apps/oxlint/test/fixtures/suggestions/files/bom_remove2.js @@ -0,0 +1,2 @@ +a = c; +d = b \ No newline at end of file diff --git a/apps/oxlint/test/fixtures/suggestions/fix-suggestions.snap.md b/apps/oxlint/test/fixtures/suggestions/fix-suggestions.snap.md index 02e45c555bfc6..aa24810647708 100644 --- a/apps/oxlint/test/fixtures/suggestions/fix-suggestions.snap.md +++ b/apps/oxlint/test/fixtures/suggestions/fix-suggestions.snap.md @@ -5,7 +5,7 @@ ``` x Error running JS plugin. | File path: /files/range_end_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 163 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 176 x Plugin `suggestions-plugin/suggestions` returned invalid suggestions. | File path: /files/range_end_out_of_bounds.js @@ -17,7 +17,7 @@ x Error running JS plugin. | File path: /files/range_end_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 171 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 185 x Plugin `suggestions-plugin/suggestions` returned invalid suggestions. | File path: /files/range_start_after_end.js @@ -29,11 +29,11 @@ x Error running JS plugin. | File path: /files/range_start_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 157 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 163 x Error running JS plugin. | File path: /files/range_start_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 165 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 172 x suggestions-plugin(suggestions): end out of bounds ,-[files/range_end_out_of_bounds.js:1:5] @@ -60,7 +60,7 @@ `---- Found 0 warnings and 12 errors. -Finished in Xms on 10 files with 1 rules using X threads. +Finished in Xms on 12 files with 1 rules using X threads. ``` # stderr @@ -80,6 +80,18 @@ rage = abacus rage = abacus ``` +# File altered: files/bom_remove.js +``` +daddy = magic; +damned = abacus +``` + +# File altered: files/bom_remove2.js +``` +daddy = magic; +damned = abacus +``` + # File altered: files/index.js ``` diff --git a/apps/oxlint/test/fixtures/suggestions/fix.snap.md b/apps/oxlint/test/fixtures/suggestions/fix.snap.md index 2974fe4da3186..b55210a65ea06 100644 --- a/apps/oxlint/test/fixtures/suggestions/fix.snap.md +++ b/apps/oxlint/test/fixtures/suggestions/fix.snap.md @@ -5,19 +5,19 @@ ``` x Error running JS plugin. | File path: /files/range_end_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 163 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 176 x Error running JS plugin. | File path: /files/range_end_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 171 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 185 x Error running JS plugin. | File path: /files/range_start_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 157 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 163 x Error running JS plugin. | File path: /files/range_start_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 165 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 172 x suggestions-plugin(suggestions): Replace "a" with "daddy" ,-[files/bom.js:1:4] @@ -75,6 +75,74 @@ : ^ `---- + x suggestions-plugin(suggestions): Replace "a" with "daddy" + ,-[files/bom_remove.js:1:4] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x suggestions-plugin(suggestions): Remove BOM + ,-[files/bom_remove.js:1:4] + 1 | ,-> a = c; + 2 | `-> d = b + `---- + + x suggestions-plugin(suggestions): Prefix "c" with "magi" + ,-[files/bom_remove.js:1:8] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x suggestions-plugin(suggestions): Prefix "d" with "damne" + ,-[files/bom_remove.js:2:1] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x suggestions-plugin(suggestions): Replace "b" with "abacus" + ,-[files/bom_remove.js:2:5] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x suggestions-plugin(suggestions): Replace "a" with "daddy" + ,-[files/bom_remove2.js:1:4] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x suggestions-plugin(suggestions): Remove BOM multiple + ,-[files/bom_remove2.js:1:4] + 1 | ,-> a = c; + 2 | `-> d = b + `---- + + x suggestions-plugin(suggestions): Prefix "c" with "magi" + ,-[files/bom_remove2.js:1:8] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x suggestions-plugin(suggestions): Prefix "d" with "damne" + ,-[files/bom_remove2.js:2:1] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x suggestions-plugin(suggestions): Replace "b" with "abacus" + ,-[files/bom_remove2.js:2:5] + 1 | a = c; + 2 | d = b + : ^ + `---- + x suggestions-plugin(suggestions): Remove debugger statement ,-[files/index.js:1:1] 1 | debugger; @@ -229,8 +297,8 @@ : ^ `---- -Found 0 warnings and 33 errors. -Finished in Xms on 10 files with 1 rules using X threads. +Found 0 warnings and 43 errors. +Finished in Xms on 12 files with 1 rules using X threads. ``` # stderr diff --git a/apps/oxlint/test/fixtures/suggestions/output.snap.md b/apps/oxlint/test/fixtures/suggestions/output.snap.md index 2974fe4da3186..b55210a65ea06 100644 --- a/apps/oxlint/test/fixtures/suggestions/output.snap.md +++ b/apps/oxlint/test/fixtures/suggestions/output.snap.md @@ -5,19 +5,19 @@ ``` x Error running JS plugin. | File path: /files/range_end_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 163 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 176 x Error running JS plugin. | File path: /files/range_end_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 171 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 185 x Error running JS plugin. | File path: /files/range_start_negative.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-10`, expected u32 at line 1 column 157 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `-9`, expected u32 at line 1 column 163 x Error running JS plugin. | File path: /files/range_start_too_large.js - | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967296`, expected u32 at line 1 column 165 + | Failed to deserialize JSON returned by `lintFile`: invalid value: integer `4294967297`, expected u32 at line 1 column 172 x suggestions-plugin(suggestions): Replace "a" with "daddy" ,-[files/bom.js:1:4] @@ -75,6 +75,74 @@ : ^ `---- + x suggestions-plugin(suggestions): Replace "a" with "daddy" + ,-[files/bom_remove.js:1:4] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x suggestions-plugin(suggestions): Remove BOM + ,-[files/bom_remove.js:1:4] + 1 | ,-> a = c; + 2 | `-> d = b + `---- + + x suggestions-plugin(suggestions): Prefix "c" with "magi" + ,-[files/bom_remove.js:1:8] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x suggestions-plugin(suggestions): Prefix "d" with "damne" + ,-[files/bom_remove.js:2:1] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x suggestions-plugin(suggestions): Replace "b" with "abacus" + ,-[files/bom_remove.js:2:5] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x suggestions-plugin(suggestions): Replace "a" with "daddy" + ,-[files/bom_remove2.js:1:4] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x suggestions-plugin(suggestions): Remove BOM multiple + ,-[files/bom_remove2.js:1:4] + 1 | ,-> a = c; + 2 | `-> d = b + `---- + + x suggestions-plugin(suggestions): Prefix "c" with "magi" + ,-[files/bom_remove2.js:1:8] + 1 | a = c; + : ^ + 2 | d = b + `---- + + x suggestions-plugin(suggestions): Prefix "d" with "damne" + ,-[files/bom_remove2.js:2:1] + 1 | a = c; + 2 | d = b + : ^ + `---- + + x suggestions-plugin(suggestions): Replace "b" with "abacus" + ,-[files/bom_remove2.js:2:5] + 1 | a = c; + 2 | d = b + : ^ + `---- + x suggestions-plugin(suggestions): Remove debugger statement ,-[files/index.js:1:1] 1 | debugger; @@ -229,8 +297,8 @@ : ^ `---- -Found 0 warnings and 33 errors. -Finished in Xms on 10 files with 1 rules using X threads. +Found 0 warnings and 43 errors. +Finished in Xms on 12 files with 1 rules using X threads. ``` # stderr diff --git a/apps/oxlint/test/fixtures/suggestions/plugin.ts b/apps/oxlint/test/fixtures/suggestions/plugin.ts index 80b32360e61f0..bf7bb4495a694 100644 --- a/apps/oxlint/test/fixtures/suggestions/plugin.ts +++ b/apps/oxlint/test/fixtures/suggestions/plugin.ts @@ -21,6 +21,39 @@ const rule: Rule = { let debuggerCount = 0; return { + Program(node) { + // Remove BOM in `bom_remove.js` and `bom_remove2.js`. + // ESLint's `unicode-bom` rule returns fixes with `range: [-1, 0]` to remove BOM. + // Check behavior is correct for both `fix` returning a single fix and returning multiple fixes. + if (filename === "bom_remove.js") { + context.report({ + message: "Remove BOM", + node, + suggest: [ + { + desc: "Do it", + fix(fixer) { + return fixer.removeRange([-1, 0]); + }, + }, + ], + }); + } else if (filename === "bom_remove2.js") { + context.report({ + message: "Remove BOM multiple", + node, + suggest: [ + { + desc: "Do it", + fix(fixer) { + return [fixer.removeRange([0, 0]), fixer.removeRange([-1, 0])]; + }, + }, + ], + }); + } + }, + DebuggerStatement(node) { debuggerCount++; diff --git a/crates/oxc_linter/src/external_linter.rs b/crates/oxc_linter/src/external_linter.rs index 2b69d2976293e..7b881f2016b03 100644 --- a/crates/oxc_linter/src/external_linter.rs +++ b/crates/oxc_linter/src/external_linter.rs @@ -83,14 +83,24 @@ pub struct LintFileResult { pub suggestions: Option>, } +/// Fix in form sent from JS to Rust. +/// +/// Offsets have 1 added to them on JS side. +/// So an original range of `[0, 10]` becomes `JsFix { start_plus_one: 1, end_plus_one: 11, text: "..." }`. +/// +/// This allows offsets which were originally -1, and they can be stored in `u32`s. +/// +/// ESLint's `unicode-bom` rule produces a fix `{ range: [-1, 0], text: "" }` to remove a BOM. +/// This becomes `JsFix { start_plus_one: 0, end_plus_one: 1, text: "" }`. #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct JsFix { - pub start: u32, - pub end: u32, + pub start_plus_one: u32, + pub end_plus_one: u32, pub text: String, } +/// Suggestion in form sent from JS to Rust. #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct JsSuggestion { @@ -98,24 +108,42 @@ pub struct JsSuggestion { pub fixes: Vec, } +const BOM: &str = "\u{feff}"; +#[expect(clippy::cast_possible_truncation)] +const BOM_LEN: u32 = BOM.len() as u32; + /// Convert a `Vec` to a single [`Fix`], including converting spans from UTF-16 to UTF-8. pub fn convert_and_merge_js_fixes( fixes: Vec, source_text: &str, span_converter: &Utf8ToUtf16, + has_bom: bool, ) -> Result { // JS should send `None` instead of `Some([])` debug_assert!(!fixes.is_empty()); let is_single = fixes.len() == 1; + let mut illegal_bom_fix_span = None; let mut fixes = fixes.into_iter().map(|fix| { - let mut span = Span::new(fix.start, fix.end); + // `start_plus_one` and `end_plus_one` are original `start` and `end` + 1. + // If either is 0, it means the fix is before a BOM. + // This is a very rare case, so handle it in a `#[cold]` function. + if fix.start_plus_one == 0 || fix.end_plus_one == 0 { + return convert_bom_fix(fix, span_converter, has_bom, &mut illegal_bom_fix_span); + } + + // Convert span from UTF-16 to UTF-8. + // Deduct 1 from `start_plus_one` and `end_plus_one` to get original offsets. + let start = fix.start_plus_one - 1; + let end = fix.end_plus_one - 1; + let mut span = Span::new(start, end); span_converter.convert_span_back(&mut span); + Fix::new(fix.text, span) }); - if is_single { + let res = if is_single { #[expect(clippy::missing_panics_doc, reason = "infallible")] let fix = fixes.next().unwrap(); @@ -134,7 +162,63 @@ pub fn convert_and_merge_js_fixes( } } else { CompositeFix::merge_fixes_fallible(fixes.collect(), source_text) + }; + + // If any `JsFix` had `start_plus_one == 0` or `end_plus_one == 0`, but the file doesn't have a BOM, + // the fix is invalid. This is a very rare case, so handle it in a `#[cold]` function. + if let Some(span) = illegal_bom_fix_span { create_illegal_bom_error(span) } else { res } +} + +/// Convert `JsFix` to `Fix` where either `start_plus_one` or `end_plus_one` is 0. +/// This means that the fix starts before the BOM. +/// +/// Convert offsets from UTF-16 to UTF-8. +/// * If file has a BOM, adjust 0 offsets manually to be before the BOM. +/// * If file doesn't have a BOM, set `illegal_span` to the span of the fix (without the BOM-adjustment). +/// `convert_and_merge_js_fixes` will return an error in this case. +/// +/// This is a very rare case, so handling this is in this separate `#[cold]` function. +#[cold] +fn convert_bom_fix( + fix: JsFix, + span_converter: &Utf8ToUtf16, + has_bom: bool, + illegal_span: &mut Option, +) -> Fix { + // Convert span from UTF-16 to UTF-8. + // Perform conversion with original offsets (`start_plus_one - 1`, `end_plus_one - 1`). + // Offsets which are 0 are clamped to 0 for this initial conversion, so the UTF-8 offsets point to start of the file, + // *after* the BOM. + let start = fix.start_plus_one.saturating_sub(1); + let end = fix.end_plus_one.saturating_sub(1); + let mut span = Span::new(start, end); + + span_converter.convert_span_back(&mut span); + + if has_bom { + // Adjust offsets which were 0 to be before the BOM + if fix.start_plus_one == 0 { + span.start -= BOM_LEN; + } + if fix.end_plus_one == 0 { + span.end -= BOM_LEN; + } + } else { + // File doesn't have a BOM, so this is an invalid fix. + // Set `illegal_span`. `convert_and_merge_js_fixes` will return an error in this case. + *illegal_span = Some(span); } + + Fix::new(fix.text, span) +} + +/// Create an error for an invalid fix which had `start_plus_one` or `end_plus_one` of 0, +/// but in a file which doesn't have a BOM. +/// +/// This is a very rare case, so handling this is in this separate `#[cold]` function. +#[cold] +fn create_illegal_bom_error(span: Span) -> Result { + Err(MergeFixesError::InvalidRange(span.start, span.end)) } #[derive(Clone)] diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 57f1d0901dcc4..82230e17813d4 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -646,6 +646,7 @@ impl Linter { fixes, original_source_text, &span_converter, + has_bom, ) { Ok(fix) => Some(fix.with_kind(fix_kind)), Err(err) => {