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

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<FileId, String> {
+    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,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<FileId> {
         self.file_map.get_file_id(&PathString::from_path(file_name))
     }

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);
 

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"
+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"

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<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 +74,68 @@ pub fn prepare_package(package: &Package, file_reader: Box<FileReader>) -> (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<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 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);