From 27427172e1edad143099a39db4a3c7ebc58cdddc Mon Sep 17 00:00:00 2001
From: Georgiy Komarov <jubnzv@gmail.com>
Date: Tue, 24 Dec 2024 02:29:27 +0000
Subject: [PATCH] chore(sendInLoop): Simplify; error logging for `callgraph`

---
 src/detectors/builtin/sendInLoop.ts    | 57 +++++++++++++++-----------
 src/internals/ir/callGraph.ts          |  2 +-
 test/detectors/SendInLoop.expected.out |  2 +-
 3 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/src/detectors/builtin/sendInLoop.ts b/src/detectors/builtin/sendInLoop.ts
index 4d2fca5e..9a1bbed1 100644
--- a/src/detectors/builtin/sendInLoop.ts
+++ b/src/detectors/builtin/sendInLoop.ts
@@ -10,6 +10,7 @@ import {
   AstStaticCall,
   AstMethodCall,
   AstContract,
+  SrcInfo,
 } from "@tact-lang/compiler/dist/grammar/ast";
 
 /**
@@ -94,12 +95,7 @@ export class SendInLoop extends AstDetector {
       stmt,
       (acc: MistiTactWarning[], expr: AstExpression) => {
         if (isSendCall(expr)) {
-          acc.push(
-            this.makeWarning("Send function called inside a loop", expr.loc, {
-              suggestion:
-                "Consider refactoring to avoid calling send functions inside loops",
-            }),
-          );
+          acc.push(this.warn("Send function called inside a loop", expr.loc));
         }
         return acc;
       },
@@ -115,26 +111,30 @@ export class SendInLoop extends AstDetector {
             expr as AstStaticCall | AstMethodCall,
             currentContractName,
           );
-          if (calleeName) {
-            const calleeNodeId = callGraph.getNodeIdByName(calleeName);
-            if (calleeNodeId !== undefined) {
-              const calleeNode = callGraph.getNode(calleeNodeId);
-              if (calleeNode && calleeNode.hasEffect(Effect.Send)) {
-                const functionName = calleeNode.name.includes("::")
-                  ? calleeNode.name.split("::").pop()
-                  : calleeNode.name;
-                acc.push(
-                  this.makeWarning(
-                    `Method "${functionName}" called inside a loop leads to a send function`,
-                    expr.loc,
-                    {
-                      suggestion:
-                        "Consider refactoring to avoid calling send functions inside loops",
-                    },
-                  ),
-                );
-              }
+          if (calleeName === undefined) {
+            this.ctx.logger.error(
+              `Cannot retrieve function name for AST node: #${expr.id}`,
+            );
+            return acc;
+          }
+          const calleeNodeId = callGraph.getNodeIdByName(calleeName);
+          if (calleeNodeId !== undefined) {
+            const calleeNode = callGraph.getNode(calleeNodeId);
+            if (calleeNode && calleeNode.hasEffect(Effect.Send)) {
+              const isMethod = calleeNode.name.includes("::");
+              const functionName = isMethod
+                ? calleeNode.name.split("::").pop()
+                : calleeNode.name;
+              acc.push(
+                this.warn(
+                  `${isMethod ? "Method" : "Function"} "${functionName}" called inside a loop leads to calling a send function`,
+                  expr.loc,
+                ),
+              );
             }
+          } else {
+            this.ctx.logger.error(`Cannot retrieve CG node: ${calleeName}`);
+            return acc;
           }
         }
         return acc;
@@ -144,6 +144,13 @@ export class SendInLoop extends AstDetector {
     return warnings;
   }
 
+  private warn(msg: string, loc: SrcInfo): MistiTactWarning {
+    return this.makeWarning(msg, loc, {
+      suggestion:
+        "Consider refactoring to avoid calling send functions inside loops",
+    });
+  }
+
   private isLoop(stmt: AstStatement): boolean {
     return (
       stmt.kind === "statement_while" ||
diff --git a/src/internals/ir/callGraph.ts b/src/internals/ir/callGraph.ts
index 87b26554..b83653cf 100644
--- a/src/internals/ir/callGraph.ts
+++ b/src/internals/ir/callGraph.ts
@@ -433,7 +433,7 @@ export class CallGraph {
     currentContractName?: string,
   ): string | undefined {
     if (expr.kind === "static_call") {
-      return expr.function?.text;
+      return expr.function.text;
     } else if (expr.kind === "method_call") {
       const methodName = expr.method?.text;
       if (methodName) {
diff --git a/test/detectors/SendInLoop.expected.out b/test/detectors/SendInLoop.expected.out
index 99b31303..15a79277 100644
--- a/test/detectors/SendInLoop.expected.out
+++ b/test/detectors/SendInLoop.expected.out
@@ -43,7 +43,7 @@ test/detectors/SendInLoop.tact:47:17:
 Help: Consider refactoring to avoid calling send functions inside loops
 See: https://nowarp.io/tools/misti/docs/detectors/SendInLoop
 
-[MEDIUM] SendInLoop: Method "deepNestSend" called inside a loop leads to a send function
+[MEDIUM] SendInLoop: Method "deepNestSend" called inside a loop leads to calling a send function
 test/detectors/SendInLoop.tact:140:17:
   139 |             if (value > 0) {
 > 140 |                 self.deepNestSend(value);