From fe6ffd601ac97f6637ff6d315aa014a435b96378 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Tue, 12 Dec 2017 13:14:08 +0100 Subject: [PATCH 01/10] Allow "mix format" to read exported configuration from dependencies --- lib/mix/lib/mix/tasks/format.ex | 119 ++++++++++++++++++++++++++++++-- 1 file changed, 113 insertions(+), 6 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index 9e1dfb8bba2..c3312d2e4cf 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -36,18 +36,26 @@ defmodule Mix.Tasks.Format do If any of the `--check-*` flags are given and a check fails, the formatted contents won't be written to disk nor printed to stdout. - ## .formatter.exs + ## `.formatter.exs` The formatter will read a `.formatter.exs` in the current directory for formatter configuration. It should return a keyword list with any of the options supported by `Code.format_string!/2`. - The `.formatter.exs` also supports an `:inputs` field which specifies the - default inputs to be used by this task: + The `.formatter.exs` also supports other options: - [ - inputs: ["mix.exs", "{config,lib,test}/**/*.{ex,exs}"] - ] + * `:input` (a list of paths and patterns) - specifies the default inputs + to be used by this task. For example, `["mix.exs", "{config,lib,test}/**/*.{ex,exs}"]`. + + * `:import_deps` (a list of dependencies as atoms) - specifies a list + of dependencies whose formatter configuration will be imported. + See the "Importing dependencies configuration" section below for more + information. + + * `:export_locals_without_parens` (a list of function names and arities) - + specifies a list of function names and arities (like `:locals_without_parens` + in `Code.format_string!/2`) that projects that depend on this project + can read and use. ## When to format code @@ -65,6 +73,41 @@ defmodule Mix.Tasks.Format do of patterns and files to `mix format`, as showed at the top of this task documentation. This list can also be set in the `.formatter.exs` under the `:inputs` key. + + ## Importing dependencies configuration + + This task supports importing formatter configuration from dependencies. + + A dependency that wants to export formatter configuration needs to have a + `.formatter.exs` file at the root of the project. In this file, the + dependency can export a `:export_locals_without_parens` option whose value + has the same shape as the value of the `:locals_without_parens` in + `Code.format_string!/2`. + + The functions listed under `:export_locals_without_parens` of a dependency + can be imported in a project by listing that dependency in the `:import_deps` + option of the formatter configuration file of the project. + + For example, consider I have a project `my_app` that depends on `my_dep`. + `my_dep` wants to export some configuration, so `my_dep/.formatter.exs` + would look like this: + + # my_dep/.formatter.exs + [ + # Regular formatter configuration for my_dep + # ... + + export_locals_without_parens: [some_dsl_call: :*] + ] + + In order to import configuration, `my_app`'s `.formatter.exs` would look like + this: + + # my_app/.formatter.exs + [ + import_deps: [:my_dep] + ] + """ @switches [ @@ -77,6 +120,7 @@ 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) args |> expand_args(formatter_opts) @@ -112,6 +156,69 @@ defmodule Mix.Tasks.Format do end end + # 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) do + case formatter_opts[:import_deps] do + deps when deps in [nil, []] -> + formatter_opts + + deps when is_list(deps) and deps != [] -> + dep_parenless_calls = + if deps_dot_formatters_stale?() do + dep_parenless_calls = eval_deps_opts(deps) + + if dep_parenless_calls != [] do + write_deps_manifest(dep_parenless_calls) + end + + dep_parenless_calls + else + read_deps_manifest() + end + + Keyword.update( + formatter_opts, + :locals_without_parens, + dep_parenless_calls, + &(&1 ++ dep_parenless_calls) + ) + + other -> + Mix.raise( + "Expected :import_deps to return a list of dependencies, got: #{inspect(other)}" + ) + end + end + + defp deps_dot_formatters_stale?() do + Mix.Utils.stale?([".formatter.exs" | Mix.Project.config_files()], [deps_manifest()]) + end + + defp deps_manifest() do + Path.join(Mix.Project.build_path(), ".cached_deps_formatter.exs") + end + + defp read_deps_manifest() do + deps_manifest() |> File.read!() |> :erlang.binary_to_term() + end + + defp write_deps_manifest(parenless_calls) do + File.write!(deps_manifest(), :erlang.term_to_binary(parenless_calls)) + end + + defp eval_deps_opts(deps) do + deps_paths = Mix.Project.deps_paths() + + for dep <- deps, + dep_dot_formatter = Path.join(Map.fetch!(deps_paths, dep), ".formatter.exs"), + File.regular?(dep_dot_formatter), + {dep_opts, _} = Code.eval_file(dep_dot_formatter), + parenless_call <- Keyword.get(dep_opts, :export_locals_without_parens, []), + uniq: true, + do: parenless_call + end + defp expand_args([], formatter_opts) do if inputs = formatter_opts[:inputs] do expand_files_and_patterns(List.wrap(inputs), ".formatter.exs") From ae2ae207a106048b1ff818a159f319b8db7ce5d3 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Tue, 12 Dec 2017 17:38:19 +0100 Subject: [PATCH 02/10] case -> cond --- lib/mix/lib/mix/tasks/format.ex | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index c3312d2e4cf..e1daaafdea2 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -159,11 +159,13 @@ 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) do - case formatter_opts[:import_deps] do - deps when deps in [nil, []] -> + deps = formatter_opts[:import_deps] + + cond do + is_nil(deps) or deps == [] -> formatter_opts - deps when is_list(deps) and deps != [] -> + is_list(deps) -> dep_parenless_calls = if deps_dot_formatters_stale?() do dep_parenless_calls = eval_deps_opts(deps) @@ -184,10 +186,8 @@ defmodule Mix.Tasks.Format do &(&1 ++ dep_parenless_calls) ) - other -> - Mix.raise( - "Expected :import_deps to return a list of dependencies, got: #{inspect(other)}" - ) + true -> + Mix.raise("Expected :import_deps to return a list of dependencies, got: #{inspect(deps)}") end end From 1766a3c083b54446f73a3cb8d0e90d5d8fbd54a9 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 13 Dec 2017 10:00:09 +0100 Subject: [PATCH 03/10] Add a test --- .../format_with_deps/deps/my_dep/.gitkeep | 0 lib/mix/test/mix/tasks/format_test.exs | 51 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 lib/mix/test/fixtures/format_with_deps/deps/my_dep/.gitkeep diff --git a/lib/mix/test/fixtures/format_with_deps/deps/my_dep/.gitkeep b/lib/mix/test/fixtures/format_with_deps/deps/my_dep/.gitkeep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/mix/test/mix/tasks/format_test.exs b/lib/mix/test/mix/tasks/format_test.exs index 83a3d87245c..58cd8a2091c 100644 --- a/lib/mix/test/mix/tasks/format_test.exs +++ b/lib/mix/test/mix/tasks/format_test.exs @@ -174,6 +174,57 @@ defmodule Mix.Tasks.FormatTest do end end + defmodule FormatWithDepsApp do + def project do + [ + app: :format_with_deps, + version: "0.1.0", + deps: [{:my_dep, "0.1.0", path: "deps/my_dep"}] + ] + end + end + + test "can read exported configuration from dependencies" do + Mix.Project.push(__MODULE__.FormatWithDepsApp) + + in_fixture "format_with_deps", fn -> + Mix.Project.build_structure() + + File.write!(".formatter.exs", """ + [import_deps: [:my_dep]] + """) + + File.write!("a.ex", """ + my_fun :foo, :bar + """) + + File.mkdir_p!("deps/my_dep/") + + File.write!("deps/my_dep/.formatter.exs", """ + [export_locals_without_parens: [my_fun: 2]] + """) + + Mix.Tasks.Format.run(["a.ex"]) + + assert File.read!("a.ex") == """ + my_fun :foo, :bar + """ + + manifest_path = "_build/dev/.cached_deps_formatter.exs" + assert File.regular?(manifest_path) + + # Let's check that the manifest gets updated if .formatter.exs changes. + File.touch!(manifest_path, {{1970, 1, 1}, {0, 0, 0}}) + %File.Stat{mtime: mtime} = File.stat!(manifest_path) + + File.touch!(".formatter.exs") + Mix.Tasks.Format.run(["a.ex"]) + %File.Stat{mtime: new_mtime} = File.stat!(manifest_path) + + assert new_mtime > mtime + end + end + test "raises on invalid arguments", context do in_tmp context.test, fn -> assert_raise Mix.Error, ~r"Expected one or more files\/patterns to be given", fn -> From 9b2ef682147c4673892b89efc078ac49884714cb Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 13 Dec 2017 10:02:31 +0100 Subject: [PATCH 04/10] Fix a logic error --- lib/mix/lib/mix/tasks/format.ex | 2 ++ lib/mix/test/mix/tasks/format_test.exs | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index e1daaafdea2..9e1a784a8db 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -204,6 +204,8 @@ defmodule Mix.Tasks.Format do end defp write_deps_manifest(parenless_calls) do + manifest = deps_manifest() + File.mkdir_p!(Path.dirname(manifest)) File.write!(deps_manifest(), :erlang.term_to_binary(parenless_calls)) end diff --git a/lib/mix/test/mix/tasks/format_test.exs b/lib/mix/test/mix/tasks/format_test.exs index 58cd8a2091c..b35c0438fcf 100644 --- a/lib/mix/test/mix/tasks/format_test.exs +++ b/lib/mix/test/mix/tasks/format_test.exs @@ -188,8 +188,6 @@ defmodule Mix.Tasks.FormatTest do Mix.Project.push(__MODULE__.FormatWithDepsApp) in_fixture "format_with_deps", fn -> - Mix.Project.build_structure() - File.write!(".formatter.exs", """ [import_deps: [:my_dep]] """) From ff7aecb35d59621e0fe932eaaa3311ab1f054863 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 13 Dec 2017 10:07:23 +0100 Subject: [PATCH 05/10] Implement Eric's feedback --- lib/mix/test/fixtures/format_with_deps/deps/my_dep/.gitkeep | 0 lib/mix/test/mix/tasks/format_test.exs | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 lib/mix/test/fixtures/format_with_deps/deps/my_dep/.gitkeep diff --git a/lib/mix/test/fixtures/format_with_deps/deps/my_dep/.gitkeep b/lib/mix/test/fixtures/format_with_deps/deps/my_dep/.gitkeep deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/lib/mix/test/mix/tasks/format_test.exs b/lib/mix/test/mix/tasks/format_test.exs index b35c0438fcf..b86222f0b28 100644 --- a/lib/mix/test/mix/tasks/format_test.exs +++ b/lib/mix/test/mix/tasks/format_test.exs @@ -184,10 +184,10 @@ defmodule Mix.Tasks.FormatTest do end end - test "can read exported configuration from dependencies" do + test "can read exported configuration from dependencies", context do Mix.Project.push(__MODULE__.FormatWithDepsApp) - in_fixture "format_with_deps", fn -> + in_tmp context.test, fn -> File.write!(".formatter.exs", """ [import_deps: [:my_dep]] """) From 5faa656527dca9ded155a95653481ff5700386cc Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 13 Dec 2017 11:49:58 +0100 Subject: [PATCH 06/10] Fix minor things --- lib/mix/lib/mix/tasks/format.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index 9e1a784a8db..c084beba92b 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -44,7 +44,7 @@ defmodule Mix.Tasks.Format do The `.formatter.exs` also supports other options: - * `:input` (a list of paths and patterns) - specifies the default inputs + * `:inputs` (a list of paths and patterns) - specifies the default inputs to be used by this task. For example, `["mix.exs", "{config,lib,test}/**/*.{ex,exs}"]`. * `:import_deps` (a list of dependencies as atoms) - specifies a list @@ -97,7 +97,7 @@ defmodule Mix.Tasks.Format do # Regular formatter configuration for my_dep # ... - export_locals_without_parens: [some_dsl_call: :*] + export_locals_without_parens: [some_dsl_call: 2, some_dsl_call: 3] ] In order to import configuration, `my_app`'s `.formatter.exs` would look like From b1e0f6aca0fb3f2f51afcdb629b826665310f9f5 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 13 Dec 2017 12:25:30 +0100 Subject: [PATCH 07/10] Fix a bunch of things --- lib/mix/lib/mix/tasks/format.ex | 83 ++++++++++++++------------ lib/mix/test/mix/tasks/format_test.exs | 40 +++++++++---- 2 files changed, 74 insertions(+), 49 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index c084beba92b..1ab253c0bcc 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -131,20 +131,8 @@ defmodule Mix.Tasks.Format do defp eval_dot_formatter(opts) do case dot_formatter(opts) do - {:ok, dot_formatter} -> - {formatter_opts, _} = Code.eval_file(dot_formatter) - - unless Keyword.keyword?(formatter_opts) do - Mix.raise( - "Expected #{inspect(dot_formatter)} to return a keyword list, " <> - "got: #{inspect(formatter_opts)}" - ) - end - - formatter_opts - - :error -> - [] + {:ok, dot_formatter} -> eval_file_with_keyword_list(dot_formatter) + :error -> [] end end @@ -159,24 +147,24 @@ 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) do - deps = formatter_opts[:import_deps] + deps = Keyword.get(formatter_opts, :import_deps, []) cond do - is_nil(deps) or deps == [] -> + deps == [] -> formatter_opts is_list(deps) -> + # Since we have dependencies listed, we write the manifest even if those dependencies + # don't export anything so that we avoid lookups everytime. + deps_manifest = Path.join(Mix.Project.manifest_path(), ".cached_deps_formatter") + dep_parenless_calls = - if deps_dot_formatters_stale?() do + if deps_dot_formatters_stale?(deps_manifest) do dep_parenless_calls = eval_deps_opts(deps) - - if dep_parenless_calls != [] do - write_deps_manifest(dep_parenless_calls) - end - + write_deps_manifest(deps_manifest, dep_parenless_calls) dep_parenless_calls else - read_deps_manifest() + read_deps_manifest(deps_manifest) end Keyword.update( @@ -191,36 +179,57 @@ defmodule Mix.Tasks.Format do end end - defp deps_dot_formatters_stale?() do - Mix.Utils.stale?([".formatter.exs" | Mix.Project.config_files()], [deps_manifest()]) - end - - defp deps_manifest() do - Path.join(Mix.Project.build_path(), ".cached_deps_formatter.exs") + defp deps_dot_formatters_stale?(deps_manifest) do + Mix.Utils.stale?([".formatter.exs" | Mix.Project.config_files()], [deps_manifest]) end - defp read_deps_manifest() do - deps_manifest() |> File.read!() |> :erlang.binary_to_term() + defp read_deps_manifest(deps_manifest) do + deps_manifest |> File.read!() |> :erlang.binary_to_term() end - defp write_deps_manifest(parenless_calls) do - manifest = deps_manifest() - File.mkdir_p!(Path.dirname(manifest)) - File.write!(deps_manifest(), :erlang.term_to_binary(parenless_calls)) + defp write_deps_manifest(deps_manifest, parenless_calls) do + File.mkdir_p!(Path.dirname(deps_manifest)) + File.write!(deps_manifest, :erlang.term_to_binary(parenless_calls)) end defp eval_deps_opts(deps) do deps_paths = Mix.Project.deps_paths() for dep <- deps, + ensure_valid_dependency!(dep, deps_paths), dep_dot_formatter = Path.join(Map.fetch!(deps_paths, dep), ".formatter.exs"), File.regular?(dep_dot_formatter), - {dep_opts, _} = Code.eval_file(dep_dot_formatter), - parenless_call <- Keyword.get(dep_opts, :export_locals_without_parens, []), + dep_opts = eval_file_with_keyword_list(dep_dot_formatter), + parenless_call <- dep_opts[:export][:locals_without_parens] || [], uniq: true, do: parenless_call end + defp ensure_valid_dependency!(dep, deps_paths) do + cond do + not is_atom(dep) -> + Mix.raise("Dependencies in :import_deps should be atoms, got: #{inspect(dep)}") + + not Map.has_key?(deps_paths, dep) -> + Mix.raise( + "Found a dependency in :import_deps that the project doesn't depend on: #{inspect(dep)}" + ) + + true -> + :ok + end + end + + defp eval_file_with_keyword_list(path) do + {opts, _} = Code.eval_file(path) + + unless Keyword.keyword?(opts) do + Mix.raise("Expected #{inspect(path)} to return a keyword list, got: #{inspect(opts)}") + end + + opts + end + defp expand_args([], formatter_opts) do if inputs = formatter_opts[:inputs] do expand_files_and_patterns(List.wrap(inputs), ".formatter.exs") diff --git a/lib/mix/test/mix/tasks/format_test.exs b/lib/mix/test/mix/tasks/format_test.exs index b86222f0b28..c4d91c80e72 100644 --- a/lib/mix/test/mix/tasks/format_test.exs +++ b/lib/mix/test/mix/tasks/format_test.exs @@ -5,6 +5,16 @@ defmodule Mix.Tasks.FormatTest do import ExUnit.CaptureIO + defmodule FormatWithDepsApp do + def project do + [ + app: :format_with_deps, + version: "0.1.0", + deps: [{:my_dep, "0.1.0", path: "deps/my_dep"}] + ] + end + end + test "formats the given files", context do in_tmp context.test, fn -> File.write!("a.ex", """ @@ -174,16 +184,6 @@ defmodule Mix.Tasks.FormatTest do end end - defmodule FormatWithDepsApp do - def project do - [ - app: :format_with_deps, - version: "0.1.0", - deps: [{:my_dep, "0.1.0", path: "deps/my_dep"}] - ] - end - end - test "can read exported configuration from dependencies", context do Mix.Project.push(__MODULE__.FormatWithDepsApp) @@ -199,7 +199,7 @@ defmodule Mix.Tasks.FormatTest do File.mkdir_p!("deps/my_dep/") File.write!("deps/my_dep/.formatter.exs", """ - [export_locals_without_parens: [my_fun: 2]] + [export: [locals_without_parens: [my_fun: 2]]] """) Mix.Tasks.Format.run(["a.ex"]) @@ -208,7 +208,7 @@ defmodule Mix.Tasks.FormatTest do my_fun :foo, :bar """ - manifest_path = "_build/dev/.cached_deps_formatter.exs" + manifest_path = Path.join(Mix.Project.manifest_path(), ".cached_deps_formatter") assert File.regular?(manifest_path) # Let's check that the manifest gets updated if .formatter.exs changes. @@ -223,6 +223,22 @@ defmodule Mix.Tasks.FormatTest do end end + test "validates dependencies in :import_deps", context do + Mix.Project.push(__MODULE__.FormatWithDepsApp) + + in_tmp context.test, fn -> + File.write!(".formatter.exs", """ + [import_deps: [:nonexistent_dep]] + """) + + message = + "Found a dependency in :import_deps that the project doesn't " <> + "depend on: :nonexistent_dep" + + assert_raise Mix.Error, message, fn -> Mix.Tasks.Format.run([]) end + end + end + test "raises on invalid arguments", context do in_tmp context.test, fn -> assert_raise Mix.Error, ~r"Expected one or more files\/patterns to be given", fn -> From cd1b39a6ba671d38cedb0e0f41abc54da8435d08 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 13 Dec 2017 12:28:42 +0100 Subject: [PATCH 08/10] Update docs --- lib/mix/lib/mix/tasks/format.ex | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index 1ab253c0bcc..64e91800165 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -52,10 +52,8 @@ defmodule Mix.Tasks.Format do See the "Importing dependencies configuration" section below for more information. - * `:export_locals_without_parens` (a list of function names and arities) - - specifies a list of function names and arities (like `:locals_without_parens` - in `Code.format_string!/2`) that projects that depend on this project - can read and use. + * `:export` (a keyword list) - specifies formatter configuration to be exported. See the + "Importing dependencies configuration" section below. ## When to format code @@ -78,13 +76,13 @@ defmodule Mix.Tasks.Format do This task supports importing formatter configuration from dependencies. - A dependency that wants to export formatter configuration needs to have a - `.formatter.exs` file at the root of the project. In this file, the - dependency can export a `:export_locals_without_parens` option whose value - has the same shape as the value of the `:locals_without_parens` in - `Code.format_string!/2`. + A dependency that wants to export formatter configuration needs to have a `.formatter.exs` file + at the root of the project. In this file, the dependency can export a `:export` option with + configuration to export. For now, only one option is supported under `:export`: + `:export_locals_without_parens` (whose value has the same shape as the value of the + `:locals_without_parens` in `Code.format_string!/2`). - The functions listed under `:export_locals_without_parens` of a dependency + The functions listed under `:locals_without_parens` in the `:export` option of a dependency can be imported in a project by listing that dependency in the `:import_deps` option of the formatter configuration file of the project. @@ -97,7 +95,9 @@ defmodule Mix.Tasks.Format do # Regular formatter configuration for my_dep # ... - export_locals_without_parens: [some_dsl_call: 2, some_dsl_call: 3] + export: [ + locals_without_parens: [some_dsl_call: 2, some_dsl_call: 3] + ] ] In order to import configuration, `my_app`'s `.formatter.exs` would look like From 6cc113ab37df983dad2515ff795f33f472336b37 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 13 Dec 2017 13:04:25 +0100 Subject: [PATCH 09/10] =?UTF-8?q?Implement=20Jos=C3=A9's=20feedback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/mix/lib/mix/tasks/format.ex | 21 +++++++++++---------- lib/mix/test/mix/tasks/format_test.exs | 8 +++----- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index 64e91800165..036f2bfb6f6 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -196,8 +196,8 @@ defmodule Mix.Tasks.Format do deps_paths = Mix.Project.deps_paths() for dep <- deps, - ensure_valid_dependency!(dep, deps_paths), - dep_dot_formatter = Path.join(Map.fetch!(deps_paths, dep), ".formatter.exs"), + dep_path = assert_valid_dep_and_fetch_path(dep, deps_paths), + dep_dot_formatter = Path.join(dep_path, ".formatter.exs"), File.regular?(dep_dot_formatter), dep_opts = eval_file_with_keyword_list(dep_dot_formatter), parenless_call <- dep_opts[:export][:locals_without_parens] || [], @@ -205,21 +205,22 @@ defmodule Mix.Tasks.Format do do: parenless_call end - defp ensure_valid_dependency!(dep, deps_paths) do - cond do - not is_atom(dep) -> - Mix.raise("Dependencies in :import_deps should be atoms, got: #{inspect(dep)}") + defp assert_valid_dep_and_fetch_path(dep, deps_paths) when is_atom(dep) do + case Map.fetch(deps_paths, dep) do + {:ok, path} -> + path - not Map.has_key?(deps_paths, dep) -> + :error -> Mix.raise( "Found a dependency in :import_deps that the project doesn't depend on: #{inspect(dep)}" ) - - true -> - :ok end end + defp assert_valid_dep_and_fetch_path(dep, _deps_paths) do + Mix.raise("Dependencies in :import_deps should be atoms, got: #{inspect(dep)}") + end + defp eval_file_with_keyword_list(path) do {opts, _} = Code.eval_file(path) diff --git a/lib/mix/test/mix/tasks/format_test.exs b/lib/mix/test/mix/tasks/format_test.exs index c4d91c80e72..46aabcbfee4 100644 --- a/lib/mix/test/mix/tasks/format_test.exs +++ b/lib/mix/test/mix/tasks/format_test.exs @@ -209,17 +209,15 @@ defmodule Mix.Tasks.FormatTest do """ manifest_path = Path.join(Mix.Project.manifest_path(), ".cached_deps_formatter") + assert File.regular?(manifest_path) - # Let's check that the manifest gets updated if .formatter.exs changes. + # Let's check that the manifest gets updated if it's stale. File.touch!(manifest_path, {{1970, 1, 1}, {0, 0, 0}}) - %File.Stat{mtime: mtime} = File.stat!(manifest_path) - File.touch!(".formatter.exs") Mix.Tasks.Format.run(["a.ex"]) - %File.Stat{mtime: new_mtime} = File.stat!(manifest_path) - assert new_mtime > mtime + assert File.stat!(manifest_path).mtime > {{1970, 1, 1}, {0, 0, 0}} end end From cd95dcc14153711e71bc4a4b5cd317d90f11440c Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Wed, 13 Dec 2017 13:07:24 +0100 Subject: [PATCH 10/10] Improve manifest name --- lib/mix/lib/mix/tasks/format.ex | 4 +++- lib/mix/test/mix/tasks/format_test.exs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/mix/lib/mix/tasks/format.ex b/lib/mix/lib/mix/tasks/format.ex index 036f2bfb6f6..5d16c2dc3b4 100644 --- a/lib/mix/lib/mix/tasks/format.ex +++ b/lib/mix/lib/mix/tasks/format.ex @@ -117,6 +117,8 @@ defmodule Mix.Tasks.Format do dry_run: :boolean ] + @deps_manifest "cached_formatter_deps" + def run(args) do {opts, args} = OptionParser.parse!(args, strict: @switches) formatter_opts = eval_dot_formatter(opts) @@ -156,7 +158,7 @@ defmodule Mix.Tasks.Format do is_list(deps) -> # Since we have dependencies listed, we write the manifest even if those dependencies # don't export anything so that we avoid lookups everytime. - deps_manifest = Path.join(Mix.Project.manifest_path(), ".cached_deps_formatter") + deps_manifest = Path.join(Mix.Project.manifest_path(), @deps_manifest) dep_parenless_calls = if deps_dot_formatters_stale?(deps_manifest) do diff --git a/lib/mix/test/mix/tasks/format_test.exs b/lib/mix/test/mix/tasks/format_test.exs index 46aabcbfee4..8f4aff0d84c 100644 --- a/lib/mix/test/mix/tasks/format_test.exs +++ b/lib/mix/test/mix/tasks/format_test.exs @@ -208,7 +208,7 @@ defmodule Mix.Tasks.FormatTest do my_fun :foo, :bar """ - manifest_path = Path.join(Mix.Project.manifest_path(), ".cached_deps_formatter") + manifest_path = Path.join(Mix.Project.manifest_path(), "cached_formatter_deps") assert File.regular?(manifest_path)