diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ad96f05..7137a120 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,8 @@ ### Unreleased +- `ArtifactOutput::write_extras` now takes the `Artifacts` directly + [#1491](https://github.com/gakonst/ethers-rs/pull/1491) - Make `ethers-solc` optional dependency of `ethers`, needs `ethers-solc` feature to activate [#1463](https://github.com/gakonst/ethers-rs/pull/1463) - Add `rawMetadata:String` field to configurable contract output diff --git a/ethers-solc/src/artifact_output/mod.rs b/ethers-solc/src/artifact_output/mod.rs index 05e56922..9ca5dcf3 100644 --- a/ethers-solc/src/artifact_output/mod.rs +++ b/ethers-solc/src/artifact_output/mod.rs @@ -17,13 +17,16 @@ use semver::Version; use serde::{de::DeserializeOwned, Serialize}; use std::{ borrow::Cow, - collections::{btree_map::BTreeMap, HashSet}, + collections::{btree_map::BTreeMap, HashMap, HashSet}, + ffi::OsString, fmt, fs, io, ops::Deref, path::{Path, PathBuf}, }; +use tracing::trace; mod configurable; +use crate::contracts::VersionedContract; pub use configurable::*; /// Represents unique artifact metadata for identifying artifacts on output @@ -100,6 +103,11 @@ impl ArtifactFile { } } +/// Internal helper type alias that maps all files for the contracts +/// `output -> [(file, name, contract)]` +pub(crate) type ArtifactFiles<'a> = + HashMap>; + /// local helper type alias `file name -> (contract name -> Vec<..>)` pub(crate) type ArtifactsMap = FileToContractsMap>>; @@ -193,6 +201,19 @@ impl Artifacts { self.0.values().find_map(|all| all.get(contract_name)) } + /// Returns the `Artifact` with matching file, contract name and version + pub fn find_artifact( + &self, + file: &str, + contract_name: &str, + version: &Version, + ) -> Option<&ArtifactFile> { + self.0 + .get(file) + .and_then(|contracts| contracts.get(contract_name)) + .and_then(|artifacts| artifacts.iter().find(|artifact| artifact.version == *version)) + } + /// Returns true if this type contains an artifact with the given path for the given contract pub fn has_contract_artifact(&self, contract_name: &str, artifact_path: &Path) -> bool { self.get_contract_artifact_files(contract_name) @@ -529,7 +550,7 @@ pub trait ArtifactOutput { artifacts.join_all(&layout.artifacts); artifacts.write_all()?; - self.write_extras(contracts, layout)?; + self.write_extras(contracts, &artifacts)?; Ok(artifacts) } @@ -550,21 +571,16 @@ pub trait ArtifactOutput { fn write_extras( &self, contracts: &VersionedContracts, - layout: &ProjectPathsConfig, + artifacts: &Artifacts, ) -> Result<()> { for (file, contracts) in contracts.as_ref().iter() { for (name, versioned_contracts) in contracts { for c in versioned_contracts { - let artifact_path = if versioned_contracts.len() > 1 { - Self::output_file_versioned(file, name, &c.version) - } else { - Self::output_file(file, name) - }; - - let file = layout.artifacts.join(artifact_path); - utils::create_parent_dir_all(&file)?; - - self.write_contract_extras(&c.contract, &file)?; + if let Some(artifact) = artifacts.find_artifact(file, name, &c.version) { + let file = &artifact.file; + utils::create_parent_dir_all(file)?; + self.write_contract_extras(&c.contract, file)?; + } } } } @@ -585,6 +601,68 @@ pub trait ArtifactOutput { .into() } + /// Returns the appropriate file name for the conflicting file. + /// + /// This should ensure that the resulting `PathBuf` is conflict free, which could be possible if + /// there are two separate contract files (in different folders) that contain the same contract: + /// + /// `src/A.sol::A` + /// `src/nested/A.sol::A` + /// + /// Which would result in the same `PathBuf` if only the file and contract name is taken into + /// account, [`Self::output_file`]. + /// + /// This return a unique output file + fn conflict_free_output_file( + already_taken: &HashSet, + conflict: PathBuf, + contract_file: impl AsRef, + ) -> PathBuf { + let mut candidate = conflict.clone(); + let contract_file = contract_file.as_ref(); + let mut current_parent = contract_file.parent(); + + while let Some(parent) = current_parent.and_then(|f| f.file_name()) { + candidate = Path::new(parent).join(&candidate); + if !already_taken.contains(&candidate) { + trace!("found alternative output file={:?} for {:?}", candidate, contract_file); + return candidate + } + current_parent = current_parent.and_then(|f| f.parent()); + } + + // this means we haven't found an alternative yet, which shouldn't actually happen since + // `contract_file` are unique, but just to be safe, handle this case in which case + // we simply numerate + + trace!("no conflict free output file found after traversing the file"); + + let mut num = 1; + + loop { + // this will attempt to find an alternate path by numerating the first component in the + // path: `+_/....sol` + let mut components = conflict.components(); + let first = components.next().expect("path not empty"); + let name = first.as_os_str(); + let mut numerated = OsString::with_capacity(name.len() + 2); + numerated.push(name); + numerated.push("_"); + numerated.push(num.to_string()); + + let candidate: PathBuf = Some(numerated.as_os_str()) + .into_iter() + .chain(components.map(|c| c.as_os_str())) + .collect(); + if !already_taken.contains(&candidate) { + trace!("found alternative output file={:?} for {:?}", candidate, contract_file); + return candidate + } + + num += 1; + } + } + /// Returns the path to the contract's artifact location based on the contract's file and name /// /// This returns `contract.sol/contract.json` by default @@ -691,46 +769,68 @@ pub trait ArtifactOutput { // this tracks all the `SourceFile`s that we successfully mapped to a contract let mut non_standalone_sources = HashSet::new(); - // loop over all files and their contracts - for (file, contracts) in contracts.as_ref().iter() { - let mut entries = BTreeMap::new(); + // this holds all output files and the contract(s) it belongs to + let artifact_files = contracts.artifact_files::(); - // loop over all contracts and their versions - for (name, versioned_contracts) in contracts { - let mut contracts = Vec::with_capacity(versioned_contracts.len()); - // check if the same contract compiled with multiple solc versions - for contract in versioned_contracts { - let source_file = sources.find_file_and_version(file, &contract.version); + // this tracks the final artifacts, which we use as lookup for checking conflicts when + // converting stand-alone artifacts in the next step + let mut final_artifact_paths = HashSet::new(); - if let Some(source) = source_file { - non_standalone_sources.insert((source.id, &contract.version)); - } + for (artifact_path, contracts) in artifact_files { + for (idx, (file, name, contract)) in contracts.iter().enumerate() { + // track `SourceFile`s that can be mapped to contracts + let source_file = sources.find_file_and_version(file, &contract.version); - let artifact_path = if versioned_contracts.len() > 1 { - Self::output_file_versioned(file, name, &contract.version) - } else { - Self::output_file(file, name) - }; - - let artifact = self.contract_to_artifact( - file, - name, - contract.contract.clone(), - source_file, - ); - - contracts.push(ArtifactFile { - artifact, - file: artifact_path, - version: contract.version.clone(), - }); + if let Some(source) = source_file { + non_standalone_sources.insert((source.id, &contract.version)); } - entries.insert(name.to_string(), contracts); + + let mut artifact_path = artifact_path.clone(); + + if contracts.len() > 1 { + // naming conflict where the `artifact_path` belongs to two conflicting + // contracts need to adjust the paths properly + + // we keep the top most conflicting file unchanged + let is_top_most = contracts.iter().enumerate().filter(|(i, _)| *i != idx).all( + |(_, (f, _, _))| { + Path::new(file).components().count() < Path::new(f).components().count() + }, + ); + if !is_top_most { + // we resolve the conflicting by finding a new unique, alternative path + artifact_path = Self::conflict_free_output_file( + &final_artifact_paths, + artifact_path, + file, + ); + } + } + + final_artifact_paths.insert(artifact_path.clone()); + + let artifact = + self.contract_to_artifact(file, name, contract.contract.clone(), source_file); + + let artifact = ArtifactFile { + artifact, + file: artifact_path, + version: contract.version.clone(), + }; + + artifacts + .entry(file.to_string()) + .or_default() + .entry(name.to_string()) + .or_default() + .push(artifact); } - artifacts.insert(file.to_string(), entries); } // extend with standalone source files and convert them to artifacts + // this is unfortunately necessary, so we can "mock" `Artifacts` for solidity files without + // any contract definition, which are not included in the `CompilerOutput` but we want to + // create Artifacts for them regardless for (file, sources) in sources.as_ref().iter() { for source in sources { if !non_standalone_sources.contains(&(source.source_file.id, &source.version)) { @@ -749,12 +849,22 @@ pub trait ArtifactOutput { if let Some(artifact) = self.standalone_source_file_to_artifact(file, source) { - let artifact_path = if sources.len() > 1 { + let mut artifact_path = if sources.len() > 1 { Self::output_file_versioned(file, name, &source.version) } else { Self::output_file(file, name) }; + if final_artifact_paths.contains(&artifact_path) { + // preventing conflict + artifact_path = Self::conflict_free_output_file( + &final_artifact_paths, + artifact_path, + file, + ); + final_artifact_paths.insert(artifact_path.clone()); + } + let entries = artifacts .entry(file.to_string()) .or_default() @@ -893,4 +1003,32 @@ mod tests { assert_artifact::(); assert_artifact::(); } + + #[test] + fn can_find_alternate_paths() { + let mut already_taken = HashSet::new(); + + let file = "v1/tokens/Greeter.sol"; + let conflict = PathBuf::from("Greeter.sol/Greeter.json"); + + let alternative = ConfigurableArtifacts::conflict_free_output_file( + &already_taken, + conflict.clone(), + file, + ); + assert_eq!(alternative, PathBuf::from("tokens/Greeter.sol/Greeter.json")); + + already_taken.insert("tokens/Greeter.sol/Greeter.json".into()); + let alternative = ConfigurableArtifacts::conflict_free_output_file( + &already_taken, + conflict.clone(), + file, + ); + assert_eq!(alternative, PathBuf::from("v1/tokens/Greeter.sol/Greeter.json")); + + already_taken.insert("v1/tokens/Greeter.sol/Greeter.json".into()); + let alternative = + ConfigurableArtifacts::conflict_free_output_file(&already_taken, conflict, file); + assert_eq!(alternative, PathBuf::from("Greeter.sol_1/Greeter.json")); + } } diff --git a/ethers-solc/src/compile/output/contracts.rs b/ethers-solc/src/compile/output/contracts.rs index e81cd3a2..7e17a15a 100644 --- a/ethers-solc/src/compile/output/contracts.rs +++ b/ethers-solc/src/compile/output/contracts.rs @@ -1,6 +1,9 @@ -use crate::artifacts::{ - contract::{CompactContractRef, Contract}, - FileToContractsMap, +use crate::{ + artifacts::{ + contract::{CompactContractRef, Contract}, + FileToContractsMap, + }, + ArtifactFiles, ArtifactOutput, }; use semver::Version; use serde::{Deserialize, Serialize}; @@ -25,6 +28,26 @@ impl VersionedContracts { self.0.keys() } + /// Returns all the artifact files mapped with their contracts + pub(crate) fn artifact_files(&self) -> ArtifactFiles { + let mut output_files = ArtifactFiles::with_capacity(self.len()); + for (file, contracts) in self.iter() { + for (name, versioned_contracts) in contracts { + for contract in versioned_contracts { + let output = if versioned_contracts.len() > 1 { + T::output_file_versioned(file, name, &contract.version) + } else { + T::output_file(file, name) + }; + let contract = (file.as_str(), name.as_str(), contract); + output_files.entry(output).or_default().push(contract); + } + } + } + + output_files + } + /// Finds the _first_ contract with the given name /// /// # Example diff --git a/ethers-solc/src/lib.rs b/ethers-solc/src/lib.rs index 55238466..ee72845b 100644 --- a/ethers-solc/src/lib.rs +++ b/ethers-solc/src/lib.rs @@ -790,9 +790,9 @@ impl ArtifactOutput for Project { fn write_extras( &self, contracts: &VersionedContracts, - layout: &ProjectPathsConfig, + artifacts: &Artifacts, ) -> Result<()> { - self.artifacts_handler().write_extras(contracts, layout) + self.artifacts_handler().write_extras(contracts, artifacts) } fn output_file_name(name: impl AsRef) -> PathBuf { diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs index bb849a7f..ca1f6a9c 100644 --- a/ethers-solc/tests/project.rs +++ b/ethers-solc/tests/project.rs @@ -1,12 +1,5 @@ //! project tests -use std::{ - collections::{BTreeMap, HashMap, HashSet}, - fs, io, - path::{Path, PathBuf}, - str::FromStr, -}; - use ethers_core::types::Address; use ethers_solc::{ artifacts::{ @@ -23,6 +16,12 @@ use ethers_solc::{ }; use pretty_assertions::assert_eq; use semver::Version; +use std::{ + collections::{BTreeMap, HashMap, HashSet}, + fs, io, + path::{Path, PathBuf}, + str::FromStr, +}; #[allow(unused)] fn init_tracing() { @@ -2065,3 +2064,67 @@ contract C { } let compiled = project.compile().unwrap(); assert!(compiled.has_compiler_errors()); } + +#[test] +fn can_handle_conflicting_files() { + let project = TempProject::::dapptools().unwrap(); + + project + .add_source( + "Greeter", + r#" + pragma solidity ^0.8.10; + + contract Greeter {} + "#, + ) + .unwrap(); + + project + .add_source( + "tokens/Greeter", + r#" + pragma solidity ^0.8.10; + + contract Greeter {} + "#, + ) + .unwrap(); + + let compiled = project.compile().unwrap(); + assert!(!compiled.has_compiler_errors()); + + let artifacts = compiled.artifacts().count(); + assert_eq!(artifacts, 2); + + // nothing to compile + let compiled = project.compile().unwrap(); + assert!(compiled.is_unchanged()); + let artifacts = compiled.artifacts().count(); + assert_eq!(artifacts, 2); + + let cache = SolFilesCache::read(project.cache_path()).unwrap(); + + let mut source_files = cache.files.keys().cloned().collect::>(); + source_files.sort_unstable(); + + assert_eq!( + source_files, + vec![PathBuf::from("src/Greeter.sol"), PathBuf::from("src/tokens/Greeter.sol"),] + ); + + let mut artifacts = project.artifacts_snapshot().unwrap().artifacts; + artifacts.strip_prefix_all(&project.paths().artifacts); + + assert_eq!(artifacts.len(), 2); + let mut artifact_files = artifacts.artifact_files().map(|f| f.file.clone()).collect::>(); + artifact_files.sort_unstable(); + + assert_eq!( + artifact_files, + vec![ + PathBuf::from("Greeter.sol/Greeter.json"), + PathBuf::from("tokens/Greeter.sol/Greeter.json"), + ] + ); +}