From 89af83a70b56f578fa8e39b8a48c7a2bb79f0287 Mon Sep 17 00:00:00 2001 From: Ryan Greenberg Date: Mon, 8 Apr 2024 12:39:07 -0400 Subject: [PATCH] Identify MustUse values passed to await, Asio\join (#16) * Update README * Add test for unused with Asio\join, await * Detect more unused return values * Mark unused --- README.md | 4 +- src/analyzer/stmt_analyzer.rs | 71 +++++++++++++++---- .../unusedFunctionCallAsync/input.hack | 12 ++++ .../unusedFunctionCallAsync/output.txt | 2 + 4 files changed, 72 insertions(+), 17 deletions(-) create mode 100644 tests/unused/UnusedExpression/unusedFunctionCallAsync/input.hack create mode 100644 tests/unused/UnusedExpression/unusedFunctionCallAsync/output.txt diff --git a/README.md b/README.md index dd8ae072..897d0aa6 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,6 @@ That will create a binary at `./target/release/hakana-default` ## Running tests -You can run all tests with: `cargo run --release test tests` +You can run all tests with: `cargo run --bin hakana --release test tests` -You can run an individual test with `cargo run test ` +You can run an individual test with `cargo run --bin hakana test ` diff --git a/src/analyzer/stmt_analyzer.rs b/src/analyzer/stmt_analyzer.rs index 9ac7261e..a3647ee4 100644 --- a/src/analyzer/stmt_analyzer.rs +++ b/src/analyzer/stmt_analyzer.rs @@ -273,6 +273,20 @@ fn detect_unused_statement_expressions( stmt: &aast::Stmt<(), ()>, context: &mut ScopeContext, ) { + if has_unused_must_use(&boxed, &statements_analyzer, &stmt, context) { + analysis_data.maybe_add_issue( + Issue::new( + IssueKind::UnusedFunctionCall, + "This function is annotated with MustUse but the returned value is not used" + .to_string(), + statements_analyzer.get_hpos(&stmt.0), + &context.function_context.calling_functionlike_id, + ), + statements_analyzer.get_config(), + statements_analyzer.get_file_path_actual(), + ); + } + let functionlike_id = if let aast::Expr_::Call(boxed_call) = &boxed.2 { get_functionlike_id_from_call( boxed_call, @@ -331,27 +345,14 @@ fn detect_unused_statement_expressions( } } } - if let Some(functionlike_id) = functionlike_id { if let FunctionLikeIdentifier::Function(function_id) = functionlike_id { let codebase = statements_analyzer.get_codebase(); - if let Some(functionlike_info) = codebase + if let Some(_functionlike_info) = codebase .functionlike_infos .get(&(function_id, StrId::EMPTY)) { - if functionlike_info.must_use { - analysis_data.maybe_add_issue( - Issue::new( - IssueKind::UnusedFunctionCall, - "This function is annotated with MustUse but the returned value is not used".to_string(), - statements_analyzer.get_hpos(&stmt.0), - &context.function_context.calling_functionlike_id, - ), - statements_analyzer.get_config(), - statements_analyzer.get_file_path_actual(), - ); - } else if let Some(expr_type) = analysis_data.get_rc_expr_type(boxed.pos()).cloned() - { + if let Some(expr_type) = analysis_data.get_rc_expr_type(boxed.pos()).cloned() { let function_name = statements_analyzer.get_interner().lookup(&function_id); if (function_name.starts_with("HH\\Lib\\Keyset\\") @@ -402,6 +403,46 @@ fn detect_unused_statement_expressions( } } +fn has_unused_must_use( + boxed: &aast::Expr<(), ()>, + statements_analyzer: &StatementsAnalyzer, + stmt: &aast::Stmt<(), ()>, + context: &mut ScopeContext, +) -> bool { + match &boxed.2 { + aast::Expr_::Call(boxed_call) => { + let functionlike_id = get_functionlike_id_from_call( + boxed_call, + Some(statements_analyzer.get_interner()), + statements_analyzer.get_file_analyzer().resolved_names, + ); + if let Some(FunctionLikeIdentifier::Function(function_id)) = functionlike_id { + // For statements like "Asio\join(some_fn());" + // Asio\join does not count as "using" the value + if function_id == StrId::ASIO_JOIN { + return boxed_call.args.iter().any(|arg| { + has_unused_must_use(&arg.1, statements_analyzer, stmt, context) + }); + } + + let codebase = statements_analyzer.get_codebase(); + if let Some(functionlike_info) = codebase + .functionlike_infos + .get(&(function_id, StrId::EMPTY)) + { + return functionlike_info.must_use; + } + } + } + aast::Expr_::Await(await_expr) => { + return has_unused_must_use(await_expr, statements_analyzer, stmt, context) + } + _ => (), + } + + return false; +} + fn analyze_awaitall( boxed: ( &Vec<(oxidized::tast::Lid, aast::Expr<(), ()>)>, diff --git a/tests/unused/UnusedExpression/unusedFunctionCallAsync/input.hack b/tests/unused/UnusedExpression/unusedFunctionCallAsync/input.hack new file mode 100644 index 00000000..9f70833f --- /dev/null +++ b/tests/unused/UnusedExpression/unusedFunctionCallAsync/input.hack @@ -0,0 +1,12 @@ +<> +async function must_use_async(): Awaitable { + return 0; +} + +function foo(): void { + Asio\join(must_use_async()); +} + +function foo_async(): void { + await must_use_async(); +} diff --git a/tests/unused/UnusedExpression/unusedFunctionCallAsync/output.txt b/tests/unused/UnusedExpression/unusedFunctionCallAsync/output.txt new file mode 100644 index 00000000..eeef0f9e --- /dev/null +++ b/tests/unused/UnusedExpression/unusedFunctionCallAsync/output.txt @@ -0,0 +1,2 @@ +ERROR: UnusedFunctionCall - input.hack:7:5 - This function is annotated with MustUse but the returned value is not used +ERROR: UnusedFunctionCall - input.hack:11:5 - This function is annotated with MustUse but the returned value is not used