Merge pull request #983 from gomoripeti/bound_vars_in_fun_exprs

Fix bogus bound variable warnings in fun expression heads
erlang-ls · Apr 16, 2021 · c3ba39d · c3ba39d
2 parents b89dfa2 + abd7eb7
commit c3ba39d
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 3 deletions.
diff --git a/apps/els_lsp/priv/code_navigation/src/diagnostics_bound_var_in_pattern.erl b/apps/els_lsp/priv/code_navigation/src/diagnostics_bound_var_in_pattern.erl
@@ -1,6 +1,6 @@
 -module(diagnostics_bound_var_in_pattern).
 
--export([f/1, g/1, h/2]).
+-export([f/1, g/1, h/2, fun_expr/1, named_fun_expr/0]).
 
 f(Var1) ->
   Var1 = 1.
@@ -18,3 +18,14 @@ h(Var3, Var4) ->
   catch Var4 ->
       error
   end.
+
+fun_expr(New) ->
+  fun(New, Var5) ->
+      Var5 = New
+  end.
+
+named_fun_expr() ->
+  fun F(New, Var6) ->
+      New = Var6,
+      F = Var6
+  end.
diff --git a/apps/els_lsp/src/els_bound_var_in_pattern_diagnostics.erl b/apps/els_lsp/src/els_bound_var_in_pattern_diagnostics.erl
@@ -61,7 +61,7 @@ find_vars_in_form(Form) ->
     function ->
       AnnotatedForm = erl_syntax_lib:annotate_bindings(Form, []),
       %% There are no bound variables in function heads or guards
-      %% so lets decend straight into the bodies
+      %% so lets descend straight into the bodies
       Clauses = erl_syntax:function_clauses(AnnotatedForm),
       ClauseBodies = lists:map(fun erl_syntax:clause_body/1, Clauses),
       fold_subtrees(ClauseBodies, []);
@@ -76,6 +76,19 @@ fold_subtrees(Subtrees, Acc) ->
 -spec find_vars_in_tree(tree(), [poi()]) -> [poi()].
 find_vars_in_tree(Tree, Acc) ->
   case erl_syntax:type(Tree) of
+    Type when Type =:= fun_expr;
+              Type =:= named_fun_expr ->
+      %% There is no bound variables in fun expression heads,
+      %% because they shadow whatever is in the input env
+      %% so lets descend straight into the bodies
+      %% (This is a workaround for erl_syntax_lib not considering
+      %%  shadowing in fun expressions)
+      Clauses = case Type of
+                  fun_expr -> erl_syntax:fun_expr_clauses(Tree);
+                  named_fun_expr -> erl_syntax:named_fun_expr_clauses(Tree)
+                end,
+      ClauseBodies = lists:map(fun erl_syntax:clause_body/1, Clauses),
+      fold_subtrees(ClauseBodies, Acc);
     match_expr ->
       Pattern = erl_syntax:match_expr_pattern(Tree),
       NewAcc = fold_pattern(Pattern, Acc),

diff --git a/apps/els_lsp/test/els_diagnostics_SUITE.erl b/apps/els_lsp/test/els_diagnostics_SUITE.erl
@@ -176,10 +176,30 @@ bound_var_in_pattern(Config) ->
           #{'end' => #{character => 12, line => 17},
             start => #{character => 8, line => 17}},
         severity => 4,
+        source => <<"BoundVarInPattern">>},
+      #{message => <<"Bound variable in pattern: Var5">>,
+        range =>
+          #{'end' => #{character => 10, line => 23},
+            start => #{character => 6, line => 23}},
+        severity => 4,
         source => <<"BoundVarInPattern">>}
+      %% erl_syntax_lib:annotate_bindings does not handle named funs correctly
+      %% #{message => <<"Bound variable in pattern: New">>,
+      %%   range =>
+      %%     #{'end' => #{character => 9, line => 28},
+      %%       start => #{character => 6, line => 28}},
+      %%   severity => 4,
+      %%   source => <<"BoundVarInPattern">>},
+      %% #{message => <<"Bound variable in pattern: F">>,
+      %%   range =>
+      %%     #{'end' => #{character => 7, line => 29},
+      %%       start => #{character => 6, line => 29}},
+      %%   severity => 4,
+      %%   source => <<"BoundVarInPattern">>}
     ],
   F = fun(#{message := M1}, #{message := M2}) -> M1 =< M2 end,
-  ?assertEqual(Expected, lists:sort(F, Diagnostics)),
+  Hints = [D || #{severity := ?DIAGNOSTIC_HINT} = D <- Diagnostics],
+  ?assertEqual(Expected, lists:sort(F, Hints)),
   ok.
 
 -spec compiler(config()) -> ok.