diff --git a/examples/simple_umbrella_app/apps/app_a/lib/app_a_helper.ex b/examples/simple_umbrella_app/apps/app_a/lib/app_a_helper.ex new file mode 100644 index 00000000..585d7bd8 --- /dev/null +++ b/examples/simple_umbrella_app/apps/app_a/lib/app_a_helper.ex @@ -0,0 +1,9 @@ +defmodule AppAHelper do + @moduledoc """ + Documentation for `AppAHelper`. + """ + + def help do + :here_you_go + end +end diff --git a/examples/simple_umbrella_app/apps/app_b/lib/app_b_helper.ex b/examples/simple_umbrella_app/apps/app_b/lib/app_b_helper.ex new file mode 100644 index 00000000..0fae316d --- /dev/null +++ b/examples/simple_umbrella_app/apps/app_b/lib/app_b_helper.ex @@ -0,0 +1,9 @@ +defmodule AppBHelper do + @moduledoc """ + Documentation for `AppBHelper`. + """ + + def help do + :here_you_go + end +end diff --git a/examples/simple_umbrella_app/mix.lock b/examples/simple_umbrella_app/mix.lock index 7f878a50..e9e06476 100644 --- a/examples/simple_umbrella_app/mix.lock +++ b/examples/simple_umbrella_app/mix.lock @@ -1,3 +1,3 @@ %{ - "gradualizer": {:git, "https://github.com/josefs/Gradualizer.git", "9e629ade733780113973fe672a51d9650ed0cd86", [ref: "9e629ad"]}, + "gradualizer": {:git, "https://github.com/josefs/Gradualizer.git", "8b4bf33ffc5596142e690c5aa4f24f2458b9b0fd", [ref: "8b4bf33"]}, } diff --git a/lib/mix/tasks/gradient.ex b/lib/mix/tasks/gradient.ex index cb62e1ce..3408081c 100644 --- a/lib/mix/tasks/gradient.ex +++ b/lib/mix/tasks/gradient.ex @@ -7,7 +7,7 @@ defmodule Mix.Tasks.Gradient do ## Command-line options * `--no-compile` - do not compile even if needed - * `--no-ex-check` - do not perform checks specyfic for Elixir + * `--no-ex-check` - do not perform checks specific for Elixir (from ElixirChecker module) * `--no-gradualizer-check` - do not perform the Gradualizer checks * `--no-specify` - do not specify missing lines in AST what can @@ -19,9 +19,10 @@ defmodule Mix.Tasks.Gradient do * `--infer` - infer type information from literals and other language constructs, * `--verbose` - show what Gradualizer is doing + * `--print-filenames` - print the name of every file being analyzed (mainly useful for tests) * `--no-fancy` - do not use fancy error messages * `--fmt-location none` - do not display location for easier comparison - * `--fmt-location brief` - display location for machine processing + * `--fmt-location brief` - display location for machine processing * `--fmt-location verbose` - display location for human readers (default) * `--no-colors` - do not use colors in printed messages @@ -36,6 +37,8 @@ defmodule Mix.Tasks.Gradient do use Mix.Task + alias Gradient.ElixirFileUtils + @options [ # skip phases options no_compile: :boolean, @@ -48,6 +51,7 @@ defmodule Mix.Tasks.Gradient do stop_on_first_error: :boolean, infer: :boolean, verbose: :boolean, + print_filenames: :boolean, # formatter options no_fancy: :boolean, fmt_location: :string, @@ -70,7 +74,11 @@ defmodule Mix.Tasks.Gradient do # Compile the project before the analysis maybe_compile_project(options) # Get paths to files - files = get_paths(user_paths) + files = get_paths(user_paths) |> filter_enabled_paths() + + if options[:print_filenames] do + print_filenames(files) + end IO.puts("Typechecking files...") @@ -84,6 +92,32 @@ defmodule Mix.Tasks.Gradient do :ok end + defp print_filenames(files) do + IO.puts("Files to check:") + + cwd = File.cwd!() <> "/" + # Range for slicing off cwd from the beginning of a string + cwd_range = String.length(cwd)..-1 + + Enum.each(files, fn {app, filenames} -> + # Print app name + case app do + "apps/" <> appname -> IO.puts("Files in app #{appname}:") + # May be nil for non-umbrella apps + _ -> :noop + end + + # Print actual filenames + for filename <- filenames do + # Convert charlist to string + filename = to_string(filename) + # If the filename starts with the cwd, don't print the cwd + filename = if String.starts_with?(filename, cwd), do: String.slice(filename, cwd_range), else: filename + IO.puts(filename) + end + end) + end + defp execute(stream, opts) do res = if opts[:crash_on_error], do: stream, else: Enum.to_list(stream) @@ -213,4 +247,98 @@ defmodule Mix.Tasks.Gradient do defp compile_path(app_name) do Mix.Project.build_path() <> "/lib/" <> to_string(app_name) <> "/ebin" end + + @spec filter_enabled_paths(map()) :: map() + def filter_enabled_paths(apps_paths) do + apps_paths + |> Enum.map(fn {app_name, app_files} -> + {app_name, filter_paths_for_app(app_name, app_files)} + end) + |> Map.new() + end + + defp filter_paths_for_app(app_name, app_files) do + config = gradient_config_for_app(app_name) + + enabled? = Keyword.get(config, :enabled, true) + file_overrides_enabled? = Keyword.get(config, :file_overrides, enabled?) + + if file_overrides_enabled? do + magic_comment = + if enabled?, do: "# gradient:disable-for-file", else: "# gradient:enable-for-file" + + filter_files_with_magic_comment(app_name, app_files, magic_comment, not enabled?) + else + if enabled?, do: app_files, else: [] + end + end + + defp gradient_config_for_app(app) do + app_config = mix_config_for_app(app)[:gradient] || [] + + if Mix.Project.umbrella?() do + # Merge in config from umbrella app + umbrella_config = Mix.Project.config()[:gradient] || [] + Keyword.merge(umbrella_config, app_config) + else + app_config + end + end + + defp path_for_app(nil), do: "" + defp path_for_app(app_name), do: app_name <> "/" + + defp mix_config_for_app(nil), do: Mix.Project.config() + + defp mix_config_for_app(app) do + # Read the file looking for a "defmodule" to get the module name of the app's mixfile + mixfile_module_str = + File.stream!(path_for_app(app) <> "mix.exs") + |> Enum.find(&String.starts_with?(&1, "defmodule")) + |> (fn a -> Regex.run(~r/defmodule ([\w.]+)/, a) end).() + |> Enum.at(1) + + mixfile_module = String.to_atom("Elixir." <> mixfile_module_str) + + # return the module's config + mixfile_module.project() + end + + defp filter_files_with_magic_comment(app_name, beam_files, comment, should_include?) do + app_path = path_for_app(app_name) |> Path.expand() + deps_path = Path.expand(mix_config_for_app(app_name)[:deps_path] || "deps") + + Enum.filter(beam_files, fn beam_path -> + # Given the path to a .beam file, find out the .ex source file path + ex_path = + ex_filename_from_beam(beam_path) + # Turn the relative path into absolute + |> Path.expand(app_path) + + # Filter out any paths representing code generated by deps + is_dep = String.starts_with?(ex_path, deps_path) + + # Filter out the files with the magic comment + has_magic_comment? = + File.stream!(ex_path) + |> Enum.any?(fn line -> String.trim(line) == comment end) + + # Negate has_magic_comment? if should_include? is false + has_magic_comment? = if should_include?, do: has_magic_comment?, else: not has_magic_comment? + + has_magic_comment? and not is_dep + end) + end + + defp ex_filename_from_beam(beam_path) do + case ElixirFileUtils.get_forms(beam_path) do + {:ok, [{:attribute, _, :file, {filename, _}} | _]} -> + # Convert the *.beam compiled filename to its corresponding *.ex source file + # (it's a charlist initially so we pipe it through to_string) + filename |> to_string() + + error -> + raise "Error resolving .ex filename from compiled .beam filename #{inspect(beam_path)}: #{inspect(error)}" + end + end end diff --git a/test/mix/tasks/gradient_test.exs b/test/mix/tasks/gradient_test.exs index 39d0ca2c..4613a95c 100644 --- a/test/mix/tasks/gradient_test.exs +++ b/test/mix/tasks/gradient_test.exs @@ -1,5 +1,6 @@ defmodule Mix.Tasks.GradientTest do - use ExUnit.Case + # Async false since these tests use the file system + use ExUnit.Case, async: false import ExUnit.CaptureIO @@ -11,6 +12,23 @@ defmodule Mix.Tasks.GradientTest do @s_wrong_ret_beam Path.join(@build_path, "Elixir.SWrongRet.beam") @s_wrong_ret_ex Path.join([@examples_path, "type", "s_wrong_ret.ex"]) + # Run `mix deps.get` on umbrella project + setup_all do + previous_cwd = File.cwd!() + + try do + # cd to umbrella app dir + File.cd!("examples/simple_umbrella_app") + + # Get deps + {_, 0} = System.cmd("mix", ["deps.get"]) + after + File.cd!(previous_cwd) + end + + :ok + end + setup do Application.put_env(:gradient, :__system_halt__, fn signal -> send(self(), {:system_halt, signal}) @@ -215,7 +233,280 @@ defmodule Mix.Tasks.GradientTest do assert_receive {:system_halt, 1} end + describe "Umbrella project app filtering:" do + # Templates for dynamically creating variations on the umbrella/subapp + # mix.exs config files at runtime + @umbrella_mix_exs """ + defmodule SimpleUmbrellaApp.MixProject do + use Mix.Project + + def project do + [ + apps_path: "apps", + version: "0.1.0", + start_permanent: Mix.env() == :prod, + deps: deps(), + <%= if not no_config do %> + gradient: [ + <%= if enabled != nil do %> + enabled: <%= enabled %>, + <% end %> + + <%= if overrides != nil do %> + file_overrides: <%= overrides %>, + <% end %> + ] + <% end %> + ] + end + + defp deps do + [{:gradient, path: "../../", override: true}] + end + end + """ + + @subapp_mix_exs """ + defmodule App<%= String.upcase(app) %>.MixProject do + use Mix.Project + + def project do + [ + app: :app_<%= app %>, + version: "0.1.0", + build_path: "../../_build", + config_path: "../../config/config.exs", + deps_path: "../../deps", + lockfile: "../../mix.lock", + elixir: "~> 1.12", + start_permanent: Mix.env() == :prod, + deps: deps(), + <%= if not no_config do %> + gradient: [ + <%= if enabled != nil do %> + enabled: <%= enabled %>, + <% end %> + + <%= if overrides != nil do %> + file_overrides: <%= overrides %>, + <% end %> + ] + <% end %> + ] + end + + <%= if app == "b" do %> + defp deps, do: [{:app_a, in_umbrella: true}] + <% else %> + defp deps, do: [] + <% end %> + end + """ + + @umbrella_app_path "examples/simple_umbrella_app" + @umbrella_mix @umbrella_app_path <> "/mix.exs" + @umbrella_app_a_mix @umbrella_app_path <> "/apps/app_a/mix.exs" + @umbrella_app_b_mix @umbrella_app_path <> "/apps/app_b/mix.exs" + + setup context do + umbrella_mix_contents = File.read!(@umbrella_mix) + app_a_mix_contents = File.read!(@umbrella_app_a_mix) + app_b_mix_contents = File.read!(@umbrella_app_b_mix) + + on_exit(fn -> + # Put back the original file contents at the end of the test + File.write!(@umbrella_mix, umbrella_mix_contents) + File.write!(@umbrella_app_a_mix, app_a_mix_contents) + File.write!(@umbrella_app_b_mix, app_b_mix_contents) + end) + + # Edit files if specified in config + context |> Map.get(:edit_files, %{}) |> edit_files() + + # Get test configs from context + umbrella_mix = Map.get(context, :umbrella, %{}) |> prep_config() + app_a_mix = Map.get(context, :app_a, %{}) |> Map.put(:app, "a") |> prep_config() + app_b_mix = Map.get(context, :app_b, %{}) |> Map.put(:app, "b") |> prep_config() + + # Write new mix.exs file contents for duration of test + File.write!(@umbrella_mix, EEx.eval_string(@umbrella_mix_exs, umbrella_mix)) + File.write!(@umbrella_app_a_mix, EEx.eval_string(@subapp_mix_exs, app_a_mix)) + File.write!(@umbrella_app_b_mix, EEx.eval_string(@subapp_mix_exs, app_b_mix)) + end + + # Merge in default configs, and convert map to keyword list + defp prep_config(mix_exs_config) do + Map.merge( + %{ + no_config: false, + enabled: nil, + overrides: nil + }, + mix_exs_config + ) + |> Keyword.new() + end + + defp edit_files(files) do + for {filename, enabled?} <- files do + filename = @umbrella_app_path <> "/apps/" <> filename + + magic_comment = + if enabled?, do: "# gradient:enable-for-file\n", else: "# gradient:disable-for-file\n" + + file_contents = File.read!(filename) + updated_file_contents = magic_comment <> file_contents + File.write!(filename, updated_file_contents) + + on_exit(fn -> + File.write!(filename, file_contents) + end) + end + end + + # Run the task on the umbrella app, and extract which files it ran on + defp run_task_and_return_files() do + output = run_shell_task("examples/simple_umbrella_app", ["--print-filenames"]) + + # Parse the output and figure out which files it said it ran on + output + |> String.split("\n") + |> Enum.reduce( + %{state: :not_started, apps_files: %{}, curr_app: nil}, + fn line, %{state: state, apps_files: apps_files, curr_app: curr_app} = acc -> + case state do + # Looking for the string "Typechecking files..." to start + :not_started -> + if line == "Files to check:" do + %{acc | state: :started} + else + acc + end + + :started -> + case line do + # Change current app + "Files in app " <> app_name_and_colon -> + # get rid of colon on the end + app_name = String.replace(app_name_and_colon, ":", "") + %{acc | curr_app: app_name} + + # This line signifies the end of the file list + "Typechecking files..." -> + %{acc | state: :finished} + + # Save filename to list for current app + filename -> + file_list_for_app = Map.get(apps_files, curr_app, []) + updated_file_list = file_list_for_app ++ [filename] + updated_apps_files = Map.put(apps_files, curr_app, updated_file_list) + %{acc | apps_files: updated_apps_files} + end + + # Already found all the files, no-op + :finished -> + acc + end + end + ) + # Return just the list of apps/files + |> Map.get(:apps_files) + end + + @tag umbrella: %{no_config: true}, app_a: %{no_config: true}, app_b: %{no_config: true} + test "defaults to enabled when no gradient config in any mix.exs files" do + assert %{ + "app_a" => [ + "_build/dev/lib/app_a/ebin/Elixir.AppA.beam", + "_build/dev/lib/app_a/ebin/Elixir.AppAHelper.beam" + ], + "app_b" => [ + "_build/dev/lib/app_b/ebin/Elixir.AppB.beam", + "_build/dev/lib/app_b/ebin/Elixir.AppBHelper.beam" + ] + } == run_task_and_return_files() + end + + @tag umbrella: %{enabled: true}, app_a: %{no_config: true}, app_b: %{no_config: true} + test "when gradient is enabled for umbrella, checks all files" do + assert %{ + "app_a" => [ + "_build/dev/lib/app_a/ebin/Elixir.AppA.beam", + "_build/dev/lib/app_a/ebin/Elixir.AppAHelper.beam" + ], + "app_b" => [ + "_build/dev/lib/app_b/ebin/Elixir.AppB.beam", + "_build/dev/lib/app_b/ebin/Elixir.AppBHelper.beam" + ] + } == run_task_and_return_files() + end + + @tag umbrella: %{enabled: false}, app_a: %{no_config: true}, app_b: %{no_config: true} + test "when gradient is disabled for umbrella, doesn't check any files" do + assert %{} == run_task_and_return_files() + end + + @tag umbrella: %{enabled: false}, app_a: %{enabled: true} + test "gradient can be enabled for a subapp even if disabled for umbrella" do + assert %{ + "app_a" => [ + "_build/dev/lib/app_a/ebin/Elixir.AppA.beam", + "_build/dev/lib/app_a/ebin/Elixir.AppAHelper.beam" + ] + } == run_task_and_return_files() + end + + @tag umbrella: %{enabled: true}, app_a: %{enabled: false} + test "gradient can be enabled for the umbrella and disabled for a subapp" do + assert %{ + "app_b" => [ + "_build/dev/lib/app_b/ebin/Elixir.AppB.beam", + "_build/dev/lib/app_b/ebin/Elixir.AppBHelper.beam" + ] + } == run_task_and_return_files() + end + + @tag umbrella: %{enabled: true, overrides: true}, + edit_files: %{"app_a/lib/app_a_helper.ex" => false} + test "individual files can be disabled" do + assert %{ + "app_a" => [ + "_build/dev/lib/app_a/ebin/Elixir.AppA.beam" + ], + "app_b" => [ + "_build/dev/lib/app_b/ebin/Elixir.AppB.beam", + "_build/dev/lib/app_b/ebin/Elixir.AppBHelper.beam" + ] + } == run_task_and_return_files() + end + + @tag umbrella: %{enabled: false, overrides: true}, edit_files: %{"app_a/lib/app_a.ex" => true} + test "individual files can be enabled" do + assert %{"app_a" => ["_build/dev/lib/app_a/ebin/Elixir.AppA.beam"]} == + run_task_and_return_files() + end + end + + # Run the task in the current process. Useful for running on a single file. def run_task(args), do: capture_io(fn -> Mix.Tasks.Gradient.run(args) end) - def test_opts(opts), do: ["--no-comile", "--no-deps"] ++ opts + # Run the task in a new process, via the shell. Useful for running on an entire project. + def run_shell_task(dir, args) do + previous_cwd = File.cwd!() + + try do + # cd to the specified dir + File.cd!(dir) + + # Run the task + {output, exit_code} = System.cmd("mix", ["gradient"] ++ args) + assert exit_code == 0, output + + output + after + File.cd!(previous_cwd) + end + end + + def test_opts(opts), do: ["--no-compile", "--no-deps"] ++ opts end