Fix #302: Get the formatter ready for OTP25 (#303)

* [#302] Get tests to run on OTP24 and OTP25 * [#302] Add support for ex_doc * [#302] Use the latest version of Hank
AdRoll · May 25, 2022 · a467bcc · a467bcc
1 parent 2ea18fc
commit a467bcc
Show file tree

Hide file tree

Showing 12 changed files with 185 additions and 68 deletions.
diff --git a/.github/workflows/erlang.yml b/.github/workflows/erlang.yml
@@ -10,7 +10,7 @@ jobs:
 
     strategy:
       matrix:
-        otp: ['22.3', '23.3', '24.3']
+        otp: ['23.3', '24.3', '25.0']
         rebar: ['3.18.0']
 
     steps:
@@ -31,4 +31,4 @@ jobs:
         path: ~/.cache/rebar3
         key: rebar3-cache-for-os-${{runner.os}}-otp-${{steps.setup-beam.outputs.otp-version}}-rebar3-${{steps.setup-beam.outputs.rebar3-version}}-hash-${{hashFiles('rebar.lock')}}
     - name: Run tests and verifications
-      run: rebar3 test
+      run: ERL_FLAGS="-enable-feature all" rebar3 test
diff --git a/nextroll.dict b/nextroll.dict
@@ -30,6 +30,7 @@ insert_pragma
 lay_clauses
 lay_items
 old-reliable
+otp25
 paragraph-format
 per-file
 plugin

diff --git a/rebar.config b/rebar.config
@@ -1,14 +1,22 @@
 {erl_opts, [warn_unused_import, warn_export_vars, verbose, report, debug_info]}.
 
-{minimum_otp_vsn, "21"}.
+{minimum_otp_vsn, "23"}.
 
-{deps, [{katana_code, "1.2.0"}]}.
+{deps, [{katana_code, "~> 2.0.0"}]}.
+
+{ex_doc,
+ [{source_url, <<"https://github.com/AdRoll/rebar3_format">>},
+  {extras, [<<"README.md">>, <<"LICENSE">>]},
+  {main, <<"readme">>}]}.
+
+{hex, [{doc, #{provider => ex_doc}}]}.
 
 {project_plugins,
  [{rebar3_hex, "~> 7.0.1"},
-  {rebar3_hank, "~> 1.2.2"},
+  {rebar3_hank, "~> 1.3.0"},
   {rebar3_lint, "~> 1.0.2"},
-  {rebar3_sheldon, "~> 0.4.2"}]}.
+  {rebar3_sheldon, "~> 0.4.2"},
+  {rebar3_ex_doc, "~> 0.2.9"}]}.
 
 {dialyzer, [{warnings, [no_return, unmatched_returns, error_handling, underspecs]}]}.
 
@@ -23,10 +31,10 @@
   {files, ["src/**/*.?rl", "src/*.app.src", "test/*.?rl"]},
   {additional_dictionaries, ["nextroll.dict", "test.dict"]}]}.
 
-{alias, [{test, [lint, spellcheck, hank, dialyzer, ct, cover]}, {format, [compile]}]}.
+{alias,
+ [{test, [spellcheck, lint, hank, dialyzer, {ct, "--verbose"}, cover, edoc]},
+  {format, [compile]}]}.
 
 {post_hooks, [{compile, "escript priv/scripts/format"}]}.
 
 {hank, [{ignore, ["test_app/**/*"]}]}.
-
-{hex, [{doc, #{provider => edoc}}]}.
diff --git a/rebar.lock b/rebar.lock
@@ -1,8 +1,8 @@
 {"1.2.0",
-[{<<"katana_code">>,{pkg,<<"katana_code">>,<<"1.2.0">>},0}]}.
+[{<<"katana_code">>,{pkg,<<"katana_code">>,<<"2.0.0">>},0}]}.
 [
 {pkg_hash,[
- {<<"katana_code">>, <<"FD90770D1414158D4C08811C3D893F27DD1BEFE493D0F13B5B011AD55E7B98F2">>}]},
+ {<<"katana_code">>, <<"4AE51EEA2BD3EE61515C3EB8A1604051F4EAB98EDC6C28BD5DFA9143773FC073">>}]},
 {pkg_hash_ext,[
- {<<"katana_code">>, <<"CD1D1E37E5568698432FAC9A9DE0D283B80A1F0EE7BE8B8B94CA144FAC469E95">>}]}
+ {<<"katana_code">>, <<"D2F3BB2F942A14DAA42BF5FFD26B8720E10466743B21681881116CD11D00BAE4">>}]}
 ].
diff --git a/src/formatters/default_formatter.erl b/src/formatters/default_formatter.erl
@@ -10,6 +10,12 @@
 %% Allow erl_syntax:syntaxTree/0 type spec
 -elvis([{elvis_style, atom_naming_convention, #{regex => "^([a-zA-Z][a-z0-9]*_?)*$"}}]).
 
+%% 'maybe' and 'else', among others
+-format #{unquote_atoms => false}.
+
+%% erl_syntax functions that only appear in OTP25
+-dialyzer([no_missing_calls]).
+
 -export([init/2, format_file/3, format/3]).
 
 -import(prettypr,
@@ -44,6 +50,7 @@
     simple_fun_expr |
     fun_expr |
     if_expr |
+    maybe_expr |
     receive_expr |
     try_expr |
     {function, prettypr:document()} |
@@ -308,16 +315,7 @@ lay_no_comments(Node, Ctxt) ->
             Pattern = erl_syntax:match_expr_pattern(Node),
             D1 = lay(Pattern, set_prec(Ctxt, PrecL)),
             D2 = lay(erl_syntax:match_expr_body(Node), set_prec(Ctxt, PrecR)),
-            D3 = case erl_syntax:type(Pattern) == underscore
-                      orelse erl_syntax:type(Pattern) == variable
-                             andalso length(erl_syntax:variable_literal(Pattern))
-                                     < Ctxt#ctxt.break_indent
-                 of
-                     true -> %% Single short variable on the left, don't nest
-                         follow(beside(D1, lay_text_float(" =")), D2, Ctxt#ctxt.break_indent);
-                     false -> %% Large pattern, nesting makes sense
-                         sep([beside(D1, lay_text_float(" =")), nest(Ctxt#ctxt.break_indent, D2)])
-                 end,
+            D3 = lay_match_expression(" =", Pattern, D1, D2, Ctxt),
             maybe_parentheses(D3, Prec, Ctxt);
         underscore ->
             text("_");
@@ -343,6 +341,8 @@ lay_no_comments(Node, Ctxt) ->
                     make_if_clause(D2, D3, Ctxt);
                 case_expr ->
                     make_case_clause(D1, D2, D3, Ctxt);
+                maybe_expr ->
+                    make_case_clause(D1, D2, D3, Ctxt);
                 receive_expr ->
                     make_case_clause(D1, D2, D3, Ctxt);
                 try_expr ->
@@ -605,6 +605,25 @@ lay_no_comments(Node, Ctxt) ->
         parentheses ->
             D = lay(erl_syntax:parentheses_body(Node), reset_prec(Ctxt)),
             lay_parentheses(D);
+        maybe_expr ->
+            Ctxt1 = reset_prec(Ctxt),
+            D0 = lay_clause_expressions(erl_syntax:maybe_expr_body(Node), Ctxt1, fun lay/2),
+            D1 = vertical([text("maybe"), nest(Ctxt1#ctxt.break_indent, D0)]),
+            case erl_syntax:maybe_expr_else(Node) of
+                none ->
+                    par([D1, text("end")]);
+                ElseNode ->
+                    ElseCs = erl_syntax:else_expr_clauses(ElseNode),
+                    D3 = lay_clauses(ElseCs, maybe_expr, Ctxt1),
+                    sep([par([D1, text("else")]), nest(Ctxt1#ctxt.break_indent, D3), text("end")])
+            end;
+        maybe_match_expr ->
+            {PrecL, Prec, PrecR} = inop_prec('='),
+            Pattern = erl_syntax:maybe_match_expr_pattern(Node),
+            D1 = lay(Pattern, set_prec(Ctxt, PrecL)),
+            D2 = lay(erl_syntax:maybe_match_expr_body(Node), set_prec(Ctxt, PrecR)),
+            D3 = lay_match_expression(" ?=", Pattern, D1, D2, Ctxt),
+            maybe_parentheses(D3, Prec, Ctxt);
         receive_expr ->
             Ctxt1 = reset_prec(Ctxt),
             case {erl_syntax:receive_expr_clauses(Node), erl_syntax:receive_expr_timeout(Node)} of
@@ -1634,3 +1653,14 @@ get_node_text(Node) ->
 
 lay_double_colon(D1, D2, Ctxt) ->
     par([beside(D1, lay_text_float(" ::")), D2], Ctxt#ctxt.break_indent).
+
+lay_match_expression(Op, Pattern, D1, D2, Ctxt) ->
+    case erl_syntax:type(Pattern) == underscore
+         orelse erl_syntax:type(Pattern) == variable
+                andalso length(erl_syntax:variable_literal(Pattern)) < Ctxt#ctxt.break_indent
+    of
+        true -> %% Single short variable on the left, don't nest
+            follow(beside(D1, lay_text_float(Op)), D2, Ctxt#ctxt.break_indent);
+        false -> %% Large pattern, nesting makes sense
+            sep([beside(D1, lay_text_float(Op)), nest(Ctxt#ctxt.break_indent, D2)])
+    end.
diff --git a/src/formatters/otp_formatter.erl b/src/formatters/otp_formatter.erl
@@ -13,7 +13,10 @@
 -elvis([{elvis_style, atom_naming_convention, #{regex => "^([a-zA-Z][a-z0-9]*_?)*$"}},
         {elvis_style, dont_repeat_yourself, #{min_complexity => 20}}]).
 
--format #{inline_clause_bodies => true}.
+-format #{inline_clause_bodies => true, unquote_atoms => false}.
+
+%% erl_syntax functions that only appear in OTP25
+-dialyzer([no_missing_calls]).
 
 -behaviour(rebar3_formatter).
 -behaviour(rebar3_ast_formatter).
@@ -56,6 +59,7 @@
     case_expr |
     fun_expr |
     if_expr |
+    maybe_expr |
     receive_expr |
     try_expr |
     {function, prettypr:document()} |
@@ -78,7 +82,6 @@
 %% =====================================================================
 %% The following functions examine and modify contexts:
 
-%% @spec (context()) -> integer()
 %% @doc Returns the operator precedence field of the pretty-printer
 %% context.
 %%
@@ -87,7 +90,6 @@
 -spec get_ctxt_precedence(context()) -> integer().
 get_ctxt_precedence(Ctxt) -> Ctxt#ctxt.prec.
 
-%% @spec (context(), integer()) -> context()
 %%
 %% @doc Updates the operator precedence field of the pretty-printer
 %% context. See the {@link //stdlib/erl_parse} module for operator precedences.
@@ -104,14 +106,12 @@ set_prec(Ctxt, Prec) ->
 reset_prec(Ctxt) ->
     set_prec(Ctxt, 0).    % used internally
 
-%% @spec (context()) -> integer()
 %% @doc Returns the paper width field of the pretty-printer context.
 %% @see set_ctxt_paperwidth/2
 
 -spec get_ctxt_paperwidth(context()) -> integer().
 get_ctxt_paperwidth(Ctxt) -> Ctxt#ctxt.paper.
 
-%% @spec (context(), integer()) -> context()
 %%
 %% @doc Updates the paper width field of the pretty-printer context.
 %%
@@ -124,14 +124,12 @@ get_ctxt_paperwidth(Ctxt) -> Ctxt#ctxt.paper.
 -spec set_ctxt_paperwidth(context(), integer()) -> context().
 set_ctxt_paperwidth(Ctxt, W) -> Ctxt#ctxt{paper = W}.
 
-%% @spec (context()) -> integer()
 %% @doc Returns the line width field of the pretty-printer context.
 %% @see set_ctxt_linewidth/2
 
 -spec get_ctxt_linewidth(context()) -> integer().
 get_ctxt_linewidth(Ctxt) -> Ctxt#ctxt.ribbon.
 
-%% @spec (context(), integer()) -> context()
 %%
 %% @doc Updates the line width field of the pretty-printer context.
 %%
@@ -144,28 +142,24 @@ get_ctxt_linewidth(Ctxt) -> Ctxt#ctxt.ribbon.
 -spec set_ctxt_linewidth(context(), integer()) -> context().
 set_ctxt_linewidth(Ctxt, W) -> Ctxt#ctxt{ribbon = W}.
 
-%% @spec (context()) -> hook()
 %% @doc Returns the hook function field of the pretty-printer context.
 %% @see set_ctxt_hook/2
 
 -spec get_ctxt_hook(context()) -> hook().
 get_ctxt_hook(Ctxt) -> Ctxt#ctxt.hook.
 
-%% @spec (context(), hook()) -> context()
 %% @doc Updates the hook function field of the pretty-printer context.
 %% @see get_ctxt_hook/1
 
 -spec set_ctxt_hook(context(), hook()) -> context().
 set_ctxt_hook(Ctxt, Hook) -> Ctxt#ctxt{hook = Hook}.
 
-%% @spec (context()) -> term()
 %% @doc Returns the user data field of the pretty-printer context.
 %% @see set_ctxt_user/2
 
 -spec get_ctxt_user(context()) -> term().
 get_ctxt_user(Ctxt) -> Ctxt#ctxt.user.
 
-%% @spec (context(), term()) -> context()
 %% @doc Updates the user data field of the pretty-printer context.
 %% @see get_ctxt_user/1
 
@@ -188,30 +182,12 @@ init(_, _) -> nostate.
 format_file(File, nostate, Opts) -> rebar3_ast_formatter:format(File, ?MODULE, Opts).
 
 %% =====================================================================
-%% @spec format(Tree::syntaxTree()) -> string()
 %% @equiv format(Tree, [])
 
 -spec format(erl_syntax:syntaxTree()) -> string().
 format(Node) -> format(Node, [], #{}).
 
 %% =====================================================================
-%% @spec format(Tree::syntaxTree(), [pos_integer()], Options::rebar3_formatter:opts()) -> string()
-%%
-%% @type syntaxTree() = erl_syntax:syntaxTree().
-%%
-%% An abstract syntax tree. See the {@link erl_syntax} module for
-%% details.
-%%
-%% @type hook() = (syntaxTree(), context(), Continuation) ->
-%%                            prettypr:document()
-%%            Continuation = (syntaxTree(), context()) ->
-%%                            prettypr:document().
-%%
-%% A call-back function for user-controlled formatting. See {@link
-%% format/2}.
-%%
-%% @type context(). A representation of the current context of the
-%% pretty-printer. Can be accessed in hook functions.
 %%
 %% @doc Pretty-prints/formats an abstract Erlang syntax tree as text. For
 %% example, if you have a `.beam' file that has been compiled with
@@ -286,14 +262,12 @@ format(Node, EmptyLines, Options) ->
     binary_to_list(unicode:characters_to_binary(PreFormatted, E)).
 
 %% =====================================================================
-%% @spec best(Tree::syntaxTree()) -> empty | prettypr:document()
 %% @equiv best(Tree, [])
 
 -spec best(erl_syntax:syntaxTree()) -> empty | prettypr:document().
 best(Node) -> best(Node, []).
 
 %% =====================================================================
-%% @spec best(Tree::syntaxTree(), Options::[term()]) ->
 %%           empty | prettypr:document()
 %%
 %% @doc Creates a fixed "best" abstract layout for a syntax tree. This
@@ -314,14 +288,12 @@ best(Node, Options) ->
     prettypr:best(layout(Node, Options), W, L).
 
 %% =====================================================================
-%% @spec layout(Tree::syntaxTree()) -> prettypr:document()
 %% @equiv layout(Tree, [])
 
 -spec layout(erl_syntax:syntaxTree()) -> prettypr:document().
 layout(Node) -> layout(Node, []).
 
 %% =====================================================================
-%% @spec layout(Tree::syntaxTree(), Options::[term()]) -> prettypr:document()
 %%
 %% @doc Creates an abstract document layout for a syntax tree. The
 %% result represents a set of possible layouts (cf. module `prettypr').
@@ -505,6 +477,7 @@ lay_no_comments(Node, Ctxt) ->
                 {function, N} -> make_fun_clause(N, D1, D2, D3, Ctxt);
                 if_expr -> make_if_clause(D2, D3, Ctxt);
                 case_expr -> make_case_clause(D1, D2, D3, Ctxt);
+                maybe_expr -> make_case_clause(D1, D2, D3, Ctxt);
                 receive_expr -> make_case_clause(D1, D2, D3, Ctxt);
                 try_expr -> make_case_clause(D1, D2, D3, Ctxt);
                 undefined ->
@@ -545,6 +518,27 @@ lay_no_comments(Node, Ctxt) ->
             D1 = lay(erl_syntax:module_qualifier_argument(Node), set_prec(Ctxt, PrecL)),
             D2 = lay(erl_syntax:module_qualifier_body(Node), set_prec(Ctxt, PrecR)),
             beside(D1, beside(text(":"), D2));
+        maybe_expr ->
+            Ctxt1 = reset_prec(Ctxt),
+            D1 = vertical(seq(erl_syntax:maybe_expr_body(Node),
+                              floating(text(",")),
+                              Ctxt1,
+                              fun lay/2)),
+            Es0 = [text("end")],
+            Es1 = case erl_syntax:maybe_expr_else(Node) of
+                      none -> Es0;
+                      ElseNode ->
+                          ElseCs = erl_syntax:else_expr_clauses(ElseNode),
+                          D3 = lay_clauses(ElseCs, maybe_expr, Ctxt1),
+                          [text("else"), nest(Ctxt1#ctxt.break_indent, D3) | Es0]
+                  end,
+            sep([par([text("maybe"), nest(Ctxt1#ctxt.break_indent, D1), hd(Es1)]) | tl(Es1)]);
+        maybe_match_expr ->
+            {PrecL, Prec, PrecR} = inop_prec('='),
+            D1 = lay(erl_syntax:maybe_match_expr_pattern(Node), set_prec(Ctxt, PrecL)),
+            D2 = lay(erl_syntax:maybe_match_expr_body(Node), set_prec(Ctxt, PrecR)),
+            D3 = follow(beside(D1, floating(text(" ?="))), D2, Ctxt#ctxt.break_indent),
+            maybe_parentheses(D3, Prec, Ctxt);
         %%
         %% The rest is in alphabetical order (except map and types)
         %%
@@ -614,7 +608,7 @@ lay_no_comments(Node, Ctxt) ->
             Ctxt1 = reset_prec(Ctxt),
             Es = seq(erl_syntax:block_expr_body(Node), lay_text_float(","), Ctxt1, fun lay/2),
             sep([text("begin"), nest(Ctxt1#ctxt.break_indent, sep(Es)), text("end")]);
-        catch_expr ->
+        'catch_expr' ->
             {Prec, PrecR} = preop_prec('catch'),
             D = lay(erl_syntax:catch_expr_body(Node), set_prec(Ctxt, PrecR)),
             D1 = follow(text("catch"), D, Ctxt#ctxt.break_indent),

diff --git a/test/otp_formatter_SUITE.erl b/test/otp_formatter_SUITE.erl
@@ -17,7 +17,7 @@ test_app(_Config) ->
         case string:to_integer(
                  erlang:system_info(otp_release))
         of
-            {N, _} when N >= 23 ->
+            {N, _} when N >= 25 ->
                 {ignore,
                  ["src/*_ignore.erl",
                   "src/comments.erl",
@@ -33,7 +33,7 @@ test_app(_Config) ->
                   "src/dodge_macros.erl",
                   "src/macros_in_specs.erl",
                   "src/receive_after.erl",
-                  "src/otp23.erl"]}
+                  "src/otp25.erl"]}
         end,
     State2 = rebar_state:set(State1, format, [Files, Formatter, IgnoredFiles]),
     {error, _} = verify(State2),

diff --git a/test/test_app_SUITE.erl b/test/test_app_SUITE.erl
@@ -50,10 +50,10 @@ init_test_app() ->
         case string:to_integer(
                  erlang:system_info(otp_release))
         of
-            {N, []} when N >= 23 ->
+            {N, []} when N >= 25 ->
                 {ignore, ["src/*_ignore.erl", "src/ignored_file_config.erl"]};
             _ ->
-                {ignore, ["src/*_ignore.erl", "src/ignored_file_config.erl", "src/otp23.erl"]}
+                {ignore, ["src/*_ignore.erl", "src/ignored_file_config.erl", "src/otp25.erl"]}
         end,
     rebar_state:set(State1, format, [Files, IgnoredFiles]).