Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
7 changes: 6 additions & 1 deletion apps/oxlint/src-js/bindings.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
/* auto-generated by NAPI-RS */
/* eslint-disable */
/** JS callback to clear loaded plugins. */
export type JsClearLoadedPluginCb =
(() => void)

/** JS callback to lint a file. */
export type JsLintFileCb =
((arg0: string, arg1: number, arg2: Uint8Array | undefined | null, arg3: Array<number>, arg4: string) => string)
Expand All @@ -15,7 +19,8 @@ export type JsLoadPluginCb =
* 1. `args`: Command line arguments (process.argv.slice(2))
* 2. `load_plugin`: Load a JS plugin from a file path.
* 3. `lint_file`: Lint a file.
* 4. `clear_loaded_plugin`: Clear loaded plugin state.
*
* Returns `true` if linting succeeded without errors, `false` otherwise.
*/
export declare function lint(args: Array<string>, loadPlugin: JsLoadPluginCb, lintFile: JsLintFileCb): Promise<boolean>
export declare function lint(args: Array<string>, loadPlugin: JsLoadPluginCb, lintFile: JsLintFileCb, clearLoadedPlugin: JsClearLoadedPluginCb): Promise<boolean>
19 changes: 16 additions & 3 deletions apps/oxlint/src-js/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { debugAssertIsNonNull } from './utils/asserts.js';
// are identical. Ditto `lintFile` and `lintFileWrapper`.
let loadPlugin: typeof loadPluginWrapper | null = null;
let lintFile: typeof lintFileWrapper | null = null;
let clearLoadedPlugin: typeof clearLoadedPluginWrapper | null = null;

/**
* Load a plugin.
Expand All @@ -21,7 +22,7 @@ function loadPluginWrapper(path: string, packageName: string | null): Promise<st
// Use promises here instead of making `loadPluginWrapper` an async function,
// to avoid a micro-tick and extra wrapper `Promise` in all later calls to `loadPluginWrapper`
return import('./plugins/index.js').then((mod) => {
({ loadPlugin, lintFile } = mod);
({ loadPlugin, lintFile, clearLoadedPlugin } = mod);
return loadPlugin(path, packageName);
});
}
Expand Down Expand Up @@ -54,11 +55,23 @@ function lintFileWrapper(
return lintFile(filePath, bufferId, buffer, ruleIds, settingsJSON);
}

/**
* Clear loaded plugin state.
*
* Delegates to `clearLoadedPlugin`, which was lazy-loaded by `loadPluginWrapper`.
* If plugins haven't been loaded yet, this is a no-op.
*/
function clearLoadedPluginWrapper(): void {
if (clearLoadedPlugin !== null) {
clearLoadedPlugin();
}
}

// Get command line arguments, skipping first 2 (node binary and script path)
const args = process.argv.slice(2);

// Call Rust, passing `loadPlugin` and `lintFile` as callbacks, and CLI arguments
const success = await lint(args, loadPluginWrapper, lintFileWrapper);
// Call Rust, passing `loadPlugin`, `lintFile`, and `clearLoadedPlugin` as callbacks, and CLI arguments
const success = await lint(args, loadPluginWrapper, lintFileWrapper, clearLoadedPluginWrapper);

// Note: It's recommended to set `process.exitCode` instead of calling `process.exit()`.
// `process.exit()` kills the process immediately and `stdout` may not be flushed before process dies.
Expand Down
16 changes: 16 additions & 0 deletions apps/oxlint/src-js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,3 +274,19 @@ function createContextAndVisitor(rule: CreateOnceRule): {

return { context, visitor, beforeHook };
}

/**
* Clear all loaded plugins and rules.
*
* This function clears the internal state of registered plugins and rules,
* allowing plugins to be reloaded from scratch. This is useful for the
* language server when restarting or reloading configuration.
*
* Note: This function is lazy-loaded and will only be available after
* the first plugin has been loaded. It will not have any effect if no
* plugins have been loaded yet.
*/
export async function clearLoadedPlugin(): Promise<void> {
const mod = await import('./plugins/index.js');
mod.clearLoadedPlugin();
}
4 changes: 2 additions & 2 deletions apps/oxlint/src-js/plugins/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { lintFile } from './lint.js';
import { loadPlugin } from './load.js';
import { clearLoadedPlugin, loadPlugin } from './load.js';

export { lintFile, loadPlugin };
export { clearLoadedPlugin, lintFile, loadPlugin };
12 changes: 12 additions & 0 deletions apps/oxlint/src-js/plugins/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,15 @@ function conformHookFn<H>(hookFn: H | null | undefined, hookName: string): H | n
if (typeof hookFn !== 'function') throw new TypeError(`\`${hookName}\` hook must be a function if provided`);
return hookFn;
}

/**
* Clear all loaded plugins and rules.
*
* This function clears the internal state of registered plugins and rules,
* allowing plugins to be reloaded from scratch. This is useful for the
* language server when restarting or reloading configuration.
*/
export function clearLoadedPlugin(): void {
registeredPluginPaths.clear();
registeredRules.length = 0;
}
18 changes: 16 additions & 2 deletions apps/oxlint/src/js_plugins/external_linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,20 @@ use oxc_linter::{

use crate::{
generated::raw_transfer_constants::{BLOCK_ALIGN, BUFFER_SIZE},
run::{JsLintFileCb, JsLoadPluginCb},
run::{JsClearLoadedPluginCb, JsLintFileCb, JsLoadPluginCb},
};

/// Wrap JS callbacks as normal Rust functions, and create [`ExternalLinter`].
pub fn create_external_linter(
load_plugin: JsLoadPluginCb,
lint_file: JsLintFileCb,
clear_loaded_plugin: JsClearLoadedPluginCb,
) -> ExternalLinter {
let rust_load_plugin = wrap_load_plugin(load_plugin);
let rust_lint_file = wrap_lint_file(lint_file);
let rust_clear_loaded_plugin = wrap_clear_loaded_plugin(clear_loaded_plugin);

ExternalLinter::new(rust_load_plugin, rust_lint_file)
ExternalLinter::new(rust_load_plugin, rust_lint_file, rust_clear_loaded_plugin)
}

/// Wrap `loadPlugin` JS callback as a normal Rust function.
Expand Down Expand Up @@ -59,6 +61,18 @@ pub enum LintFileReturnValue {
Failure(String),
}

/// Wrap `clearLoadedPlugin` JS callback as a normal Rust function.
///
/// This function is called when the `ExternalLinter` is dropped to clear loaded plugin state.
fn wrap_clear_loaded_plugin(
cb: JsClearLoadedPluginCb,
) -> oxc_linter::ExternalLinterClearLoadedPluginCb {
Box::new(move || {
// Call the JavaScript callback to clear loaded plugin state
let _ = cb.call((), ThreadsafeFunctionCallMode::Blocking);
})
Comment on lines +70 to +73
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result of cb.call() is silently discarded. If the JavaScript callback fails or throws an error, it will be ignored. Consider logging the error or adding a comment explaining why errors can be safely ignored during cleanup. This is especially important since this is called from the Drop implementation where panicking would be problematic.

Copilot uses AI. Check for mistakes.
}

/// Wrap `lintFile` JS callback as a normal Rust function.
///
/// The returned function creates a `Uint8Array` referencing the memory of the given `Allocator`,
Expand Down
34 changes: 30 additions & 4 deletions apps/oxlint/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,26 +52,48 @@ pub type JsLintFileCb = ThreadsafeFunction<
false,
>;

/// JS callback to clear loaded plugins.
#[napi]
pub type JsClearLoadedPluginCb = ThreadsafeFunction<
// Arguments
(),
// Return value
(),
// Arguments (repeated)
(),
// Error status
Status,
// CalleeHandled
false,
>;

/// NAPI entry point.
///
/// JS side passes in:
/// 1. `args`: Command line arguments (process.argv.slice(2))
/// 2. `load_plugin`: Load a JS plugin from a file path.
/// 3. `lint_file`: Lint a file.
/// 4. `clear_loaded_plugin`: Clear loaded plugin state.
///
/// Returns `true` if linting succeeded without errors, `false` otherwise.
#[expect(clippy::allow_attributes)]
#[allow(clippy::trailing_empty_array, clippy::unused_async)] // https://github.com/napi-rs/napi-rs/issues/2758
#[napi]
pub async fn lint(args: Vec<String>, load_plugin: JsLoadPluginCb, lint_file: JsLintFileCb) -> bool {
lint_impl(args, load_plugin, lint_file).await.report() == ExitCode::SUCCESS
pub async fn lint(
args: Vec<String>,
load_plugin: JsLoadPluginCb,
lint_file: JsLintFileCb,
clear_loaded_plugin: JsClearLoadedPluginCb,
) -> bool {
lint_impl(args, load_plugin, lint_file, clear_loaded_plugin).await.report() == ExitCode::SUCCESS
}

/// Run the linter.
async fn lint_impl(
args: Vec<String>,
load_plugin: JsLoadPluginCb,
lint_file: JsLintFileCb,
clear_loaded_plugin: JsClearLoadedPluginCb,
) -> CliRunResult {
// Convert String args to OsString for compatibility with bpaf
let args: Vec<std::ffi::OsString> = args.into_iter().map(std::ffi::OsString::from).collect();
Expand Down Expand Up @@ -104,10 +126,14 @@ async fn lint_impl(

// JS plugins are only supported on 64-bit little-endian platforms at present
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
let external_linter = Some(super::js_plugins::create_external_linter(load_plugin, lint_file));
let external_linter = Some(super::js_plugins::create_external_linter(
load_plugin,
lint_file,
clear_loaded_plugin,
));
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
let external_linter = {
let (_, _) = (load_plugin, lint_file);
let (_, _, _) = (load_plugin, lint_file, clear_loaded_plugin);
None
};

Expand Down
24 changes: 24 additions & 0 deletions apps/oxlint/test/clear-loaded-plugin.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { describe, it, expect } from 'vitest';
import { clearLoadedPlugin } from '../dist/index.js';

/**
* Test the clearLoadedPlugin function
*/
describe('clearLoadedPlugin', () => {
it('should be exported from the main module', () => {
expect(clearLoadedPlugin).toBeDefined();
expect(typeof clearLoadedPlugin).toBe('function');
});

it('should execute without errors', async () => {
// Call the clear function - it should not throw
await expect(clearLoadedPlugin()).resolves.toBeUndefined();
});

it('should work when called multiple times', async () => {
// Call it multiple times to ensure it doesn't error
await expect(clearLoadedPlugin()).resolves.toBeUndefined();
await expect(clearLoadedPlugin()).resolves.toBeUndefined();
await expect(clearLoadedPlugin()).resolves.toBeUndefined();
});
});
13 changes: 12 additions & 1 deletion crates/oxc_linter/src/external_linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ pub type ExternalLinterLintFileCb = Box<
+ Send,
>;

pub type ExternalLinterClearLoadedPluginCb = Box<dyn Fn() + Send + Sync>;

#[derive(Clone, Debug, Deserialize)]
pub enum PluginLoadResult {
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -47,14 +49,23 @@ pub struct JsFix {
pub struct ExternalLinter {
pub(crate) load_plugin: ExternalLinterLoadPluginCb,
pub(crate) lint_file: ExternalLinterLintFileCb,
clear_loaded_plugin: ExternalLinterClearLoadedPluginCb,
}

impl ExternalLinter {
pub fn new(
load_plugin: ExternalLinterLoadPluginCb,
lint_file: ExternalLinterLintFileCb,
clear_loaded_plugin: ExternalLinterClearLoadedPluginCb,
) -> Self {
Self { load_plugin, lint_file }
Self { load_plugin, lint_file, clear_loaded_plugin }
}
}

impl Drop for ExternalLinter {
fn drop(&mut self) {
// Clear loaded plugin state when the linter is destroyed
(self.clear_loaded_plugin)();
}
Comment on lines +65 to 69
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Drop implementation makes a blocking call to JavaScript, which is unsafe and can cause deadlocks or panics. If ExternalLinter is dropped during program shutdown or on a thread without an active JS runtime, the ThreadsafeFunctionCallMode::Blocking call will hang indefinitely or panic. Drop implementations must be infallible and non-blocking.

// Instead, use non-blocking mode and ignore errors
impl Drop for ExternalLinter {
    fn drop(&mut self) {
        // Use NonBlocking mode to avoid deadlocks during shutdown
        let _ = (self.clear_loaded_plugin)();
    }
}

Or remove the Drop implementation entirely and require explicit cleanup by the caller.

Suggested change
impl Drop for ExternalLinter {
fn drop(&mut self) {
// Clear loaded plugin state when the linter is destroyed
(self.clear_loaded_plugin)();
}
impl Drop for ExternalLinter {
fn drop(&mut self) {
// Use NonBlocking mode to avoid deadlocks during shutdown
let _ = (self.clear_loaded_plugin)();
}
}

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

}

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ pub use crate::{
},
context::{ContextSubHost, LintContext},
external_linter::{
ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, JsFix,
LintFileResult, PluginLoadResult,
ExternalLinter, ExternalLinterClearLoadedPluginCb, ExternalLinterLintFileCb,
ExternalLinterLoadPluginCb, JsFix, LintFileResult, PluginLoadResult,
},
external_plugin_store::{ExternalPluginStore, ExternalRuleId},
fixer::{Fix, FixKind, Message, PossibleFixes},
Expand Down
Loading