fix(solc): support remapping autodetection edge case (#888)

* fix(solc): support remapping autodetection edge case

* Update ethers-solc/src/remappings.rs

Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

ethers-solc/src/project_util.rs

@@ -14,6 +14,9 @@ use std::{
 };
 use tempfile::TempDir;
 
+/// A [`Project`] wrapper that lives in a new temporary directory
+///
+/// Once `TempProject` is dropped, the temp dir is automatically removed, see [`TempDir::drop()`]
 pub struct TempProject<T: ArtifactOutput = MinimalCombinedArtifacts> {
     /// temporary workspace root
     _root: TempDir,
@@ -208,6 +211,7 @@ impl<T: ArtifactOutput> AsRef<Project<T>> for TempProject<T> {
     }
 }
 
+/// commonly used options for copying entire folders
 fn dir_copy_options() -> dir::CopyOptions {
     dir::CopyOptions {
         overwrite: true,
@@ -219,6 +223,7 @@ fn dir_copy_options() -> dir::CopyOptions {
     }
 }
 
+/// commonly used options for copying files
 fn file_copy_options() -> file::CopyOptions {
     file::CopyOptions {
         overwrite: true,

ethers-solc/src/remappings.rs

@@ -335,6 +335,14 @@ struct Candidate {
     window_level: usize,
 }
 
+impl Candidate {
+    /// Returns `true` if the source dir of this candidate ends with `/src` or `/contracts`
+    fn is_candidate_source_a_source_dir(&self) -> bool {
+        self.source_dir.ends_with(DAPPTOOLS_CONTRACTS_DIR) ||
+            self.source_dir.ends_with(JS_CONTRACTS_DIR)
+    }
+}
+
 fn is_source_dir(dir: &Path) -> bool {
     dir.file_name()
         .and_then(|p| p.to_str())
@@ -411,6 +419,18 @@ fn find_remapping_candidates(
             .count() >
             1
     {
+        // here we need to check for layouts like
+        // ```test
+        // ds-test/
+        // ├── aux/demo.sol
+        // └── src/test.sol
+        // ```
+        // which effectively ignores the `aux` dir by prioritizing source dirs and keeps
+        // `ds-test/=ds-test/src/`
+        if let Some(pos) = candidates.iter().position(Candidate::is_candidate_source_a_source_dir) {
+            let c = candidates.remove(pos);
+            candidates = vec![c];
+        } else {
             // merge all candidates on the current level if the current dir is itself a candidate or
             // there are multiple nested candidates on the current level like `current/{auth,
             // tokens}/contracts/c.sol`
@@ -420,6 +440,7 @@ fn find_remapping_candidates(
                 source_dir: current_dir.to_path_buf(),
                 window_level: current_level,
             });
+        }
     } else {
         // this handles the case if there is a single nested candidate
         if let Some(candidate) = candidates.iter_mut().find(|c| c.window_level == current_level) {
@@ -429,7 +450,9 @@ fn find_remapping_candidates(
             let distance = dir_distance(&candidate.window_start, &candidate.source_dir);
             if distance > 1 && candidate.source_dir.ends_with(JS_CONTRACTS_DIR) {
                 candidate.source_dir = window_start;
-            } else if !is_source_dir(&candidate.source_dir) {
+            } else if !is_source_dir(&candidate.source_dir) &&
+                candidate.source_dir != candidate.window_start
+            {
                 candidate.source_dir = last_nested_source_dir(open, &candidate.source_dir);
             }
         }
@@ -572,6 +595,67 @@ mod tests {
         assert_eq!(remappings[0].path, format!("{}/src/", path));
     }
 
+    #[test]
+    fn can_resolve_geb_remappings() {
+        let tmp_dir = tempdir("geb").unwrap();
+        let paths = [
+            "lib/ds-token/src/test/Contract.sol",
+            "lib/ds-token/lib/ds-test/src/Contract.sol",
+            "lib/ds-token/lib/ds-test/aux/Contract.sol",
+            "lib/ds-token/lib/ds-stop/lib/ds-test/src/Contract.sol",
+            "lib/ds-token/lib/ds-stop/lib/ds-note/src/Contract.sol",
+            "lib/ds-token/lib/ds-math/lib/ds-test/aux/Contract.sol",
+            "lib/ds-token/lib/ds-math/src/Contract.sol",
+            "lib/ds-token/lib/ds-stop/lib/ds-test/aux/Contract.sol",
+            "lib/ds-token/lib/ds-stop/lib/ds-note/lib/ds-test/src/Contract.sol",
+            "lib/ds-token/lib/ds-math/lib/ds-test/src/Contract.sol",
+            "lib/ds-token/lib/ds-stop/lib/ds-auth/lib/ds-test/src/Contract.sol",
+            "lib/ds-token/lib/ds-stop/src/Contract.sol",
+            "lib/ds-token/src/Contract.sol",
+            "lib/ds-token/lib/erc20/src/Contract.sol",
+            "lib/ds-token/lib/ds-stop/lib/ds-auth/lib/ds-test/aux/Contract.sol",
+            "lib/ds-token/lib/ds-stop/lib/ds-auth/src/Contract.sol",
+            "lib/ds-token/lib/ds-stop/lib/ds-note/lib/ds-test/aux/Contract.sol",
+        ];
+        mkdir_or_touch(tmp_dir.path(), &paths[..]);
+
+        let tmp_dir_path = tmp_dir.path().join("lib");
+        let mut remappings = Remapping::find_many(&tmp_dir_path);
+        remappings.sort_unstable();
+        let mut expected = vec![
+            Remapping {
+                name: "ds-auth/".to_string(),
+                path: to_str(tmp_dir_path.join("ds-token/lib/ds-stop/lib/ds-auth/src")),
+            },
+            Remapping {
+                name: "ds-math/".to_string(),
+                path: to_str(tmp_dir_path.join("ds-token/lib/ds-math/src")),
+            },
+            Remapping {
+                name: "ds-note/".to_string(),
+                path: to_str(tmp_dir_path.join("ds-token/lib/ds-stop/lib/ds-note/src")),
+            },
+            Remapping {
+                name: "ds-stop/".to_string(),
+                path: to_str(tmp_dir_path.join("ds-token/lib/ds-stop/src")),
+            },
+            Remapping {
+                name: "ds-test/".to_string(),
+                path: to_str(tmp_dir_path.join("ds-token/lib/ds-test/src")),
+            },
+            Remapping {
+                name: "ds-token/".to_string(),
+                path: to_str(tmp_dir_path.join("ds-token/src")),
+            },
+            Remapping {
+                name: "erc20/".to_string(),
+                path: to_str(tmp_dir_path.join("ds-token/lib/erc20/src")),
+            },
+        ];
+        expected.sort_unstable();
+        pretty_assertions::assert_eq!(remappings, expected);
+    }
+
     #[test]
     fn can_resolve_oz_remappings() {
         let tmp_dir = tempdir("node_modules").unwrap();

ethers-solc/src/utils.rs

@@ -1,6 +1,9 @@
 //! Utility functions
 
-use std::path::{Component, Path, PathBuf};
+use std::{
+    collections::HashSet,
+    path::{Component, Path, PathBuf},
+};
 
 use crate::{error::SolcError, SolcIoError};
 use once_cell::sync::Lazy;
@@ -66,6 +69,46 @@ pub fn source_files(root: impl AsRef<Path>) -> Vec<PathBuf> {
         .collect()
 }
 
+/// Returns a list of _unique_ paths to all folders under `root` that contain at least one solidity
+/// file (`*.sol`).
+///
+/// # Example
+///
+/// ```no_run
+/// use ethers_solc::utils;
+/// let dirs = utils::solidity_dirs("./lib");
+/// ```
+///
+/// for following layout will return
+/// `["lib/ds-token/src", "lib/ds-token/src/test", "lib/ds-token/lib/ds-math/src", ...]`
+///
+/// ```text
+/// lib
+/// └── ds-token
+///     ├── lib
+///     │   ├── ds-math
+///     │   │   └── src/Contract.sol
+///     │   ├── ds-stop
+///     │   │   └── src/Contract.sol
+///     │   ├── ds-test
+///     │       └── src//Contract.sol
+///     └── src
+///         ├── base.sol
+///         ├── test
+///         │   ├── base.t.sol
+///         └── token.sol
+/// ```
+pub fn solidity_dirs(root: impl AsRef<Path>) -> Vec<PathBuf> {
+    let sources = source_files(root);
+    sources
+        .iter()
+        .filter_map(|p| p.parent())
+        .collect::<HashSet<_>>()
+        .into_iter()
+        .map(|p| p.to_path_buf())
+        .collect()
+}
+
 /// Returns the source name for the given source path, the ancestors of the root path
 /// `/Users/project/sources/contract.sol` -> `sources/contracts.sol`
 pub fn source_name(source: &Path, root: impl AsRef<Path>) -> &Path {