diff --git a/apps/oxlint/src-js/package/compat.ts b/apps/oxlint/src-js/package/compat.ts index 8d32c044f0cb3..a99d99929e950 100644 --- a/apps/oxlint/src-js/package/compat.ts +++ b/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 { diff --git a/apps/oxlint/src-js/plugins/context.ts b/apps/oxlint/src-js/plugins/context.ts index 0af19a9de024c..f75da10611e1e 100644 --- a/apps/oxlint/src-js/plugins/context.ts +++ b/apps/oxlint/src-js/plugins/context.ts @@ -474,10 +474,12 @@ export interface Context extends FileContext { /** * Rule ID, in form `/`. */ + // 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; /** * 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 { +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 { // } // ``` // - // 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 `/` - 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 { - 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 `/`. + // 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, + }, + }), + ); } diff --git a/apps/oxlint/src-js/plugins/lint.ts b/apps/oxlint/src-js/plugins/lint.ts index 5086453b28d3b..c91f43fd6e7c7 100644 --- a/apps/oxlint/src-js/plugins/lint.ts +++ b/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) { diff --git a/apps/oxlint/src-js/plugins/load.ts b/apps/oxlint/src-js/plugins/load.ts index 179b7c1d1e94f..aa8addbf60e89 100644 --- a/apps/oxlint/src-js/plugins/load.ts +++ b/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; + readonly context: Context; readonly isFixable: boolean; readonly hasSuggestions: boolean; readonly messages: Readonly> | null; @@ -64,7 +63,6 @@ interface RuleDetailsBase { readonly optionsSchemaValidator: SchemaValidator | false | null; // Updated for each file ruleIndex: number; - options: Readonly | 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).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); } diff --git a/apps/oxlint/src-js/plugins/options.ts b/apps/oxlint/src-js/plugins/options.ts index 07ebfcc8d9c59..2934715ee4313 100644 --- a/apps/oxlint/src-js/plugins/options.ts +++ b/apps/oxlint/src-js/plugins/options.ts @@ -283,7 +283,9 @@ export function setOptions(optionsJson: string): void { function processOptions(configOptions: Options, ruleDetails: RuleDetails): Readonly { // 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; diff --git a/apps/oxlint/src-js/utils/globals.ts b/apps/oxlint/src-js/utils/globals.ts index 705adc8c46e00..1a2cd240cd50e 100644 --- a/apps/oxlint/src-js/utils/globals.ts +++ b/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, diff --git a/apps/oxlint/test/fixtures/createOnce/output.snap.md b/apps/oxlint/test/fixtures/createOnce/output.snap.md index 4f1599bd33ad8..052f7d52a50fd 100644 --- a/apps/oxlint/test/fixtures/createOnce/output.snap.md +++ b/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; : ^ diff --git a/apps/oxlint/test/fixtures/createOnce/plugin.ts b/apps/oxlint/test/fixtures/createOnce/plugin.ts index 2a354dd358049..ffd2970efbcc6 100644 --- a/apps/oxlint/test/fixtures/createOnce/plugin.ts +++ b/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,