From bc9fd5bf7db4a74b9d0feb3b71a111e1b9e7ee13 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Fri, 3 Mar 2023 17:33:00 +0100 Subject: [PATCH 01/13] Add primops to set label's diagnostic field A previous commit introduced a more fine-grained interface for contract labels, in order to customize error diagnostic. This PR adds the Nickel primops that make this interface accessible to usercode. --- lsp/nls/src/requests/completion.rs | 4 +- src/eval/merge.rs | 17 +-- src/eval/operation.rs | 161 +++++++++++++++++++++++++++-- src/label.rs | 31 ++++-- src/parser/grammar.lalrpop | 8 ++ src/parser/lexer.rs | 8 ++ src/term/mod.rs | 20 ++++ src/typecheck/operation.rs | 24 +++++ 8 files changed, 248 insertions(+), 25 deletions(-) diff --git a/lsp/nls/src/requests/completion.rs b/lsp/nls/src/requests/completion.rs index 4aab7d6d69..5702923258 100644 --- a/lsp/nls/src/requests/completion.rs +++ b/lsp/nls/src/requests/completion.rs @@ -90,7 +90,7 @@ impl IdentWithType { if name.is_ascii() { String::from(name) } else { - format!("\"{}\"", name) + format!("\"{name}\"") } } let doc = || { @@ -810,7 +810,7 @@ mod tests { let actual = get_identifier_path(input); let expected: Option> = expected.map(|path| path.iter().map(|s| String::from(*s)).collect()); - assert_eq!(actual, expected, "test failed: {}", case_name) + assert_eq!(actual, expected, "test failed: {case_name}") } } diff --git a/src/eval/merge.rs b/src/eval/merge.rs index 3a60680b73..aeb535f947 100644 --- a/src/eval/merge.rs +++ b/src/eval/merge.rs @@ -242,22 +242,23 @@ pub fn merge( } = hashmap::split(r1.fields, r2.fields); match mode { - MergeMode::Contract(mut lbl) if !r2.attrs.open && !left.is_empty() => { + MergeMode::Contract(label) if !r2.attrs.open && !left.is_empty() => { let fields: Vec = left.keys().map(|field| format!("`{field}`")).collect(); let plural = if fields.len() == 1 { "" } else { "s" }; let fields_list = fields.join(","); - lbl.set_diagnostic_message(format!("extra field{plural} {fields_list}")); - lbl.set_diagnostic_notes(vec![ - format!("Have you misspelled a field?"), - String::from("The record contract might also be too strict. By default, record contracts exclude any field which is not listed. + let label = label + .with_diagnostic_message(format!("extra field{plural} {fields_list}")) + .with_diagnostic_notes(vec![ + format!("Have you misspelled a field?"), + String::from("The record contract might also be too strict. By default, record contracts exclude any field which is not listed. Append `, ..` at the end of the record contract, as in `{some_field | SomeContract, ..}`, to make it accept extra fields."), - ]); + ]); return Err(EvalError::BlameError { - evaluated_arg: lbl.get_evaluated_arg(cache), - label: lbl, + evaluated_arg: label.get_evaluated_arg(cache), + label, call_stack: CallStack::new(), }); } diff --git a/src/eval/operation.rs b/src/eval/operation.rs index 1752c5c9d9..4dce095e7b 100644 --- a/src/eval/operation.rs +++ b/src/eval/operation.rs @@ -1309,6 +1309,25 @@ impl VirtualMachine { .map(|(next, ..)| next) .ok_or_else(|| EvalError::NotEnoughArgs(2, String::from("trace"), pos_op)) } + UnaryOp::LabelPushDiag() => { + match_sharedterm! {t, with { + Term::Lbl(label) => { + let mut label = label; + label.push_diagnostic(); + Ok(Closure { + body: RichTerm::new(Term::Lbl(label), pos), + env + }) + } + } else { + Err(EvalError::TypeError( + String::from("Label"), + String::from("trace"), + arg_pos, + RichTerm { term: t, pos }, + )) } + } + } } } @@ -1693,13 +1712,12 @@ impl VirtualMachine { BinaryOp::Tag() => match_sharedterm! {t1, with { Term::Str(s) => match_sharedterm!{t2, with { Term::Lbl(label) => { - let mut label = label; - label.set_diagnostic_message(s); - - Ok(Closure::atomic_closure(RichTerm::new( - Term::Lbl(label), - pos_op_inh, - ))) + Ok(Closure::atomic_closure( + RichTerm::new( + Term::Lbl(label.with_diagnostic_message(s)), + pos_op_inh, + ) + )) } } else { Err(EvalError::TypeError( @@ -2629,6 +2647,135 @@ impl VirtualMachine { )) } } + BinaryOp::LabelWithMsg() => { + let t1 = t1.into_owned(); + let t2 = t2.into_owned(); + + let Term::Str(message) = t1 else { + return Err(EvalError::TypeError( + String::from("Str"), + String::from("label_with_msg, 1st argument"), + fst_pos, + RichTerm { + term: t1.into(), + pos: pos1, + }, + )) + }; + + let Term::Lbl(label) = t2 else { + return Err(EvalError::TypeError( + String::from("Lbl"), + String::from("label_with_msg, 2nd argument"), + snd_pos, + RichTerm { + term: t2.into(), + pos: pos2, + }, + )) + }; + + Ok(Closure { + body: RichTerm::new( + Term::Lbl(label.with_diagnostic_message(message)), + pos2.into_inherited(), + ), + env: env2, + }) + } + BinaryOp::LabelWithNotes() => { + let t1 = t1.into_owned(); + let t2 = t2.into_owned(); + + let Term::Array(array, _) = t1 else { + return Err(EvalError::TypeError( + String::from("Array Str"), + String::from("label_with_notes, 1st argument"), + fst_pos, + RichTerm { + term: t1.into(), + pos: pos1, + }, + )); + }; + + let notes = array + .into_iter() + .map(|element| { + let term = element.term.into_owned(); + + if let Term::Str(s) = term { + Ok(s) + } else { + Err(EvalError::TypeError( + String::from("Str"), + String::from("label_with_notes, element of 1st argument"), + TermPos::None, + RichTerm { + term: term.into(), + pos: element.pos, + }, + )) + } + }) + .collect::, _>>()?; + + let Term::Lbl(label) = t2 else { + return Err(EvalError::TypeError( + String::from("Lbl"), + String::from("label_with_notes, 2nd argument"), + snd_pos, + RichTerm { + term: t2.into(), + pos: pos2, + }, + )) + }; + + Ok(Closure { + body: RichTerm::new( + Term::Lbl(label.with_diagnostic_notes(notes)), + pos2.into_inherited(), + ), + env: env2, + }) + } + BinaryOp::LabelAppendNote() => { + let t1 = t1.into_owned(); + let t2 = t2.into_owned(); + + let Term::Str(note) = t1 else { + return Err(EvalError::TypeError( + String::from("Str"), + String::from("label_append_note, 1st argument"), + fst_pos, + RichTerm { + term: t1.into(), + pos: pos1, + }, + )); + }; + + let Term::Lbl(label) = t2 else { + return Err(EvalError::TypeError( + String::from("Lbl"), + String::from("label_append_note, 2nd argument"), + snd_pos, + RichTerm { + term: t2.into(), + pos: pos2, + }, + )) + }; + + Ok(Closure { + body: RichTerm::new( + Term::Lbl(label.append_diagnostic_note(note)), + pos2.into_inherited(), + ), + env: env2, + }) + } } } diff --git a/src/label.rs b/src/label.rs index f3f42ac2fc..0fa12320ed 100644 --- a/src/label.rs +++ b/src/label.rs @@ -431,46 +431,61 @@ impl Label { /// erase the previous value. /// /// If the diagnostic stack is empty, this methods pushes a new diagnostic with the given notes. - pub fn set_diagnostic_message(&mut self, message: impl Into) { + pub fn with_diagnostic_message(mut self, message: impl Into) -> Self { if let Some(current) = self.diagnostics.last_mut() { current.message = Some(message.into()); } else { self.diagnostics .push(ContractDiagnostic::new().with_message(message)); - } + }; + + self } /// Set the notes of the current diagnostic (the last diagnostic of the stack). Potentially /// erase the previous value. /// /// If the diagnostic stack is empty, this methods pushes a new diagnostic with the given notes. - pub fn set_diagnostic_notes(&mut self, notes: Vec) { + pub fn with_diagnostic_notes(mut self, notes: Vec) -> Self { if let Some(current) = self.diagnostics.last_mut() { current.notes = notes; } else { self.diagnostics .push(ContractDiagnostic::new().with_notes(notes)); - } + }; + + self } /// Append a note to the current diagnostic (the last diagnostic of the stack). Potentially /// erase the previous value. /// /// If the diagnostic stack is empty, this methods pushes a new diagnostic with the given note. - pub fn append_diagnostic_note(&mut self, note: impl Into) { + pub fn append_diagnostic_note(mut self, note: impl Into) -> Self { if let Some(current) = self.diagnostics.last_mut() { current.append_note(note); } else { self.diagnostics .push(ContractDiagnostic::new().with_notes(vec![note.into()])); - } + }; + + self } - /// Return a reference to the current contract diagnostic, that is the last one of the stack, - /// if any. + /// Return a reference to the current contract diagnostic, which is the last element of the + /// stack, if any. pub fn current_diagnostic(&self) -> Option<&ContractDiagnostic> { self.diagnostics.last() } + + /// Push a new fresh diagnostic on the stack if the current diagnostic isn't empty. This has + /// the effect of saving the current diagnostic, that can't then be mutated anymore by the + /// label's API. + pub fn push_diagnostic(&mut self) { + if !matches!(self.current_diagnostic(), Some(diag) if !diag.is_empty()) { + self.diagnostics.push(ContractDiagnostic::new()); + } + } } impl Default for Label { diff --git a/src/parser/grammar.lalrpop b/src/parser/grammar.lalrpop index 87c9d20715..0c8740bd23 100644 --- a/src/parser/grammar.lalrpop +++ b/src/parser/grammar.lalrpop @@ -661,6 +661,7 @@ UOp: UnaryOp = { "rec_default_op" => UnaryOp::RecDefault(), "record_empty_with_tail" => UnaryOp::RecordEmptyWithTail(), "trace" => UnaryOp::Trace(), + "label_push_diag" => UnaryOp::LabelPushDiag(), }; MatchCase: MatchCase = { @@ -847,6 +848,9 @@ BOpPre: BinaryOp = { pending_contracts: Default::default(), }, "record_remove" => BinaryOp::DynRemove(), + "label_with_msg" => BinaryOp::LabelWithMsg(), + "label_with_notes" => BinaryOp::LabelWithNotes(), + "label_append_note" => BinaryOp::LabelAppendNote(), } NOpPre: UniTerm = { @@ -1045,6 +1049,10 @@ extern { "str_from" => Token::Normal(NormalToken::ToStr), "num_from" => Token::Normal(NormalToken::NumFromStr), "enum_from" => Token::Normal(NormalToken::EnumFromStr), + "label_with_msg" => Token::Normal(NormalToken::LabelWithMsg), + "label_with_notes" => Token::Normal(NormalToken::LabelWithNotes), + "label_append_note" => Token::Normal(NormalToken::LabelAppendNote), + "label_push_diag" => Token::Normal(NormalToken::LabelPushDiag), "{" => Token::Normal(NormalToken::LBrace), "}" => Token::Normal(NormalToken::RBrace), diff --git a/src/parser/lexer.rs b/src/parser/lexer.rs index f4f64e8b0a..8fe041d583 100644 --- a/src/parser/lexer.rs +++ b/src/parser/lexer.rs @@ -310,6 +310,14 @@ pub enum NormalToken<'input> { NumFromStr, #[token("%enum_from_str%")] EnumFromStr, + #[token("%label_with_msg")] + LabelWithMsg, + #[token("%label_with_notes")] + LabelWithNotes, + #[token("%label_append_note")] + LabelAppendNote, + #[token("%label_push_diag")] + LabelPushDiag, #[token("{")] LBrace, diff --git a/src/term/mod.rs b/src/term/mod.rs index b349883fce..60e103180e 100644 --- a/src/term/mod.rs +++ b/src/term/mod.rs @@ -937,6 +937,12 @@ impl From for Term { } } +impl From for SharedTerm { + fn from(t: Term) -> Self { + SharedTerm::new(t) + } +} + impl Deref for SharedTerm { type Target = Term; @@ -1162,6 +1168,13 @@ pub enum UnaryOp { /// on the top of the stack. Operationally the same as the identity /// function Trace(), + + /// Push a new, fresh diagnostic on the diagnostic stack of a contract label. This has the + /// effect of saving the current diagnostic, as following calls to primop that modifies the + /// label's current diagnostic will modify the fresh one, istead of the one being stacked. + /// This primop shouldn't be used directly by user a priori, but is used internally during e.g. + /// contract application. + LabelPushDiag(), } // See: https://github.com/rust-lang/regex/issues/178 @@ -1319,6 +1332,13 @@ pub enum BinaryOp { /// contract `C` attached to each of them. Then uses `NAryOp::MergeContract` to /// apply this record contract to `R`. DictionaryAssume(), + + /// Set the message of the current diagnostic of a label. + LabelWithMsg(), + /// Set the notes of the current diagnostic of a label. + LabelWithNotes(), + /// Append a note to the current diagnostic of a label. + LabelAppendNote(), } impl BinaryOp { diff --git a/src/typecheck/operation.rs b/src/typecheck/operation.rs index 5a857b0170..2038f0ece1 100644 --- a/src/typecheck/operation.rs +++ b/src/typecheck/operation.rs @@ -220,6 +220,9 @@ pub fn get_uop_type( let ty = UnifType::UnifVar(state.table.fresh_type_var_id()); (mk_uniftype::str(), mk_uty_arrow!(ty.clone(), ty)) } + // Morally: Lbl -> Lbl + // Actual: Dyn -> Dyn + UnaryOp::LabelPushDiag() => (mk_uniftype::dynamic(), mk_uniftype::dynamic()), }) } @@ -405,6 +408,27 @@ pub fn get_bop_type( mk_uty_arrow!(mk_uniftype::dynamic(), ty_dict), ) } + // Morally: Str -> Lbl -> Lbl + // Actual: Str -> Dyn -> Dyn + BinaryOp::LabelWithMsg() => ( + mk_uniftype::str(), + mk_uniftype::dynamic(), + mk_uniftype::dynamic(), + ), + // Morally: Array Str -> Lbl -> Lbl + // Actual: Array Str -> Dyn -> Dyn + BinaryOp::LabelWithNotes() => ( + mk_uniftype::array(TypeF::Str), + mk_uniftype::dynamic(), + mk_uniftype::dynamic(), + ), + // Morally: Str -> Lbl -> Lbl + // Actual: Str -> Dyn -> Dyn + BinaryOp::LabelAppendNote() => ( + mk_uniftype::str(), + mk_uniftype::dynamic(), + mk_uniftype::dynamic(), + ), }) } From a6be63cb47da935c64e87361971ceda4cc51d326 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Mar 2023 18:39:12 +0100 Subject: [PATCH 02/13] Get rid of %tag%, update contract stdlib Updates the contract stdlib to expose the new label primop introduced in a previous commit. Renames existing function accordingly. Get rid of the `%tag%` primop, now replaced by `%with_message%`. --- benches/mantis/lib.ncl | 6 +- doc/manual/contracts.md | 14 +-- examples/config-gcc/config-gcc.ncl | 16 +-- src/eval/operation.rs | 82 +++++---------- src/parser/grammar.lalrpop | 2 - src/parser/lexer.rs | 10 +- src/term/mod.rs | 2 - src/typecheck/operation.rs | 6 -- stdlib/array.ncl | 10 +- stdlib/builtin.ncl | 2 +- stdlib/contract.ncl | 163 ++++++++++++++++++++++------- stdlib/internals.ncl | 16 +-- stdlib/num.ncl | 16 +-- stdlib/string.ncl | 16 +-- 14 files changed, 206 insertions(+), 155 deletions(-) diff --git a/benches/mantis/lib.ncl b/benches/mantis/lib.ncl index 8fb7e51683..f382470209 100644 --- a/benches/mantis/lib.ncl +++ b/benches/mantis/lib.ncl @@ -19,15 +19,15 @@ if str_match value then value else - contract.blame_with "no match" label - else contract.blame_with "not a string" label, + contract.blame_with_message "no match" label + else contract.blame_with_message "not a string" label, PseudoOr = fun alts label value => array.fold (fun ctr rest => if ctr.pred value then ctr.contract value else rest) - (contract.blame_with "no alternative matched" label), + (contract.blame_with_message "no alternative matched" label), OrableFromPred : (Dyn -> Bool) -> {pred : Dyn -> Bool, contract: Dyn -> Dyn -> Dyn }, OrableFromPred = fun pred_ => { diff --git a/doc/manual/contracts.md b/doc/manual/contracts.md index 162e72ffda..d7920be07f 100644 --- a/doc/manual/contracts.md +++ b/doc/manual/contracts.md @@ -61,21 +61,21 @@ let IsFoo = fun label value => if value == "foo" then value else - contract.blame_with "not equal to \"foo\"" label + contract.blame_with_message "not equal to \"foo\"" label else - contract.blame_with "not a string" label + contract.blame_with_message "not a string" label ``` A custom contract is a function of two arguments: - A `label`. Provided by the interpreter, the label contains tracking information for error reporting. Its main usage is to be passed to `contract.blame` or - `contract.blame_with` when the contract isn't satisfied. + `contract.blame_with_message` when the contract isn't satisfied. - The value being checked. Upon success, the contract must return the original value. We will see the reason why in the [laziness](#laziness) section. To signal failure, we use -`contract.blame` or its variant `contract.blame_with` that takes an additional +`contract.blame` or its variant `contract.blame_with_message` that takes an additional error message as a parameter. `blame` immediately aborts the execution and reports a contract violation error. @@ -586,17 +586,17 @@ let NumBoolDict = fun label value => if string.is_match "^\\d+$" field_name then acc # unused and always null through iteration else - contract.blame_with "field name `#{field_name}` is not a number" label + contract.blame_with_message "field name `#{field_name}` is not a number" label ) null in value |> record.map (fun name value => let label_with_msg = - contract.tag "field `#{name}` is not a boolean" label in + contract.label.with_message "field `#{name}` is not a boolean" label in contract.apply Bool label_with_msg value) |> builtin.seq check_fields else - contract.blame_with "not a record" label + contract.blame_with_message "not a record" label ``` There is a lot to unwrap here. Please refer to the [syntax](./syntax.md) section diff --git a/examples/config-gcc/config-gcc.ncl b/examples/config-gcc/config-gcc.ncl index 487890b357..402cf023c6 100644 --- a/examples/config-gcc/config-gcc.ncl +++ b/examples/config-gcc/config-gcc.ncl @@ -11,20 +11,20 @@ let GccFlag = array.any (fun x => x == string.substring 0 1 value) available then value else - contract.blame_with "unknown flag %{value}" label + contract.blame_with_message "unknown flag %{value}" label else if builtin.is_record value then if record.has_field "flag" value && record.has_field "arg" value then if array.any (fun x => x == value.flag) available then #Normalize the tag to a string value.flag ++ value.arg else - contract.blame_with "unknown flag %{value.flag}" label + contract.blame_with_message "unknown flag %{value.flag}" label else - contract.blame_with + contract.blame_with_message "bad record structure: missing field `flag` or `arg`" label else - contract.blame_with "expected record or string" label in + contract.blame_with_message "expected record or string" label in let Path = let pattern = m%"^(.+)/([^/]+)$"% in @@ -33,18 +33,18 @@ let Path = if string.is_match pattern value then value else - contract.blame_with "invalid path" label + contract.blame_with_message "invalid path" label else - contract.blame_with "not a string" label in + contract.blame_with_message "not a string" label in let SharedObjectFile = fun label value => if builtin.is_str value then if string.is_match m%"\.so$"% value then value else - contract.blame_with "not an .so file" label + contract.blame_with_message "not an .so file" label else - contract.blame_with "not a string" label in + contract.blame_with_message "not a string" label in let OptLevel = fun label value => if value == 0 || value == 1 || value == 2 then diff --git a/src/eval/operation.rs b/src/eval/operation.rs index 4dce095e7b..da2edd392a 100644 --- a/src/eval/operation.rs +++ b/src/eval/operation.rs @@ -1709,40 +1709,6 @@ impl VirtualMachine { )) } } - BinaryOp::Tag() => match_sharedterm! {t1, with { - Term::Str(s) => match_sharedterm!{t2, with { - Term::Lbl(label) => { - Ok(Closure::atomic_closure( - RichTerm::new( - Term::Lbl(label.with_diagnostic_message(s)), - pos_op_inh, - ) - )) - } - } else { - Err(EvalError::TypeError( - String::from("Label"), - String::from("tag, 2nd argument"), - snd_pos, - RichTerm { - term: t2, - pos: pos2, - }, - )) - } - } - } else { - Err(EvalError::TypeError( - String::from("Str"), - String::from("tag, 1st argument"), - fst_pos, - RichTerm { - term: t1, - pos: pos1, - }, - )) - } - }, BinaryOp::Eq() => { let mut env = Environment::new(); @@ -2675,18 +2641,28 @@ impl VirtualMachine { )) }; - Ok(Closure { - body: RichTerm::new( - Term::Lbl(label.with_diagnostic_message(message)), - pos2.into_inherited(), - ), - env: env2, - }) + Ok(Closure::atomic_closure(RichTerm::new( + Term::Lbl(label.with_diagnostic_message(message)), + pos_op_inh, + ))) } BinaryOp::LabelWithNotes() => { - let t1 = t1.into_owned(); let t2 = t2.into_owned(); + // We need to extract plain strings from a Nickel array, which most likely + // contains at least generated variables. + // As for serialization, we thus fully substitute all variables first. + let t1_subst = subst( + &self.cache, + RichTerm { + term: t1, + pos: pos1, + }, + &Environment::new(), + &env1, + ); + let t1 = t1_subst.term.into_owned(); + let Term::Array(array, _) = t1 else { return Err(EvalError::TypeError( String::from("Array Str"), @@ -2732,13 +2708,10 @@ impl VirtualMachine { )) }; - Ok(Closure { - body: RichTerm::new( - Term::Lbl(label.with_diagnostic_notes(notes)), - pos2.into_inherited(), - ), - env: env2, - }) + Ok(Closure::atomic_closure(RichTerm::new( + Term::Lbl(label.with_diagnostic_notes(notes)), + pos_op_inh, + ))) } BinaryOp::LabelAppendNote() => { let t1 = t1.into_owned(); @@ -2768,13 +2741,10 @@ impl VirtualMachine { )) }; - Ok(Closure { - body: RichTerm::new( - Term::Lbl(label.append_diagnostic_note(note)), - pos2.into_inherited(), - ), - env: env2, - }) + Ok(Closure::atomic_closure(RichTerm::new( + Term::Lbl(label.append_diagnostic_note(note)), + pos2.into_inherited(), + ))) } } } diff --git a/src/parser/grammar.lalrpop b/src/parser/grammar.lalrpop index 0c8740bd23..0ad969994d 100644 --- a/src/parser/grammar.lalrpop +++ b/src/parser/grammar.lalrpop @@ -835,7 +835,6 @@ BOpPre: BinaryOp = { "go_field" => BinaryOp::GoField(), "has_field" => BinaryOp::HasField(), "elem_at" => BinaryOp::ArrayElemAt(), - "tag" => BinaryOp::Tag(), "hash" => BinaryOp::Hash(), "serialize" => BinaryOp::Serialize(), "deserialize" => BinaryOp::Deserialize(), @@ -981,7 +980,6 @@ extern { "Bool" => Token::Normal(NormalToken::Bool), "Array" => Token::Normal(NormalToken::Array), - "tag" => Token::Normal(NormalToken::Tag), "typeof" => Token::Normal(NormalToken::Typeof), "assume" => Token::Normal(NormalToken::Assume), "array_lazy_assume" => Token::Normal(NormalToken::ArrayLazyAssume), diff --git a/src/parser/lexer.rs b/src/parser/lexer.rs index 8fe041d583..47a7494a01 100644 --- a/src/parser/lexer.rs +++ b/src/parser/lexer.rs @@ -175,8 +175,6 @@ pub enum NormalToken<'input> { #[regex("[a-zA-Z][_a-zA-Z0-9-']*-s(%+)\"", symbolic_string_prefix_and_length)] SymbolicStringStart(SymbolicStringStart<'input>), - #[token("%tag%")] - Tag, #[token("%typeof%")] Typeof, @@ -310,13 +308,13 @@ pub enum NormalToken<'input> { NumFromStr, #[token("%enum_from_str%")] EnumFromStr, - #[token("%label_with_msg")] + #[token("%label_with_message%")] LabelWithMsg, - #[token("%label_with_notes")] + #[token("%label_with_notes%")] LabelWithNotes, - #[token("%label_append_note")] + #[token("%label_append_note%")] LabelAppendNote, - #[token("%label_push_diag")] + #[token("%label_push_diag%")] LabelPushDiag, #[token("{")] diff --git a/src/term/mod.rs b/src/term/mod.rs index 60e103180e..e15f613226 100644 --- a/src/term/mod.rs +++ b/src/term/mod.rs @@ -1273,8 +1273,6 @@ pub enum BinaryOp { /// /// See `GoDom`. GoField(), - /// Set the tag text of a blame label. - Tag(), /// Extend a record with a dynamic field. /// /// Dynamic means that the field name may be an expression instead of a statically known diff --git a/src/typecheck/operation.rs b/src/typecheck/operation.rs index 2038f0ece1..025acc93c5 100644 --- a/src/typecheck/operation.rs +++ b/src/typecheck/operation.rs @@ -259,12 +259,6 @@ pub fn get_bop_type( mk_uniftype::dynamic(), mk_uty_arrow!(TypeF::Dyn, TypeF::Dyn), ), - // Str -> Dyn -> Dyn - BinaryOp::Tag() => ( - mk_uniftype::str(), - mk_uniftype::dynamic(), - mk_uniftype::dynamic(), - ), // forall a b. a -> b -> Bool BinaryOp::Eq() => ( UnifType::UnifVar(state.table.fresh_type_var_id()), diff --git a/stdlib/array.ncl b/stdlib/array.ncl index 30652e4e88..137747a3ea 100644 --- a/stdlib/array.ncl +++ b/stdlib/array.ncl @@ -17,9 +17,9 @@ if %length% value != 0 then value else - %blame% (%tag% "empty array" label) + %blame% (%label_with_message% "empty array" label) else - %blame% (%tag% "not a array" label), + %blame% (%label_with_message% "not a array" label), head : forall a. Array a -> a | doc m%" @@ -112,9 +112,9 @@ let length = %length% l in if length == 0 then acc else - let rec go = fun acc n => + let rec go = fun acc n => if n == length then acc - else + else let next_acc = %elem_at% l n |> f acc in %seq% next_acc (go next_acc (n+1)) in @@ -136,7 +136,7 @@ let length = %length% l in let rec go = fun n => if n == length then - fst + fst else go (n+1) |> f (%elem_at% l n) diff --git a/stdlib/builtin.ncl b/stdlib/builtin.ncl index 9be3764e47..865922d17e 100644 --- a/stdlib/builtin.ncl +++ b/stdlib/builtin.ncl @@ -244,7 +244,7 @@ │ ^ applied to this expression ``` "% - = fun msg label value => contract.blame_with msg label, + = fun msg label value => contract.blame_with_message msg label, fail_with | Str -> Dyn | doc m%" diff --git a/stdlib/contract.ncl b/stdlib/contract.ncl index 3b29ce9659..74673e2c97 100644 --- a/stdlib/contract.ncl +++ b/stdlib/contract.ncl @@ -5,86 +5,178 @@ Raise blame for a given label. Type: `forall a. Lbl -> a` - (for technical reasons, this element isn't actually statically typed) + (for technical reasons, this function isn't actually statically typed) Blame is the mechanism to signal contract violiation in Nickel. It ends the program execution and print a detailed report thanks to the information tracked inside the label. For example: + ```nickel IsZero = fun label value => - if value == 0 then value - else contract.blame label + if value == 0 then + value + else + contract.blame label ``` "% - = fun l => %blame% l, + = fun label => %blame% label, - blame_with + blame_with_message | doc m%" Raise blame with respect to a given label and a custom error message. Type: `forall a. Str -> Lbl -> a` - (for technical reasons, this element isn't actually statically typed) + (for technical reasons, this function isn't actually statically typed) Same as `blame`, but take an additional custom error message that will be - displayed as part of the blame error. `blame_with msg l` is equivalent to - `blame (tag msg l) + displayed as part of the blame error. `blame_with_message message label` + is equivalent to `blame (label.with_message message label)` For example: + ```nickel let IsZero = fun label value => - if value == 0 then value - else contract.blame_with "Not zero" label in + if value == 0 then + value + else + contract.blame_with_message_message "Not zero" label + in + 0 | IsZero ``` "% - = fun msg l => %blame% (%tag% msg l), + = fun message label => %blame% (%label_with_message% message label), from_predicate | doc m%" Generate a contract from a boolean predicate. Type: `(Dyn -> Bool) -> (Lbl -> Dyn -> Dyn)` - (for technical reasons, this element isn't actually statically typed) + (for technical reasons, this function isn't actually statically typed) For example: - ``` + + ```nickel let IsZero = contract.from_predicate (fun x => x == 0) in 0 | IsZero ``` "% - = fun pred l v => if pred v then v else %blame% l, + = fun pred label value => if pred value then value else %blame% label, - tag + label | doc m%" - Attach a tag, or a custom error message, to a label. If a tag was - previously set, it is erased. + The label submodule, which gathers functions that manipulate contract + labels. - Type: `Str -> Lbl -> Lbl` - (for technical reasons, this element isn't actually statically typed) - - For example: - ``` - let ContractNum = contract.from_predicate (fun x => x > 0 && x < 50) in - Contract = fun label value => - if builtin.is_num value then - ContractNum - (contract.tag "num subcontract failed! (out of bound)" label) - value - else - value in - 5 | Contract - ``` + A label is a special opaque value automatically passed by the Nickel + interpreter to contracts at contract application. A label offers a way + for custom contracts to customize the error message that will be shown + if the contract is broken. "% - = fun msg l => %tag% msg l, + = { + with_message + | doc m%" + Attach a custom error message to a label. + + Type: `Str -> Lbl -> Lbl` + (for technical reasons, this function isn't actually statically typed) + + If a custom error message was previously set, there are two + possibilities: + - the label has gone through a `contract.apply` call in-between. + In this case, the previous custom reporting data are stacked, + and using `with_message` won't erase anything, but just add + a new diagnostic on the stack. + - no `contract.apply` has taken place since the last message was + set. In this case, the current diagnostic is still the same, and + the previous error message will be erased. + + For example: + + ```nickel + let ContractNum = contract.from_predicate (fun x => x > 0 && x < 50) in + + let Contract = fun label value => + if builtin.is_num value then + contract.apply + ContractNum + (contract.label.with_message + "num subcontract failed! (out of bound)" + label + ) + value + else + value + in + + 5 | Contract + ``` + "% + = fun message label => %label_with_message% message label, + + with_notes + | doc m%" + Attach custom error notes to a label. + + Type: `Array Str -> Lbl -> Lbl` + (for technical reasons, this function isn't actually statically typed) + + If custom error notes were previously set, there are two + possibilities: + - the label has gone through a `contract.apply` call in-between. + In this case, the previous reporting data are stacked, + and using `with_notes` won't erase anything, but just add + a new diagnostic on the stack. + - no `contract.apply` has taken place since the last notes were + set. In this case, the current diagnostic is still the same, and + the previous error notes will be erased. + + For example: + + ```nickel + let ContractNum = contract.from_predicate (fun x => x > 0 && x < 50) in + + let Contract = fun label value => + if builtin.is_num value then + contract.apply + ContractNum + (label + |> contract.label.with_message "num subcontract failed! (out of bound)" + |> contract.label.with_notes [ + "The value was a number, but this number is out of the expected bound", + "The value must be a number between 0 and 50, both excluded", + ] + ) + value + else + value + in + + 5 | Contract + ``` + "% + # the %label_with_notes% operator expect an array of string that is + # fully evaluated, thus we force the notes first + = fun notes label => %label_with_notes% (%force% notes) label, + + append_note + | doc m%" + Append a note to the label's error notes. + + Type: `Str -> Lbl -> Lbl` + (for technical reasons, this function isn't actually statically typed) + "% + = fun note label => %label_append_note% note label, + }, apply | doc m%" Apply a contract to a label and a value. Type: `Contract -> Lbl -> Dyn -> Dyn` - (for technical reasons, this element isn't actually statically typed) + (for technical reasons, this function isn't actually statically typed) Nickel supports user-defined contracts defined as functions, but also as records. Moreover, the interpreter performs additional book-keeping for @@ -93,7 +185,8 @@ contract, but this function instead. For example: - ``` + + ```nickel let Nullable = fun param_contract label value => if value == null then null else contract.apply param_contract label value diff --git a/stdlib/internals.ncl b/stdlib/internals.ncl index 3e4f293c1a..241980c4e8 100644 --- a/stdlib/internals.ncl +++ b/stdlib/internals.ncl @@ -40,10 +40,10 @@ if %typeof% t == `Enum then %assume% case l t else - %blame% (%tag% "not an enum tag" l), + %blame% (%label_with_message% "not an enum tag" l), "$enum_fail" = fun l => - %blame% (%tag% "tag not included in the enum type" l), + %blame% (%label_with_message% "tag not included in the enum type" l), "$record" = fun field_contracts tail_contract l t => if %typeof% t == `Record then @@ -79,24 +79,24 @@ in tail_contract fields_with_contracts l tail_fields else - %blame% (%tag% "missing field %{%head% missing_fields}" l) + %blame% (%label_with_message% "missing field %{%head% missing_fields}" l) else - %blame% (%tag% "not a record" l), + %blame% (%label_with_message% "not a record" l), "$dyn_record" = fun contr l t => if %typeof% t == `Record then %dictionary_assume% (%go_dict% l) t contr else - %blame% (%tag% "not a record" l), + %blame% (%label_with_message% "not a record" l), "$forall_tail" = fun sy pol acc l t => if pol == (%polarity% l) then if t == {} then - let tagged_l = %tag% "polymorphic tail mismatch" l in + let tagged_l = %label_with_message% "polymorphic tail mismatch" l in let tail = %record_unseal_tail% sy tagged_l t in acc & tail else - %blame% (%tag% "extra field `%{%head% (%fields% t)}`" l) + %blame% (%label_with_message% "extra field `%{%head% (%fields% t)}`" l) else # Note: in order to correctly attribute blame, the polarity of `l` # must match the polarity of the `forall` which introduced the @@ -109,7 +109,7 @@ "$empty_tail" = fun acc l t => if t == {} then acc - else %blame% (%tag% "extra field `%{%head% (%fields% t)}`" l), + else %blame% (%label_with_message% "extra field `%{%head% (%fields% t)}`" l), # Recursive priorities operators diff --git a/stdlib/num.ncl b/stdlib/num.ncl index 667b554f54..4af6eb6400 100644 --- a/stdlib/num.ncl +++ b/stdlib/num.ncl @@ -17,9 +17,9 @@ if value % 1 == 0 then value else - %blame% (%tag% "not an integer" label) + %blame% (%label_with_message% "not an integer" label) else - %blame% (%tag% "not a number" label), + %blame% (%label_with_message% "not a number" label), Nat | doc m%" @@ -40,9 +40,9 @@ if value % 1 == 0 && value >= 0 then value else - %blame% (%tag% "not a natural" label) + %blame% (%label_with_message% "not a natural" label) else - %blame% (%tag% "not a number" label), + %blame% (%label_with_message% "not a number" label), PosNat | doc m%" @@ -63,9 +63,9 @@ if value % 1 == 0 && value > 0 then value else - %blame% (%tag% "not positive integer" label) + %blame% (%label_with_message% "not positive integer" label) else - %blame% (%tag% "not a number" label), + %blame% (%label_with_message% "not a number" label), NonZero | doc m%" @@ -84,9 +84,9 @@ if value != 0 then value else - %blame% (%tag% "non-zero" label) + %blame% (%label_with_message% "non-zero" label) else - %blame% (%tag% "not a number" label), + %blame% (%label_with_message% "not a number" label), is_int : Num -> Bool | doc m%" diff --git a/stdlib/string.ncl b/stdlib/string.ncl index 9261767350..1a02171406 100644 --- a/stdlib/string.ncl +++ b/stdlib/string.ncl @@ -22,9 +22,9 @@ else if s == "false" || s == "False" then "false" else - %blame% (%tag% "expected \"true\" or \"false\", got %{s}" l) + %blame% (%label_with_message% "expected \"true\" or \"false\", got %{s}" l) else - %blame% (%tag% "not a string" l), + %blame% (%label_with_message% "not a string" l), NumLiteral | doc m%" @@ -47,9 +47,9 @@ if is_num_literal s then s else - %blame% (%tag% "invalid num literal" l) + %blame% (%label_with_message% "invalid num literal" l) else - %blame% (%tag% "not a string" l), + %blame% (%label_with_message% "not a string" l), CharLiteral | doc m%" @@ -72,9 +72,9 @@ if length s == 1 then s else - %blame% (%tag% "length different than one" l) + %blame% (%label_with_message% "length different than one" l) else - %blame% (%tag% "not a string" l), + %blame% (%label_with_message% "not a string" l), EnumTag | doc m%" @@ -142,9 +142,9 @@ if %str_length% s > 0 then s else - %blame% (%tag% "empty string" l) + %blame% (%label_with_message% "empty string" l) else - %blame% (%tag% "not a string" l), + %blame% (%label_with_message% "not a string" l), join : Str -> Array Str -> Str | doc m%" From ad43c92b95d124508df4117b9e636ea5e7baeac3 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Mar 2023 19:15:49 +0100 Subject: [PATCH 03/13] Make contract.apply stack the current diagnostic --- src/label.rs | 2 +- stdlib/contract.ncl | 75 +++++++++++++++++++++++++++++++++------------ 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/src/label.rs b/src/label.rs index 0fa12320ed..5c4b2cf4e1 100644 --- a/src/label.rs +++ b/src/label.rs @@ -482,7 +482,7 @@ impl Label { /// the effect of saving the current diagnostic, that can't then be mutated anymore by the /// label's API. pub fn push_diagnostic(&mut self) { - if !matches!(self.current_diagnostic(), Some(diag) if !diag.is_empty()) { + if matches!(self.current_diagnostic(), Some(diag) if !diag.is_empty()) { self.diagnostics.push(ContractDiagnostic::new()); } } diff --git a/stdlib/contract.ncl b/stdlib/contract.ncl index 74673e2c97..06757aa70e 100644 --- a/stdlib/contract.ncl +++ b/stdlib/contract.ncl @@ -173,28 +173,63 @@ apply | doc m%" - Apply a contract to a label and a value. - - Type: `Contract -> Lbl -> Dyn -> Dyn` - (for technical reasons, this function isn't actually statically typed) - - Nickel supports user-defined contracts defined as functions, but also as - records. Moreover, the interpreter performs additional book-keeping for - error reporting when applying a contract in an expression `value | - Contract`. You should not use standard function application to apply a - contract, but this function instead. + Apply a contract to a label and a value. + + Type: `Contract -> Lbl -> Dyn -> Dyn` + (for technical reasons, this function isn't actually statically typed) + + Nickel supports user-defined contracts defined as functions, but also + as records. Moreover, the interpreter performs additional book-keeping + for error reporting when applying a contract in an expression + `value | Contract`. You should not use standard function application + to apply a contract, but this function instead. + + # Example + + ```nickel + let Nullable = fun param_contract label value => + if value == null then null + else contract.apply param_contract label value + in + let Contract = Nullable {foo | Num} in + ({foo = 1} | Contract) + ``` + + # Diagnostic stack + + Using `apply` will stack the current custom reporting data, and create a + fresh current working diagnostic. `apply` thus acts automatically as a + split point between a contract and its subcontracts, providing the + subcontracts a fresh label to add their diagnostic to, while remembering + the previous reporting data set by the parent contract. + + ## Illustration + + ```nickel + let ChildContract = fun label value => + label + |> contract.label.with_message "child's message" + |> contract.label.append_note "child's note" + |> contract.blame + in + + let ParentContract = fun label value => + let label = + label + |> contract.label.with_message "parent's message" + |> contract.label.append_note "parent's note" + in + contract.apply ChildContract label value + in - For example: + null | ParentContract + ``` - ```nickel - let Nullable = fun param_contract label value => - if value == null then null - else contract.apply param_contract label value - in - let Contract = Nullable {foo | Num} in - ({foo = 1} | Contract) - ``` + This example will print two diagnostics: the main one using, the + message and note of the child contract, and a secondary diagnostic, + with parent's contract message and note. "% - = fun contract label value => %assume% contract label value, + = fun contract label value => + %assume% contract (%label_push_diag% label) value, }, } From 1411079831512815b7c3bccc518750a7d3d7e21a Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Mar 2023 19:24:19 +0100 Subject: [PATCH 04/13] Add snapshot tests for custom contract diagnostics --- tests/snapshot/README.md | 2 +- .../contract_with_custom_diagnostic.ncl | 11 ++++++++ .../subcontract_nested_custom_diagnostics.ncl | 17 ++++++++++++ ...r_contract_with_custom_diagnostic.ncl.snap | 25 +++++++++++++++++ ...ontract_nested_custom_diagnostics.ncl.snap | 27 +++++++++++++++++++ 5 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 tests/snapshot/inputs/errors/contract_with_custom_diagnostic.ncl create mode 100644 tests/snapshot/inputs/errors/subcontract_nested_custom_diagnostics.ncl create mode 100644 tests/snapshot/snapshots/snapshot__error_contract_with_custom_diagnostic.ncl.snap create mode 100644 tests/snapshot/snapshots/snapshot__error_subcontract_nested_custom_diagnostics.ncl.snap diff --git a/tests/snapshot/README.md b/tests/snapshot/README.md index b8e53d0310..33a042e19e 100644 --- a/tests/snapshot/README.md +++ b/tests/snapshot/README.md @@ -22,7 +22,7 @@ please read [this section](#without-cargo-insta). 1. Run `cargo insta review`. 2. Check the output to see what changed - + ## How to add a new snapshot test 1. Add the Nickel file whose output you want to test into one of the diff --git a/tests/snapshot/inputs/errors/contract_with_custom_diagnostic.ncl b/tests/snapshot/inputs/errors/contract_with_custom_diagnostic.ncl new file mode 100644 index 0000000000..704754dabe --- /dev/null +++ b/tests/snapshot/inputs/errors/contract_with_custom_diagnostic.ncl @@ -0,0 +1,11 @@ +let Contract = fun label _value => + label + |> contract.label.with_message "main error message" + |> contract.label.with_notes [ + "This is the first note", + "This is the second note" + ] + |> contract.blame +in + +null | Contract diff --git a/tests/snapshot/inputs/errors/subcontract_nested_custom_diagnostics.ncl b/tests/snapshot/inputs/errors/subcontract_nested_custom_diagnostics.ncl new file mode 100644 index 0000000000..d154a5da66 --- /dev/null +++ b/tests/snapshot/inputs/errors/subcontract_nested_custom_diagnostics.ncl @@ -0,0 +1,17 @@ +let ChildContract = fun label value => + label + |> contract.label.with_message "child's message" + |> contract.label.append_note "child's note" + |> contract.blame +in + +let ParentContract = fun label value => + let label = + label + |> contract.label.with_message "parent's message" + |> contract.label.append_note "parent's note" + in + contract.apply ChildContract label value +in + +null | ParentContract diff --git a/tests/snapshot/snapshots/snapshot__error_contract_with_custom_diagnostic.ncl.snap b/tests/snapshot/snapshots/snapshot__error_contract_with_custom_diagnostic.ncl.snap new file mode 100644 index 0000000000..48b360a796 --- /dev/null +++ b/tests/snapshot/snapshots/snapshot__error_contract_with_custom_diagnostic.ncl.snap @@ -0,0 +1,25 @@ +--- +source: tests/snapshot/main.rs +expression: snapshot +--- +error: contract broken by a value: main error message + ┌─ :1:1 + │ + 1 │ Contract + │ -------- expected type + │ + ┌─ [INPUTS_PATH]/errors/contract_with_custom_diagnostic.ncl:11:1 + │ +11 │ null | Contract + │ ^^^^ applied to this expression + │ + = This is the first note + = This is the second note + +note: + ┌─ [INPUTS_PATH]/errors/contract_with_custom_diagnostic.ncl:11:8 + │ +11 │ null | Contract + │ ^^^^^^^^ bound here + + diff --git a/tests/snapshot/snapshots/snapshot__error_subcontract_nested_custom_diagnostics.ncl.snap b/tests/snapshot/snapshots/snapshot__error_subcontract_nested_custom_diagnostics.ncl.snap new file mode 100644 index 0000000000..e15e98e0d8 --- /dev/null +++ b/tests/snapshot/snapshots/snapshot__error_subcontract_nested_custom_diagnostics.ncl.snap @@ -0,0 +1,27 @@ +--- +source: tests/snapshot/main.rs +expression: snapshot +--- +error: contract broken by a value: child\'s message + ┌─ :1:1 + │ +1 │ ParentContract + │ -------------- expected type + │ + ┌─ (generated by evaluation):1:1 + │ +1 │ value + │ ----- evaluated to this value + │ + = child's note + +note: + ┌─ [INPUTS_PATH]/errors/subcontract_nested_custom_diagnostics.ncl:17:8 + │ +17 │ null | ParentContract + │ ^^^^^^^^^^^^^^ bound here + +note: from a parent contract violation: parent\'s message + = parent's note + + From df7a82d777cb5af72f2cbc321db42bf711c40d63 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Mar 2023 19:30:34 +0100 Subject: [PATCH 05/13] Replace format! by String::new --- src/eval/merge.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/eval/merge.rs b/src/eval/merge.rs index aeb535f947..bcfb13971e 100644 --- a/src/eval/merge.rs +++ b/src/eval/merge.rs @@ -251,7 +251,7 @@ pub fn merge( let label = label .with_diagnostic_message(format!("extra field{plural} {fields_list}")) .with_diagnostic_notes(vec![ - format!("Have you misspelled a field?"), + String::from("Have you misspelled a field?"), String::from("The record contract might also be too strict. By default, record contracts exclude any field which is not listed. Append `, ..` at the end of the record contract, as in `{some_field | SomeContract, ..}`, to make it accept extra fields."), ]); From f75126ce87ccf1cde0db33642783602ba298529d Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Mar 2023 19:35:50 +0100 Subject: [PATCH 06/13] Prefer full long name over occasional shortenings --- src/eval/operation.rs | 6 +++--- src/parser/grammar.lalrpop | 4 ++-- src/parser/lexer.rs | 2 +- src/term/mod.rs | 2 +- src/typecheck/operation.rs | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/eval/operation.rs b/src/eval/operation.rs index da2edd392a..625dc2da93 100644 --- a/src/eval/operation.rs +++ b/src/eval/operation.rs @@ -2613,14 +2613,14 @@ impl VirtualMachine { )) } } - BinaryOp::LabelWithMsg() => { + BinaryOp::LabelWithMessage() => { let t1 = t1.into_owned(); let t2 = t2.into_owned(); let Term::Str(message) = t1 else { return Err(EvalError::TypeError( String::from("Str"), - String::from("label_with_msg, 1st argument"), + String::from("label_with_message, 1st argument"), fst_pos, RichTerm { term: t1.into(), @@ -2632,7 +2632,7 @@ impl VirtualMachine { let Term::Lbl(label) = t2 else { return Err(EvalError::TypeError( String::from("Lbl"), - String::from("label_with_msg, 2nd argument"), + String::from("label_with_message, 2nd argument"), snd_pos, RichTerm { term: t2.into(), diff --git a/src/parser/grammar.lalrpop b/src/parser/grammar.lalrpop index 0ad969994d..34df4b1cb9 100644 --- a/src/parser/grammar.lalrpop +++ b/src/parser/grammar.lalrpop @@ -847,7 +847,7 @@ BOpPre: BinaryOp = { pending_contracts: Default::default(), }, "record_remove" => BinaryOp::DynRemove(), - "label_with_msg" => BinaryOp::LabelWithMsg(), + "label_with_message" => BinaryOp::LabelWithMessage(), "label_with_notes" => BinaryOp::LabelWithNotes(), "label_append_note" => BinaryOp::LabelAppendNote(), } @@ -1047,7 +1047,7 @@ extern { "str_from" => Token::Normal(NormalToken::ToStr), "num_from" => Token::Normal(NormalToken::NumFromStr), "enum_from" => Token::Normal(NormalToken::EnumFromStr), - "label_with_msg" => Token::Normal(NormalToken::LabelWithMsg), + "label_with_message" => Token::Normal(NormalToken::LabelWithMessage), "label_with_notes" => Token::Normal(NormalToken::LabelWithNotes), "label_append_note" => Token::Normal(NormalToken::LabelAppendNote), "label_push_diag" => Token::Normal(NormalToken::LabelPushDiag), diff --git a/src/parser/lexer.rs b/src/parser/lexer.rs index 47a7494a01..f969e5e0b5 100644 --- a/src/parser/lexer.rs +++ b/src/parser/lexer.rs @@ -309,7 +309,7 @@ pub enum NormalToken<'input> { #[token("%enum_from_str%")] EnumFromStr, #[token("%label_with_message%")] - LabelWithMsg, + LabelWithMessage, #[token("%label_with_notes%")] LabelWithNotes, #[token("%label_append_note%")] diff --git a/src/term/mod.rs b/src/term/mod.rs index e15f613226..1ef8a01265 100644 --- a/src/term/mod.rs +++ b/src/term/mod.rs @@ -1332,7 +1332,7 @@ pub enum BinaryOp { DictionaryAssume(), /// Set the message of the current diagnostic of a label. - LabelWithMsg(), + LabelWithMessage(), /// Set the notes of the current diagnostic of a label. LabelWithNotes(), /// Append a note to the current diagnostic of a label. diff --git a/src/typecheck/operation.rs b/src/typecheck/operation.rs index 025acc93c5..f2a41fc2c8 100644 --- a/src/typecheck/operation.rs +++ b/src/typecheck/operation.rs @@ -404,7 +404,7 @@ pub fn get_bop_type( } // Morally: Str -> Lbl -> Lbl // Actual: Str -> Dyn -> Dyn - BinaryOp::LabelWithMsg() => ( + BinaryOp::LabelWithMessage() => ( mk_uniftype::str(), mk_uniftype::dynamic(), mk_uniftype::dynamic(), From e095b76fb4a55ebbf7ed4a89348c205bc891d7bf Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Mar 2023 19:36:22 +0100 Subject: [PATCH 07/13] Fix typo in comments --- src/label.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/label.rs b/src/label.rs index 5c4b2cf4e1..8de03e59fa 100644 --- a/src/label.rs +++ b/src/label.rs @@ -430,7 +430,7 @@ impl Label { /// Set the message of the current diagnostic (the last diagnostic of the stack). Potentially /// erase the previous value. /// - /// If the diagnostic stack is empty, this methods pushes a new diagnostic with the given notes. + /// If the diagnostic stack is empty, this method pushes a new diagnostic with the given notes. pub fn with_diagnostic_message(mut self, message: impl Into) -> Self { if let Some(current) = self.diagnostics.last_mut() { current.message = Some(message.into()); @@ -445,7 +445,7 @@ impl Label { /// Set the notes of the current diagnostic (the last diagnostic of the stack). Potentially /// erase the previous value. /// - /// If the diagnostic stack is empty, this methods pushes a new diagnostic with the given notes. + /// If the diagnostic stack is empty, this method pushes a new diagnostic with the given notes. pub fn with_diagnostic_notes(mut self, notes: Vec) -> Self { if let Some(current) = self.diagnostics.last_mut() { current.notes = notes; @@ -460,7 +460,7 @@ impl Label { /// Append a note to the current diagnostic (the last diagnostic of the stack). Potentially /// erase the previous value. /// - /// If the diagnostic stack is empty, this methods pushes a new diagnostic with the given note. + /// If the diagnostic stack is empty, this method pushes a new diagnostic with the given note. pub fn append_diagnostic_note(mut self, note: impl Into) -> Self { if let Some(current) = self.diagnostics.last_mut() { current.append_note(note); From 20c8126cdeeb45b7a1b3f35f2d7238d39c68f8b5 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Mar 2023 19:49:41 +0100 Subject: [PATCH 08/13] Expand documentation of label, use notions consistently --- stdlib/contract.ncl | 56 ++++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/stdlib/contract.ncl b/stdlib/contract.ncl index 06757aa70e..893a931b25 100644 --- a/stdlib/contract.ncl +++ b/stdlib/contract.ncl @@ -67,18 +67,35 @@ label | doc m%" - The label submodule, which gathers functions that manipulate contract - labels. + The label submodule, which gathers functions that manipulate the label + of a contract. A label is a special opaque value automatically passed by the Nickel - interpreter to contracts at contract application. A label offers a way - for custom contracts to customize the error message that will be shown - if the contract is broken. + interpreter to contracts when performing a contract check. + + A label stores a stack of custom error diagnostics, than can be + manipulated by the function of this module. Labels thus offer a way to + customize the error message that will be shown if the contract is broken. + + The setters (`with_XXX` functions) always operate on the current error + diagnostic, which is the last diagnotic on the stack (if the stack is + empty, a fresh diagnostic is silently created when using a setter). + The previous diagnostics are thus archived, and can't be modified + anymore. All diagnostics will be shown during error reporting, with + the most recent being used as the main one. + + `contract.apply` is the operation that pushes a new fresh diagnostic on + the stack, saving the previously current error diagnostic. Indeed, + `contract.apply` is mostly used to apply subcontracts inside a parent + contract. Stacking the current diagnostic potentially customized by + the parent contract saves the information inside, a provide a fresh + diagnostic for the child contract to use. "% = { with_message | doc m%" - Attach a custom error message to a label. + Attach a custom error message to the current error diagnostic of a + label. Type: `Str -> Lbl -> Lbl` (for technical reasons, this function isn't actually statically typed) @@ -86,9 +103,9 @@ If a custom error message was previously set, there are two possibilities: - the label has gone through a `contract.apply` call in-between. - In this case, the previous custom reporting data are stacked, - and using `with_message` won't erase anything, but just add - a new diagnostic on the stack. + In this case, the previous diagnostic has been stacked, + and using `with_message` won't erase anything but rather modify + the fresh current diagnostic. - no `contract.apply` has taken place since the last message was set. In this case, the current diagnostic is still the same, and the previous error message will be erased. @@ -118,7 +135,8 @@ with_notes | doc m%" - Attach custom error notes to a label. + Attach custom error notes to the current error diagnostic of a + label. Type: `Array Str -> Lbl -> Lbl` (for technical reasons, this function isn't actually statically typed) @@ -126,10 +144,10 @@ If custom error notes were previously set, there are two possibilities: - the label has gone through a `contract.apply` call in-between. - In this case, the previous reporting data are stacked, - and using `with_notes` won't erase anything, but just add - a new diagnostic on the stack. - - no `contract.apply` has taken place since the last notes were + In this case, the previous diagnostic has been stacked, + and using `with_notes` won't erase anything but rather modify + the fresh current diagnostic. + - no `contract.apply` has taken place since the last message was set. In this case, the current diagnostic is still the same, and the previous error notes will be erased. @@ -157,13 +175,13 @@ 5 | Contract ``` "% - # the %label_with_notes% operator expect an array of string that is + # the %label_with_notes% operator expect an array of strings which is # fully evaluated, thus we force the notes first = fun notes label => %label_with_notes% (%force% notes) label, append_note | doc m%" - Append a note to the label's error notes. + Append a note to the notes of the current diagnostic of a label. Type: `Str -> Lbl -> Lbl` (for technical reasons, this function isn't actually statically typed) @@ -200,8 +218,8 @@ Using `apply` will stack the current custom reporting data, and create a fresh current working diagnostic. `apply` thus acts automatically as a split point between a contract and its subcontracts, providing the - subcontracts a fresh label to add their diagnostic to, while remembering - the previous reporting data set by the parent contract. + subcontracts with a fresh diagnostic to use, while remembering the + previous the previous diagnostics set by parent contracts. ## Illustration @@ -227,7 +245,7 @@ This example will print two diagnostics: the main one using, the message and note of the child contract, and a secondary diagnostic, - with parent's contract message and note. + using the parent contract message and note. "% = fun contract label value => %assume% contract (%label_push_diag% label) value, From 37ecba1872296f4579a6d232f5d7fdce02e0931e Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 7 Mar 2023 14:42:47 +0100 Subject: [PATCH 09/13] Update src/label.rs Co-authored-by: Oghenevwogaga Ebresafe --- src/label.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/label.rs b/src/label.rs index 8de03e59fa..74952dae0b 100644 --- a/src/label.rs +++ b/src/label.rs @@ -478,7 +478,7 @@ impl Label { self.diagnostics.last() } - /// Push a new fresh diagnostic on the stack if the current diagnostic isn't empty. This has + /// Push a new, fresh diagnostic on the stack if the current diagnostic isn't empty. This has /// the effect of saving the current diagnostic, that can't then be mutated anymore by the /// label's API. pub fn push_diagnostic(&mut self) { From 4f033d97ff0cde009f66fffa68c88ad49940c206 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Wed, 8 Mar 2023 14:45:43 +0100 Subject: [PATCH 10/13] Update stdlib/contract.ncl Co-authored-by: Viktor Kleen --- stdlib/contract.ncl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/contract.ncl b/stdlib/contract.ncl index 893a931b25..b562fdf140 100644 --- a/stdlib/contract.ncl +++ b/stdlib/contract.ncl @@ -88,7 +88,7 @@ the stack, saving the previously current error diagnostic. Indeed, `contract.apply` is mostly used to apply subcontracts inside a parent contract. Stacking the current diagnostic potentially customized by - the parent contract saves the information inside, a provide a fresh + the parent contract saves the information inside, and provides a fresh diagnostic for the child contract to use. "% = { From eb3b5aff9040b6e7659788a3c6b0ed914f26cc72 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Wed, 8 Mar 2023 14:46:00 +0100 Subject: [PATCH 11/13] Fix typo in comment (stdlib/contract.ncl) Co-authored-by: Viktor Kleen --- stdlib/contract.ncl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/contract.ncl b/stdlib/contract.ncl index b562fdf140..33d995b1ba 100644 --- a/stdlib/contract.ncl +++ b/stdlib/contract.ncl @@ -219,7 +219,7 @@ fresh current working diagnostic. `apply` thus acts automatically as a split point between a contract and its subcontracts, providing the subcontracts with a fresh diagnostic to use, while remembering the - previous the previous diagnostics set by parent contracts. + previous diagnostics set by parent contracts. ## Illustration From b4549ebfc693db5e9a805d77509e2107232668a7 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Wed, 8 Mar 2023 14:46:16 +0100 Subject: [PATCH 12/13] Fix typo in comment (stdlib/contract.ncl) Co-authored-by: Viktor Kleen --- stdlib/contract.ncl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/contract.ncl b/stdlib/contract.ncl index 33d995b1ba..4e6c9db58c 100644 --- a/stdlib/contract.ncl +++ b/stdlib/contract.ncl @@ -73,7 +73,7 @@ A label is a special opaque value automatically passed by the Nickel interpreter to contracts when performing a contract check. - A label stores a stack of custom error diagnostics, than can be + A label stores a stack of custom error diagnostics, that can be manipulated by the function of this module. Labels thus offer a way to customize the error message that will be shown if the contract is broken. From 8939952cf0a759a546f88332696a072ad71edcad Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Wed, 8 Mar 2023 14:47:15 +0100 Subject: [PATCH 13/13] Fix typo in comment (stdlib/contract.ncl) Co-authored-by: Viktor Kleen --- stdlib/contract.ncl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/contract.ncl b/stdlib/contract.ncl index 4e6c9db58c..b891da2a95 100644 --- a/stdlib/contract.ncl +++ b/stdlib/contract.ncl @@ -243,7 +243,7 @@ null | ParentContract ``` - This example will print two diagnostics: the main one using, the + This example will print two diagnostics: the main one, using the message and note of the child contract, and a secondary diagnostic, using the parent contract message and note. "%