From 10be09fe3038c9547e6e7d02297d08f1c5c41d74 Mon Sep 17 00:00:00 2001 From: Ulf Norell Date: Mon, 4 Feb 2019 11:56:06 +0100 Subject: [PATCH] Add checks on event constructor arguments to type checker --- src/aeso_ast_infer_types.erl | 46 ++++++++++++++++++++++++++++--- test/aeso_compiler_tests.erl | 16 ++++++----- test/contract_tests.erl | 28 ------------------- test/contracts/bad_events.aes | 25 +++++++++++++++++ test/contracts/bad_events2.aes | 23 ++++++++++++++++ test/contracts/simple_storage.aes | 1 - 6 files changed, 99 insertions(+), 40 deletions(-) delete mode 100644 test/contract_tests.erl create mode 100644 test/contracts/bad_events.aes create mode 100644 test/contracts/bad_events2.aes diff --git a/src/aeso_ast_infer_types.erl b/src/aeso_ast_infer_types.erl index 55f3730..6d3d7c7 100644 --- a/src/aeso_ast_infer_types.erl +++ b/src/aeso_ast_infer_types.erl @@ -607,11 +607,13 @@ check_event(Env, "event", Ann, Def) -> case Def of {variant_t, Cons} -> {variant_t, [ check_event_con(Env, Con) || Con <- Cons ]}; - _ -> type_error({event_must_be_variant_type, Ann}) + _ -> + type_error({event_must_be_variant_type, Ann}), + Def end; check_event(_Env, _Name, _Ann, Def) -> Def. -check_event_con(_Env, {constr_t, Ann, Con, Args}) -> +check_event_con(Env, {constr_t, Ann, Con, Args}) -> IsIndexed = fun(T) -> case aeso_syntax:get_ann(indexed, T, false) of true -> indexed; false -> notindexed @@ -619,11 +621,31 @@ check_event_con(_Env, {constr_t, Ann, Con, Args}) -> Indices = lists:map(IsIndexed, Args), Indexed = [ T || T <- Args, IsIndexed(T) == indexed ], NonIndexed = Args -- Indexed, + [ check_event_arg_type(Env, Ix, Type) || {Ix, Type} <- lists:zip(Indices, Args) ], %% TODO: Is is possible to check also the types of arguments in a sensible way? [ type_error({event_0_to_3_indexed_values, Con}) || length(Indexed) > 3 ], [ type_error({event_0_to_1_string_values, Con}) || length(NonIndexed) > 1 ], {constr_t, [{indices, Indices} | Ann], Con, Args}. +check_event_arg_type(Env, Ix, Type0) -> + Type = unfold_types_in_type(Env, Type0), + case Ix of + indexed -> [ type_error({indexed_type_must_be_word, Type0, Type}) || not is_word_type(Type) ]; + notindexed -> [ type_error({payload_type_must_be_string, Type0, Type}) || not is_string_type(Type) ] + end. + +%% Not so nice. +is_word_type({id, _, Name}) -> + lists:member(Name, ["int", "address", "hash", "bits", "bool"]); +is_word_type({app_t, _, {id, _, Name}, [_, _]}) -> + lists:member(Name, ["oracle", "oracle_query"]); +is_word_type({con, _, _}) -> true; +is_word_type({qcon, _, _}) -> true; +is_word_type(_) -> false. + +is_string_type({id, _, "string"}) -> true; +is_string_type(_) -> false. + -spec check_constructor_overlap(env(), aeso_syntax:con(), type()) -> ok | no_return(). check_constructor_overlap(Env, Con = {con, _, Name}, NewType) -> case lookup_name(Env, Name) of @@ -1686,10 +1708,26 @@ pp_error({recursive_types_not_implemented, Types}) -> true -> " is" end, io_lib:format("The following type~s recursive, which is not yet supported:\n~s", [S, [io_lib:format(" - ~s (at ~s)\n", [pp(T), pp_loc(T)]) || T <- Types]]); +pp_error({event_must_be_variant_type, Where}) -> + io_lib:format("The event type must be a variant type (at ~s)\n", [pp_loc(Where)]); +pp_error({indexed_type_must_be_word, Type, Type}) -> + io_lib:format("The indexed type ~s (at ~s) is not a word type\n", + [pp_type("", Type), pp_loc(Type)]); +pp_error({indexed_type_must_be_word, Type, Type1}) -> + io_lib:format("The indexed type ~s (at ~s) equals ~s which is not a word type\n", + [pp_type("", Type), pp_loc(Type), pp_type("", Type1)]); +pp_error({payload_type_must_be_string, Type, Type}) -> + io_lib:format("The payload type ~s (at ~s) should be string\n", + [pp_type("", Type), pp_loc(Type)]); +pp_error({payload_type_must_be_string, Type, Type1}) -> + io_lib:format("The payload type ~s (at ~s) equals ~s but it should be string\n", + [pp_type("", Type), pp_loc(Type), pp_type("", Type1)]); pp_error({event_0_to_3_indexed_values, Constr}) -> - io_lib:format("The event constructor ~s has too many indexed values (max 3)\n", [Constr]); + io_lib:format("The event constructor ~s (at ~s) has too many indexed values (max 3)\n", + [name(Constr), pp_loc(Constr)]); pp_error({event_0_to_1_string_values, Constr}) -> - io_lib:format("The event constructor ~s has too many string values (max 1)\n", [Constr]); + io_lib:format("The event constructor ~s (at ~s) has too many string values (max 1)\n", + [name(Constr), pp_loc(Constr)]); pp_error({repeated_constructor, Cs}) -> io_lib:format("Variant types must have distinct constructor names\n~s", [[ io_lib:format("~s (at ~s)\n", [pp_typed(" - ", C, T), pp_loc(C)]) || {C, T} <- Cs ]]); diff --git a/test/aeso_compiler_tests.erl b/test/aeso_compiler_tests.erl index 434513f..0880017 100644 --- a/test/aeso_compiler_tests.erl +++ b/test/aeso_compiler_tests.erl @@ -10,15 +10,10 @@ -include_lib("eunit/include/eunit.hrl"). -%% simple_compile_test_() -> ok. %% Very simply test compile the given contracts. Only basic checks %% are made on the output, just that it is a binary which indicates %% that the compilation worked. - simple_compile_test_() -> - {setup, - fun () -> ok end, %Setup - fun (_) -> ok end, %Cleanup [ {"Testing the " ++ ContractName ++ " contract", fun() -> #{byte_code := ByteCode, @@ -31,8 +26,7 @@ simple_compile_test_() -> <<"Type errors\n",ErrorString/binary>> = compile(ContractName), check_errors(lists:sort(ExpectedErrors), ErrorString) end} || - {ContractName, ExpectedErrors} <- failing_contracts() ] - }. + {ContractName, ExpectedErrors} <- failing_contracts() ]. check_errors(Expect, ErrorString) -> %% This removes the final single \n as well. @@ -168,4 +162,12 @@ failing_contracts() -> <<"The fields y, z are missing when constructing an element of type r('a) (at line 6, column 40)">>]} , {"namespace_clash", [<<"The contract Call (at line 4, column 10) has the same name as a namespace at (builtin location)">>]} + , {"bad_events", + [<<"The payload type int (at line 10, column 30) should be string">>, + <<"The payload type alias_address (at line 12, column 30) equals address but it should be string">>, + <<"The indexed type string (at line 9, column 25) is not a word type">>, + <<"The indexed type alias_string (at line 11, column 25) equals string which is not a word type">>]} + , {"bad_events2", + [<<"The event constructor BadEvent1 (at line 9, column 7) has too many string values (max 1)">>, + <<"The event constructor BadEvent2 (at line 10, column 7) has too many indexed values (max 3)">>]} ]. diff --git a/test/contract_tests.erl b/test/contract_tests.erl deleted file mode 100644 index 5f1762f..0000000 --- a/test/contract_tests.erl +++ /dev/null @@ -1,28 +0,0 @@ --module(contract_tests). - --include_lib("eunit/include/eunit.hrl"). - -make_cmd() -> "make -C " ++ aeso_test_utils:contract_path(). - -contracts_test_() -> - {setup, - fun() -> os:cmd(make_cmd()) end, - fun(_) -> os:cmd(make_cmd() ++ " clean") end, - [ {"Testing the " ++ Contract ++ " contract", - fun() -> - ?assertCmdOutput(Expected, filename:join(aeso_test_utils:contract_path(), Contract ++ "_test")) - end} || {Contract, Expected} <- contracts() ]}. - -contracts() -> - []. - %% [{"voting", - %% "Delegate before vote\n" - %% "Cake: 1\n" - %% "Beer: 2\n" - %% "Winner: Beer\n" - %% "Delegate after vote\n" - %% "Cake: 1\n" - %% "Beer: 2\n" - %% "Winner: Beer\n" - %% }]. - diff --git a/test/contracts/bad_events.aes b/test/contracts/bad_events.aes new file mode 100644 index 0000000..5f0d17c --- /dev/null +++ b/test/contracts/bad_events.aes @@ -0,0 +1,25 @@ +contract Events = + type alias_int = int + type alias_address = address + type alias_string = string + + datatype event = + Event1(indexed alias_int, indexed int, string) + | Event2(alias_string, indexed alias_address) + | BadEvent1(indexed string, string) + | BadEvent2(indexed int, int) + | BadEvent3(indexed alias_string, string) + | BadEvent4(indexed int, alias_address) + + function f1(x : int, y : string) = + Chain.event(Event1(x, x+1, y)) + + function f2(s : string) = + Chain.event(Event2(s, Call.caller)) + + function f3(x : int) = + Chain.event(Event1(x, x + 2, Int.to_str(x + 7))) + + function i2s(i : int) = Int.to_str(i) + function a2s(a : address) = Address.to_str(a) + diff --git a/test/contracts/bad_events2.aes b/test/contracts/bad_events2.aes new file mode 100644 index 0000000..02842e3 --- /dev/null +++ b/test/contracts/bad_events2.aes @@ -0,0 +1,23 @@ +contract Events = + type alias_int = int + type alias_address = address + type alias_string = string + + datatype event = + Event1(indexed alias_int, indexed int, string) + | Event2(alias_string, indexed alias_address) + | BadEvent1(string, string) + | BadEvent2(indexed int, indexed int, indexed int, indexed address) + + function f1(x : int, y : string) = + Chain.event(Event1(x, x+1, y)) + + function f2(s : string) = + Chain.event(Event2(s, Call.caller)) + + function f3(x : int) = + Chain.event(Event1(x, x + 2, Int.to_str(x + 7))) + + function i2s(i : int) = Int.to_str(i) + function a2s(a : address) = Address.to_str(a) + diff --git a/test/contracts/simple_storage.aes b/test/contracts/simple_storage.aes index 3b1a9e9..2c45a53 100644 --- a/test/contracts/simple_storage.aes +++ b/test/contracts/simple_storage.aes @@ -18,7 +18,6 @@ contract SimpleStorage = - type event = int record state = { data : int } function init(value : int) : state = { data = value }