Skip to content

fix: preserve parsing error locations in Bun - #281

Open
lumirlumir wants to merge 2 commits into
mainfrom
fix/preserve-parsing-error-locations-in-bun
Open

fix: preserve parsing error locations in Bun#281
lumirlumir wants to merge 2 commits into
mainfrom
fix/preserve-parsing-error-locations-in-bun

Conversation

@lumirlumir

@lumirlumir lumirlumir commented Sep 5, 2026

Copy link
Copy Markdown
Member

Prerequisites checklist

AI acknowledgment

  • I did not use AI to generate this PR.
  • (If the above is not checked) I have reviewed the AI-generated content before submitting.

What is the purpose of this pull request?

Environment

  • ESLint version: 10.7.0
  • @eslint/json version: 2.1.0
  • npm version: 11.16.0
  • Bun version: 1.4.2
  • Operating System: Windows x64

Which language are you using?

json

What did you do?

Install eslint@10.7.0 and @eslint/json@2.1.0, then save this as repro.mjs:

import { Linter } from "eslint";
import json from "@eslint/json";

const messages = new Linter().verify(
	"{\n//test\n}",
	[{ files: ["**/*.json"], plugins: { json }, language: "json/json" }],
	{ filename: "test.json" },
);
console.log(JSON.stringify(messages, null, 2));

Run bun repro.mjs and compare with node repro.mjs.

What did you expect to happen?

The parsing error includes line: 2 and column: 1, as it does in Node.js.

What actually happened?

Bun omits both properties:

[
	{
		"ruleId": null,
		"fatal": true,
		"severity": 2,
		"message": "Parsing error: Unexpected character '/' found."
	}
]

Link to Minimal Reproducible Example

The standalone reproduction is included above.

Related reproduction in CI: https://github.com/eslint/json/actions/runs/33971265538/job/101320084273?pr=281

What changes did you make? (Give an overview)

In Bun, the parser error's line and column are non-enumerable, so ...ex in JSONLanguage.parse() drops them. Explicitly copying line: ex.line and column: ex.column preserves the location.

Related Issues

Ref: #259

Is there anything you'd like reviewers to focus on?

I found it while working on #259.

Summary by CodeRabbit

  • Bug Fixes

    • JSON parsing errors now report the accurate line and column where the issue occurred.
  • Tests

    • Added coverage to verify error locations for invalid comments and trailing commas in JSON and JSONC modes.

@eslint-github-bot eslint-github-bot Bot added the bug Something isn't working label Sep 5, 2026
@eslintbot eslintbot added this to Triage Sep 5, 2026
@github-project-automation github-project-automation Bot moved this to Needs Triage in Triage Sep 5, 2026
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: b1089015-2190-4be7-91b2-3d614a60576a

📥 Commits

Reviewing files that changed from the base of the PR and between 780bfcd and 31f3f2c.

📒 Files selected for processing (2)
  • src/languages/json-language.js
  • tests/languages/json-language.test.js

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The JSON parser now returns line and column values for parsing failures. Tests verify locations for invalid comments and trailing commas in JSON and JSONC.

Changes

JSON error locations

Layer / File(s) Summary
Parser error metadata and validation
src/languages/json-language.js, tests/languages/json-language.test.js
Parsing errors now include parser-provided line and column values. Tests verify line 2, column 1 for invalid comments and line 3, column 1 for trailing commas in JSON and JSONC.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 31f3f

JSON parsing errors now retain line and column locations in Bun, matching Node.js behavior. The focused implementation and coverage introduce no remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving parsing error locations in Bun.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/preserve-parsing-error-locations-in-bun

Comment @coderabbitai help to get the list of available commands.

@lumirlumir
lumirlumir marked this pull request as ready for review September 5, 2026 14:24
@mdjermanovic mdjermanovic moved this from Needs Triage to Triaging in Triage Sep 5, 2026
@mdjermanovic

Copy link
Copy Markdown
Member

In Bun, the parser error's line and column are non-enumerable

How did these properties become non-enumerable? I couldn't reproduce this locally.

@DMartens

DMartens commented Sep 5, 2026

Copy link
Copy Markdown

I was able to reproduce the missing line and column property with Bun 1.4.
As lumir said, the reason is that they are not enumerable (checked via Object.getOwnPropertyDescriptors(error)).

@mdjermanovic

Copy link
Copy Markdown
Member

As lumir said, the reason is that they are not enumerable

I get it, but why are they not enumerable in bun? If these properties are generated by the parser, perhaps this problem should be fixed there.

@DMartens

DMartens commented Sep 6, 2026

Copy link
Copy Markdown

I could not find an issue for these properties in the Bun repository.
As the properties line and column of Error are non-standard, I think we should rather fix it here.

@lumirlumir

Copy link
Copy Markdown
Member Author

I’ve looked into the context further, and it seems that line and column properties on Error are non-standard and aren’t specified by ECMAScript. Therefore, whether they are enumerable may depend on the runtime implementation.

https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-error-objects

It also says:

https://tc39.es/ecma262/multipage/conformance.html#sec-conformance

image

@mdjermanovic

Copy link
Copy Markdown
Member

Okay, so Bun always sets the line and column properties on error objects. These properties represent the line and column in the file where the error was thrown, and they are non-enumerable. The problem is that we are using the same properties in the parser API, where they represent the line and column where invalid syntax was detected in the given source code. The parser overwrites values with ones expected by the API, but the properties stay non-enumerable. Is this a correct description of the cause of this problem?

@lumirlumir

Copy link
Copy Markdown
Member Author

Okay, so Bun always sets the line and column properties on error objects. These properties represent the line and column in the file where the error was thrown, and they are non-enumerable. The problem is that we are using the same properties in the parser API, where they represent the line and column where invalid syntax was detected in the given source code. The parser overwrites values with ones expected by the API, but the properties stay non-enumerable. Is this a correct description of the cause of this problem?

Yes. If I understand it correctly, I believe this is the root cause of the problem.

@mdjermanovic

Copy link
Copy Markdown
Member

I can reproduce the problem now. I was running bun without the --bun flag, so it was running ESLint with Node.js I believe.

@mdjermanovic

Copy link
Copy Markdown
Member

Then, I'm not sure what the best course of action would be out of these:

  1. Revisit the parser API, since properties clash with Bun's.
  2. Update the parser to make properties enumerable and thus consistent across runtimes.
  3. Just make the fix proposed in this PR.

@nzakas what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Triaging

Development

Successfully merging this pull request may close these issues.

4 participants