Skip to content

Commit

Permalink
perf: improve resolve nested foundry.toml remapping (#1495)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattsse committed May 4, 2022
1 parent 88b42bc commit 8478c08
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 14 deletions.
24 changes: 22 additions & 2 deletions cli/tests/it/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ forgetest_init!(can_detect_lib_foundry_toml, |prj: TestProject, mut cmd: TestCom
vec![
"ds-test/=lib/forge-std/lib/ds-test/src/".parse().unwrap(),
"forge-std/=lib/forge-std/src/".parse().unwrap(),
"nested-lib/=lib/nested-lib/src/".parse().unwrap(),
"nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
"src/=src/".parse().unwrap(),
]
Expand All @@ -400,19 +401,38 @@ forgetest_init!(can_detect_lib_foundry_toml, |prj: TestProject, mut cmd: TestCom
// nest another lib under the already nested lib
let mut config = config.clone();
config.remappings = vec![Remapping::from_str("nested-twice/=lib/nested-twice").unwrap().into()];
let nested = nested.join("lib/nested-lib");
let nested = nested.join("lib/another-lib");
pretty_err(&nested, fs::create_dir_all(&nested));
let toml_file = nested.join("foundry.toml");
pretty_err(&toml_file, fs::write(&toml_file, config.to_string_pretty().unwrap()));

let another_config = cmd.config();
let remappings = another_config.get_all_remappings();
pretty_assertions::assert_eq!(
remappings,
vec![
"another-lib/=lib/nested-lib/lib/another-lib/src/".parse().unwrap(),
"ds-test/=lib/forge-std/lib/ds-test/src/".parse().unwrap(),
"forge-std/=lib/forge-std/src/".parse().unwrap(),
"nested-lib/=lib/nested-lib/src/".parse().unwrap(),
"nested-twice/=lib/nested-lib/lib/another-lib/lib/nested-twice/".parse().unwrap(),
"nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
"src/=src/".parse().unwrap(),
]
);

config.src = "custom-source-dir".into();
pretty_err(&toml_file, fs::write(&toml_file, config.to_string_pretty().unwrap()));
let config = cmd.config();
let remappings = config.get_all_remappings();
pretty_assertions::assert_eq!(
remappings,
vec![
"another-lib/=lib/nested-lib/lib/another-lib/custom-source-dir/".parse().unwrap(),
"ds-test/=lib/forge-std/lib/ds-test/src/".parse().unwrap(),
"forge-std/=lib/forge-std/src/".parse().unwrap(),
"nested-twice/=lib/nested-lib/lib/nested-lib/lib/nested-twice/".parse().unwrap(),
"nested-lib/=lib/nested-lib/src/".parse().unwrap(),
"nested-twice/=lib/nested-lib/lib/another-lib/lib/nested-twice/".parse().unwrap(),
"nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
"src/=src/".parse().unwrap(),
]
Expand Down
62 changes: 50 additions & 12 deletions config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,10 +547,8 @@ impl Config {

/// Returns all configured [`Remappings`]
///
/// **Note:** this will add an additional `<src>/=<src path>` remapping here so imports that
/// look like `import {Foo} from "src/Foo.sol";` are properly resolved.
///
/// This is due the fact that `solc`'s VFS resolves [direct imports](https://docs.soliditylang.org/en/develop/path-resolution.html#direct-imports) that start with the source directory's name.
/// **Note:** this will add an additional `<src>/=<src path>` remapping here, see
/// [Self::get_source_dir_remapping()]
///
/// So that
///
Expand All @@ -566,7 +564,20 @@ impl Config {
/// contracts/math/math.sol
/// ```
pub fn get_all_remappings(&self) -> Vec<Remapping> {
let mut remappings: Vec<_> = self.remappings.iter().map(|m| m.clone().into()).collect();
self.remappings
.iter()
.map(|m| m.clone().into())
.chain(self.get_source_dir_remapping())
.collect()
}

/// Returns the remapping for the project's _src_ directory
///
/// **Note:** this will add an additional `<src>/=<src path>` remapping here so imports that
/// look like `import {Foo} from "src/Foo.sol";` are properly resolved.
///
/// This is due the fact that `solc`'s VFS resolves [direct imports](https://docs.soliditylang.org/en/develop/path-resolution.html#direct-imports) that start with the source directory's name.
pub fn get_source_dir_remapping(&self) -> Option<Remapping> {
if let Some(src_dir_name) =
self.src.file_name().and_then(|s| s.to_str()).filter(|s| !s.is_empty())
{
Expand All @@ -577,9 +588,10 @@ impl Config {
if !src_remapping.path.ends_with('/') {
src_remapping.path.push('/')
}
remappings.push(src_remapping);
Some(src_remapping)
} else {
None
}
remappings
}

/// Returns the `Optimizer` based on the configured settings
Expand Down Expand Up @@ -1505,6 +1517,9 @@ impl<'a> RemappingsProvider<'a> {

new_remappings.extend(remappings);

// merge all remappings of libs look up lib's foundry.toml
new_remappings.extend(self.lib_foundry_toml_remappings());

// look up lib paths
new_remappings.extend(
self.lib_paths
Expand All @@ -1514,9 +1529,6 @@ impl<'a> RemappingsProvider<'a> {
.collect::<Vec<Remapping>>(),
);

// merge all remappings of libs look up lib's foundry.toml
new_remappings.extend(self.lib_foundry_toml_remappings());

// remove duplicates
new_remappings.sort_by(|a, b| a.name.cmp(&b.name));
new_remappings.dedup_by(|a, b| a.name.eq(&b.name));
Expand All @@ -1527,10 +1539,36 @@ impl<'a> RemappingsProvider<'a> {
/// Returns all remappings declared in foundry.toml files of libraries
fn lib_foundry_toml_remappings(&self) -> impl Iterator<Item = Remapping> + '_ {
self.lib_paths.iter().map(|p| self.root.join(p)).flat_map(foundry_toml_dirs).flat_map(
|lib| {
|lib: PathBuf| {
// load config, of the nested lib if it exists
let config = Config::load_with_root(&lib).sanitized();
config.get_all_remappings().into_iter().filter(|r| r.name != "src/")

// if the configured _src_ directory is set to something that
// [Remapping::find_many()] doesn't classify as a src directory (src, contracts,
// lib), then we need to manually add a remapping here
let mut src_remapping = None;
if ![Path::new("src"), Path::new("contracts"), Path::new("lib")]
.contains(&config.src.as_path())
{
if let Some(name) = lib.file_name().and_then(|s| s.to_str()) {
let mut r = Remapping {
name: format!("{}/", name),
path: format!("{}", lib.join(&config.src).display()),
};
if !r.path.ends_with('/') {
r.path.push('/')
}
src_remapping = Some(r);
}
}

let mut remappings =
config.remappings.into_iter().map(|m| m.into()).collect::<Vec<Remapping>>();

if let Some(r) = src_remapping {
remappings.push(r);
}
remappings
},
)
}
Expand Down

0 comments on commit 8478c08

Please sign in to comment.