diff --git a/README.md b/README.md index 3f8420e..d6a1701 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Just add `:mimic` to your list of dependencies in `mix.exs`: ```elixir def deps do [ - {:mimic, "~> 1.7", only: :test} + {:mimic, "~> 1.10", only: :test} ] end ``` @@ -235,6 +235,27 @@ test "calls original function even if it has been is stubbed" do end ``` +## Experimental type checking for copied modules + +One can pass `type_check: true` when a module is copied to also get the function expected/stubbed to +validate the arguments and return value using [Ham](https://github.com/edgurgel/ham) which is essentially +what [Hammox](https://github.com/msz/hammox) improved on Mox. + +```elixir +Mimic.copy(:cowboy_req, type_check: true) +``` + +If there is any problem with the arguments or return values of the stubbed functions on your tests you might see +an error like this one: + +```elixir + ** (Mimic.TypeCheckError) :cowboy_req.parse_qs/1: 1st argument value %{} does not match 1st parameter's type :cowboy_req.req(). + Could not find a map entry matching required(:method) => binary(). +``` + +This feature is experimental at the moment which means that it might change a little bit how this +is configured and used. Feedback is welcome! + ## Implementation Details & Performance After calling `Mimic.copy(MyModule)`, calls to functions belonging to this module will first go through an ETS table to check which pid sees what (stubs, expects or call original). diff --git a/lib/mimic.ex b/lib/mimic.ex index 49b1790..91453d7 100644 --- a/lib/mimic.ex +++ b/lib/mimic.ex @@ -352,13 +352,18 @@ defmodule Mimic do ## Arguments: * `module` - the name of the module to copy. + * `opts` - Extra options + ## Options: + + * `type_check` - Must be a boolean defaulting to `false`. If `true` the arguments and return value + are validated against the module typespecs or the callback typespecs in case of a behaviour implementation. """ - @spec copy(module()) :: :ok | no_return - def copy(module) do + @spec copy(module(), keyword) :: :ok | no_return + def copy(module, opts \\ []) do with :ok <- ensure_module_not_copied(module), {:module, module} <- Code.ensure_compiled(module), - :ok <- Mimic.Server.mark_to_copy(module) do + :ok <- Mimic.Server.mark_to_copy(module, opts) do ExUnit.after_suite(fn _ -> Mimic.Server.reset(module) end) :ok else diff --git a/lib/mimic/module.ex b/lib/mimic/module.ex index e9fa143..426b898 100644 --- a/lib/mimic/module.ex +++ b/lib/mimic/module.ex @@ -15,8 +15,8 @@ defmodule Mimic.Module do :ok end - @spec replace!(module) :: :ok | {:cover.file(), binary} - def replace!(module) do + @spec replace!(module, keyword) :: :ok | {:cover.file(), binary} + def replace!(module, opts) do backup_module = original(module) result = @@ -34,7 +34,7 @@ defmodule Mimic.Module do rename_module(module, backup_module) Code.compiler_options(ignore_module_conflict: true) - create_mock(module) + create_mock(module, Map.new(opts)) Code.compiler_options(ignore_module_conflict: false) result @@ -111,8 +111,8 @@ defmodule Mimic.Module do defp rename_attribute([h | t], new_name), do: [h | rename_attribute(t, new_name)] - defp create_mock(module) do - mimic_info = module_mimic_info() + defp create_mock(module, opts) do + mimic_info = module_mimic_info(opts) mimic_behaviours = generate_mimic_behaviours(module) mimic_functions = generate_mimic_functions(module) mimic_struct = generate_mimic_struct(module) @@ -132,8 +132,8 @@ defmodule Mimic.Module do end end - defp module_mimic_info do - quote do: def(__mimic_info__, do: :ok) + defp module_mimic_info(opts) do + quote do: def(__mimic_info__, do: {:ok, unquote(Macro.escape(opts))}) end defp generate_mimic_functions(module) do diff --git a/lib/mimic/server.ex b/lib/mimic/server.ex index d7bb1e3..12865a1 100644 --- a/lib/mimic/server.ex +++ b/lib/mimic/server.ex @@ -12,7 +12,8 @@ defmodule Mimic.Server do expectations: %{}, modules_beam: %{}, modules_to_be_copied: MapSet.new(), - reset_tasks: %{} + reset_tasks: %{}, + modules_opts: %{} end defmodule Expectation do @@ -90,9 +91,9 @@ defmodule Mimic.Server do GenServer.call(__MODULE__, {:reset, module}, @long_timeout) end - @spec mark_to_copy(module) :: :ok | {:error, {:module_already_copied, module}} - def mark_to_copy(module) do - GenServer.call(__MODULE__, {:mark_to_copy, module}, @long_timeout) + @spec mark_to_copy(module, keyword) :: :ok | {:error, {:module_already_copied, module}} + def mark_to_copy(module, opts) do + GenServer.call(__MODULE__, {:mark_to_copy, module, opts}, @long_timeout) end @spec marked_to_copy?(module) :: boolean @@ -259,7 +260,8 @@ defmodule Mimic.Server do def handle_call({:stub, module, fn_name, func, arity, owner}, _from, state) do with {:ok, state} <- ensure_module_copied(module, state), - true <- valid_mode?(state, owner) do + true <- valid_mode?(state, owner), + func <- maybe_typecheck_func(module, fn_name, func) do monitor_if_not_verify_on_exit(owner, state.verify_on_exit) :ets.insert_new(__MODULE__, {{owner, module}, owner}) @@ -336,6 +338,7 @@ defmodule Mimic.Server do will_be_mocked_functions |> Enum.reduce(state.stubs, fn {fn_name, arity}, stubs -> func = anonymize_module_function(mocking_module, fn_name, arity) + func = maybe_typecheck_func(mocked_module, fn_name, func) put_in(stubs, [Access.key(owner, %{}), {mocked_module, fn_name, arity}], func) end) @@ -358,7 +361,8 @@ defmodule Mimic.Server do def handle_call({:expect, {module, fn_name, func, arity}, num_calls, owner}, _from, state) do with {:ok, state} <- ensure_module_copied(module, state), - true <- valid_mode?(state, owner) do + true <- valid_mode?(state, owner), + func <- maybe_typecheck_func(module, fn_name, func) do monitor_if_not_verify_on_exit(owner, state.verify_on_exit) :ets.insert_new(__MODULE__, {{owner, module}, owner}) @@ -462,7 +466,7 @@ defmodule Mimic.Server do {:reply, marked_to_copy?(module, state), state} end - def handle_call({:mark_to_copy, module}, _from, state) do + def handle_call({:mark_to_copy, module, opts}, _from, state) do if marked_to_copy?(module, state) do {:reply, {:error, {:module_already_copied, module}}, state} else @@ -470,7 +474,11 @@ defmodule Mimic.Server do # Otherwise just store that the module that will be copied # and ensure_module_copied/2 will copy it when # expect, stub, reject is called - state = %{state | modules_to_be_copied: MapSet.put(state.modules_to_be_copied, module)} + state = %{ + state + | modules_to_be_copied: MapSet.put(state.modules_to_be_copied, module), + modules_opts: Map.put(state.modules_opts, module, opts) + } state = if Cover.enabled_for?(module) do @@ -484,6 +492,16 @@ defmodule Mimic.Server do end end + defp maybe_typecheck_func(module, fn_name, func) do + case module.__mimic_info__() do + {:ok, %{type_check: true}} -> + Mimic.TypeCheck.wrap(module, fn_name, func) + + _ -> + func + end + end + defp marked_to_copy?(module, state) do MapSet.member?(state.modules_to_be_copied, module) end @@ -501,7 +519,7 @@ defmodule Mimic.Server do {:ok, state} MapSet.member?(state.modules_to_be_copied, module) -> - case Mimic.Module.replace!(module) do + case Mimic.Module.replace!(module, state.modules_opts[module]) do {beam_file, coverdata_path} -> modules_beam = Map.put(state.modules_beam, module, {beam_file, coverdata_path}) {:ok, %{state | modules_beam: modules_beam}} diff --git a/lib/mimic/type_check.ex b/lib/mimic/type_check.ex new file mode 100644 index 0000000..b0cb6c1 --- /dev/null +++ b/lib/mimic/type_check.ex @@ -0,0 +1,140 @@ +defmodule Mimic.TypeCheckError do + @moduledoc false + defexception [:mfa, :reasons] + + @doc false + @impl Exception + def exception([mfa, reasons]), do: %__MODULE__{mfa: mfa, reasons: reasons} + + @doc false + @impl Exception + def message(exception) do + {module, name, arity} = exception.mfa + mfa = Exception.format_mfa(module, name, arity) + "#{mfa}: #{Ham.TypeMatchError.message(exception)}" + end +end + +defmodule Mimic.TypeCheck do + @moduledoc false + + # Wrap an anoynomous function with type checking provided by Ham + @doc false + @spec wrap(module, atom, (... -> term)) :: (... -> term) + def wrap(module, fn_name, func) do + arity = :erlang.fun_info(func)[:arity] + + behaviours = + module.module_info(:attributes) + |> Keyword.get_values(:behaviour) + |> List.flatten() + + do_wrap(module, behaviours, fn_name, func, arity) + end + + defp do_wrap(module, behaviours, fn_name, func, 0) do + fn -> + apply_and_check(module, behaviours, fn_name, func, []) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 1) do + fn arg1 -> + apply_and_check(module, behaviours, fn_name, func, [arg1]) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 2) do + fn arg1, arg2 -> + apply_and_check(module, behaviours, fn_name, func, [arg1, arg2]) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 3) do + fn arg1, arg2, arg3 -> + apply_and_check(module, behaviours, fn_name, func, [arg1, arg2, arg3]) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 4) do + fn arg1, arg2, arg3, arg4 -> + apply_and_check(module, behaviours, fn_name, func, [arg1, arg2, arg3, arg4]) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 5) do + fn arg1, arg2, arg3, arg4, arg5 -> + apply_and_check(module, behaviours, fn_name, func, [arg1, arg2, arg3, arg4, arg5]) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 6) do + fn arg1, arg2, arg3, arg4, arg5, arg6 -> + apply_and_check(module, behaviours, fn_name, func, [arg1, arg2, arg3, arg4, arg5, arg6]) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 7) do + fn arg1, arg2, arg3, arg4, arg5, arg6, arg7 -> + apply_and_check(module, behaviours, fn_name, func, [ + arg1, + arg2, + arg3, + arg4, + arg5, + arg6, + arg7 + ]) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 8) do + fn arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8 -> + apply_and_check(module, behaviours, fn_name, func, [ + arg1, + arg2, + arg3, + arg4, + arg5, + arg6, + arg7, + arg8 + ]) + end + end + + defp do_wrap(module, behaviours, fn_name, func, 9) do + fn arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9 -> + apply_and_check(module, behaviours, fn_name, func, [ + arg1, + arg2, + arg3, + arg4, + arg5, + arg6, + arg7, + arg8, + arg9 + ]) + end + end + + defp do_wrap(_module, _behaviours, _fn_name, _func, arity) when arity > 9 do + raise "Too many arguments!" + end + + defp apply_and_check(module, behaviours, fn_name, func, args) do + return_value = Kernel.apply(func, args) + + case Ham.validate(module, fn_name, args, return_value, behaviours: behaviours) do + :ok -> + :ok + + {:error, error} -> + mfa = {module, fn_name, length(args)} + raise Mimic.TypeCheckError, [mfa, error.reasons] + end + + return_value + end +end diff --git a/mix.exs b/mix.exs index 13c6a9d..7783dc0 100644 --- a/mix.exs +++ b/mix.exs @@ -2,7 +2,7 @@ defmodule Mimic.Mixfile do use Mix.Project @source_url "https://github.com/edgurgel/mimic" - @version "1.9.0" + @version "1.10.0" def project do [ @@ -32,6 +32,7 @@ defmodule Mimic.Mixfile do defp deps do [ + {:ham, "~> 0.1"}, {:ex_doc, ">= 0.0.0", only: :dev, runtime: false}, {:credo, "~> 1.0", only: :dev} ] diff --git a/mix.lock b/mix.lock index 2ddb0a2..df8a038 100644 --- a/mix.lock +++ b/mix.lock @@ -5,6 +5,7 @@ "earmark_parser": {:hex, :earmark_parser, "1.4.39", "424642f8335b05bb9eb611aa1564c148a8ee35c9c8a8bba6e129d51a3e3c6769", [:mix], [], "hexpm", "06553a88d1f1846da9ef066b87b57c6f605552cfbe40d20bd8d59cc6bde41944"}, "ex_doc": {:hex, :ex_doc, "0.34.1", "9751a0419bc15bc7580c73fde506b17b07f6402a1e5243be9e0f05a68c723368", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "d441f1a86a235f59088978eff870de2e815e290e44a8bd976fe5d64470a4c9d2"}, "file_system": {:hex, :file_system, "1.0.0", "b689cc7dcee665f774de94b5a832e578bd7963c8e637ef940cd44327db7de2cd", [:mix], [], "hexpm", "6752092d66aec5a10e662aefeed8ddb9531d79db0bc145bb8c40325ca1d8536d"}, + "ham": {:hex, :ham, "0.1.0", "b864a794c16b78e3a2b6b57163821238cb8bc9781ab2cdeadf9e230531235eaa", [:mix], [], "hexpm", "2254c399f706c407921bf3b19c2d1fb76a97eb543b725d054bcdc72200a83827"}, "jason": {:hex, :jason, "1.4.1", "af1504e35f629ddcdd6addb3513c3853991f694921b1b9368b0bd32beb9f1b63", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fbb01ecdfd565b56261302f7e1fcc27c4fb8f32d56eab74db621fc154604a7a1"}, "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"}, "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"}, diff --git a/test/mimic/type_check_test.exs b/test/mimic/type_check_test.exs new file mode 100644 index 0000000..5bc5729 --- /dev/null +++ b/test/mimic/type_check_test.exs @@ -0,0 +1,79 @@ +defmodule Mimic.TypeCheckTest do + use ExUnit.Case, async: true + alias Mimic.TypeCheck + + setup do + # Force mimic to copy the module + Mimic.stub(Typecheck.Calculator, :add, fn _x, _y -> 1 end) + Mimic.stub(Typecheck.Counter, :inc, fn _x -> 1 end) + :ok + end + + describe "wrap/3" do + test "behaviour typespec return value" do + func = fn _x, _y -> :not_a_number end + func = TypeCheck.wrap(Typecheck.Calculator, :add, func) + + assert_raise( + Mimic.TypeCheckError, + ~r/Returned value :not_a_number does not match type number()/, + fn -> func.(1, 2) end + ) + end + + test "function typespec return value" do + func = fn _x -> :not_a_number end + func = TypeCheck.wrap(Typecheck.Counter, :inc, func) + + assert_raise( + Mimic.TypeCheckError, + ~r/Returned value :not_a_number does not match type number()/, + fn -> func.(1) end + ) + end + + test "function typespec overriding behaviour spec return value" do + func = fn _x, _y -> :not_a_number end + func = TypeCheck.wrap(Typecheck.Calculator, :mult, func) + + assert_raise( + Mimic.TypeCheckError, + ~r/Returned value :not_a_number does not match type number()/, + fn -> func.(1, 2) end + ) + end + + test "behaviour typespec argument" do + func = fn _x, _y -> 42 end + func = TypeCheck.wrap(Typecheck.Calculator, :add, func) + + assert_raise( + Mimic.TypeCheckError, + ~r/1st argument value :not_a_number does not match 1st parameter's type number()/, + fn -> func.(:not_a_number, 2) end + ) + end + + test "function typespec argument" do + func = fn _x -> 1 end + func = TypeCheck.wrap(Typecheck.Counter, :inc, func) + + assert_raise( + Mimic.TypeCheckError, + ~r/1st argument value :not_a_number does not match 1st parameter's type number()/, + fn -> func.(:not_a_number) end + ) + end + + test "function typespec overriding behaviour spec argument" do + func = fn _x, _y -> 42 end + func = TypeCheck.wrap(Typecheck.Calculator, :mult, func) + + assert_raise( + Mimic.TypeCheckError, + ~r/1st argument value :not_a_number does not match 1st parameter's type number()/, + fn -> func.(:not_a_number, 2) end + ) + end + end +end diff --git a/test/mimic_type_check_test.exs b/test/mimic_type_check_test.exs new file mode 100644 index 0000000..63d16c3 --- /dev/null +++ b/test/mimic_type_check_test.exs @@ -0,0 +1,90 @@ +defmodule MimicTypeCheckTest do + use ExUnit.Case, async: true + use Mimic + + alias Typecheck.Calculator + + describe "stub/3" do + test "does not raise with correct type" do + stub(Calculator, :add, fn _x, _y -> 42 end) + + assert Calculator.add(2, 7) == 42 + end + + test "raises on wrong argument" do + stub(Calculator, :add, fn _x, _y -> 42 end) + + assert_raise( + Mimic.TypeCheckError, + ~r/1st argument value :not_a_number does not match 1st parameter's type number()./, + fn -> Calculator.add(:not_a_number, 7) end + ) + end + + test "raises on wrong return value" do + stub(Calculator, :add, fn _x, _y -> :not_a_number end) + + assert_raise( + Mimic.TypeCheckError, + ~r/Returned value :not_a_number does not match type number()/, + fn -> Calculator.add(77, 7) end + ) + end + end + + defmodule InverseCalculator do + @moduledoc false + @behaviour AddAdapter + def add(_x, _y), do: :not_a_number + end + + describe "stub_with/2" do + test "raises on wrong argument" do + stub_with(Calculator, InverseCalculator) + + assert_raise( + Mimic.TypeCheckError, + ~r/1st argument value :not_a_number does not match 1st parameter's type number()./, + fn -> Calculator.add(:not_a_number, 7) end + ) + end + + test "raises on wrong return value" do + stub_with(Calculator, InverseCalculator) + + assert_raise( + Mimic.TypeCheckError, + ~r/Returned value :not_a_number does not match type number()/, + fn -> Calculator.add(77, 7) end + ) + end + end + + describe "expect/4" do + test "does not raise with correct type" do + expect(Calculator, :add, fn _x, _y -> 42 end) + + assert Calculator.add(13, 7) == 42 + end + + test "raises on wrong argument" do + expect(Calculator, :add, fn _x, _y -> 42 end) + + assert_raise( + Mimic.TypeCheckError, + ~r/1st argument value :not_a_number does not match 1st parameter's type number()./, + fn -> Calculator.add(:not_a_number, 7) end + ) + end + + test "raises on wrong return value" do + expect(Calculator, :add, fn _x, _y -> :not_a_number end) + + assert_raise( + Mimic.TypeCheckError, + ~r/Returned value :not_a_number does not match type number()/, + fn -> Calculator.add(77, 7) end + ) + end + end +end diff --git a/test/support/test_modules.ex b/test/support/test_modules.ex index 07f2d77..5cb1686 100644 --- a/test/support/test_modules.ex +++ b/test/support/test_modules.ex @@ -62,3 +62,27 @@ defimpl String.Chars, for: Structs do "{#{structs.foo}} - {#{structs.bar}}" end end + +defmodule Typecheck.Counter do + @moduledoc false + + @spec inc(number) :: number + def inc(counter), do: counter + 1 + + @spec dec(number) :: number + def dec(counter), do: counter - 1 + + @spec add(number, number) :: number + def add(counter, x), do: counter + x +end + +defmodule Typecheck.Calculator do + @moduledoc false + @behaviour AddAdapter + @behaviour MultAdapter + + def add(x, y), do: x + y + + @spec mult(integer, integer) :: integer + def mult(x, y), do: x * y +end diff --git a/test/test_helper.exs b/test/test_helper.exs index cb04afc..a98912a 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -6,4 +6,6 @@ Mimic.copy(Calculator) Mimic.copy(Counter) Mimic.copy(Structs) Mimic.copy(StructNoEnforceKeys) +Mimic.copy(Typecheck.Calculator, type_check: true) +Mimic.copy(Typecheck.Counter, type_check: true) ExUnit.start()