Skip to content

Commit

Permalink
feat: allow remapping of solidity files (#9604)
Browse files Browse the repository at this point in the history
* feat(remapping): support remapping of .sol files

Signed-off-by: jsvisa <[email protected]>

* feat(remapping): add testcase

Signed-off-by: jsvisa <[email protected]>

* typo

Signed-off-by: jsvisa <[email protected]>

---------

Signed-off-by: jsvisa <[email protected]>
  • Loading branch information
jsvisa authored Jan 21, 2025
1 parent fea3885 commit 5993795
Showing 1 changed file with 141 additions and 1 deletion.
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;
}

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 @@ -309,3 +321,131 @@ impl Provider for RemappingsProvider<'_> {
Some(Config::selected_profile())
}
}

#[cfg(test)]
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"));
}
}

0 comments on commit 5993795

Please sign in to comment.