chore!: Remove add_file and file_reader from FileManager

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]

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<FileId, PathBuf>,
     path_to_id: HashMap<PathBuf, FileId>,
-    file_reader: Box<FileReader>,
 }
 
 impl std::fmt::Debug for FileManager {
@@ -37,48 +33,32 @@ impl std::fmt::Debug for FileManager {
 }
 
 impl FileManager {
-    pub fn new(root: &Path, file_reader: Box<FileReader>) -> 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,
         }
     }
 
     pub fn as_file_map(&self) -> &FileMap {
         &self.file_map
     }
 
-    pub fn add_file(&mut self, file_name: &Path) -> Option<FileId> {
-        // 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<FileId> {
+        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<FileId> {
+    pub fn add_file_with_source_canonical_path(
+        &mut self,
+        file_name: &Path,
+        source: String,
+    ) -> Option<FileId> {
+        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,23 +249,23 @@ 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"));
     }
 
     #[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,15 +280,15 @@ 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:
         // - dir/lib.nr
         // - 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);
     }

compiler/noirc_driver/src/lib.rs

@@ -75,18 +75,20 @@ pub type CompilationResult<T> = 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:?}"));

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

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
 

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<String> {
-            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<String, JsValue>;
-        }
-
-        fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result<String> {
-            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<String> {
-        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

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

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<String, JsValue>;
-
-        }
-
-        fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result<String> {
-            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<String> {
-            std::fs::read_to_string(path_to_file)
-        }
-    }
-}