From 3db805eb2a01f1c63270d2943c899b4beb30f469 Mon Sep 17 00:00:00 2001 From: San Gillis Date: Mon, 1 Mar 2021 18:00:23 +0100 Subject: [PATCH 1/3] Use els_utils:lookup_document for lookup --- apps/els_lsp/src/els_code_navigation.erl | 8 +-- apps/els_lsp/src/els_compiler_diagnostics.erl | 25 ++++---- apps/els_lsp/src/els_crossref_diagnostics.erl | 18 +++--- apps/els_lsp/src/els_diagnostics_utils.erl | 54 +++++++----------- .../src/els_folding_range_provider.erl | 2 +- .../src/els_unused_includes_diagnostics.erl | 57 ++++++++----------- .../src/els_unused_macros_diagnostics.erl | 10 +--- 7 files changed, 69 insertions(+), 105 deletions(-) diff --git a/apps/els_lsp/src/els_code_navigation.erl b/apps/els_lsp/src/els_code_navigation.erl index 7bec52689..b6be47dcb 100644 --- a/apps/els_lsp/src/els_code_navigation.erl +++ b/apps/els_lsp/src/els_code_navigation.erl @@ -117,12 +117,8 @@ find([Uri|Uris0], Kind, Data, AlreadyVisited) -> find(Uris0, Kind, Data, AlreadyVisited); false -> AlreadyVisited2 = sets:add_element(Uri, AlreadyVisited), - case els_dt_document:lookup(Uri) of - {ok, [Document]} -> - find_in_document([Uri|Uris0], Document, Kind, Data, AlreadyVisited2); - {ok, []} -> - find(Uris0, Kind, Data, AlreadyVisited2) - end + {ok, Document} = els_utils:lookup_document(Uri), + find_in_document([Uri|Uris0], Document, Kind, Data, AlreadyVisited2) end; find(Uri, Kind, Data, AlreadyVisited) -> find([Uri], Kind, Data, AlreadyVisited). diff --git a/apps/els_lsp/src/els_compiler_diagnostics.erl b/apps/els_lsp/src/els_compiler_diagnostics.erl index 11680d84a..fd3f3a74f 100644 --- a/apps/els_lsp/src/els_compiler_diagnostics.erl +++ b/apps/els_lsp/src/els_compiler_diagnostics.erl @@ -142,21 +142,16 @@ parse_escript(Uri) -> [els_diagnostics:diagnostic()]. diagnostics(Path, List, Severity) -> Uri = els_uri:uri(els_utils:to_binary(Path)), - case els_dt_document:lookup(Uri) of - {ok, [Document]} -> - lists:flatten([[ diagnostic( Path - , MessagePath - , range(Anno) - , Document - , Module - , Desc - , Severity) - || {Anno, Module, Desc} <- Info] - || {MessagePath, Info} <- List]); - Error -> - ?LOG_INFO("diagnostics doc lookup failed [Error=~p]", [Error]), - [] - end. + {ok, Document} = els_utils:lookup_document(Uri), + lists:flatten([[ diagnostic( Path + , MessagePath + , range(Anno) + , Document + , Module + , Desc + , Severity) + || {Anno, Module, Desc} <- Info] + || {MessagePath, Info} <- List]). -spec diagnostic( string() , string() diff --git a/apps/els_lsp/src/els_crossref_diagnostics.erl b/apps/els_lsp/src/els_crossref_diagnostics.erl index 2c0cbf09d..fdb616e4e 100644 --- a/apps/els_lsp/src/els_crossref_diagnostics.erl +++ b/apps/els_lsp/src/els_crossref_diagnostics.erl @@ -33,17 +33,13 @@ is_default() -> -spec run(uri()) -> [els_diagnostics:diagnostic()]. run(Uri) -> - case els_dt_document:lookup(Uri) of - {ok, []} -> - []; - {ok, [Document|_]} -> - POIs = els_dt_document:pois(Document, [ application - , implicit_fun - , import_entry - , export_entry - ]), - [make_diagnostic(POI) || POI <- POIs, not has_definition(POI, Document)] - end. + {ok, Document} = els_utils:lookup_document(Uri), + POIs = els_dt_document:pois(Document, [ application + , implicit_fun + , import_entry + , export_entry + ]), + [make_diagnostic(POI) || POI <- POIs, not has_definition(POI, Document)]. -spec source() -> binary(). source() -> diff --git a/apps/els_lsp/src/els_diagnostics_utils.erl b/apps/els_lsp/src/els_diagnostics_utils.erl index 50ee15289..cf42071c7 100644 --- a/apps/els_lsp/src/els_diagnostics_utils.erl +++ b/apps/els_lsp/src/els_diagnostics_utils.erl @@ -31,28 +31,23 @@ included_uris(Document) -> dependencies([], Acc, _AlreadyProcessed) -> Acc; dependencies([Uri|Uris], Acc, AlreadyProcessed) -> - case els_dt_document:lookup(Uri) of - {ok, [Document]} -> - Behaviours = els_dt_document:pois(Document, [behaviour]), - ParseTransforms = els_dt_document:pois(Document, [parse_transform]), - IncludedUris = included_uris(Document), - FilteredIncludedUris = exclude_already_processed( IncludedUris - , AlreadyProcessed - ), - PTUris = lists:usort( - lists:flatten( - [pt_deps(Id) || #{id := Id} <- ParseTransforms])), - FilteredPTUris = exclude_already_processed( PTUris - , AlreadyProcessed - ), - dependencies( Uris ++ FilteredIncludedUris ++ FilteredPTUris - , Acc ++ [Id || #{id := Id} <- Behaviours ++ ParseTransforms] - ++ [els_uri:module(FPTUri) || FPTUri <- FilteredPTUris] - , sets:add_element(Uri, AlreadyProcessed)); - Error -> - ?LOG_INFO("Lookup failed [Error=~p]", [Error]), - [] - end. + {ok, Document} = els_utils:lookup_document(Uri), + Behaviours = els_dt_document:pois(Document, [behaviour]), + ParseTransforms = els_dt_document:pois(Document, [parse_transform]), + IncludedUris = included_uris(Document), + FilteredIncludedUris = exclude_already_processed( IncludedUris + , AlreadyProcessed + ), + PTUris = lists:usort( + lists:flatten( + [pt_deps(Id) || #{id := Id} <- ParseTransforms])), + FilteredPTUris = exclude_already_processed( PTUris + , AlreadyProcessed + ), + dependencies( Uris ++ FilteredIncludedUris ++ FilteredPTUris + , Acc ++ [Id || #{id := Id} <- Behaviours ++ ParseTransforms] + ++ [els_uri:module(FPTUri) || FPTUri <- FilteredPTUris] + , sets:add_element(Uri, AlreadyProcessed)). -spec exclude_already_processed([uri()], sets:set()) -> [uri()]. exclude_already_processed(Uris, AlreadyProcessed) -> @@ -62,16 +57,11 @@ exclude_already_processed(Uris, AlreadyProcessed) -> pt_deps(Module) -> case els_utils:find_module(Module) of {ok, Uri} -> - case els_dt_document:lookup(Uri) of - {ok, [Document]} -> - Applications = els_dt_document:pois(Document, [ application - , implicit_fun - ]), - applications_to_uris(Applications); - Error -> - ?LOG_INFO("Lookup failed [Error=~p]", [Error]), - [] - end; + {ok, Document} = els_utils:lookup_document(Uri), + Applications = els_dt_document:pois(Document, [ application + , implicit_fun + ]), + applications_to_uris(Applications); {error, Error} -> ?LOG_INFO("Find module failed [module=~p] [error=~p]", [Module, Error]), [] diff --git a/apps/els_lsp/src/els_folding_range_provider.erl b/apps/els_lsp/src/els_folding_range_provider.erl index 7d979c410..b9552a9a0 100644 --- a/apps/els_lsp/src/els_folding_range_provider.erl +++ b/apps/els_lsp/src/els_folding_range_provider.erl @@ -26,7 +26,7 @@ is_enabled() -> -spec handle_request(tuple(), state()) -> {folding_range_result(), state()}. handle_request({document_foldingrange, Params}, State) -> #{ <<"textDocument">> := #{<<"uri">> := Uri} } = Params, - {ok, [Document]} = els_dt_document:lookup(Uri), + {ok, Document} = els_utils:lookup_document(Uri), POIs = els_dt_document:pois(Document, [folding_range]), Response = case [folding_range(Range) || #{range := Range} <- POIs] of [] -> null; diff --git a/apps/els_lsp/src/els_unused_includes_diagnostics.erl b/apps/els_lsp/src/els_unused_includes_diagnostics.erl index bf91d6d81..9529de377 100644 --- a/apps/els_lsp/src/els_unused_includes_diagnostics.erl +++ b/apps/els_lsp/src/els_unused_includes_diagnostics.erl @@ -32,19 +32,15 @@ is_default() -> -spec run(uri()) -> [els_diagnostics:diagnostic()]. run(Uri) -> - case els_dt_document:lookup(Uri) of - {ok, []} -> - []; - {ok, [Document|_]} -> - Includes = els_dt_document:pois(Document, [include, include_lib]), - UnusedIncludes = find_unused_includes(Document, Includes), - [ els_diagnostics:make_diagnostic( - els_protocol:range(inclusion_range(UI, Document)) - , <<"Unused file: ", (filename:basename(UI))/binary>> - , ?DIAGNOSTIC_WARNING - , source() - ) || UI <- UnusedIncludes ] - end. + {ok, Document} = els_utils:lookup_document(Uri), + Includes = els_dt_document:pois(Document, [include, include_lib]), + UnusedIncludes = find_unused_includes(Document, Includes), + [ els_diagnostics:make_diagnostic( + els_protocol:range(inclusion_range(UI, Document)) + , <<"Unused file: ", (filename:basename(UI))/binary>> + , ?DIAGNOSTIC_WARNING + , source() + ) || UI <- UnusedIncludes ]. -spec source() -> binary(). source() -> @@ -87,26 +83,21 @@ expand_includes(Uri) -> expand_includes([], Graph, _Visited) -> Graph; expand_includes([Uri|Uris], Graph, Visited) -> - case els_dt_document:lookup(Uri) of - {ok, [Document]} -> - IncludedUris = els_diagnostics_utils:included_uris(Document), - NonVisitedIncludedUris = [U || U <- IncludedUris - , not sets:is_element(U, Visited)], - Dest = digraph:add_vertex(Graph, Uri), - NewGraph = lists:foldl(fun(U, G) -> - Src = digraph:add_vertex(G, U), - digraph:add_edge(G, Src, Dest), - G - end - , Graph - , IncludedUris), - expand_includes( Uris ++ NonVisitedIncludedUris - , NewGraph - , sets:add_element(Uri, Visited)); - {ok, []} -> - ?LOG_WARNING("Failed lookup while expanding includes [uri=~p]", [Uri]), - [] - end. + {ok, Document} = els_utils:lookup_document(Uri), + IncludedUris = els_diagnostics_utils:included_uris(Document), + NonVisitedIncludedUris = [U || U <- IncludedUris + , not sets:is_element(U, Visited)], + Dest = digraph:add_vertex(Graph, Uri), + NewGraph = lists:foldl(fun(U, G) -> + Src = digraph:add_vertex(G, U), + digraph:add_edge(G, Src, Dest), + G + end + , Graph + , IncludedUris), + expand_includes( Uris ++ NonVisitedIncludedUris + , NewGraph + , sets:add_element(Uri, Visited)). -spec inclusion_range(uri(), els_dt_document:item()) -> poi_range(). inclusion_range(Uri, Document) -> diff --git a/apps/els_lsp/src/els_unused_macros_diagnostics.erl b/apps/els_lsp/src/els_unused_macros_diagnostics.erl index fdf68baa7..5c69872c7 100644 --- a/apps/els_lsp/src/els_unused_macros_diagnostics.erl +++ b/apps/els_lsp/src/els_unused_macros_diagnostics.erl @@ -33,13 +33,9 @@ is_default() -> run(Uri) -> case filename:extension(Uri) of <<".erl">> -> - case els_dt_document:lookup(Uri) of - {ok, []} -> - []; - {ok, [Document|_]} -> - UnusedMacros = find_unused_macros(Document), - [make_diagnostic(POI) || POI <- UnusedMacros ] - end; + {ok, Document} = els_utils:lookup_document(Uri), + UnusedMacros = find_unused_macros(Document), + [make_diagnostic(POI) || POI <- UnusedMacros ]; _ -> [] end. From 0655cd9e418311b8cfafa7c5ead97c320e632151 Mon Sep 17 00:00:00 2001 From: San Gillis Date: Wed, 3 Mar 2021 10:22:43 +0100 Subject: [PATCH 2/3] Return error when lookup fails after indexing --- apps/els_core/src/els_utils.erl | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/apps/els_core/src/els_utils.erl b/apps/els_core/src/els_utils.erl index 28d5662a1..5ca0ab11f 100644 --- a/apps/els_core/src/els_utils.erl +++ b/apps/els_core/src/els_utils.erl @@ -132,8 +132,13 @@ lookup_document(Uri) -> {ok, []} -> Path = els_uri:path(Uri), {ok, Uri} = els_indexing:index_file(Path), - {ok, [Document]} = els_dt_document:lookup(Uri), - {ok, Document} + case els_dt_document:lookup(Uri) of + {ok, [Document]} -> + {ok, Document}; + Error -> + ?LOG_INFO("Document lookup failed [error=~p] [uri=~p]", [Error, Uri]), + {error, Error} + end end. -spec macro_string_to_term(list()) -> any(). From c9a19f47e62e906e95ee55e9f3e31c9c7ea0c62e Mon Sep 17 00:00:00 2001 From: San Gillis Date: Wed, 3 Mar 2021 10:40:43 +0100 Subject: [PATCH 3/3] Handle document lookup failure case --- apps/els_lsp/src/els_code_navigation.erl | 8 ++- apps/els_lsp/src/els_compiler_diagnostics.erl | 24 ++++---- apps/els_lsp/src/els_crossref_diagnostics.erl | 18 +++--- apps/els_lsp/src/els_diagnostics_utils.erl | 52 ++++++++++------- .../src/els_unused_includes_diagnostics.erl | 57 +++++++++++-------- .../src/els_unused_macros_diagnostics.erl | 10 +++- 6 files changed, 101 insertions(+), 68 deletions(-) diff --git a/apps/els_lsp/src/els_code_navigation.erl b/apps/els_lsp/src/els_code_navigation.erl index b6be47dcb..68c589f76 100644 --- a/apps/els_lsp/src/els_code_navigation.erl +++ b/apps/els_lsp/src/els_code_navigation.erl @@ -117,8 +117,12 @@ find([Uri|Uris0], Kind, Data, AlreadyVisited) -> find(Uris0, Kind, Data, AlreadyVisited); false -> AlreadyVisited2 = sets:add_element(Uri, AlreadyVisited), - {ok, Document} = els_utils:lookup_document(Uri), - find_in_document([Uri|Uris0], Document, Kind, Data, AlreadyVisited2) + case els_utils:lookup_document(Uri) of + {ok, Document} -> + find_in_document([Uri|Uris0], Document, Kind, Data, AlreadyVisited2); + {error, _Error} -> + find(Uris0, Kind, Data, AlreadyVisited2) + end end; find(Uri, Kind, Data, AlreadyVisited) -> find([Uri], Kind, Data, AlreadyVisited). diff --git a/apps/els_lsp/src/els_compiler_diagnostics.erl b/apps/els_lsp/src/els_compiler_diagnostics.erl index fd3f3a74f..abbba4679 100644 --- a/apps/els_lsp/src/els_compiler_diagnostics.erl +++ b/apps/els_lsp/src/els_compiler_diagnostics.erl @@ -142,16 +142,20 @@ parse_escript(Uri) -> [els_diagnostics:diagnostic()]. diagnostics(Path, List, Severity) -> Uri = els_uri:uri(els_utils:to_binary(Path)), - {ok, Document} = els_utils:lookup_document(Uri), - lists:flatten([[ diagnostic( Path - , MessagePath - , range(Anno) - , Document - , Module - , Desc - , Severity) - || {Anno, Module, Desc} <- Info] - || {MessagePath, Info} <- List]). + case els_utils:lookup_document(Uri) of + {ok, Document} -> + lists:flatten([[ diagnostic( Path + , MessagePath + , range(Anno) + , Document + , Module + , Desc + , Severity) + || {Anno, Module, Desc} <- Info] + || {MessagePath, Info} <- List]); + {error, _Error} -> + [] + end. -spec diagnostic( string() , string() diff --git a/apps/els_lsp/src/els_crossref_diagnostics.erl b/apps/els_lsp/src/els_crossref_diagnostics.erl index fdb616e4e..2bc792de8 100644 --- a/apps/els_lsp/src/els_crossref_diagnostics.erl +++ b/apps/els_lsp/src/els_crossref_diagnostics.erl @@ -33,13 +33,17 @@ is_default() -> -spec run(uri()) -> [els_diagnostics:diagnostic()]. run(Uri) -> - {ok, Document} = els_utils:lookup_document(Uri), - POIs = els_dt_document:pois(Document, [ application - , implicit_fun - , import_entry - , export_entry - ]), - [make_diagnostic(POI) || POI <- POIs, not has_definition(POI, Document)]. + case els_utils:lookup_document(Uri) of + {error, _Error} -> + []; + {ok, Document} -> + POIs = els_dt_document:pois(Document, [ application + , implicit_fun + , import_entry + , export_entry + ]), + [make_diagnostic(POI) || POI <- POIs, not has_definition(POI, Document)] + end. -spec source() -> binary(). source() -> diff --git a/apps/els_lsp/src/els_diagnostics_utils.erl b/apps/els_lsp/src/els_diagnostics_utils.erl index cf42071c7..e321c0a01 100644 --- a/apps/els_lsp/src/els_diagnostics_utils.erl +++ b/apps/els_lsp/src/els_diagnostics_utils.erl @@ -31,23 +31,27 @@ included_uris(Document) -> dependencies([], Acc, _AlreadyProcessed) -> Acc; dependencies([Uri|Uris], Acc, AlreadyProcessed) -> - {ok, Document} = els_utils:lookup_document(Uri), - Behaviours = els_dt_document:pois(Document, [behaviour]), - ParseTransforms = els_dt_document:pois(Document, [parse_transform]), - IncludedUris = included_uris(Document), - FilteredIncludedUris = exclude_already_processed( IncludedUris - , AlreadyProcessed - ), - PTUris = lists:usort( - lists:flatten( - [pt_deps(Id) || #{id := Id} <- ParseTransforms])), - FilteredPTUris = exclude_already_processed( PTUris - , AlreadyProcessed - ), - dependencies( Uris ++ FilteredIncludedUris ++ FilteredPTUris - , Acc ++ [Id || #{id := Id} <- Behaviours ++ ParseTransforms] - ++ [els_uri:module(FPTUri) || FPTUri <- FilteredPTUris] - , sets:add_element(Uri, AlreadyProcessed)). + case els_utils:lookup_document(Uri) of + {ok, Document} -> + Behaviours = els_dt_document:pois(Document, [behaviour]), + ParseTransforms = els_dt_document:pois(Document, [parse_transform]), + IncludedUris = included_uris(Document), + FilteredIncludedUris = exclude_already_processed( IncludedUris + , AlreadyProcessed + ), + PTUris = lists:usort( + lists:flatten( + [pt_deps(Id) || #{id := Id} <- ParseTransforms])), + FilteredPTUris = exclude_already_processed( PTUris + , AlreadyProcessed + ), + dependencies( Uris ++ FilteredIncludedUris ++ FilteredPTUris + , Acc ++ [Id || #{id := Id} <- Behaviours ++ ParseTransforms] + ++ [els_uri:module(FPTUri) || FPTUri <- FilteredPTUris] + , sets:add_element(Uri, AlreadyProcessed)); + {error, _Error} -> + [] + end. -spec exclude_already_processed([uri()], sets:set()) -> [uri()]. exclude_already_processed(Uris, AlreadyProcessed) -> @@ -57,11 +61,15 @@ exclude_already_processed(Uris, AlreadyProcessed) -> pt_deps(Module) -> case els_utils:find_module(Module) of {ok, Uri} -> - {ok, Document} = els_utils:lookup_document(Uri), - Applications = els_dt_document:pois(Document, [ application - , implicit_fun - ]), - applications_to_uris(Applications); + case els_utils:lookup_document(Uri) of + {ok, Document} -> + Applications = els_dt_document:pois(Document, [ application + , implicit_fun + ]), + applications_to_uris(Applications); + {error, _Error} -> + [] + end; {error, Error} -> ?LOG_INFO("Find module failed [module=~p] [error=~p]", [Module, Error]), [] diff --git a/apps/els_lsp/src/els_unused_includes_diagnostics.erl b/apps/els_lsp/src/els_unused_includes_diagnostics.erl index 9529de377..cd45ca60b 100644 --- a/apps/els_lsp/src/els_unused_includes_diagnostics.erl +++ b/apps/els_lsp/src/els_unused_includes_diagnostics.erl @@ -32,15 +32,19 @@ is_default() -> -spec run(uri()) -> [els_diagnostics:diagnostic()]. run(Uri) -> - {ok, Document} = els_utils:lookup_document(Uri), - Includes = els_dt_document:pois(Document, [include, include_lib]), - UnusedIncludes = find_unused_includes(Document, Includes), - [ els_diagnostics:make_diagnostic( - els_protocol:range(inclusion_range(UI, Document)) - , <<"Unused file: ", (filename:basename(UI))/binary>> - , ?DIAGNOSTIC_WARNING - , source() - ) || UI <- UnusedIncludes ]. + case els_utils:lookup_document(Uri) of + {error, _Error} -> + []; + {ok, Document} -> + Includes = els_dt_document:pois(Document, [include, include_lib]), + UnusedIncludes = find_unused_includes(Document, Includes), + [ els_diagnostics:make_diagnostic( + els_protocol:range(inclusion_range(UI, Document)) + , <<"Unused file: ", (filename:basename(UI))/binary>> + , ?DIAGNOSTIC_WARNING + , source() + ) || UI <- UnusedIncludes ] + end. -spec source() -> binary(). source() -> @@ -83,21 +87,26 @@ expand_includes(Uri) -> expand_includes([], Graph, _Visited) -> Graph; expand_includes([Uri|Uris], Graph, Visited) -> - {ok, Document} = els_utils:lookup_document(Uri), - IncludedUris = els_diagnostics_utils:included_uris(Document), - NonVisitedIncludedUris = [U || U <- IncludedUris - , not sets:is_element(U, Visited)], - Dest = digraph:add_vertex(Graph, Uri), - NewGraph = lists:foldl(fun(U, G) -> - Src = digraph:add_vertex(G, U), - digraph:add_edge(G, Src, Dest), - G - end - , Graph - , IncludedUris), - expand_includes( Uris ++ NonVisitedIncludedUris - , NewGraph - , sets:add_element(Uri, Visited)). + case els_utils:lookup_document(Uri) of + {ok, Document} -> + IncludedUris = els_diagnostics_utils:included_uris(Document), + NonVisitedIncludedUris = [U || U <- IncludedUris + , not sets:is_element(U, Visited)], + Dest = digraph:add_vertex(Graph, Uri), + NewGraph = lists:foldl(fun(U, G) -> + Src = digraph:add_vertex(G, U), + digraph:add_edge(G, Src, Dest), + G + end + , Graph + , IncludedUris), + expand_includes( Uris ++ NonVisitedIncludedUris + , NewGraph + , sets:add_element(Uri, Visited)); + {error, _Error} -> + ?LOG_WARNING("Failed lookup while expanding includes [uri=~p]", [Uri]), + [] + end. -spec inclusion_range(uri(), els_dt_document:item()) -> poi_range(). inclusion_range(Uri, Document) -> diff --git a/apps/els_lsp/src/els_unused_macros_diagnostics.erl b/apps/els_lsp/src/els_unused_macros_diagnostics.erl index 5c69872c7..21b8fce52 100644 --- a/apps/els_lsp/src/els_unused_macros_diagnostics.erl +++ b/apps/els_lsp/src/els_unused_macros_diagnostics.erl @@ -33,9 +33,13 @@ is_default() -> run(Uri) -> case filename:extension(Uri) of <<".erl">> -> - {ok, Document} = els_utils:lookup_document(Uri), - UnusedMacros = find_unused_macros(Document), - [make_diagnostic(POI) || POI <- UnusedMacros ]; + case els_utils:lookup_document(Uri) of + {error, _Error} -> + []; + {ok, Document} -> + UnusedMacros = find_unused_macros(Document), + [make_diagnostic(POI) || POI <- UnusedMacros ] + end; _ -> [] end.