Fix protocol_for_deps allow for git_subdir's ref and tag (#373)

* Identify as branch, even if in `git_subdir` * Move hex-specific comparison to "proper" place in code * Ease identification of legacy formats * Ease code analysis for future us * Allow tag and ref in git_subdir, as per rebar3 * Fix for backward compatibility case (with opts) * Fix test validations now that we match further * Include new file to test with * Fix as per test results. * protocol_for_deps: support git:// alongside https:// --------- Co-authored-by: Brujo Benavides <[email protected]>
inaka · Nov 12, 2024 · 6e0582c · 6e0582c
1 parent c19136f
commit 6e0582c
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 15 deletions.
diff --git a/doc_rules/elvis_project/protocol_for_deps.md b/doc_rules/elvis_project/protocol_for_deps.md
@@ -8,7 +8,7 @@ This rule was called `protocol_for_deps_rebar` before
 ## Options
 
 - `regex :: string()`.
-  - default: `(https://.*|[0-9]+([.][0-9]+)*)`.
+  - default: `^(https://|git://|\\d+(\\.\\d+)*)`.
 
 ## Example
 
@@ -21,5 +21,5 @@ This rule was called `protocol_for_deps_rebar` before
 - since [2.0.0](https://github.com/inaka/elvis_core/releases/tag/2.0.0)
 
 ```erlang
-{elvis_project, protocol_for_deps, #{ regex => "(https://.*|[0-9]+([.][0-9]+)*)" }}
+{elvis_project, protocol_for_deps, #{ regex => "^(https://|git://|\\d+(\\.\\d+)*)" }}
 ```
diff --git a/src/elvis_project.erl b/src/elvis_project.erl
@@ -30,7 +30,7 @@
 default(no_branch_deps) ->
     #{ignore => []};
 default(protocol_for_deps) ->
-    #{ignore => [], regex => "(https://.*|[0-9]+([.][0-9]+)*)"};
+    #{ignore => [], regex => "^(https://|git://|\\d+(\\.\\d+)*)"};
 default(old_configuration_format) ->
     #{}.
 
@@ -109,6 +109,9 @@ get_deps(File) ->
 %% @private
 is_branch_dep({_AppName, {_SCM, _Location, {branch, _}}}) ->
     true;
+is_branch_dep({_AppName, {git_subdir, _Url, {branch, _}, _SubDir}}) ->
+    true;
+%% Specific to plugin rebar_raw_resource
 is_branch_dep({AppName, {raw, DepResourceSpecification}}) ->
     is_branch_dep({AppName, DepResourceSpecification});
 %% Rebar2
@@ -124,25 +127,37 @@ is_hex_dep(_AppName) when is_atom(_AppName) ->
 is_hex_dep({_AppName, _Vsn, {pkg, _PackageName}})
     when is_atom(_AppName), is_list(_Vsn), is_atom(_PackageName) ->
     true;
+is_hex_dep({_AppName, {pkg, _OtherName}}) when is_atom(_AppName), is_atom(_OtherName) ->
+    true;
 is_hex_dep({_AppName, _Vsn}) when is_atom(_AppName), is_list(_Vsn) ->
     true;
 is_hex_dep(_) ->
     false.
 
 %% @private
-is_not_git_dep({_AppName, {pkg, _OtherName}}, _Regex) ->
-    false;
 is_not_git_dep({_AppName, {_SCM, Url, _Branch}}, Regex) ->
     nomatch == re:run(Url, Regex, []);
+is_not_git_dep({_AppName,
+                {git_subdir, Url, {BranchTagOrRefType, _BranchTagOrRef}, _SubDir}},
+               Regex)
+    when BranchTagOrRefType =:= branch;
+         BranchTagOrRefType =:= tag;
+         BranchTagOrRefType =:= ref ->
+    nomatch == re:run(Url, Regex, []);
+%% Specific to plugin rebar_raw_resource
 is_not_git_dep({AppName, {raw, DepResourceSpecification}}, Regex) ->
     is_not_git_dep({AppName, DepResourceSpecification}, Regex);
+%% Alternative formats, backwards compatible declarations
 is_not_git_dep({_AppName, {_SCM, Url}}, Regex) ->
     nomatch == re:run(Url, Regex, []);
 is_not_git_dep({_AppName, _Vsn, {_SCM, Url}}, Regex) ->
     nomatch == re:run(Url, Regex, []);
 is_not_git_dep({_AppName, _Vsn, {_SCM, Url, _Branch}}, Regex) ->
     nomatch == re:run(Url, Regex, []);
-is_not_git_dep({_AppName, {git_subdir, Url, {branch, _Branch}, _SubDir}}, Regex) ->
+is_not_git_dep({_AppName, _Vsn, {_SCM, Url, {BranchTagOrRefType, _Branch}}, _Opts}, Regex)
+    when BranchTagOrRefType =:= branch;
+         BranchTagOrRefType =:= tag;
+         BranchTagOrRefType =:= ref ->
     nomatch == re:run(Url, Regex, []).
 
 %% @private
@@ -163,6 +178,8 @@ dep_to_result({AppName, _}, Message, IgnoreDeps) ->
 dep_to_result({AppName, _, GitInfo}, Message, {IgnoreDeps, Regex}) ->
     dep_to_result({AppName, GitInfo}, Message, {IgnoreDeps, Regex});
 dep_to_result({AppName, _, GitInfo}, Message, IgnoreDeps) ->
+    dep_to_result({AppName, GitInfo}, Message, IgnoreDeps);
+dep_to_result({AppName, _Vsn, GitInfo, _Opts}, Message, IgnoreDeps) ->
     dep_to_result({AppName, GitInfo}, Message, IgnoreDeps).
 
 %% Old config

diff --git a/test/examples/rebar.config.fail b/test/examples/rebar.config.fail
@@ -21,7 +21,7 @@
 {deps_dir, "deps"}.
 {deps,
  [
-  {lager, "2.*", {git, "git://github.com/basho/lager.git", "2.0.0"}},
+  {lager, "2.*", {git, "git2://github.com/basho/lager.git", "2.0.0"}},
   {getopt, "0.*", {git, "[email protected]:jcomellas/getopt.git", {branch, "main"}}},
   {meck, "0.*", {git, "https://github.com/eproxus/meck.git", "0.8.2"}},
   {jiffy, "0.*", {git, "https://github.com/davisp/jiffy.git", "0.11.3"}},
@@ -30,7 +30,7 @@
   {aleppo, "0.*", {git, "https://github.com/inaka/aleppo.git", "main"}},
   {jsx, {raw, {git, "[email protected]:talentdeficit.git", {branch, "develop"}}}},
 
-  {lager, {git, "git://github.com/basho/lager.git", "2.0.0"}},
+  {lager, {git, "git2://github.com/basho/lager.git", "2.0.0"}},
   {getopt, {git, "[email protected]:jcomellas/getopt.git", {branch, "main"}}},
   {meck, {git, "https://github.com/eproxus/meck.git", "0.8.2"}},
   {jiffy, {git, "https://github.com/davisp/jiffy.git", "0.11.3"}},

diff --git a/test/examples/rebar3_2.config.success b/test/examples/rebar3_2.config.success
@@ -0,0 +1,19 @@
+{deps,
+ [
+    rebar,
+    {rebar, "1.0.0"},
+    {rebar, {pkg, rebar_fork}},
+    {rebar, "1.0.0", {pkg, rebar_fork}},
+    {rebar, {git, "git://github.com/rebar/rebar.git", {branch, "main"}}},
+    {rebar, {git, "https://github.com/rebar/rebar.git", {tag, "1.0.0"}}},
+    {rebar, {git, "https://github.com/rebar/rebar.git", {ref, "7f73b8d6"}}},
+    {rebar, {hg, "https://github.com/rebar/rebar.git", {tag, "1.0.0"}}},
+    {rebar, {git_subdir, "https://github.com/rebar/rebar.git", {branch, "main"}, "subdir"}},
+    {rebar, {git_subdir, "git://github.com/rebar/rebar.git", {tag, "1.0.0"}, "sub/dir"}},
+    {rebar, {git_subdir, "https://github.com/rebar/rebar.git", {ref, "7f73b8d6"}, "dir"}},
+    %% Alternative formats, backwards compatible declarations
+    {rebar, {git, "https://github.com/rebar/rebar.git"}},
+    {rebar, "1.0.*", {git, "https://github.com/rebar/rebar.git"}},
+    {rebar, "1.0.*", {git, "https://github.com/rebar/rebar.git", "Rev"}},
+    {rebar, ".*", {git, "https://github.com/rebar/rebar.git", {branch, "main"}}, [raw]}
+ ]}.
diff --git a/test/project_SUITE.erl b/test/project_SUITE.erl
@@ -41,16 +41,16 @@ verify_no_branch_deps(_Config) ->
     Filename = "rebar.config.fail",
     {ok, File} = elvis_test_utils:find_file(SrcDirs, Filename),
 
-    [_, _, _, _] = elvis_project:no_branch_deps(ElvisConfig, File, #{}),
+    [_, _, _, _, _] = elvis_project:no_branch_deps(ElvisConfig, File, #{}),
 
     RuleConfig = #{ignore => [jsx]},
-    [_, _] = elvis_project:no_branch_deps(ElvisConfig, File, RuleConfig),
+    [_, _, _] = elvis_project:no_branch_deps(ElvisConfig, File, RuleConfig),
 
     RuleConfig1 = #{ignore => [jsx, getopt]},
-    [] = elvis_project:no_branch_deps(ElvisConfig, File, RuleConfig1),
+    [_] = elvis_project:no_branch_deps(ElvisConfig, File, RuleConfig1),
 
     RuleConfig2 = #{ignore => [getopt]},
-    [_, _] = elvis_project:no_branch_deps(ElvisConfig, File, RuleConfig2).
+    [_, _, _] = elvis_project:no_branch_deps(ElvisConfig, File, RuleConfig2).
 
 -spec verify_protocol_for_deps(config()) -> any().
 verify_protocol_for_deps(_Config) ->
@@ -85,10 +85,15 @@ verify_hex_dep(_Config) ->
     ElvisConfig = elvis_test_utils:config(rebar_config),
     SrcDirs = elvis_config:dirs(ElvisConfig),
 
-    Filename = "rebar3.config.success",
-    {ok, File} = elvis_test_utils:find_file(SrcDirs, Filename),
+    Filename1 = "rebar3.config.success",
+    {ok, File1} = elvis_test_utils:find_file(SrcDirs, Filename1),
+
+    [] = elvis_project:protocol_for_deps(ElvisConfig, File1, #{}),
+
+    Filename2 = "rebar3_2.config.success",
+    {ok, File2} = elvis_test_utils:find_file(SrcDirs, Filename2),
 
-    [] = elvis_project:protocol_for_deps(ElvisConfig, File, #{}).
+    [] = elvis_project:protocol_for_deps(ElvisConfig, File2, #{}).
 
 -spec verify_old_config_format(config()) -> any().
 verify_old_config_format(_Config) ->