Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/oxlint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ napi = { workspace = true, features = ["async"], optional = true }
napi-derive = { workspace = true, optional = true }
rayon = { workspace = true }
rustc-hash = { workspace = true }
serde = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
simdutf8 = { workspace = true }
tempfile = { workspace = true }
Expand Down
41 changes: 34 additions & 7 deletions apps/oxlint/src-js/plugins/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from '../generated/constants.js';
import { diagnostics, setupContextForFile } from './context.js';
import { registeredRules } from './load.js';
import { assertIs } from './utils.js';
import { assertIs, getErrorMessage } from './utils.js';
import { addVisitorToCompiled, compiledVisitor, finalizeCompiledVisitor, initCompiledVisitor } from './visitor.js';

// Lazy implementation
Expand Down Expand Up @@ -43,8 +43,40 @@ const textDecoder = new TextDecoder('utf-8', { ignoreBOM: true });
// Array of `after` hooks to run after traversal. This array reused for every file.
const afterHooks: AfterHook[] = [];

// Run rules on a file.
/**
* Run rules on a file.
*
* Main logic is in separate function `lintFileImpl`, because V8 cannot optimize functions containing try/catch.
*
* @param filePath - Absolute path of file being linted
* @param bufferId - ID of buffer containing file data
* @param buffer - Buffer containing file data, or `null` if buffer with this ID was previously sent to JS
* @param ruleIds - IDs of rules to run on this file
* @returns JSON result
*/
export function lintFile(filePath: string, bufferId: number, buffer: Uint8Array | null, ruleIds: number[]): string {
try {
lintFileImpl(filePath, bufferId, buffer, ruleIds);
return JSON.stringify({ Success: diagnostics });
} catch (err) {
return JSON.stringify({ Failure: getErrorMessage(err) });
} finally {
diagnostics.length = 0;
}
}

/**
* Run rules on a file.
*
* @param filePath - Absolute path of file being linted
* @param bufferId - ID of buffer containing file data
* @param buffer - Buffer containing file data, or `null` if buffer with this ID was previously sent to JS
* @param ruleIds - IDs of rules to run on this file
* @returns Diagnostics to send back to Rust
* @throws {Error} If any parameters are invalid
* @throws {*} If any rule throws
*/
function lintFileImpl(filePath: string, bufferId: number, buffer: Uint8Array | null, ruleIds: number[]) {
// If new buffer, add it to `buffers` array. Otherwise, get existing buffer from array.
// Do this before checks below, to make sure buffer doesn't get garbage collected when not expected
// if there's an error.
Expand Down Expand Up @@ -142,9 +174,4 @@ export function lintFile(filePath: string, bufferId: number, buffer: Uint8Array
// Reset array, ready for next file
afterHooks.length = 0;
}

// Send diagnostics back to Rust
const ret = JSON.stringify(diagnostics);
diagnostics.length = 0;
return ret;
}
23 changes: 16 additions & 7 deletions apps/oxlint/src/js_plugins/external_linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use napi::{
bindgen_prelude::{FnArgs, Uint8Array},
threadsafe_function::ThreadsafeFunctionCallMode,
};
use serde::Deserialize;

use oxc_allocator::{Allocator, free_fixed_size_allocator};
use oxc_linter::{
Expand Down Expand Up @@ -48,6 +49,13 @@ fn wrap_load_plugin(cb: JsLoadPluginCb) -> ExternalLinterLoadPluginCb {
})
}

/// Result returned by `lintFile` JS callback.
#[derive(Clone, Debug, Deserialize)]
pub enum LintFileReturnValue {
Success(Vec<LintFileResult>),
Failure(String),
}

/// Wrap `lintFile` JS callback as a normal Rust function.
///
/// The returned function creates a `Uint8Array` referencing the memory of the given `Allocator`,
Expand Down Expand Up @@ -79,7 +87,7 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb {
ThreadsafeFunctionCallMode::NonBlocking,
move |result, _env| {
let _ = match &result {
Ok(r) => match serde_json::from_str::<Vec<LintFileResult>>(r) {
Ok(r) => match serde_json::from_str::<LintFileReturnValue>(r) {
Ok(v) => tx.send(Ok(v)),
Err(_e) => tx.send(Err("Failed to deserialize lint result".to_string())),
},
Expand All @@ -90,14 +98,15 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb {
},
);

if status != Status::Ok {
return Err(format!("Failed to schedule callback: {status:?}"));
}
assert!(status == Status::Ok, "Failed to schedule callback: {status:?}");

match rx.recv() {
Ok(Ok(x)) => Ok(x),
Ok(Err(e)) => Err(format!("Callback reported error: {e}")),
Err(e) => Err(format!("Callback did not respond: {e}")),
Ok(Ok(x)) => match x {
LintFileReturnValue::Success(diagnostics) => Ok(diagnostics),
LintFileReturnValue::Failure(err) => Err(err),
},
Ok(Err(err)) => panic!("Callback reported error: {err}"),
Err(err) => panic!("Callback did not respond: {err}"),
}
})
}
Expand Down
65 changes: 65 additions & 0 deletions apps/oxlint/test/__snapshots__/e2e.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,71 @@ exports[`oxlint CLI > should report an error if a custom plugin throws an error
"
`;

exports[`oxlint CLI > should report an error if a custom plugin throws an error during linting > in \`after\` hook 1`] = `
"
x Error running JS plugin.
| File path: <root>/oxc/apps/oxlint/test/fixtures/custom_plugin_lint_after_hook_error/index.js
| Error: Whoops!
| at after (<root>/apps/oxlint/test/fixtures/custom_plugin_lint_after_hook_error/test_plugin/index.js:10:19)

Found 0 warnings and 1 error.
Finished in Xms on 1 file using X threads."
`;

exports[`oxlint CLI > should report an error if a custom plugin throws an error during linting > in \`before\` hook 1`] = `
"
x Error running JS plugin.
| File path: <root>/oxc/apps/oxlint/test/fixtures/custom_plugin_lint_before_hook_error/index.js
| Error: Whoops!
| at before (<root>/apps/oxlint/test/fixtures/custom_plugin_lint_before_hook_error/test_plugin/index.js:10:19)

Found 0 warnings and 1 error.
Finished in Xms on 1 file using X threads."
`;

exports[`oxlint CLI > should report an error if a custom plugin throws an error during linting > in \`create\` method 1`] = `
"
x Error running JS plugin.
| File path: <root>/oxc/apps/oxlint/test/fixtures/custom_plugin_lint_create_error/index.js
| Error: Whoops!
| at Object.create (<root>/apps/oxlint/test/fixtures/custom_plugin_lint_create_error/test_plugin/index.js:8:15)

Found 0 warnings and 1 error.
Finished in Xms on 1 file using X threads."
`;

exports[`oxlint CLI > should report an error if a custom plugin throws an error during linting > in \`createOnce\` method 1`] = `
"Failed to parse configuration file.

x Failed to load JS plugin: ./test_plugin
| Error: Whoops!
| at Object.createOnce (<root>/apps/oxlint/test/fixtures/custom_plugin_lint_createOnce_error/test_plugin/index.js:8:15)
"
`;

exports[`oxlint CLI > should report an error if a custom plugin throws an error during linting > in \`fix\` function 1`] = `
"
x Error running JS plugin.
| File path: <root>/oxc/apps/oxlint/test/fixtures/custom_plugin_lint_fix_error/index.js
| Error: Whoops!
| at Object.fix (<root>/apps/oxlint/test/fixtures/custom_plugin_lint_fix_error/test_plugin/index.js:14:23)
| at Identifier (<root>/apps/oxlint/test/fixtures/custom_plugin_lint_fix_error/test_plugin/index.js:10:21)

Found 0 warnings and 1 error.
Finished in Xms on 1 file using X threads."
`;

exports[`oxlint CLI > should report an error if a custom plugin throws an error during linting > in visit function 1`] = `
"
x Error running JS plugin.
| File path: <root>/oxc/apps/oxlint/test/fixtures/custom_plugin_lint_visit_error/index.js
| Error: Whoops!
| at Identifier (<root>/apps/oxlint/test/fixtures/custom_plugin_lint_visit_error/test_plugin/index.js:10:19)

Found 0 warnings and 1 error.
Finished in Xms on 1 file using X threads."
`;

exports[`oxlint CLI > should report an error if a rule is not found within a custom plugin 1`] = `
"Failed to parse configuration file.

Expand Down
52 changes: 49 additions & 3 deletions apps/oxlint/test/e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { describe, expect, it } from 'vitest';
import { execa } from 'execa';

const PACKAGE_ROOT_PATH = dirname(import.meta.dirname);
const ROOT_PATH = pathJoin(PACKAGE_ROOT_PATH, '../../../');
const CLI_PATH = pathJoin(PACKAGE_ROOT_PATH, 'dist/cli.js');
const ROOT_URL = new URL('../../../', import.meta.url).href;
const FIXTURES_URL = new URL('./fixtures/', import.meta.url).href;
Expand Down Expand Up @@ -37,12 +38,19 @@ function normalizeOutput(output: string): string {
// Remove lines from stack traces which are outside `fixtures` directory.
// Replace path to repo root in stack traces with `<root>`.
lines = lines.flatMap((line) => {
// e.g. ` | at file:///path/to/oxc/apps/oxlint/test/fixtures/foor/bar.js:1:1`
// e.g. ` | at whatever (file:///path/to/oxc/apps/oxlint/test/fixtures/foor/bar.js:1:1)`
const match = line.match(/^(\s*\|\s+at (?:.+?\()?)(.+)$/);
// e.g. ` | at file:///path/to/oxc/apps/oxlint/test/fixtures/foo/bar.js:1:1`
// e.g. ` | at whatever (file:///path/to/oxc/apps/oxlint/test/fixtures/foo/bar.js:1:1)`
let match = line.match(/^(\s*\|\s+at (?:.+?\()?)(.+)$/);
if (match) {
const [, premable, at] = match;
return at.startsWith(FIXTURES_URL) ? [`${premable}<root>/${at.slice(ROOT_URL.length)}`] : [];
} else {
// e.g. ` | File path: /path/to/oxc/apps/oxlint/test/fixtures/foo/bar.js`
match = line.match(/^(\s*\|\s+File path: )(.+)$/);
if (match) {
const [, premable, path] = match;
if (path.startsWith(ROOT_PATH)) return [`${premable}<root>/${path.slice(ROOT_PATH.length)}`];
}
}
return [line];
});
Expand Down Expand Up @@ -129,6 +137,44 @@ describe('oxlint CLI', () => {
expect(normalizeOutput(stdout)).toMatchSnapshot();
});

describe('should report an error if a custom plugin throws an error during linting', () => {
it('in `create` method', async () => {
const { stdout, exitCode } = await runOxlint('test/fixtures/custom_plugin_lint_create_error');
expect(exitCode).toBe(1);
expect(normalizeOutput(stdout)).toMatchSnapshot();
});

it('in `createOnce` method', async () => {
const { stdout, exitCode } = await runOxlint('test/fixtures/custom_plugin_lint_createOnce_error');
expect(exitCode).toBe(1);
expect(normalizeOutput(stdout)).toMatchSnapshot();
});

it('in visit function', async () => {
const { stdout, exitCode } = await runOxlint('test/fixtures/custom_plugin_lint_visit_error');
expect(exitCode).toBe(1);
expect(normalizeOutput(stdout)).toMatchSnapshot();
});

it('in `before` hook', async () => {
const { stdout, exitCode } = await runOxlint('test/fixtures/custom_plugin_lint_before_hook_error');
expect(exitCode).toBe(1);
expect(normalizeOutput(stdout)).toMatchSnapshot();
});

it('in `after` hook', async () => {
const { stdout, exitCode } = await runOxlint('test/fixtures/custom_plugin_lint_after_hook_error');
expect(exitCode).toBe(1);
expect(normalizeOutput(stdout)).toMatchSnapshot();
});

it('in `fix` function', async () => {
const { stdout, exitCode } = await runOxlint('test/fixtures/custom_plugin_lint_fix_error');
expect(exitCode).toBe(1);
expect(normalizeOutput(stdout)).toMatchSnapshot();
});
});

it('should report the correct severity when using a custom plugin', async () => {
const { stdout, exitCode } = await runOxlint('test/fixtures/basic_custom_plugin_warn_severity');
expect(exitCode).toBe(0);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"plugins": ["./test_plugin"],
"categories": { "correctness": "off" },
"rules": {
"error-plugin/error": "error"
},
"ignorePatterns": ["test_plugin/**"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let x;
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export default {
meta: {
name: "error-plugin",
},
rules: {
error: {
createOnce(_context) {
return {
after() {
throw new Error("Whoops!");
},
};
},
},
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"plugins": ["./test_plugin"],
"categories": { "correctness": "off" },
"rules": {
"error-plugin/error": "error"
},
"ignorePatterns": ["test_plugin/**"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let x;
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export default {
meta: {
name: "error-plugin",
},
rules: {
error: {
createOnce(_context) {
return {
before() {
throw new Error("Whoops!");
},
};
},
},
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"plugins": ["./test_plugin"],
"categories": { "correctness": "off" },
"rules": {
"error-plugin/error": "error"
},
"ignorePatterns": ["test_plugin/**"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let x;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export default {
meta: {
name: "error-plugin",
},
rules: {
error: {
createOnce(_context) {
throw new Error("Whoops!");
},
},
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"plugins": ["./test_plugin"],
"categories": { "correctness": "off" },
"rules": {
"error-plugin/error": "error"
},
"ignorePatterns": ["test_plugin/**"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let x;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export default {
meta: {
name: "error-plugin",
},
rules: {
error: {
create(_context) {
throw new Error("Whoops!");
},
},
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"plugins": ["./test_plugin"],
"categories": { "correctness": "off" },
"rules": {
"error-plugin/error": "error"
},
"ignorePatterns": ["test_plugin/**"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let x;
Loading
Loading