feat(lint): implement use useRegexpTest nursery rule#9702
Conversation
🦋 Changeset detectedLatest commit: 32d4187 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a new nursery lint rule Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rs (1)
54-62: Tiny naming nit:UseRegexpTextStatelooks like a typo.Line 54 likely wants
UseRegexpTestState; the current name makes navigation/grep mildly awkward.Suggested rename
-pub struct UseRegexpTextState { +pub struct UseRegexpTestState { @@ - type State = UseRegexpTextState; + type State = UseRegexpTestState; @@ - Some(UseRegexpTextState { + Some(UseRegexpTestState { @@ - let UseRegexpTextState { + let UseRegexpTestState { @@ - let UseRegexpTextState { + let UseRegexpTestState {🤖 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/use_regexp_test.rs` around lines 54 - 62, The struct name UseRegexpTextState is a typo and should be renamed to UseRegexpTestState to match the rule UseRegexpTest and make searches intuitive; update the struct declaration (currently "pub struct UseRegexpTextState"), all references to that type (e.g., the impl Rule for UseRegexpTest where type State = UseRegexpTextState), and any usages/imports/tests to the new identifier UseRegexpTestState so the type and rule names are consistent.
🤖 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/use_regexp_test.rs`:
- Around line 86-98: The code only checks the regexp_node type but not the
receiver (string_node) for ".match" calls; update the run logic to validate the
receiver type when call_name == "match": call
ctx.type_of_expression(string_node) and ensure it is a string primitive/instance
(use whatever helpers exist, e.g., is_string() or is_string_instance()) and
return None if not a string, so custom objects with a .match method aren't
rewritten; keep the existing regexp checks
(is_regexp_literal/is_regexp_instance) intact and perform this receiver
validation before proceeding with the fix that transforms .match to
/regex/.test(...).
In `@crates/biome_js_analyze/tests/specs/nursery/useRegexpTest/valid.js`:
- Around line 4-5: The file declares `const result` twice which causes a
redeclaration parse error; update one of the declarations to avoid duplicate
binding—either rename the second variable (e.g., `resultExec` or `result2`) or
reuse the same name by replacing the second `const result` with an assignment to
`result`. Locate the two occurrences involving `str.match(/test/)` and
`/test/.exec(str)` and change the second declaration accordingly so both results
can coexist without redeclaration.
---
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rs`:
- Around line 54-62: The struct name UseRegexpTextState is a typo and should be
renamed to UseRegexpTestState to match the rule UseRegexpTest and make searches
intuitive; update the struct declaration (currently "pub struct
UseRegexpTextState"), all references to that type (e.g., the impl Rule for
UseRegexpTest where type State = UseRegexpTextState), and any
usages/imports/tests to the new identifier UseRegexpTestState so the type and
rule names are consistent.
🪄 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: 3e999e67-d447-4f5b-8256-f84511d6589d
⛔ Files ignored due to path filters (8)
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_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_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useRegexpTest/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useRegexpTest/valid.js.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 (9)
.changeset/hungry-meals-wash.mdcrates/biome_js_analyze/src/lint/nursery/use_regexp_test.rscrates/biome_js_analyze/src/lint/style/use_explicit_length_check.rscrates/biome_js_analyze/tests/specs/nursery/useRegexpTest/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useRegexpTest/valid.jscrates/biome_js_syntax/src/expr_ext.rscrates/biome_js_type_info/src/type.rscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_regexp_test.rs
| let (string_node, regexp_node) = if call_name == "match" { | ||
| (call_object, first_arg_expr) | ||
| } else if call_name == "exec" { | ||
| (first_arg_expr, call_object) | ||
| } else { | ||
| return None; | ||
| }; | ||
|
|
||
| let regexp_type = ctx.type_of_expression(regexp_node); | ||
|
|
||
| if !regexp_type.is_regexp_literal() && !regexp_type.is_regexp_instance() { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm current guards in the rule implementation
rg -n -C3 'call_name == "match"|type_of_expression\(|is_regexp_literal|is_regexp_instance|is_.*string' crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rs
# 2) Inspect fixtures for custom/non-string `.match(...)` receiver coverage
fd -i '.js$' crates/biome_js_analyze/tests/specs/nursery/useRegexpTest --type f \
| xargs rg -n -C2 '\.match\s*\('Repository: biomejs/biome
Length of output: 5605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Full implementation of the run method to check for all guards
sed -n '65,110p' crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rs
# 2) Full action method to see what it does
sed -n '130,180p' crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rs
# 3) Check the snapshot to see actual diagnostics
find crates/biome_js_analyze/tests/specs/nursery/useRegexpTest -name '*.snap' -o -name 'output.json' | head -5Repository: biomejs/biome
Length of output: 3093
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read snapshot file to see actual diagnostics
cat crates/biome_js_analyze/tests/specs/nursery/useRegexpTest/invalid.js.snapRepository: biomejs/biome
Length of output: 30046
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if is_in_boolean_context does type validation
rg -A 20 'fn is_in_boolean_context' crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rs
# Check if there's any type validation of string_node elsewhere
rg -B 5 -A 10 'string_node\|string_type' crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rs
# Look for any implicit semantic analysis that might handle receiver type
rg 'type_of_expression\|semantic\|as_string' crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rsRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read full file to understand is_in_boolean_context and full context
wc -l crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rs
# Get first 200 lines to see imports and helper functions
head -200 crates/biome_js_analyze/src/lint/nursery/use_regexp_test.rsRepository: biomejs/biome
Length of output: 5930
Validate receiver type for .match() calls to prevent false positives on custom objects.
The run method (lines 94–98) validates only the regexp argument type but not the receiver (string_node). For match() calls, the receiver is call_object, which is never type-checked. A custom object with a .match() method in a boolean context could be misreported and incorrectly fixed to /regex/.test(obj), changing semantics.
The current test suite (snapshot shows 29 diagnostics, all correct) does not cover this edge case; adding defensive validation ensures the rule remains sound as the codebase evolves.
🤖 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/use_regexp_test.rs` around lines 86
- 98, The code only checks the regexp_node type but not the receiver
(string_node) for ".match" calls; update the run logic to validate the receiver
type when call_name == "match": call ctx.type_of_expression(string_node) and
ensure it is a string primitive/instance (use whatever helpers exist, e.g.,
is_string() or is_string_instance()) and return None if not a string, so custom
objects with a .match method aren't rewritten; keep the existing regexp checks
(is_regexp_literal/is_regexp_instance) intact and perform this receiver
validation before proceeding with the fix that transforms .match to
/regex/.test(...).
There was a problem hiding this comment.
this seems like potentially a valid concern but isn't something that is handled by the original unicorn rule and also would probably not work for vanilla JS since it requires type info. is it something we want to consider here though for TS files?
…pdate to use different const names in test file
b1a8bf0 to
32d4187
Compare
| name: "useRegexpTest", | ||
| language: "js", | ||
| recommended: false, | ||
| sources: &[RuleSource::EslintUnicorn("prefer-regexp-test").same()], |
There was a problem hiding this comment.
The EslintE18e & EslintRegexp can also be added
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [@biomejs/biome](https://biomejs.dev) ([source](https://github.com/biomejs/biome/tree/HEAD/packages/@biomejs/biome)) | patch | `2.4.12` → `2.4.13` | --- ### Release Notes <details> <summary>biomejs/biome (@​biomejs/biome)</summary> ### [`v2.4.13`](https://github.com/biomejs/biome/blob/HEAD/packages/@​biomejs/biome/CHANGELOG.md#2413) [Compare Source](https://github.com/biomejs/biome/compare/@biomejs/biome@2.4.12...@biomejs/biome@2.4.13) ##### Patch Changes - [#​9969](biomejs/biome#9969) [`c5eb92b`](biomejs/biome@c5eb92b) Thanks [@​officialasishkumar](https://github.com/officialasishkumar)! - Added the nursery rule [`noUnnecessaryTemplateExpression`](https://biomejs.dev/linter/rules/no-unnecessary-template-expression/), which disallows template literals that only contain string literal expressions. These can be replaced with a simpler string literal. For example, the following code triggers the rule: ```js const a = `${"hello"}`; // can be 'hello' const b = `${"prefix"}_suffix`; // can be 'prefix_suffix' const c = `${"a"}${"b"}`; // can be 'ab' ``` - [#​10037](biomejs/biome#10037) [`f785e8c`](biomejs/biome@f785e8c) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed [#​9810](biomejs/biome#9810): [`noMisleadingReturnType`](https://biomejs.dev/linter/rules/no-misleading-return-type/) no longer reports false positives on a getter with a matching setter in the same namespace. ```ts class Store { get status(): string { if (Math.random() > 0.5) return "loading"; return "idle"; } set status(v: string) {} } ``` - [#​10084](biomejs/biome#10084) [`5e2f90c`](biomejs/biome@5e2f90c) Thanks [@​jiwon79](https://github.com/jiwon79)! - Fixed [#​10034](biomejs/biome#10034): [`noUselessEscapeInRegex`](https://biomejs.dev/linter/rules/no-useless-escape-in-regex/) no longer flags escapes of `ClassSetReservedPunctuator` characters (`&`, `!`, `#`, `%`, `,`, `:`, `;`, `<`, `=`, `>`, `@`, `` ` ``, `~`) inside `v`-flag character classes as useless. These characters are reserved as individual code points in `v`-mode, so the escape is required. The following pattern is now considered valid: ```js /[a-z\&]/v; ``` - [#​10063](biomejs/biome#10063) [`c9ffa16`](biomejs/biome@c9ffa16) Thanks [@​Netail](https://github.com/Netail)! - Added extra rule sources from ESLint CSS. `biome migrate eslint` should do a bit better detecting rules in your eslint configurations. - [#​10035](biomejs/biome#10035) [`946b50e`](biomejs/biome@946b50e) Thanks [@​Netail](https://github.com/Netail)! - Fixed [#​10032](biomejs/biome#10032): [useIframeSandbox](https://biomejs.dev/linter/rules/use-iframe-sandbox/) now flags if there's no initializer value. - [#​9865](biomejs/biome#9865) [`68fb8d4`](biomejs/biome@68fb8d4) Thanks [@​dyc3](https://github.com/dyc3)! - Added the new nursery rule [`useDomNodeTextContent`](https://biomejs.dev/linter/rules/use-dom-node-text-content/), which prefers `textContent` over `innerText` for DOM node text access and destructuring. For example, the following snippet triggers the rule: ```js const foo = node.innerText; ``` - [#​10023](biomejs/biome#10023) [`bd1e74f`](biomejs/biome@bd1e74f) Thanks [@​ematipico](https://github.com/ematipico)! - Added a new nursery rule [`noReactNativeDeepImports`](https://biomejs.dev/linter/rules/no-react-native-deep-imports/) that disallows deep imports from the `react-native` package. Internal paths like `react-native/Libraries/...` are not part of the public API and may change between versions. For example, the following code triggers the rule: ```js import View from "react-native/Libraries/Components/View/View"; ``` - [#​9885](biomejs/biome#9885) [`3dce737`](biomejs/biome@3dce737) Thanks [@​dyc3](https://github.com/dyc3)! - Added a new nursery rule [`useDomQuerySelector`](https://biomejs.dev/linter/rules/use-dom-query-selector/) that prefers `querySelector()` and `querySelectorAll()` over older DOM query methods such as `getElementById()` and `getElementsByClassName()`. - [#​9995](biomejs/biome#9995) [`4da9caf`](biomejs/biome@4da9caf) Thanks [@​siketyan](https://github.com/siketyan)! - Fixed [#​9994](biomejs/biome#9994): Biome now parses nested CSS rules correctly when declarations follow them inside embedded snippets. - [#​10009](biomejs/biome#10009) [`b41cc5a`](biomejs/biome@b41cc5a) Thanks [@​Jayllyz](https://github.com/Jayllyz)! - Fixed [#​10004](biomejs/biome#10004): [`noComponentHookFactories`](https://biomejs.dev/linter/rules/no-component-hook-factories/) no longer reports false positives for object methods and class methods. - [#​9988](biomejs/biome#9988) [`eabf54a`](biomejs/biome@eabf54a) Thanks [@​Netail](https://github.com/Netail)! - Tweaked the diagnostics range for [useAltText](https://biomejs.dev/linter/rules/use-alt-text), [useButtonType](https://biomejs.dev/linter/rules/use-button-type), [useHtmlLang](https://biomejs.dev/linter/rules/use-html-lang), [useIframeTitle](https://biomejs.dev/linter/rules/use-iframe-title), [useValidAriaRole](https://biomejs.dev/linter/rules/use-valid-aria-role) & [useIfameSandbox](https://biomejs.dev/linter/rules/use-iframe-sandbox) to report on the opening tag instead of the full tag. - [#​10043](biomejs/biome#10043) [`fc65902`](biomejs/biome@fc65902) Thanks [@​mujpao](https://github.com/mujpao)! - Fixed [#​10003](biomejs/biome#10003): Biome no longer panics when parsing Svelte files containing `{#}`. - [#​9815](biomejs/biome#9815) [`5cc83b1`](biomejs/biome@5cc83b1) Thanks [@​dyc3](https://github.com/dyc3)! - Added the new nursery rule [`noLoopFunc`](https://biomejs.dev/linter/rules/no-loop-func/). When enabled, it warns when a function declared inside a loop captures outer variables that can change across iterations. - [#​9702](biomejs/biome#9702) [`ef470ba`](biomejs/biome@ef470ba) Thanks [@​ryan-m-walker](https://github.com/ryan-m-walker)! - Added the nursery rule [`useRegexpTest`](https://biomejs.dev/linter/rules/use-regexp-test/) that enforces `RegExp.prototype.test()` over `String.prototype.match()` and `RegExp.prototype.exec()` in boolean contexts. `test()` returns a boolean directly, avoiding unnecessary computation of match results. **Invalid** ```js if ("hello world".match(/hello/)) { } ``` **Valid** ```js if (/hello/.test("hello world")) { } ``` - [#​9743](biomejs/biome#9743) [`245307d`](biomejs/biome@245307d) Thanks [@​leetdavid](https://github.com/leetdavid)! - Fixed [#​2245](biomejs/biome#2245): Svelte `<script>` tag language detection when the `generics` attribute contains `>` characters (e.g., `<script lang="ts" generics="T extends Record<string, unknown>">`). Biome now correctly recognizes TypeScript in such script blocks. - [#​10046](biomejs/biome#10046) [`0707de7`](biomejs/biome@0707de7) Thanks [@​Conaclos](https://github.com/Conaclos)! - Fixed [#​10038](biomejs/biome#10038): [`organizeImports`](https://biomejs.dev/assist/actions/organize-imports/) now sorts imports in TypeScript modules and declaration files. ```diff declare module "mymodule" { - import type { B } from "b"; import type { A } from "a"; + import type { B } from "b"; } ``` - [#​10012](biomejs/biome#10012) [`94ccca9`](biomejs/biome@94ccca9) Thanks [@​ematipico](https://github.com/ematipico)! - Added the nursery rule [`noReactNativeLiteralColors`](https://biomejs.dev/linter/rules/no-react-native-literal-colors/), which disallows color literals inside React Native styles. The rule belongs to the `reactNative` domain. It reports properties whose name contains `color` and whose value is a string literal when they appear inside a `StyleSheet.create(...)` call or inside a JSX attribute whose name contains `style`. ```jsx // Invalid const Hello = () => <Text style={{ backgroundColor: "#FFFFFF" }}>hi</Text>; const styles = StyleSheet.create({ text: { color: "red" }, }); ``` ```jsx // Valid const red = "#f00"; const styles = StyleSheet.create({ text: { color: red }, }); ``` - [#​10005](biomejs/biome#10005) [`131019e`](biomejs/biome@131019e) Thanks [@​ematipico](https://github.com/ematipico)! - Added the nursery rule [`noReactNativeRawText`](https://biomejs.dev/linter/rules/no-react-native-raw-text/), which disallows raw text outside of `<Text>` components in React Native. The rule belongs to the new `reactNative` domain. ```jsx // Invalid <View>some text</View> <View>{'some text'}</View> ``` ```jsx // Valid <View> <Text>some text</Text> </View> ``` Additional components can be allowlisted through the `skip` option: ```json { "options": { "skip": ["Title"] } } ``` - [#​9911](biomejs/biome#9911) [`1603f78`](biomejs/biome@1603f78) Thanks [@​Netail](https://github.com/Netail)! - Added the nursery rule [`noJsxLeakedDollar`](https://biomejs.dev/linter/rules/no-jsx-leaked-dollar), which flags text nodes with a trailing `$` if the next sibling node is a JSX expression. This could be an unintentional mistake, resulting in a '$' being rendered as text in the output. **Invalid**: ```jsx function MyComponent({ user }) { return <div>Hello ${user.name}</div>; } ``` - [#​9999](biomejs/biome#9999) [`f42405f`](biomejs/biome@f42405f) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed `noMisleadingReturnType` incorrectly flagging functions with reassigned `let` variables. - [#​10075](biomejs/biome#10075) [`295f97f`](biomejs/biome@295f97f) Thanks [@​ematipico](https://github.com/ematipico)! - Fixed [`#9983`](biomejs/biome#9983): Biome now parses functions declared inside Svelte `#snippet` blocks without throwing errors. - [#​10006](biomejs/biome#10006) [`cf4c1c9`](biomejs/biome@cf4c1c9) Thanks [@​minseong0324](https://github.com/minseong0324)! - Fixed [#​9810](biomejs/biome#9810): `noMisleadingReturnType` incorrectly flagging nested object literals with widened properties. - [#​10033](biomejs/biome#10033) [`11ddc05`](biomejs/biome@11ddc05) Thanks [@​ematipico](https://github.com/ematipico)! - Added the nursery rule [`useReactNativePlatformComponents`](https://biomejs.dev/linter/rules/use-react-native-platform-components/) that ensures platform-specific React Native components (e.g. `ProgressBarAndroid`, `ActivityIndicatorIOS`) are only imported in files with a matching platform suffix. It also reports when Android and iOS components are mixed in the same file. The following code triggers the rule when the file does not have an `.android.js` suffix: ```js // file.js import { ProgressBarAndroid } from "react-native"; ``` </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMzkuNiIsInVwZGF0ZWRJblZlciI6IjQzLjEzOS42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Reviewed-on: https://git.oirnoir.dev/OIRNOIR/YouTube-Helper-Server/pulls/1
Summary
Closes #9377
Implements the
useRegexpTestnursery rule based off of this ESLint Unicorn rule.Note: One change I made that had a slight wider impact was to update the helper
is_in_boolean_contextto include logical expressions and negation unary expressions to match the behavior of the ESLint rule. This made it necessary to update theuseExplicitLengthCheckrule by changing the order of some code to still work as before since it was not taking those contexts into consideration correctly before. If you would prefer this to be more isolated and not update that shared function and to the checks more isolated in the new rule feel free to let me know and I'd be glad to do so. I just felt it made sense to make the shared function more correct but it does potentially introduce a bit more risk to existing behavior. Let me know what you think.Test Plan
Snapshot tests added and all other tests still pass
Docs
Included in the rule docs.
AI Usage Disclosure
Claude Code was used for general codebase research, debugging issues as well as helping generate documentation and changeset under my supervision. Most of the actual code was hand written by me though.