feat: support non-js languages #743
Conversation
🦋 Changeset detectedLatest commit: d208100 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe changes introduce improved compatibility for ESLint API changes by adding a ponyfill for location calculation, update the visitor logic for AST traversal, and add support for linting and formatting JSON files using Prettier. Test infrastructure is enhanced to cover JSON cases, and new test fixtures are added for validation. Changes
Sequence Diagram(s)sequenceDiagram
participant ESLint
participant PrettierPlugin
participant SourceCode
participant getLocFromIndex
ESLint->>PrettierPlugin: Lint file (JS/JSON)
PrettierPlugin->>SourceCode: Access AST and text
PrettierPlugin->>getLocFromIndex: Compute position for diff (index)
getLocFromIndex-->>PrettierPlugin: Return line/column position
PrettierPlugin-->>ESLint: Report formatting differences
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to ee09d11 in 1 minute and 26 seconds. Click for details.
- Reviewed
100lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. eslint-plugin-prettier.js:75
- Draft comment:
Bug in getLocFromOffset: When no newline precedes the offset (e.g. offset before first '\n'), the fallback computes a negative column. Consider returning {line: 0, column: offset, offset} if no newline exists. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. eslint-plugin-prettier.js:106
- Draft comment:
Performance note: In reportDifference, the lineIndexes are recalculated for each difference. Consider caching them if multiple differences occur in a file. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. eslint-plugin-prettier.js:204
- Draft comment:
Using sourceCode.ast.type for event registration meets the non-JS support goal but consider a fallback (e.g. to 'Program') if sourceCode.ast or its type is undefined. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_I5pOsGYiTfHNgfKO
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
eslint-plugin-prettier.js (2)
106-109: Avoid recomputinglineIndexesfor every difference
reportDifferencecan be invoked dozens of times for a single file.
[...sourceCode.text.matchAll(/\n/g)].map(m => m.index)re-scans the entire source each time, which is unnecessary work.Consider caching the array once per rule execution:
- const lineIndexes = [...sourceCode.text.matchAll(/\n/g)].map(match => match.index); + // cache on the first call – `WeakMap` avoids memory leaks between files + const lineIndexes = + getLocFromOffset._cache?.get(sourceCode) ?? + [...sourceCode.text.matchAll(/\n/g)].map(m => m.index); + + getLocFromOffset._cache = getLocFromOffset._cache ?? new WeakMap(); + getLocFromOffset._cache.set(sourceCode, lineIndexes);This turns the expensive regex scan into an O(1) lookup for subsequent fixes in the same file.
35-38: Type alias readabilityThe
RuleContextalias is hard to parse and triggers Prettier warnings. Extracting the helper type keeps the line length sane:- * @typedef {Parameters<Exclude<ESLint.Plugin['rules'], undefined>[string]['create']>[0]} RuleContext + * @typedef {Exclude<ESLint.Plugin['rules'], undefined>[string]['create']} _Create + * @typedef {Parameters<_Create>[0]} RuleContextPurely a readability / lint-noise win.
🧰 Tools
🪛 ESLint
[error] 35-35: Replace
Exclude<ESLint.Plugin['rules'],·undefined>[string]['create']with⏎·*···Exclude<ESLint.Plugin['rules'],·undefined>[string]['create']⏎·*·(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eslint-plugin-prettier.js(7 hunks)
🧰 Additional context used
🪛 ESLint
eslint-plugin-prettier.js
[error] 19-20: Delete ⏎·*
(prettier/prettier)
[error] 35-35: Replace Exclude<ESLint.Plugin['rules'],·undefined>[string]['create'] with ⏎·*···Exclude<ESLint.Plugin['rules'],·undefined>[string]['create']⏎·*·
(prettier/prettier)
[error] 74-74: Replace (acc,·lineStart,·lineNo)·=>·lineStart·<·offset·?·[lineStart,·lineNo]·:·acc,·[-1,·-1] with ⏎····(acc,·lineStart,·lineNo)·=>⏎······lineStart·<·offset·?·[lineStart,·lineNo]·:·acc,⏎····[-1,·-1],⏎··
(prettier/prettier)
[error] 78-78: Insert ;
(prettier/prettier)
[error] 104-104: Replace match·=>·match.index with ⏎····match·=>·match.index,⏎··
(prettier/prettier)
[error] 105-107: Replace ⏎····getLocFromOffset(lineIndexes,·index)⏎·· with ·getLocFromOffset(lineIndexes,·index)
(prettier/prettier)
🔇 Additional comments (1)
eslint-plugin-prettier.js (1)
203-205: Guard against missingast.typeon exotic parsersUsing
[sourceCode.ast.type]is elegant, but if a third-party parser returnsundefined(observed in some experimental plugins) the property name becomes the string"undefined", creating a visitor that never fires.- [sourceCode.ast.type](node) { + [sourceCode.ast?.type ?? 'Program'](node) {The fallback keeps existing behaviour while still supporting non-JS root nodes.
dotcarmen
force-pushed
the
non-js-lang-support
branch
2 times, most recently
from
a3e1740 to
cec9313
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
eslint-plugin-prettier.js (3)
105-109: Minor: early failure message could be clearerThrowing plain
Error('prettier only supports textual source code')will surface in ESLint output without context. Consider prefixing with the rule name so users immediately know where it originates, e.g.-throw new Error('prettier only supports textual source code'); +throw new Error('prettier/prettier: non-textual SourceCode is not supported');
110-113: Micro-optimisation / readability
[...text.matchAll(/\n/g)].map(m => m.index)allocates an intermediate array ofRegExpMatchArrays just to pluckindex. A simple loop avoids the extra objects:-const lineIndexes = [...sourceCode.text.matchAll(/\n/g)].map(match => match.index); +const lineIndexes = []; +for (let i = sourceCode.text.indexOf('\n'); i !== -1; i = sourceCode.text.indexOf('\n', i + 1)) { + lineIndexes.push(i); +}Not critical, but it shaves allocations for large files.
🧰 Tools
🪛 ESLint
[error] 110-110: Replace
match·=>·match.indexwith⏎····match·=>·match.index,⏎··(prettier/prettier)
[error] 111-113: Replace
⏎····getLocFromOffset(lineIndexes,·index)⏎··with·getLocFromOffset(lineIndexes,·index)(prettier/prettier)
30-38: Typedef readability
@typedef {Parameters<Exclude<ESLint.Plugin['rules'], undefined>[string]['create']>[0]} RuleContextis correct but hard to parse. Consider a helper
typealias (e.g. in a// region typingsblock) or an inline explanatory comment to aid maintainability.🧰 Tools
🪛 ESLint
[error] 37-37: Replace
Exclude<ESLint.Plugin['rules'],·undefined>[string]['create']with⏎·*···Exclude<ESLint.Plugin['rules'],·undefined>[string]['create']⏎·*·(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eslint-plugin-prettier.js(7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: dotcarmen
PR: prettier/eslint-plugin-prettier#743
File: eslint-plugin-prettier.js:70-85
Timestamp: 2025-06-14T15:02:13.203Z
Learning: In the getLocFromOffset function for eslint-plugin-prettier.js, the correct implementation uses `lineIndexes = [-1, 0, ...lineIndexes]` to create an array of line start positions, and calculates column as `offset - lineIndexes[line]` without additional offsets.
eslint-plugin-prettier.js (2)
Learnt from: dotcarmen
PR: prettier/eslint-plugin-prettier#743
File: eslint-plugin-prettier.js:70-85
Timestamp: 2025-06-14T15:02:13.203Z
Learning: In the getLocFromOffset function for eslint-plugin-prettier.js, the correct implementation uses `lineIndexes = [-1, 0, ...lineIndexes]` to create an array of line start positions, and calculates column as `offset - lineIndexes[line]` without additional offsets.
Learnt from: dotcarmen
PR: prettier/eslint-plugin-prettier#743
File: eslint-plugin-prettier.js:0-0
Timestamp: 2025-06-14T15:04:53.666Z
Learning: In the getLocFromOffset function for eslint-plugin-prettier.js, the correct implementation uses `const withSentinel = [-1, 0, ...lineIndexes]` to create an array of line start positions without variable shadowing, and calculates column as `offset - withSentinel[line]` without additional offsets.
🪛 ESLint
eslint-plugin-prettier.js
[error] 37-37: Replace Exclude<ESLint.Plugin['rules'],·undefined>[string]['create'] with ⏎·*···Exclude<ESLint.Plugin['rules'],·undefined>[string]['create']⏎·*·
(prettier/prettier)
[error] 110-110: Replace match·=>·match.index with ⏎····match·=>·match.index,⏎··
(prettier/prettier)
[error] 291-291: Insert ·
(prettier/prettier)
🔇 Additional comments (2)
eslint-plugin-prettier.js (2)
208-208: Dynamic visitor key may break on exotic root node namesUsing computed property
[sourceCode.ast.type]is elegant, but ESLint expects visitor keys to be valid object keys. If a plugin ever returns a root type containing characters like::(unlikely but possible), the visitor would silently fail. A defensive fallback keeps the classic"Program"visitor for unexpected names:- [sourceCode.ast.type](node) { + [sourceCode.ast.type || 'Program'](node) {
70-89:⚠️ Potential issueOff-by-one & line-number mis-calculation in
getLocFromOffset()The current sentinel logic
const withSentinel = [-1, 0, ...lineIndexes]; … const column = offset - withSentinel[line];shifts every line number by +1 and makes
columnreturn 1 instead of 0 for the very first character (offset === 0).
Example: for"abc\n"the offset0is reported as{ line: 1, column: 1 }, while ESLint expects{ line: 0, column: 0 }.A minimal patch that keeps the algorithm O(n) and fixes both issues:
-function getLocFromOffset(lineIndexes, offset) { - const withSentinel = [-1, 0, ...lineIndexes]; +function getLocFromOffset(lineIndexes, offset) { + // prepend sentinel so the first element describes “before char 0” + const withSentinel = [-1, ...lineIndexes]; let line = 0; while (line + 1 < withSentinel.length && withSentinel[line + 1] < offset) { line++; } - const column = offset - withSentinel[line]; + // subtract 1 because `withSentinel[0] === -1` + const column = offset - withSentinel[line] - 1; return { line, column, offset }; }This matches ESLint’s own
getLocFromIndexbehaviour for all edge-cases (start of file, empty file, offsets sitting on a\n, etc.).⛔ Skipped due to learnings
Learnt from: dotcarmen PR: prettier/eslint-plugin-prettier#743 File: eslint-plugin-prettier.js:0-0 Timestamp: 2025-06-14T15:04:53.666Z Learning: In the getLocFromOffset function for eslint-plugin-prettier.js, the correct implementation uses `const withSentinel = [-1, 0, ...lineIndexes]` to create an array of line start positions without variable shadowing, and calculates column as `offset - withSentinel[line]` without additional offsets.
dotcarmen
force-pushed
the
non-js-lang-support
branch
from
cec9313 to
8fd708c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
eslint-plugin-prettier.js (1)
110-112: Avoiddeletefor perf-sensitive hot-path
delete this.lineIndexesde-opts the object each time. Assigningundefined(or replacing the getter with a value property once) keeps the object in fast mode:- // @ts-ignore - delete this.lineIndexes; + // @ts-ignore + this.lineIndexes = undefined;Not critical, but worth it given this runs for every fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
eslint-plugin-prettier.js(6 hunks)package.json(1 hunks)test/invalid/json.txt(1 hunks)test/prettier.mjs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (1)
eslint-plugin-prettier.js (1)
Learnt from: dotcarmen
PR: prettier/eslint-plugin-prettier#743
File: eslint-plugin-prettier.js:0-0
Timestamp: 2025-06-14T15:04:53.678Z
Learning: In the getLocFromOffset function for eslint-plugin-prettier.js, the correct implementation uses `const withSentinel = [-1, 0, ...lineIndexes]` to create an array of line start positions without variable shadowing, and calculates column as `offset - withSentinel[line]` without additional offsets.
🪛 LanguageTool
test/invalid/json.txt
[typographical] ~20-~20: Nepárový symbol: zdá sa, že chýba '"'
Context: ...[] ERRORS: [ { message: 'Replace "a":·[⏎"b",⏎{"c":⏎"d"}⏎]· with `··"a":·["b",·{·"c...
(UNPAIRED_BRACKETS)
[typographical] ~20-~20: Nepárový symbol: zdá sa, že chýba '"'
Context: ...eplace "a":·[⏎"b",⏎{"c":⏎"d"}⏎]· with ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
[typographical] ~21-~21: Nepárový symbol: zdá sa, že chýba '{'
Context: ... with ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
[typographical] ~21-~21: Nepárový symbol: zdá sa, že chýba '['
Context: ...th ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
🪛 Biome (1.9.4)
eslint-plugin-prettier.js
[error] 110-110: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (2)
test/invalid/json.txt (1)
1-23: Fixture looks fine – no action requiredThe file is a plain text fixture for RuleTester; nothing strikes me as problematic.
test/prettier.mjs (1)
28-29: Double-checkRuleTesteroptions –languagemay be ignored
RuleTester(both Classic and Flat) acceptslanguageOptions, but a plainlanguageproperty is not yet in the public API. If the current ESLint version silently drops unknown keys, the JSON tests will never run with the intended parser, causing false positives/negatives.-const jsonRuleTester = new RuleTester({ - plugins: { - json: eslintPluginJson, - }, - language: 'json/json', -}); +const jsonRuleTester = new RuleTester({ + plugins: { json: eslintPluginJson }, + languageOptions: { parser: eslintPluginJson.parsers['json'] }, +});Please verify against the ESLint version in CI; adjust as needed.
Also applies to: 384-389
JounQin
force-pushed
the
non-js-lang-support
branch
2 times, most recently
from
7b39b75 to
5944dd5
Compare
commit: |
JounQin
force-pushed
the
non-js-lang-support
branch
3 times, most recently
from
3298e53 to
c4053cb
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
test/fixtures/empty.json (1)
1-2:empty.jsonis not valid JSON – Prettier will throwAn empty file is not valid JSON, and Prettier’s JSON parser (
json/jsonc) will raise a parse error.
If the intention is to test a minimal JSON file, use{}(or[]) instead and adjust the expected fix/assert accordingly.-⏎ +{}
♻️ Duplicate comments (1)
eslint-plugin-prettier.js (1)
78-94: Off-by-one in column calculation for every line after the first
lineIndexescurrently stores the position of the newline character.
For an offset that points to the first character of the next line,
column = index - lineIndexes[line]yields1instead of0, so all reported
locations are shifted right by one column on lines > 1.Fix by caching the start offset of every line instead:
- if (!lineIndexes) { - lineIndexes = [...sourceCode.text.matchAll(/\r?\n/g)].map( - match => match.index, - ); - // first line in the file starts at byte offset 0 - lineIndexes.unshift(0); + if (!lineIndexes) { + lineIndexes = [0]; + for (const match of sourceCode.text.matchAll(/\r?\n/g)) { + // jump past '\n' or '\r\n' to the first char of the next line + lineIndexes.push(match.index + match[0].length); + } }With true line-start offsets,
columnis correct for every line, and no other
logic needs to change.
🧹 Nitpick comments (1)
test/prettier.mjs (1)
386-404: Minor: fixture assertion hard-codes single newline deletionIf you switch
empty.jsonto{}as suggested above, remember to update this expected message/range.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.changeset/weak-lamps-refuse.md(1 hunks)eslint-plugin-prettier.js(4 hunks)package.json(1 hunks)test/fixtures/empty.json(1 hunks)test/invalid/json.txt(1 hunks)test/prettier.mjs(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
eslint-plugin-prettier.js (1)
Learnt from: dotcarmen
PR: prettier/eslint-plugin-prettier#743
File: eslint-plugin-prettier.js:0-0
Timestamp: 2025-06-14T15:04:53.678Z
Learning: In the getLocFromOffset function for eslint-plugin-prettier.js, the correct implementation uses `const withSentinel = [-1, 0, ...lineIndexes]` to create an array of line start positions without variable shadowing, and calculates column as `offset - withSentinel[line]` without additional offsets.
🪛 Biome (1.9.4)
test/fixtures/empty.json
[error] 1-1: Expected an array, an object, or a literal but instead found the end of the file.
Expected an array, an object, or a literal here.
(parse)
test/prettier.mjs
[error] 33-33: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🪛 LanguageTool
test/invalid/json.txt
[typographical] ~20-~20: Nepárový symbol: zdá sa, že chýba '"'
Context: ...[] ERRORS: [ { message: 'Replace "a":·[⏎"b",⏎{"c":⏎"d"}⏎]· with `··"a":·["b",·{·"c...
(UNPAIRED_BRACKETS)
[typographical] ~20-~20: Nepárový symbol: zdá sa, že chýba '"'
Context: ...eplace "a":·[⏎"b",⏎{"c":⏎"d"}⏎]· with ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
[typographical] ~21-~21: Nepárový symbol: zdá sa, že chýba '{'
Context: ... with ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
[typographical] ~21-~21: Nepárový symbol: zdá sa, že chýba '['
Context: ...th ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
🔇 Additional comments (7)
package.json (1)
76-79: Confirm@eslint/jsonversion compatibility
@eslint/jsonis added with the range^0.12.0. Please double-check that this version supports the newlanguage/pluginsconfiguration style you’re using intest/prettier.mjs; v0.13 introduced a few breaking tweaks.test/prettier.mjs (1)
556-563: ```shell
#!/bin/bashFind runFixture calls in test/prettier.mjs
echo "runFixture invocations in test/prettier.mjs:"
grep -R "runFixture" -n test/prettier.mjs || trueecho
List JSON fixtures under test/fixtures
echo "JSON fixture files under test/fixtures:"
fd test/fixtures -t f | grep -E ".json$" || echo "No .json fixtures found"</details> <details> <summary>test/invalid/json.txt (1)</summary> `20-22`: **Unpaired bracket warnings are safe to ignore** LanguageTool flags unmatched brackets in the message string, but the string is never parsed – only shown in test output – so no action required. </details> <details> <summary>.changeset/weak-lamps-refuse.md (1)</summary> `1-6`: **Changeset looks good** The changeset accurately summarises the feature addition and uses the correct semantic-release header. </details> <details> <summary>eslint-plugin-prettier.js (3)</summary> `61-63`: **Use of WeakMap for per-file caching looks great** Leveraging a `WeakMap<SourceCode, number[]>` avoids redundant scans while letting GC reclaim memory once ESLint drops the `SourceCode` object. Nice touch. --- `111-114`: **Unified location lookup path is clean** Passing both range indices through the new helper keeps the code DRY and shields call-sites from ESLint version differences. Looks good. --- `208-210`: **Dynamic visitor key: verify `:exit` parity** Using `[sourceCode.ast.type]` makes the rule fire for any root node, but the original implementation also listened on `'Program:exit'`. If any later logic implicitly relied on running after full traversal (e.g., respecting `/* eslint-disable */` ranges), consider registering both the root and its `:exit` counterpart: ```js return { [rootType]: visitor, [`${rootType}:exit`]: visitor, };Please double-check behaviour with JS and JSON fixtures.
JounQin
force-pushed
the
non-js-lang-support
branch
from
c4053cb to
d208100
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
eslint-plugin-prettier.js (1)
78-94: Off-by-one column calculation & CRLF mis-handling ingetLocFromIndex
lineIndexescurrently stores the position of the newline character.
For an offset that points to the first character of the next line the
column is computed as1(or2for\r\n) instead of0, e.g.:txt = "abc\nx" offset 4 -> expected {line:2, column:0} // current code → column:1Storing newline indices forces every column computation to juggle
+1
adjustments and breaks on Windows endings.
A simpler, always-correct approach is to cache line-START indices and
let the math fall out naturally:@@ - let lineIndexes = lineIndexesCache.get(sourceCode); - if (!lineIndexes) { - lineIndexes = [...sourceCode.text.matchAll(/\r?\n/g)].map( - match => match.index, - ); - // first line in the file starts at byte offset 0 - lineIndexes.unshift(0); - lineIndexesCache.set(sourceCode, lineIndexes); - } + let lineIndexes = lineIndexesCache.get(sourceCode); + if (!lineIndexes) { + lineIndexes = [0]; // first line start + for (const m of sourceCode.text.matchAll(/\r?\n/g)) { + lineIndexes.push(m.index + m[0].length); // char _after_ newline + } + lineIndexesCache.set(sourceCode, lineIndexes); + } @@ - const column = index - lineIndexes[line]; + const column = index - lineIndexes[line];With starts cached, the rest of the algorithm stays O(n) and the column
is always zero-based, no special-cases needed.
Please add a quick unit test for offsets immediately after\nand
\r\nto lock this down.
🧹 Nitpick comments (3)
test/prettier.mjs (2)
28-29: Missing treeshake safeguard for optional dependency
@eslint/jsonis added un-conditionally. If this file is executed in an environment where the dev dependency is not installed (e.g. consumers running the plugin’s test suite), the import will throw. Consider lazy-requiring inside the ESLint-9 block instead, mirroring how other optional plugins are handled.-import eslintPluginJson from '@eslint/json'; +let eslintPluginJson; +try { + // eslint-disable-next-line n/no-extraneous-require + eslintPluginJson = await import('@eslint/json').then(m => m.default ?? m); +} catch {}
35-36:isESLint9heuristic may mis-detect future versions
!eslintUnsupportedApi.FlatRuleTestercorrectly identifies ESLint 9 today, but will also trigger for any later version that re-introducesFlatRuleTester. Pinning to a semver range viaeslintPackage.versionprovides a future-proof gate.eslint-plugin-prettier.js (1)
208-210: Minor: give the root-visitor a more explicit nameUsing a computed key is 👍, but an anonymous key makes stack traces and
debugging harder. Consider:- [sourceCode.ast.type](node) { + /** Root node visitor – dynamic key for non-JS languages */ + [sourceCode.ast.type](rootNode) { …Small rename, big readability win.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.changeset/weak-lamps-refuse.md(1 hunks)eslint-plugin-prettier.js(4 hunks)package.json(1 hunks)test/fixtures/empty.json(1 hunks)test/invalid/json.txt(1 hunks)test/prettier.mjs(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
test/prettier.mjs (1)
Learnt from: JounQin
PR: prettier/eslint-plugin-prettier#743
File: test/prettier.mjs:28-35
Timestamp: 2025-06-17T07:50:43.733Z
Learning: In the eslint-plugin-prettier test file, the pattern `const ESLint = eslintUnsupportedApi.FlatESLint ?? eslint.ESLint;` works correctly even though `eslint` is declared later with `var eslint;` because the hoisted declaration makes `eslint` available throughout the file, and `eslintUnsupportedApi.FlatESLint` is always truthy when this code executes, so the `eslint.ESLint` fallback is never evaluated.
eslint-plugin-prettier.js (1)
Learnt from: dotcarmen
PR: prettier/eslint-plugin-prettier#743
File: eslint-plugin-prettier.js:0-0
Timestamp: 2025-06-14T15:04:53.678Z
Learning: In the getLocFromOffset function for eslint-plugin-prettier.js, the correct implementation uses `const withSentinel = [-1, 0, ...lineIndexes]` to create an array of line start positions without variable shadowing, and calculates column as `offset - withSentinel[line]` without additional offsets.
🪛 Biome (1.9.4)
test/fixtures/empty.json
[error] 1-1: Expected an array, an object, or a literal but instead found the end of the file.
Expected an array, an object, or a literal here.
(parse)
🪛 LanguageTool
test/invalid/json.txt
[typographical] ~20-~20: Nepárový symbol: zdá sa, že chýba '"'
Context: ...[] ERRORS: [ { message: 'Replace "a":·[⏎"b",⏎{"c":⏎"d"}⏎]· with `··"a":·["b",·{·"c...
(UNPAIRED_BRACKETS)
[typographical] ~20-~20: Nepárový symbol: zdá sa, že chýba '"'
Context: ...eplace "a":·[⏎"b",⏎{"c":⏎"d"}⏎]· with ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
[typographical] ~21-~21: Nepárový symbol: zdá sa, že chýba '{'
Context: ... with ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
[typographical] ~21-~21: Nepárový symbol: zdá sa, že chýba '['
Context: ...th ··"a":·["b",·{·"c":·"d"·}]⏎', }, ]
(UNPAIRED_BRACKETS)
🔇 Additional comments (6)
test/fixtures/empty.json (1)
1-2: Empty fixture accepted — just confirm the newline is intentionalThe file only contains a single newline. That is fine for a negative-test fixture as long as:
- The lone newline is indeed required by the corresponding assertion in
test/prettier.mjs.- No other tests rely on this file being valid JSON (Biome’s parse error is therefore expected).
No change needed.
package.json (1)
76-78:@eslint/jsondev-dependency looks correctAdding
@eslint/json@^0.12.0aligns with the new JSON test coverage. Nothing else appears to depend on a newer API surface, so the chosen range is adequate.If you end up using any functionality introduced after 0.12.0, bump the version accordingly.
.changeset/weak-lamps-refuse.md (1)
1-6: Clear and concise ChangesetThe summary accurately reflects the functional change and follows the repo’s conventions.
test/invalid/json.txt (1)
1-23: Fixture content looks coherentThe CODE / OUTPUT / ERRORS sections are internally consistent; LanguageTool’s bracket warnings are inside string literals and can be ignored.
test/prettier.mjs (2)
406-413: RuleTester initialisation: uselanguageOptionsinstead oflanguage
RuleTester(classic) expects alanguageOptionsobject rather than a top-levellanguagekey. Relying on the non-standardlanguageproperty may break when ESLint tightens typings.- const jsonRuleTester = new RuleTester({ - plugins: { - json: eslintPluginJson, - }, - language: 'json/json', - }); + const jsonRuleTester = new RuleTester({ + plugins: { json: eslintPluginJson }, + languageOptions: { parser: null, language: 'json/json' }, + });
557-562: JSON override lacks explicit parser – may fail if plugin is absentThe override registers the
jsonplugin but omits a parser orlanguagedeclaration. If ESLint cannot infer the parser from the plugin, parsing will fallback to espree and the lint run will error.Add the parser/ language explicitly to be safe:
{ files: ['**/*.json'], plugins: { json: eslintPluginJson, }, + languageOptions: { language: 'json/json' }, },
|
@dotcarmen Thanks for your contribution! |
| datasource | package | from | to | | ---------- | ---------------------- | ----- | ----- | | npm | eslint-plugin-prettier | 5.4.0 | 5.5.0 | ## [v5.5.0](https://github.com/prettier/eslint-plugin-prettier/blob/HEAD/CHANGELOG.md#550) ##### Minor Changes - [#743](prettier/eslint-plugin-prettier#743) [`92f2c9c`](prettier/eslint-plugin-prettier@92f2c9c) Thanks [@dotcarmen](https://github.com/dotcarmen)! - feat: support non-js languages like `css` for `@eslint/css` and `json` for `@eslint/json` ## [v5.4.1](https://github.com/prettier/eslint-plugin-prettier/blob/HEAD/CHANGELOG.md#541) ##### Patch Changes - [#740](prettier/eslint-plugin-prettier#740) [`c21521f`](prettier/eslint-plugin-prettier@c21521f) Thanks [@JounQin](https://github.com/JounQin)! - fix(deps): bump `synckit` to v0.11.7 to fix potential `TypeError: Cannot read properties of undefined (reading 'message')` error
| datasource | package | from | to | | ---------- | ---------------------- | ----- | ----- | | npm | eslint-plugin-prettier | 5.4.0 | 5.5.0 | ## [v5.5.0](https://github.com/prettier/eslint-plugin-prettier/blob/HEAD/CHANGELOG.md#550) ##### Minor Changes - [#743](prettier/eslint-plugin-prettier#743) [`92f2c9c`](prettier/eslint-plugin-prettier@92f2c9c) Thanks [@dotcarmen](https://github.com/dotcarmen)! - feat: support non-js languages like `css` for `@eslint/css` and `json` for `@eslint/json` ## [v5.4.1](https://github.com/prettier/eslint-plugin-prettier/blob/HEAD/CHANGELOG.md#541) ##### Patch Changes - [#740](prettier/eslint-plugin-prettier#740) [`c21521f`](prettier/eslint-plugin-prettier@c21521f) Thanks [@JounQin](https://github.com/JounQin)! - fix(deps): bump `synckit` to v0.11.7 to fix potential `TypeError: Cannot read properties of undefined (reading 'message')` error
close #312
This PR aims to support non-JS languages being linted with the
prettier/prettierruleCurrently, this plugin only runs on
Programnodes, which is typically the root node for languages that are processed into JS in ESLint. However, other eslint plugins may use a different root node type, such as@eslint/json'sDocumentnode typeThis PR expands support for other languages by making 2 changes:
Programnodes when instantiating the rule, the rule now matches the type of the root node that was parsed for the entire file.@eslint/js'sgetLocFromIndex, use a new local functiongetLocFromOffsetthat transforms a byte offset into an API-compatible location.The latter change was necessary because eg
@eslint/jsondoesn't include agetLocFromIndexmethod on itsSourceCodeclass.Important
This PR expands the
prettier/prettierESLint rule to support non-JS languages by matching the root node type and introducesgetLocFromOffsetto handle byte offsets.prettier/prettierrule to support non-JS languages by matching the root node type of the parsed file instead of onlyProgramnodes.getLocFromIndexwithgetLocFromOffsetto convert byte offsets to locations, supporting languages withoutgetLocFromIndex.getLocFromOffset()to calculate line and column from byte offset ineslint-plugin-prettier.js.reportDifference()to usegetLocFromOffset()instead ofgetLocFromIndex.[sourceCode.ast.type]instead ofProgramineslint-plugin-prettier.js.This description was created by
for ee09d11. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit