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

fix(ethers_solc): hardcoded import remapping fix #2626

Merged
merged 2 commits into from
Oct 18, 2023
Merged

fix(ethers_solc): hardcoded import remapping fix #2626

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9fa541
fix: test different ends_with
plotchy Oct 6, 2023
54c3b19
add descriptive comments for remapping
plotchy Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions ethers-solc/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ impl ProjectPathsConfig {
///
/// There is no strict rule behind this, but because [`crate::remappings::Remapping::find_many`]
/// returns `'@openzeppelin/=node_modules/@openzeppelin/contracts/'` we should handle the
/// case if the remapping path ends with `contracts` and the import path starts with
/// `<remapping name>/contracts`. Otherwise we can end up with a resolved path that has a
/// case if the remapping path ends with `/contracts/` and the import path starts with
/// `<remapping name>/contracts/`. Otherwise we can end up with a resolved path that has a
/// 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.
Expand All @@ -344,11 +344,13 @@ impl ProjectPathsConfig {
import.strip_prefix(&r.name).ok().map(|stripped_import| {
let lib_path = Path::new(&r.path).join(stripped_import);

// we handle the edge case where the path of a remapping ends with "contracts"
// (`<name>/=.../contracts`) and the stripped import also starts with
// `contracts`
// we handle the edge case where the path of a remapping ends with "/contracts/"
// (`<name>/=.../contracts/`) and the stripped import also starts with
// `contracts/`
if let Ok(adjusted_import) = stripped_import.strip_prefix("contracts/") {
if r.path.ends_with("contracts/") && !lib_path.exists() {
// Wrap suffix check in `/` so this prevents matching remapping paths that
// end with '-contracts/', '_contracts/', '.contracts/' etc.
if r.path.ends_with("/contracts/") && !lib_path.exists() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add a note here with the openzeppelin-contracts as example

I need as much context as possible for all these rules

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Since it applies to more than oz, I wanted to keep it general.
we want to prevent matching on directories that end in things like -contracts, _contracts, .contracts

return Path::new(&r.path).join(adjusted_import)
}
}
Expand Down