From d7bec48bd465cea601fc2c8dc1a7ca4b23740cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 15 Jul 2020 10:58:23 +0200 Subject: [PATCH] Improve handling of local dependencies Treat all path dependencies as files in the current project instead of remote dependencies. This simplifies the logic as we no longer need to especially handle umbrella applications, as they are all path dependencies anyway. This requires Elixir v1.10+. Also note this patch completely removes `:plt_add_deps` because it did not work before. Since the `Plt.check/2` function always adds all transitive dependencies, any attempt to discard deps by setting `:plt_add_deps` to `:apps_deps` had no effect. This PR does not remove `:apps_deps` from the documentation but I would recommend so to be done in a separate PR. --- lib/dialyxir/project.ex | 140 ++++++--------------------------- lib/mix/tasks/dialyzer.ex | 3 +- test/dialyxir/project_test.exs | 22 ++---- 3 files changed, 31 insertions(+), 134 deletions(-) diff --git a/lib/dialyxir/project.ex b/lib/dialyxir/project.ex index 97b63e6a..26e4c0f1 100644 --- a/lib/dialyxir/project.ex +++ b/lib/dialyxir/project.ex @@ -22,9 +22,7 @@ defmodule Dialyxir.Project do end end - def plt_file() do - plt_path(dialyzer_config()[:plt_file]) || deps_plt() - end + def plt_file, do: plt_path(dialyzer_config()[:plt_file]) || deps_plt() defp plt_path(file) when is_binary(file), do: Path.expand(file) defp plt_path({:no_warn, file}) when is_binary(file), do: Path.expand(file) @@ -41,13 +39,9 @@ defmodule Dialyxir.Project do end def cons_apps do - # compile & load all deps paths - Mix.Tasks.Deps.Loadpaths.run([]) - # compile & load current project paths - Mix.Project.compile([]) - apps = plt_apps() || plt_add_apps() ++ include_deps() + Mix.Task.run("compile", []) - apps + (plt_apps() || plt_add_apps() ++ Enum.flat_map(local_apps(), &direct_children/1)) |> Enum.sort() |> Enum.uniq() |> Kernel.--(plt_ignore_apps()) @@ -241,84 +235,34 @@ defmodule Dialyxir.Project do end defp default_paths() do - reduce_umbrella_children([], fn paths -> - [Mix.Project.compile_path() | paths] - end) - end - - defp plt_apps, do: dialyzer_config()[:plt_apps] |> load_apps() - defp plt_add_apps, do: dialyzer_config()[:plt_add_apps] || [] |> load_apps() - defp plt_ignore_apps, do: dialyzer_config()[:plt_ignore_apps] || [] - - defp load_apps(nil), do: nil - - defp load_apps(apps) do - Enum.each(apps, &Application.load/1) - apps - end - - defp include_deps do - method = dialyzer_config()[:plt_add_deps] - - reduce_umbrella_children([], fn deps -> - deps ++ - case method do - false -> - [] - - # compatibility - true -> - deps_project() ++ deps_app(false) + build_path = Mix.Project.build_path() - :project -> - info( - "Dialyxir has deprecated plt_add_deps: :project in favor of apps_direct, which includes only runtime dependencies." - ) - - deps_project() ++ deps_app(false) - - :apps_direct -> - deps_app(false) - - :transitive -> - info( - "Dialyxir has deprecated plt_add_deps: :transitive in favor of app_tree, which includes only runtime dependencies." - ) - - deps_transitive() ++ deps_app(true) - - _app_tree -> - deps_app(true) - end - end) + for app <- local_apps() do + Path.join([build_path, "lib", Atom.to_string(app), "ebin"]) + end end - defp deps_project do - Mix.Project.config()[:deps] - |> Enum.filter(&env_dep(&1)) - |> Enum.map(&elem(&1, 0)) - end + defp plt_apps, do: dialyzer_config()[:plt_apps] + defp plt_add_apps, do: dialyzer_config()[:plt_add_apps] || [] + defp plt_ignore_apps, do: dialyzer_config()[:plt_ignore_apps] || [] - defp deps_transitive do - Mix.Project.deps_paths() - |> Map.keys() - end + defp local_apps() do + deps_apps = + for {app, scm} <- Mix.Project.deps_scms(), + not scm.fetchable?(), + do: app - @spec deps_app(boolean()) :: [atom] - defp deps_app(recursive) do - app = Mix.Project.config()[:app] - deps_app(app, recursive) - end - - @spec deps_app(atom(), boolean()) :: [atom] - defp deps_app(app, recursive) do - with_each = - if recursive do - &deps_app(&1, true) + project_apps = + if children = Mix.Project.apps_paths() do + Map.keys(children) else - fn _ -> [] end + [Mix.Project.config()[:app]] end + Enum.uniq(project_apps ++ deps_apps) -- plt_ignore_apps() + end + + defp direct_children(app) do case Application.load(app) do :ok -> nil @@ -327,46 +271,10 @@ defmodule Dialyxir.Project do nil {:error, err} -> - nil error("Error loading #{app}, dependency list may be incomplete.\n #{inspect(err)}") end - case Application.spec(app, :applications) do - [] -> - [] - - nil -> - [] - - this_apps -> - Enum.map(this_apps, with_each) - |> List.flatten() - |> Enum.concat(this_apps) - end - end - - defp env_dep(dep) do - only_envs = dep_only(dep) - only_envs == nil || Mix.env() in List.wrap(only_envs) - end - - defp dep_only({_, opts}) when is_list(opts), do: opts[:only] - defp dep_only({_, _, opts}) when is_list(opts), do: opts[:only] - defp dep_only(_), do: nil - - @spec reduce_umbrella_children(list(), (list() -> list())) :: list() - defp reduce_umbrella_children(acc, f) do - if Mix.Project.umbrella?() do - children = Mix.Dep.Umbrella.loaded() - - Enum.reduce(children, acc, fn child, acc -> - Mix.Project.in_project(child.app, child.opts[:path], fn _ -> - reduce_umbrella_children(acc, f) - end) - end) - else - f.(acc) - end + List.wrap(Application.spec(app, :applications)) end defp dialyzer_config(), do: Mix.Project.config()[:dialyzer] diff --git a/lib/mix/tasks/dialyzer.ex b/lib/mix/tasks/dialyzer.ex index b793268c..13fe2013 100644 --- a/lib/mix/tasks/dialyzer.ex +++ b/lib/mix/tasks/dialyzer.ex @@ -11,7 +11,6 @@ defmodule Mix.Tasks.Dialyzer do * `--no-compile` - do not compile even if needed. * `--no-check` - do not perform (quick) check to see if PLT needs update. * `--force-check` - force PLT check also if lock file is unchanged. - useful when dealing with local deps. * `--ignore-exit-status` - display warnings but do not halt the VM or return an exit status code * `--list-unused-filters` - list unused ignore filters useful for CI. do not use with `mix do`. @@ -161,7 +160,7 @@ defmodule Mix.Tasks.Dialyzer do if Mix.Project.get() do Project.check_config() - unless opts[:no_compile], do: Mix.Project.compile([]) + unless opts[:no_compile], do: Mix.Task.run("compile", []) _ = unless no_check?(opts) do diff --git a/test/dialyxir/project_test.exs b/test/dialyxir/project_test.exs index 7d3d5cc9..7cf1cfe3 100644 --- a/test/dialyxir/project_test.exs +++ b/test/dialyxir/project_test.exs @@ -2,7 +2,7 @@ defmodule Dialyxir.ProjectTest do alias Dialyxir.Project use ExUnit.Case - import ExUnit.CaptureIO, only: [capture_io: 1, capture_io: 2] + import ExUnit.CaptureIO, only: [capture_io: 1] defp in_project(app, f) when is_atom(app) do Mix.Project.in_project(app, "test/fixtures/#{Atom.to_string(app)}", fn _ -> f.() end) @@ -46,8 +46,7 @@ defmodule Dialyxir.ProjectTest do end) end - test "App list for default contains direct and - indirect :application dependencies" do + test "App list for default contains direct :application dependencies" do in_project(:default_apps, fn -> apps = Project.cons_apps() # direct @@ -55,12 +54,11 @@ defmodule Dialyxir.ProjectTest do # direct assert Enum.member?(apps, :public_key) # indirect - assert Enum.member?(apps, :asn1) + refute Enum.member?(apps, :asn1) end) end - test "App list for umbrella contains child dependencies - indirect :application dependencies" do + test "App list for umbrella contains direct child and :application dependencies" do in_project(:umbrella, fn -> apps = Project.cons_apps() # direct @@ -68,15 +66,14 @@ defmodule Dialyxir.ProjectTest do # direct, child1 assert Enum.member?(apps, :public_key) # indirect - assert Enum.member?(apps, :asn1) + refute Enum.member?(apps, :asn1) # direct, child2 assert Enum.member?(apps, :mix) end) end @tag :skip - test "App list for umbrella contains all child dependencies - when run from child directory" do + test "App list for umbrella contains all child dependencies when run from child directory" do in_project([:umbrella, :apps, :second_one], fn -> apps = Project.cons_apps() # direct @@ -156,13 +153,6 @@ defmodule Dialyxir.ProjectTest do end) end - test "Project with non-existent dependency" do - in_project(:nonexistent_deps, fn -> - out = capture_io(:stderr, &Project.cons_apps/0) - assert Regex.match?(~r/Error loading nonexistent, dependency list may be incomplete/, out) - end) - end - test "igonored apps are removed in umbrella projects" do in_project(:umbrella_ignore_apps, fn -> refute Enum.member?(Project.cons_apps(), :logger)