mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(linter/unicorn/prefer-regexp-test): check logical operands in boolean contexts (#27761)
Calls such as `if (a && s.match(/x/))` and `if (s.match(/x/) || a)` were skipped even though their results are only tested for truthiness. Follow `&&`/`||` operands to the enclosing boolean context, matching Unicorn, while preserving value-producing uses and nullish expressions. Add diagnostic and autofix regressions for `.match()` and `.exec()`. Addresses report 9 (`unicorn/prefer-regexp-test`) in #27746.
This commit is contained in:
camc314 committed
1 parent
3c44398772
commit
7f22fa0d8c
2 files changed
+157
-22
No files matched your search
@@ -1,13 +1,16 @@
|
||||
use oxc_allocator::{GetAddress, UnstableAddress};
|
||||
use oxc_ast::{
|
||||
AstKind,
|
||||
ast::{Expression, MemberExpression},
|
||||
};
|
||||
use oxc_diagnostics::OxcDiagnostic;
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
use oxc_span::Span;
|
||||
use oxc_span::{GetSpan, Span};
|
||||
use oxc_syntax::operator::UnaryOperator;
|
||||
|
||||
use crate::{AstNode, ast_util::outermost_paren_parent, context::LintContext, rule::Rule};
|
||||
use crate::{
|
||||
AstNode, ast_util::outermost_paren_parent, context::LintContext, rule::Rule,
|
||||
utils::is_logical_expression,
|
||||
};
|
||||
|
||||
fn prefer_regexp_test_diagnostic(span: Span) -> OxcDiagnostic {
|
||||
OxcDiagnostic::warn("Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.")
|
||||
@@ -81,46 +84,48 @@ impl Rule for PreferRegexpTest {
|
||||
_ => return,
|
||||
};
|
||||
|
||||
let Some(parent) = outermost_paren_parent(node, ctx) else {
|
||||
return;
|
||||
let mut boolean_node = node;
|
||||
let parent = loop {
|
||||
let Some(parent) = outermost_paren_parent(boolean_node, ctx) else {
|
||||
return;
|
||||
};
|
||||
if !is_logical_expression(parent) {
|
||||
break parent;
|
||||
}
|
||||
boolean_node = parent;
|
||||
};
|
||||
|
||||
match parent.kind() {
|
||||
AstKind::ForStatement(for_stmt) => {
|
||||
let Some(test) = &for_stmt.test else { return };
|
||||
|
||||
let Expression::CallExpression(call_expr2) = test else {
|
||||
return;
|
||||
};
|
||||
|
||||
// Check if the `test` of the for statement is the same node as the call expression.
|
||||
if call_expr2.address() != call_expr.unstable_address() {
|
||||
if test.get_inner_expression().span() != boolean_node.kind().span() {
|
||||
return;
|
||||
}
|
||||
}
|
||||
AstKind::ConditionalExpression(conditional_expr) => {
|
||||
let Expression::CallExpression(call_expr2) = &conditional_expr.test else {
|
||||
return;
|
||||
};
|
||||
|
||||
// Check if the `test` of the conditional expression is the same node as the call expression.
|
||||
if call_expr2.address() != call_expr.unstable_address() {
|
||||
if conditional_expr.test.get_inner_expression().span() != boolean_node.kind().span()
|
||||
{
|
||||
return;
|
||||
}
|
||||
}
|
||||
AstKind::CallExpression(call_expr) => {
|
||||
if call_expr.optional || call_expr.arguments.len() != 1 {
|
||||
return;
|
||||
}
|
||||
|
||||
let Expression::Identifier(ident) = &call_expr.callee else {
|
||||
return;
|
||||
};
|
||||
|
||||
if ident.name.as_str() != "Boolean" {
|
||||
if ident.name.as_str() != "Boolean" || !ctx.is_reference_to_global_variable(ident) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
AstKind::WhileStatement(_)
|
||||
| AstKind::DoWhileStatement(_)
|
||||
| AstKind::IfStatement(_)
|
||||
| AstKind::UnaryExpression(_) => {}
|
||||
AstKind::UnaryExpression(unary_expr)
|
||||
if unary_expr.operator == UnaryOperator::LogicalNot => {}
|
||||
AstKind::WhileStatement(_) | AstKind::DoWhileStatement(_) | AstKind::IfStatement(_) => {
|
||||
}
|
||||
_ => return,
|
||||
}
|
||||
|
||||
@@ -196,6 +201,23 @@ fn test() {
|
||||
"if (foo.match(null)) {}",
|
||||
"if (foo.match(1n)) {}",
|
||||
"if (foo.match(true)) {}",
|
||||
"const value = a && s.match(/x/);",
|
||||
"const value = s.match(/x/) || a;",
|
||||
"if (s.match(/x/) ?? a) {}",
|
||||
"if (a ?? s.match(/x/)) {}",
|
||||
"for (a && s.match(/x/); ; ) {}",
|
||||
"for (; ; a || /x/.exec(s)) {}",
|
||||
"const value = a ? s.match(/x/) || b : b && /x/.exec(s);",
|
||||
"if (consume(a && s.match(/x/))) {}",
|
||||
"const value = typeof (a && s.match(/x/));",
|
||||
"const value = +(a || /x/.exec(s));",
|
||||
"Boolean(a && s.match(/x/), b);",
|
||||
"Boolean(...(a && s.match(/x/)));",
|
||||
"Boolean?.(a && s.match(/x/));",
|
||||
"function f(Boolean) { return Boolean(a && s.match(/x/)); }",
|
||||
"if ((a && s.match(/x/)) ?? b) {}",
|
||||
"if (a ?? (b || /x/.exec(s))) {}",
|
||||
"if (a && (b ?? s.match(/x/))) {}",
|
||||
];
|
||||
|
||||
let fail = vec![
|
||||
@@ -278,6 +300,16 @@ fn test() {
|
||||
if(str.match(re));",
|
||||
"!/a/u.exec(foo)",
|
||||
"!/a/v.exec(foo)",
|
||||
"export function f(a, s) {\n if (a && s.match(/x/)) {}\n if (s.match(/x/) || a) {}\n if (s.match(/x/)) {}\n}",
|
||||
"if (s.match(/x/) && a) {}",
|
||||
"if (a || s.match(/x/)) {}",
|
||||
"if ((a && (b || (s.match(/x/))))) {}",
|
||||
"while (a && /x/.exec(s)) {}",
|
||||
"do {} while (/x/.exec(s) || a);",
|
||||
"for (; (a && s.match(/x/)); ) {}",
|
||||
"const value = (s.match(/x/) || a) ? 1 : 2;",
|
||||
"const value = Boolean(a && s.match(/x/));",
|
||||
"const value = !(a || /x/.exec(s));",
|
||||
];
|
||||
|
||||
let fix = vec![
|
||||
@@ -310,6 +342,19 @@ fn test() {
|
||||
),
|
||||
("const re = /a/; if (re.exec(foo)) {}", "const re = /a/; if (re.test(foo)) {}"),
|
||||
("const re = /a/; if (someStr.match(re)) {}", "const re = /a/; if (re.test(someStr)) {}"),
|
||||
(
|
||||
"export function f(a, s) {\n if (a && s.match(/x/)) {}\n if (s.match(/x/) || a) {}\n if (s.match(/x/)) {}\n}",
|
||||
"export function f(a, s) {\n if (a && /x/.test(s)) {}\n if (/x/.test(s) || a) {}\n if (/x/.test(s)) {}\n}",
|
||||
),
|
||||
("if (s.match(/x/) && a) {}", "if (/x/.test(s) && a) {}"),
|
||||
("if (a || s.match(/x/)) {}", "if (a || /x/.test(s)) {}"),
|
||||
("if ((a && (b || (s.match(/x/))))) {}", "if ((a && (b || (/x/.test(s))))) {}"),
|
||||
("while (a && /x/.exec(s)) {}", "while (a && /x/.test(s)) {}"),
|
||||
("do {} while (/x/.exec(s) || a);", "do {} while (/x/.test(s) || a);"),
|
||||
("for (; (a && s.match(/x/)); ) {}", "for (; (a && /x/.test(s)); ) {}"),
|
||||
("const value = (s.match(/x/) || a) ? 1 : 2;", "const value = (/x/.test(s) || a) ? 1 : 2;"),
|
||||
("const value = Boolean(a && s.match(/x/));", "const value = Boolean(a && /x/.test(s));"),
|
||||
("const value = !(a || /x/.exec(s));", "const value = !(a || /x/.test(s));"),
|
||||
];
|
||||
|
||||
Tester::new(PreferRegexpTest::NAME, PreferRegexpTest::PLUGIN, pass, fail)
|
||||
|
||||
@@ -421,3 +421,93 @@ source: crates/oxc_linter/src/tester.rs
|
||||
· ────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:2:14]
|
||||
1 │ export function f(a, s) {
|
||||
2 │ if (a && s.match(/x/)) {}
|
||||
· ─────
|
||||
3 │ if (s.match(/x/) || a) {}
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:3:9]
|
||||
2 │ if (a && s.match(/x/)) {}
|
||||
3 │ if (s.match(/x/) || a) {}
|
||||
· ─────
|
||||
4 │ if (s.match(/x/)) {}
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:4:9]
|
||||
3 │ if (s.match(/x/) || a) {}
|
||||
4 │ if (s.match(/x/)) {}
|
||||
· ─────
|
||||
5 │ }
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:7]
|
||||
1 │ if (s.match(/x/) && a) {}
|
||||
· ─────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:12]
|
||||
1 │ if (a || s.match(/x/)) {}
|
||||
· ─────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:20]
|
||||
1 │ if ((a && (b || (s.match(/x/))))) {}
|
||||
· ─────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:17]
|
||||
1 │ while (a && /x/.exec(s)) {}
|
||||
· ────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:18]
|
||||
1 │ do {} while (/x/.exec(s) || a);
|
||||
· ────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:16]
|
||||
1 │ for (; (a && s.match(/x/)); ) {}
|
||||
· ─────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:18]
|
||||
1 │ const value = (s.match(/x/) || a) ? 1 : 2;
|
||||
· ─────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:30]
|
||||
1 │ const value = Boolean(a && s.match(/x/));
|
||||
· ─────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
|
||||
⚠ unicorn(prefer-regexp-test): Prefer `RegExp#test()` over `String#match()` and `RegExp#exec()`.
|
||||
╭─[prefer_regexp_test.tsx:1:26]
|
||||
1 │ const value = !(a || /x/.exec(s));
|
||||
· ────
|
||||
╰────
|
||||
help: `RegExp#test()` exclusively returns a boolean and therefore is more efficient.
|
||||
Reference in new issue
Block a user