diff --git a/apps/els_core/src/els_dodger.erl b/apps/els_core/src/els_dodger.erl index 1cd05e527..ed695d26e 100644 --- a/apps/els_core/src/els_dodger.erl +++ b/apps/els_core/src/els_dodger.erl @@ -877,8 +877,9 @@ rewrite(Node) -> _ -> Node end; - tuple -> - case erl_syntax:tuple_elements(Node) of + Type when Type =:= tuple; + Type =:= tuple_type -> + case tuple_elements(Node, Type) of [MagicWord, A | As] -> case erl_syntax:type(MagicWord) of atom -> @@ -899,9 +900,15 @@ rewrite(Node) -> rewrite_1(Node) end. +-spec tuple_elements(erl_syntax:syntaxTree(), atom()) -> [erl_syntax:syntaxTree()]. +tuple_elements(Node, tuple) -> + erl_syntax:tuple_elements(Node); +tuple_elements(Node, tuple_type) -> + erl_syntax:tuple_type_elements(Node). + -spec rewrite_1(erl_syntax:syntaxTree()) -> erl_syntax:syntaxTree(). rewrite_1(Node) -> - case erl_syntax:subtrees(Node) of + case subtrees(Node) of [] -> Node; Gs -> @@ -911,6 +918,67 @@ rewrite_1(Node) -> erl_syntax:copy_pos(Node, Node1) end. +%% @doc Return the list of all subtrees of a syntax tree with special handling +%% for type attributes. +%% +%% Background: In erl_parse AST the arguments of a wild attribute are +%% represented as plain terms. To make response consistent +%% `attribute_arguments/1' returns the abstract format of those terms. However +%% `erl_syntax' doesn't know that `callback', `spec', `type' and `opaque' are +%% not wild attributes and have their arguments partially in abstract format +%% already. So `erl_syntax' returns the AST of the AST for these attributes. To +%% fix this we need to convert them back with `concrete/1' to be able to +%% properly traverse them. This is necessary to be able to find and rewrite +%% special expressions representing macros. +-spec subtrees(erl_syntax:syntaxTree()) -> [[erl_syntax:syntaxTree()]]. +subtrees(Node) -> + case is_type_attribute(Node) of + {true, AttrName} -> + [[erl_syntax:attribute_name(Node)], + type_attribute_arguments(Node, AttrName)]; + false -> + erl_syntax:subtrees(Node) + end. + +-spec is_type_attribute(erl_syntax:syntaxTree()) -> {true, atom()} | false. +is_type_attribute(Node) -> + case erl_syntax:type(Node) of + attribute -> + NameNode = erl_syntax:attribute_name(Node), + case erl_syntax:type(NameNode) of + atom -> + AttrName = erl_syntax:atom_value(NameNode), + case lists:member(AttrName, [callback, spec, type, opaque]) of + true -> + {true, AttrName}; + false -> + false + end; + _ -> + false + end; + _ -> + false + end. + +-spec type_attribute_arguments(erl_syntax:syntaxTree(), atom()) + -> [erl_syntax:syntaxTree()]. +type_attribute_arguments(Node, AttrName) when AttrName =:= callback; + AttrName =:= spec -> + [Arg] = erl_syntax:attribute_arguments(Node), + {FA, DefinitionClauses} = erl_syntax:concrete(Arg), + [erl_syntax:tuple([erl_syntax:abstract(FA), + erl_syntax:list(DefinitionClauses)])]; +type_attribute_arguments(Node, AttrName) when AttrName =:= opaque; + AttrName =:= type -> + [Arg] = erl_syntax:attribute_arguments(Node), + {TypeName, Definition, TypeArgs} = erl_syntax:concrete(Arg), + [erl_syntax:tuple([erl_syntax:abstract(TypeName), + Definition, + erl_syntax:list(TypeArgs)])]. + + + %% attempting a rescue operation on a token sequence for a single form %% if it could not be parsed after the normal treatment diff --git a/apps/els_lsp/src/els_parser.erl b/apps/els_lsp/src/els_parser.erl index 3ea9aad1f..1419bd7d4 100644 --- a/apps/els_lsp/src/els_parser.erl +++ b/apps/els_lsp/src/els_parser.erl @@ -65,7 +65,7 @@ parse_form(IoDevice, StartLocation, Parser, _Options) -> {eof, _EndLocation} = Eof -> Eof end. -%% @Doc Find POIs in attributes additionally using tokens to add location info +%% @doc Find POIs in attributes additionally using tokens to add location info %% missing from the syntax tree. Other attributes which don't need tokens are %% processed in `attribute/1'. -spec find_attribute_pois(erl_syntax:syntaxTree(), [erl_scan:token()]) -> @@ -73,7 +73,7 @@ parse_form(IoDevice, StartLocation, Parser, _Options) -> find_attribute_pois(Tree, Tokens) -> case erl_syntax:type(Tree) of attribute -> - try erl_syntax_lib:analyze_attribute(Tree) of + try analyze_attribute(Tree) of {export, Exports} -> %% The first atom is the attribute name, so we skip it. [_|Atoms] = [T || {atom, _, _} = T <- Tokens], @@ -111,6 +111,37 @@ find_attribute_pois(Tree, Tokens) -> [] end. +%% @doc Analyze an attribute node with special handling for type attributes. +%% +%% `erl_syntax_lib:analyze_attribute` can't handle macros in wild attribute +%% arguments. It also handles `callback', `spec', `type' and `opaque' as wild +%% attributes. Therefore `els_dodger' has to handle these forms specially and +%% here we have to adopt to the different output of `els_dodger'. +%% +%% @see els_dodger:subtrees/1 +-spec analyze_attribute(tree()) -> {atom(), term()} | preprocessor. +analyze_attribute(Tree) -> + case attribute_name_atom(Tree) of + AttrName when AttrName =:= callback; + AttrName =:= spec -> + [ArgTuple] = erl_syntax:attribute_arguments(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}}}; + 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)}}}; + _ -> + erl_syntax_lib:analyze_attribute(Tree) + end. + -spec find_compile_options_pois([any()] | tuple(), [erl_scan:token()]) -> [poi()]. find_compile_options_pois(CompileOpts, Tokens) when is_tuple(CompileOpts) -> @@ -234,7 +265,7 @@ application_with_variable(Operator, A) -> -spec attribute(tree()) -> [poi()]. attribute(Tree) -> Pos = erl_syntax:get_pos(Tree), - try erl_syntax_lib:analyze_attribute(Tree) of + try analyze_attribute(Tree) of %% Yes, Erlang allows both British and American spellings for %% keywords. {behavior, {behavior, Behaviour}} -> @@ -590,11 +621,7 @@ subtrees(Tree, type_application) -> , erl_syntax:type_application_arguments(Tree) ]; subtrees(Tree, attribute) -> - NameNode = erl_syntax:attribute_name(Tree), - AttrName = case erl_syntax:type(NameNode) of - atom -> erl_syntax:atom_value(NameNode); - _ -> NameNode - end, + AttrName = attribute_name_atom(Tree), Args = case erl_syntax:attribute_arguments(Tree) of none -> []; Args0 -> Args0 @@ -603,13 +630,16 @@ subtrees(Tree, attribute) -> subtrees(Tree, _) -> erl_syntax:subtrees(Tree). -%% Note: In erl_parse AST the arguments of a wild attribute are represented as -%% plain terms. To make response consistent `attribute_arguments/1' returns the -%% abstract format of those terms. However `erl_syntax' doesn't know that -%% `callback', `spec', `type' and `opaque' are not wild attributes and have -%% their arguments in abstract format already. So `erl_syntax' returns the AST -%% of the AST for these attributes. To fix this we need to convert them back -%% with `concrete/1' to be able to properly traverse them. +-spec attribute_name_atom(tree()) -> atom() | tree(). +attribute_name_atom(Tree) -> + NameNode = erl_syntax:attribute_name(Tree), + case erl_syntax:type(NameNode) of + atom -> + erl_syntax:atom_value(NameNode); + _ -> + Tree + end. + -spec attribute_subtrees(atom() | tree(), [tree()]) -> [[tree()]]. attribute_subtrees(AttrName, [Mod]) when AttrName =:= module; @@ -629,19 +659,8 @@ attribute_subtrees(AttrName, _) when AttrName =:= include; AttrName =:= include_lib -> []; -attribute_subtrees(AttrName, [Arg]) - when AttrName =:= callback; - AttrName =:= spec -> - {_FA, DefinitionClauses} = erl_syntax:concrete(Arg), - [DefinitionClauses]; -attribute_subtrees(AttrName, [Arg]) - when AttrName =:= type; - AttrName =:= opaque -> - {_TypeName, Definition, TypeArgs} = erl_syntax:concrete(Arg), - [TypeArgs, [Definition]]; attribute_subtrees(AttrName, Args) when is_atom(AttrName) -> - %% compile, export, export_type and wild attributes [Args]; attribute_subtrees(AttrName, Args) -> %% Attribute name not an atom, probably a macro diff --git a/apps/els_lsp/test/els_parser_SUITE.erl b/apps/els_lsp/test/els_parser_SUITE.erl index 77a85ef48..80aa8e347 100644 --- a/apps/els_lsp/test/els_parser_SUITE.erl +++ b/apps/els_lsp/test/els_parser_SUITE.erl @@ -20,6 +20,10 @@ , types_recursive/1 , opaque_recursive/1 , record_def_recursive/1 + , callback_macro/1 + , spec_macro/1 + , type_macro/1 + , opaque_macro/1 ]). %%============================================================================== @@ -174,6 +178,34 @@ assert_recursive_types(Text) -> parse_find_pois(Text, type_application)), ok. +-spec callback_macro(config()) -> ok. +callback_macro(_Config) -> + Text = "-callback foo() -> ?M().", + ?assertMatch([_], parse_find_pois(Text, callback, {foo, 0})), + ?assertMatch([_], parse_find_pois(Text, macro, 'M')), + ok. + +-spec spec_macro(config()) -> ok. +spec_macro(_Config) -> + Text = "-spec foo() -> ?M().", + ?assertMatch([_], parse_find_pois(Text, spec, {foo, 0})), + ?assertMatch([_], parse_find_pois(Text, macro, 'M')), + ok. + +-spec type_macro(config()) -> ok. +type_macro(_Config) -> + Text = "-type t() :: ?M(a, b).", + ?assertMatch([_], parse_find_pois(Text, type_definition, {t, 0})), + ?assertMatch([_], parse_find_pois(Text, macro, 'M')), + ok. + +-spec opaque_macro(config()) -> ok. +opaque_macro(_Config) -> + Text = "-opaque o() :: ?M(a, b).", + ?assertMatch([_], parse_find_pois(Text, type_definition, {o, 0})), + ?assertMatch([_], parse_find_pois(Text, macro, 'M')), + ok. + %%============================================================================== %% Helper functions %%==============================================================================