From efeda9a998fc79f28ec25b9999f8ba04ba4f34a6 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Tue, 27 Aug 2024 20:12:34 +0200 Subject: [PATCH] Prevent non-default fallback on exhaustive validator Technically, we always need a fallback just because the way the UPLC is going to work. The last case in the handler pattern matching is always going to be else ... We could optimize that away and when the validator is exhaustive, make the last handler the fallback. Yet, it's really a micro optimization that saves us one extra if/else. So the sake of getting things working, we always assume that there's a fallback but, with the extra condition that when the validator is exhaustive (i.e. there's a handler covering all purposes), the fallback HAS TO BE the default fallback (i.e. (_) => fail). This allows us to gracefully format it out, and also raise an error in case where there's an extraneous custom fallback. --- crates/aiken-lang/src/ast.rs | 42 ++++++- crates/aiken-lang/src/ast/well_known.rs | 6 +- crates/aiken-lang/src/format.rs | 48 ++++---- .../src/parser/definition/validator.rs | 26 +---- crates/aiken-lang/src/tests/check.rs | 73 ++++++++++++ crates/aiken-lang/src/tests/format.rs | 107 ++++++++++++++++++ .../format_validator_exhaustive_handlers.snap | 29 +++++ ...stive_handlers_extra_default_fallback.snap | 29 +++++ ...e_handlers_extra_non_default_fallback.snap | 33 ++++++ crates/aiken-lang/src/tipo/error.rs | 11 ++ crates/aiken-lang/src/tipo/infer.rs | 17 ++- 11 files changed, 369 insertions(+), 52 deletions(-) create mode 100644 crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers.snap create mode 100644 crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers_extra_default_fallback.snap create mode 100644 crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers_extra_non_default_fallback.snap diff --git a/crates/aiken-lang/src/ast.rs b/crates/aiken-lang/src/ast.rs index 1c94123d..ae90feda 100644 --- a/crates/aiken-lang/src/ast.rs +++ b/crates/aiken-lang/src/ast.rs @@ -242,6 +242,19 @@ fn str_to_keyword(word: &str) -> Option { pub type TypedFunction = Function, TypedExpr, TypedArg>; pub type UntypedFunction = Function<(), UntypedExpr, UntypedArg>; +impl UntypedFunction { + pub fn is_default_fallback(&self) -> bool { + matches!( + &self.arguments[..], + [UntypedArg { + by: ArgBy::ByName(ArgName::Discarded { .. }), + .. + }] + ) && matches!(&self.body, UntypedExpr::ErrorTerm { .. }) + && self.name.as_str() == well_known::VALIDATOR_ELSE + } +} + pub type TypedTest = Function, TypedExpr, TypedArgVia>; pub type UntypedTest = Function<(), UntypedExpr, UntypedArgVia>; @@ -490,6 +503,33 @@ impl Validator { } } +impl UntypedValidator { + pub fn default_fallback(location: Span) -> UntypedFunction { + Function { + arguments: vec![UntypedArg { + by: ArgBy::ByName(ArgName::Discarded { + name: "_".to_string(), + label: "_".to_string(), + location, + }), + location, + annotation: None, + doc: None, + is_validator_param: false, + }], + body: UntypedExpr::fail(None, location), + doc: None, + location, + end_position: location.end - 1, + name: well_known::VALIDATOR_ELSE.to_string(), + public: true, + return_annotation: Some(Annotation::boolean(location)), + return_type: (), + on_test_failure: OnTestFailure::FailImmediately, + } + } +} + impl TypedValidator { pub fn available_handler_names() -> Vec { vec![ @@ -621,8 +661,6 @@ impl TypedValidator { }, } }) - // FIXME: This is only needed if there's non-exhaustive patterns - // above. .chain(std::iter::once(&self.fallback).map(|fallback| { let arg = fallback.arguments.first().unwrap(); diff --git a/crates/aiken-lang/src/ast/well_known.rs b/crates/aiken-lang/src/ast/well_known.rs index 74e4a71e..a933b5e2 100644 --- a/crates/aiken-lang/src/ast/well_known.rs +++ b/crates/aiken-lang/src/ast/well_known.rs @@ -256,8 +256,7 @@ impl Type { pub fn list(t: Rc) -> Rc { Rc::new(Type::App { public: true, - // FIXME: We should probably have t.contains_opaque here? - contains_opaque: false, + contains_opaque: t.contains_opaque(), name: LIST.to_string(), module: "".to_string(), args: vec![t], @@ -290,8 +289,7 @@ impl Type { pub fn option(a: Rc) -> Rc { Rc::new(Type::App { public: true, - // FIXME: We should probably have t.contains_opaque here? - contains_opaque: false, + contains_opaque: a.contains_opaque(), name: OPTION.to_string(), module: "".to_string(), args: vec![a], diff --git a/crates/aiken-lang/src/format.rs b/crates/aiken-lang/src/format.rs index d399bd8f..f3294673 100644 --- a/crates/aiken-lang/src/format.rs +++ b/crates/aiken-lang/src/format.rs @@ -3,10 +3,10 @@ use crate::{ Annotation, ArgBy, ArgName, ArgVia, AssignmentKind, AssignmentPattern, BinOp, ByteArrayFormatPreference, CallArg, Constant, CurveType, DataType, Definition, Function, LogicalOpChainKind, ModuleConstant, OnTestFailure, Pattern, RecordConstructor, - RecordConstructorArg, RecordUpdateSpread, Span, TraceKind, TypeAlias, TypedArg, UnOp, - UnqualifiedImport, UntypedArg, UntypedArgVia, UntypedAssignmentKind, UntypedClause, - UntypedDefinition, UntypedFunction, UntypedIfBranch, UntypedModule, UntypedPattern, - UntypedRecordUpdateArg, Use, Validator, CAPTURE_VARIABLE, + RecordConstructorArg, RecordUpdateSpread, Span, TraceKind, TypeAlias, TypedArg, + TypedValidator, UnOp, UnqualifiedImport, UntypedArg, UntypedArgVia, UntypedAssignmentKind, + UntypedClause, UntypedDefinition, UntypedFunction, UntypedIfBranch, UntypedModule, + UntypedPattern, UntypedRecordUpdateArg, Use, Validator, CAPTURE_VARIABLE, }, docvec, expr::{FnStyle, UntypedExpr, DEFAULT_ERROR_STR, DEFAULT_TODO_STR}, @@ -643,27 +643,31 @@ impl<'comments> Formatter<'comments> { handler_docs.push(first_fn); } - let fallback_comments = self.pop_comments(fallback.location.start); - let fallback_doc_comments = self.doc_comments(fallback.location.start); + let is_exhaustive = handlers.len() >= TypedValidator::available_handler_names().len() - 1; - let fallback_fn = self - .definition_fn( - &fallback.public, - &fallback.name, - &fallback.arguments, - &fallback.return_annotation, - &fallback.body, - fallback.end_position, - true, - ) - .group(); + if !is_exhaustive || !fallback.is_default_fallback() { + let fallback_comments = self.pop_comments(fallback.location.start); + let fallback_doc_comments = self.doc_comments(fallback.location.start); - let fallback_fn = commented( - fallback_doc_comments.append(fallback_fn).group(), - fallback_comments, - ); + let fallback_fn = self + .definition_fn( + &fallback.public, + &fallback.name, + &fallback.arguments, + &fallback.return_annotation, + &fallback.body, + fallback.end_position, + true, + ) + .group(); - handler_docs.push(fallback_fn); + let fallback_fn = commented( + fallback_doc_comments.append(fallback_fn).group(), + fallback_comments, + ); + + handler_docs.push(fallback_fn); + } let v_body = line().append(join(handler_docs, lines(2))); diff --git a/crates/aiken-lang/src/parser/definition/validator.rs b/crates/aiken-lang/src/parser/definition/validator.rs index 66e1720b..bac310a0 100644 --- a/crates/aiken-lang/src/parser/definition/validator.rs +++ b/crates/aiken-lang/src/parser/definition/validator.rs @@ -1,6 +1,6 @@ use super::function::param; use crate::{ - ast::{self, well_known, ArgBy, ArgName}, + ast::{self, well_known}, expr::UntypedExpr, parser::{annotation, error::ParseError, expr, token::Token}, }; @@ -63,28 +63,8 @@ pub fn parser() -> impl Parser Bool { + fail + } + } + "#; + + assert!(matches!( + check_validator(parse(source_code)), + Err((_, Error::UnexpectedValidatorFallback { .. })) + )) +} diff --git a/crates/aiken-lang/src/tests/format.rs b/crates/aiken-lang/src/tests/format.rs index 65bd4f06..353e04ed 100644 --- a/crates/aiken-lang/src/tests/format.rs +++ b/crates/aiken-lang/src/tests/format.rs @@ -1140,3 +1140,110 @@ fn format_long_pair() { "# ); } + +#[test] +fn format_validator_exhaustive_handlers() { + assert_format!( + r#" + validator foo { + mint(_redeemer, _policy_id, _self) { + True + } + + spend(_datum, _redeemer, _policy_id, _self) { + True + } + + withdraw(_redeemer, _account, _self) { + True + } + + publish(_redeemer, _certificate, _self) { + True + } + + vote(_redeemer, _voter, _self) { + True + } + + propose(_redeemer, _proposal, _self) { + True + } + } + "# + ); +} + +#[test] +fn format_validator_exhaustive_handlers_extra_default_fallback() { + assert_format!( + r#" + validator foo { + mint(_redeemer, _policy_id, _self) { + True + } + + spend(_datum, _redeemer, _policy_id, _self) { + True + } + + withdraw(_redeemer, _account, _self) { + True + } + + publish(_redeemer, _certificate, _self) { + True + } + + vote(_redeemer, _voter, _self) { + True + } + + propose(_redeemer, _proposal, _self) { + True + } + + else(_) { + fail + } + } + "# + ); +} + +#[test] +fn format_validator_exhaustive_handlers_extra_non_default_fallback() { + assert_format!( + r#" + validator foo { + mint(_redeemer, _policy_id, _self) { + True + } + + spend(_datum, _redeemer, _policy_id, _self) { + True + } + + withdraw(_redeemer, _account, _self) { + True + } + + publish(_redeemer, _certificate, _self) { + True + } + + vote(_redeemer, _voter, _self) { + True + } + + propose(_redeemer, _proposal, _self) { + True + } + + else(_) { + True + } + } + "# + ); +} diff --git a/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers.snap b/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers.snap new file mode 100644 index 00000000..078ea81d --- /dev/null +++ b/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers.snap @@ -0,0 +1,29 @@ +--- +source: crates/aiken-lang/src/tests/format.rs +description: "Code:\n\nvalidator foo {\n mint(_redeemer, _policy_id, _self) {\n True\n }\n\n spend(_datum, _redeemer, _policy_id, _self) {\n True\n }\n\n withdraw(_redeemer, _account, _self) {\n True\n }\n\n publish(_redeemer, _certificate, _self) {\n True\n }\n\n vote(_redeemer, _voter, _self) {\n True\n }\n\n propose(_redeemer, _proposal, _self) {\n True\n }\n}\n" +--- +validator foo { + mint(_redeemer, _policy_id, _self) { + True + } + + spend(_datum, _redeemer, _policy_id, _self) { + True + } + + withdraw(_redeemer, _account, _self) { + True + } + + publish(_redeemer, _certificate, _self) { + True + } + + vote(_redeemer, _voter, _self) { + True + } + + propose(_redeemer, _proposal, _self) { + True + } +} diff --git a/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers_extra_default_fallback.snap b/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers_extra_default_fallback.snap new file mode 100644 index 00000000..f8052bee --- /dev/null +++ b/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers_extra_default_fallback.snap @@ -0,0 +1,29 @@ +--- +source: crates/aiken-lang/src/tests/format.rs +description: "Code:\n\nvalidator foo {\n mint(_redeemer, _policy_id, _self) {\n True\n }\n\n spend(_datum, _redeemer, _policy_id, _self) {\n True\n }\n\n withdraw(_redeemer, _account, _self) {\n True\n }\n\n publish(_redeemer, _certificate, _self) {\n True\n }\n\n vote(_redeemer, _voter, _self) {\n True\n }\n\n propose(_redeemer, _proposal, _self) {\n True\n }\n\n else(_) {\n fail\n }\n}\n" +--- +validator foo { + mint(_redeemer, _policy_id, _self) { + True + } + + spend(_datum, _redeemer, _policy_id, _self) { + True + } + + withdraw(_redeemer, _account, _self) { + True + } + + publish(_redeemer, _certificate, _self) { + True + } + + vote(_redeemer, _voter, _self) { + True + } + + propose(_redeemer, _proposal, _self) { + True + } +} diff --git a/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers_extra_non_default_fallback.snap b/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers_extra_non_default_fallback.snap new file mode 100644 index 00000000..2bcb68f4 --- /dev/null +++ b/crates/aiken-lang/src/tests/snapshots/format_validator_exhaustive_handlers_extra_non_default_fallback.snap @@ -0,0 +1,33 @@ +--- +source: crates/aiken-lang/src/tests/format.rs +description: "Code:\n\nvalidator foo {\n mint(_redeemer, _policy_id, _self) {\n True\n }\n\n spend(_datum, _redeemer, _policy_id, _self) {\n True\n }\n\n withdraw(_redeemer, _account, _self) {\n True\n }\n\n publish(_redeemer, _certificate, _self) {\n True\n }\n\n vote(_redeemer, _voter, _self) {\n True\n }\n\n propose(_redeemer, _proposal, _self) {\n True\n }\n\n else(_) {\n True\n }\n}\n" +--- +validator foo { + mint(_redeemer, _policy_id, _self) { + True + } + + spend(_datum, _redeemer, _policy_id, _self) { + True + } + + withdraw(_redeemer, _account, _self) { + True + } + + publish(_redeemer, _certificate, _self) { + True + } + + vote(_redeemer, _voter, _self) { + True + } + + propose(_redeemer, _proposal, _self) { + True + } + + else(_) { + True + } +} diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index 58f14d9c..e40a6d48 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -1101,6 +1101,16 @@ The best thing to do from here is to remove it."#))] location: Span, available_handlers: Vec, }, + + #[error("I caught an extraneous fallback handler in an already exhaustive validator\n")] + #[diagnostic(code("extraneous::fallback"))] + #[diagnostic(help( + "Validator handlers must be exhaustive and either cover all purposes, or provide a fallback handler. Here, you have successfully covered all script purposes with your handler, but left an extraneous fallback branch. I cannot let that happen, but removing it for you would probably be deemed rude. So please, remove the fallback." + ))] + UnexpectedValidatorFallback { + #[label("redundant fallback handler")] + fallback: Span, + }, } impl ExtraData for Error { @@ -1162,6 +1172,7 @@ impl ExtraData for Error { | Error::ValidatorMustReturnBool { .. } | Error::UnknownPurpose { .. } | Error::UnknownValidatorHandler { .. } + | Error::UnexpectedValidatorFallback { .. } | Error::MustInferFirst { .. } => None, Error::UnknownType { name, .. } diff --git a/crates/aiken-lang/src/tipo/infer.rs b/crates/aiken-lang/src/tipo/infer.rs index a2afb5f8..1c669236 100644 --- a/crates/aiken-lang/src/tipo/infer.rs +++ b/crates/aiken-lang/src/tipo/infer.rs @@ -9,7 +9,8 @@ use crate::{ ast::{ Annotation, ArgName, ArgVia, DataType, Definition, Function, ModuleConstant, ModuleKind, RecordConstructor, RecordConstructorArg, Tracing, TypeAlias, TypedArg, TypedDefinition, - TypedModule, TypedValidator, UntypedArg, UntypedDefinition, UntypedModule, Use, Validator, + TypedModule, TypedValidator, UntypedArg, UntypedDefinition, UntypedModule, + UntypedValidator, Use, Validator, }, tipo::{expr::infer_function, Span, Type, TypeVar}, IdGenerator, @@ -247,6 +248,20 @@ fn infer_definition( typed_handlers.push(typed_fun); } + // NOTE: Duplicates are handled when registering handler names. So if we have N + // typed handlers, they are different. The -1 represents takes out the fallback + // handler name. + let is_exhaustive = + typed_handlers.len() >= TypedValidator::available_handler_names().len() - 1; + + if is_exhaustive + && fallback != UntypedValidator::default_fallback(fallback.location) + { + return Err(Error::UnexpectedValidatorFallback { + fallback: fallback.location, + }); + } + let (typed_params, typed_fallback) = environment.in_new_scope(|environment| { let temp_params = params.iter().cloned().chain(fallback.arguments); fallback.arguments = temp_params.collect();