refactor: better eval hooks handling in contract calls

components/clarinet-cli/src/generate/contract.rs

@@ -31,11 +31,6 @@ impl GetChangesForRmContract {
         f.append_path("tests")?;
         f.append_path(&name)?;
         if !f.exists() {
-            format!(
-                "{} tests/{} doesn't exist. Skipping removal",
-                red!("Warning"),
-                name
-            );
             return Ok(());
         }
         let change = FileDeletion {

components/clarinet-deployments/src/lib.rs

@@ -56,7 +56,7 @@ pub fn setup_session_with_deployment(
 ) -> DeploymentGenerationArtifacts {
     let mut session = initiate_session_from_manifest(manifest);
     let UpdateSessionExecutionResult { contracts, .. } =
-        update_session_with_deployment_plan(&mut session, deployment, contracts_asts, false, None);
+        update_session_with_deployment_plan(&mut session, deployment, contracts_asts, None);
 
     let deps = BTreeMap::new();
     let mut diags = HashMap::new();
@@ -122,7 +122,6 @@ pub fn update_session_with_deployment_plan(
     session: &mut Session,
     deployment: &DeploymentSpecification,
     contracts_asts: Option<&BTreeMap<QualifiedContractIdentifier, ContractAST>>,
-    code_coverage_enabled: bool,
     forced_min_epoch: Option<StacksEpochId>,
 ) -> UpdateSessionExecutionResult {
     update_session_with_genesis_accounts(session, deployment);
@@ -160,13 +159,7 @@ pub fn update_session_with_deployment_plan(
                         tx.contract_name.clone(),
                     );
                     let contract_ast = contracts_asts.as_ref().and_then(|m| m.get(&contract_id));
-                    let result = handle_emulated_contract_publish(
-                        session,
-                        tx,
-                        contract_ast,
-                        epoch,
-                        code_coverage_enabled,
-                    );
+                    let result = handle_emulated_contract_publish(session, tx, contract_ast, epoch);
                     contracts.insert(contract_id, result);
                 }
                 TransactionSpecification::EmulatedContractCall(tx) => {
@@ -198,7 +191,6 @@ fn handle_emulated_contract_publish(
     tx: &EmulatedContractPublishSpecification,
     contract_ast: Option<&ContractAST>,
     epoch: StacksEpochId,
-    code_coverage_enabled: bool,
 ) -> Result<ExecutionResult, Vec<Diagnostic>> {
     let default_tx_sender = session.get_tx_sender();
     session.set_tx_sender(&tx.emulated_sender.to_string());
@@ -210,12 +202,8 @@ fn handle_emulated_contract_publish(
         clarity_version: tx.clarity_version,
         epoch,
     };
-    let test_name = if code_coverage_enabled {
-        Some("__analysis__".to_string())
-    } else {
-        None
-    };
-    let result = session.deploy_contract(&contract, None, false, test_name, contract_ast);
+
+    let result = session.deploy_contract(&contract, None, false, contract_ast);
 
     session.set_tx_sender(&default_tx_sender);
     result
@@ -250,7 +238,7 @@ fn handle_emulated_contract_call(
         &tx.emulated_sender.to_string(),
         true,
         false,
-        false,
+        vec![],
         "deployment".to_string(),
     );
     if let Err(errors) = &result {
@@ -930,7 +918,7 @@ mod tests {
             location: FileLocation::from_path_string("/contracts/contract_1.clar").unwrap(),
         };
 
-        handle_emulated_contract_publish(session, &emulated_publish_spec, None, epoch, false)
+        handle_emulated_contract_publish(session, &emulated_publish_spec, None, epoch)
     }
 
     #[test]

components/clarinet-sdk-wasm/src/core.rs

@@ -9,7 +9,7 @@ use clarinet_deployments::{
 };
 use clarinet_files::chainhook_types::StacksNetwork;
 use clarinet_files::{FileAccessor, FileLocation, ProjectManifest, WASMFileSystemAccessor};
-use clarity_repl::analysis::coverage::CoverageReporter;
+use clarity_repl::analysis::coverage::{CoverageReporter, TestCoverageReport};
 use clarity_repl::clarity::analysis::contract_interface_builder::{
     ContractInterface, ContractInterfaceFunction, ContractInterfaceFunctionAccess,
 };
@@ -19,7 +19,8 @@ use clarity_repl::clarity::vm::types::{
     PrincipalData, QualifiedContractIdentifier, StandardPrincipalData,
 };
 use clarity_repl::clarity::{
-    Address, ClarityVersion, EvaluationResult, ExecutionResult, StacksEpochId, SymbolicExpression,
+    Address, ClarityVersion, EvalHook, EvaluationResult, ExecutionResult, StacksEpochId,
+    SymbolicExpression,
 };
 use clarity_repl::repl::clarity_values::{uint8_to_string, uint8_to_value};
 use clarity_repl::repl::session::BOOT_CONTRACTS_DATA;
@@ -449,7 +450,6 @@ impl SDK {
             &mut session,
             &deployment,
             Some(&artifacts.asts),
-            false,
             Some(DEFAULT_EPOCH),
         );
 
@@ -719,6 +719,16 @@ impl SDK {
             track_coverage,
         } = self.options;
 
+        let mut hooks: Vec<&mut dyn EvalHook> = Vec::new();
+        let mut coverage_hook = if track_coverage {
+            Some(TestCoverageReport::new(test_name.clone()))
+        } else {
+            None
+        };
+        if let Some(ref mut hook) = coverage_hook {
+            hooks.push(hook)
+        }
+
         let parsed_args = args
             .iter()
             .map(|a| SymbolicExpression::atom_value(uint8_to_value(a)))
@@ -733,7 +743,7 @@ impl SDK {
                 sender,
                 allow_private,
                 track_costs,
-                track_coverage,
+                hooks,
                 test_name,
             )
             .map_err(|diagnostics| {
@@ -753,6 +763,10 @@ impl SDK {
                 message
             })?;
 
+        if let Some(coverage_hook) = coverage_hook {
+            session.coverage_reports.push(coverage_hook)
+        }
+
         Ok(execution_result_to_transaction_res(&execution))
     }
 
@@ -844,7 +858,7 @@ impl SDK {
                 epoch: session.current_epoch,
             };
 
-            match session.deploy_contract(&contract, None, false, Some(args.name.clone()), None) {
+            match session.deploy_contract(&contract, None, false, None) {
                 Ok(res) => res,
                 Err(diagnostics) => {
                     let mut message = format!(

components/clarity-lsp/src/common/state.rs

@@ -654,7 +654,6 @@ pub async fn build_state(
         &mut session,
         &deployment,
         Some(&artifacts.asts),
-        false,
         Some(StacksEpochId::Epoch21),
     );
     for (contract_id, mut result) in contracts.into_iter() {

components/clarity-repl/src/analysis/mod.rs

@@ -154,9 +154,8 @@ where
     F: FnOnce(&mut AnalysisDatabase) -> std::result::Result<T, E>,
 {
     conn.begin();
-    let result = f(conn).map_err(|e| {
+    let result = f(conn).inspect_err(|_| {
         conn.roll_back().expect("Failed to roll back");
-        e
     })?;
     conn.commit().expect("Failed to commit");
     Ok(result)

components/clarity-repl/src/repl/interpreter.rs

@@ -832,7 +832,7 @@ impl ClarityInterpreter {
         clarity_version: ClarityVersion,
         track_costs: bool,
         allow_private: bool,
-        eval_hooks: Option<Vec<&mut dyn EvalHook>>,
+        mut eval_hooks: Vec<&mut dyn EvalHook>,
     ) -> Result<ExecutionResult, String> {
         let mut conn = ClarityDatabase::new(
             &mut self.clarity_datastore,
@@ -860,11 +860,11 @@ impl ClarityInterpreter {
         let mut global_context =
             GlobalContext::new(false, CHAIN_ID_TESTNET, conn, cost_tracker, epoch);
 
-        if let Some(mut in_hooks) = eval_hooks {
-            let mut hooks: Vec<&mut dyn EvalHook> = Vec::new();
-            for hook in in_hooks.drain(..) {
-                hooks.push(hook);
-            }
+        let mut hooks: Vec<&mut dyn EvalHook> = Vec::new();
+        for hook in eval_hooks.drain(..) {
+            hooks.push(hook);
+        }
+        if !hooks.is_empty() {
             global_context.eval_hooks = Some(hooks);
         }
 

components/clarity-repl/src/repl/session.rs

@@ -183,7 +183,7 @@ impl Session {
                 };
 
                 // Result ignored, boot contracts are trusted to be valid
-                let _ = self.deploy_contract(&contract, None, false, None, None);
+                let _ = self.deploy_contract(&contract, None, false, None);
             }
         }
     }
@@ -528,7 +528,6 @@ impl Session {
         contract: &ClarityContract,
         eval_hooks: Option<Vec<&mut dyn EvalHook>>,
         cost_track: bool,
-        test_name: Option<String>,
         ast: Option<&ContractAST>,
     ) -> Result<ExecutionResult, Vec<Diagnostic>> {
         if contract.epoch != self.current_epoch {
@@ -556,32 +555,17 @@ impl Session {
             };
             return Err(vec![diagnostic]);
         }
-        let mut hooks: Vec<&mut dyn EvalHook> = Vec::new();
-        let mut coverage = test_name.map(TestCoverageReport::new);
-        if let Some(coverage) = &mut coverage {
-            hooks.push(coverage);
-        };
-
-        if let Some(mut in_hooks) = eval_hooks {
-            for hook in in_hooks.drain(..) {
-                hooks.push(hook);
-            }
-        }
 
         let contract_id =
             contract.expect_resolved_contract_identifier(Some(&self.interpreter.get_tx_sender()));
 
-        let result = self.interpreter.run(contract, ast, cost_track, Some(hooks));
+        let result = self.interpreter.run(contract, ast, cost_track, eval_hooks);
 
-        result.map(|result| {
+        result.inspect(|result| {
             if let EvaluationResult::Contract(contract_result) = &result.result {
                 self.contracts
                     .insert(contract_id.clone(), contract_result.contract.clone());
             }
-            if let Some(coverage) = coverage {
-                self.coverage_reports.push(coverage);
-            }
-            result
         })
     }
 
@@ -593,36 +577,28 @@ impl Session {
         sender: &str,
         allow_private: bool,
         track_costs: bool,
-        track_coverage: bool,
+        eval_hooks: Vec<&mut dyn EvalHook>,
         test_name: String,
     ) -> Result<ExecutionResult, Vec<Diagnostic>> {
         let initial_tx_sender = self.get_tx_sender();
+
         // Handle fully qualified contract_id and sugared syntax
         let contract_id_str = if contract.starts_with('S') {
             contract.to_string()
         } else {
             format!("{}.{}", initial_tx_sender, contract)
         };
-        let contract_id = QualifiedContractIdentifier::parse(&contract_id_str).unwrap();
-
-        let mut hooks: Vec<&mut dyn EvalHook> = vec![];
-        let mut coverage = TestCoverageReport::new(test_name.clone());
-        if track_coverage {
-            hooks.push(&mut coverage);
-        }
-
-        let clarity_version = ClarityVersion::default_for_epoch(self.current_epoch);
 
         self.set_tx_sender(sender);
         let execution = match self.interpreter.call_contract_fn(
-            &contract_id,
+            &QualifiedContractIdentifier::parse(&contract_id_str).unwrap(),
             method,
             args,
             self.current_epoch,
-            clarity_version,
+            ClarityVersion::default_for_epoch(self.current_epoch),
             track_costs,
             allow_private,
-            Some(hooks),
+            eval_hooks,
         ) {
             Ok(result) => result,
             Err(e) => {
@@ -637,10 +613,6 @@ impl Session {
         };
         self.set_tx_sender(&initial_tx_sender);
 
-        if track_coverage {
-            self.coverage_reports.push(coverage);
-        }
-
         if let Some(ref cost) = execution.cost {
             self.costs_reports.push(CostsReport {
                 test_name,
@@ -1543,7 +1515,7 @@ mod tests {
             epoch: StacksEpochId::Epoch2_05,
         };
 
-        let result = session.deploy_contract(&contract, None, false, None, None);
+        let result = session.deploy_contract(&contract, None, false, None);
         assert!(result.is_err(), "Expected error for clarity mismatch");
     }
 
@@ -1561,7 +1533,7 @@ mod tests {
             .clarity_version(ClarityVersion::Clarity2)
             .build();
 
-        let result = session.deploy_contract(&contract, None, false, None, None);
+        let result = session.deploy_contract(&contract, None, false, None);
         assert!(result.is_err());
         let err = result.unwrap_err();
         assert!(err.len() == 1);
@@ -1601,7 +1573,7 @@ mod tests {
             epoch: StacksEpochId::Epoch25,
         };
 
-        let _ = session.deploy_contract(&contract, None, false, None, None);
+        let _ = session.deploy_contract(&contract, None, false, None);
 
         // assert data-var is set to 0
         assert_eq!(
@@ -1659,7 +1631,7 @@ mod tests {
 
         // deploy default contract
         let contract = ClarityContractBuilder::default().build();
-        let result = session.deploy_contract(&contract, None, false, None, None);
+        let result = session.deploy_contract(&contract, None, false, None);
         assert!(result.is_ok());
     }
 
@@ -1710,7 +1682,7 @@ mod tests {
 
         // deploy default contract
         let contract = ClarityContractBuilder::default().build();
-        let _ = session.deploy_contract(&contract, None, false, None, None);
+        let _ = session.deploy_contract(&contract, None, false, None);
 
         dbg!(&contract);