Skip to content

Commit 4f7de70

Browse files
kaykdmunvalley
kaykdm
authored and
unvalley
committed
feat(lint): support assigned functions in noFloatingPromises (biomejs#4958)
1 parent eb3f131 commit 4f7de70

File tree

3 files changed

+196
-42
lines changed

3 files changed

+196
-42
lines changed

‎crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs

+100-41
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ use biome_js_factory::make;
66
use biome_js_semantic::SemanticModel;
77
use biome_js_syntax::{
88
binding_ext::AnyJsBindingDeclaration, AnyJsExpression, AnyJsName, AnyTsName, AnyTsReturnType,
9-
AnyTsType, JsArrowFunctionExpression, JsCallExpression, JsExpressionStatement,
10-
JsFunctionDeclaration, JsMethodClassMember, JsMethodObjectMember, JsStaticMemberExpression,
11-
JsSyntaxKind, TsReturnTypeAnnotation,
9+
AnyTsType, AnyTsVariableAnnotation, JsArrowFunctionExpression, JsCallExpression,
10+
JsExpressionStatement, JsFunctionDeclaration, JsMethodClassMember, JsMethodObjectMember,
11+
JsStaticMemberExpression, JsSyntaxKind, JsVariableDeclarator, TsReturnTypeAnnotation,
1212
};
1313
use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt, SyntaxNodeCast, TriviaPieceKind};
1414

@@ -45,6 +45,15 @@ declare_lint_rule! {
4545
/// returnsPromise().then(() => {});
4646
/// ```
4747
///
48+
/// ```ts,expect_diagnostic
49+
/// const returnsPromise = async (): Promise<string> => {
50+
/// return 'value';
51+
/// }
52+
/// async function returnsPromiseInAsyncFunction() {
53+
/// returnsPromise().then(() => {});
54+
/// }
55+
/// ```
56+
///
4857
/// ### Valid
4958
///
5059
/// ```ts
@@ -195,24 +204,27 @@ fn is_callee_a_promise(callee: &AnyJsExpression, model: &SemanticModel) -> bool
195204
let Some(any_js_binding_decl) = binding.tree().declaration() else {
196205
return false;
197206
};
198-
199-
let AnyJsBindingDeclaration::JsFunctionDeclaration(func_decl) = any_js_binding_decl
200-
else {
201-
return false;
202-
};
203-
204-
return is_function_a_promise(&func_decl);
207+
match any_js_binding_decl {
208+
AnyJsBindingDeclaration::JsFunctionDeclaration(func_decl) => {
209+
is_function_a_promise(&func_decl)
210+
}
211+
AnyJsBindingDeclaration::JsVariableDeclarator(js_var_decl) => {
212+
is_variable_initializer_a_promise(&js_var_decl)
213+
|| is_variable_annotation_a_promise(&js_var_decl)
214+
}
215+
_ => false,
216+
}
205217
}
206218
AnyJsExpression::JsStaticMemberExpression(static_member_expr) => {
207-
return is_member_expression_callee_a_promise(static_member_expr, model);
219+
is_member_expression_callee_a_promise(static_member_expr, model)
208220
}
209-
_ => {}
221+
_ => false,
210222
}
211-
false
212223
}
213224

214225
fn is_function_a_promise(func_decl: &JsFunctionDeclaration) -> bool {
215-
func_decl.async_token().is_some() || is_return_type_promise(func_decl.return_type_annotation())
226+
func_decl.async_token().is_some()
227+
|| is_return_type_a_promise(func_decl.return_type_annotation())
216228
}
217229

218230
/// Checks if a TypeScript return type annotation is a `Promise`.
@@ -240,7 +252,7 @@ fn is_function_a_promise(func_decl: &JsFunctionDeclaration) -> bool {
240252
/// ```typescript
241253
/// function doesNotReturnPromise(): void {}
242254
/// ```
243-
fn is_return_type_promise(return_type: Option<TsReturnTypeAnnotation>) -> bool {
255+
fn is_return_type_a_promise(return_type: Option<TsReturnTypeAnnotation>) -> bool {
244256
return_type
245257
.and_then(|ts_return_type_anno| ts_return_type_anno.ty().ok())
246258
.and_then(|any_ts_return_type| match any_ts_return_type {
@@ -360,32 +372,7 @@ fn is_member_expression_callee_a_promise(
360372
return false;
361373
};
362374

363-
match callee {
364-
AnyJsExpression::JsStaticMemberExpression(static_member_expr) => {
365-
return is_member_expression_callee_a_promise(&static_member_expr, model);
366-
}
367-
AnyJsExpression::JsIdentifierExpression(ident_expr) => {
368-
let Some(reference) = ident_expr.name().ok() else {
369-
return false;
370-
};
371-
let Some(binding) = model.binding(&reference) else {
372-
return false;
373-
};
374-
375-
let Some(any_js_binding_decl) = binding.tree().declaration() else {
376-
return false;
377-
};
378-
379-
let AnyJsBindingDeclaration::JsFunctionDeclaration(func_decl) = any_js_binding_decl
380-
else {
381-
return false;
382-
};
383-
return is_function_a_promise(&func_decl);
384-
}
385-
_ => {}
386-
}
387-
388-
false
375+
is_callee_a_promise(&callee, model)
389376
}
390377

391378
/// Checks if the given `JsExpressionStatement` is within an async function.
@@ -423,3 +410,75 @@ fn is_in_async_function(node: &JsExpressionStatement) -> bool {
423410
})
424411
.is_some()
425412
}
413+
414+
/// Checks if the initializer of a `JsVariableDeclarator` is an async function.
415+
///
416+
/// Example TypeScript code that would return `true`:
417+
///
418+
/// ```typescript
419+
/// const returnsPromise = async (): Promise<string> => {
420+
/// return 'value';
421+
/// }
422+
///
423+
/// const returnsPromise = async function (): Promise<string> {
424+
/// return 'value'
425+
/// }
426+
/// ```
427+
fn is_variable_initializer_a_promise(js_variable_declarator: &JsVariableDeclarator) -> bool {
428+
let Some(initializer_clause) = &js_variable_declarator.initializer() else {
429+
return false;
430+
};
431+
let Ok(expr) = initializer_clause.expression() else {
432+
return false;
433+
};
434+
match expr {
435+
AnyJsExpression::JsArrowFunctionExpression(arrow_func) => {
436+
arrow_func.async_token().is_some()
437+
|| is_return_type_a_promise(arrow_func.return_type_annotation())
438+
}
439+
AnyJsExpression::JsFunctionExpression(func_expr) => {
440+
func_expr.async_token().is_some()
441+
|| is_return_type_a_promise(func_expr.return_type_annotation())
442+
}
443+
_ => false,
444+
}
445+
}
446+
447+
/// Checks if a `JsVariableDeclarator` has a TypeScript type annotation of `Promise`.
448+
///
449+
///
450+
/// Example TypeScript code that would return `true`:
451+
/// ```typescript
452+
/// const returnsPromise: () => Promise<string> = () => {
453+
/// return Promise.resolve("value")
454+
/// }
455+
/// ```
456+
fn is_variable_annotation_a_promise(js_variable_declarator: &JsVariableDeclarator) -> bool {
457+
js_variable_declarator
458+
.variable_annotation()
459+
.and_then(|anno| match anno {
460+
AnyTsVariableAnnotation::TsTypeAnnotation(type_anno) => Some(type_anno),
461+
_ => None,
462+
})
463+
.and_then(|ts_type_anno| ts_type_anno.ty().ok())
464+
.and_then(|any_ts_type| match any_ts_type {
465+
AnyTsType::TsFunctionType(func_type) => {
466+
func_type
467+
.return_type()
468+
.ok()
469+
.and_then(|return_type| match return_type {
470+
AnyTsReturnType::AnyTsType(AnyTsType::TsReferenceType(ref_type)) => {
471+
ref_type.name().ok().map(|name| match name {
472+
AnyTsName::JsReferenceIdentifier(identifier) => {
473+
identifier.has_name("Promise")
474+
}
475+
_ => false,
476+
})
477+
}
478+
_ => None,
479+
})
480+
}
481+
_ => None,
482+
})
483+
.unwrap_or(false)
484+
}

‎crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts

+22-1
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,25 @@ function returnsPromiseWithoutAsync(): Promise<string> {
2424
}
2525

2626

27-
returnsPromiseWithoutAsync()
27+
returnsPromiseWithoutAsync()
28+
29+
30+
const returnsPromiseAssignedArrowFunction = async (): Promise<string> => {
31+
return 'value';
32+
};
33+
34+
returnsPromiseAssignedArrowFunction();
35+
36+
const returnsPromiseAssignedFunction = async function (): Promise<string> {
37+
return 'value'
38+
}
39+
40+
async function returnsPromiseAssignedFunctionInAsyncFunction(): Promise<void> {
41+
returnsPromiseAssignedFunction().then(() => { })
42+
}
43+
44+
const returnsPromiseAssignedArrowFunctionAnnotatedType: () => Promise<string> = () => {
45+
return Promise.resolve('value');
46+
};
47+
48+
returnsPromiseAssignedArrowFunctionAnnotatedType();

‎crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts.snap

+74
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,27 @@ function returnsPromiseWithoutAsync(): Promise<string> {
3131
3232
3333
returnsPromiseWithoutAsync()
34+
35+
36+
const returnsPromiseAssignedArrowFunction = async (): Promise<string> => {
37+
return 'value';
38+
};
39+
40+
returnsPromiseAssignedArrowFunction();
41+
42+
const returnsPromiseAssignedFunction = async function (): Promise<string> {
43+
return 'value'
44+
}
45+
46+
async function returnsPromiseAssignedFunctionInAsyncFunction(): Promise<void> {
47+
returnsPromiseAssignedFunction().then(() => { })
48+
}
49+
50+
const returnsPromiseAssignedArrowFunctionAnnotatedType: () => Promise<string> = () => {
51+
return Promise.resolve('value');
52+
};
53+
54+
returnsPromiseAssignedArrowFunctionAnnotatedType();
3455
```
3556
3657
# Diagnostics
@@ -136,6 +157,59 @@ invalid.ts:27:1 lint/nursery/noFloatingPromises ━━━━━━━━━━
136157
137158
> 27 │ returnsPromiseWithoutAsync()
138159
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
160+
28 │
161+
162+
i This happens when a Promise is not awaited, lacks a `.catch` or `.then` rejection handler, or is not explicitly ignored using the `void` operator.
163+
164+
165+
```
166+
167+
```
168+
invalid.ts:34:1 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
169+
170+
! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior.
171+
172+
32 │ };
173+
33 │
174+
> 34 │ returnsPromiseAssignedArrowFunction();
175+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
176+
35 │
177+
36 │ const returnsPromiseAssignedFunction = async function (): Promise<string> {
178+
179+
i This happens when a Promise is not awaited, lacks a `.catch` or `.then` rejection handler, or is not explicitly ignored using the `void` operator.
180+
181+
182+
```
183+
184+
```
185+
invalid.ts:41:3 lint/nursery/noFloatingPromises FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
186+
187+
! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior.
188+
189+
40 │ async function returnsPromiseAssignedFunctionInAsyncFunction(): Promise<void> {
190+
> 41 │ returnsPromiseAssignedFunction().then(() => { })
191+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
192+
42 │ }
193+
43 │
194+
195+
i This happens when a Promise is not awaited, lacks a `.catch` or `.then` rejection handler, or is not explicitly ignored using the `void` operator.
196+
197+
i Unsafe fix: Add await operator.
198+
199+
41 │ ··await·returnsPromiseAssignedFunction().then(()·=>·{·})
200+
│ ++++++
201+
202+
```
203+
204+
```
205+
invalid.ts:48:1 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
206+
207+
! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior.
208+
209+
46 │ };
210+
47 │
211+
> 48 │ returnsPromiseAssignedArrowFunctionAnnotatedType();
212+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
139213
140214
i This happens when a Promise is not awaited, lacks a `.catch` or `.then` rejection handler, or is not explicitly ignored using the `void` operator.
141215

0 commit comments

Comments
 (0)