fix(linter/plugins): remove getters from Context

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

@@ -176,13 +176,17 @@ function createContextAndVisitor(rule: CreateOnceRule): {
   }
 
   // Call `createOnce` with empty context object.
-  // Really, accessing `options` or calling `report` should throw, because they're illegal in `createOnce`.
-  // But any such bugs should have been caught when testing the rule in Oxlint, so should be OK to take this shortcut.
-  // `FILE_CONTEXT` prototype provides `extends` method, which is available in `createOnce`.
+  // We don't use `Object.preventExtensions` on the context object, as we need to change its prototype for each file.
   const context: Context = Object.create(FILE_CONTEXT, {
-    id: { value: "", enumerable: true, configurable: true },
+    id: { value: null, enumerable: true, configurable: true },
     options: { value: null, enumerable: true, configurable: true },
-    report: { value: null, enumerable: true, configurable: true },
+    report: {
+      value() {
+        throw new Error("Cannot report errors in `createOnce`");
+      },
+      enumerable: true,
+      configurable: true,
+    },
   });
 
   let {

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

@@ -474,10 +474,12 @@ export interface Context extends FileContext {
   /**
    * Rule ID, in form `<plugin>/<rule>`.
    */
+  // Note: This is `null` during `createOnce` call, but we keep the type simple to make it easier for the user.
   id: string;
   /**
    * Rule options for this rule on this file.
    */
+  // Note: This is `null` during `createOnce` call, but we keep the type simple to make it easier for the user.
   options: Readonly<Options>;
   /**
    * Report an error/warning.
@@ -490,7 +492,7 @@ export interface Context extends FileContext {
  * @param ruleDetails - `RuleDetails` object
  * @returns `Context` object
  */
-export function createContext(ruleDetails: RuleDetails): Readonly<Context> {
+export function createContext(ruleDetails: RuleDetails): Context {
   // Create `Context` object for rule.
   //
   // All properties are enumerable, to support a pattern which some ESLint plugins use:
@@ -508,47 +510,54 @@ export function createContext(ruleDetails: RuleDetails): Readonly<Context> {
   // }
   // ```
   //
-  // Object is frozen to prevent user mutating it.
+  // ESLint freezes the `Context` object, but we can't as we need to alter `id` and `options` properties.
+  // We get as close as we can by making `id` and `options` properties `writable: false`, and preventing user adding
+  // new properties with `Object.preventExtensions`.
   //
-  // IMPORTANT: Methods/getters must not use `this`, to support wrapped context objects
+  // `id` and `options` properties are `configurable` so we can alter them with `Object.defineProperty`.
+  // In any case, `Context` objects are specific to each rule, so if a rule does mutate the context object,
+  // that won't affect anyone other than themselves. Options objects are frozen, so user can't mutate them.
+  //
+  // IMPORTANT: `report` must not use `this`, to support wrapped context objects
   // or e.g. `const { report } = context; report(diagnostic);`.
   // https://github.com/oxc-project/oxc/issues/15325
-  return Object.freeze({
-    // Inherit from `FILE_CONTEXT`, which provides getters for file-specific properties
-    __proto__: FILE_CONTEXT,
-
-    // Rule ID, in form `<plugin>/<rule>`
-    get id(): string {
-      // It's not possible to allow access to `id` in `createOnce` in ESLint compatibility mode, so we don't
-      // allow it here either. It's probably not very useful anyway - a rule should know what its own name is!
-      if (filePath === null) throw new Error("Cannot access `context.id` in `createOnce`");
-      return ruleDetails.fullName;
-    },
-
-    // Getter for rule options for this rule on this file
-    get options(): Readonly<Options> {
-      if (filePath === null) throw new Error("Cannot access `context.options` in `createOnce`");
-      debugAssertIsNonNull(ruleDetails.options);
-      return ruleDetails.options;
-    },
-
-    /**
-     * Report error.
-     *
-     * Normally called with a single `Diagnostic` object.
-     *
-     * Can also be called with legacy positional forms:
-     * - `context.report(node, message, data?, fix?)`
-     * - `context.report(node, loc, message, data?, fix?)`
-     * These legacy forms are not included in type def for this method, as they are deprecated,
-     * but some plugins still use them, so we support them.
-     *
-     * @param diagnostic - Diagnostic object
-     * @throws {TypeError} If `diagnostic` is invalid
-     */
-    report(this: void, diagnostic: Diagnostic, ...extraArgs: unknown[]): void {
-      // Delegate to `report` implementation shared between all rules, passing rule-specific details (`RuleDetails`)
-      report(diagnostic, extraArgs, ruleDetails);
-    },
-  } as unknown as Context); // It seems TS can't understand `__proto__: FILE_CONTEXT`
+  return Object.preventExtensions(
+    Object.create(FILE_CONTEXT, {
+      // Rule ID, in form `<plugin>/<rule>`.
+      // Initially `null` during `createOnce`, set to full rule name after `createOnce` is called.
+      id: {
+        value: null!,
+        enumerable: true,
+        configurable: true,
+      },
+      // Rule options for this rule on this file.
+      // Initially `null` during `createOnce`, set to options object before linting a file in `lintFileImpl`.
+      options: {
+        value: null!,
+        enumerable: true,
+        configurable: true,
+      },
+      report: {
+        /**
+         * Report error.
+         *
+         * Normally called with a single `Diagnostic` object.
+         *
+         * Can also be called with legacy positional forms:
+         * - `context.report(node, message, data?, fix?)`
+         * - `context.report(node, loc, message, data?, fix?)`
+         * These legacy forms are not included in type def for this method, as they are deprecated,
+         * but some plugins still use them, so we support them.
+         *
+         * @param diagnostic - Diagnostic object
+         * @throws {TypeError} If `diagnostic` is invalid
+         */
+        value(this: void, diagnostic: Diagnostic, ...extraArgs: unknown[]): void {
+          // Delegate to `report` implementation shared between all rules, passing rule-specific details (`RuleDetails`)
+          report(diagnostic, extraArgs, ruleDetails);
+        },
+        enumerable: true,
+      },
+    }),
+  );
 }

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

@@ -189,8 +189,9 @@ export function lintFileImpl(
 
     // If the rule has no user-provided options, use the plugin-provided default
     // options (which falls back to `DEFAULT_OPTIONS`)
-    ruleDetails.options =
-      optionsId === DEFAULT_OPTIONS_ID ? ruleDetails.defaultOptions : allOptions[optionsId];
+    Object.defineProperty(ruleDetails.context, "options", {
+      value: optionsId === DEFAULT_OPTIONS_ID ? ruleDetails.defaultOptions : allOptions[optionsId],
+    });
 
     let { visitor } = ruleDetails;
     if (visitor === null) {

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

@@ -52,8 +52,7 @@ export type RuleDetails = CreateRuleDetails | CreateOnceRuleDetails;
 
 interface RuleDetailsBase {
   // Static properties of the rule
-  readonly fullName: string;
-  readonly context: Readonly<Context>;
+  readonly context: Context;
   readonly isFixable: boolean;
   readonly hasSuggestions: boolean;
   readonly messages: Readonly<Record<string, string>> | null;
@@ -64,7 +63,6 @@ interface RuleDetailsBase {
   readonly optionsSchemaValidator: SchemaValidator | false | null;
   // Updated for each file
   ruleIndex: number;
-  options: Readonly<Options> | null; // Initially `null`, set to options object before linting a file
 }
 
 interface CreateRuleDetails extends RuleDetailsBase {
@@ -250,7 +248,6 @@ export function registerPlugin(
 
     // Create `RuleDetails` object for rule.
     const ruleDetails: RuleDetails = {
-      fullName: fullRuleName,
       rule: rule as CreateRule, // Could also be `CreateOnceRule`, but just to satisfy type checker
       context: null!, // Filled in below
       isFixable,
@@ -259,7 +256,6 @@ export function registerPlugin(
       defaultOptions,
       optionsSchemaValidator: schemaValidator,
       ruleIndex: 0,
-      options: null,
       visitor: null,
       beforeHook: null,
       afterHook: null,
@@ -301,6 +297,10 @@ export function registerPlugin(
       (ruleDetails as unknown as Writable<CreateOnceRuleDetails>).afterHook = afterHook;
     }
 
+    // Set `id` property on `Context` object.
+    // Do this after calling `createOnce` - `createOnce` should not have access to `id` property.
+    Object.defineProperty(ruleDetails.context, "id", { value: fullRuleName });
+
     registeredRules.push(ruleDetails);
   }
 

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

@@ -283,7 +283,9 @@ export function setOptions(optionsJson: string): void {
 function processOptions(configOptions: Options, ruleDetails: RuleDetails): Readonly<Options> {
   // Throw if no schema validator provided
   const validator = ruleDetails.optionsSchemaValidator;
-  if (validator === null) throw new Error(`Rule '${ruleDetails.fullName}' does not accept options`);
+  if (validator === null) {
+    throw new Error(`Rule '${ruleDetails.context.id}' does not accept options`);
+  }
 
   // Merge with `defaultOptions` first
   const { defaultOptions } = ruleDetails;
@@ -299,7 +301,7 @@ function processOptions(configOptions: Options, ruleDetails: RuleDetails): Reado
   // `mergeOptions` cloned `defaultOptions`, so mutations made by AJV validation won't affect `defaultOptions`
   // (and `defaultOptions` is frozen anyway, so it can't be mutated).
   // `configOptions` may be mutated, but that's OK, because we only use it once.
-  if (validator !== false) validator(options, ruleDetails.fullName);
+  if (validator !== false) validator(options, ruleDetails.context.id);
 
   deepFreezeJsonArray(options);
   return options;

apps/oxlint/src-js/utils/globals.ts

@@ -14,6 +14,7 @@ export const {
   keys: ObjectKeys,
   values: ObjectValues,
   freeze: ObjectFreeze,
+  preventExtensions: ObjectPreventExtensions,
   defineProperty: ObjectDefineProperty,
   defineProperties: ObjectDefineProperties,
   create: ObjectCreate,

apps/oxlint/test/fixtures/createOnce/output.snap.md

@@ -81,13 +81,13 @@
    : ^
    `----
 
-  x create-once-plugin(always-run): createOnce: id error: Cannot access `context.id` in `createOnce`
+  x create-once-plugin(always-run): createOnce: id: null
    ,-[files/1.js:1:1]
  1 | let x;
    : ^
    `----
 
-  x create-once-plugin(always-run): createOnce: options error: Cannot access `context.options` in `createOnce`
+  x create-once-plugin(always-run): createOnce: options: null
    ,-[files/1.js:1:1]
  1 | let x;
    : ^
@@ -261,13 +261,13 @@
    : ^
    `----
 
-  x create-once-plugin(always-run): createOnce: id error: Cannot access `context.id` in `createOnce`
+  x create-once-plugin(always-run): createOnce: id: null
    ,-[files/2.js:1:1]
  1 | let y;
    : ^
    `----
 
-  x create-once-plugin(always-run): createOnce: options error: Cannot access `context.options` in `createOnce`
+  x create-once-plugin(always-run): createOnce: options: null
    ,-[files/2.js:1:1]
  1 | let y;
    : ^

apps/oxlint/test/fixtures/createOnce/plugin.ts

@@ -20,15 +20,16 @@ const alwaysRunRule: Rule = {
     // oxlint-disable-next-line typescript-eslint/no-this-alias
     const topLevelThis = this;
 
+    // Available but `null`
+    const { id, options } = context;
+
     // Check that these APIs throw here
-    const idError = tryCatch(() => context.id);
     const cwdError = tryCatch(() => context.cwd);
     const getCwdError = tryCatch(() => context.getCwd());
     const filenameError = tryCatch(() => context.filename);
     const getFilenameError = tryCatch(() => context.getFilename());
     const physicalFilenameError = tryCatch(() => context.physicalFilename);
     const getPhysicalFilenameError = tryCatch(() => context.getPhysicalFilename());
-    const optionsError = tryCatch(() => context.options);
     const sourceCodeError = tryCatch(() => context.sourceCode);
     const getSourceCodeError = tryCatch(() => context.getSourceCode());
     const settingsError = tryCatch(() => context.settings);
@@ -38,6 +39,8 @@ const alwaysRunRule: Rule = {
 
     return {
       before() {
+        context.report({ message: `createOnce: id: ${id}`, node: SPAN });
+        context.report({ message: `createOnce: options: ${JSON.stringify(options)}`, node: SPAN });
         context.report({ message: `createOnce: call count: ${createOnceCallCount}`, node: SPAN });
         context.report({
           message: `createOnce: this === rule: ${topLevelThis === alwaysRunRule}`,
@@ -48,7 +51,6 @@ const alwaysRunRule: Rule = {
           message: `createOnce: getCwd() error: ${getCwdError?.message}`,
           node: SPAN,
         });
-        context.report({ message: `createOnce: id error: ${idError?.message}`, node: SPAN });
         context.report({
           message: `createOnce: filename error: ${filenameError?.message}`,
           node: SPAN,
@@ -65,10 +67,6 @@ const alwaysRunRule: Rule = {
           message: `createOnce: getPhysicalFilename() error: ${getPhysicalFilenameError?.message}`,
           node: SPAN,
         });
-        context.report({
-          message: `createOnce: options error: ${optionsError?.message}`,
-          node: SPAN,
-        });
         context.report({
           message: `createOnce: sourceCode error: ${sourceCodeError?.message}`,
           node: SPAN,