Detector: Boolean equality

aderyn_core/src/detect/detector.rs

@@ -1,3 +1,4 @@
+use boolean_equality::BooleanEqualityDetector;
 use serde::{Deserialize, Serialize};
 use strum::{Display, EnumCount, EnumIter, EnumString};
 
@@ -68,6 +69,7 @@ pub fn get_all_issue_detectors() -> Vec<Box<dyn IssueDetector>> {
         Box::<WeakRandomnessDetector>::default(),
         Box::<PreDeclaredLocalVariableUsageDetector>::default(),
         Box::<DeletionNestedMappingDetector>::default(),
+        Box::<BooleanEqualityDetector>::default(),
     ]
 }
 
@@ -132,6 +134,7 @@ pub(crate) enum IssueDetectorNamePool {
     WeakRandomness,
     PreDeclaredLocalVariableUsage,
     DeleteNestedMapping,
+    BooleanEquality,
     // NOTE: `Undecided` will be the default name (for new bots).
     // If it's accepted, a new variant will be added to this enum before normalizing it in aderyn
     Undecided,
@@ -274,6 +277,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::Undecided => None,
     }
 }

aderyn_core/src/detect/high/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: High,
+    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, high::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/high/mod.rs

@@ -1,6 +1,7 @@
 pub(crate) mod arbitrary_transfer_from;
 pub(crate) mod avoid_abi_encode_packed;
 pub(crate) mod block_timestamp_deadline;
+pub(crate) mod boolean_equality;
 pub(crate) mod dangerous_unary_operator;
 pub(crate) mod delegate_call_in_loop;
 pub(crate) mod delegate_call_no_address_check;

reports/adhoc-sol-files-highs-only-report.json

@@ -190,6 +190,7 @@
     "dangerous-unary-operator",
     "weak-randomness",
     "pre-declared-local-variable-usage",
-    "delete-nested-mapping"
+    "delete-nested-mapping",
+    "boolean-equality"
   ]
 }

reports/report.json

@@ -1,7 +1,7 @@
 {
   "files_summary": {
-    "total_source_units": 67,
-    "total_sloc": 1904
+    "total_source_units": 68,
+    "total_sloc": 1931
   },
   "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
@@ -276,7 +280,7 @@
     ]
   },
   "issue_count": {
-    "high": 29,
+    "high": 30,
     "low": 23
   },
   "high_issues": {
@@ -1677,6 +1681,37 @@
             "src_char": "426:25"
           }
         ]
+      },
+      {
+        "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"
+          }
+        ]
       }
     ]
   },
@@ -2175,6 +2210,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,
@@ -3475,6 +3546,7 @@
     "dangerous-unary-operator",
     "weak-randomness",
     "pre-declared-local-variable-usage",
-    "delete-nested-mapping"
+    "delete-nested-mapping",
+    "boolean-equality"
   ]
 }

reports/report.md

@@ -37,6 +37,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
   - [H-27: Weak Randomness](#h-27-weak-randomness)
   - [H-28: Usage of variable before declaration.](#h-28-usage-of-variable-before-declaration)
   - [H-29: Deletion from a nested mappping.](#h-29-deletion-from-a-nested-mappping)
+  - [H-30: Boolean equality is not required.](#h-30-boolean-equality-is-not-required)
 - [Low Issues](#low-issues)
   - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners)
   - [L-2: Solmate's SafeTransferLib does not check for token contract's existence](#l-2-solmates-safetransferlib-does-not-check-for-token-contracts-existence)
@@ -69,8 +70,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
 
 | Key | Value |
 | --- | --- |
-| .sol Files | 67 |
-| Total nSLOC | 1904 |
+| .sol Files | 68 |
+| Total nSLOC | 1931 |
 
 
 ## Files Details
@@ -81,6 +82,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/ConstantsLiterals.sol | 28 |
@@ -144,14 +146,14 @@ 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** | **1904** |
+| **Total** | **1931** |
 
 
 ## Issue Summary
 
 | Category | No. of Issues |
 | --- | --- |
-| High | 29 |
+| High | 30 |
 | Low | 23 |
 
 
@@ -1672,6 +1674,41 @@ A deletion in a structure containing a mapping will not delete the mapping. The
 
 
 
+## H-30: 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>
+
+
+
 # Low Issues
 
 ## L-1: Centralization Risk for trusted owners
@@ -2201,9 +2238,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>34 Found Instances</summary>
+<details><summary>40 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