diff --git a/crates/fm/src/lib.rs b/crates/fm/src/lib.rs index cc87129fc0d..b79a2b2c700 100644 --- a/crates/fm/src/lib.rs +++ b/crates/fm/src/lib.rs @@ -38,12 +38,21 @@ pub struct FileManager { impl FileManager { // XXX: Maybe use a AsRef here, for API ergonomics pub fn add_file(&mut self, path_to_file: &Path, file_type: FileType) -> Option { - let source = file_reader::read_file_to_string(path_to_file).ok()?; + // Handle both relative file paths and std/lib virtual paths. + let base = Path::new(".").canonicalize().expect("Base path canonicalize failed"); + let res = path_to_file.canonicalize().unwrap_or_else(|_| path_to_file.to_path_buf()); + let resolved_path = res.strip_prefix(base).unwrap_or(&res); + + // Check that the resolved path already exists in the file map, if it is, we return it. + let path_to_file = virtualize_path(resolved_path, file_type); + if let Some(file_id) = self.path_to_id.get(&path_to_file) { + return Some(*file_id); + } - let file_id = self.file_map.add_file(path_to_file.to_path_buf().into(), source); - let path_to_file = virtualize_path(path_to_file, file_type); + // Otherwise we add the file + let source = file_reader::read_file_to_string(resolved_path).ok()?; + let file_id = self.file_map.add_file(resolved_path.to_path_buf().into(), source); self.register_path(file_id, path_to_file); - Some(file_id) } @@ -177,4 +186,28 @@ mod tests { // Now check for files in it's subdirectory fm.resolve_path(sub_dir_file_id, "foo").unwrap(); } + + /// Tests that two identical files that have different paths are treated as the same file + /// e.g. if we start in the dir ./src and have a file ../../foo.nr + /// that should be treated as the same file as ../ starting in ./ + /// they should both resolve to ../foo.nr + #[test] + fn path_resolve_modules_with_different_paths_as_same_file() { + let mut fm = FileManager::default(); + + // Create a lib.nr file at the root. + let dir = tempdir().unwrap(); + let sub_dir = TempDir::new_in(&dir).unwrap(); + let sub_sub_dir = TempDir::new_in(&sub_dir).unwrap(); + let file_path = dummy_file_path(&dir, "lib.nr"); + + // Create another file in a subdirectory with a convoluted path + let second_file_path = dummy_file_path(&sub_sub_dir, "./../../lib.nr"); + + // Add both files to the file manager + let file_id = fm.add_file(&file_path, FileType::Root).unwrap(); + let second_file_id = fm.add_file(&second_file_path, FileType::Root).unwrap(); + + assert_eq!(file_id, second_file_id); + } } diff --git a/crates/nargo_cli/tests/test_data/diamond_deps_0/Nargo.toml b/crates/nargo_cli/tests/test_data/diamond_deps_0/Nargo.toml new file mode 100644 index 00000000000..d5094940130 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/diamond_deps_0/Nargo.toml @@ -0,0 +1,7 @@ +[package] +authors = [""] +compiler_version = "0.7.1" + +[dependencies] +dep1 = { path = "../../test_libraries/diamond_deps_1" } +dep2 = { path = "../../test_libraries/diamond_deps_2" } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data/diamond_deps_0/Prover.toml b/crates/nargo_cli/tests/test_data/diamond_deps_0/Prover.toml new file mode 100644 index 00000000000..a713241e7dd --- /dev/null +++ b/crates/nargo_cli/tests/test_data/diamond_deps_0/Prover.toml @@ -0,0 +1,3 @@ +x = 1 +y = 1 +return = 5 \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data/diamond_deps_0/src/main.nr b/crates/nargo_cli/tests/test_data/diamond_deps_0/src/main.nr new file mode 100644 index 00000000000..f01491171bb --- /dev/null +++ b/crates/nargo_cli/tests/test_data/diamond_deps_0/src/main.nr @@ -0,0 +1,7 @@ +use dep::dep1::call_dep1_then_dep2; +use dep::dep2::call_dep2; +use dep::dep2::RESOLVE_THIS; + +fn main(x : Field, y : pub Field) -> pub Field { + call_dep1_then_dep2(x, y) + call_dep2(x, y) + RESOLVE_THIS +} \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_libraries/diamond_deps_1/Nargo.toml b/crates/nargo_cli/tests/test_libraries/diamond_deps_1/Nargo.toml new file mode 100644 index 00000000000..1241b147d83 --- /dev/null +++ b/crates/nargo_cli/tests/test_libraries/diamond_deps_1/Nargo.toml @@ -0,0 +1,6 @@ +[package] +authors = [""] +compiler_version = "0.7.1" + +[dependencies] +dep2 = { path = "../diamond_deps_2" } \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_libraries/diamond_deps_1/src/lib.nr b/crates/nargo_cli/tests/test_libraries/diamond_deps_1/src/lib.nr new file mode 100644 index 00000000000..cecb2715104 --- /dev/null +++ b/crates/nargo_cli/tests/test_libraries/diamond_deps_1/src/lib.nr @@ -0,0 +1,5 @@ +use dep::dep2::call_dep2; + +fn call_dep1_then_dep2(x : Field, y : Field) -> Field { + call_dep2(x, y) +} diff --git a/crates/nargo_cli/tests/test_libraries/diamond_deps_2/Nargo.toml b/crates/nargo_cli/tests/test_libraries/diamond_deps_2/Nargo.toml new file mode 100644 index 00000000000..7cae77988e3 --- /dev/null +++ b/crates/nargo_cli/tests/test_libraries/diamond_deps_2/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.7.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_libraries/diamond_deps_2/src/lib.nr b/crates/nargo_cli/tests/test_libraries/diamond_deps_2/src/lib.nr new file mode 100644 index 00000000000..f7745efd679 --- /dev/null +++ b/crates/nargo_cli/tests/test_libraries/diamond_deps_2/src/lib.nr @@ -0,0 +1,6 @@ +global RESOLVE_THIS = 3; + + +fn call_dep2(x : Field, y : Field) -> Field { + x + y +} diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs index 47426606da1..0e4a4c63518 100644 --- a/crates/noirc_frontend/src/graph/mod.rs +++ b/crates/noirc_frontend/src/graph/mod.rs @@ -83,7 +83,11 @@ impl CrateGraph { pub fn add_crate_root(&mut self, crate_type: CrateType, file_id: FileId) -> CrateId { let mut roots_with_file_id = self.arena.iter().filter(|(_, crate_data)| crate_data.root_file_id == file_id); - assert!(roots_with_file_id.next().is_none(), "you cannot add the same file id twice"); + + let next_file_id = roots_with_file_id.next(); + if let Some(file_id) = next_file_id { + return *file_id.0; + } let data = CrateData { root_file_id: file_id, crate_type, dependencies: Vec::new() }; let crate_id = CrateId(self.arena.len()); @@ -232,7 +236,6 @@ mod tests { assert!(graph.add_dep(crate2, CrateName::new("crate3").unwrap(), crate3).is_ok()); } #[test] - #[should_panic] fn it_works2() { let file_ids = dummy_file_ids(3); let file_id_0 = file_ids[0]; @@ -241,7 +244,10 @@ mod tests { let mut graph = CrateGraph::default(); let _crate1 = graph.add_crate_root(CrateType::Library, file_id_0); let _crate2 = graph.add_crate_root(CrateType::Library, file_id_1); - let _crate3 = graph.add_crate_root(CrateType::Library, file_id_2); - let _crate3 = graph.add_crate_root(CrateType::Library, file_id_2); + + // Adding the same file, so the crate should be the same. + let crate3 = graph.add_crate_root(CrateType::Library, file_id_2); + let crate3_2 = graph.add_crate_root(CrateType::Library, file_id_2); + assert_eq!(crate3, crate3_2); } }