From 3b178f98d88c3d83f1fad8b9854f1d97c5062120 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Mon, 7 Nov 2016 10:10:15 -0500 Subject: [PATCH 1/2] Allow variable refs in interpolated strings Adding inteprolated string support accidentally broke the case where a quoted string contained a valid variable reference. This commit fixes the bug and adds tests to prevent regressions in the future. --- lib/piper/command/parser.ex | 3 ++- src/piper_cmd2_interp_lexer.xrl | 3 +++ src/piper_cmd2_interp_parser.yrl | 35 +++++++++++++++++++++++- test/command/parser/parser_test.exs | 42 +++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 2 deletions(-) diff --git a/lib/piper/command/parser.ex b/lib/piper/command/parser.ex index fdee3f9..389c5ab 100644 --- a/lib/piper/command/parser.ex +++ b/lib/piper/command/parser.ex @@ -92,7 +92,8 @@ defmodule Piper.Command.Parser do {:error, reason} end catch - error -> SemanticError.format_error(error) + error -> + SemanticError.format_error(error) after Process.delete(:piper_cp_context) ParseContext.stop(context) diff --git a/src/piper_cmd2_interp_lexer.xrl b/src/piper_cmd2_interp_lexer.xrl index 741f9d6..16630b0 100644 --- a/src/piper_cmd2_interp_lexer.xrl +++ b/src/piper_cmd2_interp_lexer.xrl @@ -2,8 +2,11 @@ Definitions. INTERP_VALUE = \${([^\${}]|\\\$|\\{|\\})+} TEXT = (\\\^.|\\.|[^\$])+ +VAR_REF = \$([a-zA-Z0-9_\[\]\.]+) + Rules. +{VAR_REF} : {token, {text, ?advance_count(TokenChars), TokenChars}}. {INTERP_VALUE} : {token, {interp_value, ?advance_count(TokenChars), expr_text(TokenChars)}}. {TEXT} : {token, {text, ?advance_count(TokenChars), TokenChars}}. diff --git a/src/piper_cmd2_interp_parser.yrl b/src/piper_cmd2_interp_parser.yrl index 930f0ad..0eeea7f 100644 --- a/src/piper_cmd2_interp_parser.yrl +++ b/src/piper_cmd2_interp_parser.yrl @@ -9,7 +9,7 @@ interpexpr parts. Rootsymbol interpexpr. interpexpr -> - parts : ?new_ast(interp_string, ['$1']). + parts : new_interpexpr('$1'). parts -> text : [?new_ast(string, [text_to_string('$1')])]. @@ -26,3 +26,36 @@ Erlang code. text_to_string({text, Position, Value}) -> {string, Position, Value}. + +new_interpexpr(Values) -> + case are_all_strings(Values) of + true -> + concatenate_strings(Values); + false -> + ?new_ast(interp_string, [Values]) + end. + +are_all_strings([]) -> true; +are_all_strings([Str|T]) -> + case is_string(Str) of + true -> + are_all_strings(T); + false -> + false + end. + +concatenate_strings([Str]) -> Str; +concatenate_strings([H|_]=Strings) -> + Values = [maps:get('value', Str) || Str <- Strings], + Updated = 'Elixir.Enum':join(Values), + maps:put('value', Updated, H). + +is_string(Str) when is_map(Str) -> + case maps:get('__struct__', Str) of + 'Elixir.Piper.Command.Ast.String' -> + true; + _ -> + false + end; +is_string(_) -> false. + diff --git a/test/command/parser/parser_test.exs b/test/command/parser/parser_test.exs index 7e9d4f0..b793ea8 100644 --- a/test/command/parser/parser_test.exs +++ b/test/command/parser/parser_test.exs @@ -405,4 +405,46 @@ defmodule Parser.ParserTest do assert "#{ast}" == "cfn:stack-purge-delete --region=us-west-1" end + test "quoted strings containing var refs are parsed" do + {:ok, ast} = Parser.scan_and_parse("alias create \"echo $body[0]\"") + stage = ast.stages.left + assert stage.name.entity.value == "alias" + assert stage.name.bundle == nil + [first_arg, second_arg] = stage.args + assert first_arg.__struct__ == Piper.Command.Ast.String + assert "#{first_arg}" == "create" + assert second_arg.__struct__ == Piper.Command.Ast.String + assert "#{second_arg}" == "\"echo $body[0]\"" + end + + test "interpolated variable is parsed" do + {:ok, ast} = Parser.scan_and_parse("echo \"echo ${body[0]}\"") + stage = ast.stages.left + assert stage.name.entity.value == "echo" + [arg] = stage.args + assert arg.__struct__ == Piper.Command.Ast.InterpolatedString + [first_expr, second_expr] = arg.exprs + assert first_expr.__struct__ == Piper.Command.Ast.String + assert "#{first_expr}" == "echo " + assert second_expr.__struct__ == Piper.Command.Ast.Variable + assert Enum.count(second_expr.ops) == 1 + assert "#{second_expr}" == "$body[0]" + assert "#{ast}" == "echo \"echo ${body[0]}\"" + end + + test "interpolated variable w/name deref is parsed" do + {:ok, ast} = Parser.scan_and_parse("echo \"echo The current region is ${region.name}\"") + stage = ast.stages.left + assert stage.name.entity.value == "echo" + [arg] = stage.args + assert arg.__struct__ == Piper.Command.Ast.InterpolatedString + [first_expr, second_expr] = arg.exprs + assert first_expr.__struct__ == Piper.Command.Ast.String + assert "#{first_expr}" == "echo The current region is " + assert second_expr.__struct__ == Piper.Command.Ast.Variable + assert Enum.count(second_expr.ops) == 1 + assert "#{second_expr}" == "$region.name" + assert "#{ast}" == "echo \"echo The current region is ${region.name}\"" + end + end From 4991cdf02e0283a85d3df4ea9209ebaf7cc3470a Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Mon, 7 Nov 2016 10:16:19 -0500 Subject: [PATCH 2/2] Replace Erlang string handling functions w/Elixir equivs --- src/piper_cmd2_parser.yrl | 11 ++--------- src/piper_cmd2_parser_util.erl | 13 ++++++++++++- src/piper_cmd2_var_parser.yrl | 8 ++++---- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/piper_cmd2_parser.yrl b/src/piper_cmd2_parser.yrl index d603cc4..1ad0cd5 100644 --- a/src/piper_cmd2_parser.yrl +++ b/src/piper_cmd2_parser.yrl @@ -66,7 +66,7 @@ Erlang code. -export([parse_pipeline/1]). parse_pipeline(Text) when is_binary(Text) -> - parse_pipeline(elixir_string_to_list(Text)); + parse_pipeline(?parser_util:elixir_string_to_list(Text)); parse_pipeline(Text) -> case piper_cmd2_lexer:scan(Text) of {ok, Tokens, _} -> @@ -84,7 +84,7 @@ parse_pipeline(Text) -> end. format_reason({error, {_, _, Reason}}) when is_list(Reason) -> - {error, list_to_elixir_string(Reason)}; + {error, ?parser_util:list_to_elixir_string(Reason)}; format_reason({error, {_, _, Reason}}) -> format_reason(Reason); format_reason(Reason) when is_map(Reason) -> @@ -141,10 +141,3 @@ ensure_quote_type_set(Value, QuoteType) -> maps:put(quote_type, QuoteType, Value) end. -elixir_string_to_list(Text) when is_binary(Text) -> - 'Elixir.String':to_charlist(Text). - -list_to_elixir_string(Text) when is_list(Text) -> - %% Ensure we're dealing with a flat list first - Text1 = lists:flatten(Text), - 'Elixir.String.Chars':to_string(Text1). diff --git a/src/piper_cmd2_parser_util.erl b/src/piper_cmd2_parser_util.erl index 2215a0b..c7213b9 100644 --- a/src/piper_cmd2_parser_util.erl +++ b/src/piper_cmd2_parser_util.erl @@ -2,7 +2,9 @@ -export([parse_token/3, parse_token/4, - new_ast/2]). + new_ast/2, + elixir_string_to_list/1, + list_to_elixir_string/1]). parse_token(Token, Name, Handler) -> parse_token(Token, Name, Name, Handler). @@ -29,6 +31,15 @@ parse_token({_, {Line, _} = Position, Value}, Lexer, Parser, Handler) -> 'Elixir.Piper.Command.ParseContext':set_position(Context, Old) end. +elixir_string_to_list(Text) when is_binary(Text) -> + 'Elixir.String':to_charlist(Text). + +list_to_elixir_string(Text) when is_list(Text) -> + %% Ensure we're dealing with a flat list first + Text1 = lists:flatten(Text), + 'Elixir.String.Chars':to_string(Text1). + + new_ast(Name, Args) -> apply(ast_node_for_name(Name), new, Args). diff --git a/src/piper_cmd2_var_parser.yrl b/src/piper_cmd2_var_parser.yrl index 14e2a74..204c69e 100644 --- a/src/piper_cmd2_var_parser.yrl +++ b/src/piper_cmd2_var_parser.yrl @@ -46,15 +46,15 @@ index_value({integer, _, Value}) -> list_to_integer(Value). key_value({text, _, [$'|_]=Value}) -> - re:replace(Value, "^'|'$", "", [{return, binary}, global]); + re:replace(Value, "^'|'$", "", [{return, binary}, global, unicode]); key_value({text, _, [$"|_]=Value}) -> - re:replace(Value, "^\"|\"$", "", [{return, binary}, global]); + re:replace(Value, "^\"|\"$", "", [{return, binary}, global, unicode]); key_value({text, _, Value}) -> - list_to_binary(Value). + ?parser_util:list_to_elixir_string(Value). make_variable(Var) -> make_variable(Var, []). make_variable({variable, Position, Name}, Ops) -> - Var1 = {variable, Position, re:replace(Name, "^\\$", "", [{return, list}, global])}, + Var1 = {variable, Position, re:replace(Name, "^\\$", "", [{return, list}, global, unicode])}, ?new_ast(variable, [Var1, Ops]).