+
Skip to content

Conversation

siketyan
Copy link
Member

Summary

Fixes #7256
Fixes #7320

Case 1 (#7256)

The rule used to remove a string literal expression that contains multiple whitespaces. Only one whitespace was allowed in #4154.

<>{'  '}</>

is now correctly ignored by the rule.

Case 2 (#7320)

The rule didn't detect a string attribute value with braces that contains >, ", ', or }. These special characters should not be allowed when unwrapping a literal within a JSX tag, but they should be allowed in attribute values.

<Foo bar={'"'}></Foo>

is now turned into:

<Foo bar='"'></Foo>

, which is a valid JSX syntax.

Test Plan

Added some snapshot tests.

Docs

N/A

@siketyan siketyan requested review from a team September 14, 2025 13:05
@siketyan siketyan self-assigned this Sep 14, 2025
Copy link

changeset-bot bot commented Sep 14, 2025

🦋 Changeset detected

Latest commit: c2a2df6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

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

@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Sep 14, 2025
Copy link
Contributor

coderabbitai bot commented Sep 14, 2025

Walkthrough

Updates the style/useConsistentCurlyBraces rule logic in use_consistent_curly_braces.rs: simplifies JSX attribute and child handling, unconditionally removes braces around string literals in attributes, adds braces for JSX tags without them, and broadens whitespace checks via a new contains_only_spaces helper (replacing contains_single_space). Adjusts forbidden character handling in JSX children. Adds tests in valid.jsx and invalid.jsx (including cases tied to issue #7320). Introduces two changesets for patch releases documenting these fixes. No exported/public API changes.

Suggested labels

A-Linter, L-JavaScript, A-Diagnostic

Suggested reviewers

  • arendjr
  • ematipico
  • dyc3

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly names the affected lint rule and the core change (adjusting the condition to allow removing braces), and it matches the code and tests in the changeset.
Linked Issues Check ✅ Passed The code changes address the linked coding objectives: whitespace handling for JSX children is tightened via contains_only_spaces (resolving [#7256]) and attribute unwrapping logic was adjusted so safe string literals (including quotes) are unwrapped while the forbidden-char guard remains for JSX children (resolving [#7320]); corresponding tests were added to cover these behaviours.
Out of Scope Changes Check ✅ Passed The changes are limited to the useConsistentCurlyBraces rule implementation, its tests, and the release changesets; there are no unrelated file or feature modifications in the provided summary.
Description Check ✅ Passed The description clearly explains the two bug fixes (cases for #7256 and #7320), gives concrete examples, notes added tests and aligns with the changes in the diff and changesets.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/GH-7320

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (5)
crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/valid.jsx (1)

25-26: Good addition: multi‑space literal is now valid.

Covers #7256 nicely.

Consider adding a quick NBSP and tab case to guard future regressions:

<Foo>{'\u00A0'}</Foo>
<Foo>{'\t'}</Foo>
.changeset/short-cases-think.md (1)

1-5: Polish the phrasing and add a tiny example.

Grammar nits and a minimal before/after help. Also prefer “whitespace” (uncountable).

Apply:

---
 "@biomejs/biome": patch
---
 
-Fixed [#7256](https://github.com/biomejs/biome/issues/7256): The [`useConsistentCurlyBraces`](https://biomejs.dev/linter/rules/use-consistent-curly-braces/) rule now correctly ignores a string literal with braces that contains only whitespaces. Previously, literals that contains single whitespace were only allowed.
+Fixed [#7256](https://github.com/biomejs/biome/issues/7256): The [`useConsistentCurlyBraces`](https://biomejs.dev/linter/rules/use-consistent-curly-braces/) rule now correctly ignores a string literal with braces that contains only whitespace. Previously, only a single space was allowed.
+
+```jsx
+// Before (no longer reported)
+<>{'  '}</>
+// After (unchanged)
+<>{'  '}</>
+```
.changeset/little-seals-taste.md (1)

1-5: End with a period and add a before/after.

Tiny punctuation fix plus a concrete example for #7320.

Apply:

---
 "@biomejs/biome": patch
---
 
-Fixed [#7320](https://github.com/biomejs/biome/issues/7320): The [`useConsistentCurlyBraces`](https://biomejs.dev/linter/rules/use-consistent-curly-braces/) rule now correctly detects a string literal including `"` inside a JSX attribute value.
+Fixed [#7320](https://github.com/biomejs/biome/issues/7320): The [`useConsistentCurlyBraces`](https://biomejs.dev/linter/rules/use-consistent-curly-braces/) rule now correctly detects a string literal including `"` inside a JSX attribute value.
+
+```jsx
+// Before
+<Foo bar={'"'}></Foo>
+// After
+<Foo bar='"'></Foo>
+```
crates/biome_js_analyze/src/lint/style/use_consistent_curly_braces.rs (2)

361-363: Comment no longer matches behaviour.

It now guards “only spaces” (not just a single space). Tweak the wording.

Apply:

-// Don't suggest removing braces for single space or if forbidden chars found
+// Don't suggest removing braces for spaces-only content or if forbidden chars are present

407-411: Broaden “spaces only” to “whitespace only”.

Tabs and other Unicode whitespace should also be treated as “whitespace‑only” children (consistent with the wording in #7256).

Apply:

-fn contains_only_spaces(literal: &JsStringLiteralExpression) -> bool {
-    literal
-        .inner_string_text()
-        .is_ok_and(|text| text.bytes().all(|b| b == b' '))
-}
+fn contains_only_spaces(literal: &JsStringLiteralExpression) -> bool {
+    // Treat any Unicode whitespace as whitespace-only.
+    literal
+        .inner_string_text()
+        .is_ok_and(|text| text.text().chars().all(|c| c.is_whitespace()))
+}
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2da976 and c2a2df6.

⛔ Files ignored due to path filters (2)
  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/valid.jsx.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (5)
  • .changeset/little-seals-taste.md (1 hunks)
  • .changeset/short-cases-think.md (1 hunks)
  • crates/biome_js_analyze/src/lint/style/use_consistent_curly_braces.rs (3 hunks)
  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx (1 hunks)
  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/valid.jsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/*.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

.changeset/*.md: In changeset files, only use #### or ##### headers; avoid other header levels
Changeset descriptions should use past tense for what you did (e.g., "Added...")
Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")
For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")
When referencing rules or assists in changesets, include links to their documentation pages
Include a minimal code block in the changeset when applicable to demonstrate the change
End every sentence in the changeset description with a period

Files:

  • .changeset/little-seals-taste.md
  • .changeset/short-cases-think.md
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}

Files:

  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx
  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/valid.jsx
  • crates/biome_js_analyze/src/lint/style/use_consistent_curly_braces.rs
crates/biome_*/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place core crates under /crates/biome_*/

Files:

  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx
  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/valid.jsx
  • crates/biome_js_analyze/src/lint/style/use_consistent_curly_braces.rs
**/tests/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place test files under a tests/ directory in each crate

Files:

  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx
  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/valid.jsx
**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Format all Rust source files before committing (just f)

Files:

  • crates/biome_js_analyze/src/lint/style/use_consistent_curly_braces.rs
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.517Z
Learning: Update documentation when features change; for rules/assists/options, use inline Rust rustdoc
📚 Learning: 2025-09-07T17:35:00.517Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.517Z
Learning: Applies to .changeset/*.md : Include a minimal code block in the changeset when applicable to demonstrate the change

Applied to files:

  • .changeset/little-seals-taste.md
📚 Learning: 2025-09-07T17:35:00.517Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.517Z
Learning: Applies to .changeset/*.md : For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")

Applied to files:

  • .changeset/little-seals-taste.md
📚 Learning: 2025-09-07T17:35:00.517Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.517Z
Learning: Applies to .changeset/*.md : Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")

Applied to files:

  • .changeset/short-cases-think.md
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : Snapshot test `.jsonc` files must contain an array of code snippets (strings); these run in script mode (no ESM syntax)

Applied to files:

  • crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Avoid deep indentation and panics; prefer `?`, `ok()?`, and combinators (`map`, `and_then`, `filter`) over nested `if let`/`unwrap`/`expect`

Applied to files:

  • crates/biome_js_analyze/src/lint/style/use_consistent_curly_braces.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Test (depot-windows-2022-16)
  • GitHub Check: Test (depot-ubuntu-24.04-arm-16)
  • GitHub Check: Check Dependencies
  • GitHub Check: Lint project (depot-windows-2022)
  • GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
  • GitHub Check: Documentation
  • GitHub Check: autofix
  • GitHub Check: Test Node.js API
🔇 Additional comments (2)
crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx (1)

16-17: Nice catch: attribute with a single space should unwrap.

This exercises the attribute path independently from children.

crates/biome_js_analyze/src/lint/style/use_consistent_curly_braces.rs (1)

342-344: Unwrapping string‑literal attributes unconditionally is correct.

Matches JSX semantics and fixes #7320 without over‑blocking on quotes or ‘>’/’}’.

Please ensure we have attribute‑focused snapshots for each of " ' > } (children remain blocked). See suggested test in invalid.jsx.

Comment on lines +18 to +28
{/* https://github.com/biomejs/biome/issues/7320 */}
<ConnectedFieldTemplate
description1=
'Upload a CSV file containing an "email" column, and optional "first_name" and "last_name" columns'
description2={
'Upload a CSV file containing an "email" column, and optional "first_name" and "last_name" columns'
}
className={
"d-block"
}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Expand attribute quote/brace cases to lock in #7320.

Add explicit props with ", ', >, and } to prove they unwrap safely in attributes.

Example to append here:

<Foo a={'"'} b={"'"} c={'>'} d={'}'} />

🏁 Script executed:

#!/bin/bash
# Check if we already cover the four attribute characters in specs
rg -n --glob 'crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/**/*' \
  -e "<[^>]+\\{'\\\"'\\}" -e "<[^>]+\\{\"'\"\\}" -e "<[^>]+\\{'>'\\}" -e "<[^>]+\\{'}'\\}"

Length of output: 131


🏁 Script executed:

#!/bin/bash
set -euo pipefail
dir='crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces'
if [ ! -d "$dir" ]; then
  echo "DIR_NOT_FOUND: $dir"
  exit 0
fi

python3 - <<'PY'
import os,sys
root='crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces'
patterns = ["{'\"'}", "{\"'\"}", "{'>'}", "{'}'}"]
exts = ('.js','.jsx','.ts','.tsx','.mjs','.cjs')
found = False
for dp,_,files in os.walk(root):
    for f in files:
        if not f.lower().endswith(exts):
            continue
        path = os.path.join(dp,f)
        try:
            with open(path,'r',encoding='utf-8') as fh:
                for i,line in enumerate(fh,1):
                    for p in patterns:
                        if p in line:
                            print(f"{path}:{i}:{line.rstrip()}")
                            found = True
        except Exception as e:
            print(f"ERR reading {path}: {e}", file=sys.stderr)
if not found:
    print("NO_MATCHES")
PY

Length of output: 227


Add attribute-level tests for quote/brace cases

valid.jsx already contains {'}'} and {'>'} as children (crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/valid.jsx:27,33). Still need explicit attribute props for all four characters — add to the useConsistentCurlyBraces spec (e.g. crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx:18–28):

<Foo a={'"'} b={"'"} c={'>'} d={'}'} />
🤖 Prompt for AI Agents
In
crates/biome_js_analyze/tests/specs/style/useConsistentCurlyBraces/invalid.jsx
around lines 18–28, add attribute-level test cases for the four special
characters so the rule sees quote/brace cases as props; specifically add a JSX
element with props a, b, c, d whose values are the characters double-quote,
single-quote, greater-than, and closing brace respectively (use JSX attribute
expressions where appropriate to represent these characters), placing it
alongside the existing ConnectedFieldTemplate example so the file contains
explicit attribute props for all four characters.

Copy link

codspeed-hq bot commented Sep 14, 2025

CodSpeed Performance Report

Merging #7497 will not alter performance

Comparing fix/GH-7320 (c2a2df6) with main (c015765)1

Summary

✅ 133 untouched

Footnotes

  1. No successful run was found on main (b2da976) during the generation of this report, so c015765 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@siketyan siketyan merged commit bd70f40 into main Sep 14, 2025
29 of 46 checks passed
@siketyan siketyan deleted the fix/GH-7320 branch September 14, 2025 13:39
@github-actions github-actions bot mentioned this pull request Sep 14, 2025
kedevked pushed a commit to kedevked/biome that referenced this pull request Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

💅 useConsistentCurlyBraces doesn't report prop={\n'string'\n} 💅 useConsistentCurlyBraces removes whitespace

2 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载