Merge pull request #920 from gomoripeti/fix_infinite_loop_macro_attrib

Fix infinite loop when parsing macro in wild attribute name
erlang-ls · Feb 20, 2021 · 048632e · 048632e
2 parents df50bd4 + 1c2ffcb
commit 048632e
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 5 deletions.
diff --git a/apps/els_lsp/src/els_parser.erl b/apps/els_lsp/src/els_parser.erl
@@ -128,16 +128,26 @@ analyze_attribute(Tree) ->
       [FATree | _] = erl_syntax:tuple_elements(ArgTuple),
       Definition = [], %% ignore definition
       %% concrete will throw an error if `FATRee' contains any macro
-      {AttrName, {AttrName, {erl_syntax:concrete(FATree), Definition}}};
+      try erl_syntax:concrete(FATree) of
+        FA ->
+          {AttrName, {AttrName, {FA, Definition}}}
+      catch _:_ ->
+          throw(syntax_error)
+      end;
     AttrName when AttrName =:= opaque;
                   AttrName =:= type ->
       [ArgTuple] = erl_syntax:attribute_arguments(Tree),
       [TypeTree, _, ArgsListTree] = erl_syntax:tuple_elements(ArgTuple),
       Definition = [], %% ignore definition
       %% concrete will throw an error if `TyperTree' is a macro
-      {AttrName, {AttrName, {erl_syntax:concrete(TypeTree),
-                             Definition,
-                             erl_syntax:list_elements(ArgsListTree)}}};
+      try erl_syntax:concrete(TypeTree) of
+        TypeName ->
+          {AttrName, {AttrName, {TypeName,
+                                 Definition,
+                                 erl_syntax:list_elements(ArgsListTree)}}}
+      catch _:_ ->
+          throw(syntax_error)
+      end;
     _ ->
       erl_syntax_lib:analyze_attribute(Tree)
   end.
@@ -637,7 +647,7 @@ attribute_name_atom(Tree) ->
     atom ->
       erl_syntax:atom_value(NameNode);
     _ ->
-      Tree
+      NameNode
   end.
 
 -spec attribute_subtrees(atom() | tree(), [tree()]) -> [[tree()]].

diff --git a/apps/els_lsp/test/els_parser_SUITE.erl b/apps/els_lsp/test/els_parser_SUITE.erl
@@ -24,6 +24,9 @@
         , spec_macro/1
         , type_macro/1
         , opaque_macro/1
+        , wild_attrbibute_macro/1
+        , type_name_macro/1
+        , spec_name_macro/1
         ]).
 
 %%==============================================================================
@@ -206,6 +209,32 @@ opaque_macro(_Config) ->
   ?assertMatch([_], parse_find_pois(Text, macro, 'M')),
   ok.
 
+-spec wild_attrbibute_macro(config()) -> ok.
+wild_attrbibute_macro(_Config) ->
+  Text = "-?M(foo).",
+  ?assertMatch([_], parse_find_pois(Text, macro, 'M')),
+  ?assertMatch([_], parse_find_pois(Text, atom, foo)),
+  ok.
+
+type_name_macro(_Config) ->
+  %% Currently els_dodger cannot parse this type definition
+  %% Verify this does not prevent parsing following forms
+  Text = "-type ?M() -> integer() | t(). -spec f() -> any().",
+  ?assertMatch({ok, [#{kind := spec, id := {f, 0}}]}, els_parser:parse(Text)),
+  ok.
+
+spec_name_macro(_Config) ->
+  %% Currently els_dodger cannot parse this
+  %% We can only find a spec-context
+  Text1 = "-spec ?M() -> integer() | t().",
+  ?assertMatch({ok, [#{kind := spec, id := undefined}]}, els_parser:parse(Text1)),
+
+  %% Verify the parser does not crash on macros in spec function names
+  %% and it still returns POIs from the definition body
+  Text2 = "-spec ?MODULE:b() -> integer() | t().",
+  ?assertMatch([_], parse_find_pois(Text2, type_application, {t, 0})),
+  ok.
+
 %%==============================================================================
 %% Helper functions
 %%==============================================================================