diff --git a/apps/oxlint/src-js/plugins/context.ts b/apps/oxlint/src-js/plugins/context.ts index b65392fff86c7..d8e186d4e6df2 100644 --- a/apps/oxlint/src-js/plugins/context.ts +++ b/apps/oxlint/src-js/plugins/context.ts @@ -1,13 +1,17 @@ +import { getFixes } from './fix.js'; + +import type { Fix, FixFn } from './fix.ts'; import type { RuleMeta } from './types.ts'; // Diagnostic in form passed by user to `Context#report()` -interface Diagnostic { +export interface Diagnostic { message: string; node: { start: number; end: number; [key: string]: unknown; }; + fix?: FixFn; } // Diagnostic in form sent to Rust @@ -16,6 +20,7 @@ interface DiagnosticReport { start: number; end: number; ruleIndex: number; + fixes: Fix[] | null; } // Diagnostics array. Reused for every file. @@ -57,7 +62,7 @@ let getInternal: (context: Context, actionDescription: string) => InternalContex // Internal data within `Context` that don't want to expose to plugins. // Stored as `#internal` property of `Context`. -interface InternalContext { +export interface InternalContext { // Full rule name, including plugin name e.g. `my-plugin/my-rule`. id: string; // Index into `ruleIds` sent from Rust @@ -120,14 +125,16 @@ export class Context { * @param diagnostic - Diagnostic object */ report(diagnostic: Diagnostic): void { - const { ruleIndex } = getInternal(this, 'report errors'); + const internal = getInternal(this, 'report errors'); + // TODO: Validate `diagnostic` const { node } = diagnostic; diagnostics.push({ message: diagnostic.message, start: node.start, end: node.end, - ruleIndex, + ruleIndex: internal.ruleIndex, + fixes: getFixes(diagnostic, internal), }); } diff --git a/apps/oxlint/src-js/plugins/fix.ts b/apps/oxlint/src-js/plugins/fix.ts new file mode 100644 index 0000000000000..822db43c41d2c --- /dev/null +++ b/apps/oxlint/src-js/plugins/fix.ts @@ -0,0 +1,185 @@ +import { assertIs } from './utils.js'; + +import type { Diagnostic, InternalContext } from './context.ts'; +import type { Node } from './types.js'; + +const { prototype: ArrayPrototype, from: ArrayFrom } = Array, + { getPrototypeOf, hasOwn, prototype: ObjectPrototype } = Object, + { ownKeys } = Reflect, + IteratorSymbol = Symbol.iterator; + +// Type of `fix` function. +// `fix` can return a single fix, an array of fixes, or any iterator that yields fixes. +// e.g. `(function*() { yield fix1; yield fix2; })()` +export type FixFn = (fixer: typeof FIXER) => Fix | Array | IterableIterator | null; + +// Type of a fix, as returned by `fix` function. +export type Fix = { range: Range; text: string }; + +type Range = [number, number]; + +// Currently we only support `Node`s, but will add support for `Token`s later +type NodeOrToken = Node; + +// 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; + return { range: [start, start], text }; + }, + insertTextBeforeRange(range: Range, text: string): Fix { + const start = range[0]; + return { range: [start, start], text }; + }, + insertTextAfter(nodeOrToken: NodeOrToken, text: string): Fix { + const { end } = nodeOrToken; + return { range: [end, end], text }; + }, + insertTextAfterRange(range: Range, text: string): Fix { + const end = range[1]; + return { range: [end, end], text }; + }, + remove(nodeOrToken: NodeOrToken): Fix { + return { range: [nodeOrToken.start, nodeOrToken.end], text: '' }; + }, + removeRange(range: Range): Fix { + return { range, text: '' }; + }, + replaceText(nodeOrToken: NodeOrToken, text: string): Fix { + return { range: [nodeOrToken.start, nodeOrToken.end], text }; + }, + replaceTextRange(range: Range, text: string): Fix { + return { range, text }; + }, +}); + +/** + * Get fixes from a `Diagnostic`. + * + * Returns `null` if any of: + * + * 1. No `fix` function was provided in `diagnostic`. + * 2. `fix` function returns a falsy value. + * 3. `fix` function returns an empty array/iterator. + * 4. `fix` function returns an array/iterator containing only falsy values. + * + * Otherwise, returns a non-empty array of `Fix` objects. + * + * `Fix` objects are validated and conformed to expected shape. + * Does not mutate the `fixes` array returned by `fix` function, but avoids cloning if possible. + * + * This function aims to replicate ESLint's behavior as closely as possible. + * + * TODO: Are prototype checks, and checks for `toJSON` methods excessive? + * We're not handling all possible edge cases e.g. `fixes` or individual `Fix` objects being `Proxy`s or objects + * with getters. As we're not managing to be 100% bulletproof anyway, maybe we don't need to be quite so defensive. + * + * @param diagnostic - Diagnostic object + * @param internal - Internal context object + * @returns Non-empty array of `Fix` objects, or `null` if none + * @throws {Error} If rule is not marked as fixable but `fix` function returns fixes, + * or if `fix` function returns any invalid `Fix` objects + */ +export function getFixes(diagnostic: Diagnostic, internal: InternalContext): Fix[] | null { + // ESLint silently ignores non-function `fix` values, so we do the same + const { fix } = diagnostic; + if (typeof fix !== 'function') return null; + + // In ESLint, `fix` is called with `this` as a clone of the `diagnostic` object. + // We just use the original `diagnostic` object - that should be close enough. + let fixes = fix.call(diagnostic, FIXER); + + // ESLint ignores falsy values + if (!fixes) return null; + + // `fixes` can be any iterator, not just an array e.g. `fix: function*() { yield fix1; yield fix2; }` + if (IteratorSymbol in fixes) { + let isCloned = false; + + // Check prototype instead of using `Array.isArray()`, to ensure it is a native `Array`, + // not a subclass which may have overridden `toJSON()` in a way which could make `JSON.stringify()` throw + if (getPrototypeOf(fixes) !== ArrayPrototype || hasOwn(fixes, 'toJSON')) { + fixes = ArrayFrom(fixes as IterableIterator); + isCloned = true; + } + assertIs>(fixes); + + const fixesLen = fixes.length; + if (fixesLen === 0) return null; + + for (let i = 0; i < fixesLen; i++) { + const fix = fixes[i]; + + // ESLint ignores falsy values. + // Filter them out. This branch can only be taken once. + if (!fix) { + fixes = fixes.filter(Boolean); + if (fixes.length === 0) return null; + isCloned = true; + i--; + continue; + } + + const conformedFix = validateAndConformFix(fix); + if (conformedFix !== fix) { + // Don't mutate `fixes` array + if (isCloned === false) { + fixes = fixes.slice(); + isCloned = true; + } + fixes[i] = conformedFix; + } + } + } else { + fixes = [validateAndConformFix(fixes)]; + } + + // ESLint does not throw this error if `fix` function returns only falsy values. + // We've already exited if that is the case, so we're reproducing that behavior. + if (internal.meta.fixable === null) { + throw new Error('Fixable rules must set the `meta.fixable` property to "code" or "whitespace".'); + } + + return fixes; +} + +/** + * Validate that a `Fix` object is well-formed, and conform it to expected shape. + * + * - Convert `text` to string if needed. + * - Shorten `range` to 2 elements if it has extra elements. + * - Remove any additional properties on the object. + * + * Purpose is to ensure any input which ESLint accepts does not cause an error in `JSON.stringify()`, + * or in deserializing on Rust side. + * + * @param fix - Fix object to validate, possibly malformed + * @returns `Fix` object + */ +function validateAndConformFix(fix: unknown): Fix { + assertIs(fix); + let { range, text } = fix; + + // These checks follow ESLint, which throws if `range` is missing or invalid + if (!range || typeof range[0] !== 'number' || typeof range[1] !== 'number') { + throw new Error(`Fix has invalid range: ${JSON.stringify(fix, null, 2)}`); + } + + // If `fix` is already well-formed, return it as-is. + // Note: `ownKeys(fix).length === 2` rules out `fix` having a custom `toJSON` method. + const fixPrototype = getPrototypeOf(fix); + if ( + (fixPrototype === ObjectPrototype || fixPrototype === null) && ownKeys(fix).length === 2 && + getPrototypeOf(range) === ArrayPrototype && !hasOwn(range, 'toJSON') && range.length === 2 && + typeof text === 'string' + ) { + return fix; + } + + // Conform fix object to expected shape. + // Converting `text` to string follows ESLint, which does that implicitly. + return { range: [range[0], range[1]], text: String(text) }; +} diff --git a/apps/oxlint/src-js/plugins/load.ts b/apps/oxlint/src-js/plugins/load.ts index 175c312357aec..e9e9dc6b05ada 100644 --- a/apps/oxlint/src-js/plugins/load.ts +++ b/apps/oxlint/src-js/plugins/load.ts @@ -69,7 +69,9 @@ interface PluginDetails { } // Default rule metadata, used if `rule.meta` property is empty. -const emptyRuleMeta: RuleMeta = {}; +const emptyRuleMeta: RuleMeta = { + fixable: null, +}; /** * Load a plugin. @@ -125,8 +127,15 @@ async function loadPluginImpl(path: string): Promise { ruleMeta = emptyRuleMeta; } else { if (typeof ruleMeta !== 'object') throw new TypeError('Invalid `meta`'); - // TODO: Validate and conform individual properties of `meta` once they're supported - ruleMeta = emptyRuleMeta; + + let { fixable } = ruleMeta; + if (fixable === void 0) { + fixable = null; + } else if (fixable !== 'code' && fixable !== 'whitespace') { + throw new TypeError('Invalid `meta.fixable`'); + } + + ruleMeta = { fixable }; } // Create `Context` object for rule. This will be re-used for every file. diff --git a/apps/oxlint/src-js/plugins/types.ts b/apps/oxlint/src-js/plugins/types.ts index b752c27490f73..55287d5383fd0 100644 --- a/apps/oxlint/src-js/plugins/types.ts +++ b/apps/oxlint/src-js/plugins/types.ts @@ -43,5 +43,6 @@ export interface EnterExit { // Rule metadata. // TODO: Fill in all properties. export interface RuleMeta { + fixable?: 'code' | 'whitespace'; [key: string]: unknown; } diff --git a/apps/oxlint/test/__snapshots__/e2e.test.ts.snap b/apps/oxlint/test/__snapshots__/e2e.test.ts.snap index eff84f7b69720..d93e666b45d37 100644 --- a/apps/oxlint/test/__snapshots__/e2e.test.ts.snap +++ b/apps/oxlint/test/__snapshots__/e2e.test.ts.snap @@ -1,5 +1,28 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`oxlint CLI > should apply fixes when \`--fix\` is enabled 1`] = ` +"Found 0 warnings and 0 errors. +Finished in Xms on 1 file using X threads." +`; + +exports[`oxlint CLI > should apply fixes when \`--fix\` is enabled 2`] = ` +" + +let daddy = 1; +let abacus = 2; +let magic = 3; +let damned = 4; +let elephant = 5; +let feck = 6; +let numpty = 7; +let dangermouse = 8; +let granular = 9; +let cowabunga = 10; + + +" +`; + exports[`oxlint CLI > should have UTF-16 spans in AST 1`] = ` " ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed @@ -444,6 +467,106 @@ Found 1 warning and 6 errors. Finished in Xms on 1 file using X threads." `; +exports[`oxlint CLI > should not apply fixes when \`--fix\` is disabled 1`] = ` +" + x fixes-plugin(fixes): Remove debugger statement + ,-[index.js:1:1] + 1 | debugger; + : ^^^^^^^^^ + 2 | + \`---- + + x fixes-plugin(fixes): Replace "a" with "daddy" + ,-[index.js:3:5] + 2 | + 3 | let a = 1; + : ^ + 4 | let b = 2; + \`---- + + x fixes-plugin(fixes): Replace "b" with "abacus" + ,-[index.js:4:5] + 3 | let a = 1; + 4 | let b = 2; + : ^ + 5 | let c = 3; + \`---- + + x fixes-plugin(fixes): Prefix "c" with "magi" + ,-[index.js:5:5] + 4 | let b = 2; + 5 | let c = 3; + : ^ + 6 | let d = 4; + \`---- + + x fixes-plugin(fixes): Prefix "d" with "damne" + ,-[index.js:6:5] + 5 | let c = 3; + 6 | let d = 4; + : ^ + 7 | let e = 5; + \`---- + + x fixes-plugin(fixes): Postfix "e" with "lephant" + ,-[index.js:7:5] + 6 | let d = 4; + 7 | let e = 5; + : ^ + 8 | let f = 6; + \`---- + + x fixes-plugin(fixes): Postfix "f" with "eck" + ,-[index.js:8:5] + 7 | let e = 5; + 8 | let f = 6; + : ^ + 9 | let g = 7; + \`---- + + x fixes-plugin(fixes): Replace "g" with "numpty" + ,-[index.js:9:5] + 8 | let f = 6; + 9 | let g = 7; + : ^ + 10 | let h = 8; + \`---- + + x fixes-plugin(fixes): Replace "h" with "dangermouse" + ,-[index.js:10:5] + 9 | let g = 7; + 10 | let h = 8; + : ^ + 11 | let i = 9; + \`---- + + x fixes-plugin(fixes): Replace "i" with "granular" + ,-[index.js:11:5] + 10 | let h = 8; + 11 | let i = 9; + : ^ + 12 | let j = 10; + \`---- + + x fixes-plugin(fixes): Replace "j" with "cowabunga" + ,-[index.js:12:5] + 11 | let i = 9; + 12 | let j = 10; + : ^ + 13 | + \`---- + + x fixes-plugin(fixes): Remove debugger statement + ,-[index.js:14:1] + 13 | + 14 | debugger; + : ^^^^^^^^^ + \`---- + +Found 0 warnings and 12 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: diff --git a/apps/oxlint/test/e2e.test.ts b/apps/oxlint/test/e2e.test.ts index fa01fb6929ef7..58a8b2f4d4d74 100644 --- a/apps/oxlint/test/e2e.test.ts +++ b/apps/oxlint/test/e2e.test.ts @@ -1,3 +1,4 @@ +import fs from 'node:fs'; import { dirname, join as pathJoin } from 'node:path'; import { describe, expect, it } from 'vitest'; @@ -20,6 +21,10 @@ async function runOxlint(cwd: string, args: string[] = []) { return await runOxlintWithoutPlugins(cwd, ['--js-plugins', ...args]); } +async function runOxlintWithFixes(cwd: string, args: string[] = []) { + return await runOxlintWithoutPlugins(cwd, ['--js-plugins', '--fix', ...args]); +} + function normalizeOutput(output: string): string { let lines = output.split('\n'); @@ -183,4 +188,41 @@ describe('oxlint CLI', () => { expect(exitCode).toBe(1); expect(normalizeOutput(stdout)).toMatchSnapshot(); }); + + it('should not apply fixes when `--fix` is disabled', async () => { + const fixtureFilePath = pathJoin(PACKAGE_ROOT_PATH, 'test/fixtures/fixes/index.js'); + const codeBefore = fs.readFileSync(fixtureFilePath, 'utf8'); + + let error = true; + try { + const { stdout, exitCode } = await runOxlint('test/fixtures/fixes'); + expect(exitCode).toBe(1); + expect(normalizeOutput(stdout)).toMatchSnapshot(); + error = false; + } finally { + const codeAfter = fs.readFileSync(fixtureFilePath, 'utf8'); + if (codeAfter !== codeBefore) { + fs.writeFileSync(fixtureFilePath, codeBefore); + // oxlint-disable-next-line no-unsafe-finally + if (!error) throw new Error('Test modified fixture file'); + } + } + }); + + it('should apply fixes when `--fix` is enabled', async () => { + const fixtureFilePath = pathJoin(PACKAGE_ROOT_PATH, 'test/fixtures/fixes/index.js'); + const codeBefore = fs.readFileSync(fixtureFilePath, 'utf8'); + + let error = true; + try { + const { stdout, exitCode } = await runOxlintWithFixes('test/fixtures/fixes'); + expect(exitCode).toBe(0); + expect(normalizeOutput(stdout)).toMatchSnapshot(); + error = false; + } finally { + const codeAfter = fs.readFileSync(fixtureFilePath, 'utf8'); + fs.writeFileSync(fixtureFilePath, codeBefore); + if (!error) expect(codeAfter).toMatchSnapshot(); + } + }); }); diff --git a/apps/oxlint/test/fixtures/fixes/.oxlintrc.json b/apps/oxlint/test/fixtures/fixes/.oxlintrc.json new file mode 100644 index 0000000000000..f770602e470b9 --- /dev/null +++ b/apps/oxlint/test/fixtures/fixes/.oxlintrc.json @@ -0,0 +1,10 @@ +{ + "plugins": ["./test_plugin"], + "categories": { + "correctness": "off" + }, + "rules": { + "fixes-plugin/fixes": "error" + }, + "ignorePatterns": ["test_plugin/**"] +} diff --git a/apps/oxlint/test/fixtures/fixes/index.js b/apps/oxlint/test/fixtures/fixes/index.js new file mode 100644 index 0000000000000..2a6f6bfaeb243 --- /dev/null +++ b/apps/oxlint/test/fixtures/fixes/index.js @@ -0,0 +1,14 @@ +debugger; + +let a = 1; +let b = 2; +let c = 3; +let d = 4; +let e = 5; +let f = 6; +let g = 7; +let h = 8; +let i = 9; +let j = 10; + +debugger; diff --git a/apps/oxlint/test/fixtures/fixes/test_plugin/index.js b/apps/oxlint/test/fixtures/fixes/test_plugin/index.js new file mode 100644 index 0000000000000..f62dd5d48e5bc --- /dev/null +++ b/apps/oxlint/test/fixtures/fixes/test_plugin/index.js @@ -0,0 +1,137 @@ +export default { + meta: { + name: "fixes-plugin", + }, + rules: { + "fixes": { + meta: { + fixable: "code", + }, + create(context) { + let debuggerCount = 0; + return { + DebuggerStatement(node) { + debuggerCount++; + + let thisIsReport; + const report = { + message: "Remove debugger statement", + node, + fix(fixer) { + thisIsReport = this === report; + if (debuggerCount === 1) return fixer.remove(node); + return fixer.removeRange([node.start, node.end]); + }, + }; + context.report(report); + + if (!thisIsReport) context.report({ message: `this in fix function is not report object`, node }); + }, + Identifier(node) { + switch (node.name) { + case "a": + return context.report({ + message: 'Replace "a" with "daddy"', + node, + fix(fixer) { + return fixer.replaceText(node, "daddy"); + }, + }); + case "b": + return context.report({ + message: 'Replace "b" with "abacus"', + node, + fix(fixer) { + return fixer.replaceTextRange([node.start, node.end], "abacus"); + }, + }); + case "c": + return context.report({ + message: 'Prefix "c" with "magi"', + node, + fix(fixer) { + return fixer.insertTextBefore(node, "magi"); + }, + }); + case "d": + return context.report({ + message: 'Prefix "d" with "damne"', + node, + fix(fixer) { + return fixer.insertTextBeforeRange([node.start, node.end], "damne"); + }, + }); + case "e": + return context.report({ + message: 'Postfix "e" with "lephant"', + node, + fix(fixer) { + return fixer.insertTextAfter(node, "lephant"); + }, + }); + case "f": + return context.report({ + message: 'Postfix "f" with "eck"', + node, + fix(fixer) { + return fixer.insertTextAfterRange([node.start, node.end], "eck"); + }, + }); + case "g": + return context.report({ + message: 'Replace "g" with "numpty"', + node, + fix(fixer) { + // Fixes can be in any order + return [ + fixer.insertTextAfter(node, "ty"), + fixer.replaceText(node, "mp"), + fixer.insertTextBefore(node, "nu"), + ]; + }, + }); + case "h": + return context.report({ + message: 'Replace "h" with "dangermouse"', + node, + fix(fixer) { + // Fixes can be in any order + const range = [node.start, node.end]; + return [ + fixer.replaceTextRange(range, "er"), + fixer.insertTextAfterRange(range, "mouse"), + fixer.insertTextBeforeRange(range, "dang"), + ]; + }, + }); + case "i": + return context.report({ + message: 'Replace "i" with "granular"', + node, + // `fix` can be a generator function + *fix(fixer) { + yield fixer.insertTextBefore(node, "gra"); + yield fixer.replaceText(node, "nu"); + yield fixer.insertTextAfter(node, "lar"); + }, + }); + case "j": + return context.report({ + message: 'Replace "j" with "cowabunga"', + node, + // `fix` can be a generator function + *fix(fixer) { + // Fixes can be in any order + const range = [node.start, node.end]; + yield fixer.insertTextAfterRange(range, "bunga"); + yield fixer.replaceTextRange(range, "a"); + yield fixer.insertTextBeforeRange(range, "cow"); + }, + }); + } + }, + }; + }, + }, + }, +}; diff --git a/crates/oxc_linter/src/external_linter.rs b/crates/oxc_linter/src/external_linter.rs index 1338e9794b67d..e64de38447166 100644 --- a/crates/oxc_linter/src/external_linter.rs +++ b/crates/oxc_linter/src/external_linter.rs @@ -31,6 +31,14 @@ pub struct LintFileResult { pub message: String, pub start: u32, pub end: u32, + pub fixes: Option>, +} + +#[derive(Clone, Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct JsFix { + pub range: [u32; 2], + pub text: String, } #[derive(Clone)] diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index a057ad40ce710..eb23e076946f2 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -55,8 +55,8 @@ pub use crate::{ }, context::{ContextSubHost, LintContext}, external_linter::{ - ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, LintFileResult, - PluginLoadResult, + ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, JsFix, + LintFileResult, PluginLoadResult, }, external_plugin_store::{ExternalPluginStore, ExternalRuleId}, fixer::FixKind, @@ -73,7 +73,7 @@ pub use crate::{ use crate::{ config::{LintConfig, OxlintEnv, OxlintGlobals, OxlintSettings}, context::ContextHost, - fixer::{Fixer, Message, PossibleFixes}, + fixer::{CompositeFix, Fix, Fixer, Message, PossibleFixes}, rules::RuleEnum, utils::iter_possible_jest_call_node, }; @@ -342,7 +342,7 @@ impl Linter { // `external_linter` always exists when `external_rules` is not empty let external_linter = self.external_linter.as_ref().unwrap(); - let (program_offset, span_converter) = { + let (program_offset, source_text, span_converter) = { // Extract `Semantic` from `ContextHost`, and get a mutable reference to `Program`. // // It's not possible to obtain a `&mut Program` while `Semantic` exists, because `Semantic` @@ -370,7 +370,7 @@ impl Linter { let semantic = mem::take(ctx_host.semantic_mut()); let program_addr = NonNull::from(semantic.nodes().program()).addr(); let mut program_ptr = - allocator.data_end_ptr().cast::().with_addr(program_addr); + allocator.data_end_ptr().cast::>().with_addr(program_addr); drop(semantic); // SAFETY: Now that we've dropped `Semantic`, no references to any AST nodes remain, // so can get a mutable reference to `Program` without aliasing violations. @@ -383,7 +383,7 @@ impl Linter { // Get offset of `Program` within buffer (bottom 32 bits of pointer) let program_offset = ptr::from_ref(program) as u32; - (program_offset, span_converter) + (program_offset, program.source_text, span_converter) }; // Write offset of `Program` in metadata at end of buffer @@ -402,7 +402,10 @@ impl Linter { match result { Ok(diagnostics) => { for diagnostic in diagnostics { - // Convert UTF-16 offsets back to UTF-8 + // Convert UTF-16 offsets back to UTF-8. + // TODO: Validate span offsets are within bounds and `start <= end`. + // Also make sure offsets do not fall in middle of a multi-byte UTF-8 character. + // That's possible if UTF-16 offset points to middle of a surrogate pair. let mut span = Span::new(diagnostic.start, diagnostic.end); span_converter.convert_span_back(&mut span); @@ -418,12 +421,48 @@ impl Linter { continue; } + // Convert `JSFix`s fixes to `PossibleFixes`, including converting spans back to UTF-8 + let fix = if let Some(fixes) = diagnostic.fixes { + debug_assert!(!fixes.is_empty()); // JS should send `None` instead of `Some([])` + + let is_single = fixes.len() == 1; + + let fixes = fixes.into_iter().map(|fix| { + // TODO: Validate span offsets are within bounds and `start <= end`. + // Also make sure offsets do not fall in middle of a multi-byte UTF-8 character. + // That's possible if UTF-16 offset points to middle of a surrogate pair. + let mut span = Span::new(fix.range[0], fix.range[1]); + span_converter.convert_span_back(&mut span); + Fix::new(fix.text, span) + }); + + if is_single { + PossibleFixes::Single(fixes.into_iter().next().unwrap()) + } else { + let fixes = fixes.collect::>(); + match CompositeFix::merge_fixes_fallible(fixes, source_text) { + Ok(fix) => PossibleFixes::Single(fix), + Err(err) => { + ctx_host.push_diagnostic(Message::new( + OxcDiagnostic::error(format!( + "Plugin `{plugin_name}/{rule_name}` returned invalid fixes: {err}" + )), + PossibleFixes::None, + )); + PossibleFixes::None + } + } + } + } else { + PossibleFixes::None + }; + ctx_host.push_diagnostic(Message::new( OxcDiagnostic::error(diagnostic.message) .with_label(span) .with_error_code(plugin_name.to_string(), rule_name.to_string()) .with_severity(severity.into()), - PossibleFixes::None, + fix, )); } }