Skip to content

Commit 56911f2

Browse files
authored
Fix inconsistent trailing slash in remappings (foundry-rs#49)
Fix foundry-rs#47. One corresponding test is added. ## Fix strategy - Remove the code that blindly add trailing `/` when serializing `Remapping` and `RelativeRemapping`. - Preserve the trailing `/` in the `strip_prefix` method of `Remapping`. ## Rationale There are roughly two sources where `Remapping` comes from: - Generated in `find_many` function. - Loaded from configuration like `foundry.toml`. When generating `Remapping` in `find_many` function, the algorithm makes sure that the `name` and `path` of every remapping are valid folders in the file system. Then, trailing `/`s are added to both `name` and `path` in the `impl From<RelativeRemapping> for Remapping` implementation. https://github.com/Troublor/compilers/blob/93c5f46a7dd0c0d387dd94c8ea756812e2907d92/src/remappings.rs#L360-L365 When loading `Remapping` from user-provided configurations, the `name` and `path` of remappings should be set as it is, and do not add any trailing `/`s. Either way, the field `name` and `path` of `Remapping` contains intended values and should not be changed in the serialization (the implementation for `Display` trait). PS: About coding style, I tried to use `TempProject` to write the test case but it seems the `project.compile()` method **does not respect** the `remapping` I give it via `project.with_settings(settings_with_remapping)`. The remapping just does not take effect in the compilation. Not sure whether it's I don't know how to use it or `TempProject` has some bugs.
1 parent a21275d commit 56911f2

File tree

3 files changed

+45
-10
lines changed

3 files changed

+45
-10
lines changed

crates/artifacts/solc/src/remappings.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -143,16 +143,18 @@ impl fmt::Display for Remapping {
143143
}
144144
s.push(':');
145145
}
146+
let name =
147+
if !self.name.ends_with('/') { format!("{}/", self.name) } else { self.name.clone() };
146148
s.push_str(&{
147149
#[cfg(target_os = "windows")]
148150
{
149151
// ensure we have `/` slashes on windows
150152
use path_slash::PathExt;
151-
format!("{}={}", self.name, std::path::Path::new(&self.path).to_slash_lossy())
153+
format!("{}={}", name, std::path::Path::new(&self.path).to_slash_lossy())
152154
}
153155
#[cfg(not(target_os = "windows"))]
154156
{
155-
format!("{}={}", self.name, self.path)
157+
format!("{}={}", name, self.path)
156158
}
157159
});
158160

@@ -1161,7 +1163,7 @@ mod tests {
11611163
path: "a/b/c/d".to_string(),
11621164
}
11631165
);
1164-
assert_eq!(remapping.to_string(), "context:oz=a/b/c/d/".to_string());
1166+
assert_eq!(remapping.to_string(), "context:oz/=a/b/c/d/".to_string());
11651167

11661168
let remapping = "context:foo=C:/bar/src/";
11671169
let remapping = Remapping::from_str(remapping).unwrap();
@@ -1185,7 +1187,7 @@ mod tests {
11851187
remapping,
11861188
Remapping { context: None, name: "oz".to_string(), path: "a/b/c/d/".to_string() }
11871189
);
1188-
assert_eq!(remapping.to_string(), "oz=a/b/c/d/".to_string());
1190+
assert_eq!(remapping.to_string(), "oz/=a/b/c/d/".to_string());
11891191
}
11901192

11911193
#[test]

crates/compilers/src/report/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ mod tests {
492492
Unable to resolve imports:
493493
"./src/Import.sol" in "src/File.col"
494494
with remappings:
495-
oz=a/b/c/d/"#
495+
oz/=a/b/c/d/"#
496496
.trim()
497497
)
498498
}

crates/compilers/tests/project.rs

+38-5
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ fn can_flatten_on_solang_failure() {
716716
r#"// SPDX-License-Identifier: UNLICENSED
717717
pragma solidity ^0.8.10;
718718
719-
library Lib {}
719+
library Lib {}
720720
"#,
721721
)
722722
.unwrap();
@@ -748,7 +748,7 @@ pragma solidity ^0.8.10;
748748
749749
// src/Lib.sol
750750
751-
library Lib {}
751+
library Lib {}
752752
753753
// src/Contract.sol
754754
@@ -2980,17 +2980,17 @@ fn can_parse_notice() {
29802980
29812981
/**
29822982
* @notice hello
2983-
*/
2983+
*/
29842984
constructor(string memory _greeting) public {
29852985
greeting = _greeting;
29862986
}
2987-
2987+
29882988
/**
29892989
* @notice hello
29902990
*/
29912991
function xyz() public {
29922992
}
2993-
2993+
29942994
/// @notice hello
29952995
function abc() public {
29962996
}
@@ -3986,3 +3986,36 @@ fn test_can_compile_multi() {
39863986
assert!(compiled.find(&root.join("src/Counter.vy"), "Counter").is_some());
39873987
compiled.assert_success();
39883988
}
3989+
3990+
// This is a reproduction of https://github.com/foundry-rs/compilers/issues/47
3991+
#[cfg(feature = "svm-solc")]
3992+
#[test]
3993+
fn remapping_trailing_slash_issue47() {
3994+
use std::sync::Arc;
3995+
3996+
use foundry_compilers_artifacts::{EvmVersion, Source, Sources};
3997+
3998+
let mut sources = Sources::new();
3999+
sources.insert(
4000+
PathBuf::from("./C.sol"),
4001+
Source {
4002+
content: Arc::new(r#"import "@project/D.sol"; contract C {}"#.to_string()),
4003+
kind: Default::default(),
4004+
},
4005+
);
4006+
sources.insert(
4007+
PathBuf::from("./D.sol"),
4008+
Source { content: Arc::new(r#"contract D {}"#.to_string()), kind: Default::default() },
4009+
);
4010+
4011+
let mut settings = Settings { evm_version: Some(EvmVersion::Byzantium), ..Default::default() };
4012+
settings.remappings.push(Remapping {
4013+
context: None,
4014+
name: "@project".into(),
4015+
path: ".".into(),
4016+
});
4017+
let input = SolcInput { language: SolcLanguage::Solidity, sources, settings };
4018+
let compiler = Solc::find_or_install(&Version::new(0, 6, 8)).unwrap();
4019+
let output = compiler.compile_exact(&input).unwrap();
4020+
assert!(!output.has_error());
4021+
}

0 commit comments

Comments
 (0)