Repository: tweag/nickel
Pull request #1150 (2023-02-28 15:56:16)

[Refactor] ToDiagnostic -> IntoDiagnostics
Initially done as part of undertaking #1052, then separated as a standalone PR.

Errors use the trait (previously) `ToDiagnostic`, which provides a method `to_diagnostic(&self, ..) -> Vec<Diagnostic>`, but:

- Although the method takes `self` by reference, it's always used on `self` in practice (that is, we never use an error again after converting it to a diagnostic as of today). On the other hand, `&self` forces a bunch of needless clones in the implementations of `to_dagnostic`, for example having to clone `String` error messages stored in the error variant.
- The method is called `to_diagnostic` but returns a vector of diagnostics.

This PR replaces `ToDiagnostic` and `to_diagnostic` with `IntoDiagnostics` and `into_diagnostics`, taking `self` instead of `&self`, saving a few clones and reflecting more accurately the signature of the function.


Patch:
diff --git a/lsp/nls/src/files.rs b/lsp/nls/src/files.rs
index b97672f0..13ca8592 100644
--- a/lsp/nls/src/files.rs
+++ b/lsp/nls/src/files.rs
@@ -9,7 +9,7 @@ use lsp_types::{
 };
 use nickel_lang::{
     cache::{CacheError, CacheOp},
-    error::ToDiagnostic,
+    error::IntoDiagnostics,
 };
 
 use crate::trace::{param::FileUpdate, Enrich, Trace};
@@ -82,7 +82,7 @@ fn typecheck(server: &mut Server, file_id: FileId) -> Result<CacheOp<()>, Vec<Di
         .map_err(|error| match error {
             CacheError::Error(tc_error) => tc_error
                 .into_iter()
-                .flat_map(|err| err.to_diagnostic(server.cache.files_mut(), None))
+                .flat_map(|err| err.into_diagnostics(server.cache.files_mut(), None))
                 .collect(),
             CacheError::NotParsed => unreachable!(),
         })
@@ -92,12 +92,12 @@ fn parse_and_typecheck(server: &mut Server, uri: Url, file_id: FileId) -> Result
     let diagnostics = server
         .cache
         .parse(file_id)
-        .map_err(|parse_err| parse_err.to_diagnostic(server.cache.files_mut(), None))
+        .map_err(|parse_err| parse_err.into_diagnostics(server.cache.files_mut(), None))
         .map(|parse_errs| {
             // Parse errors are not fatal
             let mut d = parse_errs
                 .inner()
-                .to_diagnostic(server.cache.files_mut(), None);
+                .into_diagnostics(server.cache.files_mut(), None);
             trace!("Parsed, checking types");
             let _ = typecheck(server, file_id).map_err(|mut ty_d| d.append(&mut ty_d));
             d
diff --git a/src/error.rs b/src/error.rs
index 7635c414..0e943d82 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -302,15 +302,15 @@ impl From<Vec<ParseError>> for ParseErrors {
     }
 }
 
-impl ToDiagnostic<FileId> for ParseErrors {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for ParseErrors {
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
         self.errors
-            .iter()
-            .flat_map(|e| e.to_diagnostic(files, contract_id))
+            .into_iter()
+            .flat_map(|e| e.into_diagnostics(files, contract_id))
             .collect()
     }
 }
@@ -605,7 +605,7 @@ pub const INTERNAL_ERROR_MSG: &str =
  reporting it at https://github.com/tweag/nickel/issues with the above error message.";
 
 /// A trait for converting an error to a diagnostic.
-pub trait ToDiagnostic<FileId> {
+pub trait IntoDiagnostics<FileId> {
     /// Convert an error to a list of printable formatted diagnostic.
     ///
     /// # Arguments
@@ -622,8 +622,8 @@ pub trait ToDiagnostic<FileId> {
     /// ordered requires to sidestep a limitation of codespan. The current solution is to generate
     /// one diagnostic per callstack element. See issue
     /// [#285](https://github.com/brendanzab/codespan/issues/285).
-    fn to_diagnostic(
-        &self,
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>>;
@@ -745,31 +745,31 @@ fn secondary_term(term: &RichTerm, files: &mut Files<String>) -> Label<FileId> {
     secondary_alt(term.pos, term.as_ref().shallow_repr(), files)
 }
 
-impl ToDiagnostic<FileId> for Error {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for Error {
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
         match self {
             Error::ParseErrors(errs) => errs
                 .errors
-                .iter()
-                .flat_map(|e| e.to_diagnostic(files, contract_id))
+                .into_iter()
+                .flat_map(|e| e.into_diagnostics(files, contract_id))
                 .collect(),
-            Error::TypecheckError(err) => err.to_diagnostic(files, contract_id),
-            Error::EvalError(err) => err.to_diagnostic(files, contract_id),
-            Error::ImportError(err) => err.to_diagnostic(files, contract_id),
-            Error::SerializationError(err) => err.to_diagnostic(files, contract_id),
-            Error::IOError(err) => err.to_diagnostic(files, contract_id),
-            Error::ReplError(err) => err.to_diagnostic(files, contract_id),
+            Error::TypecheckError(err) => err.into_diagnostics(files, contract_id),
+            Error::EvalError(err) => err.into_diagnostics(files, contract_id),
+            Error::ImportError(err) => err.into_diagnostics(files, contract_id),
+            Error::SerializationError(err) => err.into_diagnostics(files, contract_id),
+            Error::IOError(err) => err.into_diagnostics(files, contract_id),
+            Error::ReplError(err) => err.into_diagnostics(files, contract_id),
         }
     }
 }
 
-impl ToDiagnostic<FileId> for EvalError {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for EvalError {
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
@@ -781,17 +781,17 @@ impl ToDiagnostic<FileId> for EvalError {
             } => {
                 let mut msg = String::new();
 
-                // Writing in a string should not raise an error, hence the fearless `unwrap()`
-                write!(&mut msg, "{}", blame_error::title(label)).unwrap();
+                write!(&mut msg, "{}", blame_error::title(&label))
+                    .expect("writing formatted data into a string should not raise an error");
 
                 if !label.tag.is_empty() {
                     write!(&mut msg, ": {}", &escape(&label.tag)).unwrap();
                 }
 
-                let (path_label, notes) = blame_error::report_ty_path(label, files);
+                let (path_label, notes) = blame_error::report_ty_path(&label, files);
                 let labels = blame_error::build_diagnostic_labels(
-                    evaluated_arg.clone(),
-                    label,
+                    evaluated_arg,
+                    &label,
                     path_label,
                     files,
                     contract_id,
@@ -802,11 +802,11 @@ impl ToDiagnostic<FileId> for EvalError {
                     .with_labels(labels)
                     .with_notes(notes)];
 
-                diagnostics.push(blame_error::note(label));
+                diagnostics.push(blame_error::note(&label));
 
                 if ty_path::has_no_dom(&label.path) {
                 } else if let Some(id) = contract_id {
-                    diagnostics.extend_with_call_stack(id, call_stack);
+                    diagnostics.extend_with_call_stack(id, &call_stack);
                 }
 
                 diagnostics
@@ -859,28 +859,28 @@ impl ToDiagnostic<FileId> for EvalError {
                 );
 
                 let labels = match orig_pos_opt {
-                    TermPos::Original(pos) | TermPos::Inherited(pos) if orig_pos_opt != &t.pos => {
+                    TermPos::Original(pos) | TermPos::Inherited(pos) if orig_pos_opt != t.pos => {
                         vec![
-                            primary(pos).with_message(label),
-                            secondary_term(t, files).with_message("evaluated to this"),
+                            primary(&pos).with_message(label),
+                            secondary_term(&t, files).with_message("evaluated to this"),
                         ]
                     }
-                    _ => vec![primary_term(t, files).with_message(label)],
+                    _ => vec![primary_term(&t, files).with_message(label)],
                 };
 
                 vec![Diagnostic::error()
                     .with_message("type error")
                     .with_labels(labels)
-                    .with_notes(vec![msg.clone()])]
+                    .with_notes(vec![msg])]
             }
-            EvalError::ParseError(parse_error) => parse_error.to_diagnostic(files, contract_id),
+            EvalError::ParseError(parse_error) => parse_error.into_diagnostics(files, contract_id),
             EvalError::NotAFunc(t, arg, pos_opt) => vec![Diagnostic::error()
                 .with_message("not a function")
                 .with_labels(vec![
-                    primary_term(t, files)
+                    primary_term(&t, files)
                         .with_message("this term is applied, but it is not a function"),
                     secondary_alt(
-                        *pos_opt,
+                        pos_opt,
                         format!(
                             "({}) ({})",
                             (*t.term).shallow_repr(),
@@ -893,7 +893,7 @@ impl ToDiagnostic<FileId> for EvalError {
             EvalError::FieldMissing(field, op, t, span_opt) => {
                 let mut labels = Vec::new();
                 let mut notes = Vec::new();
-                let field = escape(field);
+                let field = escape(&field);
 
                 if let Some(span) = span_opt.into_opt() {
                     labels.push(
@@ -935,12 +935,12 @@ impl ToDiagnostic<FileId> for EvalError {
             }
             EvalError::MergeIncompatibleArgs(t1, t2, span_opt) => {
                 let mut labels = vec![
-                    primary_term(t1, files).with_message("cannot merge this expression"),
-                    primary_term(t2, files).with_message("with this expression"),
+                    primary_term(&t1, files).with_message("cannot merge this expression"),
+                    primary_term(&t2, files).with_message("with this expression"),
                 ];
 
                 if let TermPos::Original(span) | TermPos::Inherited(span) = span_opt {
-                    labels.push(secondary(span).with_message("merged here"));
+                    labels.push(secondary(&span).with_message("merged here"));
                 }
 
                 vec![Diagnostic::error()
@@ -987,7 +987,7 @@ impl ToDiagnostic<FileId> for EvalError {
                     .with_labels(labels)
                     .with_notes(vec![String::from(INTERNAL_ERROR_MSG)])]
             }
-            EvalError::SerializationError(err) => err.to_diagnostic(files, contract_id),
+            EvalError::SerializationError(err) => err.into_diagnostics(files, contract_id),
             EvalError::DeserializationError(format, msg, span_opt) => {
                 let labels = span_opt
                     .as_opt_ref()
@@ -1008,14 +1008,14 @@ impl ToDiagnostic<FileId> for EvalError {
                 );
 
                 let labels = match eq_pos {
-                    TermPos::Original(pos) | TermPos::Inherited(pos) if eq_pos != &t.pos => {
+                    TermPos::Original(pos) | TermPos::Inherited(pos) if eq_pos != t.pos => {
                         vec![
-                            primary(pos).with_message(label),
-                            secondary_term(t, files)
+                            primary(&pos).with_message(label),
+                            secondary_term(&t, files)
                                 .with_message("problematic argument evaluated to this"),
                         ]
                     }
-                    _ => vec![primary_term(t, files).with_message(label)],
+                    _ => vec![primary_term(&t, files).with_message(label)],
                 };
 
                 vec![Diagnostic::error()
@@ -1030,16 +1030,16 @@ impl ToDiagnostic<FileId> for EvalError {
             } => {
                 let mut msg = String::new();
 
-                write!(&mut msg, "{}", blame_error::title(l)).unwrap();
+                write!(&mut msg, "{}", blame_error::title(&l)).unwrap();
                 write!(&mut msg, " - {}", action.message()).unwrap();
                 if !l.tag.is_empty() {
                     write!(&mut msg, ": {}", &escape(&l.tag)).unwrap();
                 }
 
-                let (path_label, notes) = blame_error::report_ty_path(l, files);
+                let (path_label, notes) = blame_error::report_ty_path(&l, files);
                 let labels = blame_error::build_diagnostic_labels(
-                    evaluated_arg.clone(),
-                    l,
+                    evaluated_arg,
+                    &l,
                     path_label,
                     files,
                     contract_id,
@@ -1050,11 +1050,11 @@ impl ToDiagnostic<FileId> for EvalError {
                     .with_labels(labels)
                     .with_notes(notes)];
 
-                diagnostics.push(blame_error::note(l));
+                diagnostics.push(blame_error::note(&l));
 
                 if ty_path::has_no_dom(&l.path) {
                 } else if let Some(id) = contract_id {
-                    diagnostics.extend_with_call_stack(id, call_stack);
+                    diagnostics.extend_with_call_stack(id, &call_stack);
                 }
 
                 diagnostics
@@ -1334,41 +1334,41 @@ is None but last_arrow_elem is Some"),
     }
 }
 
-impl ToDiagnostic<FileId> for ParseError {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for ParseError {
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         _contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
         let diagnostic = match self {
             ParseError::UnexpectedEOF(file_id, _expected) => {
-                let end = files.source_span(*file_id).end();
+                let end = files.source_span(file_id).end();
                 Diagnostic::error()
                     .with_message(format!(
                         "unexpected end of file when parsing {}",
-                        files.name(*file_id).to_string_lossy()
+                        files.name(file_id).to_string_lossy()
                     ))
                     .with_labels(vec![primary(&RawSpan {
                         start: end,
                         end,
-                        src_id: *file_id,
+                        src_id: file_id,
                     })])
             }
             ParseError::UnexpectedToken(span, _expected) => Diagnostic::error()
                 .with_message("unexpected token")
-                .with_labels(vec![primary(span)]),
+                .with_labels(vec![primary(&span)]),
             ParseError::ExtraToken(span) => Diagnostic::error()
                 .with_message("superfluous unexpected token")
-                .with_labels(vec![primary(span)]),
+                .with_labels(vec![primary(&span)]),
             ParseError::UnmatchedCloseBrace(span) => Diagnostic::error()
                 .with_message("unmatched closing brace \'}\'")
-                .with_labels(vec![primary(span)]),
+                .with_labels(vec![primary(&span)]),
             ParseError::InvalidEscapeSequence(span) => Diagnostic::error()
                 .with_message("invalid escape sequence")
-                .with_labels(vec![primary(span)]),
+                .with_labels(vec![primary(&span)]),
             ParseError::InvalidAsciiEscapeCode(span) => Diagnostic::error()
                 .with_message("invalid ascii escape code")
-                .with_labels(vec![primary(span)]),
+                .with_labels(vec![primary(&span)]),
             ParseError::ExternalFormatError(format, msg, span_opt) => {
                 let labels = span_opt
                     .as_ref()
@@ -1388,13 +1388,13 @@ impl ToDiagnostic<FileId> for ParseError {
                         .collect::<Vec<_>>()
                         .join(",")
                 ))
-                .with_labels(vec![primary(span)]),
+                .with_labels(vec![primary(&span)]),
             ParseError::InvalidUniRecord(illegal_span, tail_span, span) => Diagnostic::error()
                 .with_message("invalid record literal")
                 .with_labels(vec![
-                    primary(span),
-                    secondary(illegal_span).with_message("can't use this record construct"),
-                    secondary(tail_span).with_message("in presence of a tail"),
+                    primary(&span),
+                    secondary(&illegal_span).with_message("can't use this record construct"),
+                    secondary(&tail_span).with_message("in presence of a tail"),
                 ])
                 .with_notes(vec![
                     String::from("Using a polymorphic tail in a record `{ ..; a}` requires the rest of the record to be only composed of type annotations, of the form `<field>: <type>`."),
@@ -1403,7 +1403,7 @@ impl ToDiagnostic<FileId> for ParseError {
             ParseError::RecursiveLetPattern(span) => Diagnostic::error()
                 .with_message("recursive destructuring is not supported")
                 .with_labels(vec![
-                    primary(span),
+                    primary(&span),
                 ])
                 .with_notes(vec![
                     String::from("A destructuring let-binding can't be recursive. Try removing the `rec` from `let rec`."),
@@ -1412,7 +1412,7 @@ impl ToDiagnostic<FileId> for ParseError {
             ParseError::TypeVariableKindMismatch { ty_var, span } => Diagnostic::error()
                 .with_message(format!("the type variable {ty_var} is used in conflicting ways"))
                 .with_labels(vec![
-                    primary(span),
+                    primary(&span),
                 ])
                 .with_notes(vec![
                     String::from("Type variables may be used either as types, polymorphic record tails, or polymorphic enum tails."),
@@ -1424,9 +1424,9 @@ impl ToDiagnostic<FileId> for ParseError {
     }
 }
 
-impl ToDiagnostic<FileId> for TypecheckError {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for TypecheckError {
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
@@ -1441,13 +1441,13 @@ impl ToDiagnostic<FileId> for TypecheckError {
             TypecheckError::UnboundIdentifier(ident, pos_opt) =>
             // Use the same diagnostic as `EvalError::UnboundIdentifier` for consistency.
                 {
-                    EvalError::UnboundIdentifier(*ident, *pos_opt)
-                        .to_diagnostic(files, contract_id)
+                    EvalError::UnboundIdentifier(ident, pos_opt)
+                        .into_diagnostics(files, contract_id)
                 }
             TypecheckError::MissingRow(ident, expd, actual, span_opt) =>
                 vec![Diagnostic::error()
                     .with_message(format!("type error: missing row `{ident}`"))
-                    .with_labels(mk_expr_label(span_opt))
+                    .with_labels(mk_expr_label(&span_opt))
                     .with_notes(vec![
                         format!("The type of the expression was expected to be `{expd}` which contains the field `{ident}`"),
                         format!("The type of the expression was inferred to be `{actual}`, which does not contain the field `{ident}`"),
@@ -1456,7 +1456,7 @@ impl ToDiagnostic<FileId> for TypecheckError {
             TypecheckError::MissingDynTail(expd, actual, span_opt) =>
                 vec![Diagnostic::error()
                     .with_message(String::from("type error: missing dynamic tail `| Dyn`"))
-                    .with_labels(mk_expr_label(span_opt))
+                    .with_labels(mk_expr_label(&span_opt))
                     .with_notes(vec![
                         format!("The type of the expression was expected to be `{expd}` which contains the tail `| Dyn`"),
                         format!("The type of the expression was inferred to be `{actual}`, which does not contain the tail `| Dyn`"),
@@ -1466,7 +1466,7 @@ impl ToDiagnostic<FileId> for TypecheckError {
             TypecheckError::ExtraRow(ident, expd, actual, span_opt) =>
                 vec![Diagnostic::error()
                     .with_message(format!("type error: extra row `{ident}`"))
-                    .with_labels(mk_expr_label(span_opt))
+                    .with_labels(mk_expr_label(&span_opt))
                     .with_notes(vec![
                         format!("The type of the expression was expected to be `{expd}`, which does not contain the field `{ident}`"),
                         format!("The type of the expression was inferred to be `{actual}`, which contains the extra field `{ident}`"),
@@ -1475,7 +1475,7 @@ impl ToDiagnostic<FileId> for TypecheckError {
             TypecheckError::ExtraDynTail(expd, actual, span_opt) =>
                 vec![Diagnostic::error()
                     .with_message(String::from("type error: extra dynamic tail `| Dyn`"))
-                    .with_labels(mk_expr_label(span_opt))
+                    .with_labels(mk_expr_label(&span_opt))
                     .with_notes(vec![
                         format!("The type of the expression was expected to be `{expd}`, which does not contain the tail `| Dyn`"),
                         format!("The type of the expression was inferred to be `{actual}`, which contains the extra tail `| Dyn`"),
@@ -1507,10 +1507,10 @@ impl ToDiagnostic<FileId> for TypecheckError {
                 vec![
                     Diagnostic::error()
                         .with_message("incompatible types")
-                        .with_labels(mk_expr_label(span_opt))
+                        .with_labels(mk_expr_label(&span_opt))
                         .with_notes(vec![
-                            format!("The type of the expression was expected to be `{}`{}", expd, addendum(expd)),
-                            format!("The type of the expression was inferred to be `{}`{}", actual, addendum(actual)),
+                            format!("The type of the expression was expected to be `{}`{}", expd, addendum(&expd)),
+                            format!("The type of the expression was inferred to be `{}`{}", actual, addendum(&actual)),
                             String::from(last_note),
                         ])]
             }
@@ -1524,20 +1524,19 @@ impl ToDiagnostic<FileId> for TypecheckError {
                 vec![
                     Diagnostic::error()
                         .with_message("incompatible row kinds")
-                        .with_labels(mk_expr_label(span_opt))
+                        .with_labels(mk_expr_label(&span_opt))
                         .with_notes(vec![
                             format!("The row type of `{ident}` was expected to be `{expd_str}`, but was inferred to be `{actual_str}`"),
                             String::from("Enum row types and record row types are not compatible"),
                         ])]
             }
-            TypecheckError::RowMismatch(ident, expd, actual, err_, span_opt) => {
+            TypecheckError::RowMismatch(ident, expd, actual, mut err, span_opt) => {
                 // If the unification error is on a nested field, we will have a succession of
                 // `RowMismatch` errors wrapping the underlying error. In this case, instead of
                 // showing a cascade of similar error messages, we determine the full path of the
                 // nested field (e.g. `pkg.subpkg1.meta.url`) and only show once the row mismatch
                 // error followed by the underlying error.
-                let mut err = (*err_).clone();
-                let mut path = vec![*ident];
+                let mut path = vec![ident];
 
                 while let TypecheckError::RowMismatch(id_next, _, _, next, _) = *err {
                     path.push(id_next);
@@ -1572,7 +1571,7 @@ impl ToDiagnostic<FileId> for TypecheckError {
 
                 let mut diags = vec![Diagnostic::error()
                     .with_message("incompatible rows declaration")
-                    .with_labels(mk_expr_label(span_opt))
+                    .with_labels(mk_expr_label(&span_opt))
                     .with_notes(vec![
                         note1,
                         note2,
@@ -1583,7 +1582,7 @@ impl ToDiagnostic<FileId> for TypecheckError {
                 // We generate a diagnostic for the underlying error, but append a prefix to the
                 // error message to make it clear that this is not a separated error but a more
                 // precise description of why the unification of a row failed.
-                diags.extend((*err).to_diagnostic(files, contract_id).into_iter()
+                diags.extend((*err).into_diagnostics(files, contract_id).into_iter()
                     .map(|mut diag| {
                         diag.message = format!("While typing field `{}`: {}", field, diag.message);
                         diag
@@ -1594,7 +1593,7 @@ impl ToDiagnostic<FileId> for TypecheckError {
                 vec![
                     Diagnostic::error()
                         .with_message("multiple rows declaration")
-                        .with_labels(mk_expr_label(span_opt))
+                        .with_labels(mk_expr_label(&span_opt))
                         .with_notes(vec![
                             format!("The type of the expression was inferred to have the row `{}: {}`", ident, conflict.as_ref().cloned().unwrap()),
                             format!("But this type appears inside another row type, which already has a declaration for the field `{ident}`"),
@@ -1602,8 +1601,8 @@ impl ToDiagnostic<FileId> for TypecheckError {
                         ])]
             }
             TypecheckError::ArrowTypeMismatch(expd, actual, path, err, span_opt) => {
-                let PathSpan {start: expd_start, end: expd_end, ..} = ty_path::span(path.iter().peekable(), expd);
-                let PathSpan {start: actual_start, end: actual_end, ..} = ty_path::span(path.iter().peekable(), actual);
+                let PathSpan {start: expd_start, end: expd_end, ..} = ty_path::span(path.iter().peekable(), &expd);
+                let PathSpan {start: actual_start, end: actual_end, ..} = ty_path::span(path.iter().peekable(), &actual);
 
                 let mut labels = vec![
                     Label::secondary(
@@ -1617,7 +1616,7 @@ impl ToDiagnostic<FileId> for TypecheckError {
                     )
                         .with_message("does not match this part of the inferred type"),
                 ];
-                labels.extend(mk_expr_label(span_opt));
+                labels.extend(mk_expr_label(&span_opt));
 
                 let mut diags = vec![Diagnostic::error()
                     .with_message("function types mismatch")
@@ -1632,12 +1631,12 @@ impl ToDiagnostic<FileId> for TypecheckError {
                 // We generate a diagnostic for the underlying error, but append a prefix to the
                 // error message to make it clear that this is not a separated error but a more
                 // precise description of why the unification of the row failed.
-                match err.as_ref() {
+                match *err {
                     // If the underlying error is a type mismatch, printing won't add any useful
                     // information, so we just ignore it.
                     TypecheckError::TypeMismatch(_, _, _) => (),
                     err => {
-                        diags.extend(err.to_diagnostic(files, contract_id).into_iter()
+                        diags.extend(err.into_diagnostics(files, contract_id).into_iter()
                             .map(|mut diag| {
                                 diag.message = format!("While matching function types: {}", diag.message);
                                 diag
@@ -1650,7 +1649,7 @@ impl ToDiagnostic<FileId> for TypecheckError {
             TypecheckError::IncomparableFlatTypes(expd, actual, span_opt) => {
                 vec![Diagnostic::error()
                     .with_message("can't compare contract types")
-                    .with_labels(mk_expr_label(span_opt))
+                    .with_labels(mk_expr_label(&span_opt))
                     .with_notes(vec![
                         format!("The type of the expression was expected to be `{}`", expd.as_ref().shallow_repr()),
                         format!("The type of the expression was inferred to be `{}`", actual.as_ref().shallow_repr()),
@@ -1668,9 +1667,9 @@ impl ToDiagnostic<FileId> for TypecheckError {
     }
 }
 
-impl ToDiagnostic<FileId> for ImportError {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for ImportError {
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
@@ -1688,8 +1687,8 @@ impl ToDiagnostic<FileId> for ImportError {
             ImportError::ParseErrors(error, span_opt) => {
                 let mut diagnostic: Vec<Diagnostic<FileId>> = error
                     .errors
-                    .iter()
-                    .flat_map(|e| e.to_diagnostic(files, contract_id))
+                    .into_iter()
+                    .flat_map(|e| e.into_diagnostics(files, contract_id))
                     .collect();
 
                 if let Some(span) = span_opt.as_opt_ref() {
@@ -1704,9 +1703,9 @@ impl ToDiagnostic<FileId> for ImportError {
     }
 }
 
-impl ToDiagnostic<FileId> for SerializationError {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for SerializationError {
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         _contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
@@ -1718,35 +1717,35 @@ impl ToDiagnostic<FileId> for SerializationError {
                         .type_of()
                         .unwrap_or_else(|| String::from("<unevaluated>"))
                 ))
-                .with_labels(vec![primary_term(rt, files)])],
+                .with_labels(vec![primary_term(&rt, files)])],
             SerializationError::UnsupportedNull(format, rt) => vec![Diagnostic::error()
                 .with_message(format!("{format} doesn't support null values"))
-                .with_labels(vec![primary_term(rt, files)])],
+                .with_labels(vec![primary_term(&rt, files)])],
             SerializationError::NonSerializable(rt) => vec![Diagnostic::error()
                 .with_message("non serializable term")
-                .with_labels(vec![primary_term(rt, files)])],
+                .with_labels(vec![primary_term(&rt, files)])],
             SerializationError::Other(msg) => vec![Diagnostic::error()
                 .with_message("error during serialization")
-                .with_notes(vec![msg.clone()])],
+                .with_notes(vec![msg])],
         }
     }
 }
 
-impl ToDiagnostic<FileId> for IOError {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for IOError {
+    fn into_diagnostics(
+        self,
         _files: &mut Files<String>,
         _contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
         match self {
-            IOError(msg) => vec![Diagnostic::error().with_message(msg.clone())],
+            IOError(msg) => vec![Diagnostic::error().with_message(msg)],
         }
     }
 }
 
-impl ToDiagnostic<FileId> for ReplError {
-    fn to_diagnostic(
-        &self,
+impl IntoDiagnostics<FileId> for ReplError {
+    fn into_diagnostics(
+        self,
         files: &mut Files<String>,
         contract_id: Option<FileId>,
     ) -> Vec<Diagnostic<FileId>> {
@@ -1756,7 +1755,7 @@ impl ToDiagnostic<FileId> for ReplError {
                 .with_notes(vec![String::from(
                     "type `:?` or `:help` for a list of available commands.",
                 )])],
-            ReplError::InvalidQueryPath(err) => err.to_diagnostic(files, contract_id),
+            ReplError::InvalidQueryPath(err) => err.into_diagnostics(files, contract_id),
             ReplError::MissingArg { cmd, msg_opt } => {
                 let mut notes = msg_opt
                     .as_ref()
diff --git a/src/program.rs b/src/program.rs
index 279dbda3..b05c95a6 100644
--- a/src/program.rs
+++ b/src/program.rs
@@ -21,7 +21,7 @@
 //! functions in [`crate::cache`] (see [`crate::cache::Cache::mk_eval_env`]).
 //! Each such value is added to the initial environment before the evaluation of the program.
 use crate::cache::*;
-use crate::error::{Error, ParseError, ToDiagnostic};
+use crate::error::{Error, IntoDiagnostics, ParseError};
 use crate::eval;
 use crate::eval::cache::Cache as EvalCache;
 use crate::eval::VirtualMachine;
@@ -277,7 +277,7 @@ impl<EC: EvalCache> Program<EC> {
     /// Wrapper for [`report`].
     pub fn report<E>(&mut self, error: E)
     where
-        E: ToDiagnostic<FileId>,
+        E: IntoDiagnostics<FileId>,
     {
         report(self.vm.import_resolver_mut(), error, self.color_opt)
     }
@@ -285,11 +285,11 @@ impl<EC: EvalCache> Program<EC> {
     /// Build an error report as a string and return it.
     pub fn report_as_str<E>(&mut self, error: E) -> String
     where
-        E: ToDiagnostic<FileId>,
+        E: IntoDiagnostics<FileId>,
     {
         let cache = self.vm.import_resolver_mut();
         let contracts_id = cache.id_of("<stdlib/contract.ncl>");
-        let diagnostics = error.to_diagnostic(cache.files_mut(), contracts_id);
+        let diagnostics = error.into_diagnostics(cache.files_mut(), contracts_id);
         let mut buffer = Ansi::new(Cursor::new(Vec::new()));
         let config = codespan_reporting::term::Config::default();
         // write to `buffer`
@@ -375,12 +375,12 @@ pub fn query<EC: EvalCache>(
 //common to both `Program` and `Repl`. Leaving it here as a stand-alone function for now
 pub fn report<E>(cache: &mut Cache, error: E, color_opt: ColorOpt)
 where
-    E: ToDiagnostic<FileId>,
+    E: IntoDiagnostics<FileId>,
 {
     let writer = StandardStream::stderr(color_opt.into());
     let config = codespan_reporting::term::Config::default();
     let contracts_id = cache.id_of("<stdlib/contract.ncl>");
-    let diagnostics = error.to_diagnostic(cache.files_mut(), contracts_id);
+    let diagnostics = error.into_diagnostics(cache.files_mut(), contracts_id);
 
     let result = diagnostics.iter().try_for_each(|d| {
         codespan_reporting::term::emit(&mut writer.lock(), &config, cache.files_mut(), d)
diff --git a/src/repl/wasm_frontend.rs b/src/repl/wasm_frontend.rs
index 78c991a7..1fd04a47 100644
--- a/src/repl/wasm_frontend.rs
+++ b/src/repl/wasm_frontend.rs
@@ -2,7 +2,7 @@
 use super::simple_frontend::{input, serialize, InputError, InputResult};
 use super::{Repl, ReplImpl};
 use crate::cache::Cache;
-use crate::error::ToDiagnostic;
+use crate::error::IntoDiagnostics;
 use crate::eval::cache::CacheImpl;
 use crate::serialize::ExportFormat;
 use codespan::{FileId, Files};
@@ -163,7 +163,7 @@ impl WasmInitResult {
     /// Make a `WasmInitResult` result from an `InputError`.
     fn error(mut state: ReplState, error: InputError) -> Self {
         WasmInitResult {
-            msg: err_to_string(state.0.cache_mut(), &error),
+            msg: err_to_string(state.0.cache_mut(), error),
             tag: WasmResultTag::Error,
             state,
         }
@@ -195,7 +195,7 @@ impl WasmInputResult {
         let (msg, errors) = match error {
             InputError::NickelError(err) => {
                 let contracts_id = cache.id_of("<stdlib/contract.ncl>");
-                let diagnostics = err.to_diagnostic(cache.files_mut(), contracts_id);
+                let diagnostics = err.into_diagnostics(cache.files_mut(), contracts_id);
 
                 let msg = diags_to_string(cache, &diagnostics);
                 let errors: Vec<WasmErrorDiagnostic> = diagnostics
@@ -287,14 +287,14 @@ pub fn diags_to_string(cache: &mut Cache, diags: &[Diagnostic<FileId>]) -> Strin
 }
 
 /// Render an error as a string (similar to [`diags_to_string`](./meth.diags_to_string.html)).
-pub fn err_to_string(cache: &mut Cache, error: &InputError) -> String {
+pub fn err_to_string(cache: &mut Cache, error: InputError) -> String {
     match error {
         InputError::NickelError(nickel_err) => {
             let contracts_id = cache.id_of("<stdlib/contract.ncl>");
-            let diags = nickel_err.to_diagnostic(cache.files_mut(), contracts_id);
+            let diags = nickel_err.into_diagnostics(cache.files_mut(), contracts_id);
             diags_to_string(cache, &diags)
         }
-        InputError::Other(msg) => msg.clone(),
+        InputError::Other(msg) => msg,
     }
 }
 


Test patch:
diff --git a/tests/integration/contracts_fail.rs b/tests/integration/contracts_fail.rs
index 98984e97..60f98f2a 100644
--- a/tests/integration/contracts_fail.rs
+++ b/tests/integration/contracts_fail.rs
@@ -1,6 +1,6 @@
 use assert_matches::assert_matches;
 use codespan::Files;
-use nickel_lang::error::{Error, EvalError, ToDiagnostic};
+use nickel_lang::error::{Error, EvalError, IntoDiagnostics};
 
 use nickel_lang_utilities::{eval, program_from_expr};
 
@@ -259,7 +259,7 @@ fn lists_contracts() {
     // Check that reporting doesn't panic. Provide a dummy file database, as we won't report
     // the error message but just check that it can be built.
     let mut files = Files::new();
-    res.unwrap_err().to_diagnostic(&mut files, None);
+    res.unwrap_err().into_diagnostics(&mut files, None);
 
     let res = eval(
         "(%elem_at% (({foo = [(fun x => \"a\")]} | {foo: Array (forall a. a -> Num)}).foo) 0) false",
@@ -274,7 +274,7 @@ fn lists_contracts() {
         }
         err => panic!("expected blame error, got {err:?}"),
     }
-    res.unwrap_err().to_diagnostic(&mut files, None);
+    res.unwrap_err().into_diagnostics(&mut files, None);
 }
 
 #[test]