From 6abacbf62a11854eeb9b5b4e299d6ae0c12d9d92 Mon Sep 17 00:00:00 2001
From: grandizzy <grandizzy.the.egg@gmail.com>
Date: Mon, 9 Dec 2024 13:05:44 +0200
Subject: [PATCH] fix(remappings): ignore conflicting remappings

---
 crates/config/src/providers/remappings.rs | 22 +++++++---
 crates/forge/tests/cli/config.rs          | 51 +++++++++++++++++++++++
 2 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/crates/config/src/providers/remappings.rs b/crates/config/src/providers/remappings.rs
index 1d8a7b16368c..81cdf3fee236 100644
--- a/crates/config/src/providers/remappings.rs
+++ b/crates/config/src/providers/remappings.rs
@@ -69,14 +69,26 @@ impl Remappings {
     }
 
     /// Push an element to the remappings vector, but only if it's not already present.
-    pub fn push(&mut self, remapping: Remapping) {
+    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,
+            // What we're doing here is filtering for ambiguous / conflicting paths per context.
+            // For example, if we have
+            // `@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.
-            existing.name.starts_with(&remapping.name) && existing.context == remapping.context
+            // Remappings are added from root of the project down to libraries, so
+            // we also want to exclude any conflicting remappings added from libraries. For example,
+            // if we have `@utils/=src/` added in project remappings and `@utils/libraries/=src/`
+            // added in a dependency, we don't want to add the new one as it conflicts with project
+            // existing remapping.
+            let mut existing_name_path = existing.name.clone();
+            if !existing_name_path.ends_with('/') {
+                existing_name_path.push('/')
+            }
+            let is_conflicting = remapping.name.starts_with(&existing_name_path) ||
+                existing.name.starts_with(&remapping.name);
+            is_conflicting && existing.context == remapping.context
         }) {
             return;
         };
diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs
index 545cebac87a8..fcb4663d685e 100644
--- a/crates/forge/tests/cli/config.rs
+++ b/crates/forge/tests/cli/config.rs
@@ -620,6 +620,57 @@ 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>
+// Test that
+// - project defined `@openzeppelin/contracts` remapping is added
+// - library defined `@openzeppelin/contracts-upgradeable` remapping is added
+// - library defined `@openzeppelin/contracts/upgradeable` remapping is not added as it conflicts
+// with project defined `@openzeppelin/contracts` remapping
+// See <https://github.com/foundry-rs/foundry/issues/9271>
+forgetest_init!(can_prioritise_project_remappings, |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("@openzeppelin/contracts=lib/openzeppelin-contracts/").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("@openzeppelin/contracts-upgradeable/=lib/openzeppelin-upgradeable/")
+            .unwrap()
+            .into(),
+        Remapping::from_str(
+            "@openzeppelin/contracts/upgradeable/=lib/openzeppelin-contracts/upgradeable/",
+        )
+        .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/
+- @openzeppelin/contracts/=lib/openzeppelin-contracts/
+- @openzeppelin/contracts-upgradeable/=lib/dep1/lib/openzeppelin-upgradeable/
+- 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();