fix(linter/plugins): Return empty object for unimplemented parserServices#15364
Conversation
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. |
1ba6650 to
6481125
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR changes the parserServices getter in SourceCode from throwing an error to returning an empty object. This allows JavaScript plugins to access context.sourceCode.parserServices without causing runtime errors, even though parser services are not yet fully implemented.
- Changed
parserServicesto return an empty object instead of throwing an error - Updated the test utilities to handle the optional "with N rules" text in output
- Added test coverage for the
parserServicesaccess pattern
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/oxlint/src-js/plugins/source_code.ts | Changed parserServices getter to return empty object instead of throwing |
| apps/oxlint/test/utils.ts | Updated regex to handle optional rule count in output normalization |
| apps/oxlint/test/e2e.test.ts | Added test case for parserServices access |
| apps/oxlint/test/fixtures/parser_services/plugin.ts | Test plugin that accesses parserServices |
| apps/oxlint/test/fixtures/parser_services/files/index.js | Test file for the parser services test |
| apps/oxlint/test/fixtures/parser_services/.oxlintrc.json | Configuration for parser services test |
| apps/oxlint/test/fixtures/parser_services/output.snap.md | Expected output snapshot for parser services test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: magic-akari <akari.ccino@gmail.com>
|
@copilot Define an empty object in a top level |
overlookmotel
left a comment
There was a problem hiding this comment.
Copilot isn't answering my call. I'll make that change in a follow-up.
I pushed a couple of small changes to this PR.
Merge activity
You must have a Graphite account in order to use the merge queue. Create an account and try again using this link |
Follow-on after #15364. Re-use a single object for `parserServices`, instead of creating a new object each time. This is better for perf, and also matches ESLint where each access to `sourceCode.parserServices` in a `create` invocation results in the same object.
Summary
sourceCode.parserServicesfrom throwing an error to returning an empty object{}eslint-plugin-better-tailwindcss.Motivation
According to the ESLint custom parser documentation:
The current implementation throws an error when
parserServicesis accessed, which breaks plugins that use defensive checks like:Plugin authors defensively handle:
parserServicesbeing undefineddefineTemplateBodyVisitornot existingparserServicesgetter throwing an error (unexpected)Solution
Return an empty object
{}for unimplemented parser services, which:Test Plan
Related
Fixes the compatibility issue with plugins like eslint-plugin-better-tailwindcss