chore: remove FileDiagnostic (#7546)

compiler/noirc_driver/src/lib.rs

@@ -10,7 +10,7 @@ use clap::Args;
 use fm::{FileId, FileManager};
 use iter_extended::vecmap;
 use noirc_abi::{AbiParameter, AbiType, AbiValue};
-use noirc_errors::{CustomDiagnostic, DiagnosticKind, FileDiagnostic};
+use noirc_errors::{CustomDiagnostic, DiagnosticKind};
 use noirc_evaluator::brillig::BrilligOptions;
 use noirc_evaluator::create_program;
 use noirc_evaluator::errors::RuntimeError;
@@ -227,8 +227,8 @@ impl From<RuntimeError> for CompileError {
     }
 }
 
-impl From<CompileError> for FileDiagnostic {
-    fn from(error: CompileError) -> FileDiagnostic {
+impl From<CompileError> for CustomDiagnostic {
+    fn from(error: CompileError) -> CustomDiagnostic {
         match error {
             CompileError::RuntimeError(err) => err.into(),
             CompileError::MonomorphizationError(err) => err.into(),
@@ -237,10 +237,10 @@ impl From<CompileError> for FileDiagnostic {
 }
 
 /// Helper type used to signify where only warnings are expected in file diagnostics
-pub type Warnings = Vec<FileDiagnostic>;
+pub type Warnings = Vec<CustomDiagnostic>;
 
 /// Helper type used to signify where errors or warnings are expected in file diagnostics
-pub type ErrorsAndWarnings = Vec<FileDiagnostic>;
+pub type ErrorsAndWarnings = Vec<CustomDiagnostic>;
 
 /// Helper type for connecting a compilation artifact to the errors or warnings which were produced during compilation.
 pub type CompilationResult<T> = Result<(T, Warnings), ErrorsAndWarnings>;
@@ -350,20 +350,16 @@ pub fn check_crate(
 ) -> CompilationResult<()> {
     let diagnostics = CrateDefMap::collect_defs(crate_id, context, options.frontend_options());
     let crate_files = context.crate_files(&crate_id);
-    let warnings_and_errors: Vec<FileDiagnostic> = diagnostics
-        .into_iter()
-        .map(|error| {
-            let location = error.location();
-            let diagnostic = CustomDiagnostic::from(&error);
-            diagnostic.in_file(location.file)
-        })
+    let warnings_and_errors: Vec<CustomDiagnostic> = diagnostics
+        .iter()
+        .map(CustomDiagnostic::from)
         .filter(|diagnostic| {
             // We filter out any warnings if they're going to be ignored later on to free up memory.
-            !options.silence_warnings || diagnostic.diagnostic.kind != DiagnosticKind::Warning
+            !options.silence_warnings || diagnostic.kind != DiagnosticKind::Warning
         })
         .filter(|error| {
             // Only keep warnings from the crate we are checking
-            if error.diagnostic.is_warning() { crate_files.contains(&error.file_id) } else { true }
+            if error.is_warning() { crate_files.contains(&error.file) } else { true }
         })
         .collect();
 
@@ -401,16 +397,16 @@ pub fn compile_main(
         // TODO(#2155): This error might be a better to exist in Nargo
         let err = CustomDiagnostic::from_message(
             "cannot compile crate into a program as it does not contain a `main` function",
-        )
-        .in_file(FileId::default());
+            FileId::default(),
+        );
         vec![err]
     })?;
 
     let compiled_program =
         compile_no_check(context, options, main, cached_program, options.force_compile)
-            .map_err(FileDiagnostic::from)?;
+            .map_err(|error| vec![CustomDiagnostic::from(error)])?;
 
-    let compilation_warnings = vecmap(compiled_program.warnings.clone(), FileDiagnostic::from);
+    let compilation_warnings = vecmap(compiled_program.warnings.clone(), CustomDiagnostic::from);
     if options.deny_warnings && !compilation_warnings.is_empty() {
         return Err(compilation_warnings);
     }
@@ -439,14 +435,16 @@ pub fn compile_contract(
     let mut errors = warnings;
 
     if contracts.len() > 1 {
-        let err = CustomDiagnostic::from_message("Packages are limited to a single contract")
-            .in_file(FileId::default());
+        let err = CustomDiagnostic::from_message(
+            "Packages are limited to a single contract",
+            FileId::default(),
+        );
         return Err(vec![err]);
     } else if contracts.is_empty() {
         let err = CustomDiagnostic::from_message(
             "cannot compile crate into a contract as it does not contain any contracts",
-        )
-        .in_file(FileId::default());
+            FileId::default(),
+        );
         return Err(vec![err]);
     };
 
@@ -483,12 +481,8 @@ pub fn compile_contract(
 }
 
 /// True if there are (non-warning) errors present and we should halt compilation
-fn has_errors(errors: &[FileDiagnostic], deny_warnings: bool) -> bool {
-    if deny_warnings {
-        !errors.is_empty()
-    } else {
-        errors.iter().any(|error| error.diagnostic.is_error())
-    }
+fn has_errors(errors: &[CustomDiagnostic], deny_warnings: bool) -> bool {
+    if deny_warnings { !errors.is_empty() } else { errors.iter().any(|error| error.is_error()) }
 }
 
 /// Compile all of the functions associated with a Noir contract.
@@ -525,7 +519,7 @@ fn compile_contract_inner(
         let function = match compile_no_check(context, &options, function_id, None, true) {
             Ok(function) => function,
             Err(new_error) => {
-                errors.push(FileDiagnostic::from(new_error));
+                errors.push(new_error.into());
                 continue;
             }
         };

compiler/noirc_driver/tests/contracts.rs

@@ -33,10 +33,10 @@ contract Bar {}";
 
     assert_eq!(
         errors,
-        vec![
-            CustomDiagnostic::from_message("Packages are limited to a single contract")
-                .in_file(FileId::default())
-        ],
+        vec![CustomDiagnostic::from_message(
+            "Packages are limited to a single contract",
+            FileId::default()
+        )],
         "stdlib is producing warnings"
     );
 

compiler/noirc_errors/src/lib.rs

@@ -8,21 +8,3 @@ mod position;
 pub mod reporter;
 pub use position::{Located, Location, Position, Span, Spanned};
 pub use reporter::{CustomDiagnostic, DiagnosticKind};
-
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct FileDiagnostic {
-    pub file_id: fm::FileId,
-    pub diagnostic: CustomDiagnostic,
-}
-
-impl FileDiagnostic {
-    pub fn new(file_id: fm::FileId, diagnostic: CustomDiagnostic) -> FileDiagnostic {
-        FileDiagnostic { file_id, diagnostic }
-    }
-}
-
-impl From<FileDiagnostic> for Vec<FileDiagnostic> {
-    fn from(value: FileDiagnostic) -> Self {
-        vec![value]
-    }
-}

compiler/noirc_errors/src/reporter.rs

@@ -1,13 +1,14 @@
 use std::io::IsTerminal;
 
-use crate::{FileDiagnostic, Location, Span};
+use crate::{Location, Span};
 use codespan_reporting::diagnostic::{Diagnostic, Label};
 use codespan_reporting::files::Files;
 use codespan_reporting::term;
 use codespan_reporting::term::termcolor::{ColorChoice, StandardStream};
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct CustomDiagnostic {
+    pub file: fm::FileId,
     pub message: String,
     pub secondaries: Vec<CustomLabel>,
     pub notes: Vec<String>,
@@ -35,8 +36,9 @@ pub struct ReportedErrors {
 }
 
 impl CustomDiagnostic {
-    pub fn from_message(msg: &str) -> CustomDiagnostic {
+    pub fn from_message(msg: &str, file: fm::FileId) -> CustomDiagnostic {
         Self {
+            file,
             message: msg.to_owned(),
             secondaries: Vec::new(),
             notes: Vec::new(),
@@ -54,6 +56,7 @@ impl CustomDiagnostic {
         kind: DiagnosticKind,
     ) -> CustomDiagnostic {
         CustomDiagnostic {
+            file: secondary_location.file,
             message: primary_message,
             secondaries: vec![CustomLabel::new(secondary_message, secondary_location)],
             notes: Vec::new(),
@@ -109,6 +112,7 @@ impl CustomDiagnostic {
         secondary_location: Location,
     ) -> CustomDiagnostic {
         CustomDiagnostic {
+            file: secondary_location.file,
             message: primary_message,
             secondaries: vec![CustomLabel::new(secondary_message, secondary_location)],
             notes: Vec::new(),
@@ -119,10 +123,6 @@ impl CustomDiagnostic {
         }
     }
 
-    pub fn in_file(self, file_id: fm::FileId) -> FileDiagnostic {
-        FileDiagnostic::new(file_id, self)
-    }
-
     pub fn with_call_stack(mut self, call_stack: Vec<Location>) -> Self {
         self.call_stack = call_stack;
         self
@@ -185,16 +185,16 @@ impl CustomLabel {
 /// of diagnostics that were errors.
 pub fn report_all<'files>(
     files: &'files impl Files<'files, FileId = fm::FileId>,
-    diagnostics: &[FileDiagnostic],
+    diagnostics: &[CustomDiagnostic],
     deny_warnings: bool,
     silence_warnings: bool,
 ) -> ReportedErrors {
     // Report warnings before any errors
     let (warnings_and_bugs, mut errors): (Vec<_>, _) =
-        diagnostics.iter().partition(|item| !item.diagnostic.is_error());
+        diagnostics.iter().partition(|item| !item.is_error());
 
     let (warnings, mut bugs): (Vec<_>, _) =
-        warnings_and_bugs.iter().partition(|item| item.diagnostic.is_warning());
+        warnings_and_bugs.iter().partition(|item| item.is_warning());
     let mut diagnostics = if silence_warnings { Vec::new() } else { warnings };
     diagnostics.append(&mut bugs);
     diagnostics.append(&mut errors);
@@ -205,14 +205,14 @@ pub fn report_all<'files>(
     ReportedErrors { error_count }
 }
 
-impl FileDiagnostic {
+impl CustomDiagnostic {
     /// Print the report; return true if it was an error.
     pub fn report<'files>(
         &self,
         files: &'files impl Files<'files, FileId = fm::FileId>,
         deny_warnings: bool,
     ) -> bool {
-        report(files, &self.diagnostic, deny_warnings)
+        report(files, self, deny_warnings)
     }
 }
 

compiler/noirc_evaluator/src/errors.rs

@@ -8,7 +8,7 @@
 //!
 //! An Error of the latter is an error in the implementation of the compiler
 use iter_extended::vecmap;
-use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, Location};
+use noirc_errors::{CustomDiagnostic, Location};
 use noirc_frontend::signed_field::SignedField;
 use thiserror::Error;
 
@@ -73,8 +73,8 @@ pub enum SsaReport {
     Bug(InternalBug),
 }
 
-impl From<SsaReport> for FileDiagnostic {
-    fn from(error: SsaReport) -> FileDiagnostic {
+impl From<SsaReport> for CustomDiagnostic {
+    fn from(error: SsaReport) -> CustomDiagnostic {
         match error {
             SsaReport::Warning(warning) => {
                 let message = warning.to_string();
@@ -87,10 +87,10 @@ impl From<SsaReport> for FileDiagnostic {
                     },
                 };
                 let call_stack = vecmap(call_stack, |location| location);
-                let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
                 let location = call_stack.last().expect("Expected RuntimeError to have a location");
-                let diagnostic = Diagnostic::simple_warning(message, secondary_message, *location);
-                diagnostic.with_call_stack(call_stack).in_file(file_id)
+                let diagnostic =
+                    CustomDiagnostic::simple_warning(message, secondary_message, *location);
+                diagnostic.with_call_stack(call_stack)
             }
             SsaReport::Bug(bug) => {
                 let message = bug.to_string();
@@ -104,10 +104,10 @@ impl From<SsaReport> for FileDiagnostic {
                     InternalBug::AssertFailed { call_stack } => ("As a result, the compiled circuit is ensured to fail. Other assertions may also fail during execution".to_string(), call_stack)
                 };
                 let call_stack = vecmap(call_stack, |location| location);
-                let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
                 let location = call_stack.last().expect("Expected RuntimeError to have a location");
-                let diagnostic = Diagnostic::simple_bug(message, secondary_message, *location);
-                diagnostic.with_call_stack(call_stack).in_file(file_id)
+                let diagnostic =
+                    CustomDiagnostic::simple_bug(message, secondary_message, *location);
+                diagnostic.with_call_stack(call_stack)
             }
         }
     }
@@ -181,20 +181,19 @@ impl RuntimeError {
     }
 }
 
-impl From<RuntimeError> for FileDiagnostic {
-    fn from(error: RuntimeError) -> FileDiagnostic {
+impl From<RuntimeError> for CustomDiagnostic {
+    fn from(error: RuntimeError) -> CustomDiagnostic {
         let call_stack = vecmap(error.call_stack(), |location| *location);
-        let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
         let diagnostic = error.into_diagnostic();
-        diagnostic.with_call_stack(call_stack).in_file(file_id)
+        diagnostic.with_call_stack(call_stack)
     }
 }
 
 impl RuntimeError {
-    fn into_diagnostic(self) -> Diagnostic {
+    fn into_diagnostic(self) -> CustomDiagnostic {
         match self {
             RuntimeError::InternalError(cause) => {
-                Diagnostic::simple_error(
+                CustomDiagnostic::simple_error(
                     "Internal Consistency Evaluators Errors: \n
                     This is likely a bug. Consider opening an issue at https://github.com/noir-lang/noir/issues".to_owned(),
                     cause.to_string(),
@@ -206,7 +205,7 @@ impl RuntimeError {
                 let location =
                     self.call_stack().last().expect("Expected RuntimeError to have a location");
 
-                Diagnostic::simple_error(
+                CustomDiagnostic::simple_error(
                     primary_message,
                     "If attempting to fetch the length of a slice, try converting to an array. Slices only use dynamic lengths.".to_string(),
                     *location,
@@ -217,7 +216,7 @@ impl RuntimeError {
                 let location =
                     self.call_stack().last().unwrap_or_else(|| panic!("Expected RuntimeError to have a location. Error message: {message}"));
 
-                Diagnostic::simple_error(message, String::new(), *location)
+                CustomDiagnostic::simple_error(message, String::new(), *location)
             }
         }
     }

compiler/noirc_frontend/src/hir/def_collector/errors.rs

@@ -2,7 +2,6 @@ use crate::ast::{Ident, ItemVisibility, Path, UnsupportedNumericGenericType};
 use crate::hir::resolution::import::PathResolutionError;
 use crate::hir::type_check::generics::TraitGenerics;
 
-use noirc_errors::FileDiagnostic;
 use noirc_errors::{CustomDiagnostic as Diagnostic, Location};
 use thiserror::Error;
 
@@ -76,10 +75,6 @@ pub enum DefCollectorErrorKind {
 }
 
 impl DefCollectorErrorKind {
-    pub fn into_file_diagnostic(&self, file: fm::FileId) -> FileDiagnostic {
-        Diagnostic::from(self).in_file(file)
-    }
-
     pub fn location(&self) -> Location {
         match self {
             DefCollectorErrorKind::Duplicate { first_def: ident, .. }

compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -1,6 +1,6 @@
 use acvm::FieldElement;
 pub use noirc_errors::Span;
-use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, Location};
+use noirc_errors::{CustomDiagnostic as Diagnostic, Location};
 use thiserror::Error;
 
 use crate::{
@@ -195,10 +195,6 @@ pub enum ResolverError {
 }
 
 impl ResolverError {
-    pub fn into_file_diagnostic(&self, file: fm::FileId) -> FileDiagnostic {
-        Diagnostic::from(self).in_file(file)
-    }
-
     pub fn location(&self) -> Location {
         match self {
             ResolverError::DuplicateDefinition { first_location: location, .. }