From 8ade5577ebd1d01cef0474b7deb36bc65da4b68d Mon Sep 17 00:00:00 2001
From: Agost Biro <agostbiro@gmail.com>
Date: Thu, 1 Aug 2024 10:45:14 +0200
Subject: [PATCH] Address code review feedback

---
 crates/edr_napi/src/solidity_tests.rs  | 10 +++++-----
 crates/foundry/common/src/contracts.rs | 11 ++++++-----
 crates/foundry/forge/src/lib.rs        |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/crates/edr_napi/src/solidity_tests.rs b/crates/edr_napi/src/solidity_tests.rs
index fe5bab211..0f395fd97 100644
--- a/crates/edr_napi/src/solidity_tests.rs
+++ b/crates/edr_napi/src/solidity_tests.rs
@@ -3,11 +3,11 @@ mod config;
 mod runner;
 mod test_results;
 
-use std::{path::Path, sync::Arc};
+use std::{collections::BTreeMap, path::Path, sync::Arc};
 
 use artifact::Artifact;
 use forge::TestFilter;
-use foundry_common::ContractsByArtifact;
+use foundry_common::{ContractData, ContractsByArtifact};
 use napi::{
     threadsafe_function::{
         ErrorStrategy, ThreadSafeCallContext, ThreadsafeFunction, ThreadsafeFunctionCallMode,
@@ -44,11 +44,11 @@ pub fn run_solidity_tests(
             |ctx: ThreadSafeCallContext<SuiteResult>| Ok(vec![ctx.value]),
         )?;
 
-    let artifacts = artifacts
+    let known_contracts: ContractsByArtifact = artifacts
         .into_iter()
         .map(|item| Ok((item.id.try_into()?, item.contract.try_into()?)))
-        .collect::<Result<_, napi::Error>>()?;
-    let known_contracts = ContractsByArtifact::new(artifacts);
+        .collect::<Result<BTreeMap<foundry_common::ArtifactId, ContractData>, napi::Error>>()?
+        .into();
 
     let test_suites = test_suites
         .into_iter()
diff --git a/crates/foundry/common/src/contracts.rs b/crates/foundry/common/src/contracts.rs
index 74cfbc90c..fa82da23d 100644
--- a/crates/foundry/common/src/contracts.rs
+++ b/crates/foundry/common/src/contracts.rs
@@ -65,11 +65,6 @@ type ArtifactWithContractRef<'a> = (&'a ArtifactId, &'a ContractData);
 pub struct ContractsByArtifact(BTreeMap<ArtifactId, ContractData>);
 
 impl ContractsByArtifact {
-    /// Creates a new instance with the given map.
-    pub fn new(values: BTreeMap<ArtifactId, ContractData>) -> Self {
-        Self(values)
-    }
-
     /// Creates a new instance by collecting all artifacts with present bytecode
     /// from an iterator.
     ///
@@ -164,6 +159,12 @@ impl ContractsByArtifact {
     }
 }
 
+impl From<BTreeMap<ArtifactId, ContractData>> for ContractsByArtifact {
+    fn from(value: BTreeMap<ArtifactId, ContractData>) -> Self {
+        Self(value)
+    }
+}
+
 /// Wrapper type that maps an address to a contract identifier and contract ABI.
 pub type ContractsByAddress = BTreeMap<Address, (String, JsonAbi)>;
 
diff --git a/crates/foundry/forge/src/lib.rs b/crates/foundry/forge/src/lib.rs
index 52542e11f..b910f2d21 100644
--- a/crates/foundry/forge/src/lib.rs
+++ b/crates/foundry/forge/src/lib.rs
@@ -96,7 +96,7 @@ impl TestOptions {
         })
     }
 
-    /// Create a new test options configuration for Hardhat.
+    /// Creates a new test options configuration for Hardhat.
     /// As opposed to Foundry, Hardhat doesn't support inline configuration.
     pub fn new_hardhat(
         base_fuzz: FuzzConfig,