Give high preference to task over children aliases

Author: josevalim

lib/mix/lib/mix.ex

@@ -238,14 +238,10 @@ defmodule Mix do
 
   In the example above, we have defined three aliases. One is `mix c`
   which is a shortcut for `mix compile`. Another is named
-  `mix hello`, which is the equivalent to the `Mix.Tasks.Hello`
-  we have defined in the [Mix.Task section](#module-mix-task).
-
-  The third is named `mix paid_task`, which runs the task `paid.task` with
-  several arguments, including one pulled from an environment variable.
-  Defining this as a function means that the environment variable is only
-  evaluated when this specific task is run, not when `mix.exs` is loaded
-  before each `mix` command.
+  `mix hello` and the third is named `mix paid_task`, which executes
+  the code inside a custom function to invoke the `paid.task` task
+  with several arguments, including one pulled from an environment
+  variable.
 
   Aliases may also be lists, specifying multiple tasks to be run
   consecutively:
@@ -265,19 +261,13 @@ defmodule Mix do
   Where `&clean_extra/1` would be a function in your `mix.exs`
   with extra cleanup logic.
 
-  Arguments given to the alias will be appended to the arguments of
-  the last task in the list. Except when overriding an existing task.
-  In this case, the arguments will be given to the original task,
-  in order to preserve semantics. For example, in the `:clean` alias
-  above, the arguments given to the alias will be passed to "clean"
-  and not to `clean_extra/1`.
-
-  Aliases defined in the current project do not affect its dependencies
-  and aliases defined in dependencies are not accessible from the
-  current project.
+  If the alias is overriding an existing task, the arguments given
+  to the alias will be forwarded to the original task in order to
+  preserve semantics. Otherwise arguments given to the alias are
+  appended to the arguments of the last task in the list.
 
-  Aliases can be used very powerfully to also run Elixir scripts and
-  shell commands, for example:
+  Another use case of aliases is to run Elixir scripts and shell
+  commands, for example:
 
       # priv/hello1.exs
       IO.puts("Hello One")
@@ -301,10 +291,15 @@ defmodule Mix do
   then `mix cmd` to execute a command line shell script. This shows how
   powerful aliases mixed with Mix tasks can be.
 
-  Mix tasks are designed to run only once. This prevents the same task
-  from being executed multiple times. For example, if there are several tasks
-  depending on `mix compile`, the code will be compiled once. Tasks can
-  be executed again if they are explicitly reenabled using `Mix.Task.reenable/1`:
+  One commit pitfall of aliases comes when trying to invoke the same task
+  multiple times. Mix tasks are designed to run only once. This prevents
+  the same task from being executed multiple times. For example, if there
+  are several tasks depending on `mix compile`, the code will be compiled
+  only once.
+
+  Similary, `mix format` can only be invoked once. So if you have an alias
+  that attempts to invoke `mix format` multiple times, it won't work unless
+  it is explicitly reenabled using `Mix.Task.reenable/1`:
 
       another_alias: [
         "format --check-formatted priv/hello1.exs",
@@ -314,15 +309,14 @@ defmodule Mix do
       ]
 
   Some tasks are automatically reenabled though, as they are expected to
-  be invoked multiple times. They are: `mix cmd`, `mix do`, `mix loadconfig`,
-  `mix profile.cprof`, `mix profile.eprof`, `mix profile.fprof`, `mix run`,
-  and `mix xref`.
-
-  It is worth mentioning that some tasks, such as in the case of the
-  `mix format` command in the example above, can accept multiple files so it
-  could be rewritten as:
-
-      another_alias: ["format --check-formatted priv/hello1.exs priv/hello2.exs"]
+  be invoked multiple times, such as: `mix cmd`, `mix do`, `mix xref`, etc.
+
+  Finally, aliases defined in the current project do not affect its
+  dependencies and aliases defined in dependencies are not accessible
+  from the current project, with the exception of umbrella projects.
+  Umbrella projects will run the aliases of its children when the
+  umbrella project itself does not define said alias and there is no
+  task with said name.
 
   ## Environment variables
 

lib/mix/lib/mix/task.ex

@@ -367,43 +367,61 @@ defmodule Mix.Task do
       alias && Mix.TasksServer.run({:alias, task, proj}) ->
         run_alias(List.wrap(alias), args, proj, task, apps, :ok)
 
-      Mix.Project.umbrella?() && umbrella_child_alias?(task) ->
-        Mix.ProjectStack.recur(fn ->
-          recur(
-            fn _ -> Mix.Project.config()[:aliases][String.to_atom(task)] && run(task, args) end,
-            apps
-          )
-        end)
+      Mix.TasksServer.get({:task, task, proj}) ->
+        :noop
 
-      !Mix.TasksServer.get({:task, task, proj}) ->
-        run_task(proj, task, args, apps)
+      module = maybe_load_or_compile_task(proj, task) ->
+        run_task(proj, module, task, args, apps)
+
+      results = Mix.Project.umbrella?() && recur_umbrella_children_alias(task, args, apps) ->
+        results
 
       true ->
-        :noop
+        # We could not find the task, let's raise
+        get!(task)
     end
   end
 
-  defp umbrella_child_alias?(task) do
-    umbrella_deps =
-      Mix.Dep.Umbrella.cached()
-      |> Enum.filter(& &1.opts[:from_umbrella])
+  defp recur_umbrella_children_alias(task, args, apps) do
+    alias_key = String.to_atom(task)
+
+    entries =
+      Mix.ProjectStack.recur(fn ->
+        recur(
+          fn proj ->
+            alias = Mix.Project.config()[:aliases][alias_key]
 
-    Enum.find(umbrella_deps, fn %Mix.Dep{app: app, opts: opts} ->
-      Mix.Project.in_project(app, opts[:path], [inherit_parent_config_files: true], fn _ ->
-        !!Mix.Project.config()[:aliases][String.to_atom(task)]
+            cond do
+              is_nil(alias) ->
+                :error
+
+              Mix.TasksServer.run({:alias, task, proj}) ->
+                {:ok, run_alias(List.wrap(alias), args, proj, task, nil, :ok)}
+
+              true ->
+                {:ok, :noop}
+            end
+          end,
+          apps
+        )
       end)
-    end)
+
+    case for({:ok, res} <- entries, do: res) do
+      [] -> nil
+      result -> result
+    end
   end
 
-  defp run_task(proj, task, args, apps) do
+  defp maybe_load_or_compile_task(proj, task) do
     # 1. If the task is available, we run it.
     # 2. Otherwise we compile and load dependencies
     # 3. Finally, we compile the current project in hope it is available.
-    module =
-      get_task_or_run(proj, task, fn -> run("deps.loadpaths") end) ||
-        get_task_or_run(proj, task, fn -> run("compile", []) end) ||
-        get!(task)
+    get_task_or_run(proj, task, fn -> run("deps.loadpaths") end) ||
+      get_task_or_run(proj, task, fn -> run("compile", []) end) ||
+      get(task)
+  end
 
+  defp run_task(proj, module, task, args, apps) do
     recursive = recursive(module)
 
     cond do
@@ -477,7 +495,8 @@ defmodule Mix.Task do
   defp run_alias([h | t], alias_args, proj, original_task, apps, _res) when is_binary(h) do
     case OptionParser.split(h) do
       [^original_task | args] ->
-        res = run_task(proj, original_task, args ++ alias_args, apps)
+        module = maybe_load_or_compile_task(proj, original_task) || get!(original_task)
+        res = run_task(proj, module, original_task, args ++ alias_args, apps)
         run_alias(t, [], proj, original_task, apps, res)
 
       [task | args] ->

lib/mix/test/fixtures/umbrella_test/apps/bar/mix.exs

@@ -9,7 +9,7 @@ defmodule Bar.MixProject do
       # get accidentally swept up into the actual Mix test suite.
       test_pattern: "*_tests.exs",
       test_coverage: [ignore_modules: [Bar, ~r/Ignore/]],
-      aliases: [mytask: ["cmd echo bar_running"]]
+      aliases: [mytask: fn _ -> Mix.shell().info("bar_running") end]
     ]
   end
 end

lib/mix/test/fixtures/umbrella_test/apps/foo/mix.exs

@@ -8,7 +8,7 @@ defmodule Foo.MixProject do
       # Choose something besides *_test.exs so that these test files don't
       # get accidentally swept up into the actual Mix test suite.
       test_pattern: "*_tests.exs",
-      aliases: [mytask: ["cmd echo foo_running"]]
+      aliases: [mytask: fn _ -> Mix.shell().info("foo_running") end]
     ]
   end
 end

lib/mix/test/mix/task_test.exs

@@ -172,7 +172,8 @@ defmodule Mix.TaskTest do
           def project do
             [
               app: :baz,
-              version: "0.1.0"
+              version: "0.1.0",
+              aliases: [help: fn _ -> raise "oops" end]
             ]
           end
         end
@@ -186,10 +187,27 @@ defmodule Mix.TaskTest do
         end
         """)
 
-        assert [:ok, nil, :ok] = Mix.Task.run("mytask")
-
-        assert_received {:mix_shell, :run, ["foo_running" <> _]}
-        assert_received {:mix_shell, :run, ["bar_running" <> _]}
+        Mix.Task.run("compile")
+        assert_received {:mix_shell, :info, ["==> foo"]}
+        assert_received {:mix_shell, :info, ["==> bar"]}
+        assert_received {:mix_shell, :info, ["==> baz"]}
+        Mix.shell().flush()
+
+        # A child alias does not overlap an umbrella task
+        assert Mix.Task.run("help") == :ok
+
+        # A missing umbrella alias can be found in children
+        assert [:ok, :ok] = Mix.Task.run("mytask")
+        assert_received {:mix_shell, :info, ["==> foo"]}
+        assert_received {:mix_shell, :info, ["foo_running"]}
+        assert_received {:mix_shell, :info, ["==> bar"]}
+        assert_received {:mix_shell, :info, ["bar_running"]}
+        refute_received {:mix_shell, :info, ["==> baz"]}
+
+        # An unknown task or alias anywhere fails
+        assert_raise Mix.NoTaskError, fn ->
+          Mix.Task.run("completely_unknown")
+        end
       end)
     end)
   end