feat(linter): add rule noJsxLiterals
#7248
Conversation
🦋 Changeset detectedLatest commit: 1389129 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 |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds a new nursery lint rule Suggested reviewers
Nice work — tests and options look comprehensive; leave a biscuit for the CI. ✨ Finishing Touches🧪 Generate unit tests
Comment |
github-actions
bot
added
A-Project
CodSpeed Performance ReportMerging #7248 will not alter performanceComparing Summary
Footnotes |
ematipico
force-pushed
the
feat/no-literals
branch
from
69f1a77 to
3bb1f14
Compare
![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 (15)
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.json (1)
6-11: noStrings wiring looks correct; add a precedence testJSON shape matches the serde camelCase options (noStrings → no_strings). To harden behaviour, consider adding a test where both noStrings: true and allowedStrings are set to confirm that noStrings takes precedence.
Example new options fixture (place alongside the others):
{ "$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json", "linter": { "rules": { "nursery": { "noJsxLiterals": { "level": "on", "options": { "noStrings": true, "allowedStrings": ["allowed"] } } } } } }And a matching invalid fixture (e.g., invalidNoStringsWithAllowlist.jsx):
<> {/* still diagnostic because noStrings trumps allowlist */} <div>allowed</div> </>crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx (1)
1-5: Broaden attribute shapes to harden ignoreProps coverageAdd a couple more common patterns (data-*, className, template literal) to ensure all attribute string forms are ignored.
<> <Foo bar={"allowed"} /> <Foo bar="allowed"></Foo> + <Foo data-testid="allowed" /> + <Foo className={'allowed'} /> + <Foo bar={`allowed`} /> </>crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.json (1)
6-11: allowedStrings wiring looks good; consider matching edge casesNice and tidy. For completeness, add tests to verify exact, case-sensitive matching and whitespace handling (e.g., "allowed" vs "Allowed" vs " allowed "). This catches surprising false positives.
Happy to draft valid/invalid fixtures demonstrating exact-match behaviour across JSX text and props if helpful.
.changeset/nine-turkeys-dig.md (2)
5-5: Tiny grammar fix: subject–verb agreement.Change “disallow” → “disallows”.
-Added the new nursery lint rule `noJsxLiterals`, which disallow the use of string literals inside JSX. +Added the new nursery lint rule `noJsxLiterals`, which disallows the use of string literals inside JSX.
7-20: Polish the changeset: full stops, options, and a config snippet.
- Per our changeset style, end sentences with a period.
- Briefly document the rule options and show representative configuration. This keeps the entry user‑facing and actionable.
-The rule catches these cases: +The rule catches these cases. @@ </>
+#### Options.
+
+-noStrings: Also flag string literals inside JSX expressions and attribute values.
+-allowedStrings: Permit specific strings without diagnostics.
+-ignoreProps: Skip checks for JSX prop values.
+
+##### Configuration.
+
+```jsonc
+{
- "linter": {
- "rules": {
"nursery": {"noJsxLiterals": "on"}- }
- }
+}
+```+```jsonc
+{
- "linter": {
- "rules": {
"nursery": {"noJsxLiterals": {"level": "error","options": {"noStrings": true,"allowedStrings": ["OK", "Cancel"],"ignoreProps": false}}}- }
- }
+}
+```</blockquote></details> <details> <summary>crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx (1)</summary><blockquote> `1-12`: **Broaden “allowedStrings” coverage with template literals.** Add a couple of template‑literal shapes so we exercise normalisation across AST forms. ```diff <Foo bar={"allowed"} /> <Foo bar="allowed"> {'allowed' + name} </Foo> + <div>{`allowed`}</div> + <Foo bar={`allowed`} /> </>crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx (1)
12-20: Add DOM prop and template‑with‑literal parts undernoStrings.Two extra shapes commonly slip through: DOM attributes and templates containing a static chunk.
<Foo bar={`${baz}`} /> <Foo bar={"first" + "second"} /> <Foo bar={"third" + binding} /> <Foo bar={binding + "fourth"} /> + <div title="test" /> + <Foo bar={`${baz} test`} /> + <Foo bar={`test ${binding}`} /> </>crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsx (1)
12-14: Optionally assert whitespace‑only text is ignored.A minimal whitespace JSXText ensures the “trim and skip whitespace‑only” path stays intact.
<Foo bar={`${baz} test`} /> <div>{translate('my.translation.key')}</div> + <div> </div> </>crates/biome_rule_options/src/no_jsx_literals.rs (3)
3-17: Option docs: alignno_stringsdescription with actual behaviour.The rule uses
no_stringsto extend checks into JSX expressions and attributes, not “JSX children” only. Please tweak the doc comment to avoid confusion and match the rule docs in the lint file.Apply this diff:
- /// When enabled, no strings are allowed inside JSX children + /// When enabled, also flag string literals inside JSX expressions and attributes
3-17: Consistency: either rely on struct-level#[serde(default)]or annotate all fields.You’re mixing struct-level default with per-field defaults. It’s harmless, but mildly inconsistent. Consider removing the field-level
#[serde(default)]s (keeping only the struct-level one), or add it toignore_propsas well for symmetry.Possible tidy-up:
- #[serde(default)] - pub no_strings: bool, + pub no_strings: bool, @@ - #[serde(default)] - pub allowed_strings: Box<[Box<str>]>, + pub allowed_strings: Box<[Box<str>]>, @@ - /// When enabled, strings inside props are always ignored - pub ignore_props: bool, + /// When enabled, strings inside props are always ignored + pub ignore_props: bool,Alternatively, keep all three with
#[serde(default)]for clarity.
11-13: Data structure choice forallowed_strings.
Box<[Box<str>]>works, butVec<String>(orBox<[String]>) is more idiomatic and friendlier with serde tooling and future transforms (e.g. building aHashSet). If you expect small lists and care about footprint, keep as-is; otherwise consider switching.crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (4)
223-228: Visibility ofAnyJsxText.Do we need this union to be
pub? If it’s only used by this rule, prefer crate-private to reduce API surface.-declare_node_union! { - pub AnyJsxText = JsxText +declare_node_union! { + AnyJsxText = JsxText | JsStringLiteralExpression | JsxString | JsxExpressionAttributeValue }
204-219: Diagnostic wording: make it actionable and aligned with the rule name.Current message is a bit generic. Consider mirroring the rule name and suggesting the alternative.
- markup! { - "Incorrect use of string literal detected." - }, + markup! { + "Avoid string literals in JSX." + }, @@ - .note(markup! { - "Consider avoiding hardcoded strings entirely." - }), + .note(markup! { + "Move text into variables, translations, or constants (e.g. {'Hello'} or {t('hello')})." + }),
57-119: Docs/examples: great coverage. One small addition would help.Add an example showing
allowedStringsworking in attributes and expressions once the normalisation fix lands, e.g.<Component title={'Hello'} />and<Component title="Hello" />.
145-153: Attribute ancestry check is fine; minor micro-optimisation optional.
ancestors().skip(1).any(JsxAttribute::can_cast)is clear. You could bail on the firstJsxAttributewithfind_mapto avoid scanning further, but it’s micro.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (10)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (12)
.changeset/nine-turkeys-dig.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.json(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_jsx_literals.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_jsx_literals.rscrates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_rule_options/src/lib.rscrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsxcrates/biome_rule_options/src/no_jsx_literals.rscrates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
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/nursery/noJsxLiterals/validAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsxcrates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: Create changesets withjust new-changeset; store them in.changeset/with correct frontmatter (package keys and change type).
In changeset descriptions, follow content conventions: user-facing changes only; past tense for what you did; present tense for current behavior; link issues for fixes; link rules/assists; include representative code blocks; end every sentence with a period.
When adding headers in a changeset, only use #### or ##### levels.
Files:
.changeset/nine-turkeys-dig.md
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.json.changeset/nine-turkeys-dig.mdcrates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options in the `biome_rule_options` crate under `lib/`, with serde- and schemars-compatible derives and `#[serde(rename_all = "camelCase", deny_unknown_fields, default)]`
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_jsx_literals.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : `.jsonc` snapshot files must contain an array of code strings; they are interpreted in script mode (no ESM syntax)
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/specs/html/**/options.json : When non-default formatting options are needed for a test group, place an options.json (biome.json format) alongside the .html files in that folder
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.json
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
PR: biomejs/biome#7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.json
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (1)
crates/biome_js_syntax/src/expr_ext.rs (1)
value_token(1455-1464)
⏰ 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). (18)
- GitHub Check: Test Node.js API
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: autofix
🔇 Additional comments (8)
crates/biome_rule_options/src/lib.rs (1)
114-114: Good addition: options module export for no_jsx_literalsAlphabetical placement looks right (after no_irregular_whitespace, before no_label_var) and the snake_case name matches the new rule/options file. All good from my side.
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.json (1)
6-11: ignoreProps option mapped properlyCamel-cased key aligns with ignore_props in Rust; this should exercise attribute-value skipping as intended.
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx (1)
1-12: Verified: paired options file is present and correctThe
validAllowedStrings.options.jsonfixture indeed contains"allowedStrings": ["allowed"]at line 9, so the tests will stay green.
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx (1)
1-20: Great spread of cases.Nice mix of raw JSXText, string expressions, concatenations, and template usage. This should catch regressions in matching logic.
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsx (1)
1-14: Looks sensible for default behaviour.This aligns with a default that flags raw JSXText but not string expressions or props. The translate() call is a nice touch.
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsx (2)
1-20: Good negative coverage for raw JSXText.The nested fragment and multi‑line JSXText cases will exercise token trimming and range reporting.
15-20: Nice: object literal value with JSX.Covers a less common embedding that can break traversal if visitors are too narrow.
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (1)
169-185: Template literals with an interpolation are incorrectly flagged whennoStringsis true.The branch flags template expressions whenever
elements().len() <= 1. This will still catch templates with a single interpolation (e.g.`Hello ${name}`), which should be allowed. Only pure templates without interpolations should be flagged.Apply this diff:
- AnyJsExpression::JsTemplateExpression(expression) => { - return if expression.elements().len() <= 1 { - Some(expression.range()) - } else { - None - }; - } + AnyJsExpression::JsTemplateExpression(expression) => { + // Flag only fully static template literals (no interpolations) + return if expression.elements().len() == 0 { + Some(expression.range()) + } else { + None + }; + }Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (1)
191-199: Resolved: allowedStrings now works for quoted/JSX strings via inner_string_text.The past concern about comparing against raw token text (including quotes) is fixed by using
inner_string_text. Good catch and resolution.
🧹 Nitpick comments (4)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (4)
191-199: Avoid recomputing inner string; take once and reuse.Tiny micro-optimisation and improves readability.
Apply this diff:
- for allowed_string in &options.allowed_strings { - if inner_string_text(&value_token) == allowed_string.as_ref() { - return None; - } - } - - if inner_string_text(&value_token).trim().is_empty() { - return None; - } + let content = inner_string_text(&value_token); + + for allowed_string in &options.allowed_strings { + if content == allowed_string.as_ref() { + return None; + } + } + + if content.trim().is_empty() { + return None; + }
204-221: Sharpen the diagnostic message; hint at actionable alternatives.Current copy is generic. Suggest nudging towards i18n/variables and clarifying the option gate.
Apply this diff:
- RuleDiagnostic::new( + RuleDiagnostic::new( rule_category!(), state, markup! { - "Incorrect use of string literal detected." + "Avoid hardcoded string literals in JSX." }, ) .note(markup! { - "String literals in JSX can make code harder to maintain and internationalize." + "Hardcoded strings hinder maintainability and internationalisation." }) .note(markup! { - "Consider avoiding hardcoded strings entirely." + "Consider using variables or translation helpers (e.g., i18n). Enable the `noStrings` option to also flag strings inside expressions/props." }),
223-228: Scope the node union if not intended as public API.If
AnyJsxTextis only used internally by this rule, preferpub(crate)to avoid expanding the public surface. If it’s purposely public, ignore me and carry on.Apply this diff if internal-only:
-declare_node_union! { - pub AnyJsxText = JsxText +declare_node_union! { + pub(crate) AnyJsxText = JsxText | JsStringLiteralExpression | JsxString | JsxExpressionAttributeValue }
175-189: Prefer robust interpolation detection for template literals and allow‐list static templatesThe current check using
elements().len() <= 1can misclassify templates if empty quasis aren’t represented. Instead, detect interpolations directly and treat purely static templates like string literals:• Replace
- if expression.elements().len() <= 1 { + if !expression + .elements() + .into_iter() + .any(|el| el.as_js_template_element().is_some()) {or equivalently
if expression .elements() .into_iter() .all(|el| el.as_js_template_chunk_element().is_some())to catch only templates without
${…}.• For static templates, reconstruct the raw text by folding over
JsTemplateChunkElements and compare againstallowedStrings, ignoring whitespace‐only values just as with quoted literals.• Preserve the existing diagnostic range (
expression.range()) for flagged templates.Suggested tests:
- invalidNoStringsStaticTemplate.jsx:
<Comp title={Hello} />
withnoStrings: true— should be allowed when"Hello"is inallowedStrings.- validNoStringsInterpolatedTemplate.jsx:
<div>{Hello ${name}}</div>— always valid, since it contains interpolation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
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/src/lint/nursery/no_jsx_literals.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (2)
crates/biome_analyze/src/rule.rs (3)
recommended(538-541)sources(553-556)same(233-238)crates/biome_js_syntax/src/expr_ext.rs (1)
value_token(1455-1464)
⏰ 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). (25)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: autofix
- GitHub Check: Check JS Files
🔇 Additional comments (3)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (3)
121-128: Rule metadata looks solid (version, language, source mapping).
- version = "next" matches our guidelines.
- language = "jsx" plus runtime gating is good.
- Source mapped to eslint-plugin-react with
same()is tidy.
136-141: Good early exit for non-JSX files.Cheap and effective filter via
ctx.source_type::<JsFileSource>().is_jsx(). Saves pointless work. Nice.
145-153: Scopeignore_propsper JSX variant to avoid skipping nested render-prop JSXI wasn’t able to locate the broad
ancestors().any(JsxAttribute::can_cast)guard incrates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs(around lines 145–153), so please verify you’ve:
- Removed the ancestor-based check that returns
Nonewhenever anyJsxAttributeappears above the node- Moved
ignore_propshandling into eachAnyJsxTextmatch arm, so it only silences direct attribute valuesAnd don’t forget to add tests for:
validIgnorePropsNested.jsx:<Comp render={() => <>Hello</>} />should still report withignoreProps: truevalidIgnorePropsDirect.jsx:<Comp title="Hello" />should not report withignoreProps: trueCheers, and thanks for double-checking!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx (1)
3-12: Consider adding a JSXText case to harden coverageAt the moment all children are expression containers. Adding a plain JSX text node ensures allowedStrings also covers JSXText under noStrings: true.
Proposed tweak:
<> <div> {'allowed'} {'allowed'} {'allowed'} </div> + <p>allowed</p> <Foo bar={"allowed"} /> <Foo bar="allowed"> {'allowed' + name} </Foo> </>Optionally, add a separate fixture to cover template literals (e.g. {
allowed ${name}}) if the rule is meant to permit allowed substrings within interpolations. Happy to draft it.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (5)
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.options.json
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx
🧰 Additional context used
📓 Path-based instructions (3)
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/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Place all new rules in the nursery group (implement new rule files under a `nursery` directory)
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Document rules with a one-line first paragraph, `## Examples` with `### Invalid` (first) and `### Valid`, and document options under `## Options` with properly marked code blocks
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule is deprecated, set the `deprecated:` field in `declare_lint_rule!` with a reason and alternative
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : `.jsonc` snapshot files must contain an array of code strings; they are interpreted in script mode (no ESM syntax)
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.json
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/specs/html/**/options.json : When non-default formatting options are needed for a test group, place an options.json (biome.json format) alongside the .html files in that folder
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.json
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
PR: biomejs/biome#7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.json
⏰ 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). (26)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: End-to-end tests
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Check JS Files
- GitHub Check: autofix
🔇 Additional comments (3)
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.json (2)
7-12: No action needed—‘level’: “on” is supported and widely usedA quick ripgrep sweep shows dozens of spec files (JS, JSON, CSS, etc.) using
"level": "on", so it’s the established convention and won’t trigger schema errors. Leave it as is.
2-2: Ignore the off-by-one nit
The file sits six levels deep (crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals→ depth 6), so six../correctly hops back to the repo root. Adding a seventh would overshoot the root and break the path.Likely an incorrect or invalid review comment.
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx (1)
1-12: Nice coverage for allowed strings across children and propsThe fixture exercises {'allowed'} repeats, prop string in both forms, and concatenation with a name. Reads well and matches the options intent.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (1)
196-206: Nice fix: inner_string_text resolves the “quotes vs allowedStrings” mismatch.This normalisation correctly aligns allowedStrings/whitespace checks across JsxText and quoted literals. Good catch addressing the earlier pitfall.
🧹 Nitpick comments (6)
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.json (1)
1-16: Config looks correct; optional hardening by fixing intent in-place.The schema path resolves correctly from this folder, the rule is enabled, and options match the test intent. To make the test resilient if defaults change later, consider explicitly setting ignoreProps: false.
Apply this small diff:
"options": { "noStrings": true, - "allowedStrings": ["allowed"] + "allowedStrings": ["allowed"], + "ignoreProps": false }crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx (2)
2-7: Add a prop case to exercise noStrings + allowedStrings in attributes.This fixture currently checks children and expressions, but not attribute values, which are handled specially in the rule. Adding one line will cover that path and ensure allowedStrings applies there too.
Suggested insertion:
<> <div>allowed</div> <div>allowed </div> <div>{"allowed "}</div> + <Component title="allowed" description="definitely flagged" /> <>test</> </>
10-15: Consider also asserting allowedStrings works with string literals in expressions.Since noStrings is true here, a quoted literal inside an expression should be permitted when it matches allowedStrings exactly. Adding one example makes this explicit.
Suggested addition:
class Comp1 extends Component { render() { const varObjectTest = { testKey : (<div>test</div>) }; + const ok = <div>{'allowed'}</div>; return varObjectTest.testKey; } }crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (3)
175-193: Static template literals bypass allowedStrings/whitespace allowances.In the JsTemplateExpression branch, static templates (no interpolations or a single element) are always flagged, even if their content matches allowedStrings or is whitespace-only. For parity with JsxText/Js[String|x] cases, consider applying the same allowlist and whitespace checks to static templates. If this is intentional, please document that allowedStrings does not apply to template literals.
Happy to help wire a helper that extracts the cooked text for static templates and reuses the existing checks; shout if you want me to sketch it with the available syntax APIs.
209-225: Diagnostic phrasing: suggest a more directive message.“Incorrect use of string literal detected.” is a bit vague. Consider guiding the user toward the intended alternative.
Example tweak:
- "Incorrect use of string literal detected." + "Avoid hard-coded string literals in JSX; wrap in an expression or externalise the text."
126-133: Add a React domain to NoJsxLiterals
Since most nursery rules already declare domains, scopingnoJsxLiteralsto React will keep it consistent and enable curated rule sets.Locations to update:
- crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (lines 126–133)
Suggested diff:
pub NoJsxLiterals { version: "next", name: "noJsxLiterals", language: "jsx", + domains: &[RuleDomain::React], recommended: false, sources: &[RuleSource::EslintReact("jsx-no-literals").same()], }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (7)
.changeset/nine-turkeys-dig.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx(1 hunks)crates/biome_rule_options/src/no_jsx_literals.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx
- crates/biome_rule_options/src/no_jsx_literals.rs
- .changeset/nine-turkeys-dig.md
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx
🧰 Additional context used
📓 Path-based instructions (4)
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/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsxcrates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsxcrates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Use `domains` in `declare_lint_rule!` when applicable; recommended rules with domains enable only when the domain is active
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule provides a code action, add `fix_kind` in `declare_lint_rule!` and use `ctx.action_category(ctx.category(), ctx.group())` and `ctx.metadata().applicability()` when constructing actions
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Place all new rules in the nursery group (implement new rule files under a `nursery` directory)
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : Document rules with a one-line first paragraph, `## Examples` with `### Invalid` (first) and `### Valid`, and document options under `## Options` with properly marked code blocks
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : If a rule is deprecated, set the `deprecated:` field in `declare_lint_rule!` with a reason and alternative
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/*.jsonc : `.jsonc` snapshot files must contain an array of code strings; they are interpreted in script mode (no ESM syntax)
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/specs/html/**/options.json : When non-default formatting options are needed for a test group, place an options.json (biome.json format) alongside the .html files in that folder
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.json
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : For new JavaScript lint rules generated by `just new-js-lintrule`, implement the rule in the generated file under `biome_js_analyze/lib/src/lint/nursery/`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsxcrates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/tests/specs/**/{invalid*,valid*}.{js,jsx,ts,tsx,css,graphql,jsonc} : Place snapshot test cases under `tests/specs/<group>/<ruleName>/` using files prefixed with `invalid` and `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.jsoncrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
PR: biomejs/biome#7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.json
📚 Learning: 2025-08-17T08:56:30.831Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.831Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/nursery/**/*.rs : In `declare_lint_rule!` declarations, set `version: "next"`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (2)
crates/biome_analyze/src/rule.rs (1)
same(233-238)crates/biome_js_syntax/src/expr_ext.rs (1)
value_token(1455-1464)
⏰ 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). (24)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: autofix
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_css_parser)
🔇 Additional comments (2)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (2)
12-133: Docs and metadata read well; version set to "next" and sources mapped.Good structure: nursery placement, clear Invalid/Valid/Options sections, and the ESLint source reference. Nice touch restricting by language: "jsx".
150-158: Confirm broad JSX prop-ignore scope
- The
ancestors().skip(1).any(|n| JsxAttribute::can_cast(n.kind()))check at no_jsx_literals.rs:150–158 effectively ignores any nested JSX attribute contexts underignore_props, including deeply nested expressions.- A ripgrep search found no other rules using this exact ancestor pattern in
crates/biome_js_analyze/src/lint, so this is the sole occurrence.Please manually verify that this broad prop-level ignore aligns with the intended design. If you need to narrow the scope (e.g. only top-level props), consider refining the ancestor predicate.
ematipico
force-pushed
the
feat/no-literals
branch
from
f7c63e3 to
bde3c41
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx (1)
2-7: LGTM: good coverage of prop syntaxes.Covers quoted, expression, template, and common attrs. Consider adding one negative child case to prove text nodes are still flagged when ignoreProps is true.
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx (2)
2-5: LGTM: allowedStrings for JSX text.Solid. To harden parity, add one attribute case gated by noStrings: e.g., under options { noStrings: true, allowedStrings: ["allowed"] }.
8-13: Optional: exercise allowedStrings inside expression children.Add {'allowed'} to ensure literal-expr path also respects allowedStrings.
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (3)
196-204: Micro: avoid recomputing inner_string_text.Store once to reduce repeated work and keep semantics clear.
- for allowed_string in &options.allowed_strings { - if inner_string_text(&value_token) == allowed_string.as_ref() { - return None; - } - } - - if inner_string_text(&value_token).trim().is_empty() { + let content = inner_string_text(&value_token); + for allowed_string in &options.allowed_strings { + if content == allowed_string.as_ref() { + return None; + } + } + + if content.trim().is_empty() { return None; }
150-158: Scope check: ignoreProps currently ignores nested strings inside props.Because the ancestor check targets JsxAttribute, strings nested within objects (e.g., style={{ color: "red" }}) are also ignored. If the intention is to ignore only direct prop string values, this is a bit broad—worth clarifying and adding a test either way.
215-223: Polish diagnostic message.Make it specific and action-oriented.
- "Incorrect use of string literal detected." + "Unexpected string literal in JSX."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**
📒 Files selected for processing (18)
.changeset/nine-turkeys-dig.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.json(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx(1 hunks)crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.json(1 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/no_jsx_literals.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.options.json
🚧 Files skipped from review as they are similar to previous changes (14)
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.options.json
- .changeset/nine-turkeys-dig.md
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.options.json
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.options.json
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.options.json
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/valid.jsx
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStringsAllowedStrings.jsx
- crates/biome_rule_options/src/lib.rs
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.options.json
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validNoStringWithAllowedStrings.jsx
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidAllowedStrings.jsx
- crates/biome_rule_options/src/no_jsx_literals.rs
- crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsx
🧰 Additional context used
📓 Path-based instructions (4)
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/src/lint/nursery/no_jsx_literals.rscrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rscrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validIgnoreProps.jsxcrates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/**/*.rs : Place all new rules in the nursery group under biome_js_analyze/lib/src/lint/nursery
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Set the language field in declare_lint_rule! to the most specific applicable language (js/jsx/ts/tsx)
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : When deprecating a rule, add deprecated: "<reason>" to declare_lint_rule!
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : When porting a lint rule, set sources in declare_lint_rule! (RuleSource::Eslint(...).same() or .inspired())
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : In declare_lint_rule! metadata, set version: "next"
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/nursery/** : Place snapshot test cases for new rules under tests/specs/nursery/<ruleName>/ with files prefixed by invalid/valid
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Set the language field in declare_lint_rule! to the most specific applicable language (js/jsx/ts/tsx)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/**/*.rs : Place all new rules in the nursery group under biome_js_analyze/lib/src/lint/nursery
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : In declare_lint_rule! metadata, set version: "next"
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : When porting a lint rule, set sources in declare_lint_rule! (RuleSource::Eslint(...).same() or .inspired())
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : When deprecating a rule, add deprecated: "<reason>" to declare_lint_rule!
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Avoid unnecessary string allocations in rules; compare against &str or TokenText instead of calling to_string()
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : If a rule provides a code action, declare fix_kind in declare_lint_rule!
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/nursery/** : Place snapshot test cases for new rules under tests/specs/nursery/<ruleName>/ with files prefixed by invalid/valid
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/validAllowedStrings.jsx
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (2)
crates/biome_analyze/src/rule.rs (3)
recommended(554-557)sources(569-572)same(241-246)crates/biome_js_syntax/src/expr_ext.rs (1)
value_token(1547-1556)
⏰ 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). (21)
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Documentation
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: autofix
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_css_parser)
🔇 Additional comments (2)
crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs (2)
126-133: Rule metadata looks correct.version: "next", language: "jsx", and RuleSource mapping are spot on.
183-189: Static template literals bypass allowedStrings/whitespace checks
Early-returning single-element templates ({foo}) skips bothallowedStringsand empty/whitespace tests. ESLint’sjsx-no-literals(noStrings) treats`foo`identically to'foo'. To match that behavior, replace the early-return for templates inno_jsx_literals.rs(lines 183–189) with logic that:
- Uses
element.as_js_template_chunk_element().template_chunk_token()?.text_trimmed()- Runs the chunk text through
allowedStringsand whitespace skips- Returns
Some(range)only if the literal isn’t allowedPlease confirm whether static templates should be subjected to the same allowlist/whitespace checks as string literals.
| let value_token = match node { | ||
| AnyJsxText::JsxText(text) => text.value_token().ok()?, | ||
| AnyJsxText::JsStringLiteralExpression(expression) => { | ||
| if !options.no_strings { | ||
| return None; | ||
| } | ||
| expression.value_token().ok()? | ||
| } | ||
| AnyJsxText::JsxString(string) => { | ||
| if !options.no_strings { | ||
| return None; | ||
| } | ||
| string.value_token().ok()? | ||
| } | ||
| AnyJsxText::JsxExpressionAttributeValue(expression) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bug: noStrings flags string literals outside JSX.
Query includes JsStringLiteralExpression globally. Without an ancestor guard, any string literal in a JSX/TSX file is flagged when noStrings is true. Gate it to JSX contexts (attribute values or expression children).
Apply this diff:
@@
- AnyJsxText::JsStringLiteralExpression(expression) => {
+ AnyJsxText::JsStringLiteralExpression(expression) => {
if !options.no_strings {
return None;
}
+ // Ensure the literal is inside JSX (attribute value or expression child)
+ let in_jsx = expression
+ .syntax()
+ .ancestors()
+ .any(|n| JsxExpressionAttributeValue::can_cast(n.kind()) || JsxExpressionChild::can_cast(n.kind()));
+ if !in_jsx {
+ return None;
+ }
expression.value_token().ok()?
}
@@
AnyJsxText::JsxString(string) => {
if !options.no_strings {
return None;
}
string.value_token().ok()?
}And add the missing import:
@@
-use biome_js_syntax::{
- AnyJsExpression, AnyJsLiteralExpression, JsFileSource, JsStringLiteralExpression, JsxAttribute,
- JsxExpressionAttributeValue, JsxString, JsxText, inner_string_text,
-};
+use biome_js_syntax::{
+ AnyJsExpression, AnyJsLiteralExpression, JsFileSource, JsStringLiteralExpression, JsxAttribute,
+ JsxExpressionAttributeValue, JsxExpressionChild, JsxString, JsxText, inner_string_text,
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let value_token = match node { | |
| AnyJsxText::JsxText(text) => text.value_token().ok()?, | |
| AnyJsxText::JsStringLiteralExpression(expression) => { | |
| if !options.no_strings { | |
| return None; | |
| } | |
| expression.value_token().ok()? | |
| } | |
| AnyJsxText::JsxString(string) => { | |
| if !options.no_strings { | |
| return None; | |
| } | |
| string.value_token().ok()? | |
| } | |
| AnyJsxText::JsxExpressionAttributeValue(expression) => { | |
| use biome_js_syntax::{ | |
| AnyJsExpression, | |
| AnyJsLiteralExpression, | |
| JsFileSource, | |
| JsStringLiteralExpression, | |
| JsxAttribute, | |
| JsxExpressionAttributeValue, | |
| JsxExpressionChild, | |
| JsxString, | |
| JsxText, | |
| inner_string_text, | |
| }; | |
| … | |
| let value_token = match node { | |
| AnyJsxText::JsxText(text) => text.value_token().ok()?, | |
| AnyJsxText::JsStringLiteralExpression(expression) => { | |
| if !options.no_strings { | |
| return None; | |
| } | |
| // Ensure the literal is inside JSX (attribute value or expression child) | |
| let in_jsx = expression | |
| .syntax() | |
| .ancestors() | |
| .any(|n| { | |
| JsxExpressionAttributeValue::can_cast(n.kind()) | |
| || JsxExpressionChild::can_cast(n.kind()) | |
| }); | |
| if !in_jsx { | |
| return None; | |
| } | |
| expression.value_token().ok()? | |
| } | |
| AnyJsxText::JsxString(string) => { | |
| if !options.no_strings { | |
| return None; | |
| } | |
| string.value_token().ok()? | |
| } | |
| AnyJsxText::JsxExpressionAttributeValue(expression) => { | |
| … | |
| } | |
| }; |
There was a problem hiding this comment.
@coderabbitai the suggestion is incorrect. expression is already inside a JsxExpressionAttributeValue because of the match at line 174
There was a problem hiding this comment.
🧩 Analysis chain
@ematipico You're absolutely right - I apologize for the confusion. Let me investigate the AnyJsxText union to better understand the contexts it covers.
🏁 Script executed:
#!/bin/bash
# Let's examine the AnyJsxText union declaration and understand what contexts it covers
ast-grep --pattern 'declare_node_union! {
$$$
pub AnyJsxText = $$$
$$$
}'
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Let's find the AnyJsxText union declaration in the file
rg -A 10 -B 2 "declare_node_union" crates/biome_js_analyze/src/lint/nursery/no_jsx_literals.rs
Length of output: 715
🏁 Script executed:
#!/bin/bash
# Let's examine the test cases to understand what contexts are actually being tested
echo "=== Invalid test cases ==="
cat crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalid.jsx
echo -e "\n=== Invalid with noStrings test cases ==="
cat crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/invalidNoStrings.jsx
echo -e "\n=== Let's also check if there are any non-JSX string literals in these test files ==="
rg -n "const|let|var.*=" crates/biome_js_analyze/tests/specs/nursery/noJsxLiterals/
Length of output: 2372
@ematipico Thank you for the correction! You're absolutely right. I misunderstood how the AnyJsxText union works.
Looking at the union definition and the test cases, I can see that JsStringLiteralExpression in this context is already scoped to JSX contexts by the AST itself - the rule's query Ast<AnyJsxText> will only match string literal expressions that are actually within JSX elements or attributes, not arbitrary string literals elsewhere in the file.
The union is specifically designed to capture different types of text/string content that appear in JSX contexts:
JsxText: Direct text content like<div>hello</div>JsStringLiteralExpression: String literals in JSX expressions like<div>{"hello"}</div>JsxString: String attributes like<div className="test">JsxExpressionAttributeValue: Expression attribute values
My original concern about flagging string literals outside JSX contexts was unfounded. The AST structure and query mechanism already ensure proper scoping. I apologize for the confusion!
🧠 Learnings used
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : When porting a lint rule, set sources in declare_lint_rule! (RuleSource::Eslint(...).same() or .inspired())
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Set the language field in declare_lint_rule! to the most specific applicable language (js/jsx/ts/tsx)
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : If a rule provides a code action, declare fix_kind in declare_lint_rule!
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Avoid unnecessary string allocations in rules; compare against &str or TokenText instead of calling to_string()
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/**/*.rs : Place all new rules in the nursery group under biome_js_analyze/lib/src/lint/nursery
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : In declare_lint_rule! metadata, set version: "next"
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : When deprecating a rule, add deprecated: "<reason>" to declare_lint_rule!
Summary
This PR implements the rule
noJsxLiterals: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-no-literals.mdI added three options. I know we usually don't do that initially, but I thought that ones implemented should be added because they seem useful and needed for users.
Test Plan
Added test cases
Docs
Added to rule