Skip to content
Open
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
16 changes: 13 additions & 3 deletions apps/oxlint/src-js/bindings.d.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
/* auto-generated by NAPI-RS */
/* eslint-disable */
/** JS callback to create a workspace. */
export type JsCreateWorkspaceCb =
((arg0: string) => Promise<undefined>)

/** JS callback to destroy a workspace. */
export type JsDestroyWorkspaceCb =
((arg0: string) => void)

/** JS callback to lint a file. */
export type JsLintFileCb =
((arg0: string, arg1: number, arg2: Uint8Array | undefined | null, arg3: Array<number>, arg4: string) => string)
((arg0: string, arg1: string, arg2: number, arg3: Uint8Array | undefined | null, arg4: Array<number>, arg5: string) => string)

/** JS callback to load a JS plugin. */
export type JsLoadPluginCb =
((arg0: string, arg1?: string | undefined | null) => Promise<string>)
((arg0: string, arg1: string, arg2?: string | undefined | null) => Promise<string>)

/**
* NAPI entry point.
Expand All @@ -15,7 +23,9 @@ 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. `create_workspace`: Create a new workspace.
* 5. `destroy_workspace`: Destroy a workspace.
*
* 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, createWorkspace: JsCreateWorkspaceCb, destroyWorkspace: JsDestroyWorkspaceCb): Promise<boolean>
73 changes: 57 additions & 16 deletions apps/oxlint/src-js/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,34 @@ 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 createWorkspace: typeof createWorkspaceWrapper | null = null;
let destroyWorkspace: typeof destroyWorkspaceWrapper | null = null;

/**
* Load a plugin.
*
* Lazy-loads plugins code on first call, so that overhead is skipped if user doesn't use JS plugins.
* Delegates to `loadPlugin`, which was lazy-loaded by `createWorkspaceWrapper`.
*
* @param workspaceDir - Workspace root directory
* @param path - Absolute path of plugin file
* @param packageName - Optional package name from `package.json` (fallback if `plugin.meta.name` is not defined)
* @returns Plugin details or error serialized to JSON string
*/
function loadPluginWrapper(path: string, packageName: string | null): Promise<string> {
if (loadPlugin === null) {
// 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);
return loadPlugin(path, packageName);
});
}
function loadPluginWrapper(
workspaceDir: string,
path: string,
packageName: string | null,
): Promise<string> {
debugAssertIsNonNull(loadPlugin);
return loadPlugin(path, packageName);
return loadPlugin(workspaceDir, path, packageName);
}

/**
* Lint a file.
*
* Delegates to `lintFile`, which was lazy-loaded by `loadPluginWrapper`.
* Delegates to `lintFile`, which was lazy-loaded by `createWorkspaceWrapper`.
*
* @param workspaceDir - Directory of the workspace
* @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
Expand All @@ -42,23 +42,64 @@ function loadPluginWrapper(path: string, packageName: string | null): Promise<st
* @returns Diagnostics or error serialized to JSON string
*/
function lintFileWrapper(
rootDir: string,
filePath: string,
bufferId: number,
buffer: Uint8Array | null,
ruleIds: number[],
settingsJSON: string,
): string {
// `lintFileWrapper` is never called without `loadPluginWrapper` being called first,
// `lintFileWrapper` is never called without `createWorkspaceWrapper` being called first,
// so `lintFile` must be defined here
debugAssertIsNonNull(lintFile);
return lintFile(filePath, bufferId, buffer, ruleIds, settingsJSON);
return lintFile(rootDir, filePath, bufferId, buffer, ruleIds, settingsJSON);
}

/**
* Create a new workspace.
*
* Lazy-loads workspace code on first call, so that overhead is skipped if user doesn't use JS plugins.
*
* @param rootDir - Root directory of the workspace
* @returns Promise that resolves when workspace is created
*/
function createWorkspaceWrapper(rootDir: string): Promise<undefined> {
if (createWorkspace === null) {
// Use promises here instead of making `createWorkspaceWrapper` an async function,
// to avoid a micro-tick and extra wrapper `Promise` in all later calls to `createWorkspaceWrapper`
return import("./plugins/index.js").then((mod) => {
({ loadPlugin, lintFile, createWorkspace, destroyWorkspace } = mod);
return createWorkspace(rootDir);
});
}

debugAssertIsNonNull(createWorkspace);
return Promise.resolve(createWorkspace(rootDir));
}

/**
* Destroy a workspace.
*
* @param rootDir - Root directory of the workspace
*/
function destroyWorkspaceWrapper(rootDir: string): void {
// `destroyWorkspaceWrapper` is never called without `createWorkspaceWrapper` being called first,
// so `destroyWorkspace` must be defined here
debugAssertIsNonNull(destroyWorkspace);
destroyWorkspace(rootDir);
}

// 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`, `createWorkspace` and `destroyWorkspace` as callbacks, and CLI arguments
const success = await lint(
args,
loadPluginWrapper,
lintFileWrapper,
createWorkspaceWrapper,
destroyWorkspaceWrapper,
);

// 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
1 change: 1 addition & 0 deletions apps/oxlint/src-js/plugins/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export { lintFile } from "./lint.js";
export { loadPlugin } from "./load.js";
export { createWorkspace, destroyWorkspace } from "./workspace.js";
20 changes: 13 additions & 7 deletions apps/oxlint/src-js/plugins/lint.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { setupFileContext, resetFileContext } from "./context.js";
import { debugAssert, debugAssertIsNonNull, typeAssertIs } from "../utils/asserts.js";
import { getErrorMessage } from "../utils/utils.js";
import { resetFileContext, setupFileContext } from "./context.js";
import { registeredRules } from "./load.js";
import { allOptions, DEFAULT_OPTIONS_ID } from "./options.js";
import { diagnostics } from "./report.js";
import { setSettingsForFile, resetSettings } from "./settings.js";
import { resetSettings, setSettingsForFile } from "./settings.js";
import { ast, initAst, resetSourceAndAst, setupSourceForFile } from "./source_code.js";
import { typeAssertIs, debugAssert, debugAssertIsNonNull } from "../utils/asserts.js";
import { getErrorMessage } from "../utils/utils.js";
import {
addVisitorToCompiled,
compiledVisitor,
Expand Down Expand Up @@ -42,6 +42,7 @@ const PARSER_SERVICES_DEFAULT: Record<string, unknown> = Object.freeze({});
*
* Main logic is in separate function `lintFileImpl`, because V8 cannot optimize functions containing try/catch.
*
* @param workspaceDir - Directory of the workspace
* @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
Expand All @@ -50,6 +51,7 @@ const PARSER_SERVICES_DEFAULT: Record<string, unknown> = Object.freeze({});
* @returns Diagnostics or error serialized to JSON string
*/
export function lintFile(
workspaceDir: string,
filePath: string,
bufferId: number,
buffer: Uint8Array | null,
Expand All @@ -60,7 +62,7 @@ export function lintFile(
const optionsIds = ruleIds.map((_) => DEFAULT_OPTIONS_ID);

try {
lintFileImpl(filePath, bufferId, buffer, ruleIds, optionsIds, settingsJSON);
lintFileImpl(workspaceDir, filePath, bufferId, buffer, ruleIds, optionsIds, settingsJSON);
return JSON.stringify({ Success: diagnostics });
} catch (err) {
return JSON.stringify({ Failure: getErrorMessage(err) });
Expand All @@ -72,6 +74,7 @@ export function lintFile(
/**
* Run rules on a file.
*
* @param workspaceDir - Directory of the workspace
* @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
Expand All @@ -83,6 +86,7 @@ export function lintFile(
* @throws {*} If any rule throws
*/
function lintFileImpl(
workspaceDir: string,
filePath: string,
bufferId: number,
buffer: Uint8Array | null,
Expand Down Expand Up @@ -145,10 +149,12 @@ function lintFileImpl(
"Rule IDs and options IDs arrays must be the same length",
);

const rules = registeredRules.get(workspaceDir)!;

for (let i = 0, len = ruleIds.length; i < len; i++) {
const ruleId = ruleIds[i];
debugAssert(ruleId < registeredRules.length, "Rule ID out of bounds");
const ruleDetails = registeredRules[ruleId];
debugAssert(ruleId < rules.length, "Rule ID out of bounds");
const ruleDetails = rules[ruleId];

// Set `ruleIndex` for rule. It's used when sending diagnostics back to Rust.
ruleDetails.ruleIndex = i;
Expand Down
35 changes: 24 additions & 11 deletions apps/oxlint/src-js/plugins/load.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { getErrorMessage } from "../utils/utils.js";
import { createContext } from "./context.js";
import { DEFAULT_OPTIONS } from "./options.js";
import { getErrorMessage } from "../utils/utils.js";

import type { Writable } from "type-fest";
import type { SetNullable } from "../utils/types.ts";
import type { Context } from "./context.ts";
import type { Options } from "./options.ts";
import type { RuleMeta } from "./rule_meta.ts";
import type { AfterHook, BeforeHook, Visitor, VisitorWithHooks } from "./types.ts";
import type { SetNullable } from "../utils/types.ts";

const ObjectKeys = Object.keys,
{ isArray } = Array;
Expand Down Expand Up @@ -78,11 +78,11 @@ interface CreateOnceRuleDetails extends RuleDetailsBase {
}

// Absolute paths of plugins which have been loaded
const registeredPluginUrls = new Set<string>();
export const registeredPluginUrls = new Map<string, Set<string>>();

// Rule objects for loaded rules.
// Indexed by `ruleId`, which is passed to `lintFile`.
export const registeredRules: RuleDetails[] = [];
export const registeredRules: Map<string, RuleDetails[]> = new Map();

// `before` hook which makes rule never run.
const neverRunBeforeHook: BeforeHook = () => false;
Expand All @@ -102,13 +102,18 @@ interface PluginDetails {
*
* Main logic is in separate function `loadPluginImpl`, because V8 cannot optimize functions containing try/catch.
*
* @param workspaceDir - Workspace root directory
* @param url - Absolute path of plugin file as a `file://...` URL
* @param packageName - Optional package name from `package.json` (fallback if `plugin.meta.name` is not defined)
* @returns Plugin details or error serialized to JSON string
*/
export async function loadPlugin(url: string, packageName: string | null): Promise<string> {
export async function loadPlugin(
workspaceDir: string,
url: string,
packageName: string | null,
): Promise<string> {
try {
const res = await loadPluginImpl(url, packageName);
const res = await loadPluginImpl(workspaceDir, url, packageName);
return JSON.stringify({ Success: res });
} catch (err) {
return JSON.stringify({ Failure: getErrorMessage(err) });
Expand All @@ -118,19 +123,27 @@ export async function loadPlugin(url: string, packageName: string | null): Promi
/**
* Load a plugin.
*
* @param workspaceDir - Workspace root directory
* @param url - Absolute path of plugin file as a `file://...` URL
* @param packageName - Optional package name from `package.json` (fallback if `plugin.meta.name` is not defined)
* @returns - Plugin details
* @throws {Error} If workspaceDir is invalid
* @throws {Error} If plugin has already been registered
* @throws {Error} If plugin has no name
* @throws {TypeError} If one of plugin's rules is malformed, or its `createOnce` method returns invalid visitor
* @throws {TypeError} if `plugin.meta.name` is not a string
* @throws {*} If plugin throws an error during import
*/
async function loadPluginImpl(url: string, packageName: string | null): Promise<PluginDetails> {
async function loadPluginImpl(
workspaceDir: string,
url: string,
packageName: string | null,
): Promise<PluginDetails> {
if (DEBUG) {
if (registeredPluginUrls.has(url)) throw new Error("This plugin has already been registered");
registeredPluginUrls.add(url);
if (!registeredRules.has(workspaceDir)) throw new Error("Invalid workspaceDir");
if (registeredPluginUrls.get(workspaceDir)?.has(url))
throw new Error("This plugin has already been registered");
registeredPluginUrls.get(workspaceDir)?.add(url);
}

const { default: plugin } = (await import(url)) as { default: Plugin };
Expand All @@ -139,7 +152,7 @@ async function loadPluginImpl(url: string, packageName: string | null): Promise<

const pluginName = getPluginName(plugin, packageName);

const offset = registeredRules.length;
const offset = registeredRules.get(workspaceDir)?.length ?? 0;
const { rules } = plugin;
const ruleNames = ObjectKeys(rules);
const ruleNamesLen = ruleNames.length;
Expand Down Expand Up @@ -232,7 +245,7 @@ async function loadPluginImpl(url: string, packageName: string | null): Promise<
(ruleDetails as unknown as Writable<CreateOnceRuleDetails>).afterHook = afterHook;
}

registeredRules.push(ruleDetails);
registeredRules.get(workspaceDir)?.push(ruleDetails);
Copy link
Contributor

Choose a reason for hiding this comment

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

Silent failure when pushing rule. If the workspace doesn't exist, the optional chaining causes push to not execute, but no error is thrown. This creates inconsistent state where offset is calculated on line 155 assuming the workspace exists, but the rule is never actually registered.

const rules = registeredRules.get(workspaceDir);
if (!rules) {
  throw new Error(`Workspace "${workspaceDir}" has not been created`);
}
rules.push(ruleDetails);

The workspace should be validated to exist, or an error should be thrown.

Suggested change
registeredRules.get(workspaceDir)?.push(ruleDetails);
const rules = registeredRules.get(workspaceDir);
if (!rules) {
throw new Error(`Workspace "${workspaceDir}" has not been created`);
}
rules.push(ruleDetails);

Spotted by Graphite Agent

Fix in Graphite


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

}

return { name: pluginName, offset, ruleNames };
Expand Down
45 changes: 45 additions & 0 deletions apps/oxlint/src-js/plugins/workspace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Isolated Workspaces for linting plugins.
*
* Every Workspace starts with a "workspace root" directory. This directory is
* used to isolate the plugin's dependencies from other plugins and the main
* application.
*
* Each workspace can be created, used, and then cleared to free up resources.
*/

import { registeredPluginUrls, registeredRules } from "./load.js";

/**
* Set of workspace root directories.
*/
const workspaceRoots = new Set<string>();

/**
* Create a new workspace.
*/
export const createWorkspace = async (rootDir: string): Promise<undefined> => {
if (DEBUG) {
if (workspaceRoots.has(rootDir))
throw new Error(`Workspace for rootDir "${rootDir}" already exists`);
}

workspaceRoots.add(rootDir);
registeredPluginUrls.set(rootDir, new Set<string>());
registeredRules.set(rootDir, []);
};

/**
* Destroy a workspace.
*/
export const destroyWorkspace = (rootDir: string): undefined => {
if (DEBUG) {
if (!workspaceRoots.has(rootDir))
throw new Error(`Workspace for rootDir "${rootDir}" does not exist`);
if (!registeredPluginUrls.has(rootDir)) throw new Error("Invalid workspaceDir");
if (!registeredRules.has(rootDir)) throw new Error("Invalid workspaceDir");
}
workspaceRoots.delete(rootDir);
registeredPluginUrls.delete(rootDir);
registeredRules.delete(rootDir);
};
Loading
Loading