From 93caddd448b75af2cc720da9a6e5bcae8fe70ae0 Mon Sep 17 00:00:00 2001
From: Thales Macedo Garitezi <thalesmg@gmail.com>
Date: Mon, 29 Nov 2021 10:23:54 -0300
Subject: [PATCH] refactor(review): factor out common functionality

---
 scripts/update_appup.escript | 42 +++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/scripts/update_appup.escript b/scripts/update_appup.escript
index 1de4ab899..7674a941c 100755
--- a/scripts/update_appup.escript
+++ b/scripts/update_appup.escript
@@ -102,16 +102,11 @@ main(Options, Baseline) ->
                     Diffs =
                         lists:filtermap(
                           fun({App, {Upgrade, Downgrade, OldUpgrade, OldDowngrade}}) ->
-                                  DiffUp = diff_appup_instructions(Upgrade, OldUpgrade),
-                                  DiffDown = diff_appup_instructions(Downgrade, OldDowngrade),
-                                  case {DiffUp, DiffDown} of
-                                      {[], []} ->
-                                          %% no diff for external dependency
+                                  case parse_appup_diffs(Upgrade, OldUpgrade,
+                                                         Downgrade, OldDowngrade) of
+                                      ok ->
                                           false;
-                                      _ ->
-                                          Diffs = #{ up => DiffUp
-                                                   , down => DiffDown
-                                                   },
+                                      {diffs, Diffs} ->
                                           {true, {App, Diffs}}
                                   end
                           end,
@@ -237,6 +232,23 @@ diff_appup_instructions(ComputedChanges, PresentChanges) ->
       [],
       ComputedChanges).
 
+%% For external dependencies, checks if any missing diffs are present
+%% and groups them by `up' and `down' types.
+parse_appup_diffs(Upgrade, OldUpgrade, Downgrade, OldDowngrade) ->
+    DiffUp = diff_appup_instructions(Upgrade, OldUpgrade),
+    DiffDown = diff_appup_instructions(Downgrade, OldDowngrade),
+    case {DiffUp, DiffDown} of
+        {[], []} ->
+            %% no diff for external dependency; ignore
+            ok;
+        _ ->
+            set_invalid(),
+            Diffs = #{ up => DiffUp
+                     , down => DiffDown
+                     },
+            {diffs, Diffs}
+    end.
+
 %% TODO: handle regexes
 find_matching_version(Vsn, PresentChanges) ->
     proplists:get_value(Vsn, PresentChanges).
@@ -333,17 +345,13 @@ do_update_appup(App, Upgrade, Downgrade, OldUpgrade, OldDowngrade) ->
                 {ok, AppupFile} ->
                     render_appfile(AppupFile, Upgrade, Downgrade);
                 false ->
-                    DiffUp = diff_appup_instructions(Upgrade, OldUpgrade),
-                    DiffDown = diff_appup_instructions(Downgrade, OldDowngrade),
-                    case {DiffUp, DiffDown} of
-                        {[], []} ->
+                    case parse_appup_diffs(Upgrade, OldUpgrade,
+                                           Downgrade, OldDowngrade) of
+                        ok ->
                             %% no diff for external dependency; ignore
                             ok;
-                        _ ->
+                        {diffs, Diffs} ->
                             set_invalid(),
-                            Diffs = #{ up => DiffUp
-                                     , down => DiffDown
-                                     },
                             log("ERROR: Appup file for the external dependency '~p' is not complete.~n       Missing changes: ~100p~n", [App, Diffs]),
                             log("NOTE: Some changes above might be already covered by regexes.~n")
                     end