Skip to content

Commit

Permalink
Revert "fix(remappings): check if remapping to add starts with existi…
Browse files Browse the repository at this point in the history
…ng remapping name (#9246)"

This reverts commit 455ba9b.
  • Loading branch information
grandizzy committed Nov 6, 2024
1 parent 42f5eb9 commit a983639
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 44 deletions.
10 changes: 4 additions & 6 deletions crates/config/src/providers/remappings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,14 @@ impl Remappings {
}

/// Push an element to the remappings vector, but only if it's not already present.
fn push(&mut self, remapping: Remapping) {
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/=node_modules/@prb/ as existing, and
// @prb/math/=node_modules/@prb/math/src/ as the one being checked,
// @prb/math/=node_modules/@prb/math/src/ as existing, and
// @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.
// Remappings are added from root of the project down to libraries, so
// we want to exclude any conflicting remappings added from libraries.
remapping.name.starts_with(&existing.name) && existing.context == remapping.context
existing.name.starts_with(&remapping.name) && existing.context == remapping.context
}) {
self.remappings.push(remapping)
}
Expand Down
38 changes: 0 additions & 38 deletions crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -595,44 +595,6 @@ 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();
// Add `@utils/` remapping in project 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 in the `lib` folder with conflicting `@utils/libraries` remapping.
// This should be filtered out from final remappings as root project already has `@utils/`.
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(),
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 a983639

Please sign in to comment.