feat: Nargo test runtime callstacks and assert messages without strin…

…g matching (noir-lang#2953)

compiler/noirc_driver/src/lib.rs

@@ -8,6 +8,7 @@ use debug::filter_relevant_files;
 use fm::FileId;
 use noirc_abi::{AbiParameter, AbiType, ContractEvent};
 use noirc_errors::{CustomDiagnostic, FileDiagnostic};
+use noirc_evaluator::errors::RuntimeError;
 use noirc_evaluator::{create_circuit, into_abi_params};
 use noirc_frontend::graph::{CrateId, CrateName};
 use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
@@ -164,7 +165,8 @@ pub fn compile_main(
         }
     };
 
-    let compiled_program = compile_no_check(context, options, main, cached_program, force_compile)?;
+    let compiled_program = compile_no_check(context, options, main, cached_program, force_compile)
+        .map_err(FileDiagnostic::from)?;
 
     if options.print_acir {
         println!("Compiled ACIR for main (unoptimized):");
@@ -261,7 +263,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(new_error);
+                errors.push(FileDiagnostic::from(new_error));
                 continue;
             }
         };
@@ -316,7 +318,7 @@ pub fn compile_no_check(
     main_function: FuncId,
     cached_program: Option<CompiledProgram>,
     force_compile: bool,
-) -> Result<CompiledProgram, FileDiagnostic> {
+) -> Result<CompiledProgram, RuntimeError> {
     let program = monomorphize(main_function, &context.def_interner);
 
     let hash = fxhash::hash64(&program);

compiler/noirc_evaluator/src/lib.rs

@@ -3,7 +3,7 @@
 #![warn(unreachable_pub)]
 #![warn(clippy::semicolon_if_nothing_returned)]
 
-mod errors;
+pub mod errors;
 
 // SSA code to create the SSA based IR
 // for functions and execute different optimizations.

tooling/lsp/src/lib.rs

@@ -267,7 +267,7 @@ fn on_test_run_request(
                             result: "pass".to_string(),
                             message: None,
                         },
-                        TestStatus::Fail { message } => NargoTestRunResult {
+                        TestStatus::Fail { message, .. } => NargoTestRunResult {
                             id: params.id.clone(),
                             result: "fail".to_string(),
                             message: Some(message),

tooling/nargo/Cargo.toml

@@ -17,6 +17,7 @@ fm.workspace = true
 noirc_abi.workspace = true
 noirc_driver.workspace = true
 noirc_errors.workspace = true
+noirc_evaluator.workspace = true
 noirc_frontend.workspace = true
 noirc_printable_type.workspace = true
 iter-extended.workspace = true

tooling/nargo/src/errors.rs

@@ -1,4 +1,8 @@
-use acvm::{acir::circuit::OpcodeLocation, pwg::OpcodeResolutionError};
+use acvm::{
+    acir::circuit::OpcodeLocation,
+    pwg::{ErrorLocation, OpcodeResolutionError},
+};
+use noirc_errors::{debug_info::DebugInfo, CustomDiagnostic, FileDiagnostic, Location};
 use noirc_printable_type::ForeignCallError;
 use thiserror::Error;
 
@@ -57,3 +61,79 @@ pub enum ExecutionError {
     #[error(transparent)]
     SolvingError(#[from] OpcodeResolutionError),
 }
+
+/// Extracts the opcode locations from a nargo error.
+fn extract_locations_from_error(
+    error: &ExecutionError,
+    debug: &DebugInfo,
+) -> Option<Vec<Location>> {
+    let mut opcode_locations = match error {
+        ExecutionError::SolvingError(OpcodeResolutionError::BrilligFunctionFailed {
+            call_stack,
+            ..
+        })
+        | ExecutionError::AssertionFailed(_, call_stack) => Some(call_stack.clone()),
+        ExecutionError::SolvingError(OpcodeResolutionError::IndexOutOfBounds {
+            opcode_location: error_location,
+            ..
+        })
+        | ExecutionError::SolvingError(OpcodeResolutionError::UnsatisfiedConstrain {
+            opcode_location: error_location,
+        }) => match error_location {
+            ErrorLocation::Unresolved => {
+                unreachable!("Cannot resolve index for unsatisfied constraint")
+            }
+            ErrorLocation::Resolved(opcode_location) => Some(vec![*opcode_location]),
+        },
+        _ => None,
+    }?;
+
+    if let Some(OpcodeLocation::Brillig { acir_index, .. }) = opcode_locations.get(0) {
+        opcode_locations.insert(0, OpcodeLocation::Acir(*acir_index));
+    }
+
+    Some(
+        opcode_locations
+            .iter()
+            .flat_map(|opcode_location| debug.opcode_location(opcode_location).unwrap_or_default())
+            .collect(),
+    )
+}
+
+/// Tries to generate a runtime diagnostic from a nargo error. It will successfully do so if it's a runtime error with a call stack.
+pub fn try_to_diagnose_runtime_error(
+    nargo_err: &NargoError,
+    debug: &DebugInfo,
+) -> Option<FileDiagnostic> {
+    let execution_error = match nargo_err {
+        NargoError::ExecutionError(execution_error) => execution_error,
+        _ => return None,
+    };
+
+    let source_locations = extract_locations_from_error(execution_error, debug)?;
+
+    // The location of the error itself will be the location at the top
+    // of the call stack (the last item in the Vec).
+    let location = source_locations.last()?;
+
+    let message = match nargo_err {
+        NargoError::ExecutionError(ExecutionError::AssertionFailed(message, _)) => {
+            format!("Assertion failed: '{message}'")
+        }
+        NargoError::ExecutionError(ExecutionError::SolvingError(
+            OpcodeResolutionError::IndexOutOfBounds { index, array_size, .. },
+        )) => {
+            format!("Index out of bounds, array has size {array_size:?}, but index was {index:?}")
+        }
+        NargoError::ExecutionError(ExecutionError::SolvingError(
+            OpcodeResolutionError::UnsatisfiedConstrain { .. },
+        )) => "Failed constraint".into(),
+        _ => nargo_err.to_string(),
+    };
+
+    Some(
+        CustomDiagnostic::simple_error(message, String::new(), location.span)
+            .in_file(location.file)
+            .with_call_stack(source_locations),
+    )
+}

tooling/nargo/src/ops/test.rs

@@ -1,15 +1,16 @@
 use acvm::{acir::native_types::WitnessMap, BlackBoxFunctionSolver};
 use noirc_driver::{compile_no_check, CompileOptions};
-use noirc_errors::FileDiagnostic;
+use noirc_errors::{debug_info::DebugInfo, FileDiagnostic};
+use noirc_evaluator::errors::RuntimeError;
 use noirc_frontend::hir::{def_map::TestFunction, Context};
 
-use crate::NargoError;
+use crate::{errors::try_to_diagnose_runtime_error, NargoError};
 
 use super::execute_circuit;
 
 pub enum TestStatus {
     Pass,
-    Fail { message: String },
+    Fail { message: String, error_diagnostic: Option<FileDiagnostic> },
     CompileError(FileDiagnostic),
 }
 
@@ -27,9 +28,9 @@ pub fn run_test<B: BlackBoxFunctionSolver>(
             // otherwise constraints involving these expressions will not error.
             let circuit_execution =
                 execute_circuit(blackbox_solver, program.circuit, WitnessMap::new(), show_output);
-            test_status_program_compile_pass(test_function, circuit_execution)
+            test_status_program_compile_pass(test_function, program.debug, circuit_execution)
         }
-        Err(diag) => test_status_program_compile_fail(diag, test_function),
+        Err(err) => test_status_program_compile_fail(err, test_function),
     }
 }
 
@@ -39,33 +40,19 @@ pub fn run_test<B: BlackBoxFunctionSolver>(
 /// that a constraint was never satisfiable.
 /// An example of this is the program `assert(false)`
 /// In that case, we check if the test function should fail, and if so, we return `TestStatus::Pass`.
-fn test_status_program_compile_fail(
-    diag: FileDiagnostic,
-    test_function: TestFunction,
-) -> TestStatus {
+fn test_status_program_compile_fail(err: RuntimeError, test_function: TestFunction) -> TestStatus {
     // The test has failed compilation, but it should never fail. Report error.
     if !test_function.should_fail() {
-        return TestStatus::CompileError(diag);
-    }
-
-    // The test has failed compilation, check if it is because the program is never satisfiable.
-    // If it is never satisfiable, then this is the expected behavior.
-    let program_is_never_satisfiable = diag.diagnostic.message.contains("Failed constraint");
-    if !program_is_never_satisfiable {
-        // The test has failed compilation, but its a compilation error. Report error
-        return TestStatus::CompileError(diag);
+        return TestStatus::CompileError(err.into());
     }
 
-    // Given "Failed constraint: 'reason'"
-    // This method will return "reason"
-    fn extract_constraint_error_message(input: &str) -> Option<String> {
-        input.split(": '").nth(1)?.split('\'').next().map(str::to_string)
+    // The test has failed compilation, extract the assertion message if present and check if it's expected.
+    if let RuntimeError::FailedConstraint { assert_message: Some(assert_message), .. } = &err {
+        let assert_message = assert_message.clone();
+        check_expected_failure_message(test_function, &assert_message, Some(err.into()))
+    } else {
+        TestStatus::CompileError(err.into())
     }
-
-    check_expected_failure_message(
-        test_function,
-        &extract_constraint_error_message(&diag.diagnostic.message).unwrap(),
-    )
 }
 
 /// The test function compiled successfully.
@@ -74,6 +61,7 @@ fn test_status_program_compile_fail(
 /// passed/failed to determine the test status.
 fn test_status_program_compile_pass(
     test_function: TestFunction,
+    debug: DebugInfo,
     circuit_execution: Result<WitnessMap, NargoError>,
 ) -> TestStatus {
     let circuit_execution_err = match circuit_execution {
@@ -83,6 +71,7 @@ fn test_status_program_compile_pass(
             if test_function.should_fail() {
                 return TestStatus::Fail {
                     message: "error: Test passed when it should have failed".to_string(),
+                    error_diagnostic: None,
                 };
             }
             return TestStatus::Pass;
@@ -93,18 +82,26 @@ fn test_status_program_compile_pass(
     // If we reach here, then the circuit execution failed.
     //
     // Check if the function should have passed
+    let diagnostic = try_to_diagnose_runtime_error(&circuit_execution_err, &debug);
     let test_should_have_passed = !test_function.should_fail();
     if test_should_have_passed {
-        return TestStatus::Fail { message: circuit_execution_err.to_string() };
+        return TestStatus::Fail {
+            message: circuit_execution_err.to_string(),
+            error_diagnostic: diagnostic,
+        };
     }
 
-    check_expected_failure_message(
-        test_function,
-        circuit_execution_err.user_defined_failure_message().unwrap_or_default(),
-    )
+    let assertion_message =
+        circuit_execution_err.user_defined_failure_message().unwrap_or_default().to_string();
+
+    check_expected_failure_message(test_function, &assertion_message, diagnostic)
 }
 
-fn check_expected_failure_message(test_function: TestFunction, got_error: &str) -> TestStatus {
+fn check_expected_failure_message(
+    test_function: TestFunction,
+    failed_assertion: &str,
+    error_diagnostic: Option<FileDiagnostic>,
+) -> TestStatus {
     // Extract the expected failure message, if there was one
     //
     // #[test(should_fail)] will not produce any message
@@ -115,7 +112,7 @@ fn check_expected_failure_message(test_function: TestFunction, got_error: &str)
         None => return TestStatus::Pass,
     };
 
-    let expected_failure_message_matches = got_error == expected_failure_message;
+    let expected_failure_message_matches = failed_assertion == expected_failure_message;
     if expected_failure_message_matches {
         return TestStatus::Pass;
     }
@@ -125,7 +122,8 @@ fn check_expected_failure_message(test_function: TestFunction, got_error: &str)
         message: format!(
             "\nerror: Test failed with the wrong message. \nExpected: {} \nGot: {}",
             test_function.failure_reason().unwrap_or_default(),
-            got_error.trim_matches('\'')
+            failed_assertion.trim_matches('\'')
         ),
+        error_diagnostic,
     }
 }