feat: support remapping contexts #2509
Conversation
| @@ -325,23 +325,37 @@ impl ProjectPathsConfig { | |||
| /// duplicate `contracts` segment: | |||
| /// `@openzeppelin/contracts/contracts/token/ERC20/IERC20.sol` we check for this edge case | |||
| /// here so that both styles work out of the box. | |||
| pub fn resolve_library_import(&self, import: &Path) -> Option<PathBuf> { | |||
| pub fn resolve_library_import(&self, cwd: &Path, import: &Path) -> Option<PathBuf> { | |||
There was a problem hiding this comment.
why do we need a cwd now?
| if let Some(ctx) = r.context.as_ref() { | ||
| cwd.starts_with(ctx) |
There was a problem hiding this comment.
ah because of the context?
There was a problem hiding this comment.
yep we want to limit only to that path bc otherwise we end up polluting other files' remappings in the deps
| /// - A `prefix`: the path that's used in your smart contract, i.e. | ||
| /// `@openzeppelin/contracts-ethereum-package` | ||
| /// - A `target`: the absolute path of the downloaded contracts on your computer |
There was a problem hiding this comment.
can we also copy the context docs from the solc docs here?
| #[cfg(windows)] | ||
| let mut s = String::new(); | ||
| if let Some(context) = self.context.as_ref() { | ||
| #[cfg(target_os = "windows")] |
There was a problem hiding this comment.
we have tests for windows style paths but did you get to test this on a windows box? don't have one available atm :/
| ); | ||
| assert_eq!(remapping.to_string(), "context:oz=a/b/c/d/".to_string()); | ||
|
|
||
| let remapping = "context:foo=C:/bar/src/"; |
| @@ -287,7 +345,7 @@ impl fmt::Display for RelativeRemapping { | |||
| { | |||
| format!("{}={}", self.name, self.path.original().display()) | |||
| } | |||
| }; | |||
| }); | |||
There was a problem hiding this comment.
replace (target_os = "windows") with just (windows)
| RelativeRemappingPathBuf::with_root(root.as_ref(), c) | ||
| .path | ||
| .to_string_lossy() | ||
| .to_string() |
There was a problem hiding this comment.
| .to_string() | |
| .into_owned() |
| s.push_str(context); | ||
| } | ||
| s.push(':'); | ||
| } |
There was a problem hiding this comment.
replace (target_os = "windows") with just (windows)
| @@ -56,7 +56,7 @@ coins-ledger = { version = "0.8.3", default-features = false, optional = true } | |||
| semver = { workspace = true, optional = true } | |||
|
|
|||
| # trezor | |||
| trezor-client = { version = "0.1", default-features = false, features = [ | |||
| trezor-client = { version = "=0.1.0", default-features = false, features = [ | |||
There was a problem hiding this comment.
👍 this should fix the proto issue?
| if let Some(ctx) = r.context.as_ref() { | ||
| cwd.starts_with(ctx) |
There was a problem hiding this comment.
yep we want to limit only to that path bc otherwise we end up polluting other files' remappings in the deps
Motivation
solc supports remapping contexts to limit the scope of remappings to subsets of your project, e.g. libraries you import.
We want this for foundry-rs/foundry#1855
Solution
In foundry-rs/foundry#1855 we ideally would want to auto-detect the contexts as well, i.e. limit remappings from libraries to the library roots.
This PR adds support for parsing and serializing remapping contexts, and limits import resolution based on the remapping contexts.
PR Checklist