From 9096f4e170f9dc479df1a28d8d0402501f848958 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sun, 19 Dec 2021 05:32:14 +0100 Subject: [PATCH] feat(solc): revamped remapping auto detect (#706) --- ethers-solc/src/compile.rs | 1 + ethers-solc/src/remappings.rs | 209 ++++++++++++++++++++++++++++++++-- 2 files changed, 201 insertions(+), 9 deletions(-) diff --git a/ethers-solc/src/compile.rs b/ethers-solc/src/compile.rs index fb1d30b4..b2f27ab0 100644 --- a/ethers-solc/src/compile.rs +++ b/ethers-solc/src/compile.rs @@ -42,6 +42,7 @@ use once_cell::sync::Lazy; use std::sync::Mutex; #[cfg(any(test, feature = "tests"))] +#[allow(unused)] static LOCK: Lazy> = Lazy::new(|| Mutex::new(())); #[cfg(all(feature = "svm", feature = "async"))] diff --git a/ethers-solc/src/remappings.rs b/ethers-solc/src/remappings.rs index a7d197aa..3977e7d1 100644 --- a/ethers-solc/src/remappings.rs +++ b/ethers-solc/src/remappings.rs @@ -1,6 +1,11 @@ use crate::{error::SolcError, Result}; use serde::{Deserialize, Serialize}; -use std::{fmt, str::FromStr}; +use std::{ + collections::{hash_map::Entry, HashMap}, + fmt, + path::{Path, PathBuf}, + str::FromStr, +}; const DAPPTOOLS_CONTRACTS_DIR: &str = "src"; const JS_CONTRACTS_DIR: &str = "contracts"; @@ -54,16 +59,14 @@ impl FromStr for Remapping { type Err = RemappingError; fn from_str(remapping: &str) -> std::result::Result { - let mut split = remapping.split('='); - let name = split.next().ok_or(RemappingError::NoPrefix)?.to_string(); - if name.is_empty() { + let (name, path) = remapping.split_once('=').ok_or(RemappingError::NoPrefix)?; + if name.trim().is_empty() { return Err(RemappingError::NoPrefix) } - let path = split.next().ok_or(RemappingError::NoTarget)?.to_string(); - if path.is_empty() { + if path.trim().is_empty() { return Err(RemappingError::NoTarget) } - Ok(Remapping { name, path }) + Ok(Remapping { name: name.to_string(), path: path.to_string() }) } } @@ -176,6 +179,162 @@ impl Remapping { Ok(remappings) } + + /// Attempts to autodetect all remappings given a certain root path. + /// + /// This will recursively scan all subdirectories of the root path, if a subdirectory contains a + /// solidity file then this a candidate for a remapping. The name of the remapping will be the + /// folder name. + /// + /// However, there are additional rules/assumptions when it comes to determining if a candidate + /// should in fact be a remapping: + /// + /// All names and paths end with a trailing "/" + /// + /// The name of the remapping will be the parent folder of a solidity file, unless the folder is + /// named `src`, `lib` or `contracts` in which case the name of the remapping will be the parent + /// folder's name of `src`, `lib`, `contracts`: The remapping of `repo1/src/contract.sol` is + /// `name: "repo1/", path: "repo1/src/"` + /// + /// Nested remappings need to be separated by `src`, `lib` or `contracts`, The remapping of + /// `repo1/lib/ds-math/src/contract.sol` is `name: "ds-match/", "repo1/lib/ds-math/src/"` + /// + /// Remapping detection is primarily designed for dapptool's rules for lib folders, however, we + /// attempt to detect and optimize various folder structures commonly used in `node_modules` + /// dependencies. For those the same rules apply. In addition, we try to unify all + /// remappings discovered according to the rules mentioned above, so that layouts like, + // @aave/ + // ├─ governance/ + // │ ├─ contracts/ + // ├─ protocol-v2/ + // │ ├─ contracts/ + /// + /// which would be multiple rededications according to our rules ("governance", "protocol-v2"), are unified into `@aave` by looking at their common ancestor, the root of this subdirectory (`@aavee`) + pub fn find_all(root: impl AsRef) -> Vec { + /// prioritize ("a", "1/2") over ("a", "1/2/3") + fn insert_higher_path(mappings: &mut HashMap, key: String, path: PathBuf) { + match mappings.entry(key) { + Entry::Occupied(mut e) => { + if e.get().components().count() > path.components().count() { + e.insert(path); + } + } + Entry::Vacant(e) => { + e.insert(path); + } + } + } + + let mut remappings = HashMap::new(); + // iterate over all dirs that are children of the root + for dir in walkdir::WalkDir::new(root) + .follow_links(true) + .min_depth(1) + .max_depth(1) + .into_iter() + .filter_map(std::result::Result::ok) + .filter(|e| e.file_type().is_dir()) + { + let dir_path = dir.path(); + + // in node_modules there could be following pattern: + // @aave/ + // ├─ governance/ + // │ ├─ contracts/ + // ├─ protocol-v2/ + // │ ├─ contracts/ + // in such cases, the desired remapping name would be `@aave`, but at this point we + // would have detected `governance` and `protocol-v2` as remapping names. so + // now we need to unify them by checking if they share the `dir_path` as + // common ancestor. We make the assumption here, that we can only unify + // remappings if their direct parent dir is the root, so `@aave/lib` or + // `@aave/src` is not mergeable as that would violate dapptools style lib paths and + // remappings. This means we can only unify/simplify them if there is no! `src` or `lib` + // path between the remappings' path and the dir_path + if let Some(root_name) = dir_path.file_name().and_then(|f| f.to_str()) { + let mut simplified = false; + // check all remappings in this depth 1 folder + 'outer: for (name, path) in scan_children(dir_path) { + let mut p = path.as_path(); + let mut first_parent = true; + while let Some(parent) = p.parent() { + if parent == dir_path { + if !simplified { + // handle trailing src,lib,contracts dir in cases like + // `dir_path/@ens/contracts` + let path = if first_parent { path } else { dir_path.to_path_buf() }; + insert_higher_path( + &mut remappings, + format!("{}/", root_name), + path, + ); + simplified = true; + } + continue 'outer + } + if parent.ends_with("src") || parent.ends_with("lib") { + // end early + insert_higher_path(&mut remappings, name, path); + continue 'outer + } + first_parent = false; + p = parent; + } + } + } + } + remappings + .into_iter() + .map(|(name, path)| Remapping { name, path: format!("{}/", path.display()) }) + .collect() + } +} + +/// Recursively scans sub folders and checks if they contain a solidity file +fn scan_children(root: &Path) -> HashMap { + // this is a marker if the current root is already a remapping + let mut remapping = false; + + // all found remappings + let mut remappings = HashMap::new(); + + for entry in walkdir::WalkDir::new(&root) + .min_depth(1) + .max_depth(1) + .into_iter() + .filter_map(std::result::Result::ok) + { + let entry: walkdir::DirEntry = entry; + + if entry.file_type().is_file() && !remapping { + if entry.file_name().to_str().filter(|f| f.ends_with(".sol")).is_some() { + // found a solidity file + + // this will hold the actual root remapping if root is named `src` or `lib` + let actual_parent = root.parent().filter(|_| { + root.ends_with("src") || root.ends_with("lib") || root.ends_with("contracts") + }); + + let parent = actual_parent.unwrap_or(root); + if let Some(name) = parent.file_name().and_then(|f| f.to_str()) { + remappings.insert(format!("{}/", name), root.to_path_buf()); + remapping = true; + } + } + } else if entry.file_type().is_dir() { + let path = entry.path(); + // we skip `tests` and nested `node_modules` + if !path.ends_with("tests") || !path.ends_with("node_modules") { + for (name, path) in scan_children(path) { + if let Entry::Vacant(e) = remappings.entry(name) { + e.insert(path); + } + } + } + } + } + + remappings } #[cfg(test)] @@ -254,7 +413,7 @@ mod tests { mkdir_or_touch(tmp_dir_path, &paths[..]); let path = tmp_dir_path.display().to_string(); - let mut remappings = Remapping::find_many(&path).unwrap(); + let mut remappings = Remapping::find_all(&path); remappings.sort_unstable(); let mut expected = vec![ @@ -302,7 +461,7 @@ mod tests { touch(&contract2).unwrap(); let path = tmp_dir.path().display().to_string(); - let mut remappings = Remapping::find_many(&path).unwrap(); + let mut remappings = Remapping::find_all(&path); remappings.sort_unstable(); let mut expected = vec![ Remapping { @@ -317,4 +476,36 @@ mod tests { expected.sort_unstable(); assert_eq!(remappings, expected); } + + #[test] + fn hardhat_remappings() { + let tmp_dir = tempdir::TempDir::new("node_modules").unwrap(); + let tmp_dir_node_modules = tmp_dir.path().join("node_modules"); + let paths = [ + "node_modules/@aave/aave-token/contracts/token/", + "node_modules/@aave/aave-token/contracts/token/AaveToken.sol", + "node_modules/@aave/governance-v2/contracts/governance/", + "node_modules/@aave/governance-v2/contracts/governance/Executor.sol", + "node_modules/@aave/protocol-v2/contracts/protocol/lendingpool/", + "node_modules/@aave/protocol-v2/contracts/protocol/lendingpool/LendingPool.sol", + "node_modules/@ensdomains/ens/contracts/", + "node_modules/@ensdomains/ens/contracts/contract.sol", + ]; + mkdir_or_touch(tmp_dir.path(), &paths[..]); + let mut remappings = Remapping::find_all(&tmp_dir_node_modules); + remappings.sort_unstable(); + let mut expected = vec![ + Remapping { + name: "@aave/".to_string(), + path: to_str(tmp_dir_node_modules.join("@aave")), + }, + Remapping { + name: "@ensdomains/".to_string(), + path: to_str(tmp_dir_node_modules.join("@ensdomains")), + }, + ]; + expected.sort_unstable(); + + assert_eq!(remappings, expected); + } }