Detector: Boolean equality (#633)

aderyn_core/src/detect/detector.rs

@@ -74,6 +74,7 @@ pub fn get_all_issue_detectors() -> Vec<Box<dyn IssueDetector>> {
         Box::<WeakRandomnessDetector>::default(),
         Box::<PreDeclaredLocalVariableUsageDetector>::default(),
         Box::<DeletionNestedMappingDetector>::default(),
+        Box::<BooleanEqualityDetector>::default(),
         Box::<TxOriginUsedForAuthDetector>::default(),
         Box::<MsgValueUsedInLoopDetector>::default(),
         Box::<ContractLocksEtherDetector>::default(),
@@ -147,6 +148,7 @@ pub(crate) enum IssueDetectorNamePool {
     WeakRandomness,
     PreDeclaredLocalVariableUsage,
     DeleteNestedMapping,
+    BooleanEquality,
     TxOriginUsedForAuth,
     MsgValueInLoop,
     ContractLocksEther,
@@ -308,6 +310,7 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option<Box<dyn Iss
         IssueDetectorNamePool::DeleteNestedMapping => {
             Some(Box::<DeletionNestedMappingDetector>::default())
         }
+        IssueDetectorNamePool::BooleanEquality => Some(Box::<BooleanEqualityDetector>::default()),
         IssueDetectorNamePool::TxOriginUsedForAuth => {
             Some(Box::<TxOriginUsedForAuthDetector>::default())
         }

aderyn_core/src/detect/low/boolean_equality.rs

@@ -0,0 +1,50 @@
+use crate::detect::helpers::is_constant_boolean;
+use crate::issue_detector;
+use eyre::Result;
+
+issue_detector! {
+    BooleanEqualityDetector;
+
+    severity: Low,
+    title: "Boolean equality is not required.",
+    desc: "If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. Just use `if(x)` and `if(!x)` respectively.",
+    name: BooleanEquality,
+
+    |context| {
+        for binary_operation in context.binary_operations() {
+            if binary_operation.operator == "=="
+                && [
+                    binary_operation.left_expression.as_ref(),
+                    binary_operation.right_expression.as_ref(),
+                ]
+                .iter()
+                .any(|&operand| is_constant_boolean(context, operand))
+            {
+                grab!(binary_operation);
+            }
+        }
+    }
+
+}
+
+#[cfg(test)]
+mod boolean_equality_tests {
+    use serial_test::serial;
+
+    use crate::detect::{detector::IssueDetector, low::boolean_equality::BooleanEqualityDetector};
+
+    #[test]
+    #[serial]
+    fn test_boolean_equality_by_loading_contract_directly() {
+        let context = crate::detect::test_utils::load_solidity_source_unit(
+            "../tests/contract-playground/src/BooleanEquality.sol",
+        );
+
+        let mut detector = BooleanEqualityDetector::default();
+        let found = detector.detect(&context).unwrap();
+        // assert that the detector found an issue
+        assert!(found);
+        // assert that the detector found the correct number of instances
+        assert_eq!(detector.instances().len(), 4);
+    }
+}

aderyn_core/src/detect/low/mod.rs

@@ -1,3 +1,4 @@
+pub(crate) mod boolean_equality;
 pub(crate) mod centralization_risk;
 pub(crate) mod constants_instead_of_literals;
 pub(crate) mod contracts_with_todos;
@@ -24,6 +25,7 @@ pub(crate) mod useless_modifier;
 pub(crate) mod useless_public_function;
 pub(crate) mod zero_address_check;
 
+pub use boolean_equality::BooleanEqualityDetector;
 pub use centralization_risk::CentralizationRiskDetector;
 pub use constants_instead_of_literals::ConstantsInsteadOfLiteralsDetector;
 pub use contracts_with_todos::ContractsWithTodosDetector;

reports/report.json

@@ -1,7 +1,7 @@
 {
   "files_summary": {
-    "total_source_units": 77,
-    "total_sloc": 2225
+    "total_source_units": 78,
+    "total_sloc": 2252
   },
   "files_details": {
     "files_details": [
@@ -21,6 +21,10 @@
         "file_path": "src/AssemblyExample.sol",
         "n_sloc": 9
       },
+      {
+        "file_path": "src/BooleanEquality.sol",
+        "n_sloc": 27
+      },
       {
         "file_path": "src/CallGraphTests.sol",
         "n_sloc": 49
@@ -317,7 +321,7 @@
   },
   "issue_count": {
     "high": 36,
-    "low": 25
+    "low": 26
   },
   "high_issues": {
     "issues": [
@@ -2564,6 +2568,42 @@
         "description": "If the same constant literal value is used multiple times, create a constant state variable and reference it throughout the contract.",
         "detector_name": "constants-instead-of-literals",
         "instances": [
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 6,
+            "src": "170:3",
+            "src_char": "170:3"
+          },
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 13,
+            "src": "330:3",
+            "src_char": "330:3"
+          },
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 15,
+            "src": "360:3",
+            "src_char": "360:3"
+          },
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 20,
+            "src": "492:3",
+            "src_char": "492:3"
+          },
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 27,
+            "src": "653:3",
+            "src_char": "653:3"
+          },
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 29,
+            "src": "683:3",
+            "src_char": "683:3"
+          },
           {
             "contract_path": "src/Casting.sol",
             "line_no": 16,
@@ -4039,6 +4079,37 @@
             "src_char": "1175:14"
           }
         ]
+      },
+      {
+        "title": "Boolean equality is not required.",
+        "description": "If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. Just use `if(x)` and `if(!x)` respectively.",
+        "detector_name": "boolean-equality",
+        "instances": [
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 5,
+            "src": "133:14",
+            "src_char": "133:14"
+          },
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 12,
+            "src": "292:15",
+            "src_char": "292:15"
+          },
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 19,
+            "src": "454:15",
+            "src_char": "454:15"
+          },
+          {
+            "contract_path": "src/BooleanEquality.sol",
+            "line_no": 26,
+            "src": "614:16",
+            "src_char": "614:16"
+          }
+        ]
       }
     ]
   },
@@ -4101,6 +4172,7 @@
     "weak-randomness",
     "pre-declared-local-variable-usage",
     "delete-nested-mapping",
+    "boolean-equality",
     "tx-origin-used-for-auth",
     "msg-value-in-loop",
     "contract-locks-ether"

reports/report.md

@@ -70,6 +70,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
   - [L-23: Incorrect Order of Division and Multiplication](#l-23-incorrect-order-of-division-and-multiplication)
   - [L-24: Redundant statements have no effect.](#l-24-redundant-statements-have-no-effect)
   - [L-25: Public variables of a contract read in an external context (using `this`).](#l-25-public-variables-of-a-contract-read-in-an-external-context-using-this)
+  - [L-26: Boolean equality is not required.](#l-26-boolean-equality-is-not-required)
 
 
 # Summary
@@ -78,8 +79,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 
 | Key | Value |
 | --- | --- |
-| .sol Files | 77 |
-| Total nSLOC | 2225 |
+| .sol Files | 78 |
+| Total nSLOC | 2252 |
 
 
 ## Files Details
@@ -90,6 +91,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 | src/AdminContract.sol | 11 |
 | src/ArbitraryTransferFrom.sol | 37 |
 | src/AssemblyExample.sol | 9 |
+| src/BooleanEquality.sol | 27 |
 | src/CallGraphTests.sol | 49 |
 | src/Casting.sol | 126 |
 | src/CompilerBugStorageSignedIntegerArray.sol | 13 |
@@ -163,15 +165,15 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 | src/reused_contract_name/ContractB.sol | 7 |
 | src/uniswap/UniswapV2Swapper.sol | 50 |
 | src/uniswap/UniswapV3Swapper.sol | 150 |
-| **Total** | **2225** |
+| **Total** | **2252** |
 
 
 ## Issue Summary
 
 | Category | No. of Issues |
 | --- | --- |
 | High | 36 |
-| Low | 25 |
+| Low | 26 |
 
 
 # High Issues
@@ -2597,9 +2599,45 @@ Instead of marking a function as `public`, consider marking it as `external` if
 
 If the same constant literal value is used multiple times, create a constant state variable and reference it throughout the contract.
 
-<details><summary>38 Found Instances</summary>
+<details><summary>44 Found Instances</summary>
 
 
+- Found in src/BooleanEquality.sol [Line: 6](../tests/contract-playground/src/BooleanEquality.sol#L6)
+
+	```solidity
+	            return 100;
+	```
+
+- Found in src/BooleanEquality.sol [Line: 13](../tests/contract-playground/src/BooleanEquality.sol#L13)
+
+	```solidity
+	            return 200;
+	```
+
+- Found in src/BooleanEquality.sol [Line: 15](../tests/contract-playground/src/BooleanEquality.sol#L15)
+
+	```solidity
+	        return 130;
+	```
+
+- Found in src/BooleanEquality.sol [Line: 20](../tests/contract-playground/src/BooleanEquality.sol#L20)
+
+	```solidity
+	            return 100;
+	```
+
+- Found in src/BooleanEquality.sol [Line: 27](../tests/contract-playground/src/BooleanEquality.sol#L27)
+
+	```solidity
+	            return 200;
+	```
+
+- Found in src/BooleanEquality.sol [Line: 29](../tests/contract-playground/src/BooleanEquality.sol#L29)
+
+	```solidity
+	        return 130;
+	```
+
 - Found in src/Casting.sol [Line: 16](../tests/contract-playground/src/Casting.sol#L16)
 
 	```solidity
@@ -4130,3 +4168,38 @@ The contract reads it's own variable using `this` which adds an unnecessary STAT
 
 
 
+## L-26: Boolean equality is not required.
+
+If `x` is a boolean, there is no need to do `if(x == true)` or `if(x == false)`. Just use `if(x)` and `if(!x)` respectively.
+
+<details><summary>4 Found Instances</summary>
+
+
+- Found in src/BooleanEquality.sol [Line: 5](../tests/contract-playground/src/BooleanEquality.sol#L5)
+
+	```solidity
+	        if (isEven == true) {
+	```
+
+- Found in src/BooleanEquality.sol [Line: 12](../tests/contract-playground/src/BooleanEquality.sol#L12)
+
+	```solidity
+	        if (isEven == !true) {
+	```
+
+- Found in src/BooleanEquality.sol [Line: 19](../tests/contract-playground/src/BooleanEquality.sol#L19)
+
+	```solidity
+	        if (isEven == false) {
+	```
+
+- Found in src/BooleanEquality.sol [Line: 26](../tests/contract-playground/src/BooleanEquality.sol#L26)
+
+	```solidity
+	        if (isEven == !false) {
+	```
+
+</details>
+
+
+