fix(linter/plugins): get correct plugin name in all cases

apps/oxlint/.npmignore

@@ -0,0 +1,6 @@
+# Avoid ATTW errors due to nested `node_modules` directories inside `test/fixtures`
+# which contain packages with no `name` field in their `package.json`s.
+#
+# TSDown's `attw.excludeEntrypoints` option doesn't work - possible bug in TSDown?
+# https://github.com/rolldown/tsdown/issues/656
+test/fixtures/**/node_modules

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

@@ -40,7 +40,7 @@ export type JsLintFileCb =
 
 /** JS callback to load a JS plugin. */
 export type JsLoadPluginCb =
-  ((arg0: string, arg1?: string | undefined | null) => Promise<string>)
+  ((arg0: string, arg1: string | undefined | null, arg2: boolean) => Promise<string>)
 
 /** JS callback to setup configs. */
 export type JsSetupConfigsCb =

apps/oxlint/src-js/cli.ts

@@ -14,20 +14,25 @@ let lintFile: typeof lintFileWrapper | null = null;
  * Lazy-loads plugins code on first call, so that overhead is skipped if user doesn't use JS plugins.
  *
  * @param path - Absolute path of plugin file
- * @param packageName - Optional package name from `package.json` (fallback if `plugin.meta.name` is not defined)
+ * @param pluginName - Plugin name (either alias or package name)
+ * @param pluginNameIsAlias - `true` if plugin name is an alias (takes priority over name that plugin defines itself)
  * @returns Plugin details or error serialized to JSON string
  */
-function loadPluginWrapper(path: string, packageName: string | null): Promise<string> {
+function loadPluginWrapper(
+  path: string,
+  pluginName: string | null,
+  pluginNameIsAlias: boolean,
+): 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.ts").then((mod) => {
       ({ loadPlugin, lintFile, setupConfigs } = mod);
-      return loadPlugin(path, packageName);
+      return loadPlugin(path, pluginName, pluginNameIsAlias);
     });
   }
   debugAssertIsNonNull(loadPlugin);
-  return loadPlugin(path, packageName);
+  return loadPlugin(path, pluginName, pluginNameIsAlias);
 }
 
 /**

apps/oxlint/src-js/package/rule_tester.ts

@@ -962,7 +962,7 @@ function lint(test: TestCase, plugin: Plugin): Diagnostic[] {
 
   try {
     // Register plugin. This adds rule to `registeredRules` array.
-    registerPlugin(plugin, null);
+    registerPlugin(plugin, null, false);
 
     // Set up options
     const optionsId = setupOptions(test);

apps/oxlint/src-js/plugins/load.ts

@@ -2,6 +2,7 @@ import { createContext } from "./context.ts";
 import { deepFreezeJsonArray } from "./json.ts";
 import { compileSchema, DEFAULT_OPTIONS } from "./options.ts";
 import { getErrorMessage } from "../utils/utils.ts";
+import { debugAssertIsNonNull } from "../utils/asserts.ts";
 
 import type { Writable } from "type-fest";
 import type { Context } from "./context.ts";
@@ -106,18 +107,23 @@ interface PluginDetails {
  * Main logic is in separate function `loadPluginImpl`, because V8 cannot optimize functions containing try/catch.
  *
  * @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)
+ * @param pluginName - Plugin name (either alias or package name)
+ * @param pluginNameIsAlias - `true` if plugin name is an alias (takes priority over name that plugin defines itself)
  * @returns Plugin details or error serialized to JSON string
  */
-export async function loadPlugin(url: string, packageName: string | null): Promise<string> {
+export async function loadPlugin(
+  url: string,
+  pluginName: string | null,
+  pluginNameIsAlias: boolean,
+): Promise<string> {
   try {
     if (DEBUG) {
       if (registeredPluginUrls.has(url)) throw new Error("This plugin has already been registered");
       registeredPluginUrls.add(url);
     }
 
     const plugin = (await import(url)).default as Plugin;
-    const res = registerPlugin(plugin, packageName);
+    const res = registerPlugin(plugin, pluginName, pluginNameIsAlias);
     return JSON.stringify({ Success: res });
   } catch (err) {
     return JSON.stringify({ Failure: getErrorMessage(err) });
@@ -128,16 +134,21 @@ export async function loadPlugin(url: string, packageName: string | null): Promi
  * Register a plugin.
  *
  * @param plugin - Plugin
- * @param packageName - Optional package name from `package.json` (fallback if `plugin.meta.name` is not defined)
+ * @param pluginName - Plugin name (either alias or package name)
+ * @param pluginNameIsAlias - `true` if plugin name is an alias (takes priority over name that plugin defines itself)
  * @returns - Plugin details
  * @throws {Error} If `plugin.meta.name` is `null` / `undefined` and `packageName` not provided
  * @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
  */
-export function registerPlugin(plugin: Plugin, packageName: string | null): PluginDetails {
+export function registerPlugin(
+  plugin: Plugin,
+  pluginName: string | null,
+  pluginNameIsAlias: boolean,
+): PluginDetails {
   // TODO: Use a validation library to assert the shape of the plugin, and of rules
 
-  const pluginName = getPluginName(plugin, packageName);
+  pluginName = getPluginName(plugin, pluginName, pluginNameIsAlias);
 
   const offset = registeredRules.length;
   const { rules } = plugin;
@@ -282,35 +293,83 @@ export function registerPlugin(plugin: Plugin, packageName: string | null): Plug
 
 /**
  * Get plugin name.
+ *
+ * - Plugin is named with an alias in config, return the alias.
  * - If `plugin.meta.name` is defined, return it.
  * - Otherwise, fall back to `packageName`, if defined.
  * - If neither is defined, throw an error.
  *
  * @param plugin - Plugin object
- * @param packageName - Package name from `package.json`
+ * @param pluginName - Plugin name (either alias or package name)
+ * @param pluginNameIsAlias - `true` if plugin name is an alias (takes priority over name that plugin defines itself)
  * @returns Plugin name
  * @throws {TypeError} If `plugin.meta.name` is not a string
  * @throws {Error} If neither `plugin.meta.name` nor `packageName` are defined
  */
-function getPluginName(plugin: Plugin, packageName: string | null): string {
-  const pluginMeta = plugin.meta;
-  if (pluginMeta != null) {
-    const pluginMetaName = pluginMeta.name;
-    if (pluginMetaName != null) {
-      if (typeof pluginMetaName !== "string") {
-        throw new TypeError("`plugin.meta.name` must be a string if defined");
-      }
-      return pluginMetaName;
+function getPluginName(
+  plugin: Plugin,
+  pluginName: string | null,
+  pluginNameIsAlias: boolean,
+): string {
+  // If plugin is defined with an alias in config, that takes priority
+  if (pluginNameIsAlias) {
+    debugAssertIsNonNull(pluginName);
+    return pluginName;
+  }
+
+  // If plugin defines its own name, that takes priority over package name.
+  // Normalize plugin name.
+  const pluginMetaName = plugin.meta?.name;
+  if (pluginMetaName != null) {
+    if (typeof pluginMetaName !== "string") {
+      throw new TypeError("`plugin.meta.name` must be a string if defined");
     }
+    return normalizePluginName(pluginMetaName);
   }
 
-  if (packageName !== null) return packageName;
+  // Fallback to package name (which is already normalized on Rust side)
+  if (pluginName !== null) return pluginName;
 
   throw new Error(
-    "Plugin must either define `meta.name`, or be loaded from an NPM package with a `name` field in `package.json`",
+    "Plugin must either define `meta.name`, be loaded from an NPM package with a `name` field in `package.json`, " +
+      "or be given an alias in config",
   );
 }
 
+/**
+ * Normalize plugin name by stripping common ESLint plugin prefixes and suffixes.
+ *
+ * This handles the various naming conventions used in the ESLint ecosystem:
+ * - `eslint-plugin-foo` -> `foo`
+ * - `@scope/eslint-plugin` -> `@scope`
+ * - `@scope/eslint-plugin-foo` -> `@scope/foo`
+ *
+ * This logic is replicated on Rust side in `normalize_plugin_name` in `crates/oxc_linter/src/config/plugins.rs`.
+ * The 2 implementations must be kept in sync.
+ *
+ * @param name - Plugin name defined by plugin
+ * @returns Normalized plugin name
+ */
+function normalizePluginName(name: string): string {
+  const slashIndex = name.indexOf("/");
+
+  // If no slash, it's a non-scoped package. Trim off `eslint-plugin-` prefix.
+  if (slashIndex === -1) {
+    return name.startsWith("eslint-plugin-") ? name.slice("eslint-plugin-".length) : name;
+  }
+
+  const scope = name.slice(0, slashIndex),
+    rest = name.slice(slashIndex + 1);
+
+  // `@scope/eslint-plugin` -> `@scope`
+  if (rest === "eslint-plugin") return scope;
+  // `@scope/eslint-plugin-foo` -> `@scope/foo`
+  if (rest.startsWith("eslint-plugin-")) return `${scope}/${rest.slice("eslint-plugin-".length)}`;
+
+  // No normalization needed
+  return name;
+}
+
 /**
  * Validate and conform `before` / `after` hook function.
  * @param hookFn - Hook function, or `null` / `undefined`

apps/oxlint/src/js_plugins/external_linter.rs

@@ -45,11 +45,14 @@ pub enum LoadPluginReturnValue {
 ///
 /// The returned function will panic if called outside of a Tokio runtime.
 fn wrap_load_plugin(cb: JsLoadPluginCb) -> ExternalLinterLoadPluginCb {
-    Box::new(move |plugin_url, package_name| {
+    Box::new(move |plugin_url, plugin_name, plugin_name_is_alias| {
         let cb = &cb;
         let res = tokio::task::block_in_place(|| {
             tokio::runtime::Handle::current().block_on(async move {
-                cb.call_async(FnArgs::from((plugin_url, package_name))).await?.into_future().await
+                cb.call_async(FnArgs::from((plugin_url, plugin_name, plugin_name_is_alias)))
+                    .await?
+                    .into_future()
+                    .await
             })
         });
 

apps/oxlint/src/run.rs

@@ -20,11 +20,19 @@ use crate::{
 #[napi]
 pub type JsLoadPluginCb = ThreadsafeFunction<
     // Arguments
-    FnArgs<(String, Option<String>)>, // Absolute path of plugin file, optional package name
+    FnArgs<(
+        // File URL to load plugin from
+        String,
+        // Plugin name (either alias or package name).
+        // If is package name, it is pre-normalized.
+        Option<String>,
+        // `true` if plugin name is an alias (takes priority over name that plugin defines itself)
+        bool,
+    )>,
     // Return value
     Promise<String>, // `PluginLoadResult`, serialized to JSON
     // Arguments (repeated)
-    FnArgs<(String, Option<String>)>,
+    FnArgs<(String, Option<String>, bool)>,
     // Error status
     Status,
     // CalleeHandled

apps/oxlint/test/fixtures/plugin_name/.oxlintrc.json

@@ -0,0 +1,43 @@
+{
+  "plugins": ["jsdoc"],
+  "jsPlugins": [
+    // `plugin.meta.name` overrides package name
+    "plugin1",
+    // Package name used where no `plugin.meta.name`
+    "plugin2",
+    // `eslint-plugin-` prefix stripped
+    "eslint-plugin-plugin3",
+    "eslint-plugin-plugin4",
+    // `/eslint-plugin` suffix stripped
+    "@scope/eslint-plugin",
+    "@scope2/eslint-plugin",
+    // Plugin name is `jsdoc`, but that's overridden by alias
+    { "name": "js-jsdoc", "specifier": "./plugins/jsdoc.ts" },
+    { "name": "js-jsdoc2", "specifier": "jsdoc" },
+    // Plugin defines no `plugin.meta.name`, but not an error due to alias
+    { "name": "no-name-alias", "specifier": "./plugins/no_name.ts" },
+    { "name": "no-name-alias2", "specifier": "no_name" }
+  ],
+  "categories": { "correctness": "off" },
+  "rules": {
+    // `plugin1`
+    "plugin1-name-from-rule/rule": "error",
+    // `plugin2`
+    "plugin2-name-from-package/rule": "error",
+    // `eslint-plugin-plugin3`
+    "eslint-plugin-plugin3-name-from-package/rule": "error",
+    // `eslint-plugin-plugin4`
+    "eslint-plugin-plugin4-name-from-rule/rule": "error",
+    // `@scope/eslint-plugin`
+    "@scope/rule": "error",
+    // `@scope2/eslint-plugin`
+    "@scope2/rule": "error",
+    // Aliased
+    "js-jsdoc/rule": "error",
+    "js-jsdoc2/rule": "error",
+    "no-name-alias/rule": "error",
+    "no-name-alias2/rule": "error",
+    // Native `jsdoc` rule still works
+    "jsdoc/require-param": "error"
+  }
+}

apps/oxlint/test/fixtures/plugin_name/files/index.js

@@ -0,0 +1,4 @@
+/**
+ * @param foo
+ */
+function f(foo, bar) {}