fix(forge): Dedup remappings on build correctly & do not set contex…

…t by default (#5532)

* chore: dedup remappings properly on build

* chore: tests

* chore: do not use cwd

* chore: use cwd again, but do not set context

* chore: sort exactly

* chore: fmt

* chore: add docs for eventual context re-add

* fmt

* chore: update tests

* chore: fix last test

* chore: fix bounds

cli/src/cmd/forge/build/core.rs

@@ -152,8 +152,8 @@ impl<'a> From<&'a CoreBuildArgs> for Figment {
         let mut remappings = args.project_paths.get_remappings();
         remappings
             .extend(figment.extract_inner::<Vec<Remapping>>("remappings").unwrap_or_default());
-        remappings.sort_by(|a, b| a.name.cmp(&b.name));
-        remappings.dedup_by(|a, b| a.name.eq(&b.name));
+        remappings.sort_by(|a, b| (&a.context, &a.name).cmp(&(&b.context, &b.name)));
+        remappings.dedup_by(|a, b| (&a.context, &a.name).eq(&(&b.context, &b.name)));
         figment.merge(("remappings", remappings)).merge(args)
     }
 }

cli/tests/it/config.rs

@@ -144,18 +144,10 @@ forgetest_init!(
         let foundry_toml = prj.root().join(Config::FILE_NAME);
         assert!(foundry_toml.exists());
 
-        let mut profile = Config::load_with_root(prj.root());
+        let profile = Config::load_with_root(prj.root());
         // ensure that the auto-generated internal remapping for forge-std's ds-test exists
-        assert_eq!(profile.remappings.len(), 3);
-        pretty_eq!(
-            "lib/forge-std:ds-test/=lib/forge-std/lib/ds-test/src/",
-            profile.remappings[2].to_string()
-        );
-
-        // remove the auto-generated remapping to compare with `forge config` since `forge config`
-        // does not include the auto-generated remappings
-        profile.remappings.remove(2);
         assert_eq!(profile.remappings.len(), 2);
+        pretty_eq!("ds-test/=lib/forge-std/lib/ds-test/src/", profile.remappings[0].to_string());
 
         // ensure remappings contain test
         pretty_eq!("ds-test/=lib/forge-std/lib/ds-test/src/", profile.remappings[0].to_string());
@@ -433,7 +425,7 @@ forgetest_init!(can_detect_lib_foundry_toml, |prj: TestProject, mut cmd: TestCom
             "forge-std/=lib/forge-std/src/".parse().unwrap(),
             "nested-lib/=lib/nested-lib/src/".parse().unwrap(),
             // remapping is local to the lib
-            "lib/nested-lib:nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
+            "nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
         ]
     );
 
@@ -452,16 +444,14 @@ forgetest_init!(can_detect_lib_foundry_toml, |prj: TestProject, mut cmd: TestCom
         remappings,
         vec![
             // local to the lib
-            "lib/nested-lib:another-lib/=lib/nested-lib/lib/another-lib/src/".parse().unwrap(),
+            "another-lib/=lib/nested-lib/lib/another-lib/src/".parse().unwrap(),
             // global
             "ds-test/=lib/forge-std/lib/ds-test/src/".parse().unwrap(),
             "forge-std/=lib/forge-std/src/".parse().unwrap(),
             "nested-lib/=lib/nested-lib/src/".parse().unwrap(),
             // remappings local to the lib
-            "lib/nested-lib:nested-twice/=lib/nested-lib/lib/another-lib/lib/nested-twice/"
-                .parse()
-                .unwrap(),
-            "lib/nested-lib:nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
+            "nested-twice/=lib/nested-lib/lib/another-lib/lib/nested-twice/".parse().unwrap(),
+            "nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
         ]
     );
 
@@ -473,18 +463,14 @@ forgetest_init!(can_detect_lib_foundry_toml, |prj: TestProject, mut cmd: TestCom
         remappings,
         vec![
             // local to the lib
-            "lib/nested-lib:another-lib/=lib/nested-lib/lib/another-lib/custom-source-dir/"
-                .parse()
-                .unwrap(),
+            "another-lib/=lib/nested-lib/lib/another-lib/custom-source-dir/".parse().unwrap(),
             // global
             "ds-test/=lib/forge-std/lib/ds-test/src/".parse().unwrap(),
             "forge-std/=lib/forge-std/src/".parse().unwrap(),
             "nested-lib/=lib/nested-lib/src/".parse().unwrap(),
             // remappings local to the lib
-            "lib/nested-lib:nested-twice/=lib/nested-lib/lib/another-lib/lib/nested-twice/"
-                .parse()
-                .unwrap(),
-            "lib/nested-lib:nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
+            "nested-twice/=lib/nested-lib/lib/another-lib/lib/nested-twice/".parse().unwrap(),
+            "nested/=lib/nested-lib/lib/nested/".parse().unwrap(),
         ]
     );
 });

config/src/providers/remappings.rs

@@ -165,16 +165,12 @@ impl<'a> RemappingsProvider<'a> {
                     }
                 }
 
-                let mut remappings = config
-                    .remappings
-                    .into_iter()
-                    .map(Remapping::from)
-                    .map(|mut r| {
-                        // todo: windows/mac/linux
-                        r.context = Some(lib.to_string_lossy().to_string());
-                        r
-                    })
-                    .collect::<Vec<Remapping>>();
+                // Eventually, we could set context for remappings at this location,
+                // taking into account the OS platform. We'll need to be able to handle nested
+                // contexts depending on dependencies for this to work.
+                // For now, we just leave the default context (none).
+                let mut remappings =
+                    config.remappings.into_iter().map(Remapping::from).collect::<Vec<Remapping>>();
 
                 if let Some(r) = src_remapping {
                     remappings.push(r);