chore: Remove reliance on FileManager in noirc_errors (#2580)

crates/fm/src/file_map.rs

@@ -1,7 +1,6 @@
-use crate::FileManager;
-use codespan_reporting::files::{SimpleFile, SimpleFiles};
+use codespan_reporting::files::{Error, Files, SimpleFile, SimpleFiles};
 use serde::{Deserialize, Serialize};
-use std::path::PathBuf;
+use std::{ops::Range, path::PathBuf};
 
 // XXX: File and FileMap serve as opaque types, so that the rest of the library does not need to import the dependency
 // or worry about when we change the dep
@@ -74,9 +73,24 @@ impl Default for FileMap {
     }
 }
 
-impl FileManager {
-    // Needed as code_span dep requires underlying SimpleFiles
-    pub fn as_simple_files(&self) -> &SimpleFiles<PathString, String> {
-        &self.file_map.0
+impl<'a> Files<'a> for FileMap {
+    type FileId = FileId;
+    type Name = PathString;
+    type Source = &'a str;
+
+    fn name(&self, file_id: Self::FileId) -> Result<Self::Name, Error> {
+        Ok(self.0.get(file_id.as_usize())?.name().clone())
+    }
+
+    fn source(&'a self, file_id: Self::FileId) -> Result<Self::Source, Error> {
+        Ok(self.0.get(file_id.as_usize())?.source().as_ref())
+    }
+
+    fn line_index(&self, file_id: Self::FileId, byte_index: usize) -> Result<usize, Error> {
+        self.0.get(file_id.as_usize())?.line_index((), byte_index)
+    }
+
+    fn line_range(&self, file_id: Self::FileId, line_index: usize) -> Result<Range<usize>, Error> {
+        self.0.get(file_id.as_usize())?.line_range((), line_index)
     }
 }

crates/fm/src/lib.rs

@@ -37,6 +37,10 @@ impl FileManager {
         }
     }
 
+    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) {

crates/lsp/src/lib.rs

@@ -195,7 +195,7 @@ fn on_code_lens_request(
         let _ = check_crate(&mut context, crate_id, false);
 
         let fm = &context.file_manager;
-        let files = fm.as_simple_files();
+        let files = fm.as_file_map();
         let tests = context
             .get_all_test_functions_in_crate_matching(&crate_id, FunctionNameMatch::Anything);
 
@@ -211,8 +211,8 @@ fn on_code_lens_request(
                 continue;
             }
 
-            let range = byte_span_to_range(files, file_id.as_usize(), location.span.into())
-                .unwrap_or_default();
+            let range =
+                byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default();
 
             let test_command = Command {
                 title: format!("{ARROW} {TEST_CODELENS_TITLE}"),
@@ -245,8 +245,8 @@ fn on_code_lens_request(
                     continue;
                 }
 
-                let range = byte_span_to_range(files, file_id.as_usize(), location.span.into())
-                    .unwrap_or_default();
+                let range =
+                    byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default();
 
                 let compile_command = Command {
                     title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"),
@@ -294,8 +294,8 @@ fn on_code_lens_request(
                     continue;
                 }
 
-                let range = byte_span_to_range(files, file_id.as_usize(), location.span.into())
-                    .unwrap_or_default();
+                let range =
+                    byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default();
 
                 let compile_command = Command {
                     title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"),
@@ -417,7 +417,7 @@ fn on_did_save_text_document(
 
         if !file_diagnostics.is_empty() {
             let fm = &context.file_manager;
-            let files = fm.as_simple_files();
+            let files = fm.as_file_map();
 
             for FileDiagnostic { file_id, diagnostic, call_stack: _ } in file_diagnostics {
                 // Ignore diagnostics for any file that wasn't the file we saved
@@ -433,8 +433,7 @@ fn on_did_save_text_document(
                 // TODO: Should this be the first item in secondaries? Should we bail when we find a range?
                 for sec in diagnostic.secondaries {
                     // Not using `unwrap_or_default` here because we don't want to overwrite a valid range with a default range
-                    if let Some(r) = byte_span_to_range(files, file_id.as_usize(), sec.span.into())
-                    {
+                    if let Some(r) = byte_span_to_range(files, file_id, sec.span.into()) {
                         range = r
                     }
                 }

crates/nargo_cli/src/cli/compile_cmd.rs

@@ -229,9 +229,9 @@ pub(crate) fn report_errors<T>(
     deny_warnings: bool,
 ) -> Result<T, CompileError> {
     let (t, warnings) = result.map_err(|errors| {
-        noirc_errors::reporter::report_all(file_manager, &errors, deny_warnings)
+        noirc_errors::reporter::report_all(file_manager.as_file_map(), &errors, deny_warnings)
     })?;
 
-    noirc_errors::reporter::report_all(file_manager, &warnings, deny_warnings);
+    noirc_errors::reporter::report_all(file_manager.as_file_map(), &warnings, deny_warnings);
     Ok(t)
 }

crates/nargo_cli/src/cli/execute_cmd.rs

@@ -173,7 +173,7 @@ fn report_error_with_opcode_locations(
             CustomDiagnostic::simple_error(message, String::new(), location.span)
                 .in_file(location.file)
                 .with_call_stack(source_locations)
-                .report(file_manager, false);
+                .report(file_manager.as_file_map(), false);
         }
     }
 }

crates/nargo_cli/src/cli/test_cmd.rs

@@ -117,7 +117,7 @@ fn run_tests<S: BlackBoxFunctionSolver>(
             }
             TestStatus::CompileError(err) => {
                 noirc_errors::reporter::report_all(
-                    &context.file_manager,
+                    context.file_manager.as_file_map(),
                     &[err],
                     compile_options.deny_warnings,
                 );

crates/noirc_errors/src/reporter.rs

@@ -1,5 +1,6 @@
 use crate::{FileDiagnostic, Location, Span};
 use codespan_reporting::diagnostic::{Diagnostic, Label};
+use codespan_reporting::files::Files;
 use codespan_reporting::term;
 use codespan_reporting::term::termcolor::{ColorChoice, StandardStream};
 
@@ -110,8 +111,8 @@ impl CustomLabel {
 
 /// Writes the given diagnostics to stderr and returns the count
 /// of diagnostics that were errors.
-pub fn report_all(
-    files: &fm::FileManager,
+pub fn report_all<'files>(
+    files: &'files impl Files<'files, FileId = fm::FileId>,
     diagnostics: &[FileDiagnostic],
     deny_warnings: bool,
 ) -> ReportedErrors {
@@ -126,14 +127,18 @@ pub fn report_all(
 }
 
 impl FileDiagnostic {
-    pub fn report(&self, files: &fm::FileManager, deny_warnings: bool) -> bool {
+    pub fn report<'files>(
+        &self,
+        files: &'files impl Files<'files, FileId = fm::FileId>,
+        deny_warnings: bool,
+    ) -> bool {
         report(files, &self.diagnostic, Some(self.file_id), &self.call_stack, deny_warnings)
     }
 }
 
 /// Report the given diagnostic, and return true if it was an error
-pub fn report(
-    files: &fm::FileManager,
+pub fn report<'files>(
+    files: &'files impl Files<'files, FileId = fm::FileId>,
     custom_diagnostic: &CustomDiagnostic,
     file: Option<fm::FileId>,
     call_stack: &[Location],
@@ -144,7 +149,7 @@ pub fn report(
 
     let stack_trace = stack_trace(files, call_stack);
     let diagnostic = convert_diagnostic(custom_diagnostic, file, stack_trace, deny_warnings);
-    term::emit(&mut writer.lock(), &config, files.as_simple_files(), &diagnostic).unwrap();
+    term::emit(&mut writer.lock(), &config, files, &diagnostic).unwrap();
 
     deny_warnings || custom_diagnostic.is_error()
 }
@@ -154,7 +159,7 @@ fn convert_diagnostic(
     file: Option<fm::FileId>,
     stack_trace: String,
     deny_warnings: bool,
-) -> Diagnostic<usize> {
+) -> Diagnostic<fm::FileId> {
     let diagnostic = match (cd.kind, deny_warnings) {
         (DiagnosticKind::Warning, false) => Diagnostic::warning(),
         _ => Diagnostic::error(),
@@ -166,7 +171,7 @@ fn convert_diagnostic(
             .map(|sl| {
                 let start_span = sl.span.start() as usize;
                 let end_span = sl.span.end() as usize;
-                Label::secondary(file_id.as_usize(), start_span..end_span).with_message(&sl.message)
+                Label::secondary(file_id, start_span..end_span).with_message(&sl.message)
             })
             .collect()
     } else {
@@ -179,19 +184,22 @@ fn convert_diagnostic(
     diagnostic.with_message(&cd.message).with_labels(secondary_labels).with_notes(notes)
 }
 
-fn stack_trace(files: &fm::FileManager, call_stack: &[Location]) -> String {
+fn stack_trace<'files>(
+    files: &'files impl Files<'files, FileId = fm::FileId>,
+    call_stack: &[Location],
+) -> String {
     if call_stack.is_empty() {
         return String::new();
     }
 
     let mut result = "Call stack:\n".to_string();
 
     for (i, call_item) in call_stack.iter().enumerate() {
-        let path = files.path(call_item.file);
-        let source = files.fetch_file(call_item.file).source();
+        let path = files.name(call_item.file).expect("should get file path");
+        let source = files.source(call_item.file).expect("should get file source");
 
-        let (line, column) = location(source, call_item.span.start());
-        result += &format!("{}. {}.nr:{}:{}\n", i + 1, path.display(), line, column);
+        let (line, column) = location(source.as_ref(), call_item.span.start());
+        result += &format!("{}. {}.nr:{}:{}\n", i + 1, path.to_string(), line, column);
     }
 
     result