Skip to content

Commit

Permalink
Fix inconsistent trailing slash in remappings (#49)
Browse files Browse the repository at this point in the history
Fix #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.
  • Loading branch information
Troublor authored Jul 6, 2024
1 parent a21275d commit 56911f2
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
10 changes: 6 additions & 4 deletions crates/artifacts/solc/src/remappings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,18 @@ impl fmt::Display for Remapping {
}
s.push(':');
}
let name =
if !self.name.ends_with('/') { format!("{}/", self.name) } else { self.name.clone() };
s.push_str(&{
#[cfg(target_os = "windows")]
{
// ensure we have `/` slashes on windows
use path_slash::PathExt;
format!("{}={}", self.name, std::path::Path::new(&self.path).to_slash_lossy())
format!("{}={}", name, std::path::Path::new(&self.path).to_slash_lossy())
}
#[cfg(not(target_os = "windows"))]
{
format!("{}={}", self.name, self.path)
format!("{}={}", name, self.path)
}
});

Expand Down Expand Up @@ -1161,7 +1163,7 @@ mod tests {
path: "a/b/c/d".to_string(),
}
);
assert_eq!(remapping.to_string(), "context:oz=a/b/c/d/".to_string());
assert_eq!(remapping.to_string(), "context:oz/=a/b/c/d/".to_string());

let remapping = "context:foo=C:/bar/src/";
let remapping = Remapping::from_str(remapping).unwrap();
Expand All @@ -1185,7 +1187,7 @@ mod tests {
remapping,
Remapping { context: None, name: "oz".to_string(), path: "a/b/c/d/".to_string() }
);
assert_eq!(remapping.to_string(), "oz=a/b/c/d/".to_string());
assert_eq!(remapping.to_string(), "oz/=a/b/c/d/".to_string());
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion crates/compilers/src/report/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ mod tests {
Unable to resolve imports:
"./src/Import.sol" in "src/File.col"
with remappings:
oz=a/b/c/d/"#
oz/=a/b/c/d/"#
.trim()
)
}
Expand Down
43 changes: 38 additions & 5 deletions crates/compilers/tests/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ fn can_flatten_on_solang_failure() {
r#"// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.10;
library Lib {}
library Lib {}
"#,
)
.unwrap();
Expand Down Expand Up @@ -748,7 +748,7 @@ pragma solidity ^0.8.10;
// src/Lib.sol
library Lib {}
library Lib {}
// src/Contract.sol
Expand Down Expand Up @@ -2980,17 +2980,17 @@ fn can_parse_notice() {
/**
* @notice hello
*/
*/
constructor(string memory _greeting) public {
greeting = _greeting;
}
/**
* @notice hello
*/
function xyz() public {
}
/// @notice hello
function abc() public {
}
Expand Down Expand Up @@ -3986,3 +3986,36 @@ fn test_can_compile_multi() {
assert!(compiled.find(&root.join("src/Counter.vy"), "Counter").is_some());
compiled.assert_success();
}

// This is a reproduction of https://github.com/foundry-rs/compilers/issues/47
#[cfg(feature = "svm-solc")]
#[test]
fn remapping_trailing_slash_issue47() {
use std::sync::Arc;

use foundry_compilers_artifacts::{EvmVersion, Source, Sources};

let mut sources = Sources::new();
sources.insert(
PathBuf::from("./C.sol"),
Source {
content: Arc::new(r#"import "@project/D.sol"; contract C {}"#.to_string()),
kind: Default::default(),
},
);
sources.insert(
PathBuf::from("./D.sol"),
Source { content: Arc::new(r#"contract D {}"#.to_string()), kind: Default::default() },
);

let mut settings = Settings { evm_version: Some(EvmVersion::Byzantium), ..Default::default() };
settings.remappings.push(Remapping {
context: None,
name: "@project".into(),
path: ".".into(),
});
let input = SolcInput { language: SolcLanguage::Solidity, sources, settings };
let compiler = Solc::find_or_install(&Version::new(0, 6, 8)).unwrap();
let output = compiler.compile_exact(&input).unwrap();
assert!(!output.has_error());
}

0 comments on commit 56911f2

Please sign in to comment.