Fix tests

clippy

cli/src/global.rs

@@ -0,0 +1,83 @@
+//! Global state for the nickel CLI.
+
+use nickel_lang_core::{
+    error::Error as CoreError, error::Reporter, eval::cache::lazy::CBNCache, files::Files,
+    program::Program,
+};
+
+use crate::{
+    cli::GlobalOptions,
+    error::{Error, Warning},
+    input::{Prepare, PrepareError},
+};
+
+pub struct GlobalContext {
+    pub opts: GlobalOptions,
+    pub errors: Vec<Error>,
+    pub warnings: Vec<Warning>,
+}
+
+impl GlobalContext {
+    pub fn new(opts: GlobalOptions) -> Self {
+        GlobalContext {
+            opts,
+            errors: Vec::new(),
+            warnings: Vec::new(),
+        }
+    }
+
+    // This is just an alias for the trait method, but having an inherent impl
+    // makes type inference better because rust defaults to the inherit impl and
+    // doesn't get confused between `Warning` and `Error`.
+    pub fn report_result<T, E: Into<Error>>(&mut self, result: Result<T, E>) {
+        <Self as Reporter<Error>>::report_result(self, result)
+    }
+
+    pub fn with_program<
+        T,
+        P: Prepare,
+        F: FnOnce(&mut Program<CBNCache>) -> Result<T, CoreError>,
+    >(
+        &mut self,
+        preparer: &P,
+        f: F,
+    ) -> Option<T> {
+        let result = match preparer.prepare(self) {
+            Ok(mut prog) => f(&mut prog).map_err(|error| {
+                Error::Program {
+                    error,
+                    files: prog.files(),
+                }
+                .into()
+            }),
+            Err(e) => Err(e),
+        };
+
+        match result {
+            Err(PrepareError::Error(error)) => {
+                self.report(error);
+                None
+            }
+            Err(PrepareError::EarlyReturn) => None,
+            Ok(x) => Some(x),
+        }
+    }
+}
+
+impl Reporter<Error> for GlobalContext {
+    fn report(&mut self, e: Error) {
+        self.errors.push(e);
+    }
+}
+
+impl Reporter<Warning> for GlobalContext {
+    fn report(&mut self, w: Warning) {
+        self.warnings.push(w);
+    }
+}
+
+impl Reporter<(nickel_lang_core::error::Warning, Files)> for GlobalContext {
+    fn report(&mut self, (warning, files): (nickel_lang_core::error::Warning, Files)) {
+        self.warnings.push(Warning::Program { warning, files });
+    }
+}

cli/tests/snapshot/snapshots/snapshot__eval_stderr_contract_with_custom_diagnostic.ncl.snap

@@ -2,6 +2,24 @@
 source: cli/tests/snapshot/main.rs
 expression: err
 ---
+warning: plain functions as contracts are deprecated
+   ┌─ [INPUTS_PATH]/errors/contract_with_custom_diagnostic.ncl:13:1
+   │  
+ 3 │   let Contract = fun label _value =>
+   │ ╭────────────────'
+ 4 │ │   label
+ 5 │ │   |> std.contract.label.with_message "main error message"
+ 6 │ │   |> std.contract.label.with_notes [
+   · │
+ 9 │ │     ]
+10 │ │   |> std.contract.blame
+   │ ╰───────────────────────' this function
+   · │
+13 │   null | Contract
+   │   ^^^^ applied to this term
+   │  
+   = use one of the constructors in `std.contract` instead, like `std.contract.custom`
+
 error: contract broken by a value
        main error message
    ┌─ [INPUTS_PATH]/errors/contract_with_custom_diagnostic.ncl:13:1
@@ -13,5 +31,3 @@ error: contract broken by a value
    │
    = This is the first note
    = This is the second note
-
-

cli/tests/snapshot/snapshots/snapshot__eval_stderr_subcontract_nested_custom_diagnostics.ncl.snap

@@ -2,6 +2,42 @@
 source: cli/tests/snapshot/main.rs
 expression: err
 ---
+warning: plain functions as contracts are deprecated
+   ┌─ [INPUTS_PATH]/errors/subcontract_nested_custom_diagnostics.ncl:19:1
+   │  
+10 │   let ParentContract = fun label value =>
+   │ ╭──────────────────────'
+11 │ │   let label =
+12 │ │     label
+13 │ │     |> std.contract.label.with_message "parent's message"
+14 │ │     |> std.contract.label.append_note "parent's note"
+15 │ │   in
+16 │ │   std.contract.apply ChildContract label value
+   │ ╰──────────────────────────────────────────────' this function
+   · │
+19 │   null | ParentContract
+   │   ^^^^ applied to this term
+   │  
+   = use one of the constructors in `std.contract` instead, like `std.contract.custom`
+
+warning: plain functions as contracts are deprecated
+     ┌─ <stdlib/std.ncl>:1549:9
+     │
+1549 │         %contract/apply% contract (%label/push_diag% label) value,
+     │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ applied to this term
+     │
+     ┌─ [INPUTS_PATH]/errors/subcontract_nested_custom_diagnostics.ncl:3:21
+     │  
+   3 │   let ChildContract = fun label value =>
+     │ ╭─────────────────────'
+   4 │ │   label
+   5 │ │   |> std.contract.label.with_message "child's message"
+   6 │ │   |> std.contract.label.append_note "child's note"
+   7 │ │   |> std.contract.blame
+     │ ╰───────────────────────' this function
+     │  
+     = use one of the constructors in `std.contract` instead, like `std.contract.custom`
+
 error: contract broken by a value
        child's message
    ┌─ [INPUTS_PATH]/errors/subcontract_nested_custom_diagnostics.ncl:19:8
@@ -18,5 +54,3 @@ error: contract broken by a value
 
 note: from a parent contract violation: parent's message
  = parent's note
-
-

cli/tests/snapshot/snapshots/snapshot__export_stderr_non_serializable_print_path.ncl.snap

@@ -2,6 +2,17 @@
 source: cli/tests/snapshot/main.rs
 expression: err
 ---
+warning: plain functions as contracts are deprecated
+   ┌─ [INPUTS_PATH]/errors/non_serializable_print_path.ncl:17:63
+   │
+ 8 │ let SomeParametricContract = fun parameter label value => value
+   │                              ---------------------------------- this function
+   ·
+17 │           inner = { qux_miss_param | SomeParametricContract = {} },
+   │                                                               ^^ applied to this term
+   │
+   = use one of the constructors in `std.contract` instead, like `std.contract.custom`
+
 error: non serializable term
   ┌─ [INPUTS_PATH]/errors/non_serializable_print_path.ncl:8:30
   │

core/src/deserialize.rs

@@ -584,13 +584,13 @@ mod tests {
     use std::io::Cursor;
 
     use nickel_lang_utils::{
-        nickel_lang_core::{deserialize::RustDeserializationError, term::RichTerm},
+        nickel_lang_core::{
+            deserialize::RustDeserializationError, error::NullReporter, term::RichTerm,
+        },
         test_program::TestProgram,
     };
     use serde::Deserialize;
 
-    use crate::error::NullReporter;
-
     fn eval(source: &str) -> RichTerm {
         TestProgram::new_from_source(
             Cursor::new(source),

core/src/error/warning.rs

@@ -0,0 +1,40 @@
+use codespan_reporting::diagnostic::Diagnostic;
+
+use crate::error::IntoDiagnostics;
+use crate::files::{FileId, Files};
+use crate::position::TermPos;
+use crate::term::RichTerm;
+
+use super::{primary, secondary_term};
+
+pub enum Warning {
+    /// Applied a `fun label value => ...` (or match) as a contract directly,
+    /// instead of using the constract constructors in the standard library.
+    NakedFunctionContract {
+        /// The position of the function that was used as a contract.
+        func: RichTerm,
+        /// The position of the thing that the contract was applied to.
+        app_pos: TermPos,
+    },
+}
+
+impl IntoDiagnostics for Warning {
+    fn into_diagnostics(self, files: &mut Files) -> Vec<Diagnostic<FileId>> {
+        match self {
+            Warning::NakedFunctionContract { func, app_pos } => {
+                let mut labels = vec![];
+                if let Some(span) = app_pos.into_opt() {
+                    labels.push(primary(&span).with_message("applied to this term"));
+                }
+
+                labels.push(secondary_term(&func, files).with_message("this function"));
+
+                vec![Diagnostic::warning()
+                    .with_message("plain functions as contracts are deprecated")
+                    .with_labels(labels)
+                    .with_notes(vec!["use one of the constructors in `std.contract` instead, like `std.contract.custom`".to_owned()])
+                ]
+            }
+        }
+    }
+}

core/src/eval/operation.rs

@@ -1858,10 +1858,16 @@ impl<'ctx, R: ImportResolver, C: Cache> VirtualMachine<'ctx, R, C> {
                                 pos: pos1,
                             };
 
-                            self.warn(Warning::NakedFunctionContract {
-                                func: as_naked.clone(),
-                                app_pos: pos_op,
-                            });
+                            // Warn on naked function contracts, but not if they came from the
+                            // stdlib. Some the stdlib functions return naked function contracts.
+                            if let Some(pos) = pos1.as_opt_ref() {
+                                if !self.import_resolver.files().is_stdlib(pos.src_id) {
+                                    self.warn(Warning::NakedFunctionContract {
+                                        func: as_naked.clone(),
+                                        app_pos: pos_op,
+                                    });
+                                }
+                            }
 
                             if let BinaryOp::ContractApply = b_op {
                                 Closure {

core/src/program.rs

@@ -1134,7 +1134,7 @@ mod doc {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::error::EvalError;
+    use crate::error::{EvalError, NullReporter};
     use crate::eval::cache::CacheImpl;
     use crate::identifier::LocIdent;
     use crate::position::TermPos;
@@ -1146,7 +1146,7 @@ mod tests {
         let src = Cursor::new(s);
 
         let mut p: Program<CacheImpl> =
-            Program::new_from_source(src, "<test>", std::io::sink(), NullReporter).map_err(
+            Program::new_from_source(src, "<test>", std::io::sink(), NullReporter {}).map_err(
                 |io_err| {
                     Error::EvalError(EvalError::Other(
                         format!("IO error: {io_err}"),
@@ -1161,7 +1161,7 @@ mod tests {
         let src = Cursor::new(s);
 
         let mut p: Program<CacheImpl> =
-            Program::new_from_source(src, "<test>", std::io::sink(), NullReporter).map_err(
+            Program::new_from_source(src, "<test>", std::io::sink(), NullReporter {}).map_err(
                 |io_err| {
                     Error::EvalError(EvalError::Other(
                         format!("IO error: {io_err}"),

core/src/serialize.rs

@@ -583,6 +583,7 @@ pub mod toml_deser {
 mod tests {
     use super::*;
     use crate::cache::resolvers::DummyResolver;
+    use crate::error::NullReporter;
     use crate::eval::cache::CacheImpl;
     use crate::eval::VirtualMachine;
     use crate::program::Program;
@@ -592,8 +593,13 @@ mod tests {
 
     fn eval(s: &str) -> RichTerm {
         let src = Cursor::new(s);
-        let mut prog =
-            Program::<CacheImpl>::new_from_source(src, "<test>", std::io::stderr()).unwrap();
+        let mut prog = Program::<CacheImpl>::new_from_source(
+            src,
+            "<test>",
+            std::io::stderr(),
+            NullReporter {},
+        )
+        .unwrap();
         prog.eval_full().expect("program eval should succeed")
     }
 
@@ -608,9 +614,13 @@ mod tests {
     #[track_caller]
     fn assert_nickel_eq(term: RichTerm, expected: RichTerm) {
         assert_eq!(
-            VirtualMachine::<_, CacheImpl>::new(DummyResolver {}, std::io::stderr())
-                .eval(mk_term::op2(BinaryOp::Eq, term, expected))
-                .map(Term::from),
+            VirtualMachine::<_, CacheImpl>::new(
+                DummyResolver {},
+                std::io::stderr(),
+                NullReporter {}
+            )
+            .eval(mk_term::op2(BinaryOp::Eq, term, expected))
+            .map(Term::from),
             Ok(Term::Bool(true))
         )
     }

core/tests/examples/main.rs

@@ -14,9 +14,12 @@ fn check_example_file(path: &str) {
         read_annotated_test_case(path).expect("Failed to parse annotated program");
 
     // `test_resources` uses paths relative to the workspace manifesty
-    let mut p =
-        TestProgram::new_from_file(project_root().join(path), std::io::stderr(), NullReporter)
-            .expect("Failed to load program from file");
+    let mut p = TestProgram::new_from_file(
+        project_root().join(path),
+        std::io::stderr(),
+        NullReporter {},
+    )
+    .expect("Failed to load program from file");
 
     match test.annotation {
         Expectation::Pass => {

core/tests/integration/main.rs

@@ -2,7 +2,8 @@ use std::{io::Cursor, thread};
 
 use nickel_lang_core::{
     error::{
-        Error, EvalError, ExportError, ExportErrorData, ImportError, ParseError, TypecheckError,
+        Error, EvalError, ExportError, ExportErrorData, ImportError, NullReporter, ParseError,
+        TypecheckError,
     },
     term::Term,
     typecheck::TypecheckMode,
@@ -70,6 +71,7 @@ fn run_test(test_case: TestCase<Test>, path: String) {
             Cursor::new(program.clone()),
             path.as_str(),
             std::io::stderr(),
+            NullReporter {},
         )
         .expect("");
         if let Some(imports) = &test_case.annotation.nickel_path {