Skip to content

Commit a0039fd

Browse files
feat(linter): implement noUnusedExpressions (#7511)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 527cec2 commit a0039fd

File tree

16 files changed

+1146
-36
lines changed

16 files changed

+1146
-36
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Added nursery rule [`noUnusedExpressions`](https://biomejs.dev/linter/rules/no-unused-expressions/) to flag expressions used as a statement that is neither an assignment nor a function call.
6+
7+
#### Invalid examples
8+
9+
```js
10+
f // intended to call `f()` instead
11+
```
12+
13+
```js
14+
function foo() {
15+
0 // intended to `return 0` instead
16+
}
17+
```
18+
19+
#### Valid examples
20+
21+
```js
22+
f()
23+
```
24+
25+
```js
26+
function foo() {
27+
return 0;
28+
}
29+
```

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: 54 additions & 33 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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ define_categories! {
162162
"lint/correctness/useValidTypeof": "https://biomejs.dev/linter/rules/use-valid-typeof",
163163
"lint/correctness/useYield": "https://biomejs.dev/linter/rules/use-yield",
164164
"lint/nursery/noColorInvalidHex": "https://biomejs.dev/linter/rules/no-color-invalid-hex",
165+
"lint/nursery/noDuplicateDependencies": "https://biomejs.dev/linter/rules/no-duplicate-dependencies",
165166
"lint/nursery/noFloatingPromises": "https://biomejs.dev/linter/rules/no-floating-promises",
166167
"lint/nursery/noImplicitCoercion": "https://biomejs.dev/linter/rules/no-implicit-coercion",
167168
"lint/nursery/noImportCycles": "https://biomejs.dev/linter/rules/no-import-cycles",
@@ -175,6 +176,7 @@ define_categories! {
175176
"lint/nursery/noShadow": "https://biomejs.dev/linter/rules/no-shadow",
176177
"lint/nursery/noUnnecessaryConditions": "https://biomejs.dev/linter/rules/no-unnecessary-conditions",
177178
"lint/nursery/noUnresolvedImports": "https://biomejs.dev/linter/rules/no-unresolved-imports",
179+
"lint/nursery/noUnusedExpressions": "https://biomejs.dev/linter/rules/no-unused-expressions",
178180
"lint/nursery/noUnwantedPolyfillio": "https://biomejs.dev/linter/rules/no-unwanted-polyfillio",
179181
"lint/nursery/noUselessBackrefInRegex": "https://biomejs.dev/linter/rules/no-useless-backref-in-regex",
180182
"lint/nursery/noUselessCatchBinding": "https://biomejs.dev/linter/rules/no-useless-catch-binding",
@@ -312,7 +314,6 @@ define_categories! {
312314
"lint/suspicious/noDuplicateCase": "https://biomejs.dev/linter/rules/no-duplicate-case",
313315
"lint/suspicious/noDuplicateClassMembers": "https://biomejs.dev/linter/rules/no-duplicate-class-members",
314316
"lint/suspicious/noDuplicateCustomProperties": "https://biomejs.dev/linter/rules/no-duplicate-custom-properties",
315-
"lint/nursery/noDuplicateDependencies": "https://biomejs.dev/linter/rules/no-duplicate-dependencies",
316317
"lint/suspicious/noDuplicateElseIf": "https://biomejs.dev/linter/rules/no-duplicate-else-if",
317318
"lint/suspicious/noDuplicateFields": "https://biomejs.dev/linter/rules/no-duplicate-fields",
318319
"lint/suspicious/noDuplicateFontNames": "https://biomejs.dev/linter/rules/no-font-family-duplicate-names",

crates/biome_js_analyze/src/lint/nursery.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub mod no_secrets;
1414
pub mod no_shadow;
1515
pub mod no_unnecessary_conditions;
1616
pub mod no_unresolved_imports;
17+
pub mod no_unused_expressions;
1718
pub mod no_useless_catch_binding;
1819
pub mod no_useless_undefined;
1920
pub mod no_vue_data_object_declaration;
@@ -30,4 +31,4 @@ pub mod use_qwik_classlist;
3031
pub mod use_react_function_components;
3132
pub mod use_sorted_classes;
3233
pub mod use_vue_multi_word_component_names;
33-
declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_floating_promises :: NoFloatingPromises , self :: no_import_cycles :: NoImportCycles , self :: no_jsx_literals :: NoJsxLiterals , self :: no_misused_promises :: NoMisusedPromises , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_non_null_asserted_optional_chain :: NoNonNullAssertedOptionalChain , self :: no_qwik_use_visible_task :: NoQwikUseVisibleTask , self :: no_secrets :: NoSecrets , self :: no_shadow :: NoShadow , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: use_anchor_href :: UseAnchorHref , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_consistent_type_definitions :: UseConsistentTypeDefinitions , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_image_size :: UseImageSize , self :: use_max_params :: UseMaxParams , self :: use_qwik_classlist :: UseQwikClasslist , self :: use_react_function_components :: UseReactFunctionComponents , self :: use_sorted_classes :: UseSortedClasses , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } }
34+
declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_floating_promises :: NoFloatingPromises , self :: no_import_cycles :: NoImportCycles , self :: no_jsx_literals :: NoJsxLiterals , self :: no_misused_promises :: NoMisusedPromises , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_non_null_asserted_optional_chain :: NoNonNullAssertedOptionalChain , self :: no_qwik_use_visible_task :: NoQwikUseVisibleTask , self :: no_secrets :: NoSecrets , self :: no_shadow :: NoShadow , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: use_anchor_href :: UseAnchorHref , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_consistent_type_definitions :: UseConsistentTypeDefinitions , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_image_size :: UseImageSize , self :: use_max_params :: UseMaxParams , self :: use_qwik_classlist :: UseQwikClasslist , self :: use_react_function_components :: UseReactFunctionComponents , self :: use_sorted_classes :: UseSortedClasses , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } }
Lines changed: 301 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,301 @@
1+
use std::borrow::Cow;
2+
3+
use biome_analyze::{
4+
Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
5+
};
6+
use biome_console::markup;
7+
use biome_js_syntax::{
8+
AnyJsExpression, AnyJsFunction, AnyJsLiteralExpression, AnyJsRoot, JsBlockStatement,
9+
JsExpressionStatement, JsSyntaxNode, JsUnaryOperator, TsModuleBlock,
10+
};
11+
use biome_rowan::{AstNode, SyntaxResult};
12+
use biome_rule_options::no_unused_expressions::NoUnusedExpressionsOptions;
13+
14+
declare_lint_rule! {
15+
/// Disallow expression statements that are neither a function call nor an
16+
/// assignment.
17+
///
18+
/// When an expression is used as a statement, it should be explicitly clear
19+
/// what the intention behind the expression is. This is clear for function
20+
/// calls and assignments, because the call or the assignment itself is the
21+
/// primary intention behind the statement. For other expression kinds, the
22+
/// intention is much more ambiguous; it could be the expression contains
23+
/// side-effects that are not very explicit, but it could also be that it is
24+
/// an error where the author forgot to use the result of the expression,
25+
/// such as a forgotten `return` keyword, or it could point to a function
26+
/// that the author forgot to call.
27+
///
28+
/// ## Examples
29+
///
30+
/// ### Invalid
31+
///
32+
/// ```js,expect_diagnostic
33+
/// 0
34+
/// ```
35+
///
36+
/// ```js,expect_diagnostic
37+
/// if(0) 0
38+
/// ```
39+
///
40+
/// ```js,expect_diagnostic
41+
/// {0}
42+
/// ```
43+
///
44+
/// ```js,expect_diagnostic
45+
/// f(0), {}
46+
/// ```
47+
///
48+
/// ```js,expect_diagnostic
49+
/// a && b()
50+
/// ```
51+
///
52+
/// ```js,expect_diagnostic
53+
/// a, b()
54+
/// ```
55+
///
56+
/// ```js,expect_diagnostic
57+
/// c = a, b
58+
/// ```
59+
///
60+
/// ```js,expect_diagnostic
61+
/// a() && function namedFunctionInExpressionContext () {f();}
62+
/// ```
63+
///
64+
/// ```js,expect_diagnostic
65+
/// (function anIncompleteIIFE () {});
66+
/// ```
67+
///
68+
/// ```js,expect_diagnostic
69+
/// injectGlobal`body{ color: red; }`
70+
/// ```
71+
///
72+
/// ```ts,expect_diagnostic
73+
/// Set<number>
74+
/// ```
75+
///
76+
/// ```ts,expect_diagnostic
77+
/// 1 as number
78+
/// ```
79+
///
80+
/// ```ts,expect_diagnostic
81+
/// window!
82+
/// ```
83+
///
84+
/// JSX expressions are considered invalid when used as a statement too:
85+
///
86+
/// ```jsx,expect_diagnostic
87+
/// <MyComponent />
88+
/// ```
89+
///
90+
/// ```jsx,expect_diagnostic
91+
/// <></>
92+
/// ```
93+
///
94+
/// ### Valid
95+
///
96+
/// ```js
97+
/// {} // In this context, this is a block statement, not an object literal
98+
///
99+
/// { myLabel: foo() } // In this context, this is a block statement with a label and expression, not an object literal
100+
///
101+
/// function namedFunctionDeclaration () {}
102+
///
103+
/// (function aGenuineIIFE () {}());
104+
///
105+
/// f()
106+
///
107+
/// a = 0
108+
///
109+
/// new C
110+
///
111+
/// delete a.b
112+
///
113+
/// void a
114+
/// ```
115+
///
116+
/// ### Handling of Directives
117+
///
118+
/// Any stand-alone string at the start of a script, module, or function is
119+
/// considered a directive and is therefore allowed.
120+
///
121+
/// ```js
122+
/// "use strict";
123+
/// "use asm"
124+
/// "use stricter";
125+
/// "use babel"
126+
/// "any other strings like this in the directive prologue";
127+
/// "this is still the directive prologue";
128+
///
129+
/// function foo() {
130+
/// "bar";
131+
/// }
132+
///
133+
/// class Foo {
134+
/// someMethod() {
135+
/// "use strict";
136+
/// }
137+
/// }
138+
/// ```
139+
///
140+
/// The following are **not** considered valid directives:
141+
///
142+
/// ```js,expect_diagnostic
143+
/// doSomething();
144+
/// "use strict"; // this isn't in a directive prologue, because there is a non-directive statement before it
145+
/// ```
146+
///
147+
/// ```js,expect_diagnostic
148+
/// function foo() {
149+
/// "bar" + 1;
150+
/// }
151+
/// ```
152+
///
153+
/// ```js,expect_diagnostic
154+
/// class Foo {
155+
/// static {
156+
/// "use strict"; // class static blocks do not have directive prologues
157+
/// }
158+
/// }
159+
/// ```
160+
pub NoUnusedExpressions {
161+
version: "next",
162+
name: "noUnusedExpressions",
163+
language: "js",
164+
sources: &[RuleSource::Eslint("no-unused-expressions").same()],
165+
recommended: false,
166+
}
167+
}
168+
169+
impl Rule for NoUnusedExpressions {
170+
type Query = Ast<JsExpressionStatement>;
171+
type State = ();
172+
type Signals = Option<Self::State>;
173+
type Options = NoUnusedExpressionsOptions;
174+
175+
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
176+
let node = ctx.query();
177+
178+
if is_directive(node) {
179+
return None;
180+
}
181+
182+
is_disallowed(&node.expression().ok()?).ok()?.then_some(())
183+
}
184+
185+
fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
186+
let node = ctx.query();
187+
Some(
188+
RuleDiagnostic::new(
189+
rule_category!(),
190+
node.range(),
191+
markup! {
192+
"Expected an assignment or function call but found an expression instead."
193+
},
194+
)
195+
.note(markup! {
196+
"This expression may be unintentionally unused or it might be a function that you forgot to call."
197+
}),
198+
)
199+
}
200+
}
201+
202+
/// Determines if a node should be treated as a directive.
203+
///
204+
/// Only string literals are treated as directives, and only if they are not
205+
/// preceeded by any non-directive nodes.
206+
fn is_directive(node: &JsExpressionStatement) -> bool {
207+
if node
208+
.syntax()
209+
.parent()
210+
.is_some_and(|parent| !is_at_top_level(&parent))
211+
{
212+
return false;
213+
}
214+
215+
let mut node = Cow::Borrowed(node);
216+
loop {
217+
if !looks_like_directive(&node) {
218+
break false;
219+
}
220+
221+
let Some(prev_node) = node.syntax().prev_sibling() else {
222+
break true;
223+
};
224+
match JsExpressionStatement::cast(prev_node) {
225+
Some(prev_expr) => node = Cow::Owned(prev_expr),
226+
None => break false,
227+
}
228+
}
229+
}
230+
231+
/// Determines if a node exists at the top-level based on its `parent` node.
232+
fn is_at_top_level(parent: &JsSyntaxNode) -> bool {
233+
AnyJsRoot::can_cast(parent.kind())
234+
|| TsModuleBlock::can_cast(parent.kind())
235+
|| JsBlockStatement::cast_ref(parent).is_some_and(|block| {
236+
block
237+
.syntax()
238+
.parent()
239+
.is_some_and(|parent: JsSyntaxNode| AnyJsFunction::can_cast(parent.kind()))
240+
})
241+
}
242+
243+
fn looks_like_directive(node: &JsExpressionStatement) -> bool {
244+
matches!(
245+
node.expression(),
246+
Ok(AnyJsExpression::AnyJsLiteralExpression(
247+
AnyJsLiteralExpression::JsStringLiteralExpression(_)
248+
))
249+
)
250+
}
251+
252+
fn is_disallowed(expr: &AnyJsExpression) -> SyntaxResult<bool> {
253+
let is_disallowed = match expr {
254+
AnyJsExpression::AnyJsLiteralExpression(_)
255+
| AnyJsExpression::JsArrayExpression(_)
256+
| AnyJsExpression::JsArrowFunctionExpression(_)
257+
| AnyJsExpression::JsBinaryExpression(_)
258+
| AnyJsExpression::JsClassExpression(_)
259+
| AnyJsExpression::JsComputedMemberExpression(_)
260+
| AnyJsExpression::JsConditionalExpression(_)
261+
| AnyJsExpression::JsFunctionExpression(_)
262+
| AnyJsExpression::JsIdentifierExpression(_)
263+
| AnyJsExpression::JsImportMetaExpression(_)
264+
| AnyJsExpression::JsInExpression(_)
265+
| AnyJsExpression::JsInstanceofExpression(_)
266+
| AnyJsExpression::JsLogicalExpression(_)
267+
| AnyJsExpression::JsNewTargetExpression(_)
268+
| AnyJsExpression::JsObjectExpression(_)
269+
| AnyJsExpression::JsSequenceExpression(_)
270+
| AnyJsExpression::JsStaticMemberExpression(_)
271+
| AnyJsExpression::JsSuperExpression(_)
272+
| AnyJsExpression::JsTemplateExpression(_)
273+
| AnyJsExpression::JsThisExpression(_)
274+
| AnyJsExpression::JsxTagExpression(_) => true,
275+
AnyJsExpression::JsAwaitExpression(_)
276+
| AnyJsExpression::JsAssignmentExpression(_)
277+
| AnyJsExpression::JsCallExpression(_)
278+
| AnyJsExpression::JsImportCallExpression(_)
279+
| AnyJsExpression::JsNewExpression(_)
280+
| AnyJsExpression::JsYieldExpression(_)
281+
| AnyJsExpression::JsPostUpdateExpression(_)
282+
| AnyJsExpression::JsPreUpdateExpression(_)
283+
| AnyJsExpression::JsBogusExpression(_)
284+
| AnyJsExpression::JsMetavariable(_) => false,
285+
AnyJsExpression::JsParenthesizedExpression(expr) => is_disallowed(&expr.expression()?)?,
286+
AnyJsExpression::JsUnaryExpression(expr) => match expr.operator()? {
287+
JsUnaryOperator::BitwiseNot
288+
| JsUnaryOperator::LogicalNot
289+
| JsUnaryOperator::Minus
290+
| JsUnaryOperator::Plus
291+
| JsUnaryOperator::Typeof => true,
292+
JsUnaryOperator::Delete | JsUnaryOperator::Void => false,
293+
},
294+
AnyJsExpression::TsAsExpression(expr) => is_disallowed(&expr.expression()?)?,
295+
AnyJsExpression::TsInstantiationExpression(expr) => is_disallowed(&expr.expression()?)?,
296+
AnyJsExpression::TsNonNullAssertionExpression(expr) => is_disallowed(&expr.expression()?)?,
297+
AnyJsExpression::TsSatisfiesExpression(expr) => is_disallowed(&expr.expression()?)?,
298+
AnyJsExpression::TsTypeAssertionExpression(expr) => is_disallowed(&expr.expression()?)?,
299+
};
300+
Ok(is_disallowed)
301+
}

0 commit comments

Comments
 (0)