From 63d315a4bf2ea242072a880aa00fcfb4ee5aa6a6 Mon Sep 17 00:00:00 2001 From: kaykdm Date: Wed, 12 Feb 2025 20:14:45 +0900 Subject: [PATCH] feat(lint): support object method in noFloatingPromises --- .../src/lint/nursery/no_floating_promises.rs | 133 +++++++++++- .../nursery/noFloatingPromises/invalid.ts | 32 +++ .../noFloatingPromises/invalid.ts.snap | 193 +++++++++++++++++- .../specs/nursery/noFloatingPromises/valid.ts | 26 +++ .../nursery/noFloatingPromises/valid.ts.snap | 26 +++ 5 files changed, 399 insertions(+), 11 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs b/crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs index 6eee5bde46fd..11d1cb6fd7b2 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs @@ -6,14 +6,15 @@ use biome_js_factory::make; use biome_js_semantic::SemanticModel; use biome_js_syntax::{ binding_ext::AnyJsBindingDeclaration, global_identifier, AnyJsClassMember, AnyJsExpression, - AnyTsType, JsArrowFunctionExpression, JsCallExpression, JsClassDeclaration, JsClassExpression, - JsClassMemberList, JsExpressionStatement, JsExtendsClause, JsFunctionDeclaration, - JsIdentifierExpression, JsInitializerClause, JsMethodClassMember, JsMethodObjectMember, - JsStaticMemberExpression, JsSyntaxKind, JsThisExpression, JsVariableDeclarator, - TsReturnTypeAnnotation, + AnyJsObjectMember, AnyTsType, JsArrowFunctionExpression, JsCallExpression, JsClassDeclaration, + JsClassExpression, JsClassMemberList, JsExpressionStatement, JsExtendsClause, + JsFunctionDeclaration, JsIdentifierExpression, JsInitializerClause, JsMethodClassMember, + JsMethodObjectMember, JsObjectMemberList, JsStaticMemberExpression, JsSyntaxKind, + JsSyntaxToken, JsThisExpression, JsVariableDeclarator, TsReturnTypeAnnotation, }; use biome_rowan::{ - AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, SyntaxNodeCast, TriviaPieceKind, + AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, SyntaxNodeCast, TokenText, + TriviaPieceKind, }; use crate::{services::semantic::Semantic, JsRuleAction}; @@ -101,6 +102,16 @@ declare_lint_rule! { /// const api = new Api(); /// api.returnsPromise().then(() => {}).finally(() => {}); /// ``` + /// + /// ```typescript + /// const obj = { + /// async returnsPromise(): Promise { + /// return 'value'; + /// }, + /// }; + /// + /// obj.returnsPromise(); + /// ``` /// ### Valid /// /// ```ts @@ -253,6 +264,16 @@ impl Rule for NoFloatingPromises { /// returnsPromise().then(() => {}); /// ``` /// +/// ```typescript +/// const obj = { +/// async returnsPromise(): Promise { +/// return 'value'; +/// }, +/// }; +/// +/// obj['returnsPromise'](); +/// ``` +/// /// Example JavaScript code that would return `false`: /// ```typescript /// function doesNotReturnPromise() { @@ -269,6 +290,24 @@ fn is_callee_a_promise(callee: &AnyJsExpression, model: &SemanticModel) -> Optio AnyJsExpression::JsStaticMemberExpression(static_member_expr) => { is_member_expression_callee_a_promise(static_member_expr, model) } + AnyJsExpression::JsComputedMemberExpression(computed_member_expr) => { + let object = computed_member_expr.object().ok()?; + let member = computed_member_expr.member().ok()?; + let literal_expr = member.as_any_js_literal_expression()?; + let string_literal = literal_expr.as_js_string_literal_expression()?; + let value_token = string_literal.inner_string_text().ok()?; + let value_text = value_token.text(); + + match object { + AnyJsExpression::JsIdentifierExpression(js_ident_expr) => { + is_binding_a_promise(&js_ident_expr, model, Some(value_text)) + } + AnyJsExpression::JsThisExpression(js_this_expr) => { + check_this_expression(&js_this_expr, value_text, model) + } + _ => None, + } + } _ => Some(false), } } @@ -591,6 +630,9 @@ fn is_initializer_a_promise( AnyJsExpression::JsClassExpression(class_expr) => { find_and_check_class_member(&class_expr.members(), target_method_name?, model) } + AnyJsExpression::JsObjectExpression(object_expr) => { + find_and_check_object_member(&object_expr.members(), target_method_name?) + } _ => Some(false), } } @@ -729,12 +771,16 @@ fn check_this_expression( .syntax() .ancestors() .skip(1) - .find_map(|ancestor| { - if ancestor.kind() == JsSyntaxKind::JS_CLASS_MEMBER_LIST { + .find_map(|ancestor| match ancestor.kind() { + JsSyntaxKind::JS_CLASS_MEMBER_LIST => { let class_member_list = JsClassMemberList::cast(ancestor)?; - return find_and_check_class_member(&class_member_list, target_name, model); + find_and_check_class_member(&class_member_list, target_name, model) } - None + JsSyntaxKind::JS_OBJECT_MEMBER_LIST => { + let object_member_list = JsObjectMemberList::cast(ancestor)?; + find_and_check_object_member(&object_member_list, target_name) + } + _ => None, }) } @@ -870,3 +916,70 @@ fn is_ts_type_a_promise(any_ts_type: &AnyTsType) -> Option { _ => None, } } + +/// Finds a object method or property by matching the given name and checks if it is a promise. +/// +/// This function searches for a object method or property in the given `JsObjectMemberList` +/// by matching the provided `target_name`. If a matching member is found, it checks if the member +/// is a promise. +/// +/// # Arguments +/// +/// * `object_member_list` - A reference to a `JsObjectMemberList` representing the object members to search in. +/// * `target_name` - The name of the method or property to search for. +/// * `model` - A reference to the `SemanticModel` used for resolving bindings. +/// +/// # Returns +/// +/// * `Some(true)` if the class member is a promise. +/// * `Some(false)` if the class member is not a promise. +/// * `None` if there is an error in the process or if the class member is not found. +/// +fn find_and_check_object_member( + object_member_list: &JsObjectMemberList, + target_name: &str, +) -> Option { + fn extract_member_name(member: &AnyJsObjectMember) -> Option { + match member { + AnyJsObjectMember::JsPropertyObjectMember(property) => property.name().ok()?.name(), + AnyJsObjectMember::JsMethodObjectMember(method) => method.name().ok()?.name(), + _ => None, + } + } + + fn is_async_or_promise( + async_token: &Option, + return_type: Option, + ) -> bool { + async_token.is_some() || is_return_type_a_promise(return_type).unwrap_or_default() + } + + let object_member = object_member_list.iter().find_map(|member| { + let member = member.ok()?; + (extract_member_name(&member)? == target_name).then_some(member) + })?; + + match object_member { + AnyJsObjectMember::JsMethodObjectMember(method) => Some(is_async_or_promise( + &method.async_token(), + method.return_type_annotation(), + )), + AnyJsObjectMember::JsPropertyObjectMember(property) => { + let value = property.value().ok()?; + match value { + AnyJsExpression::JsArrowFunctionExpression(arrow_func) => { + Some(is_async_or_promise( + &arrow_func.async_token(), + arrow_func.return_type_annotation(), + )) + } + AnyJsExpression::JsFunctionExpression(func_expr) => Some(is_async_or_promise( + &func_expr.async_token(), + func_expr.return_type_annotation(), + )), + _ => None, + } + } + _ => None, + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts index fa9bcfccd49e..3df272004a25 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts +++ b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts @@ -265,3 +265,35 @@ invalidTestUnnamedClassInitializedExpression.returnsPromiseProperty; invalidTestUnnamedClassInitializedExpression.returnsPromiseProperty .then(() => {}) .finally(() => {}); +invalidTestClassExpression.returnsPromiseProperty + .then(() => {}) + .finally(() => {}); + +const invalidTestObject = { + returnsPromiseArrowFunction: async (): Promise => { + return "value"; + }, + + returnsPromiseFunction: async function (): Promise { + return "value"; + }, + + async returnsPromiseMethod(): Promise { + return "value"; + }, + + someMethod() { + this.returnsPromiseArrowFunction(); + this.returnsPromiseFunction().then(() => {}); + this["returnsPromiseMethod"](); + }, +}; +async function testInvalidObejctMethodCalls(): Promise { + invalidTestObject.returnsPromiseArrowFunction(); + invalidTestObject.returnsPromiseFunction().then(() => {}); + invalidTestObject + .returnsPromiseMethod() + .then(() => {}) + .finally(() => {}); + invalidTestObject["returnsPromiseMethod"](); +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts.snap index d9c3549dd021..4fa139419ecd 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/invalid.ts.snap @@ -271,6 +271,38 @@ invalidTestUnnamedClassInitializedExpression.returnsPromiseProperty; invalidTestUnnamedClassInitializedExpression.returnsPromiseProperty .then(() => {}) .finally(() => {}); +invalidTestClassExpression.returnsPromiseProperty + .then(() => {}) + .finally(() => {}); + +const invalidTestObject = { + returnsPromiseArrowFunction: async (): Promise => { + return "value"; + }, + + returnsPromiseFunction: async function (): Promise { + return "value"; + }, + + async returnsPromiseMethod(): Promise { + return "value"; + }, + + someMethod() { + this.returnsPromiseArrowFunction(); + this.returnsPromiseFunction().then(() => {}); + this["returnsPromiseMethod"](); + }, +}; +async function testInvalidObejctMethodCalls(): Promise { + invalidTestObject.returnsPromiseArrowFunction(); + invalidTestObject.returnsPromiseFunction().then(() => {}); + invalidTestObject + .returnsPromiseMethod() + .then(() => {}) + .finally(() => {}); + invalidTestObject["returnsPromiseMethod"](); +} ``` @@ -1232,9 +1264,168 @@ invalid.ts:265:1 lint/nursery/noFloatingPromises ━━━━━━━━━━ > 266 │ .then(() => {}) > 267 │ .finally(() => {}); │ ^^^^^^^^^^^^^^^^^^^ - 268 │ + 268 │ invalidTestClassExpression.returnsPromiseProperty + 269 │ .then(() => {}) 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. ``` + +``` +invalid.ts:268:1 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 266 │ .then(() => {}) + 267 │ .finally(() => {}); + > 268 │ invalidTestClassExpression.returnsPromiseProperty + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 269 │ .then(() => {}) + > 270 │ .finally(() => {}); + │ ^^^^^^^^^^^^^^^^^^^ + 271 │ + 272 │ const invalidTestObject = { + + 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. + + +``` + +``` +invalid.ts:286:3 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 285 │ someMethod() { + > 286 │ this.returnsPromiseArrowFunction(); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 287 │ this.returnsPromiseFunction().then(() => {}); + 288 │ this["returnsPromiseMethod"](); + + 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. + + +``` + +``` +invalid.ts:287:3 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 285 │ someMethod() { + 286 │ this.returnsPromiseArrowFunction(); + > 287 │ this.returnsPromiseFunction().then(() => {}); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 288 │ this["returnsPromiseMethod"](); + 289 │ }, + + 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. + + +``` + +``` +invalid.ts:288:3 lint/nursery/noFloatingPromises ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 286 │ this.returnsPromiseArrowFunction(); + 287 │ this.returnsPromiseFunction().then(() => {}); + > 288 │ this["returnsPromiseMethod"](); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 289 │ }, + 290 │ }; + + 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. + + +``` + +``` +invalid.ts:292:2 lint/nursery/noFloatingPromises FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 290 │ }; + 291 │ async function testInvalidObejctMethodCalls(): Promise { + > 292 │ invalidTestObject.returnsPromiseArrowFunction(); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 293 │ invalidTestObject.returnsPromiseFunction().then(() => {}); + 294 │ invalidTestObject + + 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. + + i Unsafe fix: Add await operator. + + 292 │ → await·invalidTestObject.returnsPromiseArrowFunction(); + │ ++++++ + +``` + +``` +invalid.ts:293:2 lint/nursery/noFloatingPromises FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 291 │ async function testInvalidObejctMethodCalls(): Promise { + 292 │ invalidTestObject.returnsPromiseArrowFunction(); + > 293 │ invalidTestObject.returnsPromiseFunction().then(() => {}); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 294 │ invalidTestObject + 295 │ .returnsPromiseMethod() + + 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. + + i Unsafe fix: Add await operator. + + 293 │ → await·invalidTestObject.returnsPromiseFunction().then(()·=>·{}); + │ ++++++ + +``` + +``` +invalid.ts:294:2 lint/nursery/noFloatingPromises FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 292 │ invalidTestObject.returnsPromiseArrowFunction(); + 293 │ invalidTestObject.returnsPromiseFunction().then(() => {}); + > 294 │ invalidTestObject + │ ^^^^^^^^^^^^^^^^^ + > 295 │ .returnsPromiseMethod() + > 296 │ .then(() => {}) + > 297 │ .finally(() => {}); + │ ^^^^^^^^^^^^^^^^^^^ + 298 │ invalidTestObject["returnsPromiseMethod"](); + 299 │ } + + 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. + + i Unsafe fix: Add await operator. + + 294 │ → await·invalidTestObject + │ ++++++ + +``` + +``` +invalid.ts:298:2 lint/nursery/noFloatingPromises FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A "floating" Promise was found, meaning it is not properly handled and could lead to ignored errors or unexpected behavior. + + 296 │ .then(() => {}) + 297 │ .finally(() => {}); + > 298 │ invalidTestObject["returnsPromiseMethod"](); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 299 │ } + 300 │ + + 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. + + i Unsafe fix: Add await operator. + + 298 │ → await·invalidTestObject["returnsPromiseMethod"](); + │ ++++++ + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts index 30ff55a40ec6..045876644470 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts +++ b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts @@ -189,4 +189,30 @@ async function testUnnamedClassExpressionMethodCalls(): Promise { unnamedValidClassExpression.returnsPromiseFunctionProperty().then(() => { }, () => { }).finally(() => { }); unnamedValidClassExpression.returnsPromiseProperty.catch(() => { }); return unnamedValidClassExpression.returnsPromiseArrowFunction(); +} + +const validTestObject = { + returnsPromiseArrowFunction: async (): Promise => { + return 'value'; + }, + + returnsPromiseFunction: async function (): Promise { + return 'value'; + }, + + async returnsPromiseMethod(): Promise { + return 'value'; + }, + + async someMethod() { + await this.returnsPromiseArrowFunction(); + this.returnsPromiseFunction().then(() => {}, () => {}).finally(() => {}); + this["returnsPromiseMethod"]().catch(() => {}); + }, +} +async function testValidObejctMethodCalls(): Promise { + await validTestObject.returnsPromiseArrowFunction(); + validTestObject.returnsPromiseFunction().catch(() => { }); + validTestObject.returnsPromiseMethod().then(() => { }, () => { }).finally(() => { }); + return validTestObject['returnsPromiseMethod'](); } \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts.snap index 7c229159aa49..917af35f91ae 100644 --- a/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/nursery/noFloatingPromises/valid.ts.snap @@ -196,4 +196,30 @@ async function testUnnamedClassExpressionMethodCalls(): Promise { unnamedValidClassExpression.returnsPromiseProperty.catch(() => { }); return unnamedValidClassExpression.returnsPromiseArrowFunction(); } + +const validTestObject = { + returnsPromiseArrowFunction: async (): Promise => { + return 'value'; + }, + + returnsPromiseFunction: async function (): Promise { + return 'value'; + }, + + async returnsPromiseMethod(): Promise { + return 'value'; + }, + + async someMethod() { + await this.returnsPromiseArrowFunction(); + this.returnsPromiseFunction().then(() => {}, () => {}).finally(() => {}); + this["returnsPromiseMethod"]().catch(() => {}); + }, +} +async function testValidObejctMethodCalls(): Promise { + await validTestObject.returnsPromiseArrowFunction(); + validTestObject.returnsPromiseFunction().catch(() => { }); + validTestObject.returnsPromiseMethod().then(() => { }, () => { }).finally(() => { }); + return validTestObject['returnsPromiseMethod'](); +} ```