From e6a0dea7028f8ca002d74abc262fa6bc1cddd74b Mon Sep 17 00:00:00 2001 From: Andreas Stenius Date: Wed, 2 Jul 2014 23:11:47 +0200 Subject: [PATCH] Check that extends-templates don't use non-block tags. Ref. #176. --- NEWS.md | 3 +++ README.markdown | 6 ++++++ include/erlydtl_ext.hrl | 3 ++- src/erlydtl_beam_compiler.erl | 37 +++++++++++++++++++++++----------- src/erlydtl_compiler.erl | 17 ++++++++++++++-- src/erlydtl_compiler_utils.erl | 23 ++++++++++++++++++++- test/erlydtl_test_defs.erl | 15 ++++++++++---- test/files/expect/extends4 | 11 ++++++++++ test/files/input/extends4 | 3 +++ 9 files changed, 98 insertions(+), 20 deletions(-) create mode 100644 test/files/expect/extends4 create mode 100644 test/files/input/extends4 diff --git a/NEWS.md b/NEWS.md index 1d0eda6..c40c8a4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,3 +11,6 @@ Standards](http://www.gnu.org/prep/standards/html_node/NEWS-File.html#NEWS-File) * Fix issue with using keywords as attributes (#177). * Fix issue when including multiple templates extending a common base template (#176). + +* New `w` option for toggling compile time warnings. Currently, there is only one, `non_block_tag`, + which is triggered on any non-block data in an extends-template. diff --git a/README.markdown b/README.markdown index 6161b27..b89574f 100644 --- a/README.markdown +++ b/README.markdown @@ -289,6 +289,12 @@ Options is a proplist possibly containing: * `verbose` - Enable verbose printing of compilation progress. Add several for even more verbose (e.g. debug) output. +* `w` - Enable/Disable compile time checks. + + Available checks: + - `non_block_tag` indicated that there is other data than `block` tags in an extends-template + (e.g. a template that begins with the `extends` tag). + * `warnings_as_errors` - Treat warnings as errors. diff --git a/include/erlydtl_ext.hrl b/include/erlydtl_ext.hrl index de659f3..b61ba58 100755 --- a/include/erlydtl_ext.hrl +++ b/include/erlydtl_ext.hrl @@ -35,7 +35,8 @@ warnings = #error_info{}, bin = undefined, lists_0_based = false, - tuples_0_based = false + tuples_0_based = false, + checks = [non_block_tag] }). %% ALL fields of ast_info{} must be lists (see erlydtl_compiler_utils:merge_info/2) diff --git a/src/erlydtl_beam_compiler.erl b/src/erlydtl_beam_compiler.erl index 488d3a5..f26ccce 100644 --- a/src/erlydtl_beam_compiler.erl +++ b/src/erlydtl_beam_compiler.erl @@ -65,7 +65,7 @@ init_treewalker/1, resolve_variable/2, resolve_variable/3, reset_block_dict/2, reset_parse_trail/2, load_library/3, load_library/4, shorten_filename/2, push_auto_escape/2, - pop_auto_escape/1]). + pop_auto_escape/1, token_pos/1, is_stripped_token_empty/1]). -include_lib("merl/include/merl.hrl"). -include("erlydtl_ext.hrl"). @@ -112,6 +112,8 @@ format_error({translation_fun, Fun}) -> io_lib:format("~s:~s/~p", [proplists:get_value(K, Info) || K <- [module, name, arity]]); true -> io_lib:format("~p", [Fun]) end]); +format_error(non_block_tag) -> + "Non-block tag in extends-template."; format_error(Error) -> erlydtl_compiler:format_error(Error). @@ -523,29 +525,40 @@ body_ast([{'extends', {string_literal, _Pos, String}} | ThisParseTree], #treewal _ -> case parse_file(File, Context) of {ok, ParentParseTree, CheckSum} -> - BlockDict = lists:foldl( - fun ({block, {identifier, _, Name}, Contents}, Dict) -> - dict:store(Name, Contents, Dict); - (_, Dict) -> Dict - end, - dict:new(), - ThisParseTree), + {BlockDict, Context1} = lists:foldl( + fun ({block, {identifier, _, Name}, Contents}, {Dict, Ctx}) -> + {dict:store(Name, Contents, Dict), Ctx}; + (Token, {Dict, Ctx}) -> + case proplists:get_bool(non_block_tag, Ctx#dtl_context.checks) of + true -> + case is_stripped_token_empty(Token) of + false -> + {Dict, ?WARN({token_pos(Token), non_block_tag}, Ctx)}; + true -> + {Dict, Ctx} + end; + false -> + {Dict, Ctx} + end + end, + {dict:new(), Context}, + ThisParseTree), {Info, TreeWalker1} = with_dependency( {File, CheckSum}, body_ast( ParentParseTree, TreeWalker#treewalker{ - context=Context#dtl_context{ + context=Context1#dtl_context{ block_dict = dict:merge( fun(_Key, _ParentVal, ChildVal) -> ChildVal end, BlockDict, Context#dtl_context.block_dict), - parse_trail = [File | Context#dtl_context.parse_trail] + parse_trail = [File | Context1#dtl_context.parse_trail] } })), {Info, reset_parse_trail( - Context#dtl_context.parse_trail, + Context1#dtl_context.parse_trail, reset_block_dict( - Context#dtl_context.block_dict, + Context1#dtl_context.block_dict, TreeWalker1))}; {error, Reason} -> empty_ast(?ERR(Reason, TreeWalker)) diff --git a/src/erlydtl_compiler.erl b/src/erlydtl_compiler.erl index e56d43f..46f31a5 100644 --- a/src/erlydtl_compiler.erl +++ b/src/erlydtl_compiler.erl @@ -278,14 +278,27 @@ init_context(ParseTrail, DefDir, Module, Options) -> errors = init_error_info(errors, Ctx#dtl_context.errors, Options), warnings = init_error_info(warnings, Ctx#dtl_context.warnings, Options), lists_0_based = proplists:get_value(lists_0_based, Options, Ctx#dtl_context.lists_0_based), - tuples_0_based = proplists:get_value(tuples_0_based, Options, Ctx#dtl_context.tuples_0_based) + tuples_0_based = proplists:get_value(tuples_0_based, Options, Ctx#dtl_context.tuples_0_based), + checks = proplists:substitute_negations( + [{no_non_block_tag}], + proplists:get_all_values(w, Options)) }, - Context = load_libraries(proplists:get_value(default_libraries, Options, []), Context0), + Context1 = load_libraries(proplists:get_value(default_libraries, Options, []), Context0), + Context = default_checks(Ctx#dtl_context.checks, Context1), case call_extension(Context, init_context, [Context]) of {ok, C} when is_record(C, dtl_context) -> C; undefined -> Context end. +default_checks([], Context) -> Context; +default_checks([C|Cs], #dtl_context{ checks = Checks0 } = Context) -> + Checks = + case proplists:get_value(C, Checks0) of + undefined -> [C|Checks0]; + _ -> Checks0 + end, + default_checks(Cs, Context#dtl_context{ checks = Checks }). + init_error_info(warnings, Ei, Options) -> case proplists:get_bool(warnings_as_errors, Options) of true -> warnings_as_errors; diff --git a/src/erlydtl_compiler_utils.erl b/src/erlydtl_compiler_utils.erl index db069a3..c0a331d 100644 --- a/src/erlydtl_compiler_utils.erl +++ b/src/erlydtl_compiler_utils.erl @@ -59,6 +59,7 @@ full_path/2, get_current_file/1, init_treewalker/1, + is_stripped_token_empty/1, load_library/2, load_library/3, load_library/4, merge_info/2, print/3, print/4, @@ -71,7 +72,8 @@ to_string/2, unescape_string_literal/1, push_auto_escape/2, - pop_auto_escape/1 + pop_auto_escape/1, + token_pos/1 ]). -include("erlydtl_ext.hrl"). @@ -303,6 +305,25 @@ pop_auto_escape(#dtl_context{ auto_escape=[_|AutoEscape] }=Context) Context#dtl_context{ auto_escape=AutoEscape }; pop_auto_escape(Context) -> Context. + +token_pos(Token) when is_tuple(Token) -> + token_pos(tuple_to_list(Token)); +token_pos([T|Ts]) when is_tuple(T) -> + case T of + {R, C}=P when is_integer(R), is_integer(C) -> P; + _ -> token_pos(tuple_to_list(T) ++ Ts) + end; +token_pos([T|Ts]) when is_list(T) -> token_pos(T ++ Ts); +token_pos([_|Ts]) -> token_pos(Ts); +token_pos([]) -> none. + +is_stripped_token_empty({string, _, S}) -> + [] == [C || C <- S, C /= 32, C /= $\r, C /= $\n, C /= $\t]; +is_stripped_token_empty({comment, _}) -> true; +is_stripped_token_empty({comment_tag, _, _}) -> true; +is_stripped_token_empty(_) -> false. + + format_error({load_library, Name, Mod, Reason}) -> io_lib:format("Failed to load library '~p' (~p): ~p", [Name, Mod, Reason]); format_error({load_from, Name, Mod, Tag}) -> diff --git a/test/erlydtl_test_defs.erl b/test/erlydtl_test_defs.erl index 9965e86..3bc9602 100644 --- a/test/erlydtl_test_defs.erl +++ b/test/erlydtl_test_defs.erl @@ -1662,7 +1662,7 @@ all_test_defs() -> "include_template", "include_path", "ssi", "extends_path", "extends_path2", "trans", "extends_for", "extends2", "extends3", "recursive_block", "extend_recursive_block", "missing", - "block_super", "wrapper"] + "block_super", "wrapper", "extends4"] ]}, {"compile_dir", [setup_compile(T) @@ -1769,10 +1769,13 @@ functional_test(F) -> setup_compile(#test{ title=F, compile_opts=Opts }=T) -> CompileOpts = [{doc_root, "../test/files/input"}|Opts], case setup_compile(F) of - {ok, [CV|CO]} -> + {ok, [CV|Other]} -> + CO = proplists:get_value(compile_opts, Other, []), + Ws = proplists:get_value(warnings, Other, []), setup(T#test{ compile_vars = CV, - compile_opts = CO ++ CompileOpts + compile_opts = CO ++ CompileOpts, + warnings = Ws }); {error, Es, Ws} -> T#test{ @@ -1817,12 +1820,16 @@ setup_compile("extends3") -> Include = template_file(input, "imaginary"), Error = {none, erlydtl_beam_compiler, {read_file, Include, enoent}}, {error, [{File, [Error]}], []}; +setup_compile("extends4") -> + File = template_file(input, "extends4"), + Warning = {{1,21}, erlydtl_beam_compiler, non_block_tag}, + {ok, [[]|[{warnings, [{File, [Warning]}]}]]}; setup_compile("missing") -> File = template_file(input, "missing"), Error = {none, erlydtl_compiler, {read_file, File, enoent}}, {error, [{File, [Error]}], []}; setup_compile("custom_tag") -> - {ok, [[]|[{custom_tags_modules, [erlydtl_custom_tags]}]]}; + {ok, [[]|[{compile_opts, [{custom_tags_modules, [erlydtl_custom_tags]}]}]]}; setup_compile("custom_tag1") -> setup_compile("custom_tag"); setup_compile("custom_tag2") -> setup_compile("custom_tag"); setup_compile("custom_tag3") -> setup_compile("custom_tag"); diff --git a/test/files/expect/extends4 b/test/files/expect/extends4 new file mode 100644 index 0000000..71bd895 --- /dev/null +++ b/test/files/expect/extends4 @@ -0,0 +1,11 @@ + + +base template + +base title + +more of base template + +base content + +end of base template diff --git a/test/files/input/extends4 b/test/files/input/extends4 new file mode 100644 index 0000000..85f7cb6 --- /dev/null +++ b/test/files/input/extends4 @@ -0,0 +1,3 @@ +{% extends "base" %} + +bad, only block level tags should be here..