-
-
Notifications
You must be signed in to change notification settings - Fork 715
fix(lint/useConsistentCurlyBraces): adjust condition to allow removing the braces #7497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: c2a2df6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
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 |
WalkthroughUpdates the Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
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 |
There was a problem hiding this 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
⛔ 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.
{/* 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" | ||
} | ||
/> |
There was a problem hiding this comment.
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.
CodSpeed Performance ReportMerging #7497 will not alter performanceComparing Summary
Footnotes |
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.is now turned into:
, which is a valid JSX syntax.
Test Plan
Added some snapshot tests.
Docs
N/A