Detector: Boolean equality

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::<ContractLocksEtherDetector>::default(),
     ]
 }
@@ -145,6 +146,7 @@ pub(crate) enum IssueDetectorNamePool {
     WeakRandomness,
     PreDeclaredLocalVariableUsage,
     DeleteNestedMapping,
+    BooleanEquality,
     ContractLocksEther,
     // 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
@@ -304,6 +306,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::ContractLocksEther => {
             Some(Box::<ContractLocksEtherDetector>::default())
         }

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 contract_locks_ether;
 pub(crate) mod dangerous_strict_equality_balance;
 pub(crate) mod dangerous_unary_operator;
@@ -12,7 +13,6 @@ pub(crate) mod enumerable_loop_removal;
 pub(crate) mod experimental_encoder;
 pub(crate) mod incorrect_caret_operator;
 pub(crate) mod incorrect_shift_order;
-pub(crate) mod misused_boolean;
 pub(crate) mod multiple_constructors;
 pub(crate) mod nested_struct_in_mapping;
 pub(crate) mod pre_declared_variable_usage;
@@ -36,6 +36,7 @@ pub(crate) mod yul_return;
 pub use arbitrary_transfer_from::ArbitraryTransferFromDetector;
 pub use avoid_abi_encode_packed::AvoidAbiEncodePackedDetector;
 pub use block_timestamp_deadline::BlockTimestampDeadlineDetector;
+pub use boolean_equality::BooleanEqualityDetector;
 pub use contract_locks_ether::ContractLocksEtherDetector;
 pub use dangerous_strict_equality_balance::DangerousStrictEqualityOnBalanceDetector;
 pub use dangerous_unary_operator::DangerousUnaryOperatorDetector;
@@ -47,7 +48,6 @@ pub use enumerable_loop_removal::EnumerableLoopRemovalDetector;
 pub use experimental_encoder::ExperimentalEncoderDetector;
 pub use incorrect_caret_operator::IncorrectUseOfCaretOperatorDetector;
 pub use incorrect_shift_order::IncorrectShiftOrderDetector;
-pub use misused_boolean::MisusedBooleanDetector;
 pub use multiple_constructors::MultipleConstructorsDetector;
 pub use nested_struct_in_mapping::NestedStructInMappingDetector;
 pub use pre_declared_variable_usage::PreDeclaredLocalVariableUsageDetector;

aderyn_core/src/detect/high/misused_boolean.rs → aderyn_core/src/detect/low/misused_boolean.rs

@@ -5,7 +5,7 @@ use eyre::Result;
 issue_detector! {
     MisusedBooleanDetector;
 
-    severity: High,
+    severity: Low,
     title: "Misused boolean with logical operators",
     desc: "The patterns `if (… || true)` and `if (.. && false)` will always evaluate to true and false respectively.",
     name: MisusedBoolean,
@@ -39,7 +39,7 @@ issue_detector! {
 mod misused_boolean_tests {
     use serial_test::serial;
 
-    use crate::detect::{detector::IssueDetector, high::misused_boolean::MisusedBooleanDetector};
+    use crate::detect::{detector::IssueDetector, low::misused_boolean::MisusedBooleanDetector};
 
     #[test]
     #[serial]

aderyn_core/src/detect/low/mod.rs

@@ -7,6 +7,7 @@ pub(crate) mod ecrecover;
 pub(crate) mod empty_blocks;
 pub(crate) mod inconsistent_type_names;
 pub(crate) mod large_literal_value;
+pub(crate) mod misused_boolean;
 pub(crate) mod non_reentrant_before_others;
 pub(crate) mod public_variable_read_in_external_context;
 pub(crate) mod push_0_opcode;
@@ -33,6 +34,7 @@ pub use ecrecover::EcrecoverDetector;
 pub use empty_blocks::EmptyBlockDetector;
 pub use inconsistent_type_names::InconsistentTypeNamesDetector;
 pub use large_literal_value::LargeLiteralValueDetector;
+pub use misused_boolean::MisusedBooleanDetector;
 pub use non_reentrant_before_others::NonReentrantBeforeOthersDetector;
 pub use public_variable_read_in_external_context::PublicVariableReadInExternalContextDetector;
 pub use push_0_opcode::PushZeroOpcodeDetector;

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

@@ -182,7 +182,6 @@
     "yul-return",
     "state-variable-shadowing",
     "unchecked-send",
-    "misused-boolean",
     "send-ether-no-checks",
     "delegate-call-unchecked-address",
     "tautological-compare",
@@ -195,6 +194,7 @@
     "weak-randomness",
     "pre-declared-local-variable-usage",
     "delete-nested-mapping",
+    "boolean-equality",
     "contract-locks-ether"
   ]
 }