`shortCircuitCondition' detector

src/detectors/builtin/shortCircuitCondition.ts

@@ -0,0 +1,115 @@
+import { CompilationUnit } from "../../internals/ir";
+import {
+  evalsToValue,
+  foldStatements,
+  forEachExpression,
+} from "../../internals/tact";
+import { MistiTactWarning, Severity } from "../../internals/warnings";
+import { ASTDetector } from "../detector";
+import {
+  AstExpression,
+  AstMethodCall,
+} from "@tact-lang/compiler/dist/grammar/ast";
+
+/**
+ * A detector that suggests optimizing boolean expressions to leverage short-circuit evaluation.
+ *
+ * ## Why is it bad?
+ * TVM supports short-circuit operations. When using logical AND (&&) operations,
+ * placing constant or cheaper conditions first can prevent unnecessary execution
+ * of expensive operations when the result is already determined.
+ *
+ * ## Example
+ * ```tact
+ * // Bad: Expensive operation is always executed
+ * if (expensive_function() && constant_false) {
+ *   // ...
+ * }
+ * ```
+ *
+ * Use instead:
+ * ```tact
+ * // Good: Expensive operation is skipped when constant_false is false
+ * if (constant_false && expensive_function()) {
+ *   // ...
+ * }
+ * ```
+ */
+export class ShortCircuitCondition extends ASTDetector {
+  severity = Severity.LOW;
+
+  async check(cu: CompilationUnit): Promise<MistiTactWarning[]> {
+    return Array.from(cu.ast.getFunctions()).reduce(
+      (acc, fun) => acc.concat(this.checkFunction(fun)),
+      [] as MistiTactWarning[],
+    );
+  }
+
+  private checkFunction(fun: any): MistiTactWarning[] {
+    return foldStatements(
+      fun,
+      (acc, stmt) => {
+        forEachExpression(stmt, (expr) => {
+          if (
+            expr.kind === "op_binary" &&
+            (expr.op === "&&" || expr.op === "||")
+          ) {
+            const leftConst = this.isConstantExpression(expr.left);
+            const rightConst = this.isConstantExpression(expr.right);
+            const leftExpensive = this.isExpensiveFunction(expr.left);
+            if (
+              (expr.op === "&&" && !leftConst && rightConst) ||
+              (expr.op === "||" && leftConst && !rightConst)
+            ) {
+              acc.push(
+                this.makeWarning(
+                  `Consider reordering: ${
+                    leftExpensive
+                      ? "Move expensive function call to the end."
+                      : "Move constant to the left."
+                  }`,
+                  expr.loc,
+                  {
+                    suggestion: `Reorder to optimize ${expr.op} condition short-circuiting`,
+                  },
+                ),
+              );
+            }
+          }
+        });
+        return acc;
+      },
+      [] as MistiTactWarning[],
+    );
+  }
+
+  private isExpensiveFunction(expr: AstExpression): boolean {
+    if (expr.kind === "method_call") {
+      if (expr.args.length > 2) return true;
+      return this.hasNestedCallsOrRecursion(expr);
+    }
+    return false;
+  }
+
+  private hasNestedCallsOrRecursion(expr: AstMethodCall): boolean {
+    for (const arg of expr.args) {
+      if (
+        arg.kind === "method_call" ||
+        (arg.kind === "id" && arg.text === expr.method.text)
+      ) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private isConstantExpression(expr: AstExpression): boolean {
+    return (
+      evalsToValue(expr, "boolean", true) ||
+      evalsToValue(expr, "boolean", false) ||
+      expr.kind === "boolean" ||
+      (expr.kind === "op_binary" &&
+        ["==", "!=", ">", "<", ">=", "<="].includes(expr.op))
+    );
+  }
+}

src/detectors/detector.ts

@@ -385,6 +385,13 @@ export const BuiltInDetectors: Record<string, DetectorEntry> = {
       ),
     enabledByDefault: true,
   },
+  ShortCircuitCondition: {
+    loader: (ctx: MistiContext) =>
+      import("./builtin/shortCircuitCondition").then(
+        (module) => new module.ShortCircuitCondition(ctx),
+      ),
+    enabledByDefault: true,
+  },
 };
 
 /**

test/detectors/ShortCircuitCondition.expected.out

@@ -0,0 +1,35 @@
+[LOW] ShortCircuitCondition: Consider reordering: Move constant to the left.
+test/detectors/ShortCircuitCondition.tact:6:17:
+  5 |     fun testCondition1(): Bool {
+> 6 |         return (self.expensiveCheck() && (self.a > 10)); // Bad
+                      ^
+  7 |     }
+Help: Reorder to optimize && condition short-circuiting
+See: https://nowarp.io/tools/misti/docs/detectors/ShortCircuitCondition
+
+[LOW] ShortCircuitCondition: Consider reordering: Move constant to the left.
+test/detectors/ShortCircuitCondition.tact:11:17:
+  10 |     fun testCondition2(): Bool {
+> 11 |         return (true || self.expensiveCheck());  // Bad
+                       ^
+  12 |     }
+Help: Reorder to optimize || condition short-circuiting
+See: https://nowarp.io/tools/misti/docs/detectors/ShortCircuitCondition
+
+[LOW] ShortCircuitCondition: Consider reordering: Move constant to the left.
+test/detectors/ShortCircuitCondition.tact:16:17:
+  15 |     fun testCondition3(): Bool {
+> 16 |         return ((self.a > 0) && self.expensiveCheck() && false); //Bad: Should warn for moving false left
+                       ^
+  17 |     }
+Help: Reorder to optimize && condition short-circuiting
+See: https://nowarp.io/tools/misti/docs/detectors/ShortCircuitCondition
+
+[LOW] ShortCircuitCondition: Consider reordering: Move constant to the left.
+test/detectors/ShortCircuitCondition.tact:21:14:
+  20 |     fun testCondition4(): Bool {
+> 21 |         if ((self.expensiveCheck() && (self.a < 3)) || ((self.a > 10) && true)) {
+                    ^
+  22 |             return true; // Bad: Should warn for moving constants to the left
+Help: Reorder to optimize && condition short-circuiting
+See: https://nowarp.io/tools/misti/docs/detectors/ShortCircuitCondition

test/detectors/ShortCircuitCondition.tact

@@ -0,0 +1,35 @@
+contract ShortCircuitTest {
+    a: Int = 5;
+
+    // Test 1: Unoptimized && condition, should warn to reorder
+    fun testCondition1(): Bool {
+        return (self.expensiveCheck() && (self.a > 10)); // Bad
+    }
+
+    // Test 2: Unoptimized || condition, should warn to reorder
+    fun testCondition2(): Bool {
+        return (true || self.expensiveCheck());  // Bad
+    }
+
+    // Test 3: Complex condition with both optimized and unoptimized parts
+    fun testCondition3(): Bool {
+        return ((self.a > 0) && self.expensiveCheck() && false); //Bad: Should warn for moving false left
+    }
+
+    // Test 4: Nested conditions
+    fun testCondition4(): Bool {
+        if ((self.expensiveCheck() && (self.a < 3)) || ((self.a > 10) && true)) {
+            return true; // Bad: Should warn for moving constants to the left
+        }
+        return false;
+    }
+
+    // Test 5: No optimization needed, should not warn
+    fun testCondition5(): Bool {
+        return (self.a > 0 && self.expensiveCheck()); // Ok
+    }
+
+    fun expensiveCheck(): Bool {
+        return true;
+    }
+}