Add support for UnusedMethodCall (#18)

* Add support for UnusedMethodCall

* pr feedback

src/analyzer/expr/assertion_finder.rs

@@ -1,5 +1,5 @@
 use super::expression_identifier::{get_dim_id, get_var_id};
-use crate::expr::expression_identifier::get_functionlike_id_from_call;
+use crate::expr::expression_identifier::get_static_functionlike_id_from_call;
 use crate::{formula_generator::AssertionContext, function_analysis_data::FunctionAnalysisData};
 use hakana_reflection_info::code_location::HPos;
 use hakana_reflection_info::function_context::FunctionLikeIdentifier;
@@ -58,7 +58,7 @@ pub(crate) fn scrape_assertions(
             );
         }
         aast::Expr_::Call(call) => {
-            let functionlike_id = get_functionlike_id_from_call(
+            let functionlike_id = get_static_functionlike_id_from_call(
                 call,
                 if let Some((_, interner)) = assertion_context.codebase {
                     Some(interner)
@@ -348,7 +348,7 @@ fn scrape_shapes_isset(
     negated: bool,
 ) {
     if let aast::Expr_::Call(call) = &var_expr.2 {
-        let functionlike_id = get_functionlike_id_from_call(
+        let functionlike_id = get_static_functionlike_id_from_call(
             call,
             if let Some((_, interner)) = assertion_context.codebase {
                 Some(interner)

src/analyzer/expr/expression_identifier.rs

@@ -1,11 +1,13 @@
+use std::rc::Rc;
+
 use hakana_reflection_info::{
     ast::get_id_name, codebase_info::CodebaseInfo, functionlike_identifier::FunctionLikeIdentifier,
-    ExprId, VarId,
+    t_atomic::TAtomic, t_union::TUnion, ExprId, VarId,
 };
 use hakana_str::{Interner, StrId};
 use rustc_hash::{FxHashMap, FxHashSet};
 
-use oxidized::{aast, ast_defs};
+use oxidized::{aast, ast::PropOrMethod, ast_defs};
 
 use crate::{scope_analyzer::ScopeAnalyzer, statements_analyzer::StatementsAnalyzer};
 
@@ -160,6 +162,16 @@ pub(crate) fn get_dim_id(
 }
 
 pub fn get_functionlike_id_from_call(
+    call_expr: &oxidized::ast::CallExpr,
+    interner: &Interner,
+    resolved_names: &FxHashMap<u32, StrId>,
+    expr_types: &FxHashMap<(u32, u32), Rc<TUnion>>,
+) -> Option<FunctionLikeIdentifier> {
+    get_static_functionlike_id_from_call(call_expr, Some(interner), resolved_names)
+        .or_else(|| get_method_id_from_call(call_expr, interner, expr_types))
+}
+
+pub fn get_static_functionlike_id_from_call(
     call: &oxidized::ast::CallExpr,
     interner: Option<&Interner>,
     resolved_names: &FxHashMap<u32, StrId>,
@@ -213,6 +225,49 @@ pub fn get_functionlike_id_from_call(
     }
 }
 
+pub fn get_method_id_from_call(
+    call_expr: &oxidized::ast::CallExpr,
+    interner: &Interner,
+    expr_types: &FxHashMap<(u32, u32), Rc<TUnion>>,
+) -> Option<FunctionLikeIdentifier> {
+    // Instance method call
+    match &call_expr.func.2 {
+        aast::Expr_::ObjGet(boxed) => {
+            let (lhs_expr, rhs_expr, _nullfetch, prop_or_method) =
+                (&boxed.0, &boxed.1, &boxed.2, &boxed.3);
+
+            if *prop_or_method == PropOrMethod::IsProp {
+                return None;
+            }
+
+            let class_id = if let Some(lhs_expr_type) = expr_types.get(&(
+                lhs_expr.1.start_offset() as u32,
+                lhs_expr.1.end_offset() as u32,
+            )) {
+                let t = lhs_expr_type.types.first().unwrap();
+                if let TAtomic::TNamedObject { name, .. } = t {
+                    name
+                } else {
+                    return None;
+                }
+            } else {
+                return None;
+            };
+
+            if let aast::Expr_::Id(method_name_node) = &rhs_expr.2 {
+                if let Some(method_id) = interner.get(&method_name_node.1) {
+                    return Some(FunctionLikeIdentifier::Method(*class_id, method_id));
+                } else {
+                    return None;
+                }
+            } else {
+                return None;
+            }
+        }
+        _ => return None,
+    }
+}
+
 pub fn get_expr_id(
     conditional: &aast::Expr<(), ()>,
     statements_analyzer: &StatementsAnalyzer,

src/analyzer/stmt_analyzer.rs

@@ -273,12 +273,11 @@ fn detect_unused_statement_expressions(
     stmt: &aast::Stmt<(), ()>,
     context: &mut ScopeContext,
 ) {
-    if has_unused_must_use(boxed, statements_analyzer) {
+    if let Some(issue_kind) = has_unused_must_use(boxed, statements_analyzer, analysis_data) {
         analysis_data.maybe_add_issue(
             Issue::new(
-                IssueKind::UnusedFunctionCall,
-                "This function is annotated with MustUse but the returned value is not used"
-                    .to_string(),
+                issue_kind,
+                "This 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,
             ),
@@ -290,8 +289,9 @@ fn detect_unused_statement_expressions(
     let functionlike_id = if let aast::Expr_::Call(boxed_call) = &boxed.2 {
         get_functionlike_id_from_call(
             boxed_call,
-            Some(statements_analyzer.get_interner()),
+            statements_analyzer.get_interner(),
             statements_analyzer.get_file_analyzer().resolved_names,
+            &analysis_data.expr_types,
         )
     } else {
         None
@@ -406,40 +406,66 @@ fn detect_unused_statement_expressions(
 fn has_unused_must_use(
     boxed: &aast::Expr<(), ()>,
     statements_analyzer: &StatementsAnalyzer,
-) -> bool {
+    analysis_data: &mut FunctionAnalysisData,
+) -> Option<IssueKind> {
     match &boxed.2 {
         aast::Expr_::Call(boxed_call) => {
-            let functionlike_id = get_functionlike_id_from_call(
+            let functionlike_id_from_call = get_functionlike_id_from_call(
                 boxed_call,
-                Some(statements_analyzer.get_interner()),
+                statements_analyzer.get_interner(),
                 statements_analyzer.get_file_analyzer().resolved_names,
+                &analysis_data.expr_types,
             );
-            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));
-                }
-
+            if let Some(functionlike_id) = functionlike_id_from_call {
                 let codebase = statements_analyzer.get_codebase();
-                if let Some(functionlike_info) = codebase
-                    .functionlike_infos
-                    .get(&(function_id, StrId::EMPTY))
-                {
-                    return functionlike_info.must_use;
+                match functionlike_id {
+                    FunctionLikeIdentifier::Function(function_id) => {
+                        // For statements like "Asio\join(some_fn());"
+                        // Asio\join does not count as "using" the value
+                        if function_id == StrId::ASIO_JOIN {
+                            for arg in boxed_call.args.iter() {
+                                let has_unused =
+                                    has_unused_must_use(&arg.1, statements_analyzer, analysis_data);
+                                if has_unused.is_some() {
+                                    return has_unused;
+                                }
+                            }
+                        }
+
+                        if let Some(functionlike_info) = codebase
+                            .functionlike_infos
+                            .get(&(function_id, StrId::EMPTY))
+                        {
+                            return if functionlike_info.must_use {
+                                Some(IssueKind::UnusedFunctionCall)
+                            } else {
+                                None
+                            };
+                        }
+                    }
+                    FunctionLikeIdentifier::Method(method_class, method_name) => {
+                        if let Some(functionlike_info) = codebase
+                            .functionlike_infos
+                            .get(&(method_class, method_name))
+                        {
+                            return if functionlike_info.must_use {
+                                Some(IssueKind::UnusedMethodCall)
+                            } else {
+                                None
+                            };
+                        }
+                    }
+                    FunctionLikeIdentifier::Closure(_, _) => (),
                 }
             }
         }
         aast::Expr_::Await(await_expr) => {
-            return has_unused_must_use(await_expr, statements_analyzer)
+            return has_unused_must_use(await_expr, statements_analyzer, analysis_data)
         }
         _ => (),
     }
 
-    false
+    None
 }
 
 fn analyze_awaitall(

src/code_info/issue.rs

@@ -128,6 +128,7 @@ pub enum IssueKind {
     UnusedClosureParameter,
     UnusedFunction,
     UnusedFunctionCall,
+    UnusedMethodCall,
     UnusedInheritedMethod,
     UnusedInterface,
     UnusedParameter,

tests/unused/UnusedExpression/unusedFunctionCallAsync/output.txt

@@ -1,2 +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
+ERROR: UnusedFunctionCall - input.hack:7:5 - This is annotated with MustUse but the returned value is not used
+ERROR: UnusedFunctionCall - input.hack:11:5 - This is annotated with MustUse but the returned value is not used

tests/unused/UnusedExpression/unusedMethodCall/input.hack

@@ -0,0 +1,16 @@
+class UnusedMethodClass {
+    <<Hakana\MustUse>>
+	public function getEncodedId(): string {
+		return '';
+	}
+
+	public function doWork(): string {
+		return '';
+	}
+}
+
+function foo(): void {
+    $c = new UnusedMethodClass();
+    $c->getEncodedId();
+    $c->doWork();
+}

tests/unused/UnusedExpression/unusedMethodCall/output.txt

@@ -0,0 +1 @@
+UnusedMethodCall

tests/unused/UnusedExpression/unusedMethodCallAsync/input.hack

@@ -0,0 +1,22 @@
+class UnusedMethodClass {
+    <<Hakana\MustUse>>
+	public async function getEncodedId(): Awaitable<string> {
+		return '';
+	}
+
+	public async function doWork(): Awaitable<string> {
+		return '';
+	}
+}
+
+async function foo(): Awaitable<void> {
+    $c = new UnusedMethodClass();
+    await $c->getEncodedId();
+    await $c->doWork();
+}
+
+function foo2(): void {
+    $c = new UnusedMethodClass();
+    Asio\join($c->getEncodedId());
+    Asio\join($c->doWork());
+}

tests/unused/UnusedExpression/unusedMethodCallAsync/output.txt

@@ -0,0 +1,2 @@
+ERROR: UnusedMethodCall - input.hack:14:5 - This is annotated with MustUse but the returned value is not used
+ERROR: UnusedMethodCall - input.hack:20:5 - This is annotated with MustUse but the returned value is not used