chore(sendInLoop): Simplify; error logging for callgraph

nowarp · Dec 24, 2024 · 2742717 · 2742717
1 parent 5cc3730
commit 2742717
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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);