Skip to content

Commit 3df2272

Browse files
CopilotSysix
andcommitted
Call clearLoadedPlugin in ExternalLinter destructor
- Add JsClearLoadedPluginCb type to handle plugin state clearing - Add clearLoadedPlugin callback parameter to create_external_linter - Implement Drop trait for ExternalLinter to call clearLoadedPlugin - Update lint and lint_impl to accept and pass clearLoadedPlugin callback - Update cli.ts to pass clearLoadedPluginWrapper callback - Export ExternalLinterClearLoadedPluginCb from oxc_linter The clearLoadedPlugin function is now automatically called when the ExternalLinter is destroyed, ensuring plugin state is properly cleaned up after linting completes. Co-authored-by: Sysix <[email protected]>
1 parent ca5a920 commit 3df2272

File tree

6 files changed

+70
-14
lines changed

6 files changed

+70
-14
lines changed

apps/oxlint/src-js/bindings.d.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
/* auto-generated by NAPI-RS */
22
/* eslint-disable */
3+
/** JS callback to clear loaded plugins. */
4+
export type JsClearLoadedPluginCb =
5+
(() => void)
6+
37
/** JS callback to lint a file. */
48
export type JsLintFileCb =
59
((arg0: string, arg1: number, arg2: Uint8Array | undefined | null, arg3: Array<number>, arg4: string) => string)
@@ -15,7 +19,8 @@ export type JsLoadPluginCb =
1519
* 1. `args`: Command line arguments (process.argv.slice(2))
1620
* 2. `load_plugin`: Load a JS plugin from a file path.
1721
* 3. `lint_file`: Lint a file.
22+
* 4. `clear_loaded_plugin`: Clear loaded plugin state.
1823
*
1924
* Returns `true` if linting succeeded without errors, `false` otherwise.
2025
*/
21-
export declare function lint(args: Array<string>, loadPlugin: JsLoadPluginCb, lintFile: JsLintFileCb): Promise<boolean>
26+
export declare function lint(args: Array<string>, loadPlugin: JsLoadPluginCb, lintFile: JsLintFileCb, clearLoadedPlugin: JsClearLoadedPluginCb): Promise<boolean>

apps/oxlint/src-js/cli.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { debugAssertIsNonNull } from './utils/asserts.js';
66
// are identical. Ditto `lintFile` and `lintFileWrapper`.
77
let loadPlugin: typeof loadPluginWrapper | null = null;
88
let lintFile: typeof lintFileWrapper | null = null;
9+
let clearLoadedPlugin: typeof clearLoadedPluginWrapper | null = null;
910

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

58+
/**
59+
* Clear loaded plugin state.
60+
*
61+
* Delegates to `clearLoadedPlugin`, which was lazy-loaded by `loadPluginWrapper`.
62+
* If plugins haven't been loaded yet, this is a no-op.
63+
*/
64+
function clearLoadedPluginWrapper(): void {
65+
if (clearLoadedPlugin !== null) {
66+
clearLoadedPlugin();
67+
}
68+
}
69+
5770
// Get command line arguments, skipping first 2 (node binary and script path)
5871
const args = process.argv.slice(2);
5972

60-
// Call Rust, passing `loadPlugin` and `lintFile` as callbacks, and CLI arguments
61-
const success = await lint(args, loadPluginWrapper, lintFileWrapper);
73+
// Call Rust, passing `loadPlugin`, `lintFile`, and `clearLoadedPlugin` as callbacks, and CLI arguments
74+
const success = await lint(args, loadPluginWrapper, lintFileWrapper, clearLoadedPluginWrapper);
6275

6376
// Note: It's recommended to set `process.exitCode` instead of calling `process.exit()`.
6477
// `process.exit()` kills the process immediately and `stdout` may not be flushed before process dies.

apps/oxlint/src/js_plugins/external_linter.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,20 @@ use oxc_linter::{
1515

1616
use crate::{
1717
generated::raw_transfer_constants::{BLOCK_ALIGN, BUFFER_SIZE},
18-
run::{JsLintFileCb, JsLoadPluginCb},
18+
run::{JsClearLoadedPluginCb, JsLintFileCb, JsLoadPluginCb},
1919
};
2020

2121
/// Wrap JS callbacks as normal Rust functions, and create [`ExternalLinter`].
2222
pub fn create_external_linter(
2323
load_plugin: JsLoadPluginCb,
2424
lint_file: JsLintFileCb,
25+
clear_loaded_plugin: JsClearLoadedPluginCb,
2526
) -> ExternalLinter {
2627
let rust_load_plugin = wrap_load_plugin(load_plugin);
2728
let rust_lint_file = wrap_lint_file(lint_file);
29+
let rust_clear_loaded_plugin = wrap_clear_loaded_plugin(clear_loaded_plugin);
2830

29-
ExternalLinter::new(rust_load_plugin, rust_lint_file)
31+
ExternalLinter::new(rust_load_plugin, rust_lint_file, rust_clear_loaded_plugin)
3032
}
3133

3234
/// Wrap `loadPlugin` JS callback as a normal Rust function.
@@ -59,6 +61,16 @@ pub enum LintFileReturnValue {
5961
Failure(String),
6062
}
6163

64+
/// Wrap `clearLoadedPlugin` JS callback as a normal Rust function.
65+
///
66+
/// This function is called when the `ExternalLinter` is dropped to clear loaded plugin state.
67+
fn wrap_clear_loaded_plugin(cb: JsClearLoadedPluginCb) -> oxc_linter::ExternalLinterClearLoadedPluginCb {
68+
Box::new(move || {
69+
// Call the JavaScript callback to clear loaded plugin state
70+
let _ = cb.call((), ThreadsafeFunctionCallMode::Blocking);
71+
})
72+
}
73+
6274
/// Wrap `lintFile` JS callback as a normal Rust function.
6375
///
6476
/// The returned function creates a `Uint8Array` referencing the memory of the given `Allocator`,

apps/oxlint/src/run.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,28 +52,43 @@ pub type JsLintFileCb = ThreadsafeFunction<
5252
false,
5353
>;
5454

55-
55+
/// JS callback to clear loaded plugins.
56+
#[napi]
57+
pub type JsClearLoadedPluginCb = ThreadsafeFunction<
58+
// Arguments
59+
(),
60+
// Return value
61+
(),
62+
// Arguments (repeated)
63+
(),
64+
// Error status
65+
Status,
66+
// CalleeHandled
67+
false,
68+
>;
5669

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

7286
/// Run the linter.
7387
async fn lint_impl(
7488
args: Vec<String>,
7589
load_plugin: JsLoadPluginCb,
7690
lint_file: JsLintFileCb,
91+
clear_loaded_plugin: JsClearLoadedPluginCb,
7792
) -> CliRunResult {
7893
// Convert String args to OsString for compatibility with bpaf
7994
let args: Vec<std::ffi::OsString> = args.into_iter().map(std::ffi::OsString::from).collect();
@@ -106,10 +121,10 @@ async fn lint_impl(
106121

107122
// JS plugins are only supported on 64-bit little-endian platforms at present
108123
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
109-
let external_linter = Some(super::js_plugins::create_external_linter(load_plugin, lint_file));
124+
let external_linter = Some(super::js_plugins::create_external_linter(load_plugin, lint_file, clear_loaded_plugin));
110125
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
111126
let external_linter = {
112-
let (_, _) = (load_plugin, lint_file);
127+
let (_, _, _) = (load_plugin, lint_file, clear_loaded_plugin);
113128
None
114129
};
115130

crates/oxc_linter/src/external_linter.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ pub type ExternalLinterLintFileCb = Box<
1616
+ Send,
1717
>;
1818

19+
pub type ExternalLinterClearLoadedPluginCb = Box<dyn Fn() + Send + Sync>;
20+
1921
#[derive(Clone, Debug, Deserialize)]
2022
pub enum PluginLoadResult {
2123
#[serde(rename_all = "camelCase")]
@@ -47,14 +49,23 @@ pub struct JsFix {
4749
pub struct ExternalLinter {
4850
pub(crate) load_plugin: ExternalLinterLoadPluginCb,
4951
pub(crate) lint_file: ExternalLinterLintFileCb,
52+
clear_loaded_plugin: ExternalLinterClearLoadedPluginCb,
5053
}
5154

5255
impl ExternalLinter {
5356
pub fn new(
5457
load_plugin: ExternalLinterLoadPluginCb,
5558
lint_file: ExternalLinterLintFileCb,
59+
clear_loaded_plugin: ExternalLinterClearLoadedPluginCb,
5660
) -> Self {
57-
Self { load_plugin, lint_file }
61+
Self { load_plugin, lint_file, clear_loaded_plugin }
62+
}
63+
}
64+
65+
impl Drop for ExternalLinter {
66+
fn drop(&mut self) {
67+
// Clear loaded plugin state when the linter is destroyed
68+
(self.clear_loaded_plugin)();
5869
}
5970
}
6071

crates/oxc_linter/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ pub use crate::{
6060
},
6161
context::{ContextSubHost, LintContext},
6262
external_linter::{
63-
ExternalLinter, ExternalLinterLintFileCb, ExternalLinterLoadPluginCb, JsFix,
64-
LintFileResult, PluginLoadResult,
63+
ExternalLinter, ExternalLinterClearLoadedPluginCb, ExternalLinterLintFileCb,
64+
ExternalLinterLoadPluginCb, JsFix, LintFileResult, PluginLoadResult,
6565
},
6666
external_plugin_store::{ExternalPluginStore, ExternalRuleId},
6767
fixer::{Fix, FixKind, Message, PossibleFixes},

0 commit comments

Comments
 (0)