Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include field path in non serializable error #1905

Merged
merged 1 commit into from
May 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions cli/tests/snapshot/inputs/errors/non_serializable_print_path.ncl
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# capture = 'stderr'
# command = ['export']

# Check that a non-serializable error prints the path in the notes. This is
# particularly useful in the case tested below, when we haven't fully applied a
# contract, because the error points to the contract definition site (at the
# time of writing), and not the usage site, which is not very useful.
let SomeParametricContract = fun parameter label value => value
in
{
foo.bar.baz =
[0, 1]
@ [
2,
(
{
inner = { qux_miss_param | SomeParametricContract = {} },
}
& { inner.qux_without_error = 1 }
),
],
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: cli/tests/snapshot/main.rs
expression: err
---
error: non serializable term
┌─ [INPUTS_PATH]/errors/non_serializable_print_path.ncl:8:30
8 │ let SomeParametricContract = fun parameter label value => value
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= When exporting field `foo.bar.baz[3].inner.qux_miss_param`
= Nickel only supports serializing to and from strings, booleans, numbers, enum tags, `null` (depending on the format), as well as records and arrays of serializable values.
= Functions and special values (such as contract labels) aren't serializable.
= If you want serialization to ignore a specific value, please use the `not_exported` metadata.
98 changes: 69 additions & 29 deletions core/src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
},
position::{RawSpan, TermPos},
repl,
serialize::ExportFormat,
serialize::{ExportFormat, NickelPointer},
term::{record::FieldMetadata, Number, RichTerm, Term},
typ::{EnumRow, RecordRow, Type, TypeF, VarKindDiscriminant},
};
Expand Down Expand Up @@ -571,9 +571,18 @@ pub enum ImportError {
),
}

/// An error occurred during serialization.
#[derive(Debug, PartialEq, Clone)]
pub enum ExportError {
pub struct ExportError {
/// The path to the field that contains a non-serializable value. This might be empty if the
/// error occurred before entering any record.
pub path: NickelPointer,
/// The cause of the error.
pub data: ExportErrorData,
}

/// The type of error occurring during serialization.
#[derive(Debug, PartialEq, Clone)]
pub enum ExportErrorData {
/// Encountered a null value for a format that doesn't support them.
UnsupportedNull(ExportFormat, RichTerm),
/// Tried exporting something else than a `String` to raw format.
Expand All @@ -590,6 +599,15 @@ pub enum ExportError {
Other(String),
}

impl From<ExportErrorData> for ExportError {
fn from(data: ExportErrorData) -> ExportError {
ExportError {
path: NickelPointer::new(),
data,
}
}
}

/// A general I/O error, occurring when reading a source file or writing an export.
#[derive(Debug, PartialEq, Eq, Clone)]
pub struct IOError(pub String);
Expand Down Expand Up @@ -2544,22 +2562,28 @@ impl IntoDiagnostics<FileId> for ExportError {
files: &mut Files<String>,
_stdlib_ids: Option<&Vec<FileId>>,
) -> Vec<Diagnostic<FileId>> {
match self {
ExportError::NotAString(rt) => vec![Diagnostic::error()
let mut notes = if !self.path.0.is_empty() {
vec![format!("When exporting field `{}`", self.path)]
} else {
vec![]
};

match self.data {
ExportErrorData::NotAString(rt) => vec![Diagnostic::error()
.with_message(format!(
"raw export expects a String value, but got {}",
rt.as_ref()
.type_of()
.unwrap_or_else(|| String::from("<unevaluated>"))
))
.with_labels(vec![primary_term(&rt, files)])],
ExportError::UnsupportedNull(format, rt) => vec![Diagnostic::error()
.with_labels(vec![primary_term(&rt, files)])
.with_notes(notes)],
ExportErrorData::UnsupportedNull(format, rt) => vec![Diagnostic::error()
.with_message(format!("{format} format doesn't support null values"))
.with_labels(vec![primary_term(&rt, files)])],
ExportError::NonSerializable(rt) => vec![Diagnostic::error()
.with_message("non serializable term")
.with_labels(vec![primary_term(&rt, files)])
.with_notes(vec![
.with_notes(notes)],
ExportErrorData::NonSerializable(rt) => {
notes.extend([
"Nickel only supports serializing to and from strings, booleans, numbers, \
enum tags, `null` (depending on the format), as well as records and arrays \
of serializable values."
Expand All @@ -2570,27 +2594,43 @@ impl IntoDiagnostics<FileId> for ExportError {
"If you want serialization to ignore a specific value, please use the \
`not_exported` metadata."
.into(),
])],
ExportError::NoDocumentation(rt) => vec![Diagnostic::error()
.with_message("no documentation found")
.with_labels(vec![primary_term(&rt, files)])
.with_notes(vec![
"documentation can only be collected from a record.".to_owned()
])],
ExportError::NumberOutOfRange { term, value } => vec![Diagnostic::error()
.with_message(format!(
"The number {} is too large (in absolute value) to be serialized.",
value.to_sci()
))
.with_labels(vec![primary_term(&term, files)])
.with_notes(vec![format!(
]);

vec![Diagnostic::error()
.with_message("non serializable term")
.with_labels(vec![primary_term(&rt, files)])
.with_notes(notes)]
}
ExportErrorData::NoDocumentation(rt) => {
notes.push("documentation can only be collected from a record.".to_owned());

vec![Diagnostic::error()
.with_message("no documentation found")
.with_labels(vec![primary_term(&rt, files)])
.with_notes(notes)]
}
ExportErrorData::NumberOutOfRange { term, value } => {
notes.push(format!(
"Only numbers in the range {:e} to {:e} can be portably serialized",
f64::MIN,
f64::MAX
)])],
ExportError::Other(msg) => vec![Diagnostic::error()
.with_message("serialization failed")
.with_notes(vec![msg])],
));

vec![Diagnostic::error()
.with_message(format!(
"The number {} is too large (in absolute value) to be serialized.",
value.to_sci()
))
.with_labels(vec![primary_term(&term, files)])
.with_notes(notes)]
}
ExportErrorData::Other(msg) => {
notes.push(msg);

vec![Diagnostic::error()
.with_message("serialization failed")
.with_notes(notes)]
}
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions core/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,11 +622,11 @@ impl<EC: EvalCache> Program<EC> {
/// Extract documentation from the program
#[cfg(feature = "doc")]
pub fn extract_doc(&mut self) -> Result<doc::ExtractedDocumentation, Error> {
use crate::error::ExportError;
use crate::error::ExportErrorData;

let term = self.eval_record_spine()?;
doc::ExtractedDocumentation::extract_from_term(&term).ok_or(Error::ExportError(
ExportError::NoDocumentation(term.clone()),
ExportErrorData::NoDocumentation(term.clone()).into(),
))
}

Expand Down Expand Up @@ -664,7 +664,7 @@ impl<EC: EvalCache> Program<EC> {

#[cfg(feature = "doc")]
mod doc {
use crate::error::{Error, ExportError, IOError};
use crate::error::{Error, ExportErrorData, IOError};
use crate::term::{RichTerm, Term};
use comrak::arena_tree::NodeEdge;
use comrak::nodes::{
Expand Down Expand Up @@ -744,7 +744,7 @@ mod doc {

pub fn write_json(&self, out: &mut dyn Write) -> Result<(), Error> {
serde_json::to_writer(out, self)
.map_err(|e| Error::ExportError(ExportError::Other(e.to_string())))
.map_err(|e| Error::ExportError(ExportErrorData::Other(e.to_string()).into()))
}

pub fn write_markdown(&self, out: &mut dyn Write) -> Result<(), Error> {
Expand Down
Loading
Loading