Skip to content

Commit

Permalink
fix(remappings): check if remapping to add starts with existing remap…
Browse files Browse the repository at this point in the history
…ping name
  • Loading branch information
grandizzy committed Nov 1, 2024
1 parent 6b0c27e commit e6e7677
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
8 changes: 5 additions & 3 deletions crates/config/src/providers/remappings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,13 @@ impl Remappings {
pub fn push(&mut self, remapping: Remapping) {
if !self.remappings.iter().any(|existing| {
// 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 existing, and
// @prb/math/=node_modules/@prb/math/src/ 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
// Remappings are added from root of the project down to libraries, so
// we want to keep
remapping.name.starts_with(&existing.name) && existing.context == remapping.context
}) {
self.remappings.push(remapping)
}
Expand Down
34 changes: 34 additions & 0 deletions crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,40 @@ forgetest_init!(can_prioritise_closer_lib_remappings, |prj, cmd| {
);
});

// Test that remappings within root of the project have priority over remappings of sub-projects.
// E.g. `@utils/libraries` mapping from library shouldn't be added if project already has `@utils`
// remapping.
// See <https://github.com/foundry-rs/foundry/issues/9146>
forgetest_init!(test_root_remappings_priority, |prj, cmd| {
let mut config = cmd.config();
config.remappings = vec![
Remapping::from_str("@utils/=src/").unwrap().into(),
Remapping::from_str("@another-utils/libraries/=src/").unwrap().into(),
];
let proj_toml_file = prj.paths().root.join("foundry.toml");
pretty_err(&proj_toml_file, fs::write(&proj_toml_file, config.to_string_pretty().unwrap()));

// create a new lib directly in the `lib` folder with conflicting remapping `forge-std/`
let nested = prj.paths().libraries[0].join("dep1");
pretty_err(&nested, fs::create_dir_all(&nested));
let mut lib_config = Config::load_with_root(&nested);
lib_config.remappings = vec![Remapping::from_str("@utils/libraries/=src/").unwrap().into()];
lib_config.remappings = vec![Remapping::from_str("@another-utils/=src/").unwrap().into()];
let lib_toml_file = nested.join("foundry.toml");
pretty_err(&lib_toml_file, fs::write(&lib_toml_file, lib_config.to_string_pretty().unwrap()));

cmd.args(["remappings", "--pretty"]).assert_success().stdout_eq(str![[r#"
Global:
- @utils/=src/
- @another-utils/libraries/=src/
- @another-utils/=lib/dep1/src/
- dep1/=lib/dep1/src/
- forge-std/=lib/forge-std/src/
"#]]);
});

// test to check that foundry.toml libs section updates on install
forgetest!(can_update_libs_section, |prj, cmd| {
cmd.git_init();
Expand Down

0 comments on commit e6e7677

Please sign in to comment.