chore!: Make file manager read-only to the compiler

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<String> + Send;
 
 #[derive(RustEmbed)]

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<FileId> {
+        // 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,7 +110,10 @@ impl FileManager {
         self.id_to_path.get(&file_id).unwrap().as_path()
     }
 
-    pub fn find_module(&mut self, anchor: FileId, mod_name: &str) -> Result<FileId, String> {
+    // 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<FileId, String> {
         let anchor_path = self.path(anchor).with_extension("");
         let anchor_dir = anchor_path.parent().unwrap();
 
@@ -107,14 +126,19 @@ 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<FileId> {
         self.file_map.get_file_id(&PathString::from_path(file_name))
     }
 }
 
+// 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 {
@@ -216,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]
@@ -257,10 +283,10 @@ 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:
@@ -273,14 +299,16 @@ mod tests {
         // 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();

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}
 

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,9 +77,19 @@ pub type CompilationResult<T> = 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);
+    }
+
     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 +100,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.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);
 

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()
+}

tooling/nargo/Cargo.toml

@@ -24,4 +24,9 @@ iter-extended.workspace = true
 serde.workspace = true
 thiserror.workspace = true
 codespan-reporting.workspace = true
-rayon = "1.8.0"
+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"

tooling/nargo/src/lib.rs

@@ -42,9 +42,50 @@
     }
 }
 
+// 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());
+    }
+
+    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<FileReader>) -> (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 +95,72 @@
 
     (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<Vec<std::path::PathBuf>> {
+    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));
+        }
+    }
+}

tooling/nargo_cli/src/cli/fmt_cmd.rs

@@ -2,6 +2,7 @@
 
 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;
@@ -14,7 +15,7 @@
 /// Format the Noir files in a workspace
 #[derive(Debug, Clone, Args)]
 pub(crate) struct FormatCommand {
     /// Run noirfmt in check mode
     #[arg(long)]
     check: bool,
 }
@@ -37,9 +38,10 @@
     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);