fix(remappings): ignore remappings of root proj dirs when merging (foundry-rs#9258)

* fix(remappings): ignore remappings of root proj dir when merging

* Remove unused code

* Add test

* Update

* Load project paths from figment

---------

Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com>

Author: 2 people

crates/cli/src/opts/build/core.rs

@@ -181,7 +181,8 @@ impl<'a> From<&'a CoreBuildArgs> for Figment {
         };
 
         // remappings should stack
-        let mut remappings = Remappings::new_with_remappings(args.project_paths.get_remappings());
+        let mut remappings = Remappings::new_with_remappings(args.project_paths.get_remappings())
+            .with_figment(&figment);
         remappings
             .extend(figment.extract_inner::<Vec<Remapping>>("remappings").unwrap_or_default());
         figment = figment.merge(("remappings", remappings.into_inner())).merge(args);

crates/config/src/lib.rs

@@ -1091,23 +1091,6 @@ impl Config {
     }
 
     /// Returns all configured remappings.
-    ///
-    /// **Note:** this will add an additional `<src>/=<src path>` remapping here, see
-    /// [Self::get_source_dir_remapping()]
-    ///
-    /// So that
-    ///
-    /// ```solidity
-    /// import "./math/math.sol";
-    /// import "contracts/tokens/token.sol";
-    /// ```
-    ///
-    /// in `contracts/contract.sol` are resolved to
-    ///
-    /// ```text
-    /// contracts/tokens/token.sol
-    /// contracts/math/math.sol
-    /// ```
     pub fn get_all_remappings(&self) -> impl Iterator<Item = Remapping> + '_ {
         self.remappings.iter().map(|m| m.clone().into())
     }

crates/config/src/providers/remappings.rs

@@ -1,7 +1,10 @@
-use crate::{foundry_toml_dirs, remappings_from_env_var, remappings_from_newline, Config};
+use crate::{
+    foundry_toml_dirs, remappings_from_env_var, remappings_from_newline, utils::get_dir_remapping,
+    Config,
+};
 use figment::{
     value::{Dict, Map},
-    Error, Metadata, Profile, Provider,
+    Error, Figment, Metadata, Profile, Provider,
 };
 use foundry_compilers::artifacts::remappings::{RelativeRemapping, Remapping};
 use std::{
@@ -16,17 +19,35 @@ use std::{
 pub struct Remappings {
     /// Remappings.
     remappings: Vec<Remapping>,
+    /// Source, test and script configured project dirs.
+    /// Remappings of these dirs from libs are ignored.
+    project_paths: Vec<Remapping>,
 }
 
 impl Remappings {
     /// Create a new `Remappings` wrapper with an empty vector.
     pub fn new() -> Self {
-        Self { remappings: Vec::new() }
+        Self { remappings: Vec::new(), project_paths: Vec::new() }
     }
 
     /// Create a new `Remappings` wrapper with a vector of remappings.
     pub fn new_with_remappings(remappings: Vec<Remapping>) -> Self {
-        Self { remappings }
+        Self { remappings, project_paths: Vec::new() }
+    }
+
+    /// Extract project paths that cannot be remapped by dependencies.
+    pub fn with_figment(mut self, figment: &Figment) -> Self {
+        let mut add_project_remapping = |path: &str| {
+            if let Ok(path) = figment.find_value(path) {
+                if let Some(remapping) = path.into_string().and_then(get_dir_remapping) {
+                    self.project_paths.push(remapping);
+                }
+            }
+        };
+        add_project_remapping("src");
+        add_project_remapping("test");
+        add_project_remapping("script");
+        self
     }
 
     /// Filters the remappings vector by name and context.
@@ -47,16 +68,28 @@ impl Remappings {
 
     /// Push an element to the remappings vector, but only if it's not already present.
     pub fn push(&mut self, remapping: Remapping) {
-        if !self.remappings.iter().any(|existing| {
+        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,
             // 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
         }) {
-            self.remappings.push(remapping)
-        }
+            return;
+        };
+
+        // Ignore remappings of root project src, test or script dir.
+        // See <https://github.com/foundry-rs/foundry/issues/3440>.
+        if self
+            .project_paths
+            .iter()
+            .any(|project_path| remapping.name.eq_ignore_ascii_case(&project_path.name))
+        {
+            return;
+        };
+
+        self.remappings.push(remapping);
     }
 
     /// Extend the remappings vector, leaving out the remappings that are already present.

crates/forge/tests/cli/config.rs

@@ -777,3 +777,39 @@ forgetest!(normalize_config_evm_version, |_prj, cmd| {
     let config: Config = serde_json::from_str(&output).unwrap();
     assert_eq!(config.evm_version, EvmVersion::Istanbul);
 });
+
+// Tests that root paths are properly resolved even if submodule specifies remappings for them.
+// See <https://github.com/foundry-rs/foundry/issues/3440>
+forgetest_init!(test_submodule_root_path_remappings, |prj, cmd| {
+    prj.add_script(
+        "BaseScript.sol",
+        r#"
+import "forge-std/Script.sol";
+
+contract BaseScript is Script {
+}
+   "#,
+    )
+    .unwrap();
+    prj.add_script(
+        "MyScript.sol",
+        r#"
+import "script/BaseScript.sol";
+
+contract MyScript is BaseScript {
+}
+   "#,
+    )
+    .unwrap();
+
+    let nested = prj.paths().libraries[0].join("another-dep");
+    pretty_err(&nested, fs::create_dir_all(&nested));
+    let mut lib_config = Config::load_with_root(&nested);
+    lib_config.remappings = vec![
+        Remapping::from_str("test/=test/").unwrap().into(),
+        Remapping::from_str("script/=script/").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.forge_fuse().args(["build"]).assert_success();
+});