Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow remapping of solidity files #9604

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
142 changes: 141 additions & 1 deletion crates/config/src/providers/remappings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,22 @@ impl Remappings {

/// Push an element to the remappings vector, but only if it's not already present.
pub fn push(&mut self, remapping: Remapping) {
// Special handling for .sol file remappings, only allow one remapping per source file.
if remapping.name.ends_with(".sol") && !remapping.path.ends_with(".sol") {
return;
}
Comment on lines +73 to +76
Copy link
Member

@zerosnacks zerosnacks Jan 7, 2025

Choose a reason for hiding this comment

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

From preliminary tests it doesn't appear we support remappings w/ Vyper sources

Not sure if this is known and / or worth spending time on implementing or something we should document as a limitation in the book (https://book.getfoundry.sh/config/vyper#5-limitations)

cc @klkvr


if self.remappings.iter().any(|existing| {
if remapping.name.ends_with(".sol") {
// For .sol files, only prevent duplicate source names in the same context
return existing.name == remapping.name &&
existing.context == remapping.context &&
existing.path == remapping.path
}

// What we're doing here is filtering for ambiguous paths. For example, if we have
// @prb/math/=node_modules/@prb/math/src/ as existing, and
// @prb/=node_modules/@prb/ as the one being checked,
// @prb/=node_modules/@prb/ as the one being checked,
// we want to keep the already existing one, which is the first one. This way we avoid
// having to deal with ambiguous paths which is unwanted when autodetecting remappings.
existing.name.starts_with(&remapping.name) && existing.context == remapping.context
Expand Down Expand Up @@ -308,3 +320,131 @@ impl Provider for RemappingsProvider<'_> {
Some(Config::selected_profile())
}
}

#[cfg(test)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks good, another approach to write remapping tests is similar with

forgetest_init!(can_prioritise_closer_lib_remappings, |prj, cmd| {

mod tests {
use super::*;

#[test]
fn test_sol_file_remappings() {
let mut remappings = Remappings::new();

// First valid remapping
remappings.push(Remapping {
context: None,
name: "MyContract.sol".to_string(),
path: "implementations/Contract1.sol".to_string(),
});

// Same source to different target (should be rejected)
remappings.push(Remapping {
context: None,
name: "MyContract.sol".to_string(),
path: "implementations/Contract2.sol".to_string(),
});

// Different source to same target (should be allowed)
remappings.push(Remapping {
context: None,
name: "OtherContract.sol".to_string(),
path: "implementations/Contract1.sol".to_string(),
});

// Exact duplicate (should be silently ignored)
remappings.push(Remapping {
context: None,
name: "MyContract.sol".to_string(),
path: "implementations/Contract1.sol".to_string(),
});

// Invalid .sol remapping (target not .sol)
remappings.push(Remapping {
context: None,
name: "Invalid.sol".to_string(),
path: "implementations/Contract1.txt".to_string(),
});

let result = remappings.into_inner();
assert_eq!(result.len(), 2, "Should only have 2 valid remappings");

// Verify the correct remappings exist
assert!(
result
.iter()
.any(|r| r.name == "MyContract.sol" && r.path == "implementations/Contract1.sol"),
"Should keep first mapping of MyContract.sol"
);
assert!(
!result
.iter()
.any(|r| r.name == "MyContract.sol" && r.path == "implementations/Contract2.sol"),
"Should keep first mapping of MyContract.sol"
);
assert!(result.iter().any(|r| r.name == "OtherContract.sol" && r.path == "implementations/Contract1.sol"),
"Should allow different source to same target");

// Verify the rejected remapping doesn't exist
assert!(
!result
.iter()
.any(|r| r.name == "MyContract.sol" && r.path == "implementations/Contract2.sol"),
"Should reject same source to different target"
);
}

#[test]
fn test_mixed_remappings() {
let mut remappings = Remappings::new();

remappings.push(Remapping {
context: None,
name: "@openzeppelin/".to_string(),
path: "lib/openzeppelin/".to_string(),
});
remappings.push(Remapping {
context: None,
name: "@openzeppelin/contracts/".to_string(),
path: "lib/openzeppelin/contracts/".to_string(),
});

remappings.push(Remapping {
context: None,
name: "MyContract.sol".to_string(),
path: "os/Contract.sol".to_string(),
});

let result = remappings.into_inner();

assert_eq!(result.len(), 3, "Should have 3 remappings");
assert!(result.iter().any(
|r| r.name == "@openzeppelin/contracts/" && r.path == "lib/openzeppelin/contracts/"
));
assert!(result.iter().any(|r| r.name == "MyContract.sol" && r.path == "os/Contract.sol"));
}

#[test]
fn test_remappings_with_context() {
let mut remappings = Remappings::new();

// Same name but different contexts
remappings.push(Remapping {
context: Some("test/".to_string()),
name: "MyContract.sol".to_string(),
path: "test/Contract.sol".to_string(),
});
remappings.push(Remapping {
context: Some("prod/".to_string()),
name: "MyContract.sol".to_string(),
path: "prod/Contract.sol".to_string(),
});

let result = remappings.into_inner();
assert_eq!(result.len(), 2, "Should allow same name with different contexts");
assert!(result
.iter()
.any(|r| r.context == Some("test/".to_string()) && r.path == "test/Contract.sol"));
assert!(result
.iter()
.any(|r| r.context == Some("prod/".to_string()) && r.path == "prod/Contract.sol"));
}
}
Loading