diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eb6381f..06c6e1b9 100644 --- a/CHANGELOG.md +++ b/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/) diff --git a/src/detectors/builtin/etaLikeSimplifications.ts b/src/detectors/builtin/etaLikeSimplifications.ts new file mode 100644 index 00000000..25b64f8e --- /dev/null +++ b/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 { + 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; + } +} diff --git a/src/detectors/detector.ts b/src/detectors/detector.ts index 70b55fc3..eb720dc5 100644 --- a/src/detectors/detector.ts +++ b/src/detectors/detector.ts @@ -385,6 +385,13 @@ export const BuiltInDetectors: Record = { ), enabledByDefault: true, }, + EtaLikeSimplifications: { + loader: (ctx: MistiContext) => + import("./builtin/etaLikeSimplifications").then( + (module) => new module.EtaLikeSimplifications(ctx), + ), + enabledByDefault: true, + }, }; /** diff --git a/test/detectors/EtaLikeSimplifications.expected.out b/test/detectors/EtaLikeSimplifications.expected.out new file mode 100644 index 00000000..ee449345 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/test/detectors/EtaLikeSimplifications.tact b/test/detectors/EtaLikeSimplifications.tact new file mode 100644 index 00000000..6513e683 --- /dev/null +++ b/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; + } +}