From 46fc43a8cc02fa2177db62ed743dcd4effea3db0 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Thu, 20 Jun 2024 09:37:06 -0400 Subject: [PATCH] improvement: use warnings instead of errors for better UX improvement: move proejct related things to `Project` namespace --- documentation/writing-generators.md | 2 +- lib/install.ex | 2 +- lib/{ => project}/application.ex | 25 +++++++-- lib/{ => project}/config.ex | 79 +++++++++++++++++++++++++---- lib/{ => project}/deps.ex | 9 +++- lib/{ => project}/formatter.ex | 20 ++++++-- test/{ => project}/config_test.exs | 75 ++++++++++++++++++--------- 7 files changed, 165 insertions(+), 47 deletions(-) rename lib/{ => project}/application.ex (83%) rename lib/{ => project}/config.ex (82%) rename lib/{ => project}/deps.ex (95%) rename lib/{ => project}/formatter.ex (81%) rename test/{ => project}/config_test.exs (70%) diff --git a/documentation/writing-generators.md b/documentation/writing-generators.md index 8c605f2..299c258 100644 --- a/documentation/writing-generators.md +++ b/documentation/writing-generators.md @@ -20,7 +20,7 @@ defmodule Mix.Tasks.YourLib.Gen.YourThing do ...some_code end """) - |> Igniter.Config.configure( + |> Igniter.Project.Config.configure( "config.exs", app_name, [:list_of_things], diff --git a/lib/install.ex b/lib/install.ex index 28c450a..9fecd6e 100644 --- a/lib/install.ex +++ b/lib/install.ex @@ -43,7 +43,7 @@ defmodule Igniter.Install do {igniter, [install | install_list]} else - {Igniter.Deps.add_dependency(igniter, install, requirement, + {Igniter.Project.Deps.add_dependency(igniter, install, requirement, error?: true, yes?: "--yes" in argv ), [install | install_list]} diff --git a/lib/application.ex b/lib/project/application.ex similarity index 83% rename from lib/application.ex rename to lib/project/application.ex index f907213..13a0212 100644 --- a/lib/application.ex +++ b/lib/project/application.ex @@ -1,4 +1,4 @@ -defmodule Igniter.Application do +defmodule Igniter.Project.Application do @moduledoc "Codemods and tools for working with Application modules." require Igniter.Code.Common @@ -83,11 +83,14 @@ defmodule Igniter.Application do zipper |> Zipper.down() |> Zipper.rightmost() - |> Igniter.Code.List.append_new_to_list(to_supervise, diff_checker) + |> Igniter.Code.List.append_new_to_list(Macro.escape(to_supervise), diff_checker) else _ -> - {:error, - "Expected `#{path}` to be an Application with a `start` function that has a `children = [...]` assignment"} + {:warning, + """ + Could not find a `children = [...]` assignment in the `start` function of the `#{application}` module. + Please ensure that #{inspect(to_supervise)} is added started by the application `#{application}` manually. + """} end end) end @@ -137,7 +140,19 @@ defmodule Igniter.Application do end _ -> - {:error, "Required a module using `Mix.Project` to exist in `mix.exs`"} + {:warning, + """ + No module in `mix.exs` using `Mix.Project` was found. Please update your `mix.exs` + to point to the application module `#{inspect(application)}`. + + For example: + + def application do + [ + mod: {#{inspect(application)}, []} + ] + end + """} end end) end diff --git a/lib/config.ex b/lib/project/config.ex similarity index 82% rename from lib/config.ex rename to lib/project/config.ex index 142ad73..0b6f26b 100644 --- a/lib/config.ex +++ b/lib/project/config.ex @@ -1,26 +1,47 @@ -defmodule Igniter.Config do +defmodule Igniter.Project.Config do @moduledoc "Codemods and utilities for modifying Elixir config files." require Igniter.Code.Function alias Igniter.Code.Common alias Sourceror.Zipper - @doc "Sets a config value in the given configuration file, if it is not already set." - @spec configure_new(Igniter.t(), Path.t(), atom(), list(atom), term()) :: Igniter.t() - def configure_new(igniter, file_path, app_name, config_path, value) do - configure(igniter, file_path, app_name, config_path, value, &{:ok, &1}) + @doc """ + Sets a config value in the given configuration file, if it is not already set. + + ## Opts + + * `failure_message` - A message to display to the user if the configuration change is unsuccessful. + """ + @spec configure_new(Igniter.t(), Path.t(), atom(), list(atom), term(), opts :: Keyword.t()) :: + Igniter.t() + def configure_new(igniter, file_path, app_name, config_path, value, opts \\ []) do + configure( + igniter, + file_path, + app_name, + config_path, + value, + Keyword.put(opts, :updater, &{:ok, &1}) + ) end - @doc "Sets a config value in the given configuration file, updating it with `updater` if it is already set." + @doc """ + Sets a config value in the given configuration file, updating it with `updater` if it is already set. + + ## Opts + + * `:updater` - A function that takes a zipper at a currently configured value and returns a new zipper with the value updated. + * `failure_message` - A message to display to the user if the configuration change is unsuccessful. + """ @spec configure( Igniter.t(), Path.t(), atom(), list(atom), term(), - (Zipper.t() -> {:ok, Zipper.t()} | :error) | nil + opts :: Keyword.t() ) :: Igniter.t() - def configure(igniter, file_name, app_name, config_path, value, updater \\ nil) do + def configure(igniter, file_name, app_name, config_path, value, opts \\ []) do file_contents = "import Config\n" file_path = Path.join("config", file_name) @@ -32,7 +53,7 @@ defmodule Igniter.Config do value -> Macro.escape(value) end - updater = updater || fn zipper -> {:ok, Common.replace_code(zipper, value)} end + updater = opts[:updater] || fn zipper -> {:ok, Common.replace_code(zipper, value)} end igniter |> ensure_default_configs_exist(file_name) @@ -49,7 +70,7 @@ defmodule Igniter.Config do false end) do nil -> - {:error, "No call to `import Config` found in configuration file"} + {:warning, bad_config_message(app_name, file_path, config_path, value, opts)} zipper -> modify_configuration_code(zipper, config_path, app_name, value, updater) @@ -79,6 +100,44 @@ defmodule Igniter.Config do """) end + defp bad_config_message(app_name, file_path, config_path, value, opts) do + path = + config_path + |> keywordify(value) + + code = + quote do + config unquote(app_name), unquote(path) + end + + message = + if opts[:failure_message] do + """ + + + #{opts[:failure_message]} + """ + else + "" + end + + or_update = + if opts[:updater] do + " or update" + else + "" + end + + """ + Please set#{or_update} the following config in #{file_path}: + + #{Macro.to_string(code)}#{String.trim_trailing(message)} + """ + end + + defp keywordify([], value), do: value + defp keywordify([key | rest], value), do: [{key, keywordify(rest, value)}] + @doc """ Modifies elixir configuration code starting at the configured zipper. diff --git a/lib/deps.ex b/lib/project/deps.ex similarity index 95% rename from lib/deps.ex rename to lib/project/deps.ex index 1a932d2..a377ba5 100644 --- a/lib/deps.ex +++ b/lib/project/deps.ex @@ -1,4 +1,4 @@ -defmodule Igniter.Deps do +defmodule Igniter.Project.Deps do @moduledoc "Codemods and utilities for managing dependencies declared in mix.exs" require Igniter.Code.Common alias Igniter.Code.Common @@ -100,7 +100,12 @@ defmodule Igniter.Deps do Igniter.Code.List.remove_index(zipper, current_declaration_index) else _ -> - {:error, "Failed to remove dependency #{inspect(name)}"} + {:warning, + """ + Failed to remove dependency #{inspect(name)} from `mix.exs`. + + Please remove the old dependency manually. + """} end end) end diff --git a/lib/formatter.ex b/lib/project/formatter.ex similarity index 81% rename from lib/formatter.ex rename to lib/project/formatter.ex index 324375b..3f5ad41 100644 --- a/lib/formatter.ex +++ b/lib/project/formatter.ex @@ -1,4 +1,4 @@ -defmodule Igniter.Formatter do +defmodule Igniter.Project.Formatter do @moduledoc "Codemods and utilities for interacting with `.formatter.exs` files" alias Igniter.Code.Common alias Sourceror.Zipper @@ -43,7 +43,14 @@ defmodule Igniter.Formatter do zipper :error -> - zipper + {:warning, + """ + Could not import dependency #{inspect(dep)} into `.formatter.exs`. + + Please add the import manually, i.e + + import_deps: [#{inspect(dep)}] + """} end end end) @@ -87,7 +94,14 @@ defmodule Igniter.Formatter do zipper _ -> - zipper + {:warning, + """ + Could not add formatter plugin #{inspect(plugin)} into `.formatter.exs`. + + Please add the import manually, i.e + + plugins: [#{inspect(plugin)}] + """} end end end) diff --git a/test/config_test.exs b/test/project/config_test.exs similarity index 70% rename from test/config_test.exs rename to test/project/config_test.exs index afd4b91..71c54df 100644 --- a/test/config_test.exs +++ b/test/project/config_test.exs @@ -1,4 +1,4 @@ -defmodule Igniter.ConfigTest do +defmodule Igniter.Project.ConfigTest do use ExUnit.Case alias Rewrite.Source @@ -6,7 +6,7 @@ defmodule Igniter.ConfigTest do describe "configure/6" do test "it creates the config file if it does not exist" do %{rewrite: rewrite} = - Igniter.Config.configure(Igniter.new(), "fake.exs", :fake, [:foo, :bar], "baz") + Igniter.Project.Config.configure(Igniter.new(), "fake.exs", :fake, [:foo, :bar], "baz") config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -26,7 +26,7 @@ defmodule Igniter.ConfigTest do config :fake, buz: [:blat] """) - |> Igniter.Config.configure("fake.exs", :fake, [:foo, :bar], "baz") + |> Igniter.Project.Config.configure("fake.exs", :fake, [:foo, :bar], "baz") config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -41,18 +41,20 @@ defmodule Igniter.ConfigTest do test "it merges the spark formatter plugins" do %{rewrite: rewrite} = Igniter.new() - |> Igniter.Config.configure( + |> Igniter.Project.Config.configure( "fake.exs", :spark, [:formatter, :"Ash.Resource"], [], - fn x -> + updater: fn x -> + x + end + ) + |> Igniter.Project.Config.configure("fake.exs", :spark, [:formatter, :"Ash.Domain"], [], + updater: fn x -> x end ) - |> Igniter.Config.configure("fake.exs", :spark, [:formatter, :"Ash.Domain"], [], fn x -> - x - end) config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -70,7 +72,7 @@ defmodule Igniter.ConfigTest do config :fake, buz: [:blat] """) - |> Igniter.Config.configure("fake.exs", :fake, [:foo], "baz") + |> Igniter.Project.Config.configure("fake.exs", :fake, [:foo], "baz") config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -87,7 +89,7 @@ defmodule Igniter.ConfigTest do |> Igniter.create_new_elixir_file("config/fake.exs", """ import Config """) - |> Igniter.Config.configure("fake.exs", :fake, [Foo.Bar, :bar], "baz") + |> Igniter.Project.Config.configure("fake.exs", :fake, [Foo.Bar, :bar], "baz") config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -103,8 +105,8 @@ defmodule Igniter.ConfigTest do |> Igniter.create_new_elixir_file("config/fake.exs", """ import Config """) - |> Igniter.Config.configure("fake.exs", :fake, [Foo.Bar, :bar], "baz") - |> Igniter.Config.configure("fake.exs", :fake, [Foo.Bar, :buz], "biz") + |> Igniter.Project.Config.configure("fake.exs", :fake, [Foo.Bar, :bar], "baz") + |> Igniter.Project.Config.configure("fake.exs", :fake, [Foo.Bar, :buz], "biz") config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -122,7 +124,7 @@ defmodule Igniter.ConfigTest do config :fake, :buz, [:blat] """) - |> Igniter.Config.configure("fake.exs", :fake, [:foo, :bar], "baz") + |> Igniter.Project.Config.configure("fake.exs", :fake, [:foo, :bar], "baz") config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -142,7 +144,7 @@ defmodule Igniter.ConfigTest do config :fake, :buz, [:blat] """) - |> Igniter.Config.configure("fake.exs", :fake, [:foo], "baz") + |> Igniter.Project.Config.configure("fake.exs", :fake, [:foo], "baz") config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -162,9 +164,11 @@ defmodule Igniter.ConfigTest do config :fake, :buz, [:blat] """) - |> Igniter.Config.configure("fake.exs", :fake, [:buz], "baz", fn list -> - Igniter.Code.List.prepend_new_to_list(list, "baz") - end) + |> Igniter.Project.Config.configure("fake.exs", :fake, [:buz], "baz", + updater: fn list -> + Igniter.Code.List.prepend_new_to_list(list, "baz") + end + ) config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -183,7 +187,7 @@ defmodule Igniter.ConfigTest do config :fake, :buz, [:blat] """) - |> Igniter.Config.configure("fake.exs", :fake, [:buz], 12) + |> Igniter.Project.Config.configure("fake.exs", :fake, [:buz], 12) config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -202,13 +206,15 @@ defmodule Igniter.ConfigTest do config :fake, foo: %{"a" => ["a", "b"]} """) - |> Igniter.Config.configure("fake.exs", :fake, [:foo], %{"b" => ["c", "d"]}, fn zipper -> - Igniter.Code.Map.set_map_key(zipper, "b", ["c", "d"], fn zipper -> - with {:ok, zipper} <- Igniter.Code.List.prepend_new_to_list(zipper, "c") do - Igniter.Code.List.prepend_new_to_list(zipper, "d") - end - end) - end) + |> Igniter.Project.Config.configure("fake.exs", :fake, [:foo], %{"b" => ["c", "d"]}, + updater: fn zipper -> + Igniter.Code.Map.set_map_key(zipper, "b", ["c", "d"], fn zipper -> + with {:ok, zipper} <- Igniter.Code.List.prepend_new_to_list(zipper, "c") do + Igniter.Code.List.prepend_new_to_list(zipper, "d") + end + end) + end + ) config_file = Rewrite.source!(rewrite, "config/fake.exs") @@ -218,5 +224,24 @@ defmodule Igniter.ConfigTest do config :fake, foo: %{"a" => ["a", "b"], "b" => ["c", "d"]} """ end + + test "it presents users with instructions on how to update a malformed config" do + %{warnings: [warning]} = + Igniter.new() + |> Igniter.create_new_elixir_file("config/fake.exs", """ + config :fake, foo: %{"a" => ["a", "b"]} + """) + |> Igniter.Project.Config.configure("fake.exs", :fake, [:foo], %{"b" => ["c", "d"]}, + failure_message: "A failure message!" + ) + + assert warning == """ + Please set the following config in config/fake.exs: + + config :fake, foo: %{"b" => ["c", "d"]} + + A failure message! + """ + end end end