Always warn about exported variables from constructs without clauses

There is never a good reason for placing variable bindings inside the
arguments of an expression.

Author: richcarl

lib/stdlib/src/erl_lint.erl

@@ -451,6 +451,13 @@ format_error_1({unbound_var,V,GuessV}) ->
 format_error_1({unsafe_var,V,{What,Where}}) ->
     {~"variable ~w unsafe in ~w ~s",
                   [V,What,format_where(Where)]};
+format_error_1({exported_subexpr_var,V,{What,Where}}) ->
+    {~"""
+      variable ~w exported from ~w ~s.
+      Exporting bindings from subexpressions is deprecated and
+      may be removed in a future version of Erlang/OTP.
+      You should move the binding of ~w before the ~w.
+      """, [V,What,format_where(Where),V,What]};
 format_error_1({exported_var,V,{What,Where}}) ->
     {~"variable ~w exported from ~w ~s",
                   [V,What,format_where(Where)]};
@@ -2699,21 +2706,27 @@ expr({atom,Anno,A}, _Vt, St) ->
     {[],keyword_warning(Anno, A, St)};
 expr({string,_Anno,_S}, _Vt, St) -> {[],St};
 expr({nil,_Anno}, _Vt, St) -> {[],St};
-expr({cons,_Anno,H,T}, Vt, St) ->
-    expr_list([H,T], Vt, St);
+expr({cons,Anno,H,T}, Vt, St) ->
+    vtupd_export_expr_list({list, Anno}, [H, T], Vt, St);
 expr({lc,_Anno,E,Qs}, Vt, St) ->
     handle_comprehension(E, Qs, Vt, St);
 expr({bc,_Anno,E,Qs}, Vt, St) ->
     handle_comprehension(E, Qs, Vt, St);
 expr({mc,_Anno,E,Qs}, Vt, St) ->
     handle_comprehension(E, Qs, Vt, St);
-expr({tuple,_Anno,Es}, Vt, St) ->
-    expr_list(Es, Vt, St);
-expr({map,_Anno,Es}, Vt, St) ->
-    map_fields(Es, Vt, check_assoc_fields(Es, St), fun expr_list/3);
+expr({tuple,Anno,Es}, Vt, St) ->
+    vtupd_export_expr_list({tuple, Anno}, Es, Vt, St);
+expr({map,Anno,Es}, Vt, St) ->
+    map_fields(Es, Vt, check_assoc_fields(Es, St),
+               fun(Es0, Vt0, St0) ->
+                       vtupd_export_expr_list({map, Anno}, Es0, Vt0, St0)
+               end);
 expr({map,Anno,Src,Es}, Vt, St) ->
-    {Svt,St1} = expr(Src, Vt, St),
-    {Fvt,St2} = map_fields(Es, Vt, St1, fun expr_list/3),
+    {Svt,St1} = vtupd_export_expr_list({map, Anno}, [Src], Vt, St),
+    {Fvt,St2} = map_fields(Es, Vt, St1,
+                           fun(Es0, Vt0, St0) ->
+                                   vtupd_export_expr_list({map, Anno}, Es0, Vt0, St0)
+                           end),
     {vtupdate(Svt, Fvt), warn_if_literal_update(Anno, Src, St2)};
 expr({record_index,Anno,Name,Field}, _Vt, St) ->
     check_record(Anno, Name, St,
@@ -2740,11 +2753,13 @@ expr({record,Anno,Rec,Name,Upds}, Vt, St0) ->
         no -> {vtmerge(Rvt, Usvt), warn_if_literal_update(Anno, Rec, St2)};
         WildAnno -> {[],add_error(WildAnno, {wildcard_in_update,Name}, St2)}
     end;
-expr({bin,_Anno,Fs}, Vt, St) ->
-    expr_bin(Fs, Vt, St, fun expr/3);
-expr({block,_Anno,Es}, Vt, St) ->
+expr({bin,Anno,Fs}, Vt, St) ->
+    {Vt1, St1} = expr_bin(Fs, Vt, St, fun expr/3),
+    {vtupd_export({binary, Anno}, Vt1, Vt), St1};
+expr({block,Anno,Es}, Vt, St) ->
     %% Unfold block into a sequence.
-    exprs(Es, Vt, St);
+    {Vt1, St1} = exprs(Es, Vt, St),
+    {vtupd_export({'begin', Anno}, Vt1, Vt), St1};
 expr({'if',Anno,Cs}, Vt, St) ->
     icrt_clauses(Cs, {'if',Anno}, Vt, St);
 expr({'case',Anno,E,Cs}, Vt, St0) ->
@@ -2816,7 +2831,7 @@ expr({call,Anno,{remote,_Ar,{atom,_Am,M},{atom,Af,F}},As}, Vt, St0) ->
     St2 = check_remote_function(Anno, M, F, As, St1),
     St3 = check_module_name(M, Anno, St2),
     St4 = check_remote_self_call(Anno, M, F, length(As), St3),
-    expr_list(As, Vt, St4);
+    vtupd_export_expr_list({call, Anno}, As, Vt, St4);
 expr({call,Anno,{remote,_Ar,M,F},As}, Vt, St0) ->
     St1 = keyword_warning(Anno, M, St0),
     St2 = keyword_warning(Anno, F, St1),
@@ -2826,14 +2841,14 @@ expr({call,Anno,{remote,_Ar,M,F},As}, Vt, St0) ->
               _ ->
                   St2
           end,
-    expr_list([M,F|As], Vt, St3);
+    vtupd_export_expr_list({call, Anno}, [M, F | As], Vt, St3);
 expr({call,Anno,{atom,Aa,F},As}, Vt, St0) ->
     St1 = keyword_warning(Aa, F, St0),
-    {Asvt,St2} = expr_list(As, Vt, St1),
+    {Asvt,St2} = vtupd_export_expr_list({call, Anno}, As, Vt, St1),
     {Asvt, check_call(Anno, F, As, Aa, St2)};
 expr({call,Anno,F,As}, Vt, St0) ->
     St = warn_invalid_call(Anno,F,St0),
-    expr_list([F|As], Vt, St);                  %They see the same variables
+    vtupd_export_expr_list({call, Anno}, [F | As], Vt, St); %They see the same variables
 expr({'try',Anno,Es,Scs,Ccs,As}, Vt, St0) ->
     %% The only exports we allow are from the try expressions to the
     %% success clauses.
@@ -2880,19 +2895,19 @@ expr({'maybe',MaybeAnno,Es,{'else',ElseAnno,Cs}}, Vt, St) ->
     Cvt2 = vtmerge(Cvt0, Cvt1),
     {vtmerge(Evt2, Cvt2),St2};
 %% No comparison or boolean operators yet.
-expr({op,_Anno,_Op,A}, Vt, St) ->
-    expr(A, Vt, St);
+expr({op,Anno,Op,A}, Vt, St) ->
+    vtupd_export_expr_list({Op, Anno}, [A], Vt, St);
 expr({op,Anno,Op,L,R}, Vt, St0) when Op =:= 'orelse'; Op =:= 'andalso' ->
-    {Evt1,St1} = expr(L, Vt, St0),
+    {Evt1, St1} = vtupd_export_expr_list({Op, Anno}, [L], Vt, St0),
     Vt1 = vtupdate(Evt1, Vt),
     {Evt2,St2} = expr(R, Vt1, St1),
     Evt3 = vtupd_unsafe({Op, Anno}, Evt2, Vt1),
     {vtmerge(Evt1, Evt3),St2};
-expr({op,_Anno,EqOp,L,R}, Vt, St0) when EqOp =:= '=:='; EqOp =:= '=/=' ->
+expr({op,Anno,EqOp,L,R}, Vt, St0) when EqOp =:= '=:='; EqOp =:= '=/=' ->
     St = expr_check_match_zero(R, expr_check_match_zero(L, St0)),
-    expr_list([L,R], Vt, St);                   %They see the same variables
-expr({op,_Anno,_Op,L,R}, Vt, St) ->
-    expr_list([L,R], Vt, St);                   %They see the same variables
+    vtupd_export_expr_list({EqOp, Anno}, [L, R], Vt, St); %They see the same variables
+expr({op,Anno,Op,L,R}, Vt, St) ->
+    vtupd_export_expr_list({Op, Anno}, [L, R], Vt, St); %They see the same variables
 %% The following are not allowed to occur anywhere!
 expr({remote,_Anno,M,_F}, _Vt, St) ->
     {[],add_error(erl_parse:first_anno(M), illegal_expr, St)};
@@ -2973,6 +2988,12 @@ expr_list(Es, Vt, St0) ->
                   vtmerge_pat(Evt, Esvt, St2)
           end, {[], St0}, Es).
 
+%% as expr_list but mark new vars as exported
+
+vtupd_export_expr_list(Where, Es, Vt, St) ->
+    {Evt, St1} = expr_list(Es, Vt, St),
+    {vtupd_export(Where, Evt, Vt), St1}.
+
 record_expr(Anno, Rec, Vt, St0) ->
     St1 = warn_invalid_record(Anno, Rec, St0),
     expr(Rec, Vt, St1).
@@ -4380,12 +4401,18 @@ do_expr_var(V, Anno, Vt, St) ->
             {[{V,{bound,used,As}}],
              add_error(Anno, {unsafe_var,V,In}, St)};
         {ok,{{export,From},_Usage,As}} ->
-            case is_warn_enabled(export_vars, St) of
+            case exported_subexpr_var(From) of
                 true ->
                     {[{V,{bound,used,As}}],
-                     add_warning(Anno, {exported_var,V,From}, St)};
+                     add_warning(Anno, {exported_subexpr_var,V,From}, St)};
                 false ->
-                    {[{V,{{export,From},used,As}}],St}
+                    case is_warn_enabled(export_vars, St) of
+                        true ->
+                            {[{V,{bound,used,As}}],
+                             add_warning(Anno, {exported_var,V,From}, St)};
+                        false ->
+                            {[{V,{{export,From},used,As}}],St}
+                    end
             end;
         {ok,{stacktrace,_Usage,As}} ->
             {[{V,{bound,used,As}}],
@@ -4408,6 +4435,14 @@ exported_var(Anno, V, From, St) ->
         false -> St
     end.
 
+%% warn about exporting from non-block subexpressions
+exported_subexpr_var({'if',_}) -> false;
+exported_subexpr_var({'case',_}) -> false;
+exported_subexpr_var({'receive',_}) -> false;
+exported_subexpr_var({'try',_}) -> false;
+exported_subexpr_var({'begin',_}) -> false;
+exported_subexpr_var(_) -> true.
+
 shadow_vars(Vt, Vt0, In, St0) ->
     case is_warn_enabled(shadow_vars, St0) of
         true ->
@@ -4473,6 +4508,16 @@ vtunsafe({Tag,Anno}, Uvt, Vt) ->
 vtupd_unsafe(Where, NewVt, OldVt) ->
     vtupdate(vtunsafe(Where, NewVt, OldVt), NewVt).
 
+%% vtexport(From, UpdVarTable, VarTable) -> ExpVarTable.
+%%  Return all new variables in UpdVarTable as exported.
+
+vtexport({Tag, FileLine}, Uvt, Vt) ->
+    Line = erl_anno:location(FileLine),
+    [{V, {{export, {Tag, Line}}, U, Ls}} || {V, {_, U, Ls}} <- vtnew(Uvt, Vt)].
+
+vtupd_export(Where, NewVt, OldVt) ->
+    vtupdate(vtexport(Where, NewVt, OldVt), NewVt).
+
 %% vtmerge(VarTable, VarTable) -> VarTable.
 %%  Merge two variables tables generating a new vartable. Give priority to
 %%  errors then warnings.