From 1d482537adb9b8c9e970de70d1a937a93e903479 Mon Sep 17 00:00:00 2001 From: tash-2s <81064017+tash-2s@users.noreply.github.com> Date: Mon, 25 Dec 2023 16:09:34 -0600 Subject: [PATCH 1/5] test: add failing test for Standard JSON Input creation ``` $ cargo test --test project --all-features can_create_standard_json_input_with_external_file Finished test [unoptimized + debuginfo] target(s) in 0.10s Running tests/project.rs (target/debug/deps/project-bc070fde513cf057) running 1 test test can_create_standard_json_input_with_external_file ... FAILED failures: ---- can_create_standard_json_input_with_external_file stdout ---- thread 'can_create_standard_json_input_with_external_file' panicked at tests/project.rs:1649:5: assertion failed: `(left == right)` Diff < left / right > : [ "src/Counter.sol", "../remapped/Parent.sol", < "/private/var/folders/1m/9vppwqks3pz4btz9gnczfmgh0000gn/T/.tmpSWn2KX/remapped/Child.sol", > "../remapped/Child.sol", ] note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace failures: can_create_standard_json_input_with_external_file test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 52 filtered out; finished in 1.38s error: test failed, to rerun pass `--test project` ``` --- tests/project.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/tests/project.rs b/tests/project.rs index 99dca4ec6..aca6a6fe1 100644 --- a/tests/project.rs +++ b/tests/project.rs @@ -12,7 +12,7 @@ use foundry_compilers::{ info::ContractInfo, project_util::*, remappings::Remapping, - Artifact, CompilerInput, ConfigurableArtifacts, ExtraOutputValues, Graph, Project, + utils, Artifact, CompilerInput, ConfigurableArtifacts, ExtraOutputValues, Graph, Project, ProjectCompileOutput, ProjectPathsConfig, Solc, TestFileFilter, }; use pretty_assertions::assert_eq; @@ -1600,6 +1600,72 @@ fn can_sanitize_bytecode_hash() { assert!(compiled.find_first("A").is_some()); } +// https://github.com/foundry-rs/foundry/issues/5307 +#[test] +fn can_create_standard_json_input_with_external_file() { + // File structure: + // . + // ├── bad_verif + // │   └── src + // │   └── Counter.sol + // └── remapped + // ├── Child.sol + // └── Parent.sol + + let dir = tempfile::tempdir().unwrap(); + let bad_verif_dir = utils::canonicalize(dir.path()).unwrap().join("bad_verif"); + let remapped_dir = utils::canonicalize(dir.path()).unwrap().join("remapped"); + fs::create_dir_all(bad_verif_dir.join("src")).unwrap(); + fs::create_dir(&remapped_dir).unwrap(); + + let mut bad_verif_project = Project::builder() + .paths(ProjectPathsConfig::dapptools(&bad_verif_dir).unwrap()) + .build() + .unwrap(); + + bad_verif_project.paths.remappings.push(Remapping { + context: None, + name: "@remapped/".into(), + path: "../remapped/".into(), + }); + bad_verif_project.allowed_paths.insert(remapped_dir.clone()); + + fs::write(remapped_dir.join("Parent.sol"), "pragma solidity >=0.8.0; import './Child.sol';") + .unwrap(); + fs::write(remapped_dir.join("Child.sol"), "pragma solidity >=0.8.0;").unwrap(); + fs::write( + bad_verif_dir.join("src/Counter.sol"), + "pragma solidity >=0.8.0; import '@remapped/Parent.sol'; contract Counter {}", + ) + .unwrap(); + + // solc compiles using the host file system; therefore, this setup is considered valid + let compiled = bad_verif_project.compile().unwrap(); + compiled.assert_success(); + + // can create project root based paths + let std_json = + bad_verif_project.standard_json_input(bad_verif_dir.join("src/Counter.sol")).unwrap(); + assert_eq!( + std_json.sources.iter().map(|(path, _)| path.clone()).collect::>(), + vec![ + PathBuf::from("src/Counter.sol"), + PathBuf::from("../remapped/Parent.sol"), + PathBuf::from("../remapped/Child.sol") + ] + ); + + // can compile using the created json + let compiler_errors = Solc::default() + .compile(&std_json) + .unwrap() + .errors + .into_iter() + .filter_map(|e| if e.severity.is_error() { Some(e.message) } else { None }) + .collect::>(); + assert!(compiler_errors.is_empty(), "{:?}", compiler_errors); +} + #[test] fn can_compile_std_json_input() { let tmp = TempProject::dapptools_init().unwrap(); From e0e1bfbfdeba7a86b912976a882c84eadcceb462 Mon Sep 17 00:00:00 2001 From: tash-2s <81064017+tash-2s@users.noreply.github.com> Date: Mon, 25 Dec 2023 16:16:12 -0600 Subject: [PATCH 2/5] fix: create valid Standard JSON to verify for projects w/ external files --- src/lib.rs | 21 ++++++++++++++++++++- tests/project.rs | 21 ++++++++++----------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8ae261d0c..23ae269ea 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -518,7 +518,26 @@ impl Project { let path: PathBuf = if let Ok(stripped) = path.strip_prefix(root) { stripped.to_slash_lossy().into_owned().into() } else { - path.to_slash_lossy().into_owned().into() + let mut new_path = path.components().collect::>(); + + for (i, (root_component, path_component)) in + root.components().zip(path.components()).enumerate() + { + if root_component == path_component { + new_path.pop_front(); + } else { + let mut parent_dirs = vec![ + std::path::Component::ParentDir; + root.components().collect::>().len() + - i + ]; + parent_dirs.extend(new_path); + new_path = parent_dirs.into(); + break; + } + } + + new_path.iter().collect::().to_slash_lossy().into_owned().into() }; (path, source.clone()) }) diff --git a/tests/project.rs b/tests/project.rs index aca6a6fe1..5dda84425 100644 --- a/tests/project.rs +++ b/tests/project.rs @@ -1605,7 +1605,7 @@ fn can_sanitize_bytecode_hash() { fn can_create_standard_json_input_with_external_file() { // File structure: // . - // ├── bad_verif + // ├── verif // │   └── src // │   └── Counter.sol // └── remapped @@ -1613,39 +1613,38 @@ fn can_create_standard_json_input_with_external_file() { // └── Parent.sol let dir = tempfile::tempdir().unwrap(); - let bad_verif_dir = utils::canonicalize(dir.path()).unwrap().join("bad_verif"); + let verif_dir = utils::canonicalize(dir.path()).unwrap().join("verif"); let remapped_dir = utils::canonicalize(dir.path()).unwrap().join("remapped"); - fs::create_dir_all(bad_verif_dir.join("src")).unwrap(); + fs::create_dir_all(verif_dir.join("src")).unwrap(); fs::create_dir(&remapped_dir).unwrap(); - let mut bad_verif_project = Project::builder() - .paths(ProjectPathsConfig::dapptools(&bad_verif_dir).unwrap()) + let mut verif_project = Project::builder() + .paths(ProjectPathsConfig::dapptools(&verif_dir).unwrap()) .build() .unwrap(); - bad_verif_project.paths.remappings.push(Remapping { + verif_project.paths.remappings.push(Remapping { context: None, name: "@remapped/".into(), path: "../remapped/".into(), }); - bad_verif_project.allowed_paths.insert(remapped_dir.clone()); + verif_project.allowed_paths.insert(remapped_dir.clone()); fs::write(remapped_dir.join("Parent.sol"), "pragma solidity >=0.8.0; import './Child.sol';") .unwrap(); fs::write(remapped_dir.join("Child.sol"), "pragma solidity >=0.8.0;").unwrap(); fs::write( - bad_verif_dir.join("src/Counter.sol"), + verif_dir.join("src/Counter.sol"), "pragma solidity >=0.8.0; import '@remapped/Parent.sol'; contract Counter {}", ) .unwrap(); // solc compiles using the host file system; therefore, this setup is considered valid - let compiled = bad_verif_project.compile().unwrap(); + let compiled = verif_project.compile().unwrap(); compiled.assert_success(); // can create project root based paths - let std_json = - bad_verif_project.standard_json_input(bad_verif_dir.join("src/Counter.sol")).unwrap(); + let std_json = verif_project.standard_json_input(verif_dir.join("src/Counter.sol")).unwrap(); assert_eq!( std_json.sources.iter().map(|(path, _)| path.clone()).collect::>(), vec![ From 5c781d025a1df4fdfb3991345b71498aff834795 Mon Sep 17 00:00:00 2001 From: tash-2s <81064017+tash-2s@users.noreply.github.com> Date: Mon, 25 Dec 2023 16:58:49 -0600 Subject: [PATCH 3/5] refactor: extract path rebasing process from standard json fuction --- src/lib.rs | 106 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 77 insertions(+), 29 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 23ae269ea..bb0fa35cb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -490,8 +490,6 @@ impl Project { &self, target: impl AsRef, ) -> Result { - use path_slash::PathExt; - let target = target.as_ref(); tracing::trace!("Building standard-json-input for {:?}", target); let graph = Graph::resolve(&self.paths)?; @@ -514,33 +512,7 @@ impl Project { let root = self.root(); let sources = sources .into_iter() - .map(|(path, source)| { - let path: PathBuf = if let Ok(stripped) = path.strip_prefix(root) { - stripped.to_slash_lossy().into_owned().into() - } else { - let mut new_path = path.components().collect::>(); - - for (i, (root_component, path_component)) in - root.components().zip(path.components()).enumerate() - { - if root_component == path_component { - new_path.pop_front(); - } else { - let mut parent_dirs = vec![ - std::path::Component::ParentDir; - root.components().collect::>().len() - - i - ]; - parent_dirs.extend(new_path); - new_path = parent_dirs.into(); - break; - } - } - - new_path.iter().collect::().to_slash_lossy().into_owned().into() - }; - (path, source.clone()) - }) + .map(|(path, source)| (rebase_path(root, path), source.clone())) .collect(); let mut settings = self.solc_config.settings.clone(); @@ -973,6 +945,43 @@ impl ArtifactOutput for Project { } } +// Rebases the given path to the base directory lexically. +// +// The returned path from this function usually starts either with a normal component (e.g., `src`) +// or a parent directory component (i.e., `..`), which is based on the base directory. Additionally, +// this function converts the path into a UTF-8 string and replaces all separators with forward +// slashes (`/`). +// +// The rebasing process is as follows: +// +// 1. Remove the leading components from the path that match the base components. +// 2. Prepend `..` components to the path, equal in number to the remaining base components. +fn rebase_path(base: impl AsRef, path: impl AsRef) -> PathBuf { + use path_slash::PathExt; + + let base = base.as_ref(); + let path = path.as_ref(); + + let mut new_path = path.components().collect::>(); + + for (i, (base_component, path_component)) in + base.components().zip(path.components()).enumerate() + { + if base_component == path_component { + new_path.pop_front(); + } else { + let mut parent_dirs = + vec![std::path::Component::ParentDir; base.components().count() - i]; + parent_dirs.extend(new_path); + new_path = parent_dirs.into(); + + break; + } + } + + new_path.iter().collect::().to_slash_lossy().into_owned().into() +} + #[cfg(test)] #[cfg(all(feature = "svm-solc", not(target_arch = "wasm32")))] mod tests { @@ -1033,4 +1042,43 @@ mod tests { let contracts = project.compile().unwrap().succeeded().output().contracts; assert_eq!(contracts.contracts().count(), 2); } + + #[test] + fn can_rebase_path() { + assert_eq!(rebase_path("a/b", "a/b/c"), PathBuf::from("c")); + assert_eq!(rebase_path("a/b", "a/c"), PathBuf::from("../c")); + assert_eq!(rebase_path("a/b", "c"), PathBuf::from("../../c")); + + assert_eq!( + rebase_path("/home/user/project", "/home/user/project/A.sol"), + PathBuf::from("A.sol") + ); + assert_eq!( + rebase_path("/home/user/project", "/home/user/project/src/A.sol"), + PathBuf::from("src/A.sol") + ); + assert_eq!( + rebase_path("/home/user/project", "/home/user/project/lib/forge-std/src/Test.sol"), + PathBuf::from("lib/forge-std/src/Test.sol") + ); + assert_eq!( + rebase_path("/home/user/project", "/home/user/A.sol"), + PathBuf::from("../A.sol") + ); + assert_eq!(rebase_path("/home/user/project", "/home/A.sol"), PathBuf::from("../../A.sol")); + assert_eq!(rebase_path("/home/user/project", "/A.sol"), PathBuf::from("../../../A.sol")); + assert_eq!( + rebase_path("/home/user/project", "/tmp/A.sol"), + PathBuf::from("../../../tmp/A.sol") + ); + + assert_eq!( + rebase_path("/Users/ah/temp/verif", "/Users/ah/temp/remapped/Child.sol"), + PathBuf::from("../remapped/Child.sol") + ); + assert_eq!( + rebase_path("/Users/ah/temp/verif", "/Users/ah/temp/verif/../remapped/Parent.sol"), + PathBuf::from("../remapped/Parent.sol") + ); + } } From 38eff707095a3867b41e917133b5a97896f8eb46 Mon Sep 17 00:00:00 2001 From: tash-2s <81064017+tash-2s@users.noreply.github.com> Date: Thu, 28 Dec 2023 11:28:49 -0600 Subject: [PATCH 4/5] refactor: more efficient way without VecDeque --- src/lib.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index bb0fa35cb..dc96b36df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -959,27 +959,30 @@ impl ArtifactOutput for Project { fn rebase_path(base: impl AsRef, path: impl AsRef) -> PathBuf { use path_slash::PathExt; - let base = base.as_ref(); - let path = path.as_ref(); + let mut base_components = base.as_ref().components(); + let mut path_components = path.as_ref().components(); - let mut new_path = path.components().collect::>(); + let mut new_path = PathBuf::new(); - for (i, (base_component, path_component)) in - base.components().zip(path.components()).enumerate() - { - if base_component == path_component { - new_path.pop_front(); - } else { - let mut parent_dirs = - vec![std::path::Component::ParentDir; base.components().count() - i]; - parent_dirs.extend(new_path); - new_path = parent_dirs.into(); + while let Some(path_component) = path_components.next() { + let base_component = base_components.next(); + + if Some(path_component) != base_component { + if base_component.is_some() { + new_path.extend( + std::iter::repeat(std::path::Component::ParentDir) + .take(base_components.count() + 1), + ); + } + + new_path.push(path_component); + new_path.extend(path_components); break; } } - new_path.iter().collect::().to_slash_lossy().into_owned().into() + new_path.to_slash_lossy().into_owned().into() } #[cfg(test)] From bf564fbcf2123251526572c02b45586cd7f65476 Mon Sep 17 00:00:00 2001 From: tash-2s <81064017+tash-2s@users.noreply.github.com> Date: Thu, 28 Dec 2023 13:38:44 -0600 Subject: [PATCH 5/5] docs: improve `rebase_path` comment --- src/lib.rs | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index dc96b36df..ac6decb41 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -947,15 +947,31 @@ impl ArtifactOutput for Project { // Rebases the given path to the base directory lexically. // -// The returned path from this function usually starts either with a normal component (e.g., `src`) -// or a parent directory component (i.e., `..`), which is based on the base directory. Additionally, -// this function converts the path into a UTF-8 string and replaces all separators with forward -// slashes (`/`). +// For instance, given the base `/home/user/project` and the path `/home/user/project/src/A.sol`, +// this function returns `src/A.sol`. // -// The rebasing process is as follows: +// This function transforms a path into a form that is relative to the base directory. The returned +// path starts either with a normal component (e.g., `src`) or a parent directory component (i.e., +// `..`). It also converts the path into a UTF-8 string and replaces all separators with forward +// slashes (`/`), if they're not. // -// 1. Remove the leading components from the path that match the base components. -// 2. Prepend `..` components to the path, equal in number to the remaining base components. +// The rebasing process can be conceptualized as follows: +// +// 1. Remove the leading components from the path that match those in the base. +// 2. Prepend `..` components to the path, matching the number of remaining components in the base. +// +// # Examples +// +// `rebase_path("/home/user/project", "/home/user/project/src/A.sol")` returns `src/A.sol`. The +// common part, `/home/user/project`, is removed from the path. +// +// `rebase_path("/home/user/project", "/home/user/A.sol")` returns `../A.sol`. First, the common +// part, `/home/user`, is removed, leaving `A.sol`. Next, as `project` remains in the base, `..` is +// prepended to the path. +// +// On Windows, paths like `a\b\c` are converted to `a/b/c`. +// +// For more examples, see the test. fn rebase_path(base: impl AsRef, path: impl AsRef) -> PathBuf { use path_slash::PathExt;