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

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