Skip to content

fix(linter/plugins): Return empty object for unimplemented parserServices#15364

Merged
overlookmotel merged 5 commits intooxc-project:mainfrom
magic-akari:fix/jslint-parser-services
Nov 6, 2025
Merged

fix(linter/plugins): Return empty object for unimplemented parserServices#15364
overlookmotel merged 5 commits intooxc-project:mainfrom
magic-akari:fix/jslint-parser-services

Conversation

@magic-akari
Copy link
Contributor

Summary

  • Changed sourceCode.parserServices from throwing an error to returning an empty object {}
  • Aligns with ESLint specification for parsers that don't implement parser services
  • Prevents crashes in ESLint plugins such as eslint-plugin-better-tailwindcss.

Motivation

According to the ESLint custom parser documentation:

services can contain any parser-dependent services (such as type checkers for nodes). The value of the services property is available to rules as context.sourceCode.parserServices. Default is an empty object.

The current implementation throws an error when parserServices is accessed, which breaks plugins that use defensive checks like:

if(typeof ctx.sourceCode.parserServices?.defineTemplateBodyVisitor === "function"){
  // ...
}

Plugin authors defensively handle:

  • parserServices being undefined
  • defineTemplateBodyVisitor not existing
  • parserServices getter throwing an error (unexpected)

Solution

Return an empty object {} for unimplemented parser services, which:

  1. Matches ESLint's documented default behavior
  2. Allows plugins' defensive checks to work correctly
  3. Provides correct semantics for "not implemented"

Test Plan

  • Existing tests should continue to pass
  • Plugins using defensive checks like the example above will no longer crash

Related

Fixes the compatibility issue with plugins like eslint-plugin-better-tailwindcss

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 6, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI C-bug Category - Bug labels Nov 6, 2025
@magic-akari magic-akari force-pushed the fix/jslint-parser-services branch from 1ba6650 to 6481125 Compare November 6, 2025 06:43
@magic-akari magic-akari marked this pull request as ready for review November 6, 2025 06:48
@magic-akari magic-akari requested a review from camc314 as a code owner November 6, 2025 06:48
Copilot AI review requested due to automatic review settings November 6, 2025 06:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 parserServices to 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 parserServices access 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>
@overlookmotel
Copy link
Member

@copilot Define an empty object in a top level const parserServices = {};. Make parserServices getter return that const, rather then creating a new object each time.

Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Nov 6, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 6, 2025

Merge activity

  • Nov 6, 2:23 PM UTC: @magic-akari we removed the merge queue label because we could not find a Graphite account associated with your GitHub profile.

You must have a Graphite account in order to use the merge queue. Create an account and try again using this link

@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Nov 6, 2025
@overlookmotel overlookmotel merged commit cc403f5 into oxc-project:main Nov 6, 2025
18 checks passed
graphite-app bot pushed a commit that referenced this pull request Nov 6, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-linter Area - Linter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants