New detector: EtaLikeSimplifications

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
+- `EtaLikeSimplifications` detector: PR [#198](https://github.com/nowarp/misti/pull/198)
 - `ShortCircuitCondition` detector: PR [#202](https://github.com/nowarp/misti/pull/202)
 ### Changed 
 - `SuspiciousMessageMode` detector now suggests using SendDefaultMode instead of 0 for mode: PR [#199](https://github.com/nowarp/misti/pull/199/)

src/detectors/builtin/etaLikeSimplifications.ts

@@ -0,0 +1,183 @@
+import { CompilationUnit } from "../../internals/ir";
+import { forEachStatement, forEachExpression } from "../../internals/tact";
+import { evalToType, evalsToValue } from "../../internals/tact/";
+import { MistiTactWarning, Severity } from "../../internals/warnings";
+import { ASTDetector } from "../detector";
+import {
+  AstNode,
+  AstStatement,
+  AstExpression,
+  AstOpBinary,
+  AstStatementReturn,
+  AstOpUnary,
+} from "@tact-lang/compiler/dist/grammar/ast";
+import { prettyPrint } from "@tact-lang/compiler/dist/prettyPrinter";
+
+/**
+ * Detects opportunities for simplifying code by eliminating redundant boolean expressions and statements.
+ *
+ * ## Why is it bad?
+ * Redundant code can make programs less efficient and harder to read. Simplifying such code improves readability,
+ * maintainability, and can prevent potential logical errors.
+ *
+ * **What it checks:**
+ * - `if` statements that return boolean literals directly based on a condition.
+ * - Comparisons of boolean expressions with boolean literals (`true` or `false`).
+ * - Conditional expressions (ternary operators) that return boolean literals.
+ *
+ * ## Example
+ *
+ * ```tact
+ * // Redundant 'if' statement:
+ * if (condition) {
+ *     return true;
+ * } else {
+ *     return false;
+ * }
+ * // Simplify to:
+ * return condition;
+ *
+ * // Redundant comparison:
+ * return a == true;
+ * // Simplify to:
+ * return a;
+ *
+ * // Redundant conditional expression:
+ * return b ? true : false;
+ * // Simplify to:
+ * return b;
+ * ```
+ */
+export class EtaLikeSimplifications extends ASTDetector {
+  severity = Severity.LOW;
+
+  async check(cu: CompilationUnit): Promise<MistiTactWarning[]> {
+    const warnings: MistiTactWarning[] = [];
+    const entries = cu.ast.getProgramEntries();
+    for (const node of entries) {
+      this.analyzeNode(node, warnings);
+    }
+    return warnings;
+  }
+
+  private analyzeNode(node: AstNode, warnings: MistiTactWarning[]): void {
+    forEachStatement(node, (stmt) => {
+      this.checkStatement(stmt, warnings);
+    });
+    forEachExpression(node, (expr) => {
+      this.checkExpression(expr, warnings);
+    });
+  }
+
+  private checkStatement(
+    stmt: AstStatement,
+    warnings: MistiTactWarning[],
+  ): void {
+    if (stmt.kind === "statement_condition") {
+      const ifStmt = stmt;
+      if (
+        ifStmt.trueStatements.length === 1 &&
+        ifStmt.falseStatements &&
+        ifStmt.falseStatements.length === 1 &&
+        ifStmt.trueStatements[0].kind === "statement_return" &&
+        ifStmt.falseStatements[0].kind === "statement_return"
+      ) {
+        const trueReturn = ifStmt.trueStatements[0] as AstStatementReturn;
+        const falseReturn = ifStmt.falseStatements[0] as AstStatementReturn;
+        if (
+          this.isBooleanLiteral(trueReturn.expression, true) &&
+          this.isBooleanLiteral(falseReturn.expression, false)
+        ) {
+          warnings.push(
+            this.makeWarning(
+              "Simplify 'if' statement by returning the condition directly",
+              stmt.loc,
+              {
+                suggestion: `return ${prettyPrint(ifStmt.condition)};`,
+              },
+            ),
+          );
+        }
+      }
+    }
+  }
+
+  private checkExpression(
+    expr: AstExpression,
+    warnings: MistiTactWarning[],
+  ): void {
+    if (expr.kind === "op_binary") {
+      const binaryExpr = expr as AstOpBinary;
+      if (binaryExpr.op === "==" || binaryExpr.op === "!=") {
+        const { right } = binaryExpr;
+        if (this.isBooleanLiteral(right)) {
+          const simplified = this.getSimplifiedBooleanExpression(binaryExpr);
+          warnings.push(
+            this.makeWarning(
+              "Redundant comparison with boolean literal",
+              expr.loc,
+              {
+                suggestion: prettyPrint(simplified),
+              },
+            ),
+          );
+        }
+      }
+    }
+    if (expr.kind === "conditional") {
+      if (
+        this.isBooleanLiteral(expr.thenBranch, true) &&
+        this.isBooleanLiteral(expr.elseBranch, false)
+      ) {
+        warnings.push(
+          this.makeWarning(
+            "Simplify conditional expression by using the condition directly",
+            expr.loc,
+            {
+              suggestion: prettyPrint(expr.condition),
+            },
+          ),
+        );
+      }
+    }
+  }
+
+  private isBooleanLiteral(
+    expr: AstExpression | null,
+    value?: boolean,
+  ): boolean {
+    if (expr == null) return false;
+    if (value === undefined) {
+      return evalToType(expr, "boolean") !== undefined;
+    }
+    return evalsToValue(expr, "boolean", value);
+  }
+
+  private getSimplifiedBooleanExpression(
+    binaryExpr: AstOpBinary,
+  ): AstExpression {
+    const negated = binaryExpr.op === "!=";
+    if (this.isBooleanLiteral(binaryExpr.right, true)) {
+      return negated
+        ? ({
+            kind: "op_unary",
+            op: "!",
+            operand: binaryExpr.left,
+            id: binaryExpr.id,
+            loc: binaryExpr.loc,
+          } as AstOpUnary)
+        : binaryExpr.left;
+    } else if (this.isBooleanLiteral(binaryExpr.right, false)) {
+      return negated
+        ? binaryExpr.left
+        : ({
+            kind: "op_unary",
+            op: "!",
+            operand: binaryExpr.left,
+            id: binaryExpr.id,
+            loc: binaryExpr.loc,
+          } as AstOpUnary);
+    }
+    return binaryExpr;
+  }
+}

src/detectors/detector.ts

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

test/detectors/EtaLikeSimplifications.expected.out

@@ -0,0 +1,26 @@
+[LOW] EtaLikeSimplifications: Simplify 'if' statement by returning the condition directly
+test/detectors/EtaLikeSimplifications.tact:4:9:
+  3 |     fun redundantIf(condition: Bool): Bool {
+> 4 |         if (condition) {
+              ^
+  5 |             return true;
+Help: return condition;
+See: https://nowarp.io/tools/misti/docs/detectors/EtaLikeSimplifications
+
+[LOW] EtaLikeSimplifications: Redundant comparison with boolean literal
+test/detectors/EtaLikeSimplifications.tact:13:16:
+  12 |     fun redundantComparison(a: Bool): Bool {
+> 13 |         return a == true;
+                      ^
+  14 |     }
+Help: a
+See: https://nowarp.io/tools/misti/docs/detectors/EtaLikeSimplifications
+
+[LOW] EtaLikeSimplifications: Simplify conditional expression by using the condition directly
+test/detectors/EtaLikeSimplifications.tact:18:16:
+  17 |     fun redundantTernary(b: Bool): Bool {
+> 18 |         return b ? true : false;
+                      ^
+  19 |     }
+Help: b
+See: https://nowarp.io/tools/misti/docs/detectors/EtaLikeSimplifications

test/detectors/EtaLikeSimplifications.tact

@@ -0,0 +1,25 @@
+contract TestContract {
+    // Bad: Example of redundant if statement
+    fun redundantIf(condition: Bool): Bool {
+        if (condition) {
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    // Bad: Example of redundant boolean comparison
+    fun redundantComparison(a: Bool): Bool {
+        return a == true;
+    }
+
+    // Bad: Example of redundant ternary expression
+    fun redundantTernary(b: Bool): Bool {
+        return b ? true : false;
+    }
+
+    // Ok: Correct usage (should not trigger warnings)
+    fun correctUsage(condition: Bool): Bool {
+        return condition;
+    }
+}