From e43d35ee56ede84f356d36281452f91b0f2ea3fe Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Thu, 8 Feb 2018 16:11:52 +0100 Subject: [PATCH 1/4] Mix - Format: Add a failing test for .formatter.exs in a different folder The new test case builds on the "can read exported configuration from dependencies"-test. It tests that `mix format` can be called with `--dot-formatter ../.formatter.exs` when the `../.formatter.exs` contains `import_deps: [:my_dep]`. The `my_dep` dependency in turn can be found in `../deps/my_dep`. Right now this test fails. It seems like `mix format` tries to load the dependency from the workdir instead rather than from the same folder the provided `.formatter.exs` resides. TL;DR: mix format tries to load dependencies from the wrong folder, when passing a `.formatter.exs` in another folder --- lib/mix/test/mix/tasks/format_test.exs | 39 ++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/lib/mix/test/mix/tasks/format_test.exs b/lib/mix/test/mix/tasks/format_test.exs index 12b71dea779..731afae997b 100644 --- a/lib/mix/test/mix/tasks/format_test.exs +++ b/lib/mix/test/mix/tasks/format_test.exs @@ -219,6 +219,45 @@ defmodule Mix.Tasks.FormatTest do end end + test "can read exported configuration from dependencies with .formatter.exs in a different folder", context do + Mix.Project.push(__MODULE__.FormatWithDepsApp) + + in_tmp context.test, fn -> + File.write!(".formatter.exs", """ + [import_deps: [:my_dep]] + """) + + File.mkdir_p!("deps/my_dep/") + + File.write!("deps/my_dep/.formatter.exs", """ + [export: [locals_without_parens: [my_fun: 2]]] + """) + + File.mkdir_p!("subfolder") + + File.cd!("subfolder", fn -> + File.write!("a.ex", """ + my_fun :foo, :bar + """) + + Mix.Tasks.Format.run(["--dot-formatter", "../.formatter.exs", "a.ex"]) + + assert File.read!("a.ex") == """ + my_fun :foo, :bar + """ + end) + + manifest_path = Path.join(Mix.Project.manifest_path(), "cached_formatter_deps") + assert File.regular?(manifest_path) + + # Let's check that the manifest gets updated if it's stale. + File.touch!(manifest_path, {{1970, 1, 1}, {0, 0, 0}}) + + Mix.Tasks.Format.run(["subfolder/a.ex"]) + assert File.stat!(manifest_path).mtime > {{1970, 1, 1}, {0, 0, 0}} + end + end + test "validates dependencies in :import_deps", context do Mix.Project.push(__MODULE__.FormatWithDepsApp) From dce4562e052b82919afdd6f1ef9fe2fdd47d768f Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Thu, 8 Feb 2018 16:30:23 +0100 Subject: [PATCH 2/4] Mix - Format: Evaluate the formatter options in the config files directory This change fetches the folder from the provided dot_formatter config file, or it returns nil. Furthermore it adds a `fetch_deps_opts(formatter_opts, in_directory: formatter_dir)` function. This function then uses a non nil `formatter_dir` to wrap the actual `fetch_deps_opts/1` call in a `File.cd!` call for the provided `formatter_dir`. Fixes #7327 --- lib/mix/lib/mix/tasks/format.ex | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index 131a1eb383f..8c1c619236f 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -126,8 +126,8 @@ defmodule Mix.Tasks.Format do def run(args) do {opts, args} = OptionParser.parse!(args, strict: @switches) - formatter_opts = eval_dot_formatter(opts) - formatter_opts = fetch_deps_opts(formatter_opts) + {formatter_dir, formatter_opts} = eval_dot_formatter(opts) + formatter_opts = fetch_deps_opts(formatter_opts, in_directory: formatter_dir) args |> expand_args(formatter_opts) @@ -138,8 +138,8 @@ defmodule Mix.Tasks.Format do defp eval_dot_formatter(opts) do case dot_formatter(opts) do - {:ok, dot_formatter} -> eval_file_with_keyword_list(dot_formatter) - :error -> [] + {:ok, dot_formatter} -> {Path.dirname(dot_formatter), eval_file_with_keyword_list(dot_formatter)} + :error -> {nil, []} end end @@ -153,6 +153,16 @@ defmodule Mix.Tasks.Format do # This function reads exported configuration from the imported dependencies and deals with # caching the result of reading such configuration in a manifest file. + defp fetch_deps_opts(formatter_opts, in_directory: nil) do + fetch_deps_opts(formatter_opts) + end + + defp fetch_deps_opts(formatter_opts, in_directory: formatter_dir) do + File.cd!(formatter_dir, fn -> + fetch_deps_opts(formatter_opts) + end) + end + defp fetch_deps_opts(formatter_opts) do deps = Keyword.get(formatter_opts, :import_deps, []) From c077733eb8de5460520ceb0dac68490a3d21c582 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Thu, 8 Feb 2018 16:40:49 +0100 Subject: [PATCH 3/4] Format: Run mix format --- lib/mix/lib/mix/tasks/format.ex | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index 8c1c619236f..9c5c3a06d1b 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -138,8 +138,11 @@ defmodule Mix.Tasks.Format do defp eval_dot_formatter(opts) do case dot_formatter(opts) do - {:ok, dot_formatter} -> {Path.dirname(dot_formatter), eval_file_with_keyword_list(dot_formatter)} - :error -> {nil, []} + {:ok, dot_formatter} -> + {Path.dirname(dot_formatter), eval_file_with_keyword_list(dot_formatter)} + + :error -> + {nil, []} end end From 80c905f50e5b1bac8f3b45c6ffe081b9d46e6707 Mon Sep 17 00:00:00 2001 From: Sascha Wolf Date: Thu, 8 Feb 2018 17:12:50 +0100 Subject: [PATCH 4/4] Format: Run mix format properly on the changed test --- lib/mix/test/mix/tasks/format_test.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/mix/test/mix/tasks/format_test.exs b/lib/mix/test/mix/tasks/format_test.exs index 731afae997b..c90dd39dfdd 100644 --- a/lib/mix/test/mix/tasks/format_test.exs +++ b/lib/mix/test/mix/tasks/format_test.exs @@ -219,7 +219,8 @@ defmodule Mix.Tasks.FormatTest do end end - test "can read exported configuration from dependencies with .formatter.exs in a different folder", context do + test "can read exported configuration from dependencies with .formatter.exs in a different folder", + context do Mix.Project.push(__MODULE__.FormatWithDepsApp) in_tmp context.test, fn ->