Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 93 additions & 12 deletions crates/config/src/providers/remappings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,19 @@ impl Remappings {
&& existing.path == remapping.path;
}

// 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.
// 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.
// Autodetected remappings are added from the root project down through its libraries,
// so an existing root alias remains authoritative over an equal or more specific
// dependency alias. For example, an existing `@utils/=src/` suppresses an incoming
// `@utils/libraries/=lib/utils/`, preventing a dependency from overriding part of the
// root namespace. The reverse direction is intentional: an existing
// `@prb/math/=src/math/` can coexist with an incoming `@prb/=lib/prb/`; the root alias
// resolves its subtree while the dependency alias acts as a fallback for the rest of
// the namespace.
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);
let is_conflicting = remapping.name.starts_with(&existing_name_path);
is_conflicting && existing.context == remapping.context
}) {
return;
Expand Down Expand Up @@ -476,4 +473,88 @@ mod tests {
.any(|r| r.context == Some("prod/".to_string()) && r.path == "prod/Contract.sol")
);
}

#[test]
fn test_root_remapping_prefix_precedence_is_directional() {
let remapping = |name: &str, path: &str| Remapping {
context: None,
name: name.to_string(),
path: path.to_string(),
};

let mut narrow_root =
Remappings::new_with_remappings(vec![remapping("pkg/sub/", "src/local/")]);
narrow_root.extend(vec![remapping("pkg/", "lib/pkg/src/")]);
assert_eq!(
narrow_root.into_inner(),
vec![remapping("pkg/sub/", "src/local/"), remapping("pkg/", "lib/pkg/src/")]
);

let mut broad_root = Remappings::new_with_remappings(vec![remapping("pkg/", "src/local/")]);
broad_root.extend(vec![
remapping("pkg/sub/", "lib/pkg/src/sub/"),
remapping("pkg-other/", "lib/pkg-other/src/"),
]);
assert_eq!(
broad_root.into_inner(),
vec![remapping("pkg/", "src/local/"), remapping("pkg-other/", "lib/pkg-other/src/")]
);

let mut duplicate = Remappings::new_with_remappings(vec![remapping("pkg/", "src/local/")]);
duplicate.extend(vec![remapping("pkg/", "lib/pkg/src/")]);
assert_eq!(duplicate.remappings, vec![remapping("pkg/", "src/local/")]);

let contextual_remapping = |context: &str, name: &str, path: &str| Remapping {
context: Some(context.to_string()),
name: name.to_string(),
path: path.to_string(),
};
let mut same_context = Remappings::new_with_remappings(vec![contextual_remapping(
"src/",
"pkg/",
"src/local/",
)]);
same_context.extend(vec![contextual_remapping("src/", "pkg/sub/", "lib/pkg/src/sub/")]);
assert_eq!(
same_context.remappings,
vec![contextual_remapping("src/", "pkg/", "src/local/")]
);

let mut different_context = Remappings::new_with_remappings(vec![contextual_remapping(
"src/",
"pkg/",
"src/local/",
)]);
different_context.extend(vec![contextual_remapping(
"test/",
"pkg/sub/",
"lib/pkg/src/sub/",
)]);
assert_eq!(
different_context.remappings,
vec![
contextual_remapping("src/", "pkg/", "src/local/"),
contextual_remapping("test/", "pkg/sub/", "lib/pkg/src/sub/"),
]
);

let mut narrow_root_without_slash =
Remappings::new_with_remappings(vec![remapping("pkg/sub", "src/local/")]);
narrow_root_without_slash.extend(vec![remapping("pkg", "lib/pkg/src/")]);
assert_eq!(
narrow_root_without_slash.remappings,
vec![remapping("pkg/sub", "src/local/"), remapping("pkg", "lib/pkg/src/")]
);

let mut broad_root_without_slash =
Remappings::new_with_remappings(vec![remapping("pkg", "src/local/")]);
broad_root_without_slash.extend(vec![
remapping("pkg/sub", "lib/pkg/src/sub/"),
remapping("pkg-other", "lib/pkg-other/src/"),
]);
assert_eq!(
broad_root_without_slash.remappings,
vec![remapping("pkg", "src/local/"), remapping("pkg-other", "lib/pkg-other/src/")]
);
}
}
100 changes: 100 additions & 0 deletions crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,106 @@ Global:
"#]]);
});

forgetest!(narrow_project_remapping_preserves_broad_dependency_fallback, |prj, cmd| {
prj.update_config(|config| {
config.remappings = vec![Remapping::from_str("pkg/sub/=src/local/").unwrap().into()];
});
prj.add_source(
"Root.sol",
r#"
import {Dep} from "pkg/Dep.sol";
import {Local} from "pkg/sub/Local.sol";

contract Root is Dep, Local {}
"#,
);
prj.add_source(
"local/Local.sol",
r#"
contract Local {
function localValue() public pure returns (uint256) {
return 1;
}
}
"#,
);
let dependency = prj.root().join("lib/pkg/src");
pretty_err(&dependency, fs::create_dir_all(&dependency));
pretty_err(
dependency.join("Dep.sol"),
fs::write(
dependency.join("Dep.sol"),
r#"
contract Dep {
function dependencyValue() public pure returns (uint256) {
return 2;
}
}
"#,
),
);

cmd.args(["remappings"]).assert_success().stdout_eq(str![[r#"
pkg/sub/=src/local/
pkg/=lib/pkg/src/

"#]]);
cmd.forge_fuse().args(["build"]).assert_success();
// Forge lint resolves imports through Solar independently of the solc build.
cmd.forge_fuse().args(["lint"]).assert_success();
});

forgetest!(broad_project_remapping_suppresses_narrow_dependency_override, |prj, cmd| {
prj.update_config(|config| {
config.remappings = vec![Remapping::from_str("pkg/=src/local/").unwrap().into()];
});
prj.add_source(
"Root.sol",
r#"
import {Selected} from "pkg/sub/Selected.sol";

contract Root is Selected {}
"#,
);
prj.add_source(
"local/sub/Selected.sol",
r#"
contract Selected {
function selectedValue() public pure returns (uint256) {
return 1;
}
}
"#,
);

let dependency = prj.root().join("lib/dep1");
pretty_err(dependency.join("src/decoy"), fs::create_dir_all(dependency.join("src/decoy")));
let mut dependency_config = Config::load_with_root(&dependency).unwrap();
dependency_config.remappings = vec![Remapping::from_str("pkg/sub/=src/decoy/").unwrap().into()];
pretty_err(
dependency.join("foundry.toml"),
fs::write(dependency.join("foundry.toml"), dependency_config.to_string_pretty().unwrap()),
);
pretty_err(
dependency.join("src/decoy/Selected.sol"),
fs::write(
dependency.join("src/decoy/Selected.sol"),
r#"
contract DependencyDecoy {}
"#,
),
);

cmd.args(["remappings"]).assert_success().stdout_eq(str![[r#"
pkg/=src/local/
dep1/=lib/dep1/src/

"#]]);
cmd.forge_fuse().args(["build"]).assert_success();
// Solar prefers the longest matching prefix, so this fails if the dependency override leaks.
cmd.forge_fuse().args(["lint"]).assert_success();
});

// Verifies the contract invariant: `forge remappings` and `forge remappings --pretty` emit
// identical stdout, even when remappings have contexts. The context prefix is part of the
// machine-readable value and must survive `--pretty` mode.
Expand Down
Loading