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

Add InvalidContractError #1824

Merged
merged 1 commit into from
Feb 21, 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# capture = 'stderr'
# command = ['eval']
{ foo | Number -> [| 'Foo 5 |] = null }
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: cli/tests/snapshot/main.rs
expression: err
---
error: invalid contract expression
┌─ [INPUTS_PATH]/errors/invalid_contract_expression.ncl:3:27
3 │ { foo | Number -> [| 'Foo 5 |] = null }
│ ^ this can't be used as a contract
= This expression is used as a contract as part of an annotation or a type expression.
= Only functions and records might be valid contracts


33 changes: 19 additions & 14 deletions core/src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,15 @@ pub enum TypecheckError {
pos: TermPos,
},
/// Within statically typed code, the typechecker must reject terms containing nonsensical
/// contracts such as `let C = { foo : 5 } in ({ foo = 5 } | C)`, which will fail at runtime.
/// contracts such as `let C = { foo : (4 + 1) } in ({ foo = 5 } | C)`, which will fail at
/// runtime.
///
/// The typechecker is currently quite conservative and simply forbids to store any custom
/// contract (flat type) in a type that appears in term position. Note that this restriction
/// doesn't apply to annotations, which aren't considered part of the statically typed block.
/// For example, `{foo = 5} | {foo : 5}` is accepted by the typechecker.
/// For example, `{foo = 5} | {foo : (4 + 1)}` is accepted by the typechecker.
FlatTypeInTermPosition {
/// The term that was in a flat type (the `5` in the example above).
/// The term that was in a flat type (the `(4 + 1)` in the example above).
flat: RichTerm,
/// The position of the entire type (the `{foo : 5}` in the example above).
pos: TermPos,
Expand Down Expand Up @@ -545,8 +546,13 @@ pub enum ParseError {
/// The previous instance of the duplicated identifier.
prev_ident: LocIdent,
},
/// There was an attempt to use a feature that hasn't been enabled
/// There was an attempt to use a feature that hasn't been enabled.
DisabledFeature { feature: String, span: RawSpan },
/// A term was used as a contract in type position, but this term has no chance to make any
/// sense as a contract. What terms make sense might evolve with time, but any given point in
/// time, there are a set of expressions that can be excluded syntactically. Currently, it's
/// mostly constants.
InvalidContract(RawSpan),
}

/// An error occurring during the resolution of an import.
Expand Down Expand Up @@ -756,6 +762,7 @@ impl ParseError {
path_elem_span,
}
}
InternalParseError::InvalidContract(span) => ParseError::InvalidContract(span),
},
}
}
Expand Down Expand Up @@ -1931,22 +1938,20 @@ impl IntoDiagnostics<FileId> for ParseError {
"Recompile nickel with `--features {}`",
feature
)]),
ParseError::InvalidContract(span) => Diagnostic::error()
.with_message("invalid contract expression")
.with_labels(vec![primary(&span).with_message("this can't be used as a contract")])
.with_notes(vec![
"This expression is used as a contract as part of an annotation or a type expression."
.to_owned(),
"Only functions and records might be valid contracts".to_owned(),
]),
};

vec![diagnostic]
}
}

// let find_row = |row_item: EnumRowsIteratorItem<'_, Type>| match row_item {
// EnumRowsIteratorItem::Row(row) if row.id.ident() == id.ident() => {
// Some(EnumRow {
// id: row.id,
// typ: row.typ.cloned().map(Box::new),
// })
// }
// _ => None,
// };

impl IntoDiagnostics<FileId> for TypecheckError {
fn into_diagnostics(
self,
Expand Down
22 changes: 10 additions & 12 deletions core/src/parser/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,14 @@ pub enum ParseError {
/// The previous instance of the duplicated identifier.
prev_ident: LocIdent,
},
/// A type variable is used in ways that imply it has muiltiple different kinds.
/// A type variable is used in ways that imply it has multiple different kinds.
///
/// This can happen in several situations, for example:
/// - a variable is used as both a type variable and a row type variable,
/// e.g. in the signature `forall r. { ; r } -> r`,
/// - a variable is used as both a record and enum row variable, e.g. in the
/// signature `forall r. [| ; r |] -> { ; r }`.
TypeVariableKindMismatch {
ty_var: LocIdent,
span: RawSpan,
},
TypeVariableKindMismatch { ty_var: LocIdent, span: RawSpan },
/// A record literal, which isn't a record type, has a field with a type annotation but without
/// a definition. While we could technically handle this situation, this is most probably an
/// error from the user, because this type annotation is useless and, maybe non-intuitively,
Expand All @@ -139,11 +136,12 @@ pub enum ParseError {
},
/// The user provided a field path on the CLI, which is expected to be only composed of
/// literals, but the parsed field path contains string interpolation.
InterpolationInStaticPath {
path_elem_span: RawSpan,
},
DisabledFeature {
feature: String,
span: RawSpan,
},
InterpolationInStaticPath { path_elem_span: RawSpan },
/// There was an attempt to use a feature that hasn't been enabled.
DisabledFeature { feature: String, span: RawSpan },
/// A term was used as a contract in type position, but this term has no chance to make any
/// sense as a contract. What terms make sense might evolve with time, but any given point in
/// time, there are a set of expressions that can be excluded syntactically. Currently, it's
/// mostly constants.
InvalidContract(RawSpan),
}
20 changes: 19 additions & 1 deletion core/src/parser/uniterm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,25 @@ impl TryFrom<UniTerm> for Type {
UniTermNode::Var(id) => Type::from(TypeF::Var(id.ident())),
UniTermNode::Record(r) => Type::try_from(r)?,
UniTermNode::Type(ty) => ty,
UniTermNode::Term(rt) => Type::from(TypeF::Flat(rt)),
UniTermNode::Term(rt) => {
if matches!(
rt.as_ref(),
Term::Null
| Term::Bool(_)
| Term::Num(_)
| Term::Str(_)
| Term::Array(..)
| Term::Enum(_)
| Term::EnumVariant { .. }
| Term::StrChunks(..)
) {
//unwrap(): uniterms are supposed to come from the parser, and thus have a
//well-defined position
return Err(ParseError::InvalidContract(ut.pos.unwrap()));
}

Type::from(TypeF::Flat(rt))
}
};

Ok(ty_without_pos.with_pos(ut.pos))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
#
# [test.metadata]
# error = 'TypecheckError::FlatTypeInTermPosition'
(let c = Number -> 5 in 3): _
(let c = Number -> (4 + 1) in 3): _
2 changes: 1 addition & 1 deletion doc/manual/merging.md
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ This evaluates to:
},
server = {
host = {
"options": "TLS"
"options" = "TLS",
}
}
}
Expand Down
Loading