Alias plugin promoted to built-in command

- Uses the code at https://github.com/tsloughter/rebar_alias and brings
  it within rebar3
- adds safety checks to prevent redefining built-in commands or obvious
  circular dependencies between commands (indirect circular deps are
  still possible)
- adds tests
- adds a systest to ensure no clash with the existing plugin

src/rebar.app.src

@@ -73,6 +73,7 @@
                      rebar_prv_update,
                      rebar_prv_upgrade,
                      rebar_prv_version,
-                     rebar_prv_xref]}
+                     rebar_prv_xref,
+                     rebar_prv_alias]} % must run last to prevent overloads
         ]}
  ]}.

src/rebar_prv_alias.erl

@@ -0,0 +1,102 @@
+%%% @doc Meta-provider that dynamically compiles providers
+%%% to run aliased commands.
+%%%
+%%% This is hackish and out-there, but this module has graduated
+%%% from a plugin at https://github.com/tsloughter/rebar_alias after
+%%% years of stability. Only some error checks were added
+-module(rebar_prv_alias).
+
+-export([init/1]).
+-include("rebar.hrl").
+
+%% ===================================================================
+%% Public API
+%% ===================================================================
+-spec init(rebar_state:t()) -> {ok, rebar_state:t()}.
+init(State) ->
+    Aliases = rebar_state:get(State, alias, []),
+    lists:foldl(fun({Alias, Cmds}, {ok, StateAcc}) ->
+                    case validate_provider(Alias, Cmds, State) of
+                        true -> init_alias(Alias, Cmds, StateAcc);
+                        false -> {ok, State}
+                    end
+                end, {ok, State}, Aliases).
+
+init_alias(Alias, Cmds, State) ->
+    Module = list_to_atom("rebar_prv_alias_" ++ atom_to_list(Alias)),
+
+    MF = module(Module),
+    EF = exports(),
+    FF = do_func(Cmds),
+
+    {ok, _, Bin} = compile:forms([MF, EF, FF]),
+    code:load_binary(Module, "none", Bin),
+
+    Provider = providers:create([
+            {name, Alias},
+            {module, Module},
+            {bare, true},
+            {deps, []},
+            {example, example(Alias)},
+            {opts, []},
+            {short_desc, desc(Cmds)},
+            {desc, desc(Cmds)}
+    ]),
+    {ok, rebar_state:add_provider(State, Provider)}.
+
+validate_provider(Alias, Cmds, State) ->
+    %% This would be caught and prevented anyway, but the warning
+    %% is friendlier
+    case providers:get_provider(Alias, rebar_state:providers(State)) of
+        not_found ->
+            %% check for circular deps in the alias.
+            case not proplists:is_defined(Alias, Cmds) of
+                true -> true;
+                false ->
+                    ?WARN("Alias ~p contains itself and would never "
+                          "terminate. It will be ignored.",
+                          [Alias]),
+                    false
+            end;
+        _ ->
+            ?WARN("Alias ~p is already the name of a command in "
+                  "the default namespace and will be ignored.",
+                  [Alias]),
+            false
+    end.
+
+
+example(Alias) ->
+    "rebar3 " ++ atom_to_list(Alias).
+
+desc(Cmds) ->
+    "Equivalent to running: rebar3 do " ++
+    rebar_string:join(lists:map(fun({Cmd, Args}) ->
+                                        atom_to_list(Cmd) ++ " " ++ Args;
+                                   (Cmd) ->
+                                        atom_to_list(Cmd)
+                                end, Cmds), ",").
+
+module(Name) ->
+    {attribute,1,module,Name}.
+
+exports() ->
+    {attribute,1,export,[{do,1}]}.
+
+do_func(Cmds) ->
+    {function,1,do,1,
+     [{clause,1,
+       [{var,1,'State'}],
+       [],
+       [{call,1,
+         {remote,1,{atom,1,rebar_prv_do},{atom,1,do_tasks}},
+         [to_args(Cmds),{var,1,'State'}]}]}]}.
+
+
+to_args([]) ->
+    {nil,1};
+to_args([{Cmd, Args} | Rest]) ->
+    {cons,1,{tuple,1,[{string,1,atom_to_list(Cmd)},{string,1,Args}]}, to_args(Rest)};
+to_args([Cmd | Rest]) ->
+    {cons,1,{tuple,1,[{string,1,atom_to_list(Cmd)},{nil,1}]}, to_args(Rest)}.
+

src/rebar_state.erl

@@ -391,7 +391,7 @@ add_provider(State=#state_t{providers=Providers, allow_provider_overrides=false}
                            case {providers:impl(P), providers:namespace(P)} of
                                {Name, Namespace} ->
                                    ?DEBUG("Not adding provider ~p ~p from module ~p because it already exists from module ~p",
-                                          [Namespace, Name, providers:module(P), Module]),
+                                          [Namespace, Name, Module, providers:module(P)]),
                                    true;
                                _ ->
                                    false

systest/all_SUITE.erl

@@ -29,7 +29,7 @@ end_per_testcase(_Name, Config) ->
     Config.
 
 all() ->
-    [noop, resource_plugins].
+    [noop, resource_plugins, alias_clash].
 
 %groups() ->
 %    [{plugins, [shuffle], []},
@@ -53,6 +53,21 @@ resource_plugins(Config) ->
     ct:pal("Rebar3 Output:~n~s",[Output]),
     ok.
 
+alias_clash() ->
+    [{doc, "checking that the provider won't get plugin interference."},
+     {timetrap, 10000}].
+alias_clash(Config) ->
+    {ok, Help} = rebar3("help", Config), % should be redefined, but by the plugin
+    ?assertNotEqual(nomatch,
+        re:run(Help, "Alias help is already the name of a command[a-z ]+and will be ignored")
+    ),
+    {ok, Output} = rebar3("test", Config, [{env, [{"DEBUG", "1"}]}]),
+    ?assertNotEqual(nomatch, re:run(Output, "cover summary written to:")),
+    ?assertNotEqual(nomatch,
+        re:run(Output, "Not adding provider default test from module rebar_prv_alias_test "
+                       "because it already exists from module rebar_prv_alias_test")),
+    ok.
+
 %%%%%%%%%%%%%%%
 %%% Helpers %%%
 %%%%%%%%%%%%%%%
@@ -62,7 +77,9 @@ set_name_config(Atom, Config) ->
                      atom_to_list(?MODULE)++"_data", atom_to_list(Atom)])}
      | Config].
 
-rebar3(Args, Config) ->
+rebar3(Args, Config) -> rebar3(Args, Config, []).
+
+rebar3(Args, Config, UserOpts) ->
     Exec = case os:type() of
         {win32, _} ->
             "rebar3.cmd";
@@ -70,6 +87,7 @@ rebar3(Args, Config) ->
             "rebar3"
     end,
     Cmd = Exec ++ " " ++ Args,
-    Opts = [{cd, ?config(path, Config)}, return_on_error, use_stdout],
+    Opts = [{cd, ?config(path, Config)}, return_on_error, use_stdout
+            | UserOpts],
     ct:pal("Calling rebar3 ~s with options ~p", [Cmd, Opts]),
     rebar_utils:sh(Cmd, Opts).

systest/all_SUITE_data/alias_clash/rebar.config

@@ -0,0 +1,4 @@
+{alias, [{help, [version]}, % should be skipped, but be overriden by plugin
+         {test, [compile, {eunit, "-c"}, cover]}]}.
+
+{plugins, [rebar_alias]}. % should be overridden

systest/all_SUITE_data/alias_clash/src/alias_clash.app.src

@@ -0,0 +1,15 @@
+{application, alias_clash,
+ [{description, "An OTP library"},
+  {vsn, "0.1.0"},
+  {registered, []},
+  {applications,
+   [kernel,
+    stdlib
+   ]},
+  {env,[]},
+  {modules, []},
+
+  {maintainers, []},
+  {licenses, ["Apache 2.0"]},
+  {links, []}
+ ]}.

systest/all_SUITE_data/alias_clash/src/alias_clash.erl

@@ -0,0 +1,13 @@
+-module(alias_clash).
+
+%% API exports
+-export([]).
+
+%%====================================================================
+%% API functions
+%%====================================================================
+
+
+%%====================================================================
+%% Internal functions
+%%====================================================================

test/rebar_alias_SUITE.erl

@@ -0,0 +1,113 @@
+-module(rebar_alias_SUITE).
+-compile([export_all]).
+
+-include_lib("common_test/include/ct.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+init_per_suite(Config) -> Config.
+end_per_suite(_Config) -> ok.
+
+init_per_testcase(_, Config) ->
+    rebar_test_utils:init_rebar_state(Config, "alias_").
+
+end_per_testcase(_, _Config) ->
+    ok.
+
+all() -> [command, args, many, override_default, no_circular].
+         %% namespaces: unsupported, untested.
+
+command() ->
+    [{doc, "Runs multiple regular commands as one alias"}].
+command(Config) ->
+    AppDir = ?config(apps, Config),
+
+    Name = rebar_test_utils:create_random_name("alias_command_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+    RebarConfig = [{alias, [{test, [compile, unlock]}]}],
+
+    %% compile job ran
+    rebar_test_utils:run_and_check(Config, RebarConfig,
+                                   ["test"], {ok, [{app, Name}]}),
+    %% unlock job also ran
+    Lockfile = filename:join(?config(apps, Config), "rebar.lock"),
+    ?assertNot(filelib:is_file(Lockfile)),
+    ok.
+
+args() ->
+    [{doc, "Runs multiple regular commands as one alias, some of "
+           "which have default arguments"}].
+args(Config) ->
+    AppDir = ?config(apps, Config),
+
+    Name = rebar_test_utils:create_random_name("alias_args_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+    RebarConfig = [{alias, [{test, [{eunit,"-c"}, cover]}]}],
+
+    %% test job ran (compiled and succeeded)
+    rebar_test_utils:run_and_check(Config, RebarConfig,
+                                   ["test"], {ok, [{app, Name}]}),
+    %% cover job also ran, meaning eunit had coverage on, otherwise
+    %% the index file is not generated.
+    CoverFile = filename:join([?config(apps, Config),
+                               "_build", "test", "cover", "index.html"]),
+    ?assert(filelib:is_file(CoverFile)),
+    ok.
+
+many() ->
+    [{doc, "Multiple aliases may be registered"}].
+many(Config) ->
+    AppDir = ?config(apps, Config),
+
+    Name = rebar_test_utils:create_random_name("alias_args_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+    RebarConfig = [{alias, [{test, [{eunit,"-c"}, cover]},
+                            {nolock, [compile, unlock]}]}],
+
+    %% test job ran (compiled and succeeded)
+    rebar_test_utils:run_and_check(Config, RebarConfig,
+                                   ["test"], {ok, [{app, Name}]}),
+    rebar_test_utils:run_and_check(Config, RebarConfig,
+                                   ["nolock"], {ok, [{app, Name}]}),
+    %% both jobs ran (see args/1 and command/1)
+    CoverFile = filename:join([?config(apps, Config),
+                               "_build", "test", "cover", "index.html"]),
+    ?assert(filelib:is_file(CoverFile)),
+    Lockfile = filename:join(?config(apps, Config), "rebar.lock"),
+    ?assertNot(filelib:is_file(Lockfile)),
+    ok.
+
+override_default() ->
+    [{doc, "An alias cannot take over a default provider"}].
+override_default(Config) ->
+    AppDir = ?config(apps, Config),
+
+    Name = rebar_test_utils:create_random_name("alias_override_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+    RebarConfig = [{alias, [{compile, [help]}]}],
+
+    %% App compiles anyway
+    rebar_test_utils:run_and_check(Config, RebarConfig, ["compile"],
+                                   {ok, [{app, Name}]}),
+    ok.
+
+no_circular() ->
+    [{doc, "An alias cannot define itself as itself"},
+     {timetrap, 2000}].
+no_circular(Config) ->
+    AppDir = ?config(apps, Config),
+
+    Name = rebar_test_utils:create_random_name("alias_circular_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+    RebarConfig = [{alias, [{test, [help, {test,"-a"}, compile]}]}],
+
+    %% Code does not deadlock forever and errors by not knowing
+    %% the command
+    rebar_test_utils:run_and_check(Config, RebarConfig, ["test"],
+                {error, [$C,$o,$m,$m,$a,$n,$d,$ ,"test",$ ,$n,$o,$t,$ ,
+                         $f,$o,$u,$n,$d]}),
+    ok.