feat(lint): add noReactNativeLiteralColors#10012
Conversation
🦋 Changeset detectedLatest commit: 230566b 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 |
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
noReactNativeRawTextnoReactNativeLiteralColors
WalkthroughAdds a new JavaScript React Native lint rule Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_react_native_literal_colors.rs`:
- Around line 63-67: The rule declaration for NoReactNativeLiteralColors uses
RuleSource::EslintReactNative("no-color-literals").same(), but since the ported
logic diverged from the original ESLint rule you should call .inspired() instead
of .same(); update the source expression for NoReactNativeLiteralColors (the
RuleSource::EslintReactNative(...) chain) to use .inspired() so the metadata
correctly reflects that this rule is only inspired by the upstream rule.
- Around line 148-164: The function has_color_literal_value currently misses
parenthesized string literals (e.g.,
AnyJsExpression::AnyJsParenthesizedExpression wrapping a
JsStringLiteralExpression), so update has_color_literal_value to treat
parenthesized string literals as color literals: either add pattern arms to
match AnyJsParenthesizedExpression and check its inner expression for
is_string_literal (or recursively unwrap parentheses until a non-parenthesized
node is reached), and apply the same handling for conditional branches
(consequent/alternate) so parenthesised string literals are recognized the same
as bare string literals; reference function has_color_literal_value and the
is_string_literal checks in the conditional branch when making the change.
- Around line 175-190: The is_stylesheet_create function is passing
Some("StyleSheet") to is_framework_api_reference which treats unresolved
identifiers named StyleSheet as matching the React Native API; update the call
in is_stylesheet_create (the function handling JsCallExpression checks) to pass
None instead of Some("StyleSheet") so only actual imports from
REACT_NATIVE_PACKAGE_NAMES are accepted and unresolved/local StyleSheet
identifiers are not flagged.
In `@crates/biome_js_syntax/src/expr_ext.rs`:
- Around line 1371-1375: The is_string_literal helper currently only matches
direct string literal nodes (function is_string_literal) so parenthesised forms
like ('#fff') are missed; update is_string_literal to unwrap
AnyJsParenthesizedExpression (or recursively strip parenthesis wrappers) and
then test the inner expression for
AnyJsLiteralExpression::JsStringLiteralExpression so both direct and
parenthesised string literals return true (either add a loop that peels
Self::AnyJsParenthesizedExpression to its inner expression before matching, or
make the match accept the parenthesized variant that contains a string literal).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 709ecf95-7ffd-43e7-b132-281654214854
⛔ Files ignored due to path filters (10)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_configuration/src/generated/domain_selector.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_configuration/src/generated/linter_options_check.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/invalidReactNativeImport.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/validCustomStyleSheet.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/validLocalStyleSheet.jsx.snapis excluded by!**/*.snapand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (10)
.changeset/no-react-native-literal-colors.mdcrates/biome_js_analyze/src/lint/nursery/no_react_native_literal_colors.rscrates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/invalidReactNativeImport.jsxcrates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/validCustomStyleSheet.jsxcrates/biome_js_analyze/tests/specs/nursery/noReactNativeLiteralColors/validLocalStyleSheet.jsxcrates/biome_js_syntax/src/expr_ext.rscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_react_native_literal_colors.rs
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_react_native_literal_colors.rs (1)
148-165: Nested ternaries partially detected.For deeply nested conditionals like
flag ? (flag2 ? '#fff' : '#000') : theme.color, only the outer branches are checked. The inner string literals would slip through. This is likely acceptable for a nursery rule — just flagging it for awareness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/src/lint/nursery/no_react_native_literal_colors.rs` around lines 148 - 165, The function has_color_literal_value only checks the immediate consequent/alternate of a JsConditionalExpression, so nested ternaries like flag ? (flag2 ? '#fff' : '#000') : theme.color escape detection; update has_color_literal_value to recursively inspect conditional branches by calling itself on consequent() and alternate() (and also treat JsStringLiteralExpression as before) so any depth of nested AnyJsConditionalExpression nested inside AnyJsExpression is checked for string literal color values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_react_native_literal_colors.rs`:
- Around line 148-165: The function has_color_literal_value only checks the
immediate consequent/alternate of a JsConditionalExpression, so nested ternaries
like flag ? (flag2 ? '#fff' : '#000') : theme.color escape detection; update
has_color_literal_value to recursively inspect conditional branches by calling
itself on consequent() and alternate() (and also treat JsStringLiteralExpression
as before) so any depth of nested AnyJsConditionalExpression nested inside
AnyJsExpression is checked for string literal color values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87c09093-10c2-4533-91d5-064e17507038
⛔ Files ignored due to path filters (3)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand 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 (2)
crates/biome_js_analyze/src/lint/nursery/no_react_native_literal_colors.rscrates/biome_string_case/src/lib.rs
Summary
This PR adds the new nursery rule
noReactNativeLiteralColorsto the react native domain. Here's the docs of the original rule https://github.com/Intellicode/eslint-plugin-react-native/blob/master/docs/rules/no-color-literals.mdI used a coding agent, but then I changed the rule's logic. Tests have been added by the coding agent.
Test Plan
Added new tests
Docs