feat(useRegexpExec): partially cover new RegExp()
#8370
Conversation
🦋 Changeset detectedLatest commit: e58a808 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 |
WalkthroughThis PR enhances the Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs (1)
96-131: Consider flatteningevaluate_new_expressioncontrol flowThe nested
len/iter().nth/iter().nextchain is correct but a bit dense; a small refactor using earlyreturns and a singlematchover(first_arg, second_arg)(from one iterator) would make it easier to follow while keeping the logic the same. No rush, but worth a tidy‑up when you next touch this rule.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/useRegexpExec/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useRegexpExec/valid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/plain-hairs-boil.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs(3 hunks)crates/biome_js_analyze/tests/specs/nursery/useRegexpExec/invalid.js(1 hunks)crates/biome_js_analyze/tests/specs/nursery/useRegexpExec/valid.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use thedbg!()macro for debugging output during testing, and pass the--show-outputflag tocargoto view debug output
Usecargo torcargo testto run tests; for a single test, pass the test name after thetestcommand
Use snapshot testing with theinstacrate; runcargo insta accept,cargo insta reject, orcargo insta reviewto manage snapshot changes
Write doctests as doc comments with code blocks; the code inside code blocks will be run during the testing phase
Usejust f(alias forjust format) to format Rust and TOML files before committing
Files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
.changeset/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/**/*.md: Create changesets for user-facing changes usingjust new-changeset; use headers with####or#####only; keep descriptions concise (1-3 sentences) and focus on user-facing changes
Use past tense when describing what was done ('Added new feature'), present tense when describing Biome behavior ('Biome now supports'); end sentences with a full stop
For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use adiffcode block
Files:
.changeset/plain-hairs-boil.md
🧠 Learnings (17)
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.jsonc : Use `.jsonc` files in test specs with code snippets as array of strings to test rules in script environment (no import/export syntax)
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useRegexpExec/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useRegexpExec/valid.js
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.{js,ts,tsx,jsx,json,css} : Test rules using snapshot tests via the `insta` library with test cases in `tests/specs/<group>/<rule_name>/` directories prefixed by `invalid` or `valid`
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/useRegexpExec/invalid.js
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Check if a variable is global before banning it to avoid false positives when the variable is redeclared in local scope; use the semantic model to verify global scope
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noRestricted` prefix for rules that report user-banned entities (e.g., `noRestrictedGlobals`)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use language tags in documentation code blocks (js, ts, tsx, json, css) and order properties consistently as: language, then `expect_diagnostic`, then options modifiers, then `ignore`, then `file=path`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `action` function must return a `JsRuleAction` (or equivalent language-specific action type) with category `ctx.action_category(ctx.category(), ctx.group())` and applicability from `ctx.metadata().applicability()`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `declare_node_union!` macro to query multiple node types at once by joining them into an enum with `Any*Like` naming convention
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : New rules must be placed inside the `nursery` group as an incubation space exempt from semantic versioning, with promotion to appropriate groups done during minor/major releases
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules must be implemented using the `Rule` trait with type parameters: `Query` (node type to analyze), `State` (information for signals), `Signals` (return type from run function), and `Options` (rule configuration)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `diagnostic` function must return a `RuleDiagnostic` that defines the message reported to the user using the `markup!` macro
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use `Semantic<T>` query type instead of `Ast<T>` when a rule needs to access the semantic model for binding references and scope information
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use `rule_category!()` macro instead of dynamic string parsing to refer to rule diagnostic categories for compile-time validation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Framework-specific rules should be named using the `use` or `no` prefix followed by the framework name (e.g., `noVueReservedProps`)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Rules should use the `use` prefix naming convention when the sole intention is to mandate a single concept (e.g., `useValidLang` to enforce valid HTML lang attribute values)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Implement custom `Queryable` types and `Visitor` traits for rules requiring deep AST inspection to avoid redundant traversal passes
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `useShorthand` prefix for rules that report syntax rewritable using equivalent compact syntax (e.g., `useShorthandAssign`)
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs
📚 Learning: 2025-11-28T09:08:10.091Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-28T09:08:10.091Z
Learning: Applies to .changeset/**/*.md : For new lint rules, show an example of an invalid case in an inline code snippet or code block; for rule changes, demonstrate what is now invalid or valid; for formatter changes, use a `diff` code block
Applied to files:
.changeset/plain-hairs-boil.md
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs (1)
crates/biome_js_syntax/src/expr_ext.rs (1)
callee(33-38)
⏰ 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). (11)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: autofix
- GitHub Check: Test Node.js API
🔇 Additional comments (4)
crates/biome_js_analyze/tests/specs/nursery/useRegexpExec/valid.js (1)
23-23: Valid coverage fornew RegExp("thing", "g")looks goodNice addition to exercise the string-pattern global case and keep the rule’s behaviour well‑specified without broadening its scope.
crates/biome_js_analyze/tests/specs/nursery/useRegexpExec/invalid.js (1)
7-8: Good edge-case invalid for overridden flagsThis neatly covers
new RegExp(/thing/g, ''), where the ctor strips the global flag and should now be caught by the rule; a very useful regression test.crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs (1)
6-7: Helper extraction keepsruntidyPulling the literal and
new RegExpchecks intoevaluate_regexp_literal/evaluate_new_expressionmakes the main rule logic easier to read while preserving the existing short‑circuit behaviour.Also applies to: 70-73
.changeset/plain-hairs-boil.md (1)
1-11: Changeset text and example look spot-onConcise description in the right tense plus an explicit invalid example is exactly what we want for documenting this rule tweak. Based on learnings, this fits the
.changesetguidance nicely.
CodSpeed Performance ReportMerging #8370 will not alter performanceComparing Summary
Footnotes
|
arendjr
left a comment
There was a problem hiding this comment.
I’m not going to block this, because strictly speaking it is an improvement, but I don’t think this is implemented the right way. Using the type information we have available, we should perform an “instance of” check to see if the value evaluates to something that is a RegExp instance. This should get us coverage for most situations where a variable is passed that refers to a RegExp too.
What do you think, @Netail ?
Agreed Nice new test cases tho :) |
Summary
This is a partial fix for #8285. The
useRegexpExecrule now covers cases wherenew RegExp()is called directly as an argument toString#match.Test Plan
The basic test cases for this change were already present in the corresponding test files for this rule, and I've added more tests to cover edge cases like
'something'.match(new RegExp(/thing/g, '')).