From fd03585b888a3e79ee8c6cfceb7f241bac4cf8f2 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 10:00:32 +0100 Subject: [PATCH 01/36] Module: Add a simple `exists?/1` function which checks for existence --- lib/knigge/module.ex | 6 +++++- test/knigge/module_test.exs | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 test/knigge/module_test.exs diff --git a/lib/knigge/module.ex b/lib/knigge/module.ex index a640c45..a38b7f9 100644 --- a/lib/knigge/module.ex +++ b/lib/knigge/module.ex @@ -13,9 +13,13 @@ defmodule Knigge.Module do def exists?(module, %Options{} = opts) do if opts.check_if_exists? do - Code.ensure_loaded?(module) or Module.open?(module) + exists?(module) else true end end + + def exists?(module) do + Module.open?(module) or Code.ensure_loaded?(module) + end end diff --git a/test/knigge/module_test.exs b/test/knigge/module_test.exs new file mode 100644 index 0000000..f048423 --- /dev/null +++ b/test/knigge/module_test.exs @@ -0,0 +1,15 @@ +defmodule Knigge.ModuleTest do + use ExUnit.Case, async: true + + alias Knigge.Module + + describe ".exists?/1" do + test "returns false for a non-existing module" do + assert Module.exists?(This.Does.Not.Exist) == false + end + + test "returns true for an existing module" do + assert Module.exists?(Knigge) == true + end + end +end From 55b66b9fa063849f4ce5ec050fd04b69d1377da3 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 11:07:33 +0100 Subject: [PATCH 02/36] Module: Rewrite the tests as doctests --- lib/knigge/module.ex | 37 +++++++++++++++++++++++++++++++++++++ test/knigge/module_test.exs | 12 +----------- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/lib/knigge/module.ex b/lib/knigge/module.ex index a38b7f9..55c0223 100644 --- a/lib/knigge/module.ex +++ b/lib/knigge/module.ex @@ -3,6 +3,8 @@ defmodule Knigge.Module do alias Knigge.Options + module = inspect(__MODULE__) + def ensure_exists!(module, opts, env) do unless Knigge.Module.exists?(module, opts) do Knigge.Error.module_not_loaded!(module, env) @@ -11,6 +13,29 @@ defmodule Knigge.Module do module end + @doc """ + Returns true if a module exists, false otherwise. Always returns true if + `check_if_exists?` is set to `false` on the `Knigge.Options` struct. + + ## Examples + + iex> options = %Knigge.Options{check_if_exists?: true} + iex> #{module}.exists?(This.Does.Not.Exist, options) + false + + iex> options = %Knigge.Options{check_if_exists?: false} + iex> #{module}.exists?(This.Does.Not.Exist, options) + true + + iex> options = %Knigge.Options{check_if_exists?: true} + iex> #{module}.exists?(Knigge, options) + true + + iex> options = %Knigge.Options{check_if_exists?: false} + iex> #{module}.exists?(Knigge, options) + true + """ + @spec exists?(module :: module()) :: boolean() def exists?(module, %Options{} = opts) do if opts.check_if_exists? do exists?(module) @@ -19,6 +44,18 @@ defmodule Knigge.Module do end end + @doc """ + Returns true if a module exists, false otherwise. + + ## Examples + + iex> #{module}.exists?(This.Does.Not.Exist) + false + + iex> #{module}.exists?(Knigge) + true + """ + @spec exists?(module :: module()) :: boolean() def exists?(module) do Module.open?(module) or Code.ensure_loaded?(module) end diff --git a/test/knigge/module_test.exs b/test/knigge/module_test.exs index f048423..5461a92 100644 --- a/test/knigge/module_test.exs +++ b/test/knigge/module_test.exs @@ -1,15 +1,5 @@ defmodule Knigge.ModuleTest do use ExUnit.Case, async: true - alias Knigge.Module - - describe ".exists?/1" do - test "returns false for a non-existing module" do - assert Module.exists?(This.Does.Not.Exist) == false - end - - test "returns true for an existing module" do - assert Module.exists?(Knigge) == true - end - end + doctest Knigge.Module end From 0cc8ae2c157e4fb2025566af62b48209f2b07104 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 11:14:22 +0100 Subject: [PATCH 03/36] Module: Add a `fetch_for_app/1` function The function returns all modules for given app which `use Knigge`; see the docs for details. --- lib/knigge.ex | 4 ++++ lib/knigge/module.ex | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/lib/knigge.ex b/lib/knigge.ex index f8eaa7b..978404a 100644 --- a/lib/knigge.ex +++ b/lib/knigge.ex @@ -202,6 +202,10 @@ defmodule Knigge do @__knigge__ {:options, options} + @doc "Acts as a \"flag\" to mark this module as a Knigge module." + @spec __knigge__() :: :ok + def __knigge__, do: :ok + @doc "Access Knigge internal values, such as the implementation being delegated to etc." @spec __knigge__(:behaviour) :: module() @spec __knigge__(:implementation) :: module() diff --git a/lib/knigge/module.ex b/lib/knigge/module.ex index 55c0223..221d97b 100644 --- a/lib/knigge/module.ex +++ b/lib/knigge/module.ex @@ -59,4 +59,34 @@ defmodule Knigge.Module do def exists?(module) do Module.open?(module) or Code.ensure_loaded?(module) end + + @doc """ + Returns all modules which `use Knigge` for the given app. If the app does not + exist an error is returned. To determine if `Knigge` is `use`d we check if the + module exports the `__knigge__/0` function which acts as a "tag". + + `fetch_for_app/1` makes use of `Code.ensure_loaded?/1` to force the module + being loaded. Since this results in a `GenServer.call/3` to the code server + __do not use this__ in a hot code path, as it __will__ result in a slowdown! + + ## Examples + + iex> #{module}.fetch_for_app(:this_does_not_exist) + {:error, :undefined} + + iex> #{module}.fetch_for_app(:knigge) + {:ok, []} + """ + @spec fetch_for_app(app :: atom()) :: {:ok, list(module())} | {:error, :undefined} + def fetch_for_app(app) do + with {:ok, modules} <- :application.get_key(app, :modules) do + {:ok, Enum.filter(modules, &uses_knigge?/1)} + else + :undefined -> {:error, :undefined} + end + end + + defp uses_knigge?(module) do + Code.ensure_loaded?(module) and function_exported?(module, :__knigge__, 0) + end end From 6c9792ac3dfb2bd399947dc4b137e77e74686378 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 11:24:06 +0100 Subject: [PATCH 04/36] Deps: Make credo version constraint more liberal --- mix.exs | 2 +- mix.lock | 42 +++++++++++++++++++++--------------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/mix.exs b/mix.exs index e5a57cc..3d86dac 100644 --- a/mix.exs +++ b/mix.exs @@ -45,7 +45,7 @@ defmodule Knigge.MixProject do defp deps do [ # No Runtime - {:credo, "~> 1.0.0", only: [:dev, :test], runtime: false}, + {:credo, ">= 1.0.0", only: [:dev, :test], runtime: false}, {:dialyxir, "~> 1.0.0-rc.6", only: [:dev], runtime: false}, {:ex_doc, "~> 0.21", only: :dev, runtime: false}, diff --git a/mix.lock b/mix.lock index 17e716a..1acb25e 100644 --- a/mix.lock +++ b/mix.lock @@ -1,23 +1,23 @@ %{ - "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm"}, - "certifi": {:hex, :certifi, "2.5.1", "867ce347f7c7d78563450a18a6a28a8090331e77fa02380b4a21962a65d36ee5", [:rebar3], [{:parse_trans, "~>3.3", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm"}, - "credo": {:hex, :credo, "1.0.5", "fdea745579f8845315fe6a3b43e2f9f8866839cfbc8562bb72778e9fdaa94214", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"}, - "dialyxir": {:hex, :dialyxir, "1.0.0-rc.6", "78e97d9c0ff1b5521dd68041193891aebebce52fc3b93463c0a6806874557d7d", [:mix], [{:erlex, "~> 0.2.1", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm"}, - "earmark": {:hex, :earmark, "1.3.3", "5e8be428fcef362692b6dbd7dc55bdc7023da26d995cb3fb19aa4bd682bfd3f9", [:mix], [], "hexpm"}, - "erlex": {:hex, :erlex, "0.2.4", "23791959df45fe8f01f388c6f7eb733cc361668cbeedd801bf491c55a029917b", [:mix], [], "hexpm"}, - "ex_doc": {:hex, :ex_doc, "0.21.1", "5ac36660846967cd869255f4426467a11672fec3d8db602c429425ce5b613b90", [:mix], [{:earmark, "~> 1.3", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm"}, - "excoveralls": {:hex, :excoveralls, "0.11.1", "dd677fbdd49114fdbdbf445540ec735808250d56b011077798316505064edb2c", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"}, - "hackney": {:hex, :hackney, "1.15.1", "9f8f471c844b8ce395f7b6d8398139e26ddca9ebc171a8b91342ee15a19963f4", [:rebar3], [{:certifi, "2.5.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.4", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm"}, - "idna": {:hex, :idna, "6.0.0", "689c46cbcdf3524c44d5f3dde8001f364cd7608a99556d8fbd8239a5798d4c10", [:rebar3], [{:unicode_util_compat, "0.4.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm"}, - "inch_ex": {:hex, :inch_ex, "2.0.0", "24268a9284a1751f2ceda569cd978e1fa394c977c45c331bb52a405de544f4de", [:mix], [{:bunt, "~> 0.2", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"}, - "jason": {:hex, :jason, "1.1.2", "b03dedea67a99223a2eaf9f1264ce37154564de899fd3d8b9a21b1a6fd64afe7", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm"}, - "makeup": {:hex, :makeup, "1.0.0", "671df94cf5a594b739ce03b0d0316aa64312cee2574b6a44becb83cd90fb05dc", [:mix], [{:nimble_parsec, "~> 0.5.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm"}, - "makeup_elixir": {:hex, :makeup_elixir, "0.14.0", "cf8b7c66ad1cff4c14679698d532f0b5d45a3968ffbcbfd590339cb57742f1ae", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm"}, - "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm"}, - "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm"}, - "mox": {:hex, :mox, "0.5.1", "f86bb36026aac1e6f924a4b6d024b05e9adbed5c63e8daa069bd66fb3292165b", [:mix], [], "hexpm"}, - "nimble_parsec": {:hex, :nimble_parsec, "0.5.0", "90e2eca3d0266e5c53f8fbe0079694740b9c91b6747f2b7e3c5d21966bba8300", [:mix], [], "hexpm"}, - "parse_trans": {:hex, :parse_trans, "3.3.0", "09765507a3c7590a784615cfd421d101aec25098d50b89d7aa1d66646bc571c1", [:rebar3], [], "hexpm"}, - "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.4", "f0eafff810d2041e93f915ef59899c923f4568f4585904d010387ed74988e77b", [:make, :mix, :rebar3], [], "hexpm"}, - "unicode_util_compat": {:hex, :unicode_util_compat, "0.4.1", "d869e4c68901dd9531385bb0c8c40444ebf624e60b6962d95952775cac5e90cd", [:rebar3], [], "hexpm"}, + "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"}, + "certifi": {:hex, :certifi, "2.5.1", "867ce347f7c7d78563450a18a6a28a8090331e77fa02380b4a21962a65d36ee5", [:rebar3], [{:parse_trans, "~>3.3", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm", "805abd97539caf89ec6d4732c91e62ba9da0cda51ac462380bbd28ee697a8c42"}, + "credo": {:hex, :credo, "1.0.5", "fdea745579f8845315fe6a3b43e2f9f8866839cfbc8562bb72778e9fdaa94214", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "16105fac37c5c4b3f6e1f70ba0784511fec4275cd8bb979386e3c739cf4e6455"}, + "dialyxir": {:hex, :dialyxir, "1.0.0-rc.6", "78e97d9c0ff1b5521dd68041193891aebebce52fc3b93463c0a6806874557d7d", [:mix], [{:erlex, "~> 0.2.1", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "49496d63267bc1a4614ffd5f67c45d9fc3ea62701a6797975bc98bc156d2763f"}, + "earmark": {:hex, :earmark, "1.3.3", "5e8be428fcef362692b6dbd7dc55bdc7023da26d995cb3fb19aa4bd682bfd3f9", [:mix], [], "hexpm", "a4f21ad675cd496b4b124b00cd7884add73ef836bbfeaa6a85a818df5690a182"}, + "erlex": {:hex, :erlex, "0.2.4", "23791959df45fe8f01f388c6f7eb733cc361668cbeedd801bf491c55a029917b", [:mix], [], "hexpm", "4a12ebc7cd8f24f2d0fce93d279fa34eb5068e0e885bb841d558c4d83c52c439"}, + "ex_doc": {:hex, :ex_doc, "0.21.1", "5ac36660846967cd869255f4426467a11672fec3d8db602c429425ce5b613b90", [:mix], [{:earmark, "~> 1.3", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "93d2fee94d2f88abf507628378371ea5fab08ed03fa59a6daa3d4469d9159ddd"}, + "excoveralls": {:hex, :excoveralls, "0.11.1", "dd677fbdd49114fdbdbf445540ec735808250d56b011077798316505064edb2c", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "493daf5a2dd92d022a1c29e7edcc30f1bce1ffe10fb3690fac63889346d3af2f"}, + "hackney": {:hex, :hackney, "1.15.1", "9f8f471c844b8ce395f7b6d8398139e26ddca9ebc171a8b91342ee15a19963f4", [:rebar3], [{:certifi, "2.5.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "6.0.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "1.0.1", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "1.1.4", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}], "hexpm", "c2790c9f0f7205f4a362512192dee8179097394400e745e4d20bab7226a8eaad"}, + "idna": {:hex, :idna, "6.0.0", "689c46cbcdf3524c44d5f3dde8001f364cd7608a99556d8fbd8239a5798d4c10", [:rebar3], [{:unicode_util_compat, "0.4.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "4bdd305eb64e18b0273864920695cb18d7a2021f31a11b9c5fbcd9a253f936e2"}, + "inch_ex": {:hex, :inch_ex, "2.0.0", "24268a9284a1751f2ceda569cd978e1fa394c977c45c331bb52a405de544f4de", [:mix], [{:bunt, "~> 0.2", [hex: :bunt, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "96d0ec5ecac8cf63142d02f16b7ab7152cf0f0f1a185a80161b758383c9399a8"}, + "jason": {:hex, :jason, "1.1.2", "b03dedea67a99223a2eaf9f1264ce37154564de899fd3d8b9a21b1a6fd64afe7", [:mix], [{:decimal, "~> 1.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fdf843bca858203ae1de16da2ee206f53416bbda5dc8c9e78f43243de4bc3afe"}, + "makeup": {:hex, :makeup, "1.0.0", "671df94cf5a594b739ce03b0d0316aa64312cee2574b6a44becb83cd90fb05dc", [:mix], [{:nimble_parsec, "~> 0.5.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "a10c6eb62cca416019663129699769f0c2ccf39428b3bb3c0cb38c718a0c186d"}, + "makeup_elixir": {:hex, :makeup_elixir, "0.14.0", "cf8b7c66ad1cff4c14679698d532f0b5d45a3968ffbcbfd590339cb57742f1ae", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "d4b316c7222a85bbaa2fd7c6e90e37e953257ad196dc229505137c5e505e9eff"}, + "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"}, + "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"}, + "mox": {:hex, :mox, "0.5.1", "f86bb36026aac1e6f924a4b6d024b05e9adbed5c63e8daa069bd66fb3292165b", [:mix], [], "hexpm", "052346cf322311c49a0f22789f3698eea030eec09b8c47367f0686ef2634ae14"}, + "nimble_parsec": {:hex, :nimble_parsec, "0.5.0", "90e2eca3d0266e5c53f8fbe0079694740b9c91b6747f2b7e3c5d21966bba8300", [:mix], [], "hexpm", "5c040b8469c1ff1b10093d3186e2e10dbe483cd73d79ec017993fb3985b8a9b3"}, + "parse_trans": {:hex, :parse_trans, "3.3.0", "09765507a3c7590a784615cfd421d101aec25098d50b89d7aa1d66646bc571c1", [:rebar3], [], "hexpm", "17ef63abde837ad30680ea7f857dd9e7ced9476cdd7b0394432af4bfc241b960"}, + "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.4", "f0eafff810d2041e93f915ef59899c923f4568f4585904d010387ed74988e77b", [:make, :mix, :rebar3], [], "hexpm", "603561dc0fd62f4f2ea9b890f4e20e1a0d388746d6e20557cafb1b16950de88c"}, + "unicode_util_compat": {:hex, :unicode_util_compat, "0.4.1", "d869e4c68901dd9531385bb0c8c40444ebf624e60b6962d95952775cac5e90cd", [:rebar3], [], "hexpm", "1d1848c40487cdb0b30e8ed975e34e025860c02e419cb615d255849f3427439d"}, } From ef441ba9ddfd230dda27ed7b9126d2096f6733a1 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 12:08:02 +0100 Subject: [PATCH 05/36] Deps: Add `bunt` as a dependency --- mix.exs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mix.exs b/mix.exs index 3d86dac..924fe66 100644 --- a/mix.exs +++ b/mix.exs @@ -44,6 +44,8 @@ defmodule Knigge.MixProject do # Run "mix help deps" to learn about dependencies. defp deps do [ + {:bunt, "~> 0.2"}, + # No Runtime {:credo, ">= 1.0.0", only: [:dev, :test], runtime: false}, {:dialyxir, "~> 1.0.0-rc.6", only: [:dev], runtime: false}, From 670daec4ae95f8c95fec5063ac9261107f5e1dbd Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 12:20:19 +0100 Subject: [PATCH 06/36] Mix.Tasks - knigge.ensure_exists: Add a first version of the task --- lib/mix/tasks/knigge/ensure_exists.ex | 79 +++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 lib/mix/tasks/knigge/ensure_exists.ex diff --git a/lib/mix/tasks/knigge/ensure_exists.ex b/lib/mix/tasks/knigge/ensure_exists.ex new file mode 100644 index 0000000..436760f --- /dev/null +++ b/lib/mix/tasks/knigge/ensure_exists.ex @@ -0,0 +1,79 @@ +defmodule Mix.Tasks.Knigge.EnsureExists do + use Mix.Task + + @exit_codes %{ + unknown_app: 1, + missing_module: 2 + } + @exit_reasons Map.keys(@exit_codes) + @unknown_error_code 64 + + @impl Mix.Task + def run(_args) do + calling_app() + |> fetch_modules_to_check() + |> check_if_implementations_exist() + |> exit_with() + end + + defp calling_app, do: Mix.Project.get().project()[:app] + + defp fetch_modules_to_check(app) do + with :ok <- Application.load(app), + {:error, :undefined} <- Knigge.Module.fetch_for_app(app) do + error("Unable to load modules for #{app}, are you sure the app exists?") + + {:error, :unknown_app} + else + {:ok, modules} -> {:ok, app, modules} + other -> other + end + end + + defp check_if_implementations_exist({:ok, app, []}) do + warn("No modules in `#{app}` found which `use Knigge`.") + end + + defp check_if_implementations_exist({:ok, _app, modules}) do + Enum.reduce(modules, :ok, fn module, result -> + module + |> check_if_implementation_exists() + |> to_result(result) + end) + end + + defp check_if_implementations_exist(other), do: other + + defp check_if_implementation_exists(module) do + implementation = module.__knigge__(:implementation) + + if Knigge.Module.exists?(implementation) do + success("Implementation `#{inspect(implementation)}` for `#{inspect(module)}` exists.") + else + error("Implementation `#{inspect(implementation)}` for `#{inspect(module)}` is missing!") + end + end + + defp to_result(:ok, result), do: result + defp to_result({:error, _} = error, _result), do: error + + defp exit_with({:error, reason}) when reason in @exit_reasons do + exit({:shutdown, @exit_codes[reason]}) + end + + defp exit_with({:error, unknown_reason}) do + error("An unknown error occurred: #{inspect(unknown_reason)}") + + exit({:shutdown, @unknown_error_code}) + end + + defp exit_with(_), do: :ok + + @success_emoji "✅" + @error_emoji "❌" + @warn_emoji "❓" + + defp success(message), do: Bunt.puts([@success_emoji, " ", :green, message]) + defp error(message), do: Bunt.warn([@error_emoji, " ", :red, message]) + defp warn(message), do: Bunt.warn([@warn_emoji, " ", :gold, message]) +end From 0b7996398b91b859ddec30f2f6d8243a63d75888 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 14:10:40 +0100 Subject: [PATCH 07/36] CLI: Move the output into a separate module --- lib/knigge/cli/output.ex | 14 +++++ lib/mix/tasks/knigge/ensure_exists.ex | 81 ++++++++++++++++++--------- 2 files changed, 68 insertions(+), 27 deletions(-) create mode 100644 lib/knigge/cli/output.ex diff --git a/lib/knigge/cli/output.ex b/lib/knigge/cli/output.ex new file mode 100644 index 0000000..3c42e5c --- /dev/null +++ b/lib/knigge/cli/output.ex @@ -0,0 +1,14 @@ +defmodule Knigge.CLI.Output do + @moduledoc false + + def linebreak, do: IO.puts("") + + def info(message, meta), do: Bunt.puts([:blue, format(message, meta)]) + def success(message, meta), do: Bunt.puts([:green, format(message, meta)]) + def error(message, meta), do: Bunt.warn([:red, format(message, meta)]) + def warn(message, meta), do: Bunt.warn([:gold, format(message, meta)]) + + def format(message, app: app) do + ["[#{app}] ", message] + end +end diff --git a/lib/mix/tasks/knigge/ensure_exists.ex b/lib/mix/tasks/knigge/ensure_exists.ex index 436760f..e2d5fe0 100644 --- a/lib/mix/tasks/knigge/ensure_exists.ex +++ b/lib/mix/tasks/knigge/ensure_exists.ex @@ -1,6 +1,10 @@ defmodule Mix.Tasks.Knigge.EnsureExists do use Mix.Task + import Knigge.CLI.Output + + @recursive true + @exit_codes %{ unknown_app: 1, missing_module: 2 @@ -10,70 +14,93 @@ defmodule Mix.Tasks.Knigge.EnsureExists do @impl Mix.Task def run(_args) do - calling_app() + began_at = current_millis() + app = calling_app() + + app + |> begin() |> fetch_modules_to_check() - |> check_if_implementations_exist() - |> exit_with() + |> verify_implementations(app) + |> finish(app, began_at) + |> exit_with(app) end + defp current_millis, do: :os.system_time(:millisecond) + defp calling_app, do: Mix.Project.get().project()[:app] + defp begin(app) do + info("Verify Knigge implementations.", app: app) + + app + end + defp fetch_modules_to_check(app) do - with :ok <- Application.load(app), + Mix.Task.run("compile") + + with :ok <- ensure_loaded(app), {:error, :undefined} <- Knigge.Module.fetch_for_app(app) do - error("Unable to load modules for #{app}, are you sure the app exists?") + error("Unable to load modules for #{app}, are you sure the app exists?", app: app) {:error, :unknown_app} - else - {:ok, modules} -> {:ok, app, modules} + end + end + + defp ensure_loaded(app) do + case Application.load(app) do + :ok -> :ok + {:error, {:already_loaded, ^app}} -> :ok other -> other end end - defp check_if_implementations_exist({:ok, app, []}) do - warn("No modules in `#{app}` found which `use Knigge`.") + defp verify_implementations({:ok, []}, app) do + warn("No modules in `#{app}` found which `use Knigge`.", app: app) + + :ok end - defp check_if_implementations_exist({:ok, _app, modules}) do + defp verify_implementations({:ok, modules}, app) do Enum.reduce(modules, :ok, fn module, result -> module - |> check_if_implementation_exists() + |> verify_implementation(app) |> to_result(result) end) end - defp check_if_implementations_exist(other), do: other + defp verify_implementations(other, _app), do: other - defp check_if_implementation_exists(module) do + defp verify_implementation(module, app) do implementation = module.__knigge__(:implementation) if Knigge.Module.exists?(implementation) do - success("Implementation `#{inspect(implementation)}` for `#{inspect(module)}` exists.") + success("#{inspect(module)} -> #{inspect(implementation)} (exists)", app: app) else - error("Implementation `#{inspect(implementation)}` for `#{inspect(module)}` is missing!") + error("#{inspect(module)} -> #{inspect(implementation)} (missing)", app: app) end end defp to_result(:ok, result), do: result defp to_result({:error, _} = error, _result), do: error - defp exit_with({:error, reason}) when reason in @exit_reasons do + defp finish(result, app, began_at) do + duration = Float.round((current_millis() - began_at) / 1_000, 3) + + info("Completed in #{duration} seconds.", app: app) + linebreak() + + result + end + + defp exit_with({:error, reason}, _app) when reason in @exit_reasons do exit({:shutdown, @exit_codes[reason]}) end - defp exit_with({:error, unknown_reason}) do - error("An unknown error occurred: #{inspect(unknown_reason)}") + defp exit_with({:error, unknown_reason}, app) do + error("An unknown error occurred: #{inspect(unknown_reason)}", app: app) exit({:shutdown, @unknown_error_code}) end - defp exit_with(_), do: :ok - - @success_emoji "✅" - @error_emoji "❌" - @warn_emoji "❓" - - defp success(message), do: Bunt.puts([@success_emoji, " ", :green, message]) - defp error(message), do: Bunt.warn([@error_emoji, " ", :red, message]) - defp warn(message), do: Bunt.warn([@warn_emoji, " ", :gold, message]) + defp exit_with(_, _app), do: :ok end From 19d7e45b051dcec61e6560c7def255cf1461814b Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 14:26:25 +0100 Subject: [PATCH 08/36] Mix.Task: Rename `knigge.ensure_exists` to `knigge.verify` --- lib/mix/tasks/knigge/{ensure_exists.ex => verify.ex} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename lib/mix/tasks/knigge/{ensure_exists.ex => verify.ex} (96%) diff --git a/lib/mix/tasks/knigge/ensure_exists.ex b/lib/mix/tasks/knigge/verify.ex similarity index 96% rename from lib/mix/tasks/knigge/ensure_exists.ex rename to lib/mix/tasks/knigge/verify.ex index e2d5fe0..4322850 100644 --- a/lib/mix/tasks/knigge/ensure_exists.ex +++ b/lib/mix/tasks/knigge/verify.ex @@ -1,4 +1,4 @@ -defmodule Mix.Tasks.Knigge.EnsureExists do +defmodule Mix.Tasks.Knigge.Verify do use Mix.Task import Knigge.CLI.Output @@ -30,7 +30,7 @@ defmodule Mix.Tasks.Knigge.EnsureExists do defp calling_app, do: Mix.Project.get().project()[:app] defp begin(app) do - info("Verify Knigge implementations.", app: app) + info("Verify Knigge facades.", app: app) app end From d3d7b8f663b6c635ebb3cebb6bbdc9274e3d8749 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 14:26:46 +0100 Subject: [PATCH 09/36] Verify: Add a `Knigge.Verify` module ATM the module "only" allows to verify if the implementation exists. --- lib/knigge/verify.ex | 19 +++++++++++++++++++ test/knigge/verify_test.exs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 lib/knigge/verify.ex create mode 100644 test/knigge/verify_test.exs diff --git a/lib/knigge/verify.ex b/lib/knigge/verify.ex new file mode 100644 index 0000000..23c75f2 --- /dev/null +++ b/lib/knigge/verify.ex @@ -0,0 +1,19 @@ +defmodule Knigge.Verify do + @moduledoc false + + @doc """ + Checks if the given `Knigge` module's implementation exists. Returns an error if not. + """ + @spec implementation(module :: module()) :: + {:ok, implementation :: module()} + | {:error, {:missing, implementation :: module()}} + def implementation(module) do + implementation = module.__knigge__(:implementation) + + if Knigge.Module.exists?(implementation) do + {:ok, implementation} + else + {:error, {:missing, implementation}} + end + end +end diff --git a/test/knigge/verify_test.exs b/test/knigge/verify_test.exs new file mode 100644 index 0000000..9d2eeab --- /dev/null +++ b/test/knigge/verify_test.exs @@ -0,0 +1,35 @@ +defmodule Knigge.VerifyTest do + use ExUnit.Case, async: true + + defmodule FacadeWithImpl do + use Knigge, implementation: Knigge.VerifyTest.FacadeImpl + + @callback some_function() :: :ok + end + + defmodule FacadeWithoutImpl do + use Knigge, + implementation: Does.Not.Exist, + # Surpresses some warnings + delegate_at_runtime?: true + + @callback some_function() :: :ok + end + + defmodule FacadeImpl do + @behaviour Knigge.VerifyTest.FacadeWithImpl + + def some_function, do: :ok + end + + describe ".implementation/1" do + test "returns :ok if the implementation exists" do + assert Knigge.Verify.implementation(FacadeWithImpl) == {:ok, FacadeImpl} + end + + test "returns an error if the implementation is missing" do + assert Knigge.Verify.implementation(FacadeWithoutImpl) == + {:error, {:missing, Does.Not.Exist}} + end + end +end From a90a32ebfe7178bfcb2ebaf0967c76e66195a964 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 15:23:35 +0100 Subject: [PATCH 10/36] Mix.Task: Have a first working version for `knigge.verify` --- lib/knigge/cli/output.ex | 14 +-- lib/mix/tasks/knigge/verify.ex | 160 +++++++++++++++++++++++---------- 2 files changed, 117 insertions(+), 57 deletions(-) diff --git a/lib/knigge/cli/output.ex b/lib/knigge/cli/output.ex index 3c42e5c..f9190f7 100644 --- a/lib/knigge/cli/output.ex +++ b/lib/knigge/cli/output.ex @@ -1,14 +1,8 @@ defmodule Knigge.CLI.Output do @moduledoc false - def linebreak, do: IO.puts("") - - def info(message, meta), do: Bunt.puts([:blue, format(message, meta)]) - def success(message, meta), do: Bunt.puts([:green, format(message, meta)]) - def error(message, meta), do: Bunt.warn([:red, format(message, meta)]) - def warn(message, meta), do: Bunt.warn([:gold, format(message, meta)]) - - def format(message, app: app) do - ["[#{app}] ", message] - end + def info(message), do: Bunt.puts([:blue, message]) + def success(message), do: Bunt.puts([:green, message]) + def error(message), do: Bunt.warn([:red, message]) + def warn(message), do: Bunt.warn([:gold, message]) end diff --git a/lib/mix/tasks/knigge/verify.ex b/lib/mix/tasks/knigge/verify.ex index 4322850..e014769 100644 --- a/lib/mix/tasks/knigge/verify.ex +++ b/lib/mix/tasks/knigge/verify.ex @@ -3,6 +3,17 @@ defmodule Mix.Tasks.Knigge.Verify do import Knigge.CLI.Output + defmodule Context do + @moduledoc false + + defstruct app: nil, + modules: [], + existing: [], + missing: [], + error: nil, + began_at: nil + end + @recursive true @exit_codes %{ @@ -14,93 +25,148 @@ defmodule Mix.Tasks.Knigge.Verify do @impl Mix.Task def run(_args) do - began_at = current_millis() - app = calling_app() - - app - |> begin() - |> fetch_modules_to_check() - |> verify_implementations(app) - |> finish(app, began_at) - |> exit_with(app) + calling_app() + |> run_for() + |> exit_with() end - defp current_millis, do: :os.system_time(:millisecond) + defp calling_app do + Mix.Task.run("compile") - defp calling_app, do: Mix.Project.get().project()[:app] + Mix.Project.get().project()[:app] + end - defp begin(app) do - info("Verify Knigge facades.", app: app) + defp run_for(app) do + began_at = current_millis() - app + with {:ok, modules} <- fetch_modules_to_check(app) do + %Context{ + app: calling_app(), + modules: modules, + began_at: began_at + } + |> begin() + |> verify_implementations() + |> finish() + end end - defp fetch_modules_to_check(app) do - Mix.Task.run("compile") + defp current_millis, do: :os.system_time(:millisecond) + defp fetch_modules_to_check(app) do with :ok <- ensure_loaded(app), - {:error, :undefined} <- Knigge.Module.fetch_for_app(app) do - error("Unable to load modules for #{app}, are you sure the app exists?", app: app) + {:ok, modules} <- Knigge.Module.fetch_for_app(app) do + {:ok, modules} + else + {:error, :undefined} -> + error("Unable to load modules for #{app || "current app"}, are you sure the app exists?") + + {:error, :unknown_app} - {:error, :unknown_app} + other -> + other end end + defp ensure_loaded(nil), do: {:error, :undefined} + defp ensure_loaded(app) do case Application.load(app) do :ok -> :ok {:error, {:already_loaded, ^app}} -> :ok + {:error, {'no such file or directory', _}} -> {:error, :undefined} other -> other end end - defp verify_implementations({:ok, []}, app) do - warn("No modules in `#{app}` found which `use Knigge`.", app: app) + defp begin(%Context{app: app, modules: modules} = context) do + info("Verify #{length(modules)} Knigge facades in '#{app}'.") - :ok + context end - defp verify_implementations({:ok, modules}, app) do - Enum.reduce(modules, :ok, fn module, result -> - module - |> verify_implementation(app) - |> to_result(result) - end) - end + defp verify_implementations(%Context{app: app, modules: []} = context) do + warn("\nNo modules in `#{app}` found which `use Knigge`.") - defp verify_implementations(other, _app), do: other + context + end - defp verify_implementation(module, app) do - implementation = module.__knigge__(:implementation) + defp verify_implementations(context) do + context.modules + |> Enum.map(&verify_implementation/1) + |> Enum.group_by(&elem(&1, 0), &elem(&1, 1)) + |> merge_with_context(context) + end - if Knigge.Module.exists?(implementation) do - success("#{inspect(module)} -> #{inspect(implementation)} (exists)", app: app) - else - error("#{inspect(module)} -> #{inspect(implementation)} (missing)", app: app) + defp verify_implementation(module) do + case Knigge.Verify.implementation(module) do + {:ok, implementation} -> {:existing, {module, implementation}} + {:error, {:missing, implementation}} -> {:missing, {module, implementation}} end end - defp to_result(:ok, result), do: result - defp to_result({:error, _} = error, _result), do: error + defp merge_with_context(%{missing: _} = result, context) do + context + |> Map.put(:error, :missing_module) + |> Map.merge(result) + end - defp finish(result, app, began_at) do - duration = Float.round((current_millis() - began_at) / 1_000, 3) + defp merge_with_context(result, context), do: Map.merge(context, result) + + defp finish(context) do + print_result(context) + completed_in(context) + + maybe_error(context) + end + + defp print_result(context) do + print_existing(context) + print_missing(context) + end + + defp print_existing(%Context{existing: []}), do: :ok + + defp print_existing(%Context{existing: facades, modules: modules}) do + success("\n#{length(facades)}/#{length(modules)} Facades passed:") - info("Completed in #{duration} seconds.", app: app) - linebreak() + facades + |> Enum.map_join("\n", fn {module, implementation} -> + " #{inspect(module)} -> #{inspect(implementation)}" + end) + |> success() + end + + defp print_missing(%Context{missing: []}), do: :ok + + defp print_missing(%Context{missing: facades, modules: modules}) do + error("\n#{length(facades)}/#{length(modules)} Facades failed:") + + facades + |> Enum.map_join("\n", fn {module, implementation} -> + " #{inspect(module)} -> #{inspect(implementation)}" + end) + |> error() + end - result + defp completed_in(%Context{began_at: began_at}) do + duration = Float.round((current_millis() - began_at) / 1_000, 3) + + info("\nCompleted in #{duration} seconds.\n") end - defp exit_with({:error, reason}, _app) when reason in @exit_reasons do + defp maybe_error(%Context{error: nil}), do: :ok + defp maybe_error(%Context{error: reason}), do: {:error, reason} + + defp exit_with({:error, reason}) when reason in @exit_reasons do exit({:shutdown, @exit_codes[reason]}) end - defp exit_with({:error, unknown_reason}, app) do - error("An unknown error occurred: #{inspect(unknown_reason)}", app: app) + defp exit_with({:error, unknown_reason}) do + error("An unknown error occurred: #{inspect(unknown_reason)}") exit({:shutdown, @unknown_error_code}) end - defp exit_with(_, _app), do: :ok + defp exit_with(_), do: :ok end From f2bbeac6c641e9b0eb135aef853cac82935b249b Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 15:26:02 +0100 Subject: [PATCH 11/36] tool-versions: Bump elixir and erlang to the latest versions --- .tool-versions | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.tool-versions b/.tool-versions index 8313347..be680ce 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,2 @@ -elixir 1.9.0-otp-22 -erlang 22.0.7 +elixir 1.10.1 +erlang 22.2.7 From db7dafa7d38a2b852f50491d7effce9d3ad3d67f Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 15:27:31 +0100 Subject: [PATCH 12/36] Mix.Task: Add a bit better error output for `knigge.verify` --- lib/mix/tasks/knigge/verify.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mix/tasks/knigge/verify.ex b/lib/mix/tasks/knigge/verify.ex index e014769..6446fe3 100644 --- a/lib/mix/tasks/knigge/verify.ex +++ b/lib/mix/tasks/knigge/verify.ex @@ -144,7 +144,7 @@ defmodule Mix.Tasks.Knigge.Verify do facades |> Enum.map_join("\n", fn {module, implementation} -> - " #{inspect(module)} -> #{inspect(implementation)}" + " #{inspect(module)} -> #{inspect(implementation)} (implementation does not exist)" end) |> error() end From 1f81fd1c6056569de11daa05f638939350b85db9 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 15:38:03 +0100 Subject: [PATCH 13/36] Options: Remove `check_if_exists?` as an option Log a deprecation warning when somebody uses the option. --- lib/knigge/options.ex | 49 ++++++++++++++---------------------- test/knigge/options_test.exs | 13 +++++++++- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/lib/knigge/options.ex b/lib/knigge/options.ex index 895996d..38e6797 100644 --- a/lib/knigge/options.ex +++ b/lib/knigge/options.ex @@ -1,6 +1,5 @@ defmodule Knigge.Options do @defaults [ - check_if_exists?: true, delegate_at_runtime?: [only: :test], do_not_delegate: [], warn: true @@ -32,18 +31,6 @@ defmodule Knigge.Options do __Default__: the `use`ing `__MODULE__`. - ### `check_if_exists?` - Controls how `Knigge` checks if the given modules exist, accepts: - - - a boolean (turn the check fully on or off) - - one or many environment names (atom or list of atoms) - only checks in the given environments - - `[only: ]` - equivalent to the option above - - `[except: ]` - only checks if the current environment is __not__ contained in the list - - __Default__: `Application.gev_env(:knigge, :check_if_exists?, #{ - inspect(@defaults[:check_if_exists?]) - })` - ### `config_key` The configuration key from which `Knigge` should fetch the implementation. @@ -83,7 +70,6 @@ defmodule Knigge.Options do @type optional :: [ behaviour: behaviour(), config_key: config_key(), - check_if_exists?: boolean_or_envs(), delegate_at_runtime?: boolean_or_envs(), do_not_delegate: do_not_delegate(), warn: warn() @@ -101,7 +87,6 @@ defmodule Knigge.Options do @type t :: %__MODULE__{ implementation: module() | {:config, otp_app(), config_key()}, behaviour: behaviour(), - check_if_exists?: boolean(), delegate_at_runtime?: boolean(), do_not_delegate: do_not_delegate(), warn: warn() @@ -109,7 +94,6 @@ defmodule Knigge.Options do defstruct [ :behaviour, - :check_if_exists?, :delegate_at_runtime?, :do_not_delegate, :implementation, @@ -143,21 +127,33 @@ defmodule Knigge.Options do end defp map_deprecated(opts) when is_list(opts) do - for {key, _} = kv <- opts do + opts + |> Enum.map(fn {key, _} = kv -> case map_deprecated(kv) do ^kv -> kv - {new_key, _} = kv -> + {new_key, _} = kv when is_atom(new_key) -> IO.warn("Knigge encountered the deprecated option `#{key}`, please use `#{new_key}`.") kv + + message when is_binary(message) -> + IO.warn( + "Knigge encountered the deprecated option `#{key}`, this option is no longer supported; #{ + message + }." + ) + + nil end - end + end) + |> Enum.reject(&is_nil/1) end - # TODO: Log deprecated - defp map_deprecated({:check_if_exists, value}), do: {:check_if_exists?, value} + defp map_deprecated({key, _}) + when key in [:check_if_exists, :check_if_exists?], + do: "please use the mix task `mix knigge.verify`" defp map_deprecated({:delegate_at, :compile_time}), do: {:delegate_at_runtime?, false} defp map_deprecated({:delegate_at, :runtime}), do: {:delegate_at_runtime?, true} @@ -187,7 +183,7 @@ defmodule Knigge.Options do defp defaults_from_config do :knigge |> Application.get_all_env() - |> Keyword.take([:check_if_exists?, :delegate_at_runtime?, :warn]) + |> Keyword.take([:delegate_at_runtime?, :warn]) end defp transform(opts, with_env: env) when is_list(opts) do @@ -195,7 +191,7 @@ defmodule Knigge.Options do end defp transform(key, envs, with_env: env) - when key in [:check_if_exists?, :delegate_at_runtime?], + when key in [:delegate_at_runtime?], do: active_env?(env, envs) defp transform(_key, value, with_env: _), do: value @@ -226,9 +222,6 @@ defmodule Knigge.Options do iex> Knigge.Options.validate!(otp_app: :knigge) [otp_app: :knigge] - iex> Knigge.Options.validate!(otp_app: :knigge, check_if_exists?: [:test, :prod]) - [otp_app: :knigge, check_if_exists?: [:test, :prod]] - iex> Knigge.Options.validate!(implementation: SomeModule, otp_app: :knigge) ** (ArgumentError) Knigge expects either the :implementation or the :otp_app option but both were given. @@ -240,9 +233,6 @@ defmodule Knigge.Options do iex> Knigge.Options.validate!(otp_app: :knigge, delegate_at_runtime?: "test") ** (ArgumentError) Knigge received invalid value for `delegate_at_runtime?`. Expected boolean or environment (atom or list of atoms) but received: "test" - - iex> Knigge.Options.validate!(otp_app: :knigge, check_if_exists?: "test") - ** (ArgumentError) Knigge received invalid value for `check_if_exists?`. Expected boolean or environment (atom or list of atoms) but received: "test" """ @spec validate!(raw()) :: no_return def validate!(opts) do @@ -306,7 +296,6 @@ defmodule Knigge.Options do @option_types [ behaviour: :module, - check_if_exists?: :envs, delegate_at_runtime?: :envs, do_not_delegate: :keyword, implementation: :module, diff --git a/test/knigge/options_test.exs b/test/knigge/options_test.exs index efe3306..2418021 100644 --- a/test/knigge/options_test.exs +++ b/test/knigge/options_test.exs @@ -11,7 +11,18 @@ defmodule Knigge.OptionsTest do warnings = capture_io(:stderr, fn -> valid_opts(check_if_exists: true) end) assert warnings =~ - "Knigge encountered the deprecated option `check_if_exists`, please use `check_if_exists?`." + "Knigge encountered the deprecated option `check_if_exists`, " <> + "this option is no longer supported; " <> + "please use the mix task `mix knigge.verify`." + end + + test "using `check_if_exists?` prints a deprecation warning" do + warnings = capture_io(:stderr, fn -> valid_opts(check_if_exists?: true) end) + + assert warnings =~ + "Knigge encountered the deprecated option `check_if_exists?`, " <> + "this option is no longer supported; " <> + "please use the mix task `mix knigge.verify`." end test "using `delegate_at` prints a deprecation warning" do From 0b49c635337ebaf203b9bfe01545bc7ab8edd639 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 15:48:40 +0100 Subject: [PATCH 14/36] Knigge: Cleanup any existence checks and leftover code from that --- config/test.exs | 1 - lib/knigge.ex | 7 ++-- lib/knigge/code.ex | 13 +++----- lib/knigge/code/default.ex | 11 +++--- lib/knigge/code/delegate.ex | 6 ++-- lib/knigge/implementation.ex | 8 ++--- lib/knigge/module.ex | 35 ++------------------ test/behaviour/with_missing_modules_test.exs | 32 +++--------------- 8 files changed, 24 insertions(+), 89 deletions(-) diff --git a/config/test.exs b/config/test.exs index 48ef3b6..a651774 100644 --- a/config/test.exs +++ b/config/test.exs @@ -1,5 +1,4 @@ use Mix.Config config :knigge, - check_if_exists?: [except: :test], delegate_at_runtime?: false diff --git a/lib/knigge.ex b/lib/knigge.ex index 978404a..f0abd13 100644 --- a/lib/knigge.ex +++ b/lib/knigge.ex @@ -198,7 +198,7 @@ defmodule Knigge do behaviour = options |> Knigge.Behaviour.fetch!() - |> Knigge.Module.ensure_exists!(options, __ENV__) + |> Knigge.Module.ensure_exists!(__ENV__) @__knigge__ {:options, options} @@ -219,10 +219,7 @@ defmodule Knigge do Knigge.Implementation.fetch!(__knigge__(:options)) end else - implementation = - options - |> Knigge.Implementation.fetch!() - |> Knigge.Module.ensure_exists!(options, __ENV__) + implementation = Knigge.Implementation.fetch!(options) def __knigge__(:implementation) do unquote(implementation) diff --git a/lib/knigge/code.ex b/lib/knigge/code.ex index 9bc058b..26ad001 100644 --- a/lib/knigge/code.ex +++ b/lib/knigge/code.ex @@ -63,27 +63,24 @@ defmodule Knigge.Code do Default.callback_to_defdefault(callback, from: module, default: default, - delegate_at_runtime?: delegate_at_runtime?, - env: env + delegate_at_runtime?: delegate_at_runtime? ) end true -> Delegate.callback_to_defdelegate(callback, from: module, - delegate_at_runtime?: delegate_at_runtime?, - env: env + delegate_at_runtime?: delegate_at_runtime? ) end end end defp get_behaviour(module, env) do - opts = Knigge.options!(module) - - opts + module + |> Knigge.options!() |> Knigge.Behaviour.fetch!() - |> Knigge.Module.ensure_exists!(opts, env) + |> Knigge.Module.ensure_exists!(env) end defp get_callbacks(module) do diff --git a/lib/knigge/code/default.ex b/lib/knigge/code/default.ex index 6c3dede..30a2c23 100644 --- a/lib/knigge/code/default.ex +++ b/lib/knigge/code/default.ex @@ -9,10 +9,9 @@ defmodule Knigge.Code.Default do {name, arity}, from: module, default: {args, block}, - delegate_at_runtime?: false, - env: env + delegate_at_runtime?: false ) do - implementation = Implementation.fetch_for!(module, env) + implementation = Implementation.fetch_for!(module) cond do Module.open?(implementation) -> @@ -26,8 +25,7 @@ defmodule Knigge.Code.Default do {name, arity}, from: module, default: {args, block}, - delegate_at_runtime?: true, - env: env + delegate_at_runtime?: true ) function_exported?(implementation, name, arity) -> @@ -48,8 +46,7 @@ defmodule Knigge.Code.Default do {name, arity}, from: _module, default: {args, block}, - delegate_at_runtime?: true, - env: _env + delegate_at_runtime?: true ) do quote do def unquote(name)(unquote_splicing(args)) do diff --git a/lib/knigge/code/delegate.ex b/lib/knigge/code/delegate.ex index dcc8a65..2f02fbc 100644 --- a/lib/knigge/code/delegate.ex +++ b/lib/knigge/code/delegate.ex @@ -3,14 +3,14 @@ defmodule Knigge.Code.Delegate do alias Knigge.Implementation - def callback_to_defdelegate({name, arity}, from: module, delegate_at_runtime?: false, env: env) do + def callback_to_defdelegate({name, arity}, from: module, delegate_at_runtime?: false) do callback_to_defdelegate({name, arity}, from: module, - to: Implementation.fetch_for!(module, env) + to: Implementation.fetch_for!(module) ) end - def callback_to_defdelegate({name, arity}, from: _module, delegate_at_runtime?: true, env: _env) do + def callback_to_defdelegate({name, arity}, from: _module, delegate_at_runtime?: true) do quote bind_quoted: [name: name, arity: arity] do args = Macro.generate_arguments(arity, __MODULE__) diff --git a/lib/knigge/implementation.ex b/lib/knigge/implementation.ex index 9e6e243..a639e88 100644 --- a/lib/knigge/implementation.ex +++ b/lib/knigge/implementation.ex @@ -17,11 +17,9 @@ defmodule Knigge.Implementation do implementation end - def fetch_for!(module, env) do - opts = Knigge.options!(module) - - opts + def fetch_for!(module) do + module + |> Knigge.options!() |> Knigge.Implementation.fetch!() - |> Knigge.Module.ensure_exists!(opts, env) end end diff --git a/lib/knigge/module.ex b/lib/knigge/module.ex index 221d97b..cf77266 100644 --- a/lib/knigge/module.ex +++ b/lib/knigge/module.ex @@ -5,45 +5,14 @@ defmodule Knigge.Module do module = inspect(__MODULE__) - def ensure_exists!(module, opts, env) do - unless Knigge.Module.exists?(module, opts) do + def ensure_exists!(module, env) do + unless Knigge.Module.exists?(module) do Knigge.Error.module_not_loaded!(module, env) end module end - @doc """ - Returns true if a module exists, false otherwise. Always returns true if - `check_if_exists?` is set to `false` on the `Knigge.Options` struct. - - ## Examples - - iex> options = %Knigge.Options{check_if_exists?: true} - iex> #{module}.exists?(This.Does.Not.Exist, options) - false - - iex> options = %Knigge.Options{check_if_exists?: false} - iex> #{module}.exists?(This.Does.Not.Exist, options) - true - - iex> options = %Knigge.Options{check_if_exists?: true} - iex> #{module}.exists?(Knigge, options) - true - - iex> options = %Knigge.Options{check_if_exists?: false} - iex> #{module}.exists?(Knigge, options) - true - """ - @spec exists?(module :: module()) :: boolean() - def exists?(module, %Options{} = opts) do - if opts.check_if_exists? do - exists?(module) - else - true - end - end - @doc """ Returns true if a module exists, false otherwise. diff --git a/test/behaviour/with_missing_modules_test.exs b/test/behaviour/with_missing_modules_test.exs index e269a3a..494b3c6 100644 --- a/test/behaviour/with_missing_modules_test.exs +++ b/test/behaviour/with_missing_modules_test.exs @@ -14,38 +14,24 @@ defmodule Behaviour.WithMissingModulesTest do def some_function, do: nil end - test "raises a CompileError when the Implementation does not exist" do - assert_raise CompileError, - ~r"the given module could not be found: DoesNotExist", - fn -> - define_facade( - behaviour: Behaviour, - implementation: DoesNotExist, - check_if_exists?: :test - ) - end - end - test "raises a CompileError when the Behaviour does not exist" do assert_raise CompileError, ~r"the given module could not be found: MissingBehaviour", fn -> define_facade( behaviour: MissingBehaviour, - implementation: Implementation, - check_if_exists?: [except: :dev] + implementation: Implementation ) end end - test "raises a CompileError when both don't exist and `only: [:test, :dev]` is passed for existence check" do + test "raises a CompileError when both don't exist" do assert_raise CompileError, - ~r"the given module could not be found: DoesNotExist", + ~r"the given module could not be found: MissingBehaviour", fn -> define_facade( - behaviour: Behaviour, - implementation: DoesNotExist, - check_if_exists?: [only: [:test, :dev]] + behaviour: MissingBehaviour, + implementation: DoesNotExist ) end end @@ -54,14 +40,6 @@ defmodule Behaviour.WithMissingModulesTest do define_facade(behaviour: Behaviour, implementation: Implementation) end - test "does not raise any error when the implementation is missing but check_if_exists is set to false" do - define_facade( - behaviour: Behaviour, - implementation: MissingImplementation, - check_if_exists?: false - ) - end - defp define_facade(opts) do defmodule_salted Facade do use Knigge, opts From af1a4fa65a9aa61c1dbe37457266c5129860eb6c Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 28 Feb 2020 15:53:41 +0100 Subject: [PATCH 15/36] Options: Allow to pass environments to `warn` --- lib/knigge/options.ex | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/knigge/options.ex b/lib/knigge/options.ex index 38e6797..13db4f5 100644 --- a/lib/knigge/options.ex +++ b/lib/knigge/options.ex @@ -57,7 +57,13 @@ defmodule Knigge.Options do __Default__: `[]` ### `warn` - If set to `false` this disables all warnings generated by `Knigge`, use with care. + Allows to control in which environments `Knigge` should generate warnings, use with care. + Accepts: + + - a boolean (`true` always warns | `false` never warns) + - one or many environment names (atom or list of atoms) - only warns in the given environments + - `[only: ]` - equivalent to the option above + - `[except: ]` - only warns if the current environment is __not__ contained in the list __Default__: `Application.get_env(:knigge, :warn, #{inspect(@defaults[:warn])})` """ @@ -72,7 +78,7 @@ defmodule Knigge.Options do config_key: config_key(), delegate_at_runtime?: boolean_or_envs(), do_not_delegate: do_not_delegate(), - warn: warn() + warn: boolean_or_envs() ] @type behaviour :: module() @@ -82,14 +88,13 @@ defmodule Knigge.Options do @type do_not_delegate :: keyword(arity()) @type envs :: atom() | list(atom()) @type otp_app :: atom() - @type warn :: boolean() @type t :: %__MODULE__{ implementation: module() | {:config, otp_app(), config_key()}, behaviour: behaviour(), delegate_at_runtime?: boolean(), do_not_delegate: do_not_delegate(), - warn: warn() + warn: boolean() } defstruct [ @@ -191,7 +196,7 @@ defmodule Knigge.Options do end defp transform(key, envs, with_env: env) - when key in [:delegate_at_runtime?], + when key in [:delegate_at_runtime?, :warn], do: active_env?(env, envs) defp transform(_key, value, with_env: _), do: value @@ -301,7 +306,7 @@ defmodule Knigge.Options do implementation: :module, otp_app: :atom, config_key: :atom, - warn: :boolean + warn: :envs ] @option_names Keyword.keys(@option_types) From 379dd1f54babbfb7d8ec5ad8aa6c672b45c0a427 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 10:11:39 +0100 Subject: [PATCH 16/36] CLI - Output: By default print to stdio and allow passing stderr --- lib/knigge/cli/output.ex | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/knigge/cli/output.ex b/lib/knigge/cli/output.ex index f9190f7..146cd2f 100644 --- a/lib/knigge/cli/output.ex +++ b/lib/knigge/cli/output.ex @@ -1,8 +1,15 @@ defmodule Knigge.CLI.Output do @moduledoc false - def info(message), do: Bunt.puts([:blue, message]) - def success(message), do: Bunt.puts([:green, message]) - def error(message), do: Bunt.warn([:red, message]) - def warn(message), do: Bunt.warn([:gold, message]) + @device :stdio + + def info(device \\ @device, message), do: print(device, [:blue, message]) + def success(device \\ @device, message), do: print(device, [:green, message]) + def error(device \\ @device, message), do: print(device, [:red, message]) + def warn(device \\ @device, message), do: print(device, [:gold, message]) + + def print(device \\ @device, message) + + def print(:stdio, message), do: Bunt.puts(message) + def print(:stderr, message), do: Bunt.warn(message) end From 5e758be83ae0d9a6739f975e8f63c7941555039c Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 10:23:14 +0100 Subject: [PATCH 17/36] Module: Remove an unused alias --- lib/knigge/module.ex | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/knigge/module.ex b/lib/knigge/module.ex index cf77266..a243d12 100644 --- a/lib/knigge/module.ex +++ b/lib/knigge/module.ex @@ -1,8 +1,6 @@ defmodule Knigge.Module do @moduledoc false - alias Knigge.Options - module = inspect(__MODULE__) def ensure_exists!(module, env) do From 0d3e0ce5c20031f336d29a8cb738b3cbb3499118 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 10:38:06 +0100 Subject: [PATCH 18/36] Verification.Context: Move the context into a separate module Also include a bunch of helper functions --- lib/knigge/verification/context.ex | 107 ++++++++++++++++++++++ test/knigge/verification/context_test.exs | 5 + 2 files changed, 112 insertions(+) create mode 100644 lib/knigge/verification/context.ex create mode 100644 test/knigge/verification/context_test.exs diff --git a/lib/knigge/verification/context.ex b/lib/knigge/verification/context.ex new file mode 100644 index 0000000..f813e90 --- /dev/null +++ b/lib/knigge/verification/context.ex @@ -0,0 +1,107 @@ +defmodule Knigge.Verification.Context do + @moduledoc false + + @type t :: %__MODULE__{ + app: atom(), + modules: list(module()), + existing: list(facade_with_module()), + missing: list(facade_with_module()), + error: nil | any(), + began_at: milliseconds(), + finished_at: milliseconds() + } + @type facade_with_module :: {facade :: module(), impl :: module()} + @type milliseconds :: non_neg_integer() + defstruct app: nil, + modules: [], + existing: [], + missing: [], + error: nil, + began_at: nil, + finished_at: nil + + module = inspect(__MODULE__) + + @spec new() :: t() + @spec new(params :: map() | Keyword.t()) :: t() + def new(params \\ %{}) + + def new(params) when is_list(params) do + params |> Map.new() |> new() + end + + def new(params) do + struct!(__MODULE__, with_defaults(params)) + end + + defp with_defaults(params) do + Map.put_new(params, :began_at, current_millis()) + end + + defp current_millis, do: :os.system_time(:millisecond) + + @doc """ + Sets the `finished_at` field to the given time in milliseconds. + + If none was given it uses the current time. + + ## Examples + + iex> context = %#{module}{finished_at: nil} + iex> context = #{module}.finished(context) + iex> context.finished_at <= :os.system_time(:millisecond) + true + + iex> context = %#{module}{finished_at: nil} + iex> context = #{module}.finished(context, 123) + iex> context.finished_at + 123 + + iex> context = %#{module}{finished_at: 123} + iex> new_context = #{module}.finished(context) + iex> context.finished_at != new_context.finished_at + true + """ + @spec finished(t()) :: t() + @spec finished(t(), milliseconds()) :: t() + def finished(%__MODULE__{} = context, finished_at \\ current_millis()) do + %__MODULE__{context | finished_at: finished_at} + end + + @doc """ + Returns the duration between `began_at` and `finished_at`. Uses the current + time if `finished_at` is `nil`. + + ## Examples + + iex> context = %#{module}{began_at: 100, finished_at: 110} + iex> #{module}.duration(context) + 10 + + iex> now = :os.system_time(:millisecond) + iex> context = %#{module}{began_at: now - 100} + iex> duration = #{module}.duration(context) + iex> duration >= 100 + true + """ + @spec duration(t()) :: milliseconds() + def duration(%__MODULE__{began_at: began_at, finished_at: finished_at}) do + (finished_at || current_millis()) - began_at + end + + @doc """ + Returns whether or not this context is considered an error. + + ## Examples + + iex> context = %#{module}{error: nil} + iex> #{module}.error?(context) + false + + iex> context = %#{module}{error: :some_error} + iex> #{module}.error?(context) + true + """ + @spec error?(t()) :: boolean() + def error?(%__MODULE__{error: error}), do: not is_nil(error) +end diff --git a/test/knigge/verification/context_test.exs b/test/knigge/verification/context_test.exs new file mode 100644 index 0000000..eb5cc1c --- /dev/null +++ b/test/knigge/verification/context_test.exs @@ -0,0 +1,5 @@ +defmodule Knigge.Verification.ContextTest do + use ExUnit.Case, async: true + + doctest Knigge.Verification.Context +end From 8b8f217d813b29357d8471f4800021b234ae6e2d Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 10:50:39 +0100 Subject: [PATCH 19/36] Verification.Context: Add a `for_app` function which loads the modules --- lib/knigge/verification/context.ex | 48 ++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/lib/knigge/verification/context.ex b/lib/knigge/verification/context.ex index f813e90..6112163 100644 --- a/lib/knigge/verification/context.ex +++ b/lib/knigge/verification/context.ex @@ -40,6 +40,54 @@ defmodule Knigge.Verification.Context do defp current_millis, do: :os.system_time(:millisecond) + @doc """ + Loads the modules for the given app which `use Knigge`. + + Returns an error when the app does not exist or loading it fails. + + ## Example + + iex> {:ok, context} = #{module}.for_app(:knigge) + iex> context.app + :knigge + iex> context.modules + [] + + iex> context = %#{module}{began_at: 123} + iex> {:ok, context} = #{module}.for_app(context, :knigge) + iex> context.began_at + 123 + iex> context.app + :knigge + iex> context.modules + [] + + iex> #{module}.for_app(:does_not_exist) + {:error, {:unknown_app, :does_not_exist}} + """ + @spec for_app(app :: atom()) :: {:ok, t()} | {:error, reason :: any()} + @spec for_app(t(), app :: atom()) :: {:ok, t()} | {:error, reason :: any()} + def for_app(context \\ new(), app) + + def for_app(%__MODULE__{} = context, app) do + with :ok <- ensure_loaded(app), + {:ok, modules} <- Knigge.Module.fetch_for_app(app) do + {:ok, %__MODULE__{context | app: app, modules: modules}} + end + end + + defp ensure_loaded(nil), do: {:error, {:unknown_app, nil}} + + defp ensure_loaded(app) do + case Application.load(app) do + :ok -> :ok + {:error, {:already_loaded, ^app}} -> :ok + {:error, {'no such file or directory', _}} -> {:error, {:unknown_app, app}} + {:error, :undefined} -> {:error, {:unknown_app, app}} + other -> other + end + end + @doc """ Sets the `finished_at` field to the given time in milliseconds. From 16d0a0d5fdd023820b763770c39fb087c56726e7 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 11:00:18 +0100 Subject: [PATCH 20/36] Verification.Context: Add `is_error` as guard --- lib/knigge/verification/context.ex | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/knigge/verification/context.ex b/lib/knigge/verification/context.ex index 6112163..6b2723d 100644 --- a/lib/knigge/verification/context.ex +++ b/lib/knigge/verification/context.ex @@ -140,16 +140,32 @@ defmodule Knigge.Verification.Context do @doc """ Returns whether or not this context is considered an error. + Can be used in guards. + ## Examples + iex> require #{module} iex> context = %#{module}{error: nil} - iex> #{module}.error?(context) + iex> #{module}.is_error(context) false + iex> require #{module} iex> context = %#{module}{error: :some_error} - iex> #{module}.error?(context) + iex> #{module}.is_error(context) true """ + @spec is_error(t()) :: boolean() + defguard is_error(context) + when :erlang.is_map_key(:__struct__, context) and + :erlang.map_get(:__struct__, context) == __MODULE__ and + :erlang.is_map_key(:error, context) and + :erlang.map_get(:error, context) != nil + + @doc """ + Returns whether or not this context is considered an error. + + Uses `is_error/1`. + """ @spec error?(t()) :: boolean() - def error?(%__MODULE__{error: error}), do: not is_nil(error) + def error?(context), do: is_error(context) end From cf17cac0532908bc6a8fab7cc1c65012e203a5da Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 11:19:29 +0100 Subject: [PATCH 21/36] Task - knigge.verify: Rewrite to use the new `Knigge.Verification.Context` --- lib/mix/tasks/knigge/verify.ex | 98 ++++++++++++++-------------------- 1 file changed, 40 insertions(+), 58 deletions(-) diff --git a/lib/mix/tasks/knigge/verify.ex b/lib/mix/tasks/knigge/verify.ex index 6446fe3..3da2c91 100644 --- a/lib/mix/tasks/knigge/verify.ex +++ b/lib/mix/tasks/knigge/verify.ex @@ -3,26 +3,12 @@ defmodule Mix.Tasks.Knigge.Verify do import Knigge.CLI.Output - defmodule Context do - @moduledoc false + alias Knigge.Verification.Context - defstruct app: nil, - modules: [], - existing: [], - missing: [], - error: nil, - began_at: nil - end + require Context @recursive true - @exit_codes %{ - unknown_app: 1, - missing_module: 2 - } - @exit_reasons Map.keys(@exit_codes) - @unknown_error_code 64 - @impl Mix.Task def run(_args) do calling_app() @@ -37,28 +23,13 @@ defmodule Mix.Tasks.Knigge.Verify do end defp run_for(app) do - began_at = current_millis() - - with {:ok, modules} <- fetch_modules_to_check(app) do - %Context{ - app: calling_app(), - modules: modules, - began_at: began_at - } - |> begin() + with {:ok, context} <- Context.for_app(app) do + context + |> begin_verification() |> verify_implementations() - |> finish() - end - end - - defp current_millis, do: :os.system_time(:millisecond) - - defp fetch_modules_to_check(app) do - with :ok <- ensure_loaded(app), - {:ok, modules} <- Knigge.Module.fetch_for_app(app) do - {:ok, modules} + |> finish_verification() else - {:error, :undefined} -> + {:error, {:unknown_app, app}} -> error("Unable to load modules for #{app || "current app"}, are you sure the app exists?") {:error, :unknown_app} @@ -68,18 +39,7 @@ defmodule Mix.Tasks.Knigge.Verify do end end - defp ensure_loaded(nil), do: {:error, :undefined} - - defp ensure_loaded(app) do - case Application.load(app) do - :ok -> :ok - {:error, {:already_loaded, ^app}} -> :ok - {:error, {'no such file or directory', _}} -> {:error, :undefined} - other -> other - end - end - - defp begin(%Context{app: app, modules: modules} = context) do + defp begin_verification(%Context{app: app, modules: modules} = context) do info("Verify #{length(modules)} Knigge facades in '#{app}'.") context @@ -107,22 +67,23 @@ defmodule Mix.Tasks.Knigge.Verify do defp merge_with_context(%{missing: _} = result, context) do context - |> Map.put(:error, :missing_module) + |> Map.put(:error, :missing_modules) |> Map.merge(result) end defp merge_with_context(result, context), do: Map.merge(context, result) - defp finish(context) do - print_result(context) - completed_in(context) - - maybe_error(context) + defp finish_verification(context) do + context + |> print_result() + |> completed_in() end defp print_result(context) do print_existing(context) print_missing(context) + + context end defp print_existing(%Context{existing: []}), do: :ok @@ -149,19 +110,40 @@ defmodule Mix.Tasks.Knigge.Verify do |> error() end - defp completed_in(%Context{began_at: began_at}) do - duration = Float.round((current_millis() - began_at) / 1_000, 3) + defp completed_in(context) do + context = Context.finished(context) + + duration = + context + |> Context.duration() + |> Kernel./(1_000) + |> Float.round(3) info("\nCompleted in #{duration} seconds.\n") + + context end - defp maybe_error(%Context{error: nil}), do: :ok - defp maybe_error(%Context{error: reason}), do: {:error, reason} + defp exit_with(%Context{error: :missing_modules} = context) do + error( + :stderr, + "Validation failed for #{length(context.missing)}/#{length(context.modules)} facades." + ) + exit_with({:error, :missing_modules}) + end + + defp exit_with(%Context{} = context) when Context.is_error(context) do + exit_with({:error, context.error}) + end + + @exit_codes %{unknown_app: 1, missing_modules: 2} + @exit_reasons Map.keys(@exit_codes) defp exit_with({:error, reason}) when reason in @exit_reasons do exit({:shutdown, @exit_codes[reason]}) end + @unknown_error_code 64 defp exit_with({:error, unknown_reason}) do error("An unknown error occurred: #{inspect(unknown_reason)}") From 557f58da1855845c52c9421b07021f7947b917b9 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 11:25:46 +0100 Subject: [PATCH 22/36] Verification: Move the actual verification logic into the Verification module Now the `knigge.verify` task only delegates the verification and contains the output logic printing messages to the CLI --- lib/knigge/verification.ex | 53 ++++++++++++++++++++++++++++++++++ lib/knigge/verify.ex | 19 ------------ lib/mix/tasks/knigge/verify.ex | 41 ++++++-------------------- 3 files changed, 62 insertions(+), 51 deletions(-) create mode 100644 lib/knigge/verification.ex delete mode 100644 lib/knigge/verify.ex diff --git a/lib/knigge/verification.ex b/lib/knigge/verification.ex new file mode 100644 index 0000000..bd07d8f --- /dev/null +++ b/lib/knigge/verification.ex @@ -0,0 +1,53 @@ +defmodule Knigge.Verification do + @moduledoc false + + alias __MODULE__.Context + + @doc """ + Runs all verifications for the given `#{inspect(Context)}`. + + At the moment this only consists of checking whether or not the Implementations exist. + """ + @spec run(Context.t()) :: Context.t() + def run(%Context{} = context) do + verify_implementations(context) + end + + defp verify_implementations(context) do + context.modules + |> Enum.map(&verify_implementation/1) + |> Enum.group_by(&elem(&1, 0), &elem(&1, 1)) + |> merge_with_context(context) + end + + defp verify_implementation(module) do + case Verification.implementation(module) do + {:ok, implementation} -> {:existing, {module, implementation}} + {:error, {:missing, implementation}} -> {:missing, {module, implementation}} + end + end + + defp merge_with_context(%{missing: _} = result, context) do + context + |> Map.put(:error, :missing_modules) + |> Map.merge(result) + end + + defp merge_with_context(result, context), do: Map.merge(context, result) + + @doc """ + Checks if the given `Knigge` module's implementation exists. Returns an error if not. + """ + @spec implementation(module :: module()) :: + {:ok, implementation :: module()} + | {:error, {:missing, implementation :: module()}} + def implementation(module) do + implementation = module.__knigge__(:implementation) + + if Knigge.Module.exists?(implementation) do + {:ok, implementation} + else + {:error, {:missing, implementation}} + end + end +end diff --git a/lib/knigge/verify.ex b/lib/knigge/verify.ex deleted file mode 100644 index 23c75f2..0000000 --- a/lib/knigge/verify.ex +++ /dev/null @@ -1,19 +0,0 @@ -defmodule Knigge.Verify do - @moduledoc false - - @doc """ - Checks if the given `Knigge` module's implementation exists. Returns an error if not. - """ - @spec implementation(module :: module()) :: - {:ok, implementation :: module()} - | {:error, {:missing, implementation :: module()}} - def implementation(module) do - implementation = module.__knigge__(:implementation) - - if Knigge.Module.exists?(implementation) do - {:ok, implementation} - else - {:error, {:missing, implementation}} - end - end -end diff --git a/lib/mix/tasks/knigge/verify.ex b/lib/mix/tasks/knigge/verify.ex index 3da2c91..b18fb02 100644 --- a/lib/mix/tasks/knigge/verify.ex +++ b/lib/mix/tasks/knigge/verify.ex @@ -3,6 +3,7 @@ defmodule Mix.Tasks.Knigge.Verify do import Knigge.CLI.Output + alias Knigge.Verification alias Knigge.Verification.Context require Context @@ -11,22 +12,20 @@ defmodule Mix.Tasks.Knigge.Verify do @impl Mix.Task def run(_args) do + Mix.Task.run("compile") + calling_app() |> run_for() |> exit_with() end - defp calling_app do - Mix.Task.run("compile") - - Mix.Project.get().project()[:app] - end + defp calling_app, do: Mix.Project.get().project()[:app] defp run_for(app) do with {:ok, context} <- Context.for_app(app) do context |> begin_verification() - |> verify_implementations() + |> Verification.run() |> finish_verification() else {:error, {:unknown_app, app}} -> @@ -39,40 +38,18 @@ defmodule Mix.Tasks.Knigge.Verify do end end - defp begin_verification(%Context{app: app, modules: modules} = context) do - info("Verify #{length(modules)} Knigge facades in '#{app}'.") - - context - end - - defp verify_implementations(%Context{app: app, modules: []} = context) do - warn("\nNo modules in `#{app}` found which `use Knigge`.") + defp begin_verification(%Context{modules: []} = context) do + warn("No modules in `#{app}` found which `use Knigge`.") context end - defp verify_implementations(context) do - context.modules - |> Enum.map(&verify_implementation/1) - |> Enum.group_by(&elem(&1, 0), &elem(&1, 1)) - |> merge_with_context(context) - end - - defp verify_implementation(module) do - case Knigge.Verify.implementation(module) do - {:ok, implementation} -> {:existing, {module, implementation}} - {:error, {:missing, implementation}} -> {:missing, {module, implementation}} - end - end + defp begin_verification(%Context{app: app, modules: modules} = context) do + info("Verify #{length(modules)} Knigge facades in '#{app}'.") - defp merge_with_context(%{missing: _} = result, context) do context - |> Map.put(:error, :missing_modules) - |> Map.merge(result) end - defp merge_with_context(result, context), do: Map.merge(context, result) - defp finish_verification(context) do context |> print_result() From ab95c968b8e767a01e4a20fd9fcac24ba09b1f7c Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 11:59:20 +0100 Subject: [PATCH 23/36] Mix.Task - knigge.verify: Fix a missing variable --- lib/mix/tasks/knigge/verify.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mix/tasks/knigge/verify.ex b/lib/mix/tasks/knigge/verify.ex index b18fb02..fb864b7 100644 --- a/lib/mix/tasks/knigge/verify.ex +++ b/lib/mix/tasks/knigge/verify.ex @@ -38,7 +38,7 @@ defmodule Mix.Tasks.Knigge.Verify do end end - defp begin_verification(%Context{modules: []} = context) do + defp begin_verification(%Context{app: app, modules: []} = context) do warn("No modules in `#{app}` found which `use Knigge`.") context From 9d767ff6fb4677dd65d80924763f0186947be263 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 12:01:15 +0100 Subject: [PATCH 24/36] Verification: Call the correct function after moving the verification --- lib/knigge/verification.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/knigge/verification.ex b/lib/knigge/verification.ex index bd07d8f..b13f5da 100644 --- a/lib/knigge/verification.ex +++ b/lib/knigge/verification.ex @@ -21,7 +21,7 @@ defmodule Knigge.Verification do end defp verify_implementation(module) do - case Verification.implementation(module) do + case check_implementation(module) do {:ok, implementation} -> {:existing, {module, implementation}} {:error, {:missing, implementation}} -> {:missing, {module, implementation}} end @@ -38,10 +38,10 @@ defmodule Knigge.Verification do @doc """ Checks if the given `Knigge` module's implementation exists. Returns an error if not. """ - @spec implementation(module :: module()) :: + @spec check_implementation(module :: module()) :: {:ok, implementation :: module()} | {:error, {:missing, implementation :: module()}} - def implementation(module) do + def check_implementation(module) do implementation = module.__knigge__(:implementation) if Knigge.Module.exists?(implementation) do From 476265079d0e6ebac4f7b5090f1af71bc8983600 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 12:03:01 +0100 Subject: [PATCH 25/36] Test: Rename the missed VerifyTest to VerificationTest --- .../{verify_test.exs => verification_test.exs} | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) rename test/knigge/{verify_test.exs => verification_test.exs} (60%) diff --git a/test/knigge/verify_test.exs b/test/knigge/verification_test.exs similarity index 60% rename from test/knigge/verify_test.exs rename to test/knigge/verification_test.exs index 9d2eeab..a8ffdab 100644 --- a/test/knigge/verify_test.exs +++ b/test/knigge/verification_test.exs @@ -1,8 +1,8 @@ -defmodule Knigge.VerifyTest do +defmodule Knigge.VerificationTest do use ExUnit.Case, async: true defmodule FacadeWithImpl do - use Knigge, implementation: Knigge.VerifyTest.FacadeImpl + use Knigge, implementation: Knigge.VerificationTest.FacadeImpl @callback some_function() :: :ok end @@ -17,18 +17,18 @@ defmodule Knigge.VerifyTest do end defmodule FacadeImpl do - @behaviour Knigge.VerifyTest.FacadeWithImpl + @behaviour Knigge.VerificationTest.FacadeWithImpl def some_function, do: :ok end - describe ".implementation/1" do + describe ".check_implementation/1" do test "returns :ok if the implementation exists" do - assert Knigge.Verify.implementation(FacadeWithImpl) == {:ok, FacadeImpl} + assert Knigge.Verification.check_implementation(FacadeWithImpl) == {:ok, FacadeImpl} end test "returns an error if the implementation is missing" do - assert Knigge.Verify.implementation(FacadeWithoutImpl) == + assert Knigge.Verification.check_implementation(FacadeWithoutImpl) == {:error, {:missing, Does.Not.Exist}} end end From de3da627d2846e54ea44236bdda261855d3fbd75 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 12:09:50 +0100 Subject: [PATCH 26/36] Test - Verification: Test the `run/1` function --- test/knigge/verification_test.exs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/test/knigge/verification_test.exs b/test/knigge/verification_test.exs index a8ffdab..09b6e21 100644 --- a/test/knigge/verification_test.exs +++ b/test/knigge/verification_test.exs @@ -1,6 +1,9 @@ defmodule Knigge.VerificationTest do use ExUnit.Case, async: true + alias Knigge.Verification + alias Knigge.Verification.Context + defmodule FacadeWithImpl do use Knigge, implementation: Knigge.VerificationTest.FacadeImpl @@ -22,13 +25,33 @@ defmodule Knigge.VerificationTest do def some_function, do: :ok end + describe ".run/1" do + test "returns a context without an error when passing a context with only `FacadeWithImpl`" do + raw_context = %Context{app: :knigge, modules: [FacadeWithImpl]} + context = Verification.run(raw_context) + + assert context.existing == [{FacadeWithImpl, FacadeImpl}] + assert context.missing == [] + assert context.error == nil + end + + test "returns a context containing an error when passing a context with `FacadeWithImpl` and `FacadeWithoutImpl`" do + raw_context = %Context{app: :knigge, modules: [FacadeWithImpl, FacadeWithoutImpl]} + context = Verification.run(raw_context) + + assert context.existing == [{FacadeWithImpl, FacadeImpl}] + assert context.missing == [{FacadeWithoutImpl, Does.Not.Exist}] + assert context.error == :missing_modules + end + end + describe ".check_implementation/1" do test "returns :ok if the implementation exists" do - assert Knigge.Verification.check_implementation(FacadeWithImpl) == {:ok, FacadeImpl} + assert Verification.check_implementation(FacadeWithImpl) == {:ok, FacadeImpl} end test "returns an error if the implementation is missing" do - assert Knigge.Verification.check_implementation(FacadeWithoutImpl) == + assert Verification.check_implementation(FacadeWithoutImpl) == {:error, {:missing, Does.Not.Exist}} end end From 54a92a609469a9453c88e1ef6fbae5c91bcad418 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 12:19:08 +0100 Subject: [PATCH 27/36] Verification.Context: Add some more doctests --- lib/knigge/verification/context.ex | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/lib/knigge/verification/context.ex b/lib/knigge/verification/context.ex index 6b2723d..43e47cc 100644 --- a/lib/knigge/verification/context.ex +++ b/lib/knigge/verification/context.ex @@ -22,6 +22,21 @@ defmodule Knigge.Verification.Context do module = inspect(__MODULE__) + @doc """ + Creates a new `#{module}` struct while setting the `began_at` to now. + + ## Example + + iex> context = #{module}.new() + iex> context.began_at <= #{module}.timestamp() + true + + iex> context = #{module}.new(began_at: 123, app: :foobar) + iex> context.began_at + 123 + iex> context.app + :foobar + """ @spec new() :: t() @spec new(params :: map() | Keyword.t()) :: t() def new(params \\ %{}) @@ -35,10 +50,10 @@ defmodule Knigge.Verification.Context do end defp with_defaults(params) do - Map.put_new(params, :began_at, current_millis()) + Map.put_new(params, :began_at, timestamp()) end - defp current_millis, do: :os.system_time(:millisecond) + def timestamp, do: :os.system_time(:millisecond) @doc """ Loads the modules for the given app which `use Knigge`. @@ -97,7 +112,7 @@ defmodule Knigge.Verification.Context do iex> context = %#{module}{finished_at: nil} iex> context = #{module}.finished(context) - iex> context.finished_at <= :os.system_time(:millisecond) + iex> context.finished_at <= #{module}.timestamp() true iex> context = %#{module}{finished_at: nil} @@ -112,7 +127,7 @@ defmodule Knigge.Verification.Context do """ @spec finished(t()) :: t() @spec finished(t(), milliseconds()) :: t() - def finished(%__MODULE__{} = context, finished_at \\ current_millis()) do + def finished(%__MODULE__{} = context, finished_at \\ timestamp()) do %__MODULE__{context | finished_at: finished_at} end @@ -126,7 +141,7 @@ defmodule Knigge.Verification.Context do iex> #{module}.duration(context) 10 - iex> now = :os.system_time(:millisecond) + iex> now = #{module}.timestamp() iex> context = %#{module}{began_at: now - 100} iex> duration = #{module}.duration(context) iex> duration >= 100 @@ -134,7 +149,7 @@ defmodule Knigge.Verification.Context do """ @spec duration(t()) :: milliseconds() def duration(%__MODULE__{began_at: began_at, finished_at: finished_at}) do - (finished_at || current_millis()) - began_at + (finished_at || timestamp()) - began_at end @doc """ From 5f5882acef1637f5bf0c41ce8d07176fafc086ea Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 12:19:21 +0100 Subject: [PATCH 28/36] Coveralls: Skip the mix task from code coverage --- coveralls.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 coveralls.json diff --git a/coveralls.json b/coveralls.json new file mode 100644 index 0000000..be108de --- /dev/null +++ b/coveralls.json @@ -0,0 +1,5 @@ +{ + "skip_files": [ + "lib/mix/tasks" + ] +} From 583545a6f75868384ced1fb5bc503ff6a8a0928a Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 13:35:38 +0100 Subject: [PATCH 29/36] Docs: Add an "article" on the existence check --- pages/The Existence Check.md | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 pages/The Existence Check.md diff --git a/pages/The Existence Check.md b/pages/The Existence Check.md new file mode 100644 index 0000000..a1727f2 --- /dev/null +++ b/pages/The Existence Check.md @@ -0,0 +1,54 @@ +# The Existence Check + +Before version 1.2.0 `Knigge` used to try to verify at compile time whether the implementation module of a facade exists. +If `Knigge` found the implementation missing it would raise an error. + +The idea being that this would catch spelling mistakes and the like **before** the application. +Having a spelling error in delegating to the implementation is **no bueno**: it will crash and burn in a horrible disaster at runtime. + +At the time the idea seemed great - and to be honest I still think it does - but reality turned out to be more complicated. + +## Knigge VS the Compiler + +Sometimes `Knigge` would find the implementing module to be missing even though it was there, no spelling error, no nothing. +Anybody who had the pleasure experiencing this would - very rightfully I must say - wonder what the hell was going on. + +As it turns out there is no guarantee about the order in which the Elixir compiler will compile the modules of your project. +While it does ensure that dependencies get resolved - such as specifying a `@behaviour` in your module - it will happily chug along compiling your modules in parallel. + +And don't get me wrong, that's a good thing, I like that the compiler does this. +It's a great way to speed up compilation, and proves that there are no weird interdependencies in the order of compilation; but it does lead to a problem with `Knigge`. + +You see, sometimes the compiler might start with your implementation module `MyImplementation`. +It would then encounter the `@behaviour MyFacade` line, interrupt compilation of the module and compile `MyFacade`. +In cases like this the existence check works just fine. + +In other cases the compiler might start with compiling `MyFacade`. +Finding no dependencies it would happily chug along, resolve `use Knigge` and ... BOOM! +`Knigge` would raise a tantrum because it cannot find `MyImplementation`. + +So what can we do about it? + +## Introducing `mix knigge.verify` + +Instead of doing the existence check at compile time `Knigge` now offers the `knigge.verify` mix task. + +By using a `Mix.Task` `Knigge` bypasses the whole compilation conundrum since the task runs after your project was fully compiled. + +The task scans your app for all modules which `use Knigge` by checking for a `__knigge__/0` function. If this function is found the task fetches the implementation of the facade using `__knigge__(:implementation)` and then verifies that the returned module actually exists. + +After performing this check for all found modules it prints the results and exits with an error code if an implementing module was found missing. + +As such you can easily integrate `mix knigge.verify` into your CI pipeline to ensure that all implementations exist before pushing to production. + +## Roadmap + +In addition to having the `mix knigge.verify` task I would like to create a compiler step which performs the existence check. This could then be added to your project in `mix.exs` under the `compilers` key in your `project` function (similar to how `phoenix` and `gettext` add additional compiler steps). + +Furthermore I could imagine adding additional verification steps in the future: + +- ensuring the implementation actually implements all necessary callbacks +- somehow integrating with `dialyzer` to check the types of the implementation + +But until the necessary research and experiments have been done it's hard to say where the journey will go. +Nevertheless feel free to open issues to discuss potential features at any time. From e03b3e081e2ef2e5eb87101c1dd3f672cac0de09 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 14:45:08 +0100 Subject: [PATCH 30/36] README: Add a section on `mix knigge.verify` --- README.md | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 91ab772..1c85846 100644 --- a/README.md +++ b/README.md @@ -22,11 +22,16 @@ passing the behaviour which should be "facaded" as an option. ## Overview -- [Installation](#installation) -- [Motivation](#motivation) -- [Examples](#examples) -- [Options](#options) -- [Knigge and the `:test` environment](#knigge-and-the-test-environment) +- [Knigge](#knigge) + - [Overview](#overview) + - [Installation](#installation) + - [Motivation](#motivation) + - [Examples](#examples) + - [`defdefault` - Fallback implementations for optional callbacks](#defdefault---fallback-implementations-for-optional-callbacks) + - [Options](#options) + - [Verifying your Implementations - `mix knigge.verify`](#verifying-your-implementations---mix-kniggeverify) + - [Knigge and the `:test` environment](#knigge-and-the-test-environment) + - [Compiler Warnings](#compiler-warnings) ## Installation @@ -195,6 +200,16 @@ as option - by default `Knigge` delegates at runtime in your `:test`s. For further information about options check the [`Knigge.Options` module](https://hexdocs.pm/knigge/Knigge.Options.html). +## Verifying your Implementations - `mix knigge.verify` + +Before version 1.2.0 `Knigge` tried to check at compile time if the implementation of your facade existed. +Due to the way the Elixir compiler goes about compiling your modules this didn't work as expected - [checkout this page if you're interested in the details](https://hexdocs.pm/knigge/the-existence-check.html). + +As an alternative `Knigge` now offers the `mix knigge.verify` task which verifies that the implementation modules of your facades actually exist. +The task returns with an error code when an implementation is missing, which allows you to plug it into your CI pipeline - for example as `MIX_ENV=prod mix knigge.verify`. + +For details check the documentation of `mix knigge.verify` by running `mix help knigge.verify`. + ## Knigge and the `:test` environment To give the maximum amount of flexibility `Knigge` delegates at runtime in your @@ -211,7 +226,7 @@ In case you change the `delegate_at_runtime?` configuration to anything which excludes the `:test` environment you will - most likely - encounter compiler warnings like this: -``` +```text warning: function MyMock.my_great_callback/1 is undefined (module MyMock is not available) lib/my_facade.ex:1 From 0cd1119db0564f6bef9520e844f63dfa7f76a275 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 14:45:35 +0100 Subject: [PATCH 31/36] Mix - Docs: Specify a bunch of options for ex_doc --- mix.exs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/mix.exs b/mix.exs index 924fe66..3ec8cc8 100644 --- a/mix.exs +++ b/mix.exs @@ -26,6 +26,7 @@ defmodule Knigge.MixProject do # Hex description: description(), + docs: docs(), package: package(), version: @version ] @@ -68,6 +69,30 @@ defmodule Knigge.MixProject do "An opinionated way of dealing with behaviours." end + @extras Path.wildcard("pages/**/*.md") + def docs do + [ + main: "Knigge", + source_ref: "v#{@version}", + source_url: "https://github.com/sascha-wolf/knigge", + extras: @extras, + groups_for_modules: [ + "Overview & Configuration": [ + Knigge, + Knigge.Options + ], + "Code Generation": [ + Knigge.Behaviour, + Knigge.Code, + Knigge.Implementation + ], + Verification: [ + Knigge.Verification + ] + ] + ] + end + def package do [ files: ["lib", "mix.exs", "LICENSE*", "README*", "version"], From d04045c4a956ec6b10601f4bf58b3268ef786fb2 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 14:57:33 +0100 Subject: [PATCH 32/36] Mix.Task - knigge.verify: Add documentation --- lib/mix/tasks/knigge/verify.ex | 35 +++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/mix/tasks/knigge/verify.ex b/lib/mix/tasks/knigge/verify.ex index fb864b7..d807b45 100644 --- a/lib/mix/tasks/knigge/verify.ex +++ b/lib/mix/tasks/knigge/verify.ex @@ -8,7 +8,40 @@ defmodule Mix.Tasks.Knigge.Verify do require Context - @recursive true + @shortdoc "Verify the validity of your facades and their implementations." + @moduledoc """ + #{@shortdoc} + + At the moment `knigge.verify` "only" ensures that the implementation modules + of your facades exist. Running the task on a code base with two facades might + look like this: + + $ mix knigge.verify + Verify 2 Knigge facades in 'my_app'. + + 1/2 Facades passed: + MyApp.MyGreatFacade -> MyApp.MyGreatImpl + + 1/2 Facades failed: + MyApp.AnotherFacade -> MyApp.AnothrImpl (implementation does not exist) + + Completed in 0.009 seconds. + + Validation failed for 1/2 facades. + + The attentive reader might have noticed that `MyApp.AnothrImpl` contains a + spelling error: `Anothr` instead of `Another`. + + Catching errors like this is the main responsibility of `knigge.verify`. When + an issue is detected the task will exit with an error code, which allows you + to use it in your CI pipeline - for example before you build your production + release. + + ## Options + + At the moment `knigge.verify` offers no options. If you think this should be + different please open an issue. + """ @impl Mix.Task def run(_args) do From cc2768dd2b64119db38c8f0292248f0c7a846a49 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 15:15:29 +0100 Subject: [PATCH 33/36] OTP: Add a compile time switch due to OTP 21 features --- lib/knigge/otp.ex | 6 +++ lib/knigge/verification/context.ex | 80 ++++++++++++++++++------------ lib/mix/tasks/knigge/verify.ex | 14 +++++- 3 files changed, 67 insertions(+), 33 deletions(-) create mode 100644 lib/knigge/otp.ex diff --git a/lib/knigge/otp.ex b/lib/knigge/otp.ex new file mode 100644 index 0000000..bb9c556 --- /dev/null +++ b/lib/knigge/otp.ex @@ -0,0 +1,6 @@ +defmodule Knigge.OTP do + @moduledoc false + + @otp_release :otp_release |> :erlang.system_info() |> List.to_string() |> String.to_integer() + def release, do: @otp_release +end diff --git a/lib/knigge/verification/context.ex b/lib/knigge/verification/context.ex index 43e47cc..3798af1 100644 --- a/lib/knigge/verification/context.ex +++ b/lib/knigge/verification/context.ex @@ -152,35 +152,53 @@ defmodule Knigge.Verification.Context do (finished_at || timestamp()) - began_at end - @doc """ - Returns whether or not this context is considered an error. - - Can be used in guards. - - ## Examples - - iex> require #{module} - iex> context = %#{module}{error: nil} - iex> #{module}.is_error(context) - false - - iex> require #{module} - iex> context = %#{module}{error: :some_error} - iex> #{module}.is_error(context) - true - """ - @spec is_error(t()) :: boolean() - defguard is_error(context) - when :erlang.is_map_key(:__struct__, context) and - :erlang.map_get(:__struct__, context) == __MODULE__ and - :erlang.is_map_key(:error, context) and - :erlang.map_get(:error, context) != nil - - @doc """ - Returns whether or not this context is considered an error. - - Uses `is_error/1`. - """ - @spec error?(t()) :: boolean() - def error?(context), do: is_error(context) + if Knigge.OTP.release() >= 21 do + @doc """ + Returns whether or not this context is considered an error. + + Can be used in guards. + + ## Examples + + iex> require #{module} + iex> context = %#{module}{error: nil} + iex> #{module}.is_error(context) + false + + iex> require #{module} + iex> context = %#{module}{error: :some_error} + iex> #{module}.is_error(context) + true + """ + @spec is_error(t()) :: boolean() + defguard is_error(context) + when :erlang.is_map_key(:__struct__, context) and + :erlang.map_get(:__struct__, context) == __MODULE__ and + :erlang.is_map_key(:error, context) and + :erlang.map_get(:error, context) != nil + + @doc """ + Returns whether or not this context is considered an error. + + Uses `is_error/1`. + """ + @spec error?(t()) :: boolean() + def error?(context), do: is_error(context) + else + @doc """ + Returns whether or not this context is considered an error. + + ## Examples + + iex> context = %#{module}{error: nil} + iex> #{module}.error?(context) + false + + iex> context = %#{module}{error: :some_error} + iex> #{module}.error?(context) + true + """ + @spec error?(t()) :: boolean() + def error?(%__MODULE__{error: error}), do: not is_nil(error) + end end diff --git a/lib/mix/tasks/knigge/verify.ex b/lib/mix/tasks/knigge/verify.ex index d807b45..e95cbf1 100644 --- a/lib/mix/tasks/knigge/verify.ex +++ b/lib/mix/tasks/knigge/verify.ex @@ -143,8 +143,18 @@ defmodule Mix.Tasks.Knigge.Verify do exit_with({:error, :missing_modules}) end - defp exit_with(%Context{} = context) when Context.is_error(context) do - exit_with({:error, context.error}) + if Knigge.OTP.release() >= 21 do + defp exit_with(%Context{} = context) when Context.is_error(context) do + exit_with({:error, context.error}) + end + else + defp exit_with(%Context{} = context) do + if Context.error?(context) do + exit_with({:error, context.error}) + else + :ok + end + end end @exit_codes %{unknown_app: 1, missing_modules: 2} From dfdc2a6bffa6c516e27cc38c07bc7f785490b48f Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 15:26:49 +0100 Subject: [PATCH 34/36] Knigge: Add the `mix knigge.verify` section also the moduledoc --- lib/knigge.ex | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/knigge.ex b/lib/knigge.ex index f0abd13..cc6b100 100644 --- a/lib/knigge.ex +++ b/lib/knigge.ex @@ -141,6 +141,16 @@ defmodule Knigge do For further information about options check the `Knigge.Options` module. + ## Verifying your Implementations - `mix knigge.verify` + + Before version 1.2.0 `Knigge` tried to check at compile time if the implementation of your facade existed. + Due to the way the Elixir compiler goes about compiling your modules this didn't work as expected - [checkout this page if you're interested in the details](https://hexdocs.pm/knigge/the-existence-check.html). + + As an alternative `Knigge` now offers the `mix knigge.verify` task which verifies that the implementation modules of your facades actually exist. + The task returns with an error code when an implementation is missing, which allows you to plug it into your CI pipeline - for example as `MIX_ENV=prod mix knigge.verify`. + + For details check the documentation of `mix knigge.verify` by running `mix help knigge.verify`. + ## Knigge and the `:test` environment To give the maximum amount of flexibility `Knigge` delegates at runtime in your From 9734606050e1ac518ef97a6d9b276a3f43089d18 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 15:27:13 +0100 Subject: [PATCH 35/36] README: Add some more badges --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 1c85846..61e3aa4 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,9 @@ [![Build Status](https://travis-ci.org/sascha-wolf/knigge.svg?branch=master)](https://travis-ci.org/sascha-wolf/knigge) [![Coverage Status](https://coveralls.io/repos/github/sascha-wolf/knigge/badge.svg?branch=master)](https://coveralls.io/github/sascha-wolf/knigge?branch=master) [![Inline docs](https://inch-ci.org/github/sascha-wolf/knigge.svg?branch=master)](https://inch-ci.org/github/sascha-wolf/knigge) +[![Hexdocs.pm](https://img.shields.io/badge/hexdocs-online-blue)](https://hexdocs.pm/knigge/) [![Hex.pm](https://img.shields.io/hexpm/v/knigge.svg)](https://hex.pm/packages/knigge) +[![Hex.pm Downloads](https://img.shields.io/hexpm/dt/knigge)](https://hex.pm/packages/knigge) [![Featured - ElixirRadar](https://img.shields.io/badge/featured-ElixirRadar-543A56)](https://app.rdstation.com.br/mail/0ddee1c8-2ce9-405b-b95f-09c883099090?utm_campaign=elixir_radar_202&utm_medium=email&utm_source=RD+Station) [![Featured - ElixirWeekly](https://img.shields.io/badge/featured-ElixirWeekly-875DB0)](https://elixirweekly.net/issues/161) From 027790f9b8ed09adcf2a6719d69e857b058e8b66 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Fri, 6 Mar 2020 15:27:27 +0100 Subject: [PATCH 36/36] Project: Bump version from 1.1.1 to 1.2.0 --- version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version b/version index 524cb55..26aaba0 100644 --- a/version +++ b/version @@ -1 +1 @@ -1.1.1 +1.2.0