Warnings for Deletions and friendly errors

Apps that are no longer used are not automatically deleted, but we tell
users it can be done. This is safer while we're not sure of the
correctness of these messages.

Error messages are added for transient dependencies and dependencies not
found.

3 files changed, 57 insertions, 20 deletions

diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl
index 5ee2be6..1cae5aa 100644
--- a/src/rebar_prv_upgrade.erl
+++ b/src/rebar_prv_upgrade.erl
@@ -29,11 +29,14 @@ init(State) ->
                                                    {module, ?MODULE},
                                                    {bare, false},
                                                    {deps, ?DEPS},
-                                                   {example, "rebar upgrade cowboy[,ranch]"},
-                                                   {short_desc, "Upgrade dependency."},
-                                                   {desc, ""},
+                                                   {example, "rebar3 upgrade [cowboy[,ranch]]"},
+                                                   {short_desc, "Upgrade dependencies."},
+                                                   {desc, "Upgrade project dependecies. Mentioning no application "
+                                                          "will upgrade all dependencies. To upgrade specific dependencies, "
+                                                          "their names can be listed in the command."},
                                                    {opts, [
-                                                          {package, undefined, undefined, string, "Packages to upgrade."}
+                                                          {package, undefined, undefined, string,
+                                                           "List of packages to upgrade. If not specified, all dependencies are upgraded."}
                                                           ]}])),
     {ok, State1}.
 
@@ -42,11 +45,11 @@ do(State) ->
     {Args, _} = rebar_state:command_parsed_args(State),
     Locks = rebar_state:get(State, {locks, default}, []),
     Deps = rebar_state:get(State, deps),
-    Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args)), Locks),
+    Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args, <<"">>)), Locks),
     case prepare_locks(Names, Deps, Locks, []) of
         {error, Reason} ->
             {error, Reason};
-        {Locks0, _Unlocks0} -> % unlocks may be useful for deletions
+        {Locks0, _Unlocks0} ->
             Deps0 = top_level_deps(Deps, Locks),
             State1 = rebar_state:set(State, {deps, default}, Deps0),
             State2 = rebar_state:set(State1, {locks, default}, Locks0),
@@ -54,6 +57,10 @@ do(State) ->
             Res = rebar_prv_install_deps:do(State3),
             case Res of
                 {ok, State4} ->
+                    info_useless(
+                        [element(1,Lock) || Lock <- Locks],
+                        [rebar_app_info:name(App) || App <- rebar_state:lock(State4)]
+                    ),
                     rebar_prv_lock:do(State4);
                 _ ->
                     Res
@@ -61,6 +68,12 @@ do(State) ->
     end.
 
 -spec format_error(any()) -> iolist().
+format_error({unknown_dependency, Name}) ->
+    io_lib:format("Dependency ~ts not found", [Name]);
+format_error({transitive_dependency, Name}) ->
+    io_lib:format("Dependency ~ts is transient and cannot be safely upgraded. "
+                  "Promote it to your top-level rebar.config file to upgrade it.",
+                  [Name]);
 format_error(Reason) ->
     io_lib:format("~p", [Reason]).
 
@@ -82,7 +95,7 @@ prepare_locks([Name|Names], Deps, Locks, Unlocks) ->
             AtomName = binary_to_atom(Name, utf8),
             case lists:keyfind(AtomName, 1, Deps) of
                 false ->
-                    {error, {unknown_dependency, Name}};
+                    {error, {?MODULE, {unknown_dependency, Name}}};
                 Dep ->
                     Source = case Dep of
                         {_, Src} -> Src;
@@ -90,15 +103,14 @@ prepare_locks([Name|Names], Deps, Locks, Unlocks) ->
                     end,
                     {NewLocks, NewUnlocks} = unlock_higher_than(0, Locks -- [Lock]),
                     prepare_locks(Names,
-                                  %deps_like_locks(Deps, [{Name,Source,0} | NewLocks]),
                                   Deps,
                                   NewLocks,
                                   [{Name, Source, 0} | NewUnlocks ++ Unlocks])
             end;
         {_, _, Level} when Level > 0 ->
-            {error, {transitive_dependency,Name}};
+            {error, {?MODULE, {transitive_dependency,Name}}};
         false ->
-            {error, {unknown_dependency,Name}}
+            {error, {?MODULE, {unknown_dependency,Name}}}
     end.
 
 top_level_deps(Deps, Locks) ->
@@ -113,3 +125,9 @@ unlock_higher_than(Level, [App = {_,_,AppLevel} | Apps], Locks, Unlocks) ->
        AppLevel =< Level -> unlock_higher_than(Level, Apps, [App | Locks], Unlocks)
     end.
 
+info_useless(Old, New) ->
+    [?INFO("App ~ts is no longer needed and can be deleted.", [Name])
+     || Name <- Old,
+        not lists:member(Name, New)],
+    ok.
+
diff --git a/test/mock_git_resource.erl b/test/mock_git_resource.erl
index 2f7a72d..d2f0207 100644
--- a/test/mock_git_resource.erl
+++ b/test/mock_git_resource.erl
@@ -60,7 +60,7 @@ mock_update(Opts) ->
         ?MOD, needs_update,
         fun(_Dir, {git, Url, _Ref}) ->
             App = app(Url),
-            ct:pal("Needed update? ~p (~p) -> ~p", [App, {Url,_Ref}, lists:member(App, ToUpdate)]),
+%            ct:pal("Needed update? ~p (~p) -> ~p", [App, {Url,_Ref}, lists:member(App, ToUpdate)]),
             lists:member(App, ToUpdate)
         end).
 
diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl
index e0bb011..39b9687 100644
--- a/test/rebar_upgrade_SUITE.erl
+++ b/test/rebar_upgrade_SUITE.erl
@@ -9,7 +9,8 @@ groups() ->
     [{all, [], [top_a, top_b, top_c, top_d1, top_d2, top_e,
                 pair_a, pair_b, pair_ab, pair_c, pair_all,
                 triplet_a, triplet_b, triplet_c,
-                tree_a, tree_b, tree_c, tree_c2, tree_ac, tree_all]},
+                tree_a, tree_b, tree_c, tree_c2, tree_ac, tree_all,
+                delete_d]},
      {git, [], [{group, all}]},
      {pkg, [], [{group, all}]}].
 
@@ -83,7 +84,7 @@ upgrades(top_b) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"B", {error, {transitive_dependency, <<"B">>}}}};
+     {"B", {error, {rebar_prv_upgrade, {transitive_dependency, <<"B">>}}}}};
 upgrades(top_c) ->
      %% Original tree
     {[{"A", "1", [{"B", [{"D", "1", []}]},
@@ -96,7 +97,7 @@ upgrades(top_c) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"C", {error, {transitive_dependency, <<"C">>}}}};
+     {"C", {error, {rebar_prv_upgrade, {transitive_dependency, <<"C">>}}}}};
 upgrades(top_d1) ->
      %% Original tree
     {[{"A", "1", [{"B", [{"D", "1", []}]},
@@ -109,7 +110,7 @@ upgrades(top_d1) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"D", {error, {transitive_dependency, <<"D">>}}}};
+     {"D", {error, {rebar_prv_upgrade, {transitive_dependency, <<"D">>}}}}};
 upgrades(top_d2) ->
      %% Original tree
     {[{"A", "1", [{"B", [{"D", "1", []}]},
@@ -122,7 +123,7 @@ upgrades(top_d2) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"D", {error, {transitive_dependency, <<"D">>}}}};
+     {"D", {error, {rebar_prv_upgrade, {transitive_dependency, <<"D">>}}}}};
 upgrades(top_e) ->
      %% Original tree
     {[{"A", "1", [{"B", [{"D", "1", []}]},
@@ -135,7 +136,7 @@ upgrades(top_e) ->
      %% Modified apps, gobally
      ["A","B","D"],
      %% upgrade vs. new tree
-     {"E", {error, {unknown_dependency, <<"E">>}}}};
+     {"E", {error, {rebar_prv_upgrade, {unknown_dependency, <<"E">>}}}}};
 upgrades(pair_a) ->
     {[{"A", "1", [{"C", "1", []}]},
       {"B", "1", [{"D", "1", []}]}
@@ -171,7 +172,7 @@ upgrades(pair_c) ->
       {"B", "2", [{"D", "2", []}]}
      ],
      ["A","B","C","D"],
-     {"C", {error, {transitive_dependency, <<"C">>}}}};
+     {"C", {error, {rebar_prv_upgrade, {transitive_dependency, <<"C">>}}}}};
 upgrades(pair_all) ->
     {[{"A", "1", [{"C", "1", []}]},
       {"B", "1", [{"D", "1", []}]}
@@ -340,7 +341,17 @@ upgrades(tree_all) ->
      ["C","I"],
      {"", [{"A","1"}, "D", "J", "E", {"I","1"},
            {"B","1"}, "F", "G",
-           {"C","1"}, "H"]}}.
+           {"C","1"}, "H"]}};
+upgrades(delete_d) ->
+    {[{"A", "1", [{"B", [{"D", "1", []}]},
+                  {"C", [{"D", "2", []}]}]}
+     ],
+     [{"A", "2", [{"B", []},
+                  {"C", []}]}
+     ],
+     ["A","B", "C"],
+     %% upgrade vs. new tree
+     {"", [{"A","2"}, "B", "C"]}}.
 
 %% TODO: add a test that verifies that unlocking files and then
 %% running the upgrade code is enough to properly upgrade things.
@@ -353,7 +364,6 @@ top_level_deps([{{pkg, Name, Vsn, _URL}, _} | Deps]) ->
 
 mock_deps(git, Deps, Upgrades) ->
     catch mock_git_resource:unmock(),
-    ct:pal("mocked: ~p", [flat_deps(Deps)]),
     mock_git_resource:mock([{deps, flat_deps(Deps)}, {upgrade, Upgrades}]);
 mock_deps(pkg, Deps, Upgrades) ->
     catch mock_pkg_resource:unmock(),
@@ -435,6 +445,15 @@ tree_c2(Config) -> run(Config).
 tree_ac(Config) -> run(Config).
 tree_all(Config) -> run(Config).
 
+delete_d(Config) ->
+    meck:new(rebar_log, [no_link, passthrough]),
+    run(Config),
+    Infos = [{Str, Args}
+            || {_, {rebar_log, log, [info, Str, Args]}, _} <- meck:history(rebar_log)],
+    meck:unload(rebar_log),
+    ?assertNotEqual([],
+                    [1 || {"App ~ts is no longer needed and can be deleted.",
+                           [<<"D">>]} <- Infos]).
 run(Config) ->
     apply(?config(mock, Config), []),
     {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),