fix(linter/plugins): remove getters from Context#20115
Merged
graphite-app[bot] merged 1 commit intomainfrom Mar 8, 2026
Merged
fix(linter/plugins): remove getters from Context#20115graphite-app[bot] merged 1 commit intomainfrom
Context#20115graphite-app[bot] merged 1 commit intomainfrom
Conversation
Member
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
github-actions
bot
added
A-linter
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Oxlint’s JS rule Context shape to better match ESLint’s expectations by replacing getter-based properties (notably options) with value properties, fixing interoperability with eslint-plugin-jsdoc and similar plugins that copy property descriptors.
Changes:
- Reworked
Contextcreation to use data properties forid/options, and prevent extension instead of freezing. - Updated rule registration/linting to populate
context.idandcontext.optionsviaObject.defineProperty. - Adjusted compat-layer and createOnce fixture/snapshots to reflect
id/optionsbeing present (asnull) duringcreateOnce.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/oxlint/src-js/plugins/context.ts | Replaces getter-based id/options with value properties; switches from Object.freeze to Object.preventExtensions. |
| apps/oxlint/src-js/plugins/load.ts | Stops storing a separate fullName; sets context.id after createOnce. |
| apps/oxlint/src-js/plugins/lint.ts | Sets per-file context.options via Object.defineProperty rather than ruleDetails.options. |
| apps/oxlint/src-js/plugins/options.ts | Uses ruleDetails.context.id for option validation/error messages instead of ruleDetails.fullName. |
| apps/oxlint/src-js/package/compat.ts | Makes compat createOnce context expose id/options as null values and throws on report during createOnce. |
| apps/oxlint/src-js/utils/globals.ts | Exposes Object.preventExtensions for TSDown global-rewrite support. |
| apps/oxlint/test/fixtures/createOnce/plugin.ts | Updates fixture expectations: id/options available as null in createOnce. |
| apps/oxlint/test/fixtures/createOnce/output.snap.md | Updates snapshot output to match fixture behavior changes. |
Member
Author
Merge activity
|
Fix an incompatibility with `eslint-plugin-jsdoc`. It relies on properties of `context` being values rather than getters: https://github.com/gajus/eslint-plugin-jsdoc/blob/26276d428631a2625575607cba7875dab6797091/src/buildForbidRuleDefinition.js#L86-L103 When `options` property is a getter, this code throws an error because it attempts to define a property with both `value` and `get` in its descriptor. This PR replaces the `id` and `options` getters with value properties. The getters were there to prevent the `Context` object from being mutated by rule code, and allow the object to be frozen, as it is in ESLint. But each rule gets its own `Context` object, so if it chooses to mutate it, it only harms itself, and can't impact other rules/plugins. In ESLint, trying to alter the frozen `Context` object would be a hard error, so it's doubtful that any ESLint plugins do this anyway. This is no real downside to this change for us, so we might as well make the change, and fix `eslint-plugin-jsdoc` and any other plugins which may depend on ESLint's behavior here.
graphite-app
bot
force-pushed
the
om/03-08-fix_linter_plugins_remove_getters_from_context_
branch
from
650b97a to
0861d9a
Compare
graphite-app
bot
deleted the
om/03-08-fix_linter_plugins_remove_getters_from_context_
branch
camc314
pushed a commit
that referenced
this pull request
Mar 9, 2026
# Oxlint ### 🚀 Features - e6b604f oxlint: Auto-enable gitlab formatter on GitLab (#20076) (camc314) - 2488a68 linter: Add .oxlintrc.jsonc config file support (#19870) (Scott S.) - 61bf388 linter: Add `options.reportUnusedDisableDirectives` to config file (#19799) (Peter Wagenet) - c92422b oxlint: Auto-enable github formatter on GitHub Actions (#19944) (Boshen) - 0337c6d linter: Implement typescript/no-unecessary-type-conversion (#19955) (camc314) - 2919313 linter: Introduce denyWarnings config options (#19926) (camc314) - a607119 linter: Introduce maxWarnings config option (#19777) (camc314) ### 🐛 Bug Fixes - 0861d9a linter/plugins: Remove getters from `Context` (#20115) (overlookmotel) - 92cfb14 linter/plugins: Fix types for `walkProgram` and `walkProgramWithCfg` (#20081) (overlookmotel) - 10e211f oxlint/lsp: Send other code actions besides `source.fixAll` if requested (#20042) (Sysix) - 602daaa linter/plugins: Fix type definition for `VisitorObject` (#20065) (overlookmotel) - ee0491e apps,napi: Explicitly specify libs in tsconfigs (#20071) (camc314) - b6e9499 linter: Fix the logic for `unicorn/prefer-dom-node-remove` to handle literal callees as well as arguments. (#20059) (connorshea) - 3874ae1 linter: Update `unicorn/prefer-query-selector` to also catch `getElementsByName()`. (#20060) (connorshea) - 77c93fb linter: Handle array-type shorthand inside union members (#20034) (camc314) - 50eb160 linter/no-unused-vars: Allow unused type params in ambient module blocks (#19615) (Don Isaac) - 1dd0d21 linter/no-restricted-imports: Apply regex pattern checks to side-effect imports (#20028) (camc314) - 7f3d735 linter: Error when --type-check is used without --type-aware (#20025) (camc314) - eea201c unicorn/prefer-string-slice: Avoid unsafe autofix for substr-to-slice (#20010) (camc314) - 50359dc oxlint/lsp: Detect `reportUnusedDisableDirectives` from oxlint config, change lsp `unusedDisableDirectives` default value to `None` (#20011) (Sysix) - 4bc84b1 linter/plugins: Allow `null` and `undefined` for `rule.meta.fixable` (#20008) (overlookmotel) - 753e27e linter/role-supports-aria-props: Add `aria-posinset` to supported `option` ARIA properties (#20003) (JongKyung Lee) - f57b2c9 linter/plugins: Fix return types of tokens methods (#19985) (overlookmotel) - 27ee4fc linter/no-loss-of-precision: Avoid double rounding for negative exponents (#19999) (camc314) - 77a94bb linter: Avoid no-loss-of-precision false positive for 3e-308 (#19992) (camc314) - 6245c56 linter/no-unused-private-class-members: Treat logical lhs access as usage (#19991) (camc314) - 65891e3 linter: Avoid prefer-const false positive for mixed-scope destructuring (#19982) (camc314) - 89991fe linter: Avoid prefer-const false positive for operator reassignments (#19975) (camc314) - 87318e7 oxlint/lsp: Load js config with reforcing fs read (#19551) (Sysix) - d40a942 linter/no-useless-constructor: Mark fixer as suggestion (#19961) (camc314) - ccbd959 linter/prefer-code-point: Report String.fromCharCode member references (#19931) (camc314) - 14fbbfc linter: Add help text to oxc/no-rest-spread-properties rule (#19900) (Subin Kim) ### ⚡ Performance - 2baa5fb napi: Unify build-test profile to coverage for cache sharing (#20090) (Boshen) - 77f1c71 linter/plugins: Make tokens class instances (#19980) (overlookmotel) - 758b424 linter/plugins: Reduce memory copies for tokens (#19979) (overlookmotel) - 236847f linter/plugins: Cache token objects (#19978) (overlookmotel) - 94b597a linter/plugins: Store tokens as a `Box<[Token]>` (#19969) (overlookmotel) ### 📚 Documentation - 2c0010a linter/plugins: Move comment about "bivariance hack" into generated code (#20082) (overlookmotel) - 7538f09 linter: Improve `import/extensions` and `import/no-named-as-default` rule docs. (#20053) (connorshea) - 1f909cf linter: Improve docs for `unicorn/require-post-message-target-origin` rule. (#20061) (connorshea) - 12ae35c oxlint/lsp: Remove outdated ToDo for `LintOptions.run` (#20012) (Sysix) - 3be73e6 linter/plugins: Fix JSDoc comments for tokens methods (#20004) (overlookmotel) - 48ef285 linter: Update `--config` docs (#19965) (camc314) - 6ea49a0 linter: Fix some identation issues for the generated types used in `oxlint.config.ts`. (#19942) (connorshea) - 6c0e0b5 linter: Add oxlint.config.ts to the config docs. (#19941) (connorshea) - 160e423 linter: Add a note that the typeAware and typeCheck options require oxlint-tsgolint (#19940) (connorshea) - d54c275 linter: Improve rule docs for 27 unicorn rules (#19903) (connorshea) # Oxfmt ### 🚀 Features - ee26215 oxfmt: Support css-in-js substitution (#20019) (leaysgur) - 0f0ff51 oxfmt: Display default settings was used message in cli stats (#19939) (leaysgur) - 88815b8 oxfmt: Reintroduce stats line for write mode (#19938) (leaysgur) ### 🐛 Bug Fixes - ee0491e apps,napi: Explicitly specify libs in tsconfigs (#20071) (camc314) - 92f4490 oxfmt: Apply `is_ignored_dir` for glob paths too (#20056) (leaysgur) - 114f974 oxfmt/lsp: Prefer language_id over file extension when formatting (#19977) (copilot-swe-agent) ### ⚡ Performance - 2baa5fb napi: Unify build-test profile to coverage for cache sharing (#20090) (Boshen) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix an incompatibility with
eslint-plugin-jsdoc. It relies on properties ofcontextbeing values rather than getters:https://github.com/gajus/eslint-plugin-jsdoc/blob/26276d428631a2625575607cba7875dab6797091/src/buildForbidRuleDefinition.js#L86-L103
When
optionsproperty is a getter, this code throws an error because it attempts to define a property with bothvalueandgetin its descriptor.This PR replaces the
idandoptionsgetters with value properties.The getters were there to prevent the
Contextobject from being mutated by rule code, and allow the object to be frozen, as it is in ESLint. But each rule gets its ownContextobject, so if it chooses to mutate it, it only harms itself, and can't impact other rules/plugins. In ESLint, trying to alter the frozenContextobject would be a hard error, so it's doubtful that any ESLint plugins do this anyway.This is no real downside to this change for us, so we might as well make the change, and fix
eslint-plugin-jsdocand any other plugins which may depend on ESLint's behavior here.The typedef for
Contextis now technically incorrect -idandoptionsproperties can benullduringcreateOnce, which is not reflected in the types. But that's a pragmatic choice. The type of these properties is correct for anywhere that it's actually useful to access these properties.