From c138636767a8553a5894a25dc08b95a5b708882c Mon Sep 17 00:00:00 2001 From: Daniel Berkompas Date: Mon, 14 Dec 2015 08:59:27 -0800 Subject: [PATCH 1/2] Warn about reserved names As mentioned in #10, you cannot use a TwiML verb name as a parameter in a function definition, like this: ```elixir Enum.each [1, 2] fn(number) -> say "Press #{number}" end ``` This commit adds a nice warning message if a user attempts to do this, telling them that the "number" verb is reserved. --- lib/ex_twiml.ex | 29 +++++++++++++++++-- lib/ex_twiml/reserved_name_error.ex | 33 ++++++++++++++++++++++ test/ex_twiml/reserved_name_error_test.exs | 17 +++++++++++ test/ex_twiml_test.exs | 18 ++++++++++++ 4 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 lib/ex_twiml/reserved_name_error.ex create mode 100644 test/ex_twiml/reserved_name_error_test.exs diff --git a/lib/ex_twiml.ex b/lib/ex_twiml.ex index 38a5e5e..2f8048e 100644 --- a/lib/ex_twiml.ex +++ b/lib/ex_twiml.ex @@ -67,6 +67,8 @@ defmodule ExTwiml do import ExTwiml.Utilities + alias ExTwiml.ReservedNameError + @verbs [ # Nested :gather, :dial, :message, @@ -99,7 +101,7 @@ defmodule ExTwiml do # The buffer's state is a list of XML fragments. New fragments are # inserted by other macros. Finally, all the fragments are joined # together in a string. - {:ok, var!(buffer, Twiml)} = start_buffer([header]) + {:ok, var!(buffer, Twiml)} = start_buffer([header]) # Wrap the whole block in a tag tag :response do @@ -107,7 +109,9 @@ defmodule ExTwiml do # `tag` and `text` macros. This gives the impression that there # is a macro for each verb, when in fact it all expands to only # two macros. - unquote(Macro.postwalk(block, &postwalk/1)) + unquote(block + |> Macro.prewalk(&prewalk(&1, __CALLER__.file)) + |> Macro.postwalk(&postwalk/1)) end xml = render(var!(buffer, Twiml)) # Convert buffer to string @@ -174,6 +178,14 @@ defmodule ExTwiml do # Private API ## + # Check function definitions for reserved variable names + defp prewalk({:fn, _, [{:-> , _, [[vars], _]}]} = ast, file_name) do + assert_no_verbs!(vars, file_name) + ast + end + + defp prewalk(ast, _file_name), do: ast + # {:text, [], ["Hello World"]} defp postwalk({:text, _meta, [string]}) do # Just add the text to the buffer. Nothing else needed. @@ -239,4 +251,17 @@ defmodule ExTwiml do put_buffer var!(buffer, Twiml), create_tag(:self_closed, unquote(verb), unquote(options)) end end + + defp assert_no_verbs!({name, _, _} = var, file_name) + when is_atom(name) and name in @verbs do + raise ReservedNameError, [var, file_name] + end + + defp assert_no_verbs!(vars, file_name) when is_tuple(elem(vars, 0)) do + vars + |> Tuple.to_list + |> Enum.each(&assert_no_verbs!(&1, file_name)) + end + + defp assert_no_verbs!(vars, _file_name), do: vars end diff --git a/lib/ex_twiml/reserved_name_error.ex b/lib/ex_twiml/reserved_name_error.ex new file mode 100644 index 0000000..e555c19 --- /dev/null +++ b/lib/ex_twiml/reserved_name_error.ex @@ -0,0 +1,33 @@ +defmodule ExTwiml.ReservedNameError do + @moduledoc """ + This error is thrown if you try to use TwiML verb name as a variable name + in your `twiml` block. + + ## Example + + This code will raise the error, because `number` is a reserved name. + + twiml do + Enum.each [1, 2], fn(number) -> + # ... + end + end + """ + + defexception [:message] + + @doc false + def exception([{name, context, _}, file_name]) do + file_name = Path.relative_to_cwd(file_name) + name = to_string(name) + + message = ~s""" + "#{name}" is a reserved name in #{file_name}:#{context[:line]}, because it + is used to generate the <#{String.capitalize(name)} /> TwiML verb. + + Please use a different variable name. + """ + + %__MODULE__{message: message} + end +end diff --git a/test/ex_twiml/reserved_name_error_test.exs b/test/ex_twiml/reserved_name_error_test.exs new file mode 100644 index 0000000..27d2d2f --- /dev/null +++ b/test/ex_twiml/reserved_name_error_test.exs @@ -0,0 +1,17 @@ +defmodule ExTwiml.ReservedNameErrorTest do + use ExUnit.Case + + alias ExTwiml.ReservedNameError + + test ".exception returns a nice exception" do + %{message: message} = ReservedNameError.exception([{:number, [line: 1], []}, + "test/test.ex"]) + + assert message == ~s""" + "number" is a reserved name in test/test.ex:1, because it + is used to generate the TwiML verb. + + Please use a different variable name. + """ + end +end diff --git a/test/ex_twiml_test.exs b/test/ex_twiml_test.exs index 2a077df..6662998 100644 --- a/test/ex_twiml_test.exs +++ b/test/ex_twiml_test.exs @@ -1,7 +1,10 @@ defmodule ExTwimlTest do use ExUnit.Case, async: false + import ExTwiml + alias ExTwiml.ReservedNameError + doctest ExTwiml test "can render the verb" do @@ -226,6 +229,21 @@ defmodule ExTwimlTest do assert_twiml markup, "123" end + test ".twiml warns of reserved variable names" do + ast = quote do + twiml do + Enum.each [1, 2], fn(number) -> + say "#{number}" + end + end + end + + assert_raise ReservedNameError, fn -> + # Simulate compiling the macro + Macro.expand(ast, __ENV__) + end + end + defp assert_twiml(lhs, rhs) do assert lhs == "#{rhs}" end From e4a44e10075ad712269c5eb7c5e36929a1acef88 Mon Sep 17 00:00:00 2001 From: Daniel Berkompas Date: Mon, 14 Dec 2015 09:07:13 -0800 Subject: [PATCH 2/2] Resolve dialyzer errors --- .travis.yml | 14 ++++---------- lib/ex_twiml/reserved_name_error.ex | 1 + 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4abb63c..237638a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,19 +1,13 @@ language: elixir elixir: - - 1.0.3 - - 1.0.4 + - 1.1.1 otp_release: - - 17.4 + - 18.0 before_script: - - export PATH=`pwd`/elixir/bin:$PATH - - export PLT_FILENAME=elixir-${TRAVIS_ELIXIR_VERSION}_$TRAVIS_OTP_RELEASE.plt - - export PLT_LOCATION=/home/travis/$PLT_FILENAME - - wget -O $PLT_LOCATION https://raw.github.com/danielberkompas/travis_elixir_plts/master/$PLT_FILENAME - mix local.hex --force - mix deps.get --only test script: - mix test - - dialyzer --no_check_plt --plt $PLT_LOCATION -Wno_match -Wno_return --no_native _build/test/lib/ex_twiml/ebin after_script: -- MIX_ENV=docs mix deps.get -- MIX_ENV=docs mix inch.report + - MIX_ENV=docs mix deps.get + - MIX_ENV=docs mix inch.report diff --git a/lib/ex_twiml/reserved_name_error.ex b/lib/ex_twiml/reserved_name_error.ex index e555c19..089daed 100644 --- a/lib/ex_twiml/reserved_name_error.ex +++ b/lib/ex_twiml/reserved_name_error.ex @@ -17,6 +17,7 @@ defmodule ExTwiml.ReservedNameError do defexception [:message] @doc false + @spec exception(list) :: %__MODULE__{} def exception([{name, context, _}, file_name]) do file_name = Path.relative_to_cwd(file_name) name = to_string(name)