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

Get rid of uninformative generic notes for higher-order contract errors #1564

Merged
merged 1 commit into from
Sep 1, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@ error: contract broken by the caller of `map`
3 │ std.array.map std.function.id 'not-an-array
│ ------------- evaluated to this expression
= This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `Number -> Number`.
2. `f` is called with an argument of the wrong type: e.g. `f false`.
= Either change the contract accordingly, or call `f` with an argument of the right type.
= Note: this is an illustrative example. The actual error may involve deeper nested functions calls.

note:
┌─ [INPUTS_PATH]/errors/caller_contract_violation.ncl:3:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,5 @@ error: contract broken by a function
1 │ "a"
│ --- evaluated to this value
= This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `Bool -> Number`.
2. `f` returns a value of the wrong type: e.g. `f = fun c => "string"` while `Number` is expected.
= Either change the contract accordingly, or change the return value of `f`


Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,5 @@ error: contract broken by the function `f`
│ ------ ------------- evaluated to this expression
│ │
│ expected return type
= This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `Bool -> Number`.
2. `f` returns a value of the wrong type: e.g. `f = fun c => "string"` while `Number` is expected.
= Either change the contract accordingly, or change the return value of `f`


Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: tests/snapshot/main.rs
source: cli/tests/snapshot/main.rs
expression: err
---
error: contract broken by the caller: field not allowed in tail: `x`
Expand All @@ -14,12 +14,6 @@ error: contract broken by the caller: field not allowed in tail: `x`
1 │ { ... }
│ ------- evaluated to this value
= This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `Number -> Number`.
2. `f` is called with an argument of the wrong type: e.g. `f false`.
= Either change the contract accordingly, or call `f` with an argument of the right type.
= Note: this is an illustrative example. The actual error may involve deeper nested functions calls.

note:
┌─ [INPUTS_PATH]/errors/record_forall_constraints_contract.ncl:3:58
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,5 @@ error: contract broken by a function
1 │ "string"
│ -------- evaluated to this value
= This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `Bool -> Number`.
2. `f` returns a value of the wrong type: e.g. `f = fun c => "string"` while `Number` is expected.
= Either change the contract accordingly, or change the return value of `f`


108 changes: 19 additions & 89 deletions core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1446,121 +1446,64 @@ mod blame_error {
}

/// Generate a codespan label that describes the [type path][crate::label::ty_path::Path] of a
/// (Nickel) label, and notes to hint at the situation that may have caused the corresponding
/// error.
pub fn report_ty_path(
files: &mut Files<String>,
l: &label::Label,
) -> (Label<FileId>, Vec<String>) {
let end_note = String::from("Note: this is an illustrative example. The actual error may involve deeper nested functions calls.");

/// (Nickel) label.
pub fn report_ty_path(files: &mut Files<String>, l: &label::Label) -> Label<FileId> {
let PathSpan {
span,
last,
last_arrow_elem,
} = path_span(files, &l.path, &l.typ);

let (msg, notes) = match (last, last_arrow_elem) {
let msg = match (last, last_arrow_elem) {
// The type path doesn't contain any arrow, and the failing subcontract is the
// contract for the elements of an array
(Some(ty_path::Elem::Array), None) => (String::from("expected array element type"), Vec::new()),
(Some(ty_path::Elem::Array), None) => "expected array element type",
// The type path doesn't contain any arrow, and the failing subcontract is the contract
// for the fields of a dictionary
(Some(ty_path::Elem::Dict), None) => (String::from("expected dictionary field type"), Vec::new()),
(Some(ty_path::Elem::Dict), None) => "expected dictionary field type",
// The type path doesn't contain any arrow, and the failing subcontract is the contract
// for the field of a record
(Some(ty_path::Elem::Field(_)), None) => (String::from("expected field type"), Vec::new()),
(Some(ty_path::Elem::Field(_)), None) => "expected field type",
// The original contract contains an arrow, and the path is only composed of codomains.
// Then polarity is necessarily true and the cause of the blame is the return value of
// the function
(Some(_), Some(ty_path::Elem::Codomain)) if ty_path::has_no_dom(&l.path) => {
(
String::from("expected return type"),
vec![
String::from(
"This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `Bool -> Number`.
2. `f` returns a value of the wrong type: e.g. `f = fun c => \"string\"` while `Number` is expected.",
),
String::from(
"Either change the contract accordingly, or change the return value of `f`",
),
],
)
"expected return type"
}
// The original contract contains an arrow, the subcontract is the domain of an
// arrow, and the polarity is positive. The function is to be blamed for calling an
// argument on a value of the wrong type.
(Some(_), Some(ty_path::Elem::Domain)) if l.polarity == Polarity::Positive => {
(
String::from("expected type of an argument of an inner call"),
vec![
String::from("This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `(String -> String) -> String)`.
2. `f` takes another function `g` as an argument: e.g. `f = fun g => g 0`.
3. `f` calls `g` with an argument that does not respect the contract: e.g. `g 0` while `String -> String` is expected."),
String::from("Either change the contract accordingly, or call `g` with a `String` argument."),
end_note,
],
)
"expected type of an argument of an inner call"
}
// The original contract contains an arrow, the subcontract is the codomain of an
// arrow, and the polarity is positive. The function is to be blamed for calling a
// higher-order function argument on a function which returns a value of the wrong
// type.
(Some(_), Some(ty_path::Elem::Codomain)) if l.polarity == Polarity::Positive => {
(
String::from("expected return type of a sub-function passed as an argument of an inner call"),
vec![
String::from("This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `((Number -> Number) -> Number) -> Number)`.
2. `f` take another function `g` as an argument: e.g. `f = fun g => g (fun x => true)`.
3. `g` itself takes a function as an argument.
4. `f` passes a function that does not respect the contract to `g`: e.g. `g (fun x => true)` (expected to be of type `Number -> Number`)."),
String::from("Either change the contract accordingly, or call `g` with a function that returns a value of type `Number`."),
end_note,
],
)
"expected return type of a sub-function passed as an argument of an inner call"
}
// The original contract contains an arrow, the subcontract is the domain of an arrow,
// and the polarity is negative. The caller is to be blamed for providing an argument
// of the wrong type.
(Some(_), Some(ty_path::Elem::Domain)) => {
(
String::from("expected type of the argument provided by the caller"),
vec![
String::from("This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `Number -> Number`.
2. `f` is called with an argument of the wrong type: e.g. `f false`."),
String::from("Either change the contract accordingly, or call `f` with an argument of the right type."),
end_note,
],
)
"expected type of the argument provided by the caller"
}
// The original contract contains an arrow, the subcontract is the codomain of an
// arrow, and the polarity is negative. The caller is to be blamed for providing a
// higher-order function argument which returns a value of the wrong type.
(Some(_), Some(ty_path::Elem::Codomain)) => {
(
String::from("expected return type of a function provided by the caller"),
vec![
String::from("This error may happen in the following situation:
1. A function `f` is bound by a contract: e.g. `(Number -> Number) -> Number`.
2. `f` takes another function `g` as an argument: e.g. `f = fun g => g 0`.
3. `f` is called by with an argument `g` that does not respect the contract: e.g. `f (fun x => false)`."),
String::from("Either change the contract accordingly, or call `f` with a function that returns a value of the right type."),
end_note,
],
)
"expected return type of a function provided by the caller"
}
// If there is a last arrow element, then there must be last element
(None, Some(_)) => panic!("blame error reporting: inconsistent path analysis, last_elem\
is None but last_arrow_elem is Some"),
_ => (String::from("expected type"), Vec::new()),
(None, Some(_)) => panic!(
"blame error reporting: inconsistent path analysis, last_elem\
is None but last_arrow_elem is Some"
),
_ => "expected type",
};

let label = secondary(&span).with_message(msg);
(label, notes)
secondary(&span).with_message(msg.to_owned())
}

/// Return a note diagnostic showing where a contract was bound.
Expand Down Expand Up @@ -1613,7 +1556,7 @@ is None but last_arrow_elem is Some"),
let contract_notes = head_contract_diagnostic
.map(|diag| diag.notes)
.unwrap_or_default();
let (path_label, notes_higher_order) = report_ty_path(files, &label);
let path_label = report_ty_path(files, &label);

let labels = build_diagnostic_labels(evaluated_arg, &label, path_label, files, stdlib_ids);

Expand All @@ -1627,21 +1570,8 @@ is None but last_arrow_elem is Some"),
.with_labels(labels)
.with_notes(contract_notes),
);

if !notes_higher_order.is_empty() {
diagnostics.push(
Diagnostic::note()
.with_message("due to a function contract violation")
.with_notes(notes_higher_order),
);
}
} else {
diagnostics.push(
Diagnostic::error()
.with_message(msg)
.with_labels(labels)
.with_notes(notes_higher_order),
);
diagnostics.push(Diagnostic::error().with_message(msg).with_labels(labels));
}

for ctr_diag in contract_diagnostics {
Expand Down