From ed11d4417c96582ce0298838c443273a84ffb6a7 Mon Sep 17 00:00:00 2001 From: Angelika Tyborska Date: Mon, 28 Mar 2022 08:42:50 +0200 Subject: [PATCH] Extend local calls to match when referencing the current module by name or __MODULE__ (#270) * Add failing tests * Allow local calls by referencing the module in assert_call * Add extra Lasagna test * Add more tests --- docs/recipes.md | 2 + .../exercise_test/assert_call/compiler.ex | 45 +++-- .../assert_call/indirect_call_test.exs | 20 +- .../exercise_test/assert_call_test.exs | 172 ++++++++++++++++++ .../exercise_test/assert_no_call_test.exs | 52 ++++-- .../test_suite/lasagna_test.exs | 55 ++++-- 6 files changed, 307 insertions(+), 39 deletions(-) diff --git a/docs/recipes.md b/docs/recipes.md index c03780cc..bf336fe0 100644 --- a/docs/recipes.md +++ b/docs/recipes.md @@ -8,6 +8,8 @@ assert_call "description" do end ``` +This will also find local calls that reference the module by name or by `__MODULE__`. + ## Find function call by module name and function name (any arity, any argument names) ```elixir diff --git a/lib/elixir_analyzer/exercise_test/assert_call/compiler.ex b/lib/elixir_analyzer/exercise_test/assert_call/compiler.ex index f70f8331..bde5d6cd 100644 --- a/lib/elixir_analyzer/exercise_test/assert_call/compiler.ex +++ b/lib/elixir_analyzer/exercise_test/assert_call/compiler.ex @@ -128,7 +128,7 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.Compiler do modules = Map.merge(modules_in_scope, in_function_modules) match_called_fn? = - matching_function_call?(node, called_fn, modules) and + matching_function_call?(node, called_fn, modules, module) and not in_function?({module, name}, called_fn) match_calling_fn? = in_function?({module, name}, calling_fn) or is_nil(calling_fn) @@ -151,30 +151,52 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.Compiler do @spec matching_function_call?( Macro.t(), AssertCall.function_signature(), - %{[atom] => [atom] | keyword()} + %{[atom] => [atom] | keyword()}, + module() ) :: boolean() # For erlang libraries: :math._ or :math.pow def matching_function_call?( {{:., _, [module_path, name]}, _, _args}, {module_path, search_name}, - _modules + _modules, + _in_module ) when search_name in [:_, name] do true end # No module path in search - def matching_function_call?({_, _, args} = function, {nil, name}, _modules) + def matching_function_call?({_, _, args} = function, {nil, search_name}, modules, in_module) when is_list(args) do case function do # function call without parentheses in a pipe - {:|>, _, [_arg, {^name, _, atom}]} when is_atom(atom) -> true + {:|>, _, [_arg, {^search_name, _, atom}]} when is_atom(atom) -> + true + # function call with captured notation - {:/, _, [{^name, _, atom}, arity]} when is_atom(atom) and is_integer(arity) -> true + {:/, _, [{^search_name, _, atom}, arity]} when is_atom(atom) and is_integer(arity) -> + true + # with parentheses - {^name, _, _args} -> true - _ -> false + {^search_name, _, _args} -> + true + + # local calls that unnecessarily reference the module by name + {{:., _, [{:__aliases__, _, _}, ^search_name]}, _, _args} -> + matching_function_call?(function, {in_module, search_name}, modules, in_module) + + # local calls that unnecessarily reference the module via __MODULE__ + {{:., meta1, [{:__MODULE__, _, _}, name]}, meta2, args} -> + matching_function_call?( + {{:., meta1, [{:__aliases__, [], in_module}, name]}, meta2, args}, + {in_module, search_name}, + modules, + in_module + ) + + _ -> + false end end @@ -182,7 +204,8 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.Compiler do def matching_function_call?( {{:., _, [{:__aliases__, _, [head | tail] = ast_path}, name]}, _, _args}, {module_path, search_name}, - modules + modules, + _in_module ) when search_name in [:_, name] do # Searching for A.B.C.function() @@ -198,7 +221,7 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.Compiler do end # No module path in AST - def matching_function_call?({name, _, args}, {module_path, search_name}, modules) + def matching_function_call?({name, _, args}, {module_path, search_name}, modules, _in_module) when is_list(args) and search_name in [:_, name] do case modules[List.wrap(module_path)] do nil -> false @@ -206,7 +229,7 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.Compiler do end end - def matching_function_call?(_, _, _), do: false + def matching_function_call?(_, _, _, _), do: false @doc """ node is a module definition diff --git a/test/elixir_analyzer/exercise_test/assert_call/indirect_call_test.exs b/test/elixir_analyzer/exercise_test/assert_call/indirect_call_test.exs index d15faf34..60c36064 100644 --- a/test/elixir_analyzer/exercise_test/assert_call/indirect_call_test.exs +++ b/test/elixir_analyzer/exercise_test/assert_call/indirect_call_test.exs @@ -66,6 +66,24 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.IndirectCallTest do final_function(:math.pi()) end end, + # via two helpers unnecessarily referencing the module in local calls + defmodule AssertCallVerification do + def main_function() do + AssertCallVerification.helper("") + |> do_something() + end + + def helper(path) do + __MODULE__.helper_2(path) + end + + def helper_2(path) do + Elixir.Mix.Utils.read_path(path) + + :math.pi() + |> AssertCallVerification.final_function() + end + end, # Full path for the helper function defmodule AssertCallVerification do def main_function() do @@ -200,7 +218,7 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCall.IndirectCallTest do end, defmodule AssertCallVerification do # Internal modules don't fool assert_call - defmodule UnrelateInternaldModule do + defmodule UnrelatedInternalModule do def main_function() do helper("") |> do_something() diff --git a/test/elixir_analyzer/exercise_test/assert_call_test.exs b/test/elixir_analyzer/exercise_test/assert_call_test.exs index 1b26f0fa..38134be7 100644 --- a/test/elixir_analyzer/exercise_test/assert_call_test.exs +++ b/test/elixir_analyzer/exercise_test/assert_call_test.exs @@ -88,6 +88,178 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertCallTest do end end + test_exercise_analysis "finds local calls if they use the module name to reference the function", + comments: [] do + [ + defmodule AssertCallVerification do + def function() do + x = List.first([1, 2, 3]) + result = AssertCallVerification.helper() + IO.puts(result) + + AssertCallVerification.private_helper() |> IO.puts() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end, + # call helper with capture notation + defmodule AssertCallVerification do + def function() do + x = List.first([1, 2, 3]) + result = Enum.map(result, &AssertCallVerification.helper/0) + IO.puts(result) + + private_helper() |> IO.puts() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end, + # indirect call + defmodule AssertCallVerification do + def function() do + x = List.first([1, 2, 3]) + result = AssertCallVerification.extra_helper() + IO.puts(result) + + AssertCallVerification.private_helper() |> IO.puts() + end + + def extra_helper() do + AssertCallVerification.helper() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end + ] + end + + test_exercise_analysis "finds local calls if they use __MODULE__ to reference the function", + comments: [] do + [ + defmodule AssertCallVerification do + def function() do + x = List.first([1, 2, 3]) + result = __MODULE__.helper() + IO.puts(result) + + __MODULE__.private_helper() |> IO.puts() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end, + # call helper with capture notation + defmodule AssertCallVerification do + def function() do + x = List.first([1, 2, 3]) + result = Enum.map(result, &__MODULE__.helper/0) + IO.puts(result) + + private_helper() |> IO.puts() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end, + # indirect call + defmodule AssertCallVerification do + def function() do + x = List.first([1, 2, 3]) + result = __MODULE__.extra_helper() + IO.puts(result) + + __MODULE__.private_helper() |> IO.puts() + end + + def extra_helper() do + __MODULE__.helper() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end + ] + end + + test_exercise_analysis "doesn't find local calls if they're same-named functions from a different module", + comments: [ + "didn't find a local call to helper/0", + "didn't find a local call to helper/0 within function/0", + "didn't find a local call to private_helper/0", + "didn't find a local call to private_helper/0 within function/0" + ] do + [ + defmodule AssertCallVerification do + def function() do + x = List.first([1, 2, 3]) + result = OtherModule.helper() + IO.puts(result) + + OtherModule.private_helper() |> IO.puts() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end, + # call helper with capture notation + defmodule AssertCallVerification do + def function() do + x = List.first([1, 2, 3]) + result = Enum.map(result, &OtherModule.helper/0) + IO.puts(result) + + OtherModule.private_helper() |> IO.puts() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end + ] + end + test_exercise_analysis "missing call to IO.puts/1 in solution", comments: [ "didn't find a call to IO.puts/1 anywhere in solution", diff --git a/test/elixir_analyzer/exercise_test/assert_no_call_test.exs b/test/elixir_analyzer/exercise_test/assert_no_call_test.exs index 2640095d..eb1b1523 100644 --- a/test/elixir_analyzer/exercise_test/assert_no_call_test.exs +++ b/test/elixir_analyzer/exercise_test/assert_no_call_test.exs @@ -22,20 +22,50 @@ defmodule ElixirAnalyzer.ExerciseTest.AssertNoCallTest do test_exercise_analysis "found a local call to helper function", comments: ["found a local call to helper/0"] do - defmodule AssertNoCallVerification do - def function() do - helper() |> IO.puts() - private_helper() - end + [ + defmodule AssertNoCallVerification do + def function() do + helper() |> IO.puts() + private_helper() + end - def helper do - :helped - end + def helper do + :helped + end - defp private_helper do - :privately_helped + defp private_helper do + :privately_helped + end + end, + defmodule AssertNoCallVerification do + def function() do + helper() |> IO.puts() + AssertNoCallVerification.private_helper() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end + end, + defmodule AssertNoCallVerification do + def function() do + helper() |> IO.puts() + __MODULE__.private_helper() + end + + def helper do + :helped + end + + defp private_helper do + :privately_helped + end end - end + ] end test_exercise_analysis "found a local call to from specific function", diff --git a/test/elixir_analyzer/test_suite/lasagna_test.exs b/test/elixir_analyzer/test_suite/lasagna_test.exs index 42777450..b5338b2c 100644 --- a/test/elixir_analyzer/test_suite/lasagna_test.exs +++ b/test/elixir_analyzer/test_suite/lasagna_test.exs @@ -31,27 +31,50 @@ defmodule ElixirAnalyzer.TestSuite.LasagnaTest do test_exercise_analysis "other reasonable solution", comments: [] do - defmodule Lasagna do - def expected_minutes_in_oven() do - 40 - end + [ + defmodule Lasagna do + def expected_minutes_in_oven() do + 40 + end - def remaining_minutes_in_oven(actual) do - expected_minutes_in_oven() - actual - end + def remaining_minutes_in_oven(actual) do + expected_minutes_in_oven() - actual + end - def preparation_time_in_minutes(layers) do - 2 * layers - end + def preparation_time_in_minutes(layers) do + 2 * layers + end - def total_time_in_minutes(layers, actual) do - actual + preparation_time_in_minutes(layers) - end + def total_time_in_minutes(layers, actual) do + actual + preparation_time_in_minutes(layers) + end - def alarm() do - "Ding!" + def alarm() do + "Ding!" + end + end, + defmodule Lasagna do + def expected_minutes_in_oven() do + 40 + end + + def remaining_minutes_in_oven(actual) do + Lasagna.expected_minutes_in_oven() - actual + end + + def preparation_time_in_minutes(layers) do + 2 * layers + end + + def total_time_in_minutes(layers, actual) do + actual + __MODULE__.preparation_time_in_minutes(layers) + end + + def alarm() do + "Ding!" + end end - end + ] end describe "function reuse" do