Merge branch 'egil/stdlib/fix-maps-alias/ERL-135/OTP-13534'

* egil/stdlib/fix-maps-alias/ERL-135/OTP-13534:
  stdlib: Strengthen map pattern tests
  stdlib: Add lint tests for parallel match of maps
  stdlib: Refactor erl_lint_SUITE
  stdlib: Fix linting of map key variables

2 files changed, 162 insertions, 91 deletions

diff --git a/lib/stdlib/src/erl_lint.erl b/lib/stdlib/src/erl_lint.erl
index b14102ac38..2508f96b91 100644
--- a/lib/stdlib/src/erl_lint.erl
+++ b/lib/stdlib/src/erl_lint.erl
@@ -1502,7 +1502,7 @@ pattern({op,_Line,'++',{string,_Li,_S},R}, Vt, Old, Bvt, St) ->
 pattern({match,_Line,Pat1,Pat2}, Vt, Old, Bvt, St0) ->
     {Lvt,Bvt1,St1} = pattern(Pat1, Vt, Old, Bvt, St0),
     {Rvt,Bvt2,St2} = pattern(Pat2, Vt, Old, Bvt, St1),
-    St3 = reject_bin_alias(Pat1, Pat2, St2),
+    St3 = reject_invalid_alias(Pat1, Pat2, Vt, St2),
     {vtmerge_pat(Lvt, Rvt),vtmerge_pat(Bvt1,Bvt2),St3};
 %% Catch legal constant expressions, including unary +,-.
 pattern(Pat, _Vt, _Old, _Bvt, St) ->
@@ -1517,56 +1517,77 @@ pattern_list(Ps, Vt, Old, Bvt0, St) ->
                   {vtmerge_pat(Pvt, Psvt),vtmerge_pat(Bvt,Bvt1),St1}
           end, {[],[],St}, Ps).
 
-%% reject_bin_alias(Pat, Expr, St) -> St'
+
+
+%% reject_invalid_alias(Pat, Expr, Vt, St) -> St'
 %%  Reject aliases for binary patterns at the top level.
+%%  Reject aliases for maps patterns at the top level.
+%%  The variables table (Vt) are for maps checkking.
+
+reject_invalid_alias_expr({bin,_,_}=P, {match,_,P0,E}, Vt, St0) ->
+    St = reject_invalid_alias(P, P0, Vt, St0),
+    reject_invalid_alias_expr(P, E, Vt, St);
+reject_invalid_alias_expr({map,_,_}=P, {match,_,P0,E}, Vt, St0) ->
+    St = reject_invalid_alias(P, P0, Vt, St0),
+    reject_invalid_alias_expr(P, E, Vt, St);
+reject_invalid_alias_expr({match,_,_,_}=P, {match,_,P0,E}, Vt, St0) ->
+    St = reject_invalid_alias(P, P0, Vt, St0),
+    reject_invalid_alias_expr(P, E, Vt, St);
+reject_invalid_alias_expr(_, _, _, St) -> St.
 
-reject_bin_alias_expr({bin,_,_}=P, {match,_,P0,E}, St0) ->
-    St = reject_bin_alias(P, P0, St0),
-    reject_bin_alias_expr(P, E, St);
-reject_bin_alias_expr({match,_,_,_}=P, {match,_,P0,E}, St0) ->
-    St = reject_bin_alias(P, P0, St0),
-    reject_bin_alias_expr(P, E, St);
-reject_bin_alias_expr(_, _, St) -> St.
 
 
-%% reject_bin_alias(Pat1, Pat2, St) -> St'
+%% reject_invalid_alias(Pat1, Pat2, St) -> St'
 %%  Aliases of binary patterns, such as <<A:8>> = <<B:4,C:4>> or even
 %%  <<A:8>> = <<A:8>>, are not allowed. Traverse the patterns in parallel
 %%  and generate an error if any binary aliases are found.
 %%    We generate an error even if is obvious that the overall pattern can't
 %%  possibly match, for instance, {a,<<A:8>>,c}={x,<<A:8>>} WILL generate an
 %%  error.
+%%    Maps should reject unbound variables here.
 
-reject_bin_alias({bin,Line,_}, {bin,_,_}, St) ->
+reject_invalid_alias({bin,Line,_}, {bin,_,_}, _, St) ->
     add_error(Line, illegal_bin_pattern, St);
-reject_bin_alias({cons,_,H1,T1}, {cons,_,H2,T2}, St0) ->
-    St = reject_bin_alias(H1, H2, St0),
-    reject_bin_alias(T1, T2, St);
-reject_bin_alias({tuple,_,Es1}, {tuple,_,Es2}, St) ->
-    reject_bin_alias_list(Es1, Es2, St);
-reject_bin_alias({record,_,Name1,Pfs1}, {record,_,Name2,Pfs2},
+reject_invalid_alias({map,_Line,Ps1}, {map,_,Ps2}, Vt, St0) ->
+    Fun = fun ({map_field_exact,L,{var,_,K},_V}, Sti) ->
+                  case is_var_bound(K,Vt) of
+                      true ->
+                          Sti;
+                      false ->
+                          add_error(L, {unbound_var,K}, Sti)
+                  end;
+              ({map_field_exact,_L,_K,_V}, Sti) ->
+                  Sti
+          end,
+    foldl(Fun, foldl(Fun, St0, Ps1), Ps2);
+reject_invalid_alias({cons,_,H1,T1}, {cons,_,H2,T2}, Vt, St0) ->
+    St = reject_invalid_alias(H1, H2, Vt, St0),
+    reject_invalid_alias(T1, T2, Vt, St);
+reject_invalid_alias({tuple,_,Es1}, {tuple,_,Es2}, Vt, St) ->
+    reject_invalid_alias_list(Es1, Es2, Vt, St);
+reject_invalid_alias({record,_,Name1,Pfs1}, {record,_,Name2,Pfs2}, Vt,
                  #lint{records=Recs}=St) ->
     case {dict:find(Name1, Recs),dict:find(Name2, Recs)} of
         {{ok,{_Line1,Fields1}},{ok,{_Line2,Fields2}}} ->
-	    reject_bin_alias_rec(Pfs1, Pfs2, Fields1, Fields2, St);
+	    reject_invalid_alias_rec(Pfs1, Pfs2, Fields1, Fields2, Vt, St);
         {_,_} ->
 	    %% One or more non-existing records. (An error messages has
 	    %% already been generated, so we are done here.)
 	    St
     end;
-reject_bin_alias({match,_,P1,P2}, P, St0) ->
-    St = reject_bin_alias(P1, P, St0),
-    reject_bin_alias(P2, P, St);
-reject_bin_alias(P, {match,_,_,_}=M, St) ->
-    reject_bin_alias(M, P, St);
-reject_bin_alias(_P1, _P2, St) -> St.
-
-reject_bin_alias_list([E1|Es1], [E2|Es2], St0) ->
-    St = reject_bin_alias(E1, E2, St0),
-    reject_bin_alias_list(Es1, Es2, St);
-reject_bin_alias_list(_, _, St) -> St.
-
-reject_bin_alias_rec(PfsA0, PfsB0, FieldsA0, FieldsB0, St) ->
+reject_invalid_alias({match,_,P1,P2}, P, Vt, St0) ->
+    St = reject_invalid_alias(P1, P, Vt, St0),
+    reject_invalid_alias(P2, P, Vt, St);
+reject_invalid_alias(P, {match,_,_,_}=M, Vt, St) ->
+    reject_invalid_alias(M, P, Vt, St);
+reject_invalid_alias(_P1, _P2, _Vt, St) -> St.
+
+reject_invalid_alias_list([E1|Es1], [E2|Es2], Vt, St0) ->
+    St = reject_invalid_alias(E1, E2, Vt, St0),
+    reject_invalid_alias_list(Es1, Es2, Vt, St);
+reject_invalid_alias_list(_, _, _, St) -> St.
+
+reject_invalid_alias_rec(PfsA0, PfsB0, FieldsA0, FieldsB0, Vt, St) ->
     %% We treat records as if they have been converted to tuples.
     PfsA1 = rbia_field_vars(PfsA0),
     PfsB1 = rbia_field_vars(PfsB0),
@@ -1582,7 +1603,7 @@ reject_bin_alias_rec(PfsA0, PfsB0, FieldsA0, FieldsB0, St) ->
     D = sofs:projection({external,fun({_,_,P1,_,P2}) -> {P1,P2} end}, C),
     E = sofs:to_external(D),
     {Ps1,Ps2} = lists:unzip(E),
-    reject_bin_alias_list(Ps1, Ps2, St).
+    reject_invalid_alias_list(Ps1, Ps2, Vt, St).
 
 rbia_field_vars(Fs) ->
     [{Name,Pat} || {record_field,_,{atom,_,Name},Pat} <- Fs].
@@ -2284,7 +2305,7 @@ expr({'catch',Line,E}, Vt, St0) ->
 expr({match,_Line,P,E}, Vt, St0) ->
     {Evt,St1} = expr(E, Vt, St0),
     {Pvt,Bvt,St2} = pattern(P, vtupdate(Evt, Vt), St1),
-    St = reject_bin_alias_expr(P, E, St2),
+    St = reject_invalid_alias_expr(P, E, Vt, St2),
     {vtupdate(Bvt, vtmerge(Evt, Pvt)),St};
 %% No comparison or boolean operators yet.
 expr({op,_Line,_Op,A}, Vt, St) ->
@@ -2381,7 +2402,7 @@ is_valid_call(Call) ->
         _ -> true
     end.
 
-%% is_valid_map_key(K,St) -> true | false
+%% is_valid_map_key(K) -> true | false
 %%   variables are allowed for patterns only at the top of the tree
 
 is_valid_map_key({var,_,_}) -> true;
@@ -3413,6 +3434,14 @@ warn_unused_vars(U, Vt, St0) ->
     UVt = map(fun ({V,{State,_,Ls}}) -> {V,{State,used,Ls}} end, U),
     {vtmerge(Vt, UVt), St1}.
 
+
+is_var_bound(V, Vt) ->
+    case orddict:find(V, Vt) of
+        {ok,{bound,_Usage,_}} -> true;
+        _ -> false
+    end.
+
+
 %% vtupdate(UpdVarTable, VarTable) -> VarTable.
 %%  Add the variables in the updated vartable to VarTable. The variables
 %%  will be updated with their property in UpdVarTable. The state of
diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl
index 6fea198af3..b0214e5238 100644
--- a/lib/stdlib/test/erl_lint_SUITE.erl
+++ b/lib/stdlib/test/erl_lint_SUITE.erl
@@ -33,47 +33,38 @@
 -define(privdir, proplists:get_value(priv_dir, Conf)).
 -endif.
 
--export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, 
-	 init_per_group/2,end_per_group/2, 
-	 init_per_testcase/2, end_per_testcase/2]).
-
--export([ 
-	  unused_vars_warn_basic/1, 
-	  unused_vars_warn_lc/1, 
-	  unused_vars_warn_rec/1,
-	  unused_vars_warn_fun/1, 
-	  unused_vars_OTP_4858/1,
-          unused_unsafe_vars_warn/1,
-	  export_vars_warn/1,
-	  shadow_vars/1,
-	  unused_import/1,
-	  unused_function/1,
-	  unsafe_vars/1,unsafe_vars2/1,
-	  unsafe_vars_try/1,
-	  unsized_binary_in_bin_gen_pattern/1,
-	  guard/1, otp_4886/1, otp_4988/1, otp_5091/1, otp_5276/1, otp_5338/1,
-	  otp_5362/1, otp_5371/1, otp_7227/1, otp_5494/1, otp_5644/1, otp_5878/1,
-	  otp_5917/1, otp_6585/1, otp_6885/1, otp_10436/1, otp_11254/1,
-          otp_11772/1, otp_11771/1, otp_11872/1,
-          export_all/1,
-	  bif_clash/1,
-	  behaviour_basic/1, behaviour_multiple/1, otp_11861/1,
-	  otp_7550/1,
-	  otp_8051/1,
-	  format_warn/1,
-	  on_load_successful/1, on_load_failing/1, 
-	  too_many_arguments/1,
-	  basic_errors/1,bin_syntax_errors/1,
-          predef/1,
-          maps/1,maps_type/1,otp_11851/1,otp_11879/1,otp_13230/1,
-	  record_errors/1
-        ]).
-
-init_per_testcase(_Case, Config) ->
-    Config.
-
-end_per_testcase(_Case, _Config) ->
-    ok.
+-export([all/0, suite/0, groups/0]).
+
+-export([unused_vars_warn_basic/1,
+         unused_vars_warn_lc/1,
+         unused_vars_warn_rec/1,
+         unused_vars_warn_fun/1,
+         unused_vars_OTP_4858/1,
+         unused_unsafe_vars_warn/1,
+         export_vars_warn/1,
+         shadow_vars/1,
+         unused_import/1,
+         unused_function/1,
+         unsafe_vars/1,unsafe_vars2/1,
+         unsafe_vars_try/1,
+         unsized_binary_in_bin_gen_pattern/1,
+         guard/1, otp_4886/1, otp_4988/1, otp_5091/1, otp_5276/1, otp_5338/1,
+         otp_5362/1, otp_5371/1, otp_7227/1, otp_5494/1, otp_5644/1, otp_5878/1,
+         otp_5917/1, otp_6585/1, otp_6885/1, otp_10436/1, otp_11254/1,
+         otp_11772/1, otp_11771/1, otp_11872/1,
+         export_all/1,
+         bif_clash/1,
+         behaviour_basic/1, behaviour_multiple/1, otp_11861/1,
+         otp_7550/1,
+         otp_8051/1,
+         format_warn/1,
+         on_load_successful/1, on_load_failing/1,
+         too_many_arguments/1,
+         basic_errors/1,bin_syntax_errors/1,
+         predef/1,
+         maps/1,maps_type/1,maps_parallel_match/1,
+         otp_11851/1,otp_11879/1,otp_13230/1,
+         record_errors/1]).
 
 suite() ->
     [{ct_hooks,[ts_install_cth]},
@@ -91,7 +82,8 @@ all() ->
      bif_clash, behaviour_basic, behaviour_multiple, otp_11861,
      otp_7550, otp_8051, format_warn, {group, on_load},
      too_many_arguments, basic_errors, bin_syntax_errors, predef,
-     maps, maps_type, otp_11851, otp_11879, otp_13230,
+     maps, maps_type, maps_parallel_match,
+     otp_11851, otp_11879, otp_13230,
      record_errors].
 
 groups() -> 
@@ -101,19 +93,6 @@ groups() ->
        unused_vars_OTP_4858, unused_unsafe_vars_warn]},
      {on_load, [], [on_load_successful, on_load_failing]}].
 
-init_per_suite(Config) ->
-    Config.
-
-end_per_suite(_Config) ->
-    ok.
-
-init_per_group(_GroupName, Config) ->
-    Config.
-
-end_per_group(_GroupName, Config) ->
-    Config.
-
-
 
 %% Warnings for unused variables in some simple cases.
 unused_vars_warn_basic(Config) when is_list(Config) ->
@@ -3583,8 +3562,6 @@ predef(Config) when is_list(Config) ->
     ok.
 
 maps(Config) ->
-    %% TODO: test key patterns, not done because map patterns are going to be
-    %% changed a lot.
     Ts = [{illegal_map_construction,
            <<"t() ->
                   #{ a := b,
@@ -3626,6 +3603,26 @@ maps(Config) ->
 	   {errors,[{1,erl_lint,illegal_map_construction},
                     {1,erl_lint,{unbound_var,'X'}}],
             []}},
+          {legal_map_pattern,
+	   <<"
+               -record(mapkey, {a=1,b=2}).
+               t(M,K1) ->
+                     #{ a := 1,
+                        $a := 1, $z := 99,
+                        #{a=>val} := 2,
+                        K1 := 1337,
+                        #mapkey{a = 10} := wat,
+                        #{{a,val}=>val} := 2,
+                        #{ \"hi\" => wazzup, hi => ho } := yep,
+                        ok := 1.0,
+                        [3+3] := nope,
+                        1.0 := yep,
+                        {3.0+3} := nope,
+                        {yep} := yep
+                      } = M.
+	   ">>,
+	   [],
+	   []},
 	  {legal_map_construction,
 	   <<"t(V) -> #{ a => 1,
 			#{a=>V} => 2,
@@ -3692,6 +3689,51 @@ maps_type(Config) when is_list(Config) ->
     [] = run(Config, Ts),
     ok.
 
+maps_parallel_match(Config) when is_list(Config) ->
+    Ts = [{parallel_map_patterns_unbound1,
+           <<"
+           t(#{} = M) ->
+               #{K := V} = #{k := K} = M,
+               V.
+           ">>,
+           [],
+           {errors,[{3,erl_lint,{unbound_var,'K'}}],[]}},
+          {parallel_map_patterns_unbound2,
+           <<"
+           t(#{} = M) ->
+               #{K1 := V1} =
+               #{K2 := V2} =
+               #{k1 := K1,k2 := K2} = M,
+               [V1,V2].
+           ">>,
+           [],
+           {errors,[{3,erl_lint,{unbound_var,'K1'}},
+                    {3,erl_lint,{unbound_var,'K1'}},
+                    {4,erl_lint,{unbound_var,'K2'}},
+                    {4,erl_lint,{unbound_var,'K2'}}],[]}},
+          {parallel_map_patterns_bound,
+           <<"
+           t(#{} = M,K1,K2) ->
+               #{K1 := V1} =
+               #{K2 := V2} =
+               #{k1 := K1,k2 := K2} = M,
+               [V1,V2].
+           ">>,
+           [],
+           []},
+          {parallel_map_patterns_literal,
+           <<"
+           t(#{} = M) ->
+               #{k1 := V1} =
+               #{k2 := V2} =
+               #{k1 := V1,k2 := V2} = M,
+               [V1,V2].
+           ">>,
+           [],
+           []}],
+    [] = run(Config, Ts),
+    ok.
+
 %% OTP-11851: More atoms can be used as type names + bug fixes.
 otp_11851(Config) when is_list(Config) ->
     Ts = [