Skip to content

Commit 7f67749

Browse files
authored
fix(lint/js): improve diagnostics to better follow rule pillars (#9880)
1 parent fc9d715 commit 7f67749

9 files changed

Lines changed: 183 additions & 114 deletions

File tree

.changeset/empty-dryers-fix.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Improved the diagnostics for [`useFind`](https://biomejs.dev/linter/rules/use-find/) to better explain the problem, why it matters, and how to fix it.

.changeset/fuzzy-beers-shine.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Improved the diagnostics for [`useRegexpExec`](https://biomejs.dev/linter/rules/use-regexp-exec/) to better explain the problem, why it matters, and how to fix it.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Improved the diagnostics for [`useArraySome`](https://biomejs.dev/linter/rules/use-array-some/) to better explain the problem, why it matters, and how to fix it.

crates/biome_js_analyze/src/lint/nursery/use_array_some.rs

Lines changed: 76 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -128,30 +128,57 @@ impl Rule for UseArraySome {
128128
*pattern
129129
}
130130
};
131-
let description = match pattern {
132-
DetectedPattern::FilterLengthComparison => "filter(...).length comparison",
133-
DetectedPattern::FindIndexComparison => "findIndex(...) !== -1",
134-
DetectedPattern::FindLastIndexComparison => "findLastIndex(...) !== -1",
135-
DetectedPattern::FindExistenceComparison => "find(...) existence comparison",
136-
DetectedPattern::FindLastExistenceComparison => "findLast(...) existence comparison",
137-
DetectedPattern::FindFamilyAsBoolean => "find-family used as boolean",
131+
let problem = match pattern {
132+
DetectedPattern::FilterLengthComparison => {
133+
markup! { "This expression uses "<Emphasis>".filter()"</Emphasis>" and then checks whether the result is empty." }
134+
}
135+
DetectedPattern::FindIndexComparison => {
136+
markup! { "This expression uses "<Emphasis>".findIndex()"</Emphasis>" to test whether any array element matches." }
137+
}
138+
DetectedPattern::FindLastIndexComparison => {
139+
markup! { "This expression uses "<Emphasis>".findLastIndex()"</Emphasis>" to test whether any array element matches." }
140+
}
141+
DetectedPattern::FindExistenceComparison => {
142+
markup! { "This expression uses "<Emphasis>".find()"</Emphasis>" only to check whether an array element exists." }
143+
}
144+
DetectedPattern::FindLastExistenceComparison => {
145+
markup! { "This expression uses "<Emphasis>".findLast()"</Emphasis>" only to check whether an array element exists." }
146+
}
147+
DetectedPattern::FindFamilyAsBoolean => {
148+
markup! { "This expression uses a "<Emphasis>".find()"</Emphasis>"-style call as a boolean test." }
149+
}
138150
};
139151

140-
let mut diag = RuleDiagnostic::new(
141-
rule_category!(),
142-
range,
143-
markup! {
144-
"Prefer "<Emphasis>".some()"</Emphasis>" over "<Emphasis>{description}</Emphasis>"."
145-
},
146-
);
147-
if matches!(pattern, DetectedPattern::FilterLengthComparison) {
148-
diag = diag.note(markup! {
149-
"Using "<Emphasis>".filter()"</Emphasis>" followed by a length check iterates the entire array unnecessarily. "<Emphasis>".some()"</Emphasis>" stops at the first match."
150-
});
152+
let reason = match pattern {
153+
DetectedPattern::FilterLengthComparison => {
154+
markup! { <Emphasis>".some()"</Emphasis>" matches that intent better and can stop as soon as it finds a match." }
155+
}
156+
_ => {
157+
markup! { <Emphasis>".some()"</Emphasis>" makes that intent clearer because it returns a boolean directly." }
158+
}
159+
};
160+
161+
let has_action = matches!(state, UseArraySomeState::Fix { .. });
162+
163+
let mut diag = RuleDiagnostic::new(rule_category!(), range, problem);
164+
diag = diag.note(reason);
165+
if !has_action {
166+
let how = match pattern {
167+
DetectedPattern::FilterLengthComparison
168+
| DetectedPattern::FindIndexComparison
169+
| DetectedPattern::FindLastIndexComparison
170+
| DetectedPattern::FindFamilyAsBoolean => {
171+
markup! { "Use "<Emphasis>".some()"</Emphasis>" instead." }
172+
}
173+
DetectedPattern::FindExistenceComparison => {
174+
markup! { "Use "<Emphasis>".some()"</Emphasis>" if you only need to know whether any element matches." }
175+
}
176+
DetectedPattern::FindLastExistenceComparison => {
177+
markup! { "Use "<Emphasis>".some()"</Emphasis>" if you only need to know whether any element matches, regardless of order." }
178+
}
179+
};
180+
diag = diag.note(how);
151181
}
152-
diag = diag.note(markup! {
153-
"Use "<Emphasis>".some()"</Emphasis>" when you only need to know if any element matches."
154-
});
155182
Some(diag)
156183
}
157184

@@ -173,7 +200,10 @@ impl Rule for UseArraySome {
173200
let updated_call = call.clone().with_callee(updated_callee);
174201

175202
let mut mutation = ctx.root().begin();
176-
mutation.replace_node(replace_node.clone(), AnyJsExpression::JsCallExpression(updated_call));
203+
mutation.replace_node(
204+
replace_node.clone(),
205+
AnyJsExpression::JsCallExpression(updated_call),
206+
);
177207

178208
Some(JsRuleAction::new(
179209
ctx.metadata().action_category(ctx.category(), ctx.group()),
@@ -206,23 +236,22 @@ fn detect_filter_length_pattern(call: &JsCallExpression) -> Option<UseArraySomeS
206236
let operator = comparison.operator().ok()?;
207237

208238
// Determine which side is the length expression and normalize the operator
209-
let (literal, normalized_op) =
210-
if expression_matches_target(&left, parent_member.syntax()) {
211-
// arr.filter(...).length OP literal
212-
(right, operator)
213-
} else if expression_matches_target(&right, parent_member.syntax()) {
214-
// literal OP arr.filter(...).length — swap the operator
215-
let swapped = match operator {
216-
JsBinaryOperator::GreaterThan => JsBinaryOperator::LessThan,
217-
JsBinaryOperator::GreaterThanOrEqual => JsBinaryOperator::LessThanOrEqual,
218-
JsBinaryOperator::LessThan => JsBinaryOperator::GreaterThan,
219-
JsBinaryOperator::LessThanOrEqual => JsBinaryOperator::GreaterThanOrEqual,
220-
other => other, // symmetric operators like !== stay the same
221-
};
222-
(left, swapped)
223-
} else {
224-
return None;
239+
let (literal, normalized_op) = if expression_matches_target(&left, parent_member.syntax()) {
240+
// arr.filter(...).length OP literal
241+
(right, operator)
242+
} else if expression_matches_target(&right, parent_member.syntax()) {
243+
// literal OP arr.filter(...).length — swap the operator
244+
let swapped = match operator {
245+
JsBinaryOperator::GreaterThan => JsBinaryOperator::LessThan,
246+
JsBinaryOperator::GreaterThanOrEqual => JsBinaryOperator::LessThanOrEqual,
247+
JsBinaryOperator::LessThan => JsBinaryOperator::GreaterThan,
248+
JsBinaryOperator::LessThanOrEqual => JsBinaryOperator::GreaterThanOrEqual,
249+
other => other, // symmetric operators like !== stay the same
225250
};
251+
(left, swapped)
252+
} else {
253+
return None;
254+
};
226255

227256
let matches = match normalized_op {
228257
JsBinaryOperator::GreaterThan => is_number_literal_value(&literal, 0.0),
@@ -288,8 +317,8 @@ fn detect_find_existence_comparison_pattern(
288317
}
289318

290319
let other = if left_matches_call { right } else { left };
291-
let strict_undefined = matches!(operator, JsBinaryOperator::StrictInequality)
292-
&& is_undefined_expression(&other);
320+
let strict_undefined =
321+
matches!(operator, JsBinaryOperator::StrictInequality) && is_undefined_expression(&other);
293322
let loose_nullish =
294323
matches!(operator, JsBinaryOperator::Inequality) && is_nullish_expression(&other);
295324

@@ -318,11 +347,7 @@ fn nearest_parent_binary_expression(node: &JsSyntaxNode) -> Option<JsBinaryExpre
318347
}
319348

320349
fn expression_matches_target(expression: &AnyJsExpression, target: &JsSyntaxNode) -> bool {
321-
expression
322-
.clone()
323-
.omit_parentheses()
324-
.syntax()
325-
.eq(target)
350+
expression.clone().omit_parentheses().syntax().eq(target)
326351
}
327352

328353
fn is_number_literal_value(expression: &AnyJsExpression, value: f64) -> bool {
@@ -361,7 +386,12 @@ fn is_undefined_expression(expression: &AnyJsExpression) -> bool {
361386
.clone()
362387
.omit_parentheses()
363388
.as_static_value()
364-
.is_some_and(|value| matches!(value, biome_js_syntax::static_value::StaticValue::Undefined(_)))
389+
.is_some_and(|value| {
390+
matches!(
391+
value,
392+
biome_js_syntax::static_value::StaticValue::Undefined(_)
393+
)
394+
})
365395
}
366396

367397
fn is_nullish_expression(expression: &AnyJsExpression) -> bool {

crates/biome_js_analyze/src/lint/nursery/use_find.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,14 @@ impl Rule for UseFind {
109109
rule_category!(),
110110
state,
111111
markup! {
112-
"Prefer using Array#find() over Array#filter[0]."
112+
"This call uses "<Emphasis>"Array#filter()"</Emphasis>" to retrieve a single matching element."
113113
},
114114
)
115115
.note(markup! {
116-
"Use Array#find() instead of Array#filter[0] to improve readability."
116+
<Emphasis>"Array#find()"</Emphasis>" expresses that intent more clearly and can stop once it finds the first match."
117+
})
118+
.note(markup! {
119+
"Use "<Emphasis>"Array#find()"</Emphasis>" instead."
117120
}),
118121
)
119122
}

crates/biome_js_analyze/src/lint/nursery/use_regexp_exec.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,14 @@ impl Rule for UseRegexpExec {
9595
rule_category!(),
9696
node.range(),
9797
markup! {
98-
"Prefer "<Emphasis>"RegExp#exec()"</Emphasis>" over "<Emphasis>"String#match()"</Emphasis>" when searching within a string."
98+
"This call uses "<Emphasis>"String#match()"</Emphasis>" with a regular expression."
9999
},
100100
)
101101
.note(markup! {
102-
"Use "<Emphasis>"RegExp#exec()"</Emphasis>" instead of "<Emphasis>"String#match()"</Emphasis>" for consistent and slightly faster regex matching."
102+
<Emphasis>"RegExp#exec()"</Emphasis>" communicates the regex operation more directly and keeps this pattern consistent."
103+
})
104+
.note(markup! {
105+
"Use "<Emphasis>"RegExp#exec()"</Emphasis>" instead."
103106
}),
104107
)
105108
}

0 commit comments

Comments
 (0)