From 7e0a3d11ad3c5473ae4411d8e90be96f3a69c7d1 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 21:07:41 +0000 Subject: [PATCH 01/33] make `find_module` be an immutable reference --- compiler/fm/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index ea964055759..2b16cdeb3c9 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -94,7 +94,7 @@ impl FileManager { self.id_to_path.get(&file_id).unwrap().as_path() } - pub fn find_module(&mut self, anchor: FileId, mod_name: &str) -> Result { + pub fn find_module(&self, anchor: FileId, mod_name: &str) -> Result { let anchor_path = self.path(anchor).with_extension(""); let anchor_dir = anchor_path.parent().unwrap(); @@ -107,9 +107,10 @@ impl FileManager { anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")) }; - self.add_file(&candidate).ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string()) + self.name_to_id(candidate.clone()).ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string()) } + // TODO: This should accept a &Path instead of a PathBuf pub fn name_to_id(&self, file_name: PathBuf) -> Option { self.file_map.get_file_id(&PathString::from_path(file_name)) } From b2ff05b6aebe11e0c67da6c1ae2c4c95dd5b1823 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 21:08:12 +0000 Subject: [PATCH 02/33] add method in nargo that pre-populates the FileManager --- tooling/nargo/src/lib.rs | 87 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index ef014fb436b..fee3a3a3088 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -42,9 +42,29 @@ pub fn prepare_dependencies( } } +// We will pre-populate the file manager with all the files in the package +// This is so that we can avoid having to read from disk when we are compiling +// +// This does not require parsing because we are interested in the files under the src directory +// it may turn out that we do not need to include some Noir files that we add to the file +// manager +pub fn insert_all_files_for_package_into_file_manager(package : &Package, file_manager : &mut FileManager) { + // Start off at the root directory of the package and add all of the files located + // in that directory. + let root_path = package.root_dir.clone(); + + // Get all files in the package and add them to the file manager + let paths = get_all_paths_in_dir(&root_path).expect("could not get all paths in the package"); + for path in paths { + file_manager.add_file(path.as_path()); + } +} + pub fn prepare_package(package: &Package, file_reader: Box) -> (Context, CrateId) { // TODO: FileManager continues to leak into various crates - let fm = FileManager::new(&package.root_dir, file_reader); + let mut fm = FileManager::new(&package.root_dir, file_reader); + insert_all_files_for_package_into_file_manager(package, &mut fm); + let graph = CrateGraph::default(); let mut context = Context::new(fm, graph); @@ -54,3 +74,68 @@ pub fn prepare_package(package: &Package, file_reader: Box) -> (Cont (context, crate_id) } + +// Get all paths in the directory and subdirectories. +// +// Panics: If the path is not a path to a directory. +// +// TODO: Along with prepare_package, this function is an abstraction leak +// TODO given that this crate should not know about the file manager. +// TODO: We can clean this up in a future refactor +fn get_all_paths_in_dir(dir: &std::path::Path) -> std::io::Result> { + assert!(dir.is_dir(), "directory {dir:?} is not a path to a directory"); + + let mut paths = Vec::new(); + + if dir.is_dir() { + for entry in std::fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + if path.is_dir() { + let mut sub_paths = get_all_paths_in_dir(&path)?; + paths.append(&mut sub_paths); + } else { + paths.push(path); + } + } + } + + Ok(paths) +} + +#[cfg(test)] + mod tests { + use crate::get_all_paths_in_dir; + use std::{fs::{self, File}, path::Path}; + use tempfile::tempdir; + + fn create_test_dir_structure(temp_dir: &Path) -> std::io::Result<()> { + fs::create_dir(temp_dir.join("subdir1"))?; + File::create(temp_dir.join("subdir1/file1.txt"))?; + fs::create_dir(temp_dir.join("subdir2"))?; + File::create(temp_dir.join("subdir2/file2.txt"))?; + File::create(temp_dir.join("file3.txt"))?; + Ok(()) + } + + #[test] + fn test_get_all_paths_in_dir() { + let temp_dir = tempdir().expect("could not create a temporary directory"); + create_test_dir_structure(temp_dir.path()).expect("could not create test directory structure"); + + let paths = get_all_paths_in_dir(temp_dir.path()).expect("could not get all paths in the test directory"); + + // This should be the paths to all of the files in the directory and the subdirectory + let expected_paths = vec![ + temp_dir.path().join("file3.txt"), + temp_dir.path().join("subdir1/file1.txt"), + temp_dir.path().join("subdir2/file2.txt"), + ]; + + assert_eq!(paths.len(), expected_paths.len()); + for path in expected_paths { + assert!(paths.contains(&path)); + } + } + + } \ No newline at end of file From 55923488827a2b5d194e425997106a6de8294417 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 21:08:49 +0000 Subject: [PATCH 03/33] change add_file to `name_to_id` -- we assume that the file manager has already been populated --- compiler/noirc_driver/src/lib.rs | 4 ++-- tooling/nargo_cli/src/cli/fmt_cmd.rs | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 23869fc2a61..d564be4d382 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -78,7 +78,7 @@ pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { let std_file_id = context.file_manager.add_file(&path_to_std_lib_file).unwrap(); let std_crate_id = context.crate_graph.add_stdlib(std_file_id); - let root_file_id = context.file_manager.add_file(file_name).unwrap(); + let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).unwrap(); let root_crate_id = context.crate_graph.add_crate_root(root_file_id); @@ -89,7 +89,7 @@ pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { // Adds the file from the file system at `Path` to the crate graph pub fn prepare_dependency(context: &mut Context, file_name: &Path) -> CrateId { - let root_file_id = context.file_manager.add_file(file_name).unwrap(); + let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).expect("files are expected to be added to the FileManager before reaching the compiler"); let crate_id = context.crate_graph.add_crate(root_file_id); diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index ec3d373a483..ea90282f354 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -2,6 +2,7 @@ use std::{fs::DirEntry, path::Path}; use clap::Args; use fm::FileManager; +use nargo::insert_all_files_for_package_into_file_manager; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_driver::NOIR_ARTIFACT_VERSION_STRING; use noirc_errors::CustomDiagnostic; @@ -37,9 +38,10 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr for package in &workspace { let mut file_manager = FileManager::new(&package.root_dir, Box::new(|path| std::fs::read_to_string(path))); - + insert_all_files_for_package_into_file_manager(package, &mut file_manager); + visit_noir_files(&package.root_dir.join("src"), &mut |entry| { - let file_id = file_manager.add_file(&entry.path()).expect("file exists"); + let file_id = file_manager.name_to_id(entry.path().to_path_buf()).expect("The file should exist since we added all files in the package into the file manager"); let (parsed_module, errors) = parse_file(&file_manager, file_id); let is_all_warnings = errors.iter().all(ParserError::is_warning); From d5fd4e0cef0f369d69db9dda4b032a0a49bd8e95 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 21:08:56 +0000 Subject: [PATCH 04/33] cargo --- Cargo.lock | 1 + tooling/nargo/Cargo.toml | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 58b86705381..df702fef5ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2403,6 +2403,7 @@ dependencies = [ "rayon", "rustc_version", "serde", + "tempfile", "thiserror", ] diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index f8269459968..2c224d87055 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -24,4 +24,7 @@ iter-extended.workspace = true serde.workspace = true thiserror.workspace = true codespan-reporting.workspace = true -rayon = "1.8.0" \ No newline at end of file +rayon = "1.8.0" +# TODO: This dependency is used to generate unit tests for `get_all_paths_in_dir` +# TODO: once that method is moved to nargo_cli, we can move this dependency to nargo_cli +tempfile = "3.2.0" \ No newline at end of file From 2f137b70e5d0497869a071bde8e8c557e3dfe684 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 21:27:20 +0000 Subject: [PATCH 05/33] Update tooling/nargo/src/lib.rs --- tooling/nargo/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index fee3a3a3088..adc7ab61940 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -80,7 +80,7 @@ pub fn prepare_package(package: &Package, file_reader: Box) -> (Cont // Panics: If the path is not a path to a directory. // // TODO: Along with prepare_package, this function is an abstraction leak -// TODO given that this crate should not know about the file manager. +// TODO: given that this crate should not know about the file manager. // TODO: We can clean this up in a future refactor fn get_all_paths_in_dir(dir: &std::path::Path) -> std::io::Result> { assert!(dir.is_dir(), "directory {dir:?} is not a path to a directory"); From 41ae81f08c9c3abae0cebbd8555ddaf22ed2eef8 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:32:35 +0000 Subject: [PATCH 06/33] add a method in file manager that allows us to add a file with its source --- compiler/fm/src/lib.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index 2b16cdeb3c9..5b41a2717b7 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -73,6 +73,22 @@ impl FileManager { Some(file_id) } + // TODO: this will become the default strategy for adding files. Possibly with file_reader. + // TODO: we are still migrating to this strategy, so we keep the old one for now. + // TODO: For the stdlib crate, we need to do this preemptively due to the way we special handle it + pub fn add_file_with_source(&mut self, file_name: &Path, source : String) -> Option { + // Check that the file name already exists in the file map, if it is, we return it. + if let Some(file_id) = self.path_to_id.get(file_name) { + return Some(*file_id); + } + let file_name_path_buf = file_name.to_path_buf(); + + // Otherwise we add the file + let file_id = self.file_map.add_file(file_name_path_buf.clone().into(), source); + self.register_path(file_id, file_name_path_buf); + Some(file_id) + } + fn register_path(&mut self, file_id: FileId, path: PathBuf) { let old_value = self.id_to_path.insert(file_id, path.clone()); assert!( @@ -94,6 +110,9 @@ impl FileManager { self.id_to_path.get(&file_id).unwrap().as_path() } + // TODO: This should also ideally not be here, so that the file manager + // TODO: does not know about rust modules. + // TODO: Ideally this is moved to def_collector_mod and we make this method accept a FileManager pub fn find_module(&self, anchor: FileId, mod_name: &str) -> Result { let anchor_path = self.path(anchor).with_extension(""); let anchor_dir = anchor_path.parent().unwrap(); @@ -116,6 +135,9 @@ impl FileManager { } } +// TODO: This should not be here because the file manager should not know about the +// TODO: rust modules. See comment on `find_module`` +// TODO: Moreover, the check for main, lib, mod should ideally not be done here /// Returns true if a module's child module's are expected to be in the same directory. /// Returns false if they are expected to be in a subdirectory matching the name of the module. fn should_check_siblings_for_module(module_path: &Path, parent_path: &Path) -> bool { From c7f576c056cbc97b671687137150327dc6f5e627 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:32:52 +0000 Subject: [PATCH 07/33] add deprecation TODO to file_reader --- compiler/fm/src/file_reader.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/fm/src/file_reader.rs b/compiler/fm/src/file_reader.rs index d17aefeda7e..e7ffd93df03 100644 --- a/compiler/fm/src/file_reader.rs +++ b/compiler/fm/src/file_reader.rs @@ -5,6 +5,11 @@ use std::path::Path; // Based on the environment, we either read files using the rust standard library or we // read files using the javascript host function +// TODO: DO NOT MERGE PR WITH THIS TODO +// TODO: We have duplicated this logic in noirc_driver. For now, we leave this here +// TODO: until we make the breaking change to the API which will allow us to remove this +// TODO: file. + pub type FileReader = dyn Fn(&Path) -> std::io::Result + Send; #[derive(RustEmbed)] From a2460e6e2dc3a69379b99f6df954cd5c217cf323 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:33:31 +0000 Subject: [PATCH 08/33] add stdlib file in noirc_driver that returns the stdlib paths alongside their source code --- compiler/noirc_driver/src/stdlib.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 compiler/noirc_driver/src/stdlib.rs diff --git a/compiler/noirc_driver/src/stdlib.rs b/compiler/noirc_driver/src/stdlib.rs new file mode 100644 index 00000000000..b6fa76ff862 --- /dev/null +++ b/compiler/noirc_driver/src/stdlib.rs @@ -0,0 +1,24 @@ +use rust_embed::RustEmbed; + +#[derive(RustEmbed)] +#[folder = "../../noir_stdlib/src"] +#[cfg_attr(not(target_os = "windows"), prefix = "std/")] +#[cfg_attr(target_os = "windows", prefix = r"std\")] // Note reversed slash direction +struct StdLibAssets; + +// Returns a vector of tuples containing the path to a stdlib file in the std lib crate +// along with the source code of that file. +// +// This is needed because when we preload the file manager, it needs to know where +// the source code for the stdlib is. The stdlib is treated special because it comes with +// the compiler and is never included as a dependency like other user defined crates. +pub(crate) fn stdlib_paths_with_source() -> Vec<(String, String)> { + StdLibAssets::iter() + .map(|path| { + let source = std::str::from_utf8(StdLibAssets::get(&path).unwrap().data.as_ref()) + .unwrap() + .to_string(); + (path.to_string(), source) + }) + .collect() +} \ No newline at end of file From 270a0f41e21b0e92f63b2524c303872e66f5a87b Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:33:59 +0000 Subject: [PATCH 09/33] add the contents of the stdlib whenever we call prepare_crate --- compiler/noirc_driver/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index d564be4d382..0ba50ad9692 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -23,6 +23,7 @@ mod abi_gen; mod contract; mod debug; mod program; +mod stdlib; use debug::filter_relevant_files; @@ -76,6 +77,12 @@ pub type CompilationResult = Result<(T, Warnings), ErrorsAndWarnings>; pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { let path_to_std_lib_file = Path::new(STD_CRATE_NAME).join("lib.nr"); let std_file_id = context.file_manager.add_file(&path_to_std_lib_file).unwrap(); + let stdlib_paths_with_source = stdlib::stdlib_paths_with_source(); + for (path, source) in stdlib_paths_with_source { + let path_buf = std::path::Path::new(&path); + context.file_manager.add_file_with_source(path_buf, source); + } + let std_crate_id = context.crate_graph.add_stdlib(std_file_id); let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).unwrap(); From 52d33a5adcf0344d0463a0b7c15c57e6e0f322e7 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:34:04 +0000 Subject: [PATCH 10/33] cargo --- Cargo.lock | 1 + compiler/noirc_driver/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index df702fef5ec..31a6c3932df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2627,6 +2627,7 @@ dependencies = [ "noirc_errors", "noirc_evaluator", "noirc_frontend", + "rust-embed", "serde", ] diff --git a/compiler/noirc_driver/Cargo.toml b/compiler/noirc_driver/Cargo.toml index c717efed6f5..0faa6c7770a 100644 --- a/compiler/noirc_driver/Cargo.toml +++ b/compiler/noirc_driver/Cargo.toml @@ -21,6 +21,7 @@ iter-extended.workspace = true fm.workspace = true serde.workspace = true fxhash.workspace = true +rust-embed = "6.6.0" aztec_macros ={path = "../../aztec_macros", optional = true} From a38c27c98898357efe01d001547b0710f9d6f547 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:35:56 +0000 Subject: [PATCH 11/33] cargo fmt --- compiler/fm/src/lib.rs | 7 ++++--- compiler/noirc_driver/src/lib.rs | 5 ++++- compiler/noirc_driver/src/stdlib.rs | 2 +- tooling/nargo/src/lib.rs | 25 ++++++++++++++++--------- tooling/nargo_cli/src/cli/fmt_cmd.rs | 4 ++-- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index 5b41a2717b7..e84904b8bc2 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -76,7 +76,7 @@ impl FileManager { // TODO: this will become the default strategy for adding files. Possibly with file_reader. // TODO: we are still migrating to this strategy, so we keep the old one for now. // TODO: For the stdlib crate, we need to do this preemptively due to the way we special handle it - pub fn add_file_with_source(&mut self, file_name: &Path, source : String) -> Option { + pub fn add_file_with_source(&mut self, file_name: &Path, source: String) -> Option { // Check that the file name already exists in the file map, if it is, we return it. if let Some(file_id) = self.path_to_id.get(file_name) { return Some(*file_id); @@ -126,7 +126,8 @@ impl FileManager { anchor_path.join(format!("{mod_name}.{FILE_EXTENSION}")) }; - self.name_to_id(candidate.clone()).ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string()) + self.name_to_id(candidate.clone()) + .ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string()) } // TODO: This should accept a &Path instead of a PathBuf @@ -135,7 +136,7 @@ impl FileManager { } } -// TODO: This should not be here because the file manager should not know about the +// TODO: This should not be here because the file manager should not know about the // TODO: rust modules. See comment on `find_module`` // TODO: Moreover, the check for main, lib, mod should ideally not be done here /// Returns true if a module's child module's are expected to be in the same directory. diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 0ba50ad9692..71ca721bce2 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -96,7 +96,10 @@ pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { // Adds the file from the file system at `Path` to the crate graph pub fn prepare_dependency(context: &mut Context, file_name: &Path) -> CrateId { - let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).expect("files are expected to be added to the FileManager before reaching the compiler"); + let root_file_id = context + .file_manager + .name_to_id(file_name.to_path_buf()) + .expect("files are expected to be added to the FileManager before reaching the compiler"); let crate_id = context.crate_graph.add_crate(root_file_id); diff --git a/compiler/noirc_driver/src/stdlib.rs b/compiler/noirc_driver/src/stdlib.rs index b6fa76ff862..5a91e3f45b5 100644 --- a/compiler/noirc_driver/src/stdlib.rs +++ b/compiler/noirc_driver/src/stdlib.rs @@ -21,4 +21,4 @@ pub(crate) fn stdlib_paths_with_source() -> Vec<(String, String)> { (path.to_string(), source) }) .collect() -} \ No newline at end of file +} diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index adc7ab61940..8fdb4d88c70 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -46,13 +46,16 @@ pub fn prepare_dependencies( // This is so that we can avoid having to read from disk when we are compiling // // This does not require parsing because we are interested in the files under the src directory -// it may turn out that we do not need to include some Noir files that we add to the file +// it may turn out that we do not need to include some Noir files that we add to the file // manager -pub fn insert_all_files_for_package_into_file_manager(package : &Package, file_manager : &mut FileManager) { +pub fn insert_all_files_for_package_into_file_manager( + package: &Package, + file_manager: &mut FileManager, +) { // Start off at the root directory of the package and add all of the files located // in that directory. let root_path = package.root_dir.clone(); - + // Get all files in the package and add them to the file manager let paths = get_all_paths_in_dir(&root_path).expect("could not get all paths in the package"); for path in paths { @@ -104,9 +107,12 @@ fn get_all_paths_in_dir(dir: &std::path::Path) -> std::io::Result std::io::Result<()> { @@ -121,9 +127,11 @@ fn get_all_paths_in_dir(dir: &std::path::Path) -> std::io::Result std::io::Result Result<(), CliErr for package in &workspace { let mut file_manager = FileManager::new(&package.root_dir, Box::new(|path| std::fs::read_to_string(path))); - insert_all_files_for_package_into_file_manager(package, &mut file_manager); - + insert_all_files_for_package_into_file_manager(package, &mut file_manager); + visit_noir_files(&package.root_dir.join("src"), &mut |entry| { let file_id = file_manager.name_to_id(entry.path().to_path_buf()).expect("The file should exist since we added all files in the package into the file manager"); let (parsed_module, errors) = parse_file(&file_manager, file_id); From 6a5bf3389ab20424f053233087ed00c79f3b97a3 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:38:01 +0000 Subject: [PATCH 12/33] move tempfile to dev dependencies --- tooling/nargo/Cargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index 2c224d87055..48741c367a5 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -25,6 +25,8 @@ serde.workspace = true thiserror.workspace = true codespan-reporting.workspace = true rayon = "1.8.0" + +[dev-dependencies] # TODO: This dependency is used to generate unit tests for `get_all_paths_in_dir` # TODO: once that method is moved to nargo_cli, we can move this dependency to nargo_cli tempfile = "3.2.0" \ No newline at end of file From f72ce9993efb93a30346ce0ad0b609f7a4bdbc49 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:53:46 +0000 Subject: [PATCH 13/33] add files into file manager in test since find_module does not add files into the file manager anymore --- compiler/fm/src/lib.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index e84904b8bc2..bfe5b81dfcd 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -240,9 +240,11 @@ mod tests { use super::*; use tempfile::{tempdir, TempDir}; - fn create_dummy_file(dir: &TempDir, file_name: &Path) { + // Returns the absolute path to the file + fn create_dummy_file(dir: &TempDir, file_name: &Path) -> PathBuf { let file_path = dir.path().join(file_name); - let _file = std::fs::File::create(file_path).unwrap(); + let _file = std::fs::File::create(&file_path).unwrap(); + file_path } #[test] @@ -281,30 +283,30 @@ mod tests { // Create a lib.nr file at the root. // we now have dir/lib.nr - let file_name = Path::new("lib.nr"); - create_dummy_file(&dir, file_name); - - let file_id = fm.add_file(file_name).unwrap(); - + let lib_nr_path = create_dummy_file(&dir, Path::new("lib.nr")); + let file_id = fm.add_file(lib_nr_path.as_path()).expect("could not add file to file manager and obtain a FileId"); + // Create a sub directory // we now have: // - dir/lib.nr // - dir/sub_dir let sub_dir = TempDir::new_in(&dir).unwrap(); let sub_dir_name = sub_dir.path().file_name().unwrap().to_str().unwrap(); - + // Add foo.nr to the subdirectory // we no have: // - dir/lib.nr // - dir/sub_dir/foo.nr - create_dummy_file(&sub_dir, Path::new("foo.nr")); - + let foo_nr_path =create_dummy_file(&sub_dir, Path::new("foo.nr")); + fm.add_file(foo_nr_path.as_path()); + // Add a parent module for the sub_dir // we no have: // - dir/lib.nr // - dir/sub_dir.nr // - dir/sub_dir/foo.nr - create_dummy_file(&dir, Path::new(&format!("{sub_dir_name}.nr"))); + let sub_dir_nr_path = create_dummy_file(&dir, Path::new(&format!("{sub_dir_name}.nr"))); + fm.add_file(sub_dir_nr_path.as_path()); // First check for the sub_dir.nr file and add it to the FileManager let sub_dir_file_id = fm.find_module(file_id, sub_dir_name).unwrap(); From b30ad5473dc64af10bba8e2acb8f9c4f59954ff3 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:59:28 +0000 Subject: [PATCH 14/33] insert all files for this packages dependencies into the file manager too --- tooling/nargo/src/lib.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 8fdb4d88c70..7c81b6bcbac 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -61,6 +61,24 @@ pub fn insert_all_files_for_package_into_file_manager( for path in paths { file_manager.add_file(path.as_path()); } + + insert_all_files_for_packages_dependencies_into_file_manager(package, file_manager); +} + +// Inserts all files for the dependencies of the package into the file manager +// too +fn insert_all_files_for_packages_dependencies_into_file_manager( + package: &Package, + file_manager: &mut FileManager, +) { + for (_, dep) in package.dependencies.iter() { + match dep { + Dependency::Local { package } | Dependency::Remote { package } => { + insert_all_files_for_package_into_file_manager(package, file_manager); + insert_all_files_for_packages_dependencies_into_file_manager(package, file_manager); + } + } + } } pub fn prepare_package(package: &Package, file_reader: Box) -> (Context, CrateId) { From 15a94ee507fe8594499d5ec71c0b35ee7d13b409 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 22:59:41 +0000 Subject: [PATCH 15/33] cargo fmt --- compiler/fm/src/lib.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index bfe5b81dfcd..4f4eb9d9838 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -284,22 +284,24 @@ mod tests { // Create a lib.nr file at the root. // we now have dir/lib.nr let lib_nr_path = create_dummy_file(&dir, Path::new("lib.nr")); - let file_id = fm.add_file(lib_nr_path.as_path()).expect("could not add file to file manager and obtain a FileId"); - + let file_id = fm + .add_file(lib_nr_path.as_path()) + .expect("could not add file to file manager and obtain a FileId"); + // Create a sub directory // we now have: // - dir/lib.nr // - dir/sub_dir let sub_dir = TempDir::new_in(&dir).unwrap(); let sub_dir_name = sub_dir.path().file_name().unwrap().to_str().unwrap(); - + // Add foo.nr to the subdirectory // we no have: // - dir/lib.nr // - dir/sub_dir/foo.nr - let foo_nr_path =create_dummy_file(&sub_dir, Path::new("foo.nr")); + let foo_nr_path = create_dummy_file(&sub_dir, Path::new("foo.nr")); fm.add_file(foo_nr_path.as_path()); - + // Add a parent module for the sub_dir // we no have: // - dir/lib.nr From 45993614fd13e23300fc2afefc38dbcb638cbf20 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 23:00:59 +0000 Subject: [PATCH 16/33] remove un-needed fully qualified path --- compiler/noirc_driver/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 71ca721bce2..a62c30e7d9d 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -79,8 +79,7 @@ pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { let std_file_id = context.file_manager.add_file(&path_to_std_lib_file).unwrap(); let stdlib_paths_with_source = stdlib::stdlib_paths_with_source(); for (path, source) in stdlib_paths_with_source { - let path_buf = std::path::Path::new(&path); - context.file_manager.add_file_with_source(path_buf, source); + context.file_manager.add_file_with_source(Path::new(&path), source); } let std_crate_id = context.crate_graph.add_stdlib(std_file_id); From f20109e2b09420a75249a760ce871f074aa38e98 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 23:03:02 +0000 Subject: [PATCH 17/33] Add note on stdlib --- compiler/noirc_driver/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index a62c30e7d9d..7f8d91dfa76 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -77,6 +77,11 @@ pub type CompilationResult = Result<(T, Warnings), ErrorsAndWarnings>; pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { let path_to_std_lib_file = Path::new(STD_CRATE_NAME).join("lib.nr"); let std_file_id = context.file_manager.add_file(&path_to_std_lib_file).unwrap(); + + // Add the stdlib contents to the file manager, since every package automatically has a dependency + // on the stdlib. For other dependencies, we read the package.Dependencies file to add their file + // contents to the file manager. However since the dependency on the stdlib is implicit, we need + // to manually add it here. let stdlib_paths_with_source = stdlib::stdlib_paths_with_source(); for (path, source) in stdlib_paths_with_source { context.file_manager.add_file_with_source(Path::new(&path), source); From 4413350ca79839092360a05ca28b0a40a3caef84 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Sun, 10 Dec 2023 23:06:09 +0000 Subject: [PATCH 18/33] cargo fmt --- compiler/noirc_driver/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 7f8d91dfa76..71998412261 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -77,9 +77,9 @@ pub type CompilationResult = Result<(T, Warnings), ErrorsAndWarnings>; pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { let path_to_std_lib_file = Path::new(STD_CRATE_NAME).join("lib.nr"); let std_file_id = context.file_manager.add_file(&path_to_std_lib_file).unwrap(); - + // Add the stdlib contents to the file manager, since every package automatically has a dependency - // on the stdlib. For other dependencies, we read the package.Dependencies file to add their file + // on the stdlib. For other dependencies, we read the package.Dependencies file to add their file // contents to the file manager. However since the dependency on the stdlib is implicit, we need // to manually add it here. let stdlib_paths_with_source = stdlib::stdlib_paths_with_source(); From d6a24e8529a8c831d20a3fa3058c4c3871252288 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 00:22:36 +0000 Subject: [PATCH 19/33] add comment to process_dep_graph and fix setup_test_context --- compiler/wasm/src/compile.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index e7fd3dd5212..246e6ab7750 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -200,6 +200,12 @@ pub fn compile( } } +// Root dependencies are dependencies which the entry-point package relies upon. +// These will be in the Nargo.toml of the package being compiled. +// +// Library dependencies are transitive dependencies; for example, if the entry-point relies +// upon some library `lib1`. Then the packages that `lib1` depend upon will be placed in the +// `library_dependencies` list and the `lib1` will be placed in the `root_dependencies` list. fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyGraph) { let mut crate_names: HashMap<&CrateName, CrateId> = HashMap::new(); @@ -320,10 +326,10 @@ mod test { } fn setup_test_context() -> Context { - let fm = FileManager::new(Path::new("/"), Box::new(mock_get_non_stdlib_asset)); + let mut fm = FileManager::new(Path::new("/"), Box::new(mock_get_non_stdlib_asset)); + fm.add_file_with_source(Path::new("/main.nr"), "stuff".to_string()); let graph = CrateGraph::default(); let mut context = Context::new(fm, graph); - prepare_crate(&mut context, Path::new("/main.nr")); context From 552a4dda457835992577952a11b698e2216cadb0 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 00:59:00 +0000 Subject: [PATCH 20/33] replace expect with unwrap_or_else: expect doesn't allow you to add placeholders like panic! and format! do --- compiler/noirc_driver/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 71998412261..ba0e12cd7e7 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -103,7 +103,7 @@ pub fn prepare_dependency(context: &mut Context, file_name: &Path) -> CrateId { let root_file_id = context .file_manager .name_to_id(file_name.to_path_buf()) - .expect("files are expected to be added to the FileManager before reaching the compiler"); + .unwrap_or_else(|| panic!("files are expected to be added to the FileManager before reaching the compiler file_path: {file_name:?}")); let crate_id = context.crate_graph.add_crate(root_file_id); From 3f8551b6749c191b30fc8744125d4ab9a5c35db6 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 01:00:15 +0000 Subject: [PATCH 21/33] try: add a `PathToFileSourceMap` object --- compiler/wasm/src/compile.rs | 91 ++++++++++++++++++++++++++++++++---- 1 file changed, 83 insertions(+), 8 deletions(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 246e6ab7750..f4aea0874d3 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -126,6 +126,31 @@ struct DependencyGraph { library_dependencies: HashMap>, } +#[wasm_bindgen] +// This is a map containing the paths of all of the files in the entry-point crate and +// the transitive dependencies of the entry-point crate. +// +// This is for all intents and purposes the file system that the compiler will use to resolve/compile +// files in the crate being compiled and its dependencies. +#[derive(Deserialize, Default)] +pub struct PathToFileSourceMap(HashMap); + +#[wasm_bindgen] +impl PathToFileSourceMap { + #[wasm_bindgen(constructor)] + pub fn new() -> PathToFileSourceMap { + PathToFileSourceMap::default() + } + // Inserts a path and its source code into the map. + // + // Returns true, if there was already source code in the map for the given path + pub fn add_source_code(&mut self, path: String, source_code: String) -> bool { + let path_buf = Path::new(&path).to_path_buf(); + let old_value = self.0.insert(path_buf, source_code); + old_value.is_some() + } +} + pub enum CompileResult { Contract { contract: PreprocessedContract, debug: DebugArtifact }, Program { program: PreprocessedProgram, debug: DebugArtifact }, @@ -136,6 +161,7 @@ pub fn compile( entry_point: String, contracts: Option, dependency_graph: Option, + file_source_map: PathToFileSourceMap, ) -> Result { console_error_panic_hook::set_once(); @@ -154,7 +180,7 @@ pub fn compile( let path = Path::new(&entry_point); let crate_id = prepare_crate(&mut context, path); - process_dependency_graph(&mut context, dependency_graph); + process_dependency_graph(&mut context, dependency_graph, file_source_map); let compile_options = CompileOptions::default(); @@ -204,11 +230,20 @@ pub fn compile( // These will be in the Nargo.toml of the package being compiled. // // Library dependencies are transitive dependencies; for example, if the entry-point relies -// upon some library `lib1`. Then the packages that `lib1` depend upon will be placed in the +// upon some library `lib1`. Then the packages that `lib1` depend upon will be placed in the // `library_dependencies` list and the `lib1` will be placed in the `root_dependencies` list. -fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyGraph) { +fn process_dependency_graph( + context: &mut Context, + dependency_graph: DependencyGraph, + file_source_map: PathToFileSourceMap, +) { let mut crate_names: HashMap<&CrateName, CrateId> = HashMap::new(); + // Add all files from the file_source_map into the file manager + for (path, source) in file_source_map.0 { + context.file_manager.add_file_with_source(path.as_path(), source); + } + for lib in &dependency_graph.root_dependencies { let crate_id = add_noir_lib(context, lib); crate_names.insert(lib, crate_id); @@ -285,6 +320,14 @@ fn preprocess_contract(contract: CompiledContract) -> CompileResult { CompileResult::Contract { contract: preprocessed_contract, debug: debug_artifact } } +// TODO: This is no longer needed because we are passing in a map from every path +// TODO: to the source file. +// TODO: how things get resolved are now the responsibility of the caller +// TODO: We will have future PRs which make this resolution nicer by taking in a Nargo.toml +// TODO: and producing paths with source files, though for now, I think this API is okay +// +// TODO: We will also be able to remove the file_reader being a parameter to FileManager but +// TODO will stay until we have this working so we don't break the API too much. cfg_if::cfg_if! { if #[cfg(target_os = "wasi")] { fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result { @@ -318,6 +361,8 @@ mod test { hir::Context, }; + use crate::compile::PathToFileSourceMap; + use super::{process_dependency_graph, DependencyGraph}; use std::{collections::HashMap, path::Path}; @@ -345,7 +390,7 @@ mod test { let dependency_graph = DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() }; - process_dependency_graph(&mut context, dependency_graph); + process_dependency_graph(&mut context, dependency_graph, PathToFileSourceMap::default()); // one stdlib + one root crate assert_eq!(context.crate_graph.number_of_crates(), 2); @@ -359,7 +404,13 @@ mod test { library_dependencies: HashMap::new(), }; - process_dependency_graph(&mut context, dependency_graph); + let path_to_source_map = PathToFileSourceMap( + vec![(Path::new("/lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string())] + .into_iter() + .collect(), + ); + + process_dependency_graph(&mut context, dependency_graph, path_to_source_map); assert_eq!(context.crate_graph.number_of_crates(), 3); } @@ -372,7 +423,12 @@ mod test { library_dependencies: HashMap::new(), }; - process_dependency_graph(&mut context, dependency_graph); + let path_to_source_map = PathToFileSourceMap( + vec![(Path::new("lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string())] + .into_iter() + .collect(), + ); + process_dependency_graph(&mut context, dependency_graph, path_to_source_map); assert_eq!(context.crate_graph.number_of_crates(), 3); } @@ -388,7 +444,17 @@ mod test { ]), }; - process_dependency_graph(&mut context, dependency_graph); + let path_to_source_map = PathToFileSourceMap( + vec![ + (Path::new("lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string()), + (Path::new("lib2/lib.nr").to_path_buf(), "fn foo() {}".to_string()), + (Path::new("lib3/lib.nr").to_path_buf(), "fn foo() {}".to_string()), + ] + .into_iter() + .collect(), + ); + + process_dependency_graph(&mut context, dependency_graph, path_to_source_map); assert_eq!(context.crate_graph.number_of_crates(), 5); } @@ -401,7 +467,16 @@ mod test { library_dependencies: HashMap::from([(crate_name("lib2"), vec![crate_name("lib3")])]), }; - process_dependency_graph(&mut context, dependency_graph); + let path_to_source_map = PathToFileSourceMap( + vec![ + (Path::new("lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string()), + (Path::new("lib2/lib.nr").to_path_buf(), "fn foo() {}".to_string()), + (Path::new("lib3/lib.nr").to_path_buf(), "fn foo() {}".to_string()), + ] + .into_iter() + .collect(), + ); + process_dependency_graph(&mut context, dependency_graph, path_to_source_map); assert_eq!(context.crate_graph.number_of_crates(), 5); } From dd94b46bece6b94840db953dbe2e5937deb83754 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 01:07:40 +0000 Subject: [PATCH 22/33] remove extraneous forward slash --- compiler/wasm/src/compile.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index f4aea0874d3..afd44fd981b 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -405,7 +405,7 @@ mod test { }; let path_to_source_map = PathToFileSourceMap( - vec![(Path::new("/lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string())] + vec![(Path::new("lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string())] .into_iter() .collect(), ); From 3495de5976f6e006d728f341e5dd317e1b74093b Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 02:26:10 +0000 Subject: [PATCH 23/33] add file_manager_with_source_map method so we ensure that FileManager always has its data --- compiler/wasm/src/compile.rs | 80 +++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index afd44fd981b..715583f5db8 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -172,15 +172,15 @@ pub fn compile( DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() } }; - let root = Path::new("/"); - let fm = FileManager::new(root, Box::new(get_non_stdlib_asset)); + let fm = file_manager_with_source_map(file_source_map); + let graph = CrateGraph::default(); let mut context = Context::new(fm, graph); let path = Path::new(&entry_point); let crate_id = prepare_crate(&mut context, path); - process_dependency_graph(&mut context, dependency_graph, file_source_map); + process_dependency_graph(&mut context, dependency_graph); let compile_options = CompileOptions::default(); @@ -226,24 +226,34 @@ pub fn compile( } } +// Create a new FileManager with the given source map +// +// Note: Use this method whenever initializing a new FileManager +// to ensure that the file manager contains all of the files +// that one intends the compiler to use. +// +// For all intents and purposes, the file manager being returned +// should be considered as immutable. +fn file_manager_with_source_map(source_map: PathToFileSourceMap) -> FileManager { + let root = Path::new("/"); + let mut fm = FileManager::new(root, Box::new(get_non_stdlib_asset)); + + for (path, source) in source_map.0 { + fm.add_file_with_source(path.as_path(), source); + } + + fm +} + // Root dependencies are dependencies which the entry-point package relies upon. // These will be in the Nargo.toml of the package being compiled. // // Library dependencies are transitive dependencies; for example, if the entry-point relies // upon some library `lib1`. Then the packages that `lib1` depend upon will be placed in the // `library_dependencies` list and the `lib1` will be placed in the `root_dependencies` list. -fn process_dependency_graph( - context: &mut Context, - dependency_graph: DependencyGraph, - file_source_map: PathToFileSourceMap, -) { +fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyGraph) { let mut crate_names: HashMap<&CrateName, CrateId> = HashMap::new(); - // Add all files from the file_source_map into the file manager - for (path, source) in file_source_map.0 { - context.file_manager.add_file_with_source(path.as_path(), source); - } - for lib in &dependency_graph.root_dependencies { let crate_id = add_noir_lib(context, lib); crate_names.insert(lib, crate_id); @@ -354,7 +364,6 @@ cfg_if::cfg_if! { #[cfg(test)] mod test { - use fm::FileManager; use noirc_driver::prepare_crate; use noirc_frontend::{ graph::{CrateGraph, CrateName}, @@ -363,16 +372,18 @@ mod test { use crate::compile::PathToFileSourceMap; - use super::{process_dependency_graph, DependencyGraph}; + use super::{file_manager_with_source_map, process_dependency_graph, DependencyGraph}; use std::{collections::HashMap, path::Path}; fn mock_get_non_stdlib_asset(_path_to_file: &Path) -> std::io::Result { Ok("".to_string()) } - fn setup_test_context() -> Context { - let mut fm = FileManager::new(Path::new("/"), Box::new(mock_get_non_stdlib_asset)); - fm.add_file_with_source(Path::new("/main.nr"), "stuff".to_string()); + fn setup_test_context(source_map: PathToFileSourceMap) -> Context { + let mut fm = file_manager_with_source_map(source_map); + // Add this due to us calling prepare_crate on "/main.nr" below + fm.add_file_with_source(Path::new("/main.nr"), "fn foo() {}".to_string()); + let graph = CrateGraph::default(); let mut context = Context::new(fm, graph); prepare_crate(&mut context, Path::new("/main.nr")); @@ -386,11 +397,13 @@ mod test { #[test] fn test_works_with_empty_dependency_graph() { - let mut context = setup_test_context(); let dependency_graph = DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() }; - process_dependency_graph(&mut context, dependency_graph, PathToFileSourceMap::default()); + let source_map = PathToFileSourceMap::default(); + let mut context = setup_test_context(source_map); + + process_dependency_graph(&mut context, dependency_graph); // one stdlib + one root crate assert_eq!(context.crate_graph.number_of_crates(), 2); @@ -398,44 +411,45 @@ mod test { #[test] fn test_works_with_root_dependencies() { - let mut context = setup_test_context(); let dependency_graph = DependencyGraph { root_dependencies: vec![crate_name("lib1")], library_dependencies: HashMap::new(), }; - let path_to_source_map = PathToFileSourceMap( + let source_map = PathToFileSourceMap( vec![(Path::new("lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string())] .into_iter() .collect(), ); - process_dependency_graph(&mut context, dependency_graph, path_to_source_map); + let mut context = setup_test_context(source_map); + + process_dependency_graph(&mut context, dependency_graph); assert_eq!(context.crate_graph.number_of_crates(), 3); } #[test] fn test_works_with_duplicate_root_dependencies() { - let mut context = setup_test_context(); let dependency_graph = DependencyGraph { root_dependencies: vec![crate_name("lib1"), crate_name("lib1")], library_dependencies: HashMap::new(), }; - let path_to_source_map = PathToFileSourceMap( + let source_map = PathToFileSourceMap( vec![(Path::new("lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string())] .into_iter() .collect(), ); - process_dependency_graph(&mut context, dependency_graph, path_to_source_map); + let mut context = setup_test_context(source_map); + + process_dependency_graph(&mut context, dependency_graph); assert_eq!(context.crate_graph.number_of_crates(), 3); } #[test] fn test_works_with_transitive_dependencies() { - let mut context = setup_test_context(); let dependency_graph = DependencyGraph { root_dependencies: vec![crate_name("lib1")], library_dependencies: HashMap::from([ @@ -444,7 +458,7 @@ mod test { ]), }; - let path_to_source_map = PathToFileSourceMap( + let source_map = PathToFileSourceMap( vec![ (Path::new("lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string()), (Path::new("lib2/lib.nr").to_path_buf(), "fn foo() {}".to_string()), @@ -454,20 +468,20 @@ mod test { .collect(), ); - process_dependency_graph(&mut context, dependency_graph, path_to_source_map); + let mut context = setup_test_context(source_map); + process_dependency_graph(&mut context, dependency_graph); assert_eq!(context.crate_graph.number_of_crates(), 5); } #[test] fn test_works_with_missing_dependencies() { - let mut context = setup_test_context(); let dependency_graph = DependencyGraph { root_dependencies: vec![crate_name("lib1")], library_dependencies: HashMap::from([(crate_name("lib2"), vec![crate_name("lib3")])]), }; - let path_to_source_map = PathToFileSourceMap( + let source_map = PathToFileSourceMap( vec![ (Path::new("lib1/lib.nr").to_path_buf(), "fn foo() {}".to_string()), (Path::new("lib2/lib.nr").to_path_buf(), "fn foo() {}".to_string()), @@ -476,7 +490,9 @@ mod test { .into_iter() .collect(), ); - process_dependency_graph(&mut context, dependency_graph, path_to_source_map); + + let mut context = setup_test_context(source_map); + process_dependency_graph(&mut context, dependency_graph); assert_eq!(context.crate_graph.number_of_crates(), 5); } From 851fab19180b848adcb19364dc73b208b2e52ad7 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 02:26:54 +0000 Subject: [PATCH 24/33] add expect --- compiler/noirc_driver/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index ba0e12cd7e7..08763b69725 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -89,7 +89,7 @@ pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { let std_crate_id = context.crate_graph.add_stdlib(std_file_id); - let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).unwrap(); + let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).unwrap_or_else(|| panic!("files are expected to be added to the FileManager before reaching the compiler file_path: {file_name:?}")); let root_crate_id = context.crate_graph.add_crate_root(root_file_id); From 3cd35fd39948e9fb6e1655e8aa060a4132fe487a Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 02:27:11 +0000 Subject: [PATCH 25/33] fix node tests --- compiler/wasm/test/node/index.test.ts | 44 +++++++++++++-------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/compiler/wasm/test/node/index.test.ts b/compiler/wasm/test/node/index.test.ts index c0d5f88e407..404ba85d331 100644 --- a/compiler/wasm/test/node/index.test.ts +++ b/compiler/wasm/test/node/index.test.ts @@ -9,8 +9,7 @@ import { } from '../shared'; import { readFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; -import { compile } from '@noir-lang/noir_wasm'; -import { initializeResolver } from '@noir-lang/source-resolver'; +import { compile, PathToFileSourceMap } from '@noir-lang/noir_wasm'; // eslint-disable-next-line @typescript-eslint/no-explicit-any async function getPrecompiledSource(path: string): Promise { @@ -21,7 +20,12 @@ async function getPrecompiledSource(path: string): Promise { describe('noir wasm compilation', () => { describe('can compile simple scripts', () => { it('matching nargos compilation', async () => { - const wasmCircuit = await compile(join(__dirname, simpleScriptSourcePath)); + const sourceFileMap = new PathToFileSourceMap(); + sourceFileMap.add_source_code( + join(__dirname, simpleScriptSourcePath), + readFileSync(join(__dirname, simpleScriptSourcePath), 'utf-8'), + ); + const wasmCircuit = await compile(join(__dirname, simpleScriptSourcePath), undefined, undefined, sourceFileMap); const cliCircuit = await getPrecompiledSource(simpleScriptExpectedArtifact); if (!('program' in wasmCircuit)) { @@ -36,32 +40,28 @@ describe('noir wasm compilation', () => { }); describe('can compile scripts with dependencies', () => { + let map: PathToFileSourceMap; beforeEach(() => { // this test requires a custom resolver in order to correctly resolve dependencies - initializeResolver((file) => { - switch (file) { - case '/script/main.nr': - return readFileSync(join(__dirname, depsScriptSourcePath), 'utf-8'); - case '/lib_a/lib.nr': - return readFileSync(join(__dirname, libASourcePath), 'utf-8'); - - case '/lib_b/lib.nr': - return readFileSync(join(__dirname, libBSourcePath), 'utf-8'); - - default: - return ''; - } - }); + map = new PathToFileSourceMap(); + map.add_source_code('script/main.nr', readFileSync(join(__dirname, depsScriptSourcePath), 'utf-8')); + map.add_source_code('lib_a/lib.nr', readFileSync(join(__dirname, libASourcePath), 'utf-8')); + map.add_source_code('lib_b/lib.nr', readFileSync(join(__dirname, libBSourcePath), 'utf-8')); }); it('matching nargos compilation', async () => { - const wasmCircuit = await compile('/script/main.nr', false, { - root_dependencies: ['lib_a'], - library_dependencies: { - lib_a: ['lib_b'], + const wasmCircuit = await compile( + 'script/main.nr', + false, + { + root_dependencies: ['lib_a'], + library_dependencies: { + lib_a: ['lib_b'], + }, }, - }); + map, + ); const cliCircuit = await getPrecompiledSource(depsScriptExpectedArtifact); From d2590d5b7c1c9abd69136e2ec938bfb08c52a862 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 02:35:09 +0000 Subject: [PATCH 26/33] fix browser tests and naming nit --- .../test/browser/compile_prove_verify.test.ts | 21 +++---- .../test/browser/recursion.test.ts | 19 +++--- .../onchain_recursive_verification.test.ts | 33 +++++++--- .../test/node/smart_contract_verifier.test.ts | 8 ++- compiler/wasm/test/browser/index.test.ts | 63 +++++++------------ compiler/wasm/test/node/index.test.ts | 19 +++--- 6 files changed, 75 insertions(+), 88 deletions(-) diff --git a/compiler/integration-tests/test/browser/compile_prove_verify.test.ts b/compiler/integration-tests/test/browser/compile_prove_verify.test.ts index 2aef56c23f9..29e2fbc55b8 100644 --- a/compiler/integration-tests/test/browser/compile_prove_verify.test.ts +++ b/compiler/integration-tests/test/browser/compile_prove_verify.test.ts @@ -1,17 +1,17 @@ import { expect } from '@esm-bundle/chai'; -import { Logger } from 'tslog'; import * as TOML from 'smol-toml'; -import { initializeResolver } from '@noir-lang/source-resolver'; -import newCompiler, { CompiledProgram, compile, init_log_level as compilerLogLevel } from '@noir-lang/noir_wasm'; +import newCompiler, { + CompiledProgram, + PathToFileSourceMap, + compile, + init_log_level as compilerLogLevel, +} from '@noir-lang/noir_wasm'; import { Noir } from '@noir-lang/noir_js'; import { InputMap } from '@noir-lang/noirc_abi'; import { BarretenbergBackend } from '@noir-lang/backend_barretenberg'; import { getFile } from './utils.js'; -import { TEST_LOG_LEVEL } from '../environment.js'; - -const logger = new Logger({ name: 'test', minLevel: TEST_LOG_LEVEL }); await newCompiler(); @@ -33,14 +33,11 @@ const suite = Mocha.Suite.create(mocha.suite, 'Noir end to end test'); suite.timeout(60 * 20e3); //20mins function getCircuit(noirSource: string): CompiledProgram { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - initializeResolver((id: string) => { - logger.debug('source-resolver: resolving:', id); - return noirSource; - }); + const sourceMap = new PathToFileSourceMap(); + sourceMap.add_source_code('main.nr', noirSource); // We're ignoring this in the resolver but pass in something sensible. - const result = compile('/main.nr'); + const result = compile('main.nr', undefined, undefined, sourceMap); if (!('program' in result)) { throw new Error('Compilation failed'); } diff --git a/compiler/integration-tests/test/browser/recursion.test.ts b/compiler/integration-tests/test/browser/recursion.test.ts index 308be81417f..faa317b2c3c 100644 --- a/compiler/integration-tests/test/browser/recursion.test.ts +++ b/compiler/integration-tests/test/browser/recursion.test.ts @@ -2,8 +2,12 @@ import { expect } from '@esm-bundle/chai'; import { TEST_LOG_LEVEL } from '../environment.js'; import { Logger } from 'tslog'; -import { initializeResolver } from '@noir-lang/source-resolver'; -import newCompiler, { CompiledProgram, compile, init_log_level as compilerLogLevel } from '@noir-lang/noir_wasm'; +import newCompiler, { + CompiledProgram, + PathToFileSourceMap, + compile, + init_log_level as compilerLogLevel, +} from '@noir-lang/noir_wasm'; import { acvm, abi, Noir } from '@noir-lang/noir_js'; import * as TOML from 'smol-toml'; @@ -27,14 +31,9 @@ const circuit_main = 'test_programs/execution_success/assert_statement'; const circuit_recursion = 'compiler/integration-tests/circuits/recursion'; function getCircuit(noirSource: string): CompiledProgram { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - initializeResolver((id: string) => { - logger.debug('source-resolver: resolving:', id); - return noirSource; - }); - - // We're ignoring this in the resolver but pass in something sensible. - const result = compile('/main.nr'); + const sourceMap = new PathToFileSourceMap(); + sourceMap.add_source_code('main.nr', noirSource); + const result = compile('main.nr', undefined, undefined, sourceMap); if (!('program' in result)) { throw new Error('Compilation failed'); } diff --git a/compiler/integration-tests/test/node/onchain_recursive_verification.test.ts b/compiler/integration-tests/test/node/onchain_recursive_verification.test.ts index 353678b470b..6c20d44882b 100644 --- a/compiler/integration-tests/test/node/onchain_recursive_verification.test.ts +++ b/compiler/integration-tests/test/node/onchain_recursive_verification.test.ts @@ -5,7 +5,12 @@ import { readFileSync } from 'node:fs'; import { resolve } from 'path'; import toml from 'toml'; -import { compile, CompiledProgram, init_log_level as compilerLogLevel } from '@noir-lang/noir_wasm'; +import { + compile, + CompiledProgram, + init_log_level as compilerLogLevel, + PathToFileSourceMap, +} from '@noir-lang/noir_wasm'; import { Noir } from '@noir-lang/noir_js'; import { BarretenbergBackend, flattenPublicInputs } from '@noir-lang/backend_barretenberg'; import { Field, InputMap } from '@noir-lang/noirc_abi'; @@ -13,16 +18,24 @@ import { Field, InputMap } from '@noir-lang/noirc_abi'; compilerLogLevel('INFO'); it(`smart contract can verify a recursive proof`, async () => { - const inner_source_path = resolve(`../../test_programs/execution_success/assert_statement/src/main.nr`); - const inner_program = (compile(inner_source_path) as { program: CompiledProgram }).program; - - const recursion_source_path = resolve(`./circuits/recursion/src/main.nr`); - const recursion_program = (compile(recursion_source_path) as { program: CompiledProgram }).program; + const innerSourcePath = resolve(`../../test_programs/execution_success/assert_statement/src/main.nr`); + const sourceMapInnerProgram = new PathToFileSourceMap(); + sourceMapInnerProgram.add_source_code(innerSourcePath, readFileSync(innerSourcePath, 'utf-8')); + const innerProgram = ( + compile(innerSourcePath, undefined, undefined, sourceMapInnerProgram) as { program: CompiledProgram } + ).program; + + const recursionSourcePath = resolve(`./circuits/recursion/src/main.nr`); + const sourceMapRecursionProgram = new PathToFileSourceMap(); + sourceMapRecursionProgram.add_source_code(recursionSourcePath, readFileSync(recursionSourcePath, 'utf-8')); + const recursionProgram = ( + compile(recursionSourcePath, undefined, undefined, sourceMapRecursionProgram) as { program: CompiledProgram } + ).program; // Intermediate proof - const inner_backend = new BarretenbergBackend(inner_program); - const inner = new Noir(inner_program); + const inner_backend = new BarretenbergBackend(innerProgram); + const inner = new Noir(innerProgram); const inner_prover_toml = readFileSync( resolve(`../../test_programs/execution_success/assert_statement/Prover.toml`), @@ -41,8 +54,8 @@ it(`smart contract can verify a recursive proof`, async () => { // Final proof - const recursion_backend = new BarretenbergBackend(recursion_program); - const recursion = new Noir(recursion_program, recursion_backend); + const recursion_backend = new BarretenbergBackend(recursionProgram); + const recursion = new Noir(recursionProgram, recursion_backend); const recursion_inputs: InputMap = { verification_key: vkAsFields, diff --git a/compiler/integration-tests/test/node/smart_contract_verifier.test.ts b/compiler/integration-tests/test/node/smart_contract_verifier.test.ts index 7dafada0ffb..5b3d0e2d337 100644 --- a/compiler/integration-tests/test/node/smart_contract_verifier.test.ts +++ b/compiler/integration-tests/test/node/smart_contract_verifier.test.ts @@ -5,7 +5,7 @@ import { readFileSync } from 'node:fs'; import { resolve } from 'path'; import toml from 'toml'; -import { compile, init_log_level as compilerLogLevel } from '@noir-lang/noir_wasm'; +import { PathToFileSourceMap, compile, init_log_level as compilerLogLevel } from '@noir-lang/noir_wasm'; import { Noir } from '@noir-lang/noir_js'; import { BarretenbergBackend, flattenPublicInputs } from '@noir-lang/backend_barretenberg'; @@ -31,9 +31,11 @@ test_cases.forEach((testInfo) => { const base_relative_path = '../..'; const test_case = testInfo.case; - const noir_source_path = resolve(`${base_relative_path}/${test_case}/src/main.nr`); + const noirSourcePath = resolve(`${base_relative_path}/${test_case}/src/main.nr`); + const sourceMap = new PathToFileSourceMap(); + sourceMap.add_source_code(noirSourcePath, readFileSync(noirSourcePath, 'utf-8')); - const compileResult = compile(noir_source_path); + const compileResult = compile(noirSourcePath, undefined, undefined, sourceMap); if (!('program' in compileResult)) { throw new Error('Compilation failed'); } diff --git a/compiler/wasm/test/browser/index.test.ts b/compiler/wasm/test/browser/index.test.ts index 8a3f82ffff9..346c20c834c 100644 --- a/compiler/wasm/test/browser/index.test.ts +++ b/compiler/wasm/test/browser/index.test.ts @@ -1,6 +1,5 @@ import { expect } from '@esm-bundle/chai'; -import initNoirWasm, { compile } from '@noir-lang/noir_wasm'; -import { initializeResolver } from '@noir-lang/source-resolver'; +import initNoirWasm, { PathToFileSourceMap, compile } from '@noir-lang/noir_wasm'; import { depsScriptExpectedArtifact, depsScriptSourcePath, @@ -28,23 +27,11 @@ async function getPrecompiledSource(path: string): Promise { describe('noir wasm', () => { describe('can compile script without dependencies', () => { - beforeEach(async () => { - const source = await getFileContent(simpleScriptSourcePath); - initializeResolver((id: string) => { - console.log(`Resolving source ${id}`); - - if (typeof source === 'undefined') { - throw Error(`Could not resolve source for '${id}'`); - } else if (id !== '/main.nr') { - throw Error(`Unexpected id: '${id}'`); - } else { - return source; - } - }); - }); - it('matching nargos compilation', async () => { - const wasmCircuit = await compile('/main.nr'); + const sourceMap = new PathToFileSourceMap(); + sourceMap.add_source_code('main.nr', await getFileContent(simpleScriptSourcePath)); + + const wasmCircuit = await compile('main.nr', undefined, undefined, sourceMap); const cliCircuit = await getPrecompiledSource(simpleScriptExpectedArtifact); if (!('program' in wasmCircuit)) { @@ -59,37 +46,29 @@ describe('noir wasm', () => { }); describe('can compile script with dependencies', () => { - beforeEach(async () => { + it('matching nargos compilation', async () => { const [scriptSource, libASource, libBSource] = await Promise.all([ getFileContent(depsScriptSourcePath), getFileContent(libASourcePath), getFileContent(libBSourcePath), ]); - initializeResolver((file: string) => { - switch (file) { - case '/script/main.nr': - return scriptSource; - - case '/lib_a/lib.nr': - return libASource; - - case '/lib_b/lib.nr': - return libBSource; - - default: - return ''; - } - }); - }); - - it('matching nargos compilation', async () => { - const wasmCircuit = await compile('/script/main.nr', false, { - root_dependencies: ['lib_a'], - library_dependencies: { - lib_a: ['lib_b'], + const sourceMap = new PathToFileSourceMap(); + sourceMap.add_source_code('script/main.nr', scriptSource); + sourceMap.add_source_code('lib_a/lib.nr', libASource); + sourceMap.add_source_code('lib_b/lib.nr', libBSource); + + const wasmCircuit = await compile( + 'script/main.nr', + false, + { + root_dependencies: ['lib_a'], + library_dependencies: { + lib_a: ['lib_b'], + }, }, - }); + sourceMap, + ); if (!('program' in wasmCircuit)) { throw Error('Expected program to be present'); diff --git a/compiler/wasm/test/node/index.test.ts b/compiler/wasm/test/node/index.test.ts index 404ba85d331..5cf9e3be2df 100644 --- a/compiler/wasm/test/node/index.test.ts +++ b/compiler/wasm/test/node/index.test.ts @@ -20,12 +20,12 @@ async function getPrecompiledSource(path: string): Promise { describe('noir wasm compilation', () => { describe('can compile simple scripts', () => { it('matching nargos compilation', async () => { - const sourceFileMap = new PathToFileSourceMap(); - sourceFileMap.add_source_code( + const sourceMap = new PathToFileSourceMap(); + sourceMap.add_source_code( join(__dirname, simpleScriptSourcePath), readFileSync(join(__dirname, simpleScriptSourcePath), 'utf-8'), ); - const wasmCircuit = await compile(join(__dirname, simpleScriptSourcePath), undefined, undefined, sourceFileMap); + const wasmCircuit = await compile(join(__dirname, simpleScriptSourcePath), undefined, undefined, sourceMap); const cliCircuit = await getPrecompiledSource(simpleScriptExpectedArtifact); if (!('program' in wasmCircuit)) { @@ -40,14 +40,11 @@ describe('noir wasm compilation', () => { }); describe('can compile scripts with dependencies', () => { - let map: PathToFileSourceMap; + const sourceMap: PathToFileSourceMap = new PathToFileSourceMap(); beforeEach(() => { - // this test requires a custom resolver in order to correctly resolve dependencies - - map = new PathToFileSourceMap(); - map.add_source_code('script/main.nr', readFileSync(join(__dirname, depsScriptSourcePath), 'utf-8')); - map.add_source_code('lib_a/lib.nr', readFileSync(join(__dirname, libASourcePath), 'utf-8')); - map.add_source_code('lib_b/lib.nr', readFileSync(join(__dirname, libBSourcePath), 'utf-8')); + sourceMap.add_source_code('script/main.nr', readFileSync(join(__dirname, depsScriptSourcePath), 'utf-8')); + sourceMap.add_source_code('lib_a/lib.nr', readFileSync(join(__dirname, libASourcePath), 'utf-8')); + sourceMap.add_source_code('lib_b/lib.nr', readFileSync(join(__dirname, libBSourcePath), 'utf-8')); }); it('matching nargos compilation', async () => { @@ -60,7 +57,7 @@ describe('noir wasm compilation', () => { lib_a: ['lib_b'], }, }, - map, + sourceMap, ); const cliCircuit = await getPrecompiledSource(depsScriptExpectedArtifact); From 5b867d531eee2f950fef67512ffb347a67fc236b Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 17:38:47 +0000 Subject: [PATCH 27/33] chore!: Remove `add_file` and `file_reader` from FileManager (#3762) # Description This removes FileManager from add_file and uses add_file_with_source ## Problem\* Resolves ## Summary\* ## Additional Context ## Documentation\* Check one: - [ ] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [ ] I have tested the changes locally. - [ ] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- Cargo.lock | 3 - compiler/fm/Cargo.toml | 1 - compiler/fm/src/file_reader.rs | 54 --------------- compiler/fm/src/lib.rs | 65 +++++++------------ compiler/noirc_driver/src/lib.rs | 10 +-- compiler/noirc_frontend/src/tests.rs | 3 +- compiler/wasm/Cargo.toml | 1 - compiler/wasm/src/compile.rs | 40 +----------- tooling/lsp/Cargo.toml | 1 - tooling/lsp/src/lib.rs | 28 +------- tooling/lsp/src/notifications/mod.rs | 4 +- tooling/lsp/src/requests/code_lens_request.rs | 4 +- tooling/lsp/src/requests/goto_definition.rs | 3 +- tooling/lsp/src/requests/test_run.rs | 3 +- tooling/lsp/src/requests/tests.rs | 4 +- tooling/nargo/src/lib.rs | 10 +-- tooling/nargo/src/ops/compile.rs | 6 +- tooling/nargo_cli/src/cli/check_cmd.rs | 3 +- tooling/nargo_cli/src/cli/compile_cmd.rs | 6 +- tooling/nargo_cli/src/cli/fmt_cmd.rs | 3 +- tooling/nargo_cli/src/cli/test_cmd.rs | 3 +- 21 files changed, 54 insertions(+), 201 deletions(-) delete mode 100644 compiler/fm/src/file_reader.rs diff --git a/Cargo.lock b/Cargo.lock index 31a6c3932df..220eb803c55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1608,7 +1608,6 @@ version = "0.20.0" dependencies = [ "codespan-reporting", "iter-extended", - "rust-embed", "serde", "tempfile", ] @@ -2537,7 +2536,6 @@ version = "0.20.0" dependencies = [ "acvm", "async-lsp", - "cfg-if", "codespan-lsp", "codespan-reporting", "fm", @@ -2562,7 +2560,6 @@ version = "0.20.0" dependencies = [ "acvm", "build-data", - "cfg-if", "console_error_panic_hook", "fm", "getrandom", diff --git a/compiler/fm/Cargo.toml b/compiler/fm/Cargo.toml index 9e4309693ed..699f709e9b5 100644 --- a/compiler/fm/Cargo.toml +++ b/compiler/fm/Cargo.toml @@ -9,7 +9,6 @@ license.workspace = true [dependencies] codespan-reporting.workspace = true -rust-embed = "6.6.0" serde.workspace = true [dev-dependencies] diff --git a/compiler/fm/src/file_reader.rs b/compiler/fm/src/file_reader.rs deleted file mode 100644 index e7ffd93df03..00000000000 --- a/compiler/fm/src/file_reader.rs +++ /dev/null @@ -1,54 +0,0 @@ -use rust_embed::RustEmbed; -use std::io::{Error, ErrorKind}; -use std::path::Path; - -// Based on the environment, we either read files using the rust standard library or we -// read files using the javascript host function - -// TODO: DO NOT MERGE PR WITH THIS TODO -// TODO: We have duplicated this logic in noirc_driver. For now, we leave this here -// TODO: until we make the breaking change to the API which will allow us to remove this -// TODO: file. - -pub type FileReader = dyn Fn(&Path) -> std::io::Result + Send; - -#[derive(RustEmbed)] -#[folder = "../../noir_stdlib/src"] -#[cfg_attr(not(target_os = "windows"), prefix = "std/")] -#[cfg_attr(target_os = "windows", prefix = r"std\")] // Note reversed slash direction -struct StdLibAssets; - -#[cfg(target_os = "windows")] -pub(super) fn is_stdlib_asset(path: &Path) -> bool { - path.starts_with("std\\") -} - -#[cfg(not(target_os = "windows"))] -pub(super) fn is_stdlib_asset(path: &Path) -> bool { - path.starts_with("std/") -} - -fn get_stdlib_asset(path: &Path) -> std::io::Result { - if !is_stdlib_asset(path) { - return Err(Error::new(ErrorKind::InvalidInput, "requested a non-stdlib asset")); - } - - match StdLibAssets::get(path.to_str().unwrap()) { - Some(std_lib_asset) => { - Ok(std::str::from_utf8(std_lib_asset.data.as_ref()).unwrap().to_string()) - } - - None => Err(Error::new(ErrorKind::NotFound, "invalid stdlib path")), - } -} - -pub(crate) fn read_file_to_string( - path_to_file: &Path, - get_non_stdlib_asset: &impl Fn(&Path) -> std::io::Result, -) -> std::io::Result { - if is_stdlib_asset(path_to_file) { - get_stdlib_asset(path_to_file) - } else { - get_non_stdlib_asset(path_to_file) - } -} diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index 4f4eb9d9838..3f1f5e09acc 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -4,11 +4,8 @@ #![warn(clippy::semicolon_if_nothing_returned)] mod file_map; -mod file_reader; pub use file_map::{File, FileId, FileMap, PathString}; -use file_reader::is_stdlib_asset; -pub use file_reader::FileReader; use std::{ collections::HashMap, @@ -22,7 +19,6 @@ pub struct FileManager { file_map: file_map::FileMap, id_to_path: HashMap, path_to_id: HashMap, - file_reader: Box, } impl std::fmt::Debug for FileManager { @@ -37,13 +33,12 @@ impl std::fmt::Debug for FileManager { } impl FileManager { - pub fn new(root: &Path, file_reader: Box) -> Self { + pub fn new(root: &Path) -> Self { Self { root: root.normalize(), file_map: Default::default(), id_to_path: Default::default(), path_to_id: Default::default(), - file_reader, } } @@ -51,34 +46,19 @@ impl FileManager { &self.file_map } - pub fn add_file(&mut self, file_name: &Path) -> Option { - // Handle both relative file paths and std/lib virtual paths. - let resolved_path: PathBuf = if is_stdlib_asset(file_name) { - // Special case for stdlib where we want to read specifically the `std/` relative path - // TODO: The stdlib path should probably be an absolute path rooted in something people would never create - file_name.to_path_buf() - } else { - self.root.join(file_name).normalize() - }; - - // Check that the resolved path already exists in the file map, if it is, we return it. - if let Some(file_id) = self.path_to_id.get(&resolved_path) { - return Some(*file_id); - } - - // Otherwise we add the file - let source = file_reader::read_file_to_string(&resolved_path, &self.file_reader).ok()?; - let file_id = self.file_map.add_file(resolved_path.clone().into(), source); - self.register_path(file_id, resolved_path); - Some(file_id) + pub fn add_file_with_source(&mut self, file_name: &Path, source: String) -> Option { + let file_name = self.root.join(file_name).normalize(); + self.add_file_with_source_canonical_path(&file_name, source) } - // TODO: this will become the default strategy for adding files. Possibly with file_reader. - // TODO: we are still migrating to this strategy, so we keep the old one for now. - // TODO: For the stdlib crate, we need to do this preemptively due to the way we special handle it - pub fn add_file_with_source(&mut self, file_name: &Path, source: String) -> Option { + pub fn add_file_with_source_canonical_path( + &mut self, + file_name: &Path, + source: String, + ) -> Option { + let file_name = file_name.normalize(); // Check that the file name already exists in the file map, if it is, we return it. - if let Some(file_id) = self.path_to_id.get(file_name) { + if let Some(file_id) = self.path_to_id.get(&file_name) { return Some(*file_id); } let file_name_path_buf = file_name.to_path_buf(); @@ -254,9 +234,9 @@ mod tests { let entry_file_name = Path::new("my_dummy_file.nr"); create_dummy_file(&dir, entry_file_name); - let mut fm = FileManager::new(dir.path(), Box::new(|path| std::fs::read_to_string(path))); + let mut fm = FileManager::new(dir.path()); - let file_id = fm.add_file(entry_file_name).unwrap(); + let file_id = fm.add_file_with_source(entry_file_name, "fn foo() {}".to_string()).unwrap(); let dep_file_name = Path::new("foo.nr"); create_dummy_file(&dir, dep_file_name); @@ -269,9 +249,9 @@ mod tests { let file_name = Path::new("foo.nr"); create_dummy_file(&dir, file_name); - let mut fm = FileManager::new(dir.path(), Box::new(|path| std::fs::read_to_string(path))); + let mut fm = FileManager::new(dir.path()); - let file_id = fm.add_file(file_name).unwrap(); + let file_id = fm.add_file_with_source(file_name, "fn foo() {}".to_string()).unwrap(); assert!(fm.path(file_id).ends_with("foo.nr")); } @@ -279,13 +259,13 @@ mod tests { #[test] fn path_resolve_sub_module() { let dir = tempdir().unwrap(); - let mut fm = FileManager::new(dir.path(), Box::new(|path| std::fs::read_to_string(path))); + let mut fm = FileManager::new(dir.path()); // Create a lib.nr file at the root. // we now have dir/lib.nr let lib_nr_path = create_dummy_file(&dir, Path::new("lib.nr")); let file_id = fm - .add_file(lib_nr_path.as_path()) + .add_file_with_source(lib_nr_path.as_path(), "fn foo() {}".to_string()) .expect("could not add file to file manager and obtain a FileId"); // Create a sub directory @@ -300,7 +280,7 @@ mod tests { // - dir/lib.nr // - dir/sub_dir/foo.nr let foo_nr_path = create_dummy_file(&sub_dir, Path::new("foo.nr")); - fm.add_file(foo_nr_path.as_path()); + fm.add_file_with_source(foo_nr_path.as_path(), "fn foo() {}".to_string()); // Add a parent module for the sub_dir // we no have: @@ -308,7 +288,7 @@ mod tests { // - dir/sub_dir.nr // - dir/sub_dir/foo.nr let sub_dir_nr_path = create_dummy_file(&dir, Path::new(&format!("{sub_dir_name}.nr"))); - fm.add_file(sub_dir_nr_path.as_path()); + fm.add_file_with_source(sub_dir_nr_path.as_path(), "fn foo() {}".to_string()); // First check for the sub_dir.nr file and add it to the FileManager let sub_dir_file_id = fm.find_module(file_id, sub_dir_name).unwrap(); @@ -327,7 +307,7 @@ mod tests { let sub_dir = TempDir::new_in(&dir).unwrap(); let sub_sub_dir = TempDir::new_in(&sub_dir).unwrap(); - let mut fm = FileManager::new(dir.path(), Box::new(|path| std::fs::read_to_string(path))); + let mut fm = FileManager::new(dir.path()); // Create a lib.nr file at the root. let file_name = Path::new("lib.nr"); @@ -337,8 +317,9 @@ mod tests { let second_file_name = PathBuf::from(sub_sub_dir.path()).join("./../../lib.nr"); // Add both files to the file manager - let file_id = fm.add_file(file_name).unwrap(); - let second_file_id = fm.add_file(&second_file_name).unwrap(); + let file_id = fm.add_file_with_source(file_name, "fn foo() {}".to_string()).unwrap(); + let second_file_id = + fm.add_file_with_source(&second_file_name, "fn foo() {}".to_string()).unwrap(); assert_eq!(file_id, second_file_id); } diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 08763b69725..3fb164472e4 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -75,18 +75,20 @@ pub type CompilationResult = Result<(T, Warnings), ErrorsAndWarnings>; /// Adds the file from the file system at `Path` to the crate graph as a root file pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId { - let path_to_std_lib_file = Path::new(STD_CRATE_NAME).join("lib.nr"); - let std_file_id = context.file_manager.add_file(&path_to_std_lib_file).unwrap(); - // Add the stdlib contents to the file manager, since every package automatically has a dependency // on the stdlib. For other dependencies, we read the package.Dependencies file to add their file // contents to the file manager. However since the dependency on the stdlib is implicit, we need // to manually add it here. let stdlib_paths_with_source = stdlib::stdlib_paths_with_source(); for (path, source) in stdlib_paths_with_source { - context.file_manager.add_file_with_source(Path::new(&path), source); + context.file_manager.add_file_with_source_canonical_path(Path::new(&path), source); } + let path_to_std_lib_file = Path::new(STD_CRATE_NAME).join("lib.nr"); + let std_file_id = context + .file_manager + .name_to_id(path_to_std_lib_file) + .expect("stdlib file id is expected to be present"); let std_crate_id = context.crate_graph.add_stdlib(std_file_id); let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).unwrap_or_else(|| panic!("files are expected to be added to the FileManager before reaching the compiler file_path: {file_name:?}")); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 13ce71c4616..16db445df6d 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -59,8 +59,7 @@ mod test { src: &str, ) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) { let root = std::path::Path::new("/"); - let fm = FileManager::new(root, Box::new(|path| std::fs::read_to_string(path))); - //let fm = FileManager::new(root, Box::new(get_non_stdlib_asset)); + let fm = FileManager::new(root); let graph = CrateGraph::default(); let mut context = Context::new(fm, graph); let root_file_id = FileId::dummy(); diff --git a/compiler/wasm/Cargo.toml b/compiler/wasm/Cargo.toml index 9ece26c6df4..6c7a8f26ac4 100644 --- a/compiler/wasm/Cargo.toml +++ b/compiler/wasm/Cargo.toml @@ -21,7 +21,6 @@ noirc_errors.workspace = true wasm-bindgen.workspace = true serde.workspace = true js-sys.workspace = true -cfg-if.workspace = true console_error_panic_hook.workspace = true gloo-utils.workspace = true diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 715583f5db8..13b366819b0 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -235,8 +235,8 @@ pub fn compile( // For all intents and purposes, the file manager being returned // should be considered as immutable. fn file_manager_with_source_map(source_map: PathToFileSourceMap) -> FileManager { - let root = Path::new("/"); - let mut fm = FileManager::new(root, Box::new(get_non_stdlib_asset)); + let root = Path::new(""); + let mut fm = FileManager::new(root); for (path, source) in source_map.0 { fm.add_file_with_source(path.as_path(), source); @@ -330,38 +330,6 @@ fn preprocess_contract(contract: CompiledContract) -> CompileResult { CompileResult::Contract { contract: preprocessed_contract, debug: debug_artifact } } -// TODO: This is no longer needed because we are passing in a map from every path -// TODO: to the source file. -// TODO: how things get resolved are now the responsibility of the caller -// TODO: We will have future PRs which make this resolution nicer by taking in a Nargo.toml -// TODO: and producing paths with source files, though for now, I think this API is okay -// -// TODO: We will also be able to remove the file_reader being a parameter to FileManager but -// TODO will stay until we have this working so we don't break the API too much. -cfg_if::cfg_if! { - if #[cfg(target_os = "wasi")] { - fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result { - std::fs::read_to_string(path_to_file) - } - } else { - use std::io::{Error, ErrorKind}; - - #[wasm_bindgen(module = "@noir-lang/source-resolver")] - extern "C" { - #[wasm_bindgen(catch)] - fn read_file(path: &str) -> Result; - } - - fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result { - let path_str = path_to_file.to_str().unwrap(); - match read_file(path_str) { - Ok(buffer) => Ok(buffer), - Err(_) => Err(Error::new(ErrorKind::Other, "could not read file using wasm")), - } - } - } -} - #[cfg(test)] mod test { use noirc_driver::prepare_crate; @@ -375,10 +343,6 @@ mod test { use super::{file_manager_with_source_map, process_dependency_graph, DependencyGraph}; use std::{collections::HashMap, path::Path}; - fn mock_get_non_stdlib_asset(_path_to_file: &Path) -> std::io::Result { - Ok("".to_string()) - } - fn setup_test_context(source_map: PathToFileSourceMap) -> Context { let mut fm = file_manager_with_source_map(source_map); // Add this due to us calling prepare_crate on "/main.nr" below diff --git a/tooling/lsp/Cargo.toml b/tooling/lsp/Cargo.toml index 67778c744db..b9f47c94103 100644 --- a/tooling/lsp/Cargo.toml +++ b/tooling/lsp/Cargo.toml @@ -22,7 +22,6 @@ noirc_frontend.workspace = true serde.workspace = true serde_json.workspace = true tower.workspace = true -cfg-if.workspace = true async-lsp = { workspace = true, features = ["omni-trait"] } serde_with = "3.2.0" fm.workspace = true diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 1474085a330..3699286ee8d 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -7,7 +7,7 @@ use std::{ collections::HashMap, future::Future, ops::{self, ControlFlow}, - path::{Path, PathBuf}, + path::PathBuf, pin::Pin, task::{self, Poll}, }; @@ -176,29 +176,3 @@ fn byte_span_to_range<'a, F: files::Files<'a> + ?Sized>( None } } - -cfg_if::cfg_if! { - if #[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))] { - use wasm_bindgen::{prelude::*, JsValue}; - - #[wasm_bindgen(module = "@noir-lang/source-resolver")] - extern "C" { - - #[wasm_bindgen(catch)] - fn read_file(path: &str) -> Result; - - } - - fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result { - let path_str = path_to_file.to_str().unwrap(); - match read_file(path_str) { - Ok(buffer) => Ok(buffer), - Err(_) => Err(Error::new(ErrorKind::Other, "could not read file using wasm")), - } - } - } else { - fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result { - std::fs::read_to_string(path_to_file) - } - } -} diff --git a/tooling/lsp/src/notifications/mod.rs b/tooling/lsp/src/notifications/mod.rs index f6484f49d48..9ae48e6c7e2 100644 --- a/tooling/lsp/src/notifications/mod.rs +++ b/tooling/lsp/src/notifications/mod.rs @@ -13,7 +13,7 @@ use crate::types::{ PublishDiagnosticsParams, }; -use crate::{byte_span_to_range, get_non_stdlib_asset, get_package_tests_in_crate, LspState}; +use crate::{byte_span_to_range, get_package_tests_in_crate, LspState}; pub(super) fn on_initialized( _state: &mut LspState, @@ -111,7 +111,7 @@ pub(super) fn on_did_save_text_document( let diagnostics: Vec<_> = workspace .into_iter() .flat_map(|package| -> Vec { - let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); + let (mut context, crate_id) = prepare_package(package); let file_diagnostics = match check_crate(&mut context, crate_id, false) { Ok(((), warnings)) => warnings, diff --git a/tooling/lsp/src/requests/code_lens_request.rs b/tooling/lsp/src/requests/code_lens_request.rs index 602ed268981..320fd519d30 100644 --- a/tooling/lsp/src/requests/code_lens_request.rs +++ b/tooling/lsp/src/requests/code_lens_request.rs @@ -8,7 +8,7 @@ use noirc_driver::{check_crate, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::hir::FunctionNameMatch; use crate::{ - byte_span_to_range, get_non_stdlib_asset, + byte_span_to_range, types::{CodeLens, CodeLensParams, CodeLensResult, Command, LogMessageParams, MessageType}, LspState, }; @@ -83,7 +83,7 @@ fn on_code_lens_request_inner( let mut lenses: Vec = vec![]; for package in &workspace { - let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); + let (mut context, crate_id) = prepare_package(package); // We ignore the warnings and errors produced by compilation for producing code lenses // because we can still get the test functions even if compilation fails let _ = check_crate(&mut context, crate_id, false); diff --git a/tooling/lsp/src/requests/goto_definition.rs b/tooling/lsp/src/requests/goto_definition.rs index 706c95d44a3..c44704bcfdd 100644 --- a/tooling/lsp/src/requests/goto_definition.rs +++ b/tooling/lsp/src/requests/goto_definition.rs @@ -52,8 +52,7 @@ fn on_goto_definition_inner( let mut definition_position = None; for package in &workspace { - let (mut context, crate_id) = - nargo::prepare_package(package, Box::new(crate::get_non_stdlib_asset)); + let (mut context, crate_id) = nargo::prepare_package(package); // We ignore the warnings and errors produced by compilation while resolving the definition let _ = noirc_driver::check_crate(&mut context, crate_id, false); diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 962fe99a49b..66fb51289d5 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -10,7 +10,6 @@ use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::hir::FunctionNameMatch; use crate::{ - get_non_stdlib_asset, types::{NargoTestRunParams, NargoTestRunResult}, LspState, }; @@ -51,7 +50,7 @@ fn on_test_run_request_inner( // Since we filtered on crate name, this should be the only item in the iterator match workspace.into_iter().next() { Some(package) => { - let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); + let (mut context, crate_id) = prepare_package(package); if check_crate(&mut context, crate_id, false).is_err() { let result = NargoTestRunResult { id: params.id.clone(), diff --git a/tooling/lsp/src/requests/tests.rs b/tooling/lsp/src/requests/tests.rs index 6b94b921a06..dc0ccad6d21 100644 --- a/tooling/lsp/src/requests/tests.rs +++ b/tooling/lsp/src/requests/tests.rs @@ -7,7 +7,7 @@ use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSele use noirc_driver::{check_crate, NOIR_ARTIFACT_VERSION_STRING}; use crate::{ - get_non_stdlib_asset, get_package_tests_in_crate, + get_package_tests_in_crate, types::{NargoPackageTests, NargoTestsParams, NargoTestsResult}, LspState, }; @@ -53,7 +53,7 @@ fn on_tests_request_inner( let package_tests: Vec<_> = workspace .into_iter() .filter_map(|package| { - let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); + let (mut context, crate_id) = prepare_package(package); // We ignore the warnings and errors produced by compilation for producing tests // because we can still get the test functions even if compilation fails let _ = check_crate(&mut context, crate_id, false); diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 7c81b6bcbac..2b5ac8e5a00 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -16,7 +16,7 @@ pub mod workspace; use std::collections::BTreeMap; -use fm::{FileManager, FileReader}; +use fm::FileManager; use noirc_driver::{add_dep, prepare_crate, prepare_dependency}; use noirc_frontend::{ graph::{CrateGraph, CrateId, CrateName}, @@ -59,7 +59,9 @@ pub fn insert_all_files_for_package_into_file_manager( // Get all files in the package and add them to the file manager let paths = get_all_paths_in_dir(&root_path).expect("could not get all paths in the package"); for path in paths { - file_manager.add_file(path.as_path()); + let source = std::fs::read_to_string(path.as_path()) + .unwrap_or_else(|_| panic!("could not read file {:?} into string", path)); + file_manager.add_file_with_source(path.as_path(), source); } insert_all_files_for_packages_dependencies_into_file_manager(package, file_manager); @@ -81,9 +83,9 @@ fn insert_all_files_for_packages_dependencies_into_file_manager( } } -pub fn prepare_package(package: &Package, file_reader: Box) -> (Context, CrateId) { +pub fn prepare_package(package: &Package) -> (Context, CrateId) { // TODO: FileManager continues to leak into various crates - let mut fm = FileManager::new(&package.root_dir, file_reader); + let mut fm = FileManager::new(&package.root_dir); insert_all_files_for_package_into_file_manager(package, &mut fm); let graph = CrateGraph::default(); diff --git a/tooling/nargo/src/ops/compile.rs b/tooling/nargo/src/ops/compile.rs index d4164eaa865..7b6506e4fb3 100644 --- a/tooling/nargo/src/ops/compile.rs +++ b/tooling/nargo/src/ops/compile.rs @@ -70,8 +70,7 @@ pub fn compile_program( np_language: Language, is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> (FileManager, CompilationResult) { - let (mut context, crate_id) = - prepare_package(package, Box::new(|path| std::fs::read_to_string(path))); + let (mut context, crate_id) = prepare_package(package); let program_artifact_path = workspace.package_build_path(package); let mut debug_artifact_path = program_artifact_path.clone(); @@ -111,8 +110,7 @@ fn compile_contract( np_language: Language, is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> (FileManager, CompilationResult) { - let (mut context, crate_id) = - prepare_package(package, Box::new(|path| std::fs::read_to_string(path))); + let (mut context, crate_id) = prepare_package(package); let (contract, warnings) = match noirc_driver::compile_contract(&mut context, crate_id, compile_options) { Ok(contracts_and_warnings) => contracts_and_warnings, diff --git a/tooling/nargo_cli/src/cli/check_cmd.rs b/tooling/nargo_cli/src/cli/check_cmd.rs index 57b36b8932b..e4bc29bf39e 100644 --- a/tooling/nargo_cli/src/cli/check_cmd.rs +++ b/tooling/nargo_cli/src/cli/check_cmd.rs @@ -55,8 +55,7 @@ pub(crate) fn run( } fn check_package(package: &Package, compile_options: &CompileOptions) -> Result<(), CompileError> { - let (mut context, crate_id) = - prepare_package(package, Box::new(|path| std::fs::read_to_string(path))); + let (mut context, crate_id) = prepare_package(package); check_crate_and_report_errors( &mut context, crate_id, diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 02ba0d13e87..3c2de841d13 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -175,8 +175,7 @@ fn compile_program( np_language: Language, is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> (FileManager, CompilationResult) { - let (mut context, crate_id) = - prepare_package(package, Box::new(|path| std::fs::read_to_string(path))); + let (mut context, crate_id) = prepare_package(package); let program_artifact_path = workspace.package_build_path(package); let mut debug_artifact_path = program_artifact_path.clone(); @@ -238,8 +237,7 @@ fn compile_contract( np_language: Language, is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> (FileManager, CompilationResult) { - let (mut context, crate_id) = - prepare_package(package, Box::new(|path| std::fs::read_to_string(path))); + let (mut context, crate_id) = prepare_package(package); let (contract, warnings) = match noirc_driver::compile_contract(&mut context, crate_id, compile_options) { Ok(contracts_and_warnings) => contracts_and_warnings, diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index cb3b8343592..0c2ca71eba3 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -36,8 +36,7 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr let mut check_exit_code_one = false; for package in &workspace { - let mut file_manager = - FileManager::new(&package.root_dir, Box::new(|path| std::fs::read_to_string(path))); + let mut file_manager = FileManager::new(&package.root_dir); insert_all_files_for_package_into_file_manager(package, &mut file_manager); visit_noir_files(&package.root_dir.join("src"), &mut |entry| { diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 1b6dbcab34a..c91cd83f84b 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -86,8 +86,7 @@ fn run_tests( show_output: bool, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let (mut context, crate_id) = - prepare_package(package, Box::new(|path| std::fs::read_to_string(path))); + let (mut context, crate_id) = prepare_package(package); check_crate_and_report_errors( &mut context, crate_id, From 5ebf83a94b65de26595c245c035576f196eac1ef Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 17:40:38 +0000 Subject: [PATCH 28/33] Update compiler/fm/src/lib.rs --- compiler/fm/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index 3f1f5e09acc..2a823093517 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -47,7 +47,7 @@ impl FileManager { } pub fn add_file_with_source(&mut self, file_name: &Path, source: String) -> Option { - let file_name = self.root.join(file_name).normalize(); + let file_name = self.root.join(file_name); self.add_file_with_source_canonical_path(&file_name, source) } From 419273e5ac89dcab8885ac9ef04124c4ecee5c77 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Mon, 11 Dec 2023 17:56:00 +0000 Subject: [PATCH 29/33] file_reader is no longer being used --- compiler/fm/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index 899d1a93eb7..9900be209f7 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -10,9 +10,6 @@ pub use file_map::{File, FileId, FileMap, PathString}; // Re-export for the lsp pub use codespan_reporting::files as codespan_files; -use file_reader::is_stdlib_asset; -pub use file_reader::FileReader; - use std::{ collections::HashMap, path::{Component, Path, PathBuf}, From 1e9ac97ab7bc9342580159a6bbc5d2e55cb8642b Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 13 Dec 2023 13:31:49 +0000 Subject: [PATCH 30/33] chore: add simple doc comments for `add_file_with_source` methods --- compiler/fm/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler/fm/src/lib.rs b/compiler/fm/src/lib.rs index 9900be209f7..2a54e58d3b9 100644 --- a/compiler/fm/src/lib.rs +++ b/compiler/fm/src/lib.rs @@ -49,11 +49,17 @@ impl FileManager { &self.file_map } + /// Adds a source file to the [`FileManager`]. + /// + /// The `file_name` is expected to be relative to the [`FileManager`]'s root directory. pub fn add_file_with_source(&mut self, file_name: &Path, source: String) -> Option { let file_name = self.root.join(file_name); self.add_file_with_source_canonical_path(&file_name, source) } + /// Adds a source file to the [`FileManager`] using a path which is not appended to the root path. + /// + /// This should only be used for the stdlib as these files do not exist on the user's filesystem. pub fn add_file_with_source_canonical_path( &mut self, file_name: &Path, From 9aa0bb71ba164266917f5c427ac42340c1c36bc5 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Dec 2023 18:06:37 +0000 Subject: [PATCH 31/33] Update tooling/nargo/src/lib.rs Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- tooling/nargo/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 2b5ac8e5a00..edaf4c48f97 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -84,7 +84,6 @@ fn insert_all_files_for_packages_dependencies_into_file_manager( } pub fn prepare_package(package: &Package) -> (Context, CrateId) { - // TODO: FileManager continues to leak into various crates let mut fm = FileManager::new(&package.root_dir); insert_all_files_for_package_into_file_manager(package, &mut fm); From 3e26e3ca4f0820006ed729903a94d34486f993bc Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Dec 2023 18:22:24 +0000 Subject: [PATCH 32/33] read entry_path and get all files in its parent --- tooling/nargo/src/lib.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index edaf4c48f97..6bee306b52c 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -52,12 +52,16 @@ pub fn insert_all_files_for_package_into_file_manager( package: &Package, file_manager: &mut FileManager, ) { - // Start off at the root directory of the package and add all of the files located - // in that directory. - let root_path = package.root_dir.clone(); + // Start off at the entry path and read all files in the parent directory. + let entry_path_parent = package + .entry_path + .parent() + .unwrap_or_else(|| panic!("The entry path is expected to be a single file within a directory and so should have a parent {:?}", package.entry_path)) + .clone(); // Get all files in the package and add them to the file manager - let paths = get_all_paths_in_dir(&root_path).expect("could not get all paths in the package"); + let paths = + get_all_paths_in_dir(&entry_path_parent).expect("could not get all paths in the package"); for path in paths { let source = std::fs::read_to_string(path.as_path()) .unwrap_or_else(|_| panic!("could not read file {:?} into string", path)); From 0b5333222d975d09a2a394b5dbe4bef29c556e69 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Wed, 13 Dec 2023 18:25:06 +0000 Subject: [PATCH 33/33] clippy --- tooling/nargo/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 6bee306b52c..6f3d36febba 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -61,7 +61,7 @@ pub fn insert_all_files_for_package_into_file_manager( // Get all files in the package and add them to the file manager let paths = - get_all_paths_in_dir(&entry_path_parent).expect("could not get all paths in the package"); + get_all_paths_in_dir(entry_path_parent).expect("could not get all paths in the package"); for path in paths { let source = std::fs::read_to_string(path.as_path()) .unwrap_or_else(|_| panic!("could not read file {:?} into string", path));