Skip to content

Commit ef470ba

Browse files
feat(lint): implement use useRegexpTest nursery rule (#9702)
1 parent 7a2f6cf commit ef470ba

17 files changed

Lines changed: 1116 additions & 14 deletions

File tree

.changeset/hungry-meals-wash.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
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.
6+
7+
**Invalid**
8+
9+
``` js
10+
if ("hello world".match(/hello/)) {}
11+
```
12+
13+
**Valid**
14+
15+
```js
16+
if (/hello/.test("hello world")) {}
17+
```

crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_configuration/src/analyzer/linter/rules.rs

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_configuration/src/generated/linter_options_check.rs

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_diagnostics_categories/src/categories.rs

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
use biome_analyze::{
2+
Ast, FixKind, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
3+
};
4+
use biome_console::markup;
5+
use biome_js_factory::make;
6+
use biome_js_syntax::{
7+
AnyJsExpression, AnyJsName, JsCallExpression, JsNewExpression, is_in_boolean_context,
8+
};
9+
use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt};
10+
use biome_rule_options::use_regexp_test::UseRegexpTestOptions;
11+
12+
use crate::JsRuleAction;
13+
14+
declare_lint_rule! {
15+
/// Enforce the use of `RegExp.prototype.test()` over `String.prototype.match()` and `RegExp.prototype.exec()` in boolean contexts.
16+
///
17+
/// When checking whether a string matches a regular expression, `RegExp.prototype.test()` is more appropriate
18+
/// than `String.prototype.match()` and `RegExp.prototype.exec()` because it returns a boolean directly.
19+
/// In contrast, `match()` and `exec()` return match objects or arrays, which involves unnecessary computation
20+
/// when only a true/false result is needed.
21+
///
22+
/// The fix is marked as unsafe because `match()` and `exec()` can have side effects when used with
23+
/// [global or sticky](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/lastIndex) regular expressions,
24+
/// since they advance the `lastIndex` property differently than `test()`.
25+
///
26+
/// ## Examples
27+
///
28+
/// ### Invalid
29+
///
30+
/// ```js,expect_diagnostic
31+
/// if ("hello world".match(/hello/)) {}
32+
/// ```
33+
///
34+
/// ```js,expect_diagnostic
35+
/// if (/hello/.exec("hello world")) {}
36+
/// ```
37+
///
38+
/// ### Valid
39+
///
40+
/// ```js
41+
/// if (/hello/.test("hello world")) {}
42+
/// ```
43+
///
44+
pub UseRegexpTest {
45+
version: "next",
46+
name: "useRegexpTest",
47+
language: "js",
48+
recommended: false,
49+
sources: &[RuleSource::EslintUnicorn("prefer-regexp-test").same()],
50+
fix_kind: FixKind::Unsafe,
51+
}
52+
}
53+
54+
pub struct UseRegexpTestState {
55+
string_node: AnyJsExpression,
56+
regexp_node: AnyJsExpression,
57+
}
58+
59+
impl Rule for UseRegexpTest {
60+
type Query = Ast<JsCallExpression>;
61+
type State = UseRegexpTestState;
62+
type Signals = Option<Self::State>;
63+
type Options = UseRegexpTestOptions;
64+
65+
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
66+
let node = ctx.query();
67+
68+
if !is_in_boolean_context(node.syntax()).unwrap_or(false) {
69+
return None;
70+
}
71+
72+
let binding = node.callee().ok()?.omit_parentheses();
73+
let callee = binding.as_js_static_member_expression()?;
74+
let call_object = &callee.object().ok()?;
75+
let call_name = callee
76+
.member()
77+
.ok()?
78+
.as_js_name()?
79+
.value_token()
80+
.ok()?
81+
.token_text_trimmed();
82+
83+
let args = node.arguments().ok()?.args();
84+
85+
if args.len() != 1 {
86+
return None;
87+
}
88+
89+
let first_arg = args.first()?.ok()?;
90+
let first_arg_expr = first_arg.as_any_js_expression()?;
91+
92+
let (string_node, regexp_node) = if call_name.text() == "match" {
93+
(call_object, first_arg_expr)
94+
} else if call_name.text() == "exec" {
95+
(first_arg_expr, call_object)
96+
} else {
97+
return None;
98+
};
99+
100+
if !is_regexp(regexp_node) {
101+
return None;
102+
}
103+
104+
Some(UseRegexpTestState {
105+
regexp_node: regexp_node.clone(),
106+
string_node: string_node.clone(),
107+
})
108+
}
109+
110+
fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
111+
let node = ctx.query();
112+
113+
Some(RuleDiagnostic::new(
114+
rule_category!(),
115+
node.range(),
116+
markup! {
117+
<Emphasis>".match()"</Emphasis>" and "<Emphasis>".exec()"</Emphasis>" are not designed for boolean checks."
118+
},
119+
)
120+
.note(markup! {
121+
"These methods return match objects or arrays, which involves unnecessary computation when only checking for a match."
122+
}))
123+
}
124+
125+
fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
126+
let node = ctx.query();
127+
let mut mutation = ctx.root().begin();
128+
129+
let UseRegexpTestState {
130+
string_node,
131+
regexp_node,
132+
} = state;
133+
134+
let binding = node.callee().ok()?.omit_parentheses();
135+
let callee = binding.as_js_static_member_expression()?;
136+
let call_object = &callee.object().ok()?;
137+
let call_name = callee.member().ok()?;
138+
139+
let args = node.arguments().ok()?.args();
140+
let first_arg = args.first()?.ok()?;
141+
let first_arg_expr = first_arg.as_any_js_expression()?;
142+
143+
mutation.replace_node(call_object.clone(), regexp_node.clone());
144+
mutation.replace_node(
145+
call_name,
146+
AnyJsName::JsName(make::js_name(make::ident("test"))),
147+
);
148+
mutation.replace_node(first_arg_expr.clone(), string_node.clone());
149+
150+
Some(JsRuleAction::new(
151+
ctx.metadata().action_category(ctx.category(), ctx.group()),
152+
ctx.metadata().applicability(),
153+
markup! {
154+
"Use .test() instead."
155+
},
156+
mutation,
157+
))
158+
}
159+
}
160+
161+
/// Returns `true` if the expression is a regex literal (`/pattern/`) or a
162+
/// `new RegExp(...)` / `new window.RegExp(...)` / `new globalThis.RegExp(...)`
163+
/// constructor call.
164+
fn is_regexp(expr: &AnyJsExpression) -> bool {
165+
let expr = expr.clone().omit_parentheses();
166+
match expr {
167+
AnyJsExpression::AnyJsLiteralExpression(literal) => {
168+
literal.as_js_regex_literal_expression().is_some()
169+
}
170+
AnyJsExpression::JsNewExpression(new_expr) => is_regexp_constructor(&new_expr),
171+
_ => false,
172+
}
173+
}
174+
175+
fn is_regexp_constructor(expr: &JsNewExpression) -> bool {
176+
let Ok(callee) = expr.callee() else {
177+
return false;
178+
};
179+
180+
match callee {
181+
AnyJsExpression::JsIdentifierExpression(id) => id
182+
.name()
183+
.ok()
184+
.and_then(|n| n.value_token().ok())
185+
.is_some_and(|t| t.token_text_trimmed().text() == "RegExp"),
186+
AnyJsExpression::JsStaticMemberExpression(member) => {
187+
let object_is_global = member
188+
.object()
189+
.ok()
190+
.and_then(|obj| obj.as_js_identifier_expression()?.name().ok()?.value_token().ok())
191+
.is_some_and(|t| {
192+
let name = t.token_text_trimmed();
193+
name.text() == "window" || name.text() == "globalThis"
194+
});
195+
196+
if !object_is_global {
197+
return false;
198+
}
199+
200+
member
201+
.member()
202+
.ok()
203+
.and_then(|m| m.as_js_name()?.value_token().ok())
204+
.is_some_and(|t| t.token_text_trimmed().text() == "RegExp")
205+
}
206+
_ => false,
207+
}
208+
}

crates/biome_js_analyze/src/lint/style/use_explicit_length_check.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,20 @@ impl Rule for UseExplicitLengthCheck {
190190
});
191191
}
192192

193+
if let Some((boolean_expr, is_negative)) = get_boolean_ancestor(member_expr_syntax) {
194+
let check = if is_negative {
195+
LengthCheck::Zero
196+
} else {
197+
LengthCheck::NonZero
198+
};
199+
200+
return Some(UseExplicitLengthCheckState {
201+
check,
202+
node: boolean_expr,
203+
member_name,
204+
});
205+
}
206+
193207
if is_in_boolean_context(member_expr_syntax).unwrap_or(false) {
194208
return Some(UseExplicitLengthCheckState {
195209
check: LengthCheck::NonZero,
@@ -215,20 +229,6 @@ impl Rule for UseExplicitLengthCheck {
215229
});
216230
}
217231

218-
if let Some((boolean_expr, is_negative)) = get_boolean_ancestor(member_expr_syntax) {
219-
let check = if is_negative {
220-
LengthCheck::Zero
221-
} else {
222-
LengthCheck::NonZero
223-
};
224-
225-
return Some(UseExplicitLengthCheckState {
226-
check,
227-
node: boolean_expr,
228-
member_name,
229-
});
230-
}
231-
232232
None
233233
}
234234

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
if (str.match(/test/)) {}
2+
if (/test/.exec(str)) {}
3+
if (str.match(/test/i)) {}
4+
while (str.match(/test/)) {}
5+
for (; str.match(/test/);) {}
6+
do {} while (str.match(/test/));
7+
if (obj.prop.match(/test/)) {}
8+
if (/test/.exec(obj.prop)) {}
9+
if (str.match(/a/)) {} if (/b/.exec(str)) {}
10+
if (str.match(new RegExp('test'))) {}
11+
if (new RegExp('test').exec(str)) {}
12+
if (str.match(new window.RegExp('test'))) {}
13+
if (str.match(new globalThis.RegExp('test'))) {}
14+
const x = str.match(/test/) ? a : b;
15+
if (!str.match(/test/)) {}
16+
if (str.match(/test/) && other) {}
17+
if (other && str.match(/test/)) {}
18+
if (str.match(/test/) || fallback) {}
19+
if (fallback || str.match(/test/)) {}
20+
if (a && !str.match(/test/)) {}
21+
if (!!str.match(/test/)) {}
22+
23+
// trivia is preserved for code actions
24+
if (
25+
str.match(
26+
// comment
27+
/test/)
28+
) {}

0 commit comments

Comments
 (0)