chore!: Make file manager read-only to the compiler (#3760)

# Description

This is a PR that moves us towards having the compiler see the
FileManager as an immutable object. How it gets populated will now be
upto the caller.

One issue we have so far is that caller, can be in the wasm context or
the native context, so we have file_reader being a component that we
conditionally compile depending on the context.

Ideally we move to a case where both package source resolution and
dependency source resolution is done beforehand, and the compiler
assumes that FileManager has both the source for its current crate and
all of the crates dependencies.

This separation of concerns makes it easier to incrementally compile
parts of the compiler in the future. For example, if the FileManager has
not changed, then we know that the compilation result should not
change,and we do not need to recompile.

Further, lets say we add a pass that solely does parsing of the root
files in the File Manager, If a particular parsed AST has not changed,
then we do not need to parse that file again.

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## 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.

---------

Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

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,16 +4,12 @@
 #![warn(clippy::semicolon_if_nothing_returned)]
 
 mod file_map;
-mod file_reader;
 
 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},
@@ -26,7 +22,6 @@ pub struct FileManager {
     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 {
@@ -41,39 +36,45 @@ 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()
-        };
+    /// 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<FileId> {
+        let file_name = self.root.join(file_name);
+        self.add_file_with_source_canonical_path(&file_name, source)
+    }
 
-        // 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) {
+    /// 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,
+        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) {
             return Some(*file_id);
         }
+        let file_name_path_buf = file_name.to_path_buf();
 
         // 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);
+        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)
     }
 
@@ -98,7 +99,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();
 
@@ -111,14 +115,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 {
@@ -220,9 +229,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]
@@ -232,9 +243,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);
@@ -247,24 +258,24 @@ 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 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_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
         // we now have:
@@ -277,14 +288,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_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
-        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_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();
@@ -303,7 +316,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");
@@ -313,8 +326,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/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');
   }

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