Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

Commit

Permalink
fix(solc): mimic hardhat import resolver when in node_modules (#928)
Browse files Browse the repository at this point in the history
* fix: treat node_modules differently

* test: update hardhat test

* chore(clippy): make clippy happy
  • Loading branch information
mattsse authored Feb 18, 2022
1 parent f2796cc commit 5b2c1fa
Showing 1 changed file with 78 additions and 16 deletions.
94 changes: 78 additions & 16 deletions ethers-solc/src/remappings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl Remapping {
/// 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
/// (`@aave`)
pub fn find_many(root: impl AsRef<Path>) -> Vec<Remapping> {
pub fn find_many(dir: impl AsRef<Path>) -> Vec<Remapping> {
/// prioritize
/// - ("a", "1/2") over ("a", "1/2/3")
/// - if a path ends with `src`
Expand All @@ -163,8 +163,10 @@ impl Remapping {
// all combined remappings from all subdirs
let mut all_remappings = HashMap::new();

let dir = dir.as_ref();
let is_inside_node_modules = dir.ends_with("node_modules");
// iterate over all dirs that are children of the root
for dir in walkdir::WalkDir::new(root)
for dir in walkdir::WalkDir::new(dir)
.follow_links(true)
.min_depth(1)
.max_depth(1)
Expand All @@ -173,9 +175,9 @@ impl Remapping {
.filter(|e| e.file_type().is_dir())
{
let depth1_dir = dir.path();

// check all remappings in this depth 1 folder
let candidates = find_remapping_candidates(depth1_dir, depth1_dir, 0);
let candidates =
find_remapping_candidates(depth1_dir, depth1_dir, 0, is_inside_node_modules);

for candidate in candidates {
if let Some(name) = candidate.window_start.file_name().and_then(|s| s.to_str()) {
Expand Down Expand Up @@ -359,13 +361,55 @@ impl Candidate {
///
/// Which should be resolved to the top level dir `@openzeppelin`
///
/// We also treat candidates with a `node_modules` parent directory differently and consider
/// them to `hardhat` style. In which case the trailing library barrier `contracts` will be
/// stripped from the remapping path. This differs from dapptools style which does not include
/// the library barrier path `src` in the solidity import statements. For example, for
/// dapptools you could have
///
/// ```text
/// <root>/lib/<library>
/// ├── src
/// ├── A.sol
/// ├── B.sol
/// ```
///
/// with remapping `library/=library/src/`
///
/// whereas with hardhat's import resolver the import statement
///
/// ```text
/// <root>/node_modules/<library>
/// ├── contracts
/// ├── A.sol
/// ├── B.sol
/// ```
/// with the simple remapping `library/=library/` because hardhat's lib resolver essentially
/// joins the import path inside a solidity file with the `nodes_modules` folder when it tries
/// to find an imported solidity file. For example
///
/// ```solidity
/// import "hardhat/console.sol";
/// ```
/// expects the file to be at: `<root>/node_modules/hardhat/console.sol`.
///
/// In order to support these cases, we treat the Dapptools case as the outlier, in which case
/// we only keep the candidate that ends with `src`
///
/// - `candidates`: list of viable remapping candidates
/// - `current_dir`: the directory that's currently processed, like `@openzeppelin/contracts`
/// - `current_level`: the number of nested library dirs encountered
/// - `window_start`: This contains the root directory of the current window. In other words
/// this will be the parent directory of the most recent library barrier, which will be
/// `@openzeppelin` if the `current_dir` is `@openzeppelin/contracts` See also
/// [`next_nested_window()`]
/// - `is_inside_node_modules` whether we're inside a `node_modules` lib
fn merge_on_same_level(
candidates: &mut Vec<Candidate>,
current_dir: &Path,
current_level: usize,
window_start: PathBuf,
is_inside_node_modules: bool,
) {
if let Some(pos) =
candidates.iter().position(|c| c.source_dir.ends_with(DAPPTOOLS_CONTRACTS_DIR))
Expand All @@ -377,11 +421,14 @@ impl Candidate {
// there are multiple nested candidates on the current level like `current/{auth,
// tokens}/contracts/c.sol`
candidates.retain(|c| c.window_level != current_level);
candidates.push(Candidate {
window_start,
source_dir: current_dir.to_path_buf(),
window_level: current_level,
});

let source_dir = if is_inside_node_modules {
window_start.clone()
} else {
current_dir.to_path_buf()
};

candidates.push(Candidate { window_start, source_dir, window_level: current_level });
}
}

Expand Down Expand Up @@ -432,6 +479,7 @@ fn find_remapping_candidates(
current_dir: &Path,
open: &Path,
current_level: usize,
is_inside_node_modules: bool,
) -> Vec<Candidate> {
// this is a marker if the current root is a candidate for a remapping
let mut is_candidate = false;
Expand Down Expand Up @@ -470,15 +518,24 @@ fn find_remapping_candidates(

// check if the subdir is a lib barrier, in which case we open a new window
if is_lib_dir(subdir) {
candidates.extend(find_remapping_candidates(subdir, subdir, current_level + 1));
candidates.extend(find_remapping_candidates(
subdir,
subdir,
current_level + 1,
is_inside_node_modules,
));
} else {
// continue scanning with the current window
candidates.extend(find_remapping_candidates(subdir, open, current_level));
candidates.extend(find_remapping_candidates(
subdir,
open,
current_level,
is_inside_node_modules,
));
}
}
}
}

// need to find the actual next window in the event `open` is a lib dir
let window_start = next_nested_window(open, current_dir);
// finally, we need to merge, adjust candidates from the same level and opening window
Expand All @@ -489,7 +546,13 @@ fn find_remapping_candidates(
.count() >
1
{
Candidate::merge_on_same_level(&mut candidates, current_dir, current_level, window_start);
Candidate::merge_on_same_level(
&mut candidates,
current_dir,
current_level,
window_start,
is_inside_node_modules,
);
} 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) {
Expand Down Expand Up @@ -840,7 +903,6 @@ mod tests {
];
mkdir_or_touch(tmp_dir.path(), &paths[..]);
let remappings = Remapping::find_many(&tmp_dir_node_modules);

let mut paths = ProjectPathsConfig::hardhat(tmp_dir.path()).unwrap();
paths.remappings = remappings;

Expand All @@ -850,7 +912,7 @@ mod tests {
assert!(resolved.exists());

// adjust remappings
paths.remappings[0].name = "@openzeppelin/contracts/".to_string();
paths.remappings[0].name = "@openzeppelin/".to_string();

let resolved = paths
.resolve_library_import(Path::new("@openzeppelin/contracts/token/ERC20/IERC20.sol"))
Expand Down Expand Up @@ -1029,7 +1091,7 @@ mod tests {
},
Remapping {
name: "@openzeppelin/".to_string(),
path: to_str(tmp_dir_node_modules.join("@openzeppelin/contracts")),
path: to_str(tmp_dir_node_modules.join("@openzeppelin")),
},
Remapping {
name: "eth-gas-reporter/".to_string(),
Expand Down

0 comments on commit 5b2c1fa

Please sign in to comment.