Detector: Missing Inheritance

aderyn_core/src/detect/detector.rs

@@ -95,6 +95,7 @@ pub fn get_all_issue_detectors() -> Vec<Box<dyn IssueDetector>> {
         Box::<BuiltinSymbolShadowDetector>::default(),
         Box::<VoidConstructorDetector>::default(),
         Box::<FunctionSelectorCollisionDetector>::default(),
+        Box::<MissingInheritanceDetector>::default(),
         Box::<UnusedImportDetector>::default(),
         Box::<UncheckedLowLevelCallDetector>::default(),
         Box::<FucntionPointerInConstructorDetector>::default(),
@@ -110,6 +111,7 @@ pub fn get_all_detectors_names() -> Vec<String> {
 #[derive(Debug, PartialEq, EnumString, Display)]
 #[strum(serialize_all = "kebab-case")]
 pub(crate) enum IssueDetectorNamePool {
+    MissingInheritance,
     UnusedImport,
     VoidConstructor,
     UncheckedLowLevelCall,
@@ -203,6 +205,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option<Box<dyn Iss
     // Expects a valid detector_name
     let detector_name = IssueDetectorNamePool::from_str(detector_name).ok()?;
     match detector_name {
+        IssueDetectorNamePool::MissingInheritance => {
+            Some(Box::<MissingInheritanceDetector>::default())
+        }
         IssueDetectorNamePool::LocalVariableShadowing => {
             Some(Box::<LocalVariableShadowingDetector>::default())
         }

aderyn_core/src/detect/low/missing_inheritance.rs

@@ -0,0 +1,200 @@
+use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::convert::identity;
+use std::error::Error;
+
+use crate::ast::{ASTNode, ContractKind, NodeID};
+
+use crate::capture;
+use crate::context::browser::ExtractVariableDeclarations;
+use crate::detect::detector::IssueDetectorNamePool;
+use crate::{
+    context::workspace_context::WorkspaceContext,
+    detect::detector::{IssueDetector, IssueSeverity},
+};
+use eyre::Result;
+
+#[derive(Default)]
+pub struct MissingInheritanceDetector {
+    // Keys are: [0] source file name, [1] line number, [2] character location of node.
+    // Do not add items manually, use `capture!` to add nodes to this BTreeMap.
+    found_instances: BTreeMap<(String, usize, String), NodeID>,
+    hints: BTreeMap<(String, usize, String), String>,
+}
+
+impl IssueDetector for MissingInheritanceDetector {
+    fn detect(&mut self, context: &WorkspaceContext) -> Result<bool, Box<dyn Error>> {
+        // Key -> Contract ID, Value -> Collection of function selectors in the contract
+        let mut contract_function_selectors: HashMap<NodeID, Vec<String>> = Default::default();
+
+        // Key -> Contract ID, Value -> Set of contract/interface IDs in it's heirarchy
+        let mut inheritance_map: HashMap<NodeID, Vec<NodeID>> = Default::default();
+
+        for contract in context.contract_definitions() {
+            if let Some(full_contract) = &contract.linearized_base_contracts {
+                inheritance_map
+                    .entry(contract.id)
+                    .or_insert(Vec::from_iter(full_contract.iter().copied()));
+
+                for contract_node_id in full_contract {
+                    if let Some(ASTNode::ContractDefinition(contract_node)) =
+                        context.nodes.get(contract_node_id)
+                    {
+                        let function_selectors: Vec<String> = contract_node
+                            .function_definitions()
+                            .iter()
+                            .flat_map(|f| f.function_selector.clone())
+                            .collect();
+
+                        let all_variables =
+                            ExtractVariableDeclarations::from(contract_node).extracted;
+
+                        let state_variable_function_selectors: Vec<String> = all_variables
+                            .into_iter()
+                            .flat_map(|v| v.function_selector.clone())
+                            .collect();
+
+                        let mut all_function_selectors = Vec::with_capacity(
+                            function_selectors.len() + state_variable_function_selectors.len(),
+                        );
+                        all_function_selectors.extend(function_selectors);
+                        all_function_selectors.extend(state_variable_function_selectors);
+
+                        contract_function_selectors
+                            .entry(contract.id)
+                            .or_insert(all_function_selectors);
+                    }
+                }
+            }
+        }
+
+        let mut results: HashMap<NodeID, BTreeSet<String>> = Default::default();
+
+        for (contract_id, contract_selectors) in &contract_function_selectors {
+            if contract_selectors.is_empty() {
+                continue;
+            }
+            if let Some(ASTNode::ContractDefinition(c)) = context.nodes.get(contract_id) {
+                if c.kind != ContractKind::Contract || c.is_abstract.map_or(false, identity) {
+                    continue;
+                }
+            }
+            let inheritances = inheritance_map.entry(*contract_id).or_default();
+            for (potentially_missing_inheritance, missing_function_selectors) in
+                &contract_function_selectors
+            {
+                // Check that it's not empty
+                if missing_function_selectors.is_empty() {
+                    continue;
+                }
+
+                // Check that it's not the same contract
+                if potentially_missing_inheritance == contract_id {
+                    continue;
+                }
+
+                // Check that it's not already inherited
+                if inheritances.contains(potentially_missing_inheritance) {
+                    continue;
+                }
+
+                if let Some(ASTNode::ContractDefinition(c)) =
+                    context.nodes.get(potentially_missing_inheritance)
+                {
+                    if c.kind == ContractKind::Interface || c.is_abstract.map_or(false, identity) {
+                        // Check that the contract is compatible with the missing inheritance
+                        if missing_function_selectors
+                            .iter()
+                            .all(|s| contract_selectors.contains(s))
+                        {
+                            results
+                                .entry(*contract_id)
+                                .or_default()
+                                .insert(c.name.clone());
+                        }
+                    }
+                }
+            }
+        }
+
+        for (contract, missing_inheritances) in results {
+            if let Some(ASTNode::ContractDefinition(c)) = context.nodes.get(&contract) {
+                // If the contract c already has some inheritance, don't report it because we want
+                // to respect the developer's choice.
+                if c.linearized_base_contracts
+                    .as_ref()
+                    .is_some_and(|chain| chain.len() != 1)
+                {
+                    continue;
+                }
+                let missing_inheritances_vector =
+                    missing_inheritances.iter().cloned().collect::<Vec<_>>();
+                let missing_inheritaces_string = missing_inheritances_vector.join(", ");
+
+                let hint = format!(
+                    "Is this contract supposed to implement an interface? Consider extending one of the following: {}",
+                    missing_inheritaces_string
+                );
+
+                capture!(self, context, c, hint);
+            }
+        }
+
+        Ok(!self.found_instances.is_empty())
+    }
+
+    fn severity(&self) -> IssueSeverity {
+        IssueSeverity::Low
+    }
+
+    fn hints(&self) -> BTreeMap<(String, usize, String), String> {
+        self.hints.clone()
+    }
+
+    fn title(&self) -> String {
+        String::from("Potentially missing inheritance for contract.")
+    }
+
+    fn description(&self) -> String {
+        String::from("There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract.")
+    }
+
+    fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> {
+        self.found_instances.clone()
+    }
+
+    fn name(&self) -> String {
+        format!("{}", IssueDetectorNamePool::MissingInheritance)
+    }
+}
+
+#[cfg(test)]
+mod missing_inheritance_tests {
+    use serial_test::serial;
+
+    use crate::detect::{
+        detector::IssueDetector, low::missing_inheritance::MissingInheritanceDetector,
+    };
+
+    #[test]
+    #[serial]
+    fn test_missing_inheritance() {
+        let context = crate::detect::test_utils::load_solidity_source_unit(
+            "../tests/contract-playground/src/MissingInheritance.sol",
+        );
+
+        let mut detector = MissingInheritanceDetector::default();
+        let found = detector.detect(&context).unwrap();
+        // assert that the detector found an issue
+        assert!(found);
+
+        println!("{:#?}", detector.instances());
+
+        // assert that the detector found the correct number of instances
+        assert_eq!(detector.instances().len(), 2);
+        // assert the severity is low
+        assert_eq!(
+            detector.severity(),
+            crate::detect::detector::IssueSeverity::Low
+        );
+    }
+}

aderyn_core/src/detect/low/mod.rs

@@ -17,6 +17,7 @@ pub(crate) mod function_pointer_in_constructor;
 pub(crate) mod inconsistent_type_names;
 pub(crate) mod large_literal_value;
 pub(crate) mod local_variable_shadowing;
+pub(crate) mod missing_inheritance;
 pub(crate) mod non_reentrant_before_others;
 pub(crate) mod public_variable_read_in_external_context;
 pub(crate) mod push_0_opcode;
@@ -59,6 +60,7 @@ pub use function_pointer_in_constructor::FucntionPointerInConstructorDetector;
 pub use inconsistent_type_names::InconsistentTypeNamesDetector;
 pub use large_literal_value::LargeLiteralValueDetector;
 pub use local_variable_shadowing::LocalVariableShadowingDetector;
+pub use missing_inheritance::MissingInheritanceDetector;
 pub use non_reentrant_before_others::NonReentrantBeforeOthersDetector;
 pub use public_variable_read_in_external_context::PublicVariableReadInExternalContextDetector;
 pub use push_0_opcode::PushZeroOpcodeDetector;

reports/report.json

@@ -1,7 +1,7 @@
 {
   "files_summary": {
-    "total_source_units": 106,
-    "total_sloc": 3470
+    "total_source_units": 107,
+    "total_sloc": 3509
   },
   "files_details": {
     "files_details": [
@@ -185,6 +185,10 @@
         "file_path": "src/LocalVariableShadow.sol",
         "n_sloc": 23
       },
+      {
+        "file_path": "src/MissingInheritance.sol",
+        "n_sloc": 39
+      },
       {
         "file_path": "src/MisusedBoolean.sol",
         "n_sloc": 67
@@ -433,7 +437,7 @@
   },
   "issue_count": {
     "high": 42,
-    "low": 41
+    "low": 42
   },
   "high_issues": {
     "issues": [
@@ -5923,6 +5927,34 @@
           }
         ]
       },
+      {
+        "title": "Potentially missing inheritance for contract.",
+        "description": "There is an interface / abstract contract that is potentially missing (not included in) the inheritance of this contract.",
+        "detector_name": "missing-inheritance",
+        "instances": [
+          {
+            "contract_path": "src/MissingInheritance.sol",
+            "line_no": 7,
+            "src": "83:25",
+            "src_char": "83:25",
+            "hint": "Is this contract supposed to implement an interface? Consider extending one of the following: IMissingInheritanceCounter"
+          },
+          {
+            "contract_path": "src/MissingInheritance.sol",
+            "line_no": 41,
+            "src": "754:16",
+            "src_char": "754:16",
+            "hint": "Is this contract supposed to implement an interface? Consider extending one of the following: IMissingChild, IMissingParent"
+          },
+          {
+            "contract_path": "src/TestERC20.sol",
+            "line_no": 4,
+            "src": "70:9",
+            "src_char": "70:9",
+            "hint": "Is this contract supposed to implement an interface? Consider extending one of the following: IERC20"
+          }
+        ]
+      },
       {
         "title": "Unused Imports",
         "description": "Redundant import statement. Consider removing it.",
@@ -6254,6 +6286,7 @@
     "builtin-symbol-shadow",
     "void-constructor",
     "function-selector-collision",
+    "missing-inheritance",
     "unused-import",
     "unchecked-low-level-call",
     "function-pointer-in-constructor",