diff --git a/.vscode/launch.json b/.vscode/launch.json index 7fe0604b..1296b590 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -26,7 +26,7 @@ "type": "extensionHost", "request": "launch", "runtimeExecutable": "${execPath}", - "args": ["--extensionDevelopmentPath=${workspaceRoot}" ], + "args": ["--disable-extensions", "--extensionDevelopmentPath=${workspaceRoot}" ], "sourceMaps": true, "outFiles": ["${workspaceRoot}/out"], }, diff --git a/CHANGELOG.md b/CHANGELOG.md index ccb9b0da..bf44d752 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,24 @@ # Change log -## ## Version 1.2.3 (September 22, 2026) +## Version 1.2.4 (September 26, 2026) + +* [356](https://github.com/pgourlain/vscode_erlang/issues/356) : Header files (`.hrl`) treated as modules + - Opening a `.hrl` no longer reports `no module definition` and `record ... is unused` for every record it declares: the pull-diagnostics path (`textDocument/diagnostic`, `workspace/diagnostic`) linted any file on its own, unlike the push path which only ever lints `.erl` files. Both now lint `.erl` files only + +* Diagnostics are pushed only + - Every warning and error showed twice on hover, and each quick fix was offered twice (the Problems view merged the identical entries, hiding it there): the server advertised pull diagnostics next to push, and vscode-languageclient pulls open documents on open, on tab change and on refresh whatever `diagnosticPullOptions` says, into a second collection. The server no longer advertises `diagnosticProvider`, so there is one channel and one collection, whatever the client (VS Code, Neovim, Emacs, Kiro...) + - Files that are not open still get their diagnostics, now pushed too: every module of the project is linted from disk in the background after the project scan, one file at a time, and relinted when it changes on disk. Closing a document keeps its diagnostics, recomputed from disk; a deleted file, a file excluded from the project and every file when `erlang.linting` is turned off are cleared + - Fast typing: `textDocument/didOpen`, `didChange`, `didClose` and `didSave` are now applied in the order they are received. They used to be handled concurrently, so two quick incremental edits could be applied out of order and corrupt the server's copy of the document + +* Header files (`.hrl`) and the modules including them + - Fixing and saving a header now relints every module that includes it, directly or through another header, open or not: a module kept showing the header's old error until the module itself was edited. Headers changed outside the editor are picked up too (the file watcher now covers `.hrl`) + +* Diagnostics for files with non-ASCII names or messages + - A module whose path contains characters outside Latin-1 (e.g. a folder named in Japanese) got no diagnostics at all: turning its path into the diagnostic failed and stopped the whole lint. Accented characters in a path or a message (e.g. an unused variable `Été`) were sent to the client as Latin-1 instead of UTF-8. Paths and messages are now encoded as UTF-8 + +--- + +## Version 1.2.3 (September 22, 2026) * [317](https://github.com/pgourlain/vscode_erlang/issues/317), [316](https://github.com/pgourlain/vscode_erlang/issues/316) : Syntax highlighting of `fun` expressions - `fun () -> ... end` and `fun() -> ... end` (a fun with no parameter) highlighted `fun` as a function name instead of a keyword: the rule matching the `fun()` *type* of a `-spec`/`-type` also matched the zero-arity fun *expression*. It now requires that the `)` is not followed by `->`, so the `fun()` type introduced for [297](https://github.com/pgourlain/vscode_erlang/issues/297) keeps working diff --git a/apps/erlangbridge/src/gen_lsp_doc_server.erl b/apps/erlangbridge/src/gen_lsp_doc_server.erl index 9255d2cd..8fa51066 100644 --- a/apps/erlangbridge/src/gen_lsp_doc_server.erl +++ b/apps/erlangbridge/src/gen_lsp_doc_server.erl @@ -4,13 +4,12 @@ %% API -export([start_link/0]). --export([document_opened/2, document_changed/2, document_range_changed/4, document_closed/1, opened_documents/0, get_document_contents/1, get_document_version/1, parse_document/1]). +-export([document_opened/2, document_changed/2, document_range_changed/4, document_closed/1, opened_documents/0, get_document_contents/1, get_document_version/1, parse_document/1, reparse/1]). -export([project_file_added/1, project_file_changed/1, project_file_deleted/1]). -export([get_syntax_tree/1, get_dodged_syntax_tree/1, get_references/1, get_inlayhints/1]). -export([get_semantic_tokens_cache/1, store_semantic_tokens_cache/3]). --export([get_diagnostics_cache/1, store_diagnostics_cache/2]). -export([root_available/0, config_change/0, project_modules/0, get_module_file/1, get_module_files/1, get_build_dir/0, find_source_file/1]). --export([all_project_files/0]). +-export([all_project_files/0, get_includers/1]). %% Cache management -export([delete_unused_caches/2, @@ -132,6 +131,17 @@ parse_document(File) -> ok end. +%% @doc Parse File again now, from disk unless it is open - for a closed file +%% known to have changed on disk (or just closed with its edits discarded) +%% while the tree held for it is still stamped as current. +%% +%% No diagnostics cache is kept next to the trees: with push only, a file is +%% linted once per event that can change its diagnostics (open document edit, +%% close, change on disk, project scan) and never again on a client's request, +%% so there is no repeated lint for a cache to save. +reparse(File) -> + parse_and_store(File, File). + project_file_added(File) -> gen_server:cast(?SERVER, {project_file_added, File}). @@ -196,23 +206,6 @@ get_semantic_tokens_cache(File) -> store_semantic_tokens_cache(File, ResultId, Tokens) -> ?XETS:insert(document_semantic_tokens, {File, ResultId, Tokens}). -%% @doc Diagnostics as last computed for File, or undefined when never -%% computed or computed against an older revision of the document. -%% -%% The LSP 3.17 pull endpoints (lsp_handlers:textDocument_diagnostic/2 and -%% workspace_diagnostic/2) would otherwise re-lint every project file on every -%% pull. Keyed by the same document_version counter the push path uses, and -%% dropped outright by parse_and_store/2, so a reparse always recomputes. -get_diagnostics_cache(File) -> - Version = get_document_version(File), - case ?XETS:lookup(document_diagnostics, File) of - [{File, Version, Diagnostics}] -> Diagnostics; - _ -> undefined - end. - -store_diagnostics_cache(File, Diagnostics) -> - ?XETS:insert(document_diagnostics, {File, get_document_version(File), Diagnostics}). - root_available() -> gen_server:cast(?SERVER, root_available). @@ -278,9 +271,9 @@ start_link() -> safe_new_table(syntax_tree, ?XETS, set, ExtraCreateOpts), safe_new_table(dodged_syntax_tree, ?XETS, set, ExtraCreateOpts), safe_new_table(references, ets, bag, []), + safe_new_table(document_includes, ets, set, []), safe_new_table(document_inlayhints, ?XETS, set, ExtraCreateOpts), safe_new_table(document_semantic_tokens, ?XETS, set, ExtraCreateOpts), - safe_new_table(document_diagnostics, ?XETS, set, ExtraCreateOpts), gen_server:start_link({local, ?SERVER}, ?MODULE, [],[]). init(_Args) -> @@ -347,7 +340,6 @@ terminate(_Reason, _State) -> delete_cache_file(dodged_syntax_tree), delete_cache_file(document_inlayhints), delete_cache_file(document_semantic_tokens), - delete_cache_file(document_diagnostics), ok. code_change(_OldVersion, State, _Extra) -> @@ -550,6 +542,7 @@ delete_project_files([File | Files], State) -> ?XETS:delete(syntax_tree, File), ?XETS:delete(dodged_syntax_tree, File), ets:delete(references, File), + ets:delete(document_includes, File), ?XETS:delete(document_inlayhints, File), ?XETS:delete(document_semantic_tokens, File), Module = filename:rootname(filename:basename(File)), @@ -619,13 +612,13 @@ parse_and_store(File, ContentsFile) -> end. parse_and_store(File, ContentsFile, Version) -> - ?XETS:delete(document_diagnostics, File), {SyntaxTree, DodgedSyntaxTree} = lsp_parse:parse_source_file(File, ContentsFile), case SyntaxTree of undefined -> ok; _ -> ?XETS:insert(syntax_tree, {File, Version, SyntaxTree}), + ets:insert(document_includes, {File, included_files(File, SyntaxTree)}), ets:delete(references, File), ?XETS:delete(document_inlayhints, File), lsp_navigation:fold_references(fun (Reference, Line, Column, End, _) -> @@ -638,6 +631,36 @@ parse_and_store(File, ContentsFile, Version) -> _ -> ?XETS:insert(dodged_syntax_tree, {File, Version, DodgedSyntaxTree}) end. +%% Every file epp entered while parsing File - its headers, and theirs. +included_files(File, SyntaxTree) -> + lists:usort([normalize_path(Included) || + {attribute, _, file, {Included, _}} <- SyntaxTree, Included =/= File]). + +%% @doc Project files whose last parse included Hrl, directly or through +%% another header - what has to be reparsed when Hrl changes. A scan of the +%% whole index rather than a reverse one: it only runs when a header changes, +%% while a reverse index would cost a table scan on every parse to drop the +%% file's old entries. +get_includers(Hrl) -> + Target = normalize_path(Hrl), + ets:foldl(fun ({File, Included}, Acc) -> + case lists:member(Target, Included) of + true -> [File | Acc]; + false -> Acc + end + end, [], document_includes). + +%% Absolute, with `.` and `..` segments resolved, so that an include path +%% like "src/../include/x.hrl" matches the watcher's "include/x.hrl". +normalize_path(Path) -> + [Root | Segments] = filename:split(filename:absname(unicode:characters_to_list(Path))), + filename:join([Root | lists:reverse(lists:foldl(fun + (".", Acc) -> Acc; + ("..", [_ | Acc]) -> Acc; + ("..", []) -> []; + (Segment, Acc) -> [Segment | Acc] + end, [], Segments))]). + %% `undefined` for never parsed, `{Version, Tree}` otherwise - Version being %% the document_version the contents were read at (see parse_document/1). get_tree(TreeType, File) -> diff --git a/apps/erlangbridge/src/gen_lsp_server.erl b/apps/erlangbridge/src/gen_lsp_server.erl index 35addabf..1e25f1e9 100644 --- a/apps/erlangbridge/src/gen_lsp_server.erl +++ b/apps/erlangbridge/src/gen_lsp_server.erl @@ -194,6 +194,22 @@ send_to_client(Socket, Method, Body) -> gen_tcp:send(Socket, <
>). +%% Document synchronisation notifications are applied right here, in the +%% socket process, in the order they came off the wire: an incremental +%% didChange is positioned against the buffer left by the previous one, so +%% two of them handled by independent processes could land in either order, +%% or both read the same buffer and lose one edit. Their handlers only touch +%% the buffer tables and hand any linting to lsp_diagnostics, so this does not +%% hold up the socket. Everything else still gets a process of its own. +dispatch(Socket, #{method := Method} = Input) when + Method =:= <<"textDocument/didOpen">>; + Method =:= <<"textDocument/didChange">>; + Method =:= <<"textDocument/didClose">>; + Method =:= <<"textDocument/didSave">> -> + do_contents(Socket, Input); +dispatch(Socket, Input) -> + spawn(fun() -> do_contents(Socket, Input) end). + handle_tcp_data(Socket, Contents, State) -> StateWithContents = State#state{contents = <<(State#state.contents)/binary, Contents/binary>>}, StateWithLength = case StateWithContents#state.content_length of @@ -223,12 +239,12 @@ handle_tcp_data(Socket, Contents, State) -> StateWithLength; ContentLength when ContentLength =:= byte_size(StateWithLength#state.contents) -> {ok, Input, _} = vscode_jsone_decode:decode(StateWithLength#state.contents, [{keys, atom}]), - spawn(fun() -> do_contents(Socket, Input) end), + dispatch(Socket, Input), StateWithLength#state{contents = <<"">>, content_length = undefined}; ContentLength when ContentLength < byte_size(StateWithLength#state.contents) -> ShorterContents = binary:part(StateWithLength#state.contents, 0, ContentLength), {ok, Input, _} = vscode_jsone_decode:decode(ShorterContents, [{keys, atom}]), - spawn(fun() -> do_contents(Socket, Input) end), + dispatch(Socket, Input), handle_tcp_data( Socket, binary:part(StateWithLength#state.contents, ContentLength, byte_size(StateWithLength#state.contents) - ContentLength), diff --git a/apps/erlangbridge/src/lsp_diagnostics.erl b/apps/erlangbridge/src/lsp_diagnostics.erl index 6e4433e7..c350284d 100644 --- a/apps/erlangbridge/src/lsp_diagnostics.erl +++ b/apps/erlangbridge/src/lsp_diagnostics.erl @@ -1,20 +1,34 @@ -module(lsp_diagnostics). -behavior(gen_server). -%% Coordination point for the two diagnostic paths that must not fight each -%% other: the push path (textDocument/publishDiagnostics) and the LSP 3.17 -%% pull path (textDocument/diagnostic, workspace/diagnostic). +%% Schedules every lint whose result is pushed to the client +%% (textDocument/publishDiagnostics - the only diagnostics channel, the server +%% advertises no pull support): +%% +%% - open documents: one debounced validation per file (schedule_validation/2), +%% run as soon as the debounce elapses; +%% - closed project files: a background queue linted one file at a time +%% (schedule_background_validation/2, schedule_disk_validation/2), so a +%% project scan never floods the VM with hundreds of concurrent lints. +%% +%% It also remembers which files currently show diagnostics, so that files +%% leaving the project, or everything when erlang.linting is turned off, can +%% be cleared: with push only, the client keeps whatever it was last sent. %% %% Deliberately a process of its own rather than state on gen_lsp_server: %% gen_lsp_server owns the socket and parks in gen_tcp:accept/1, so it cannot %% also service casts promptly, and tests that exercise diagnostics have no way -%% to stand one up. +%% to stand one up. Lints themselves never run in here, nor in +%% gen_lsp_doc_server or gen_lsp_server. %% API -export([start_link/0]). --export([schedule_refresh/1, wait_for_change/1, notify_changed/0]). -export([schedule_configuration_request/2]). --export([schedule_validation/2, cancel_validation/1]). +-export([schedule_validation/2, schedule_validation/3, cancel_validation/1]). +-export([schedule_background_validation/2, schedule_disk_validation/2]). +-export([project_scanned/2, clear_all/1, published/2]). +%% For lsp_diagnostic_SUITE: nothing pending, nothing running. +-export([idle/0]). %% gen_server callbacks -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]). @@ -23,10 +37,6 @@ -define(SERVER, ?MODULE). -%% A burst of pushes is coalesced into a single workspace/diagnostic/refresh. -%% One refresh per push restarts the client's whole workspace pull every time. --define(REFRESH_DEBOUNCE_MS, 300). - %% A burst of workspace/didChangeConfiguration notifications (VS Code can %% fire Workspace.onDidChangeConfiguration more than once per logical %% settings change) is coalesced into a single workspace/configuration @@ -36,40 +46,27 @@ -define(CONFIGURATION_DEBOUNCE_MS, 300). %% Keystrokes inside a textDocument/didChange burst are coalesced into a -%% single validation per file. Open documents are reported through the push -%% channel only (see lsp_handlers:workspace_diagnostic_item/2), so a change -%% has to revalidate to keep unsaved edits - a quick fix's WorkspaceEdit -%% among them - from leaving a fixed problem in the Problems list; this is -%% what keeps that off the per-keystroke path. +%% single validation per file, so that an unsaved edit - a quick fix's +%% WorkspaceEdit among them - republishes the file's diagnostics without +%% linting on every keystroke. -define(VALIDATION_DEBOUNCE_MS, 400). --record(state, {refresh_timer, refresh_socket, waiters = [], - configuration_timer, configuration_socket, configuration_source, - validation_timers = #{} :: #{file:filename() => reference()}}). +%% `cached` lints the syntax tree gen_lsp_doc_server already holds (parsed on +%% demand if missing); `reparse` parses the file again first (its buffer when +%% open, the disk otherwise), for when it is known to have changed under a +%% tree still stamped as current - changed on disk, or one of its headers did. +-type mode() :: cached | reparse. + +-record(state, {configuration_timer, configuration_socket, configuration_source, + validation_timers = #{} :: #{file:filename() => {reference(), mode()}}, + background = [] :: [{file:filename(), mode()}], + background_socket, + running :: undefined | file:filename(), + published = #{} :: #{file:filename() => true}}). start_link() -> gen_server:start_link({local, ?SERVER}, ?MODULE, [], []). -%% @doc Ask for one workspace/diagnostic/refresh, debounced. -schedule_refresh(Socket) -> - gen_server:cast(?SERVER, {schedule_refresh, Socket}). - -%% @doc Block the calling process until diagnostics change anywhere, or Timeout -%% elapses. lsp_handlers:workspace_diagnostic/2 uses this to honour the -%% workspace pull being a long poll - answering an all-unchanged pull at once -%% makes the client re-pull immediately, spinning the server. -wait_for_change(Timeout) -> - Ref = make_ref(), - gen_server:cast(?SERVER, {register_waiter, self(), Ref}), - receive - {diagnostics_changed, Ref} -> changed - after Timeout -> - timeout - end. - -notify_changed() -> - gen_server:cast(?SERVER, notify_changed). - %% @doc Ask for one workspace/configuration round-trip, debounced. Source %% identifies the triggering notification (rides into the request id via %% gen_lsp_server:next_request_id/1, see lsp_handlers:request_configuration/2) @@ -78,36 +75,63 @@ notify_changed() -> schedule_configuration_request(Socket, Source) -> gen_server:cast(?SERVER, {schedule_configuration_request, Socket, Source}). -%% @doc Ask for one validation (lint + publishDiagnostics) of File, debounced -%% per file - a later call within the window restarts the wait, so a burst of -%% edits lints once, at the end. +%% @doc Ask for one validation (lint + publishDiagnostics) of the open +%% document File, debounced per file - a later call within the window +%% restarts the wait, so a burst of edits lints once, at the end. schedule_validation(Socket, File) -> - gen_server:cast(?SERVER, {schedule_validation, Socket, File}). + schedule_validation(Socket, File, cached). + +%% @doc Same, reparsing the buffer first when Mode is `reparse` - for an open +%% module whose header changed. A pending `reparse` is never downgraded. +schedule_validation(Socket, File, Mode) -> + gen_server:cast(?SERVER, {schedule_validation, Socket, File, Mode}). -%% @doc Drop any validation still pending for File. Called when the document -%% closes: textDocument/didClose clears the file's push diagnostics, and a -%% debounced validation landing just after that would publish them straight -%% back - for a file the workspace pull has meanwhile taken back over, i.e. -%% the duplicate all over again. +%% @doc Drop any debounced validation still pending for File. Called when the +%% document closes: from then on the file is linted from disk, through the +%% background queue. cancel_validation(File) -> gen_server:cast(?SERVER, {cancel_validation, File}). +%% @doc Queue closed project files for linting, one at a time, behind whatever +%% is already queued. A file already queued keeps its place; a file open by +%% the time its turn comes is skipped, its own push path owns it. +schedule_background_validation(Socket, Files) -> + gen_server:cast(?SERVER, {schedule_background_validation, Socket, Files, cached, back}). + +%% @doc Lint File from disk ahead of the rest of the queue: a document just +%% closed, a closed file created, changed or deleted on disk, or one whose +%% header changed. A file that no longer exists has its diagnostics cleared. +schedule_disk_validation(Socket, File) -> + gen_server:cast(?SERVER, {schedule_background_validation, Socket, [File], reparse, front}). + +%% @doc The project scan has finished and found Files: lint each of them in +%% the background, and clear any file still showing diagnostics that is no +%% longer part of the project (e.g. newly excluded by search.exclude) - unless +%% it is open, in which case its buffer still owns what it shows. +project_scanned(Socket, Files) -> + gen_server:cast(?SERVER, {project_scanned, Socket, Files}). + +%% @doc erlang.linting was turned off: drop every pending lint and clear every +%% file still showing diagnostics. +clear_all(Socket) -> + gen_server:cast(?SERVER, {clear_all, Socket}). + +%% @doc Bookkeeping for lsp_handlers:send_diagnostics/3 - which files are +%% currently showing something, so they can be cleared later. +published(File, Diagnostics) -> + gen_server:cast(?SERVER, {published, File, Diagnostics =/= []}). + +idle() -> + gen_server:call(?SERVER, idle). + init(_Args) -> {ok, #state{}}. +handle_call(idle, _From, #state{validation_timers = Timers, background = Queue, running = Running} = State) -> + {reply, map_size(Timers) =:= 0 andalso Queue =:= [] andalso Running =:= undefined, State}; handle_call(_Request, _From, State) -> {reply, ok, State}. -handle_cast({schedule_refresh, Socket}, #state{refresh_timer = undefined} = State) -> - Timer = erlang:send_after(?REFRESH_DEBOUNCE_MS, self(), send_refresh), - {noreply, State#state{refresh_timer = Timer, refresh_socket = Socket}}; -handle_cast({schedule_refresh, Socket}, State) -> - %% A refresh is already pending - coalesce into it. - {noreply, State#state{refresh_socket = Socket}}; - -handle_cast({register_waiter, Pid, Ref}, State) -> - {noreply, State#state{waiters = [{Pid, Ref} | State#state.waiters]}}; - handle_cast({schedule_configuration_request, Socket, Source}, #state{configuration_timer = undefined} = State) -> Timer = erlang:send_after(?CONFIGURATION_DEBOUNCE_MS, self(), send_configuration_request), {noreply, State#state{configuration_timer = Timer, configuration_socket = Socket, @@ -116,29 +140,48 @@ handle_cast({schedule_configuration_request, Socket, Source}, State) -> %% A request is already pending - coalesce into it. {noreply, State#state{configuration_socket = Socket, configuration_source = Source}}; -handle_cast({schedule_validation, Socket, File}, #state{validation_timers = Timers} = State) -> - case maps:get(File, Timers, undefined) of - undefined -> ok; - Pending -> erlang:cancel_timer(Pending) +handle_cast({schedule_validation, Socket, File, Mode}, #state{validation_timers = Timers} = State) -> + Mode2 = case maps:get(File, Timers, undefined) of + undefined -> Mode; + {_, PendingMode} = Pending -> cancel_timer(Pending), max_mode(Mode, PendingMode) end, Timer = erlang:send_after(?VALIDATION_DEBOUNCE_MS, self(), {validate, Socket, File}), - {noreply, State#state{validation_timers = Timers#{File => Timer}}}; + {noreply, State#state{validation_timers = Timers#{File => {Timer, Mode2}}}}; handle_cast({cancel_validation, File}, #state{validation_timers = Timers} = State) -> - case maps:get(File, Timers, undefined) of - undefined -> ok; - Pending -> erlang:cancel_timer(Pending) - end, + cancel_timer(maps:get(File, Timers, undefined)), %% A {validate, ...} message already in this process's own mailbox is %% dropped by handle_info's cancelled clause below, which is why the %% timers map is the authority on what is still wanted. {noreply, State#state{validation_timers = maps:remove(File, Timers)}}; -handle_cast(notify_changed, State) -> - lists:foreach(fun ({Pid, Ref}) -> - is_process_alive(Pid) andalso (Pid ! {diagnostics_changed, Ref}) - end, State#state.waiters), - {noreply, State#state{waiters = []}}; +handle_cast({schedule_background_validation, Socket, Files, Mode, Where}, State) -> + Queue = enqueue(Files, Mode, Where, State#state.background), + {noreply, run_next(State#state{background = Queue, background_socket = Socket})}; + +handle_cast({project_scanned, Socket, Files}, #state{published = Published} = State) -> + Opened = gen_lsp_doc_server:opened_documents(), + Gone = [File || File <- maps:keys(Published), + not lists:member(File, Files), not lists:member(File, Opened)], + ?LOG(<<"diag">>, "project_scanned: ~p files to lint, clearing ~p", [length(Files), Gone]), + lists:foreach(fun (File) -> lsp_handlers:send_diagnostics(Socket, File, []) end, Gone), + Queue = enqueue(Files, cached, back, [Item || {File, _} = Item <- State#state.background, + not lists:member(File, Gone)]), + {noreply, run_next(State#state{background = Queue, background_socket = Socket, + published = maps:without(Gone, Published)})}; + +handle_cast({clear_all, Socket}, #state{published = Published, validation_timers = Timers} = State) -> + ?LOG(<<"diag">>, "clear_all: linting disabled, clearing ~p", [maps:keys(Published)]), + lists:foreach(fun cancel_timer/1, maps:values(Timers)), + lists:foreach(fun (File) -> lsp_handlers:send_diagnostics(Socket, File, []) end, maps:keys(Published)), + %% A lint already running is not stopped: validate_file/2 checks + %% erlang.linting before it publishes anything. + {noreply, State#state{background = [], validation_timers = #{}, published = #{}}}; + +handle_cast({published, File, true}, #state{published = Published} = State) -> + {noreply, State#state{published = Published#{File => true}}}; +handle_cast({published, File, false}, #state{published = Published} = State) -> + {noreply, State#state{published = maps:remove(File, Published)}}; handle_cast(stop, State) -> {stop, normal, State}; @@ -146,30 +189,28 @@ handle_cast(stop, State) -> handle_cast(_Request, State) -> {noreply, State}. -handle_info(send_refresh, #state{refresh_socket = undefined} = State) -> - {noreply, State#state{refresh_timer = undefined}}; -handle_info(send_refresh, #state{refresh_socket = Socket} = State) -> - ?LOG(<<"diag">>, "request_diagnostic_refresh: asking client to re-pull diagnostics", []), - gen_lsp_server:send_to_client(Socket, <<"workspace/diagnostic/refresh">>, #{ - id => gen_lsp_server:next_request_id(<<"workspace_diagnostic_refresh">>), - method => <<"workspace/diagnostic/refresh">>, - params => null - }), - {noreply, State#state{refresh_timer = undefined}}; -%% Linting a large file is slow and this process also services the workspace -%% pull's waiters, so the validation itself runs outside it - like -%% gen_lsp_server does for every incoming message. +%% Linting a large file is slow and this process has to stay responsive, so +%% the validation itself runs outside it - like gen_lsp_server does for every +%% incoming request. handle_info({validate, Socket, File}, #state{validation_timers = Timers} = State) -> - case maps:is_key(File, Timers) of - false -> + case maps:get(File, Timers, undefined) of + undefined -> %% cancel_validation/1 came in after this timer had already fired ?LOG(<<"diag">>, "schedule_validation: dropping cancelled validation of ~p", [File]), {noreply, State}; - true -> - ?LOG(<<"diag">>, "schedule_validation: debounce elapsed, validating ~p", [File]), - spawn(fun () -> lsp_handlers:validate_file(Socket, File) end), + {_Timer, Mode} -> + ?LOG(<<"diag">>, "schedule_validation: debounce elapsed, validating ~p (~p)", [File, Mode]), + spawn(fun () -> + Mode =:= reparse andalso gen_lsp_doc_server:reparse(File), + lsp_handlers:validate_file(Socket, File) + end), {noreply, State#state{validation_timers = maps:remove(File, Timers)}} end; +handle_info({worker_result, {background, File}, _Result}, #state{running = File} = State) -> + {noreply, run_next(State#state{running = undefined})}; +handle_info({worker_error, {background, File}, Error}, #state{running = File} = State) -> + lsp_log:error(<<"diag">>, "background validation of ~p failed: ~p", [File, Error]), + {noreply, run_next(State#state{running = undefined})}; handle_info(send_configuration_request, #state{configuration_socket = undefined} = State) -> {noreply, State#state{configuration_timer = undefined}}; handle_info(send_configuration_request, #state{configuration_socket = Socket, @@ -185,3 +226,42 @@ terminate(_Reason, _State) -> code_change(_OldVersion, State, _Extra) -> {ok, State}. + +cancel_timer(undefined) -> ok; +cancel_timer({Timer, _Mode}) -> erlang:cancel_timer(Timer). + +max_mode(reparse, _) -> reparse; +max_mode(_, reparse) -> reparse; +max_mode(cached, cached) -> cached. + +%% Deduplicated: a file already queued stays queued once, `reparse` winning over +%% `cached` (a reparse covers a plain lint, not the other way around), and +%% moved to the front when asked for there. +enqueue(Files, Mode, Where, Queue) -> + lists:foldl(fun (File, Acc) -> + {Mode2, Rest} = case lists:keytake(File, 1, Acc) of + {value, {File, reparse}, Others} -> {reparse, Others}; + {value, {File, cached}, Others} -> {Mode, Others}; + false -> {Mode, Acc} + end, + case {Where, Rest =:= Acc} of + {front, _} -> [{File, Mode2} | Rest]; + %% already queued: keep its place, only the mode may change + {back, false} -> lists:keystore(File, 1, Acc, {File, Mode2}); + {back, true} -> Acc ++ [{File, Mode2}] + end + end, Queue, case Where of front -> lists:reverse(Files); back -> Files end). + +run_next(#state{running = undefined, background = [{File, Mode} | Rest], + background_socket = Socket} = State) -> + case gen_lsp_doc_server:get_document_contents(File) of + undefined -> + worker:start(fun () -> lsp_handlers:validate_closed_file(Socket, File, Mode) end, + {background, File}), + State#state{running = File, background = Rest}; + _Open -> + %% Opened since it was queued - its own push path lints the buffer. + run_next(State#state{background = Rest}) + end; +run_next(State) -> + State. diff --git a/apps/erlangbridge/src/lsp_handlers.erl b/apps/erlangbridge/src/lsp_handlers.erl index 718de33a..504234e4 100644 --- a/apps/erlangbridge/src/lsp_handlers.erl +++ b/apps/erlangbridge/src/lsp_handlers.erl @@ -19,21 +19,18 @@ -export([textDocument_foldingRange/2, textDocument_selectionRange/2]). -export([completionItem_resolve/2, textDocument_documentLink/2, documentLink_resolve/2]). -export([codeLens_resolve/2, inlayHint_resolve/2]). --export([textDocument_diagnostic/2, workspace_diagnostic/2, workspace_diagnostic_refresh/2]). -export([erlang_discoverTests/2, erlang_runTests/2]). -%% Exported for lsp_diagnostic_SUITE, which drives the stale-version drop and -%% the refresh nudge directly - they are unreachable through a handler. +%% Exported for lsp_diagnostic_SUITE, which drives the stale-version drop +%% directly - it is unreachable through a handler. -export([maybe_send_diagnostics/4]). %% Exported for lsp_diagnostics, which debounces workspace/didChangeConfiguration -%% notifications into a single call, and textDocument/didChange edits into a -%% single validation per file. --export([request_configuration/2, validate_file/2]). +%% notifications into a single call, textDocument/didChange edits into a +%% single validation per file, lints closed project files in the background and +%% clears files that should no longer show anything. +-export([request_configuration/2, validate_file/2, validate_closed_file/3, send_diagnostics/3]). -include("lsp_log.hrl"). -%% How long workspace_diagnostic/2 parks a pull that has nothing new to report. --define(WORKSPACE_DIAGNOSTIC_POLL_MS, 30000). - initialize(_Socket, Params) -> RootPath = resolve_root(Params), gen_lsp_config_server:update_config(root, RootPath), @@ -82,7 +79,11 @@ initialize(_Socket, Params) -> typeHierarchyProvider => true, %% task 4.6 inlineValueProvider => true, inlayHintProvider => #{resolveProvider => true}, %% task 5.8 - diagnosticProvider => #{interFileDependencies => true, workspaceDiagnostics => true}, %% task 5.9 + %% No diagnosticProvider: diagnostics are pushed only + %% (textDocument/publishDiagnostics), closed project files included - + %% see lsp_diagnostics. Advertising the pull model as well made + %% vscode-languageclient keep a second collection for the same files, + %% so every problem showed twice on hover and in quick fixes. workspaceSymbolProvider => #{resolveProvider => true}, %% task 4.1 workspace => #{ workspaceFolders => #{supported => true, changeNotifications => true} @@ -180,8 +181,9 @@ configuration(Socket, [ErlangSection, FilesSection, ComputedSection, HttpSection true -> ?LOG(<<"diag">>, "configuration: unchanged, skipping project scan and validation of ~p", [Documents]); false -> - %% Scan workspace for source files - gen_lsp_doc_server:config_change(), + %% Scan workspace for source files, then lint the closed ones in + %% the background (or clear everything if linting is now off). + rescan_project(Socket), ?LOG(<<"diag">>, "configuration: Starting validation for documents ~p", [Documents]), %% NOTE: no send_diagnostics(Socket, File, []) clear ahead of %% validate_file here - see the "Why validate_file alone is enough" @@ -211,19 +213,66 @@ configuration(Socket, [ErlangSection, FilesSection, ComputedSection, HttpSection workspace_didChangeConfiguration(Socket, _Params) -> lsp_diagnostics:schedule_configuration_request(Socket, <<"didChangeConfiguration">>). -workspace_didChangeWatchedFiles(_Socket, Params) -> - lists:foreach(fun - (#{uri := Uri, type := 1} = Change) -> % Created - ?LOG(<<"diag">>, "watchedFiles: Created ~p", [Change]), - gen_lsp_doc_server:project_file_added(lsp_utils:file_uri_to_file(Uri)); - (#{uri := Uri, type := 2} = Change) -> % Changed - ?LOG(<<"diag">>, "watchedFiles: Changed ~p", [Change]), - gen_lsp_doc_server:project_file_changed(lsp_utils:file_uri_to_file(Uri)); - (#{uri := Uri, type := 3} = Change) -> % Deleted - ?LOG(<<"diag">>, "watchedFiles: Deleted ~p", [Change]), - gen_lsp_doc_server:project_file_deleted(lsp_utils:file_uri_to_file(Uri)) +%% A closed file changed on disk is relinted from disk (a deleted one is +%% cleared). An open one is left alone: its buffer, not the disk, is what its +%% diagnostics describe. A header is not a project module: only the modules +%% that include it are relinted. +workspace_didChangeWatchedFiles(Socket, Params) -> + lists:foreach(fun (#{uri := Uri, type := Type} = Change) -> + ?LOG(<<"diag">>, "watchedFiles: ~p", [Change]), + File = lsp_utils:file_uri_to_file(Uri), + case {filename:extension(File), Type} of + {".hrl", _} -> + revalidate_includers(Socket, File); + {_, 1} -> % Created + gen_lsp_doc_server:project_file_added(File), + revalidate_closed_file(Socket, File); + {_, 2} -> % Changed + gen_lsp_doc_server:project_file_changed(File), + revalidate_closed_file(Socket, File); + {_, 3} -> % Deleted + gen_lsp_doc_server:project_file_deleted(File), + revalidate_closed_file(Socket, File) + end end, maps:get(changes, Params)). +%% @doc Relint every module whose last parse included Hrl: their syntax +%% trees hold the header as it was then, and nothing about the module itself +%% changed to make get_syntax_tree/1 reparse it. +revalidate_includers(Socket, Hrl) -> + lists:foreach(fun (File) -> + case gen_lsp_doc_server:get_document_contents(File) of + undefined -> lsp_diagnostics:schedule_disk_validation(Socket, File); + _Open -> lsp_diagnostics:schedule_validation(Socket, File, reparse) + end + end, gen_lsp_doc_server:get_includers(Hrl)). + +revalidate_closed_file(Socket, File) -> + case {filename:extension(File), gen_lsp_doc_server:get_document_contents(File)} of + {".erl", undefined} -> lsp_diagnostics:schedule_disk_validation(Socket, File); + _ -> ok + end. + +%% @doc Rescan the project, then push diagnostics for every closed project +%% file - or, with erlang.linting off, clear every file showing any. +%% +%% The scan runs in gen_lsp_doc_server, from a cast that does not know the +%% socket. all_project_files/0 is a call to that same server, so it is only +%% answered once the config_change cast sent just before has been handled, +%% i.e. once the scan is done: the socket stays here, with the handler that +%% has it, and the scan needs no knowledge of diagnostics. +rescan_project(Socket) -> + gen_lsp_doc_server:config_change(), + case gen_lsp_config_server:linting() of + true -> + %% Only modules are linted (#356). + Files = [File || File <- gen_lsp_doc_server:all_project_files(), + filename:extension(File) =:= ".erl"], + lsp_diagnostics:project_scanned(Socket, Files); + _ -> + lsp_diagnostics:clear_all(Socket) + end. + %% @doc Only handles the case where the server started with no root at all %% (single-file mode) and a folder is then added to the workspace: that %% first added folder is adopted as the root and scanned. @@ -232,18 +281,22 @@ workspace_didChangeWatchedFiles(_Socket, Params) -> %% additions or removals are not reflected - see resolve_root/1's comment %% and task 1.3's scope note. Real multi-root support is a separate, %% follow-up task. -workspace_didChangeWorkspaceFolders(_Socket, Params) -> +workspace_didChangeWorkspaceFolders(Socket, Params) -> #{event := #{added := Added}} = Params, case {gen_lsp_config_server:root(), Added} of {"", [#{uri := Uri} | _]} -> NewRoot = lsp_utils:to_string(lsp_utils:file_uri_to_file(Uri)), gen_lsp_config_server:update_config(root, NewRoot), gen_lsp_doc_server:root_available(), - gen_lsp_doc_server:config_change(); + rescan_project(Socket); _ -> ok end. +%% didOpen, didChange, didClose and didSave run in gen_lsp_server's socket +%% process, in wire order (see gen_lsp_server:dispatch/2), so they only update +%% the buffer and hand the lint to lsp_diagnostics. No parse here either: +%% gen_lsp_doc_server:get_syntax_tree/1 parses the buffer on demand. textDocument_didOpen(Socket, Params) -> File = lsp_utils:file_uri_to_file(mapmapget(textDocument, uri, Params)), ?LOG(<<"diag">>, "didOpen: ~p version=~p", [File, mapmapfind(textDocument, version, Params, fun () -> undefined end)]), @@ -251,31 +304,32 @@ textDocument_didOpen(Socket, Params) -> %% Not gated on autosave() any more - see textDocument_didChange/2. With %% autosave off, the gate left a freshly opened document with no %% diagnostics at all until its first edit. - gen_lsp_doc_server:parse_document(File), - ?LOG(<<"diag">>, "didOpen: Parsing document for file ~p", [File]), - validate_file(Socket, File). + lsp_diagnostics:schedule_validation(Socket, File). +%% Closing a document does not clear its diagnostics: a closed project file +%% keeps showing its problems, computed from disk - which is what it holds +%% once unsaved edits are discarded. Other files (e.g. rebar.config) are only +%% linted while open, so closing one clears it. textDocument_didClose(Socket, Params) -> File = lsp_utils:file_uri_to_file(mapmapget(textDocument, uri, Params)), ?LOG(<<"diag">>, "didClose: ~p", [File]), - %% Before the clear, so a debounced edit cannot republish just after it. lsp_diagnostics:cancel_validation(File), - send_diagnostics(Socket, File, []), gen_lsp_doc_server:document_closed(File), - %% The file is the pull channel's again now (workspace_diagnostic_item/2 - %% reports only *open* documents as empty), so release the workspace - %% pull's long poll instead of leaving the problems it holds hidden until - %% the poll times out. - lsp_diagnostics:notify_changed(), - request_diagnostic_refresh(Socket). + case filename:extension(File) of + ".erl" -> lsp_diagnostics:schedule_disk_validation(Socket, File); + _ -> send_diagnostics(Socket, File, []) + end. textDocument_didSave(Socket, Params) -> File = lsp_utils:file_uri_to_file(mapmapget(textDocument, uri, Params)), ?LOG(<<"diag">>, "didSave: ~p", [File]), - %% Not gated on autosave() any more - see textDocument_didChange/2. - gen_lsp_doc_server:parse_document(File), - ?LOG(<<"diag">>, "didsave: Parsing document for file ~p", [File]), - validate_file(Socket, File). + case filename:extension(File) of + %% epp reads headers from disk, so saving is what changes them for + %% the modules including them. + ".hrl" -> revalidate_includers(Socket, File); + %% Not gated on autosave() any more - see textDocument_didChange/2. + _ -> lsp_diagnostics:schedule_validation(Socket, File) + end. %% Content changes are applied in the order the client sent them - each %% one (range-based or, still legal even under Incremental sync, a full @@ -287,11 +341,9 @@ textDocument_didChange(Socket, Params) -> ?LOG(<<"diag">>, "didChange: ~p version=~p changes=~p", [File, mapmapfind(textDocument, version, Params, fun () -> undefined end), length(ContentChanges)]), lists:foreach(fun (ContentChange) -> apply_content_change(File, ContentChange) end, ContentChanges), - %% Not gated on autosave() any more: an open document's diagnostics come - %% from this push channel alone (workspace_diagnostic_item/2 reports open - %% documents as empty), so skipping revalidation here would leave a - %% problem listed until the next save - which is what made an applied - %% quick fix look like it had done nothing. No parse_document/1 call: + %% Not gated on autosave() any more: skipping revalidation here would + %% leave a problem listed until the next save - which is what made an + %% applied quick fix look like it had done nothing. No parse_document/1 call: %% gen_lsp_doc_server:get_syntax_tree/1 now reparses the buffer by itself %% when the cached tree predates this edit, so only the files a request %% actually touches get reparsed. @@ -808,6 +860,18 @@ validate_file(Socket, File) -> ok end. +%% @doc Background lint of a file that is not open (see lsp_diagnostics): +%% from disk, reparsed first when Mode is `reparse`. A file no longer on disk is +%% cleared. +validate_closed_file(Socket, File, Mode) -> + case filelib:is_regular(File) of + false -> + maybe_send_diagnostics(Socket, File, gen_lsp_doc_server:get_document_version(File), []); + true -> + Mode =:= reparse andalso gen_lsp_doc_server:reparse(File), + validate_file(Socket, File) + end. + validate_parsed_source_file(Socket, File) -> ValidatingVersion = gen_lsp_doc_server:get_document_version(File), ?LOG(<<"diag">>, "Validating parsed source file ~p at version ~p", [File, ValidatingVersion]), @@ -885,52 +949,22 @@ request_configuration(Socket, Source) -> %% before a since-applied edit (e.g. a quick fix's workspace/applyEdit %% firing a normal textDocument/didChange), from finishing late and %% clobbering a fresher/correct publishDiagnostics with stale results. -%% -%% Also nudges pull-mode clients: a client showing a diagnostic it pulled -%% via textDocument/diagnostic rather than push is not guaranteed to -%% re-pull right after this edit on its own schedule, so a fresh push is -%% followed by a workspace/diagnostic/refresh request asking it to re-pull -%% now. That request is *debounced* in gen_lsp_server (a burst of pushes -%% produces one refresh) - one refresh per push restarts the client's whole -%% workspace pull each time and is a large part of what made the pull spin. -%% Not gated on the client having declared -%% workspace.diagnostics.refreshSupport - a client that never declared -%% support simply has nothing registered for the request and -%% ignores/errors on it harmlessly, matching how request_configuration/2 -%% already sends without checking a capability first. +%% A closed file is at version 0, so a background lint that started before the +%% file was opened is dropped the same way. maybe_send_diagnostics(Socket, File, ValidatingVersion, Diagnostics) -> case gen_lsp_doc_server:get_document_version(File) of ValidatingVersion -> ?LOG(<<"diag">>, "maybe_send_diagnostics: pushing for ~p at version ~p (~p diagnostics)", [File, ValidatingVersion, length(Diagnostics)]), - send_diagnostics(Socket, File, Diagnostics), - %% Releases any workspace/diagnostic long poll parked in - %% workspace_diagnostic/2 waiting for exactly this. - lsp_diagnostics:notify_changed(), - request_diagnostic_refresh(Socket); + send_diagnostics(Socket, File, Diagnostics); CurrentVersion -> ?LOG(<<"diag">>, "maybe_send_diagnostics: SKIPPED for ~p, stale version ~p (current ~p)", [File, ValidatingVersion, CurrentVersion]) end. -%% @doc No-op handler for the client's reply to a server-initiated -%% workspace/diagnostic/refresh request (see request_diagnostic_refresh/1) -%% - the response body carries nothing meaningful (LSP 3.17 defines it as -%% an empty result), it only needs to be routed here instead of falling -%% into gen_lsp_server's "Notification not handled" error log. -workspace_diagnostic_refresh(_Socket, _Result) -> - ?LOG(<<"diag">>, "workspace_diagnostic_refresh: client acknowledged refresh", []), - ok. - -%% @doc Server-initiated request (LSP 3.17 workspace/diagnostic/refresh) -%% asking every pull-capable client to discard whatever diagnostics it last -%% pulled and pull again. See the comment on maybe_send_diagnostics/4. -%% lsp_diagnostics does the actual send, debounced, with a unique request id. -request_diagnostic_refresh(Socket) -> - lsp_diagnostics:schedule_refresh(Socket). - send_diagnostics(Socket, File, Diagnostics) -> ?LOG(<<"diag">>, "send_diagnostics: ~p (~p diagnostics)", [File, length(Diagnostics)]), + lsp_diagnostics:published(File, Diagnostics), gen_lsp_server:send_to_client(Socket, <<"textDocument/publishDiagnostics">>, #{ method => <<"textDocument/publishDiagnostics">>, params => #{ @@ -941,115 +975,26 @@ send_diagnostics(Socket, File, Diagnostics) -> to_lsp_diagnostic(Diagnostic) -> Info = maps:get(info, Diagnostic), - #{ + LspDiagnostic = #{ severity => severity(maps:get(type, Diagnostic)), range => get_range(Info), message => maps:get(message, Info), source => lsp_utils:try_get(source, Diagnostic, <<"erl">>), data => lsp_utils:try_get(correlation_data, Diagnostic, null) - }. - -%% @doc task 5.9: `textDocument/diagnostic` (LSP 3.17 pull model), -%% alongside the existing push (publishDiagnostics, unchanged - the two -%% are meant to coexist per spec) - reuses the exact same validation and -%% wire-shape as the push path (lsp_syntax:validate_parsed_source_file/1, -%% to_lsp_diagnostic/1), just returned synchronously instead of sent as a -%% notification. -%% A `resultId` is what lets the client say "this is what I already have" on -%% the next pull (as `previousResultId` here, `previousResultIds` for the -%% workspace pull) and lets us answer `unchanged` instead of resending - and, -%% for workspace_diagnostic/2 below, recognise that there is nothing to report -%% at all. -textDocument_diagnostic(_Socket, Params) -> - Uri = mapmapget(textDocument, uri, Params), - File = lsp_utils:file_uri_to_file(Uri), - ?LOG(<<"diag">>, "textDocument_diagnostic: client pulled ~p", [File]), - Diagnostics = diagnostics_for(File), - ResultId = result_id(File, Diagnostics), - case maps:get(previousResultId, Params, undefined) of - ResultId -> - #{kind => <<"unchanged">>, resultId => ResultId}; - _ -> - #{kind => <<"full">>, resultId => ResultId, items => Diagnostics} - end. - -%% @doc `workspace/diagnostic` - the same, for every project file, so -%% problems can be seen without opening each one. -%% -%% This is a *long poll*, not a plain request/response: vscode-languageclient -%% re-issues it as soon as we answer. Answering an all-unchanged pull straight -%% away therefore spins the server, re-linting every project file per round - -%% which is exactly what it did before every report carried a resultId. So when -%% nothing has changed since the resultIds the client sent back, park here until -%% something does (or ?WORKSPACE_DIAGNOSTIC_POLL_MS elapses) and answer then. -%% -%% Blocking is safe: gen_lsp_server spawns a process per incoming message, so -%% this holds up nothing but this one request. cancelRequest/2 is a no-op, so a -%% cancelled pull stays parked until the timeout rather than being released -%% early - bounded, and cheap while parked. -workspace_diagnostic(_Socket, Params) -> - Previous = previous_result_ids(Params), - ?LOG(<<"diag">>, "workspace_diagnostic: client pulled whole workspace (~p files)", - [length(gen_lsp_doc_server:all_project_files())]), - case workspace_diagnostic_items(Previous) of - {changed, Items} -> - #{items => Items}; - {unchanged, Items} -> - case lsp_diagnostics:wait_for_change(?WORKSPACE_DIAGNOSTIC_POLL_MS) of - changed -> - {_, FreshItems} = workspace_diagnostic_items(Previous), - ?LOG(<<"diag">>, "workspace_diagnostic: long poll woken by a change", []), - #{items => FreshItems}; - timeout -> - ?LOG(<<"diag">>, "workspace_diagnostic: long poll timed out, reporting unchanged", []), - #{items => Items} - end - end. - -%% Reports for every project file, plus whether any of them differs from what -%% the client says it already has. -workspace_diagnostic_items(Previous) -> - Opened = gen_lsp_doc_server:opened_documents(), - Items = [workspace_diagnostic_item(File, Previous, lists:member(File, Opened)) - || File <- gen_lsp_doc_server:all_project_files()], - case lists:any(fun (#{kind := Kind}) -> Kind =:= <<"full">> end, Items) of - true -> {changed, Items}; - false -> {unchanged, Items} - end. - -%% An open document is reported as having nothing, whatever it actually has: -%% while it is open, its diagnostics belong to the push channel -%% (textDocument/publishDiagnostics, see maybe_send_diagnostics/4). Reporting -%% them here as well puts the same problem in two DiagnosticCollections at -%% once - the client's pull collection and the push one - which is what -%% showed every problem twice in the Problems list, twice on hover, and -%% offered each quick fix twice (code_actions/3 emits one action per -%% diagnostic in the request context). -%% -%% Reported as an empty `full` report rather than left out of the list: a URI -%% simply missing keeps whatever the client last pulled for it, so dropping -%% the file would freeze its stale pull entry on screen next to the live push -%% one. textDocument_didClose/2 clears the push side, and the next pull sees -%% the file as closed again and reports it for real. -workspace_diagnostic_item(File, Previous, IsOpen) -> - Uri = lsp_utils:file_uri_to_vscode_uri(lsp_utils:file_to_file_uri(File)), - Diagnostics = case IsOpen of - true -> []; - false -> diagnostics_for(File) - end, - ResultId = result_id(File, Diagnostics), - case maps:get(Uri, Previous, undefined) of - ResultId -> - #{uri => Uri, version => null, kind => <<"unchanged">>, resultId => ResultId}; - _ -> - #{uri => Uri, version => null, kind => <<"full">>, resultId => ResultId, items => Diagnostics} + }, + case maps:get(related, Diagnostic, undefined) of + undefined -> + LspDiagnostic; + %% Problems reported on an -include line link to where they really + %% are (see lsp_syntax:extract_group/4). + Related -> + LspDiagnostic#{relatedInformation => [#{ + location => #{uri => lsp_utils:file_uri_to_vscode_uri(lsp_utils:file_to_file_uri(File)), + range => lsp_utils:client_range(Line, Character, Line, Character + 1)}, + message => Message} + || #{file := File, line := Line, character := Character, message := Message} <- Related]} end. -%% `previousResultIds` is a list of #{uri, value} - flatten it to a lookup map. -previous_result_ids(Params) -> - maps:from_list([{Uri, Value} || - #{uri := Uri, value := Value} <- maps:get(previousResultIds, Params, [])]). - %% @doc `erlang/discoverTests` - task 6.1. Delegates to lsp_testing.erl. erlang_discoverTests(Socket, Params) -> lsp_testing:discover_tests(Socket, Params). @@ -1060,22 +1005,6 @@ erlang_discoverTests(Socket, Params) -> erlang_runTests(Socket, Params) -> lsp_testing:run_tests(Socket, Params). -diagnostics_for(File) -> - case gen_lsp_doc_server:get_diagnostics_cache(File) of - undefined -> - ErrorsWarnings = lsp_syntax:validate_parsed_source_file(File), - Diagnostics = lists:map(fun to_lsp_diagnostic/1, maps:get(errors_warnings, ErrorsWarnings, [])), - gen_lsp_doc_server:store_diagnostics_cache(File, Diagnostics), - Diagnostics; - Cached -> - Cached - end. - -%% @doc Stable identity of a file's current diagnostics, so an unchanged pull -%% can be recognised without resending them. -result_id(File, Diagnostics) -> - integer_to_binary(erlang:phash2({File, Diagnostics})). - get_range(Info) -> LS = maps:get(line, Info), CS = maps:get(character, Info), diff --git a/apps/erlangbridge/src/lsp_parse.erl b/apps/erlangbridge/src/lsp_parse.erl index cca83b3d..232e59da 100644 --- a/apps/erlangbridge/src/lsp_parse.erl +++ b/apps/erlangbridge/src/lsp_parse.erl @@ -272,9 +272,14 @@ rebar_define_to_epp_define([H|T]) -> update_file_in_forms(File, File, FileSyntaxTree) -> FileSyntaxTree; +%% epp names files with strings, while ContentsFile is a binary whenever +%% erlang.tmpdir came from the client (a JSON string): compare them as strings, +%% or the temporary copy's name stays in the tree and its problems are taken +%% for an included file's. update_file_in_forms(File, ContentsFile, FileSyntaxTree) -> + ContentsFileName = unicode:characters_to_list(ContentsFile), lists:map(fun - ({attribute, A1, file, {FunContentsFile, A2}}) when FunContentsFile =:= ContentsFile -> + ({attribute, A1, file, {FunContentsFile, A2}}) when FunContentsFile =:= ContentsFileName -> {attribute, A1, file, {File, A2}}; (Form) -> Form diff --git a/apps/erlangbridge/src/lsp_syntax.erl b/apps/erlangbridge/src/lsp_syntax.erl index 44bc1cc0..69a096a8 100644 --- a/apps/erlangbridge/src/lsp_syntax.erl +++ b/apps/erlangbridge/src/lsp_syntax.erl @@ -12,7 +12,8 @@ validate_parsed_source_file(File) -> ParseTranformModules = parse_transforms(FileSyntaxTree, File), ModulesToDelete = load_not_loaded_modules(BehaviourModulea ++ ParseTranformModules), NewFileSyntaxTree = parse_transform(FileSyntaxTree, ParseTranformModules), - Result = lint(NewFileSyntaxTree, File), + IncludeDirectiveLines = include_directive_lines(gen_lsp_doc_server:get_dodged_syntax_tree(File)), + Result = lint(NewFileSyntaxTree, File, IncludeDirectiveLines), % Too long and bad effect on performance with large project %Result = combine_lint(ErlLintResult, NewFileSyntaxTree, File), code_delete(ModulesToDelete), @@ -100,33 +101,112 @@ parse_transform(FileSyntaxTree, Transformers) -> FileSyntaxTree, Transformers). %FileSyntaxTree. -lint(FileSyntaxTree, File) -> +lint(FileSyntaxTree, File, IncludeDirectiveLines) -> LintResult = case lsp_utils:is_erlang_lib_file(File) of false when FileSyntaxTree /= undefined -> erl_lint:module(FileSyntaxTree, File, [strong_validation, {feature, all, enable}]); _ -> {ok, []} end, + %% erl_lint groups what it reports by file: the module itself, plus one + %% group per included file that has something to say. + %% The module is whatever file epp started from - named in the tree's first + %% -file attribute - rather than File, which differs from it should the + %% tree still carry the name of a temporary copy of the buffer. + Main = main_file(File, FileSyntaxTree), + IncludeLines = include_lines(Main, FileSyntaxTree, IncludeDirectiveLines), + Extract = fun (Type, Groups) -> + lists:flatmap(fun (Group) -> + extract_group(Type, Main, IncludeLines, filter_unused_functions(Group)) + end, Groups) + end, case LintResult of - {ok, []} -> - #{parse_result => true}; - {ok, [Warnings]} -> - ErrorsWarnings = extract_error_or_warning(<<"warning">>, filter_unused_functions(Warnings)), - #{parse_result => true, errors_warnings => ErrorsWarnings}; - {error, [Errors], []} -> - ErrorsWarnings = extract_error_or_warning(<<"error">>, Errors), - #{parse_result => true, errors_warnings => ErrorsWarnings}; - {error, [Errors], [Warnings]} -> - ErrorsWarnings = extract_error_or_warning(<<"error">>, Errors) ++ - extract_error_or_warning(<<"warning">>, filter_unused_functions(Warnings)), - #{parse_result => true, errors_warnings => ErrorsWarnings}; - {error, [], [Warnings]} -> - ErrorsWarnings = extract_error_or_warning(<<"warning">>, filter_unused_functions(Warnings)), - #{parse_result => true, errors_warnings => ErrorsWarnings}; + {ok, Warnings} -> + lint_result(Extract(<<"warning">>, Warnings)); + {error, Errors, Warnings} -> + lint_result(Extract(<<"error">>, Errors) ++ Extract(<<"warning">>, Warnings)); _Any -> - #{parse_result => false, error_message => <<"lint error">>} + #{parse_result => false, error_message => <<"lint error">>} end. +main_file(_File, [{attribute, _, file, {Main, _}} | _]) -> Main; +main_file(File, _Forms) -> File. + +lint_result([]) -> #{parse_result => true}; +lint_result(ErrorsWarnings) -> #{parse_result => true, errors_warnings => ErrorsWarnings}. + +extract_group(_Type, _File, _IncludeLines, []) -> + []; +extract_group(Type, File, _IncludeLines, {File, _} = Group) -> + extract_error_or_warning(Type, Group); +extract_group(Type, _File, IncludeLines, {IncludedFile, Infos}) -> + %% Problems in an included file have no place of their own in the module: + %% they go on the -include line that brought the file in, as a single + %% diagnostic naming the file and linking to each of them - the links + %% carry the messages, so the hover does not repeat each one. Their + %% positions are the header's, so it carries no correlation data - a quick + %% fix would edit the module at them. + IncludeLine = maps:get(IncludedFile, IncludeLines, 1), + Related = [begin + #{line := Line, character := Column, message := Message} = extract_info(Info), + #{file => IncludedFile, line => Line, character => Column, message => Message} + end || Info <- Infos], + Count = length(Related), + Summary = unicode:characters_to_binary( + io_lib:format("~p ~ts~ts in included file ~ts", + [Count, Type, plural(Count), filename:basename(IncludedFile)])), + [#{type => Type, + file => unicode:characters_to_binary(IncludedFile), + info => #{line => IncludeLine, character => 1, message => Summary}, + related => Related}]. + +plural(1) -> ""; +plural(_) -> "s". + +%% @doc Every file included by File, directly or through another header, +%% mapped to the line of the -include in File that brought it in. epp marks +%% entering an included file with -file(Included, _) and coming back with +%% -file(File, L), L being where File resumes - the line after the -include +%% when its `.` is followed by a bare newline, but the -include line itself +%% with CRLF line endings, a comment after the `.`, or no final newline. So L +%% only bounds it: the -include is the last one at or before L, read from the +%% epp_dodger tree, which keeps the directives epp expands. +include_lines(_File, undefined, _IncludeDirectiveLines) -> + #{}; +include_lines(File, Forms, IncludeDirectiveLines) -> + {Lines, _Pending} = lists:foldl(fun + ({attribute, _, file, {F, L}}, {Acc, Pending}) when F =:= File -> + IncludeLine = include_line_before(L, IncludeDirectiveLines), + {lists:foldl(fun (Included, In) -> + maps:merge(#{Included => IncludeLine}, In) + end, Acc, Pending), []}; + ({attribute, _, file, {F, _}}, {Acc, Pending}) -> + {Acc, [F | Pending]}; + (_, Acc) -> + Acc + end, {#{}, []}, Forms), + Lines. + +include_line_before(L, IncludeDirectiveLines) -> + case [Line || Line <- IncludeDirectiveLines, Line =< L] of + [] -> max(L - 1, 1); % no dodged tree: assume a bare newline + Before -> lists:last(Before) + end. + +%% Sorted lines of the -include and -include_lib directives in a dodged tree. +include_directive_lines(undefined) -> + []; +include_directive_lines(DodgedForms) -> + lists:usort(lists:filtermap(fun (Form) -> + try erl_syntax:type(Form) =:= attribute andalso + lists:member(erl_syntax:atom_value(erl_syntax:attribute_name(Form)), + [include, include_lib]) of + true -> {true, erl_anno:line(erl_syntax:get_pos(Form))}; + false -> false + catch _:_ -> false + end + end, DodgedForms)). + % combine_lint(LintResult, SyntaxTree, File) -> % case LintResult of % #{parse_result := ParseResult, errors_warnings := ErrorsWarnings} @@ -305,7 +385,7 @@ extract_error_or_warning(_Type, {_, []}) -> extract_error_or_warning(Type, ErrorsOrWarnings) -> [#{type => Type, file => - erlang:list_to_binary(element(1, ErrorsOrWarnings)), + unicode:characters_to_binary(element(1, ErrorsOrWarnings)), info => extract_info(X), correlation_data => correlation_data(X)} || X <- element(2, ErrorsOrWarnings)]. @@ -319,7 +399,7 @@ extract_info({{Line, Column}, Module, MessageBody}) -> #{ line => Line, character => Column, - message => erlang:list_to_binary(lists:flatten(apply(Module, format_error, [MessageBody]), [])) + message => unicode:characters_to_binary(apply(Module, format_error, [MessageBody])) }. %% @doc Task 2.1 infrastructure: a JSON-safe, machine-readable identity for diff --git a/apps/erlangbridge/test/lsp_diagnostic_SUITE.erl b/apps/erlangbridge/test/lsp_diagnostic_SUITE.erl index a5e5ca52..18cf046f 100644 --- a/apps/erlangbridge/test/lsp_diagnostic_SUITE.erl +++ b/apps/erlangbridge/test/lsp_diagnostic_SUITE.erl @@ -7,24 +7,30 @@ -include("./testlog.hrl"). -%% Task 5.9: textDocument/diagnostic (per-document pull) and workspace/ -%% diagnostic (project-wide pull), alongside the existing push model -%% (unchanged - lsp_syntax_SUITE/lsp_codeaction_SUITE already exercise -%% that). Both reuse to_lsp_diagnostic/1, the exact same wire shape -%% push already uses (lsp_handlers:send_diagnostics/3) - with_warning.erl -%% has one real erl_lint warning (X unused); clean.erl has none. +%% Diagnostics are pushed only (textDocument/publishDiagnostics): open +%% documents from their buffer, closed project files from disk, through +%% lsp_diagnostics' background queue. No pull (textDocument/diagnostic, +%% workspace/diagnostic): running both made the client hold two collections +%% for the same file, so every problem showed twice on hover and in quick +%% fixes. with_warning.erl has one real erl_lint warning (X unused); +%% clean.erl has none; records.hrl is a header, never linted (#356). all() -> [ - pull_diagnostic_reports_a_real_warning, - pull_diagnostic_reports_nothing_for_a_clean_file, - workspace_diagnostic_covers_every_project_file, - workspace_diagnostic_reports_an_open_document_as_empty, + initialize_does_not_advertise_pull_diagnostics, + project_scan_pushes_diagnostics_for_closed_files, + closing_a_document_republishes_its_on_disk_diagnostics, + watched_file_change_republishes_a_closed_file, + deleted_file_is_cleared, + excluded_file_is_cleared_on_rescan, + linting_disabled_clears_published_diagnostics, + open_document_is_published_once, + an_open_header_file_is_never_linted, + closed_includer_is_relinted_when_its_header_changes, + open_includer_is_relinted_when_its_header_is_saved, + header_change_is_not_registered_as_a_module, + open_buffer_problems_stay_on_their_own_line, an_unsaved_edit_republishes_diagnostics_for_the_new_contents, stale_validation_result_is_dropped_once_a_newer_edit_has_landed, - fresh_publish_is_followed_by_a_diagnostic_refresh_request, - pull_diagnostic_reports_unchanged_when_result_id_matches, - workspace_diagnostic_long_polls_until_something_changes, - diagnostic_refreshes_are_debounced_and_uniquely_identified, identical_configuration_is_not_revalidated, configuration_never_publishes_an_empty_clear_ahead_of_the_real_state, concurrent_configuration_calls_do_not_clobber_a_later_edit @@ -47,51 +53,197 @@ init_per_testcase(_TestCase, Config) -> gen_lsp_doc_server:config_change(), Config. +%% Every case starts with nothing open, default settings, and no lint left +%% over from the previous case that could land on its socket. +end_per_testcase(_TestCase, Config) -> + lists:foreach(fun gen_lsp_doc_server:document_closed/1, gen_lsp_doc_server:opened_documents()), + gen_lsp_config_server:update_config(erlang, #{verbose => false}), + gen_lsp_config_server:update_config(search, #{}), + gen_lsp_config_server:update_config(computed, #{}), + wait_until_idle(100), + Config. + %%%%%%%%%%%%%%%% %% test cases %% %%%%%%%%%%%%%%%% -pull_diagnostic_reports_a_real_warning(Config) -> - #{kind := <<"full">>, items := [Diagnostic]} = pull(Config, "with_warning.erl"), - ?assertEqual(<<"variable 'X' is unused">>, maps:get(message, Diagnostic)), - ?assertEqual(2, maps:get(severity, Diagnostic)). - -pull_diagnostic_reports_nothing_for_a_clean_file(Config) -> - ?assertMatch(#{kind := <<"full">>, resultId := _, items := []}, pull(Config, "clean.erl")). - -%% One WorkspaceFullDocumentDiagnosticReport per project file, each with -%% its own uri and the same items shape the single-document pull uses. -workspace_diagnostic_covers_every_project_file(_Config) -> - #{items := Reports} = lsp_handlers:workspace_diagnostic(undefined, #{}), - ?assertEqual(2, length(Reports)), - [WithWarningReport] = [R || R <- Reports, binary:match(maps:get(uri, R), <<"with_warning.erl">>) =/= nomatch], - ?assertMatch(#{kind := <<"full">>, version := null, resultId := _, items := [_]}, WithWarningReport), - [CleanReport] = [R || R <- Reports, binary:match(maps:get(uri, R), <<"clean.erl">>) =/= nomatch], - ?assertMatch(#{kind := <<"full">>, resultId := _, items := []}, CleanReport). - -%% Regression test for every problem being listed twice (and every quick fix -%% offered twice): while a document is open, its diagnostics belong to the -%% push channel, so the workspace pull must not report them a second time -%% into the client's own DiagnosticCollection. Reported as an empty `full` -%% report rather than omitted - a URI missing from the report keeps whatever -%% the client last pulled for it, which would freeze the duplicate on screen -%% instead of clearing it. -workspace_diagnostic_reports_an_open_document_as_empty(Config) -> - AppDir = ?config(data_dir, Config), - File = filename:join(AppDir, "with_warning.erl"), +%% Without diagnosticProvider, vscode-languageclient never pulls: one channel, +%% one collection, no duplicate. +initialize_does_not_advertise_pull_diagnostics(_Config) -> + #{capabilities := Capabilities} = lsp_handlers:initialize(undefined, #{rootPath => null}), + ?assertNot(maps:is_key(diagnosticProvider, Capabilities)). + +%% Problems for files never opened come from the project scan: each closed +%% module is linted from disk and pushed - a clean one as an explicit empty +%% list - and a header is not linted at all. +project_scan_pushes_diagnostics_for_closed_files(Config) -> + Raw = configure(#{}), + ?assertMatch([[#{message := <<"variable 'X' is unused">>}]], + publishes_for(Raw, data_file(Config, "with_warning.erl"))), + ?assertEqual([[]], publishes_for(Raw, data_file(Config, "clean.erl"))), + ?assertEqual([], publishes_for(Raw, data_file(Config, "records.hrl"))). + +%% Closing a document no longer clears it: its unsaved edits are gone, so +%% what it shows from then on is what is on disk. +closing_a_document_republishes_its_on_disk_diagnostics(Config) -> + File = data_file(Config, "with_warning.erl"), {ok, Content} = file:read_file(File), + Uri = lsp_utils:file_to_file_uri(File), + + {ServerSocket, ClientSocket} = open_socket_pair(), + lsp_handlers:textDocument_didOpen(ServerSocket, #{textDocument => + #{uri => Uri, text => Content, version => 1}}), + lsp_handlers:textDocument_didChange(ServerSocket, #{ + textDocument => #{uri => Uri, version => 2}, + contentChanges => [#{text => fixed_with_warning()}]}), + EditRaw = drain_raw(ClientSocket, 5000, <<>>), + ?assertEqual([[]], publishes_for(EditRaw, File)), + + lsp_handlers:textDocument_didClose(ServerSocket, #{textDocument => #{uri => Uri}}), + CloseRaw = drain_raw(ClientSocket, 5000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + ?assertMatch([[#{message := <<"variable 'X' is unused">>}]], publishes_for(CloseRaw, File)). + +%% A closed file edited outside the editor (another tool, a branch switch) is +%% relinted from disk - reparsed, not linted from the tree held for it. +watched_file_change_republishes_a_closed_file(Config) -> + File = filename:join(?config(priv_dir, Config), "watched.erl"), + ok = file:write_file(File, with_warning_module(watched)), + {ServerSocket, ClientSocket} = open_socket_pair(), - ?assertMatch(#{kind := <<"full">>, items := [_]}, workspace_report(<<"with_warning.erl">>)), + watched_file_event(ServerSocket, File, 1), + CreatedRaw = drain_raw(ClientSocket, 5000, <<>>), + ?assertMatch([[#{message := <<"variable 'X' is unused">>}]], publishes_for(CreatedRaw, File)), - gen_lsp_doc_server:document_opened(File, Content), - ?assertMatch(#{kind := <<"full">>, items := []}, workspace_report(<<"with_warning.erl">>)), - %% the file that stayed closed is still reported for real (clean.erl has - %% nothing to report, but it is reported) - ?assertMatch(#{kind := <<"full">>, items := []}, workspace_report(<<"clean.erl">>)), + ok = file:write_file(File, <<"-module(watched).\n-export([go/0]).\n\ngo() -> ok.\n">>), + watched_file_event(ServerSocket, File, 2), + ChangedRaw = drain_raw(ClientSocket, 5000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + ?assertEqual([[]], publishes_for(ChangedRaw, File)). - %% closing it hands the file back to the pull channel - gen_lsp_doc_server:document_closed(File), - ?assertMatch(#{kind := <<"full">>, items := [_]}, workspace_report(<<"with_warning.erl">>)). +deleted_file_is_cleared(Config) -> + File = filename:join(?config(priv_dir, Config), "deleted.erl"), + ok = file:write_file(File, with_warning_module(deleted)), + {ServerSocket, ClientSocket} = open_socket_pair(), + watched_file_event(ServerSocket, File, 1), + CreatedRaw = drain_raw(ClientSocket, 5000, <<>>), + ?assertMatch([[_]], publishes_for(CreatedRaw, File)), + + ok = file:delete(File), + watched_file_event(ServerSocket, File, 3), + DeletedRaw = drain_raw(ClientSocket, 5000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + ?assertEqual([[]], publishes_for(DeletedRaw, File)). + +%% With push only, the client keeps whatever it was last sent: a file that +%% leaves the project has to be cleared explicitly. +excluded_file_is_cleared_on_rescan(Config) -> + File = data_file(Config, "with_warning.erl"), + ?assertMatch([[_]], publishes_for(configure(#{}), File)), + Raw = configure(#{exclude => #{'**/with_warning.erl' => true}}), + ?assertEqual([[]], publishes_for(Raw, File)). + +linting_disabled_clears_published_diagnostics(Config) -> + File = data_file(Config, "with_warning.erl"), + ?assertMatch([[_]], publishes_for(configure(#{}), File)), + Raw = configure(#{}, #{linting => false}), + ?assertEqual([[]], publishes_for(Raw, File)), + ?assertEqual([], publishes_for(Raw, data_file(Config, "clean.erl"))). + +%% The duplicate on hover came from the same file being reported through two +%% channels: opening a file now yields one push, and nothing asks the client +%% to pull. +open_document_is_published_once(Config) -> + File = data_file(Config, "with_warning.erl"), + {ok, Content} = file:read_file(File), + {ServerSocket, ClientSocket} = open_socket_pair(), + lsp_handlers:textDocument_didOpen(ServerSocket, #{textDocument => + #{uri => lsp_utils:file_to_file_uri(File), text => Content, version => 1}}), + Raw = drain_raw(ClientSocket, 5000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + ?assertMatch([[#{message := <<"variable 'X' is unused">>}]], publishes_for(Raw, File)), + ?assertEqual(nomatch, binary:match(Raw, <<"workspace\\/diagnostic\\/refresh">>)). + +%% #356: a header is not a module - linting it on its own reported "no module +%% definition" and every record it declares as unused. +an_open_header_file_is_never_linted(Config) -> + File = data_file(Config, "records.hrl"), + {ok, Content} = file:read_file(File), + {ServerSocket, ClientSocket} = open_socket_pair(), + lsp_handlers:textDocument_didOpen(ServerSocket, #{textDocument => + #{uri => lsp_utils:file_to_file_uri(File), text => Content, version => 1}}), + Raw = drain_raw(ClientSocket, 3000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + ?assertEqual([], publishes_for(Raw, File)). + +%% A module's syntax tree holds its headers as they were when it was parsed, +%% and nothing about the module itself changes when one of them does - so a +%% fixed header left its error on every module including it until that +%% module was edited. A header change seen by the file watcher relints them. +closed_includer_is_relinted_when_its_header_changes(Config) -> + {Hrl, File} = write_includer(Config, "closed_dep.hrl", "closed_includer"), + {ServerSocket, ClientSocket} = open_socket_pair(), + watched_file_event(ServerSocket, File, 1), + BrokenRaw = drain_raw(ClientSocket, 5000, <<>>), + [[Diagnostic]] = publishes_for(BrokenRaw, File), + %% on the -include line (0-based 1), linking to the header + ?assertMatch(#{range := #{start := #{line := 1}}, + relatedInformation := [#{location := #{range := #{start := #{line := 0}}}}]}, + Diagnostic), + [#{location := #{uri := HrlUri}}] = maps:get(relatedInformation, Diagnostic), + ?assertEqual(lsp_utils:file_uri_to_vscode_uri(lsp_utils:file_to_file_uri(Hrl)), HrlUri), + + ok = file:write_file(Hrl, <<"-record(r, {a}).\n">>), + watched_file_event(ServerSocket, Hrl, 2), + FixedRaw = drain_raw(ClientSocket, 5000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + ?assertEqual([[]], publishes_for(FixedRaw, File)). + +%% Same for an open module, whose header is saved from the editor: its +%% buffer is reparsed, since the buffer itself did not change. +open_includer_is_relinted_when_its_header_is_saved(Config) -> + {Hrl, File} = write_includer(Config, "open_dep.hrl", "open_includer"), + {ok, Content} = file:read_file(File), + {ServerSocket, ClientSocket} = open_socket_pair(), + lsp_handlers:textDocument_didOpen(ServerSocket, #{textDocument => + #{uri => lsp_utils:file_to_file_uri(File), text => Content, version => 1}}), + OpenRaw = drain_raw(ClientSocket, 5000, <<>>), + ?assertMatch([[_]], publishes_for(OpenRaw, File)), + + ok = file:write_file(Hrl, <<"-record(r, {a}).\n">>), + lsp_handlers:textDocument_didSave(ServerSocket, #{textDocument => + #{uri => lsp_utils:file_to_file_uri(Hrl)}}), + SavedRaw = drain_raw(ClientSocket, 5000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + ?assertEqual([[]], publishes_for(SavedRaw, File)). + +%% The watcher now reports headers too; they must not be added to the project +%% as if they were modules. +header_change_is_not_registered_as_a_module(Config) -> + Hrl = filename:join(?config(priv_dir, Config), "not_a_module.hrl"), + ok = file:write_file(Hrl, <<"-define(X, 1).\n">>), + watched_file_event(undefined, Hrl, 1), + ?assertNot(lists:member("not_a_module", gen_lsp_doc_server:project_modules())). + +%% An open buffer is parsed from a temporary copy. The client sends +%% erlang.tmpdir as a JSON string, i.e. a binary, so the copy's path was a +%% binary while epp names files with strings: the module's own -file +%% attribute kept the copy's name, and its problems - taken for a header's - +%% all landed on line 1 of the module, prefixed with the copy's random name. +open_buffer_problems_stay_on_their_own_line(Config) -> + gen_lsp_config_server:update_config(computed, + #{tmpdir => unicode:characters_to_binary(?config(priv_dir, Config))}), + File = data_file(Config, "with_warning.erl"), + {ok, Content} = file:read_file(File), + {ServerSocket, ClientSocket} = open_socket_pair(), + lsp_handlers:textDocument_didOpen(ServerSocket, #{textDocument => + #{uri => lsp_utils:file_to_file_uri(File), text => Content, version => 1}}), + Raw = drain_raw(ClientSocket, 5000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + [[Diagnostic]] = publishes_for(Raw, File), + ?assertMatch(#{message := <<"variable 'X' is unused">>, + range := #{start := #{line := 4}}}, Diagnostic), + ?assertNot(maps:is_key(relatedInformation, Diagnostic)). %% Regression test for "the quick fix is applied but the problem stays in the %% Problems list": a quick fix's WorkspaceEdit reaches the server as a plain @@ -115,10 +267,9 @@ an_unsaved_edit_republishes_diagnostics_for_the_new_contents(Config) -> %% the fix: use X, so erl_lint has nothing left to say. No didSave, and %% no parse_document/1 - exactly what a quick fix's applyEdit produces. - Fixed = <<"-module(with_warning).\n-export([go/0]).\n\ngo() ->\n X = 1,\n X.\n">>, lsp_handlers:textDocument_didChange(ServerSocket, #{ textDocument => #{uri => Uri, version => 2}, - contentChanges => [#{text => Fixed}]}), + contentChanges => [#{text => fixed_with_warning()}]}), ChangeRaw = drain_raw(ClientSocket, 5000, <<>>), gen_tcp:close(ServerSocket), gen_tcp:close(ClientSocket), @@ -131,8 +282,8 @@ an_unsaved_edit_republishes_diagnostics_for_the_new_contents(Config) -> %% Regression test for the out-of-order validate_file/2 race behind the %% "quick fix leaves a stale red squiggle" bug: every incoming LSP message -%% is handled by gen_lsp_server via an independent spawn/1 -%% (gen_lsp_server.erl:201/206), with no ordering guarantee, so a slow +%% but document sync is handled by gen_lsp_server via an independent spawn/1, +%% with no ordering guarantee between them, so a slow %% validate started against an OLD document version can finish and publish %% *after* a newer, correct one already did - clobbering the client's %% diagnostics with stale results. Reproduced here deterministically (no @@ -161,9 +312,8 @@ stale_validation_result_is_dropped_once_a_newer_edit_has_landed(Config) -> %% The current validate_file, finishing after, must publish. lsp_handlers:maybe_send_diagnostics(ServerSocket, File, FreshVersion, []), - %% Both the stale-drop and the fresh publish's own refresh request (see - %% the next test) can be on the wire here - drain raw bytes rather than - %% decoding a single framed message, and check what actually landed. + %% Drain raw bytes rather than decoding a single framed message, and + %% check what actually landed. Raw = drain_raw(ClientSocket, 5000, <<>>), gen_tcp:close(ServerSocket), gen_tcp:close(ClientSocket), @@ -173,85 +323,6 @@ stale_validation_result_is_dropped_once_a_newer_edit_has_landed(Config) -> %% Exactly the fresh, empty diagnostics list was published. ?assertNotEqual(nomatch, binary:match(Raw, <<"\"diagnostics\":[]">>)). -%% A pull-mode client (textDocument/diagnostic) may be showing a diagnostic -%% it pulled once and is not guaranteed to re-pull right after an edit on -%% its own schedule - every fresh publish must also nudge it to re-pull via -%% workspace/diagnostic/refresh (LSP 3.17), sent unconditionally (task -%% history: not gated on the client having declared -%% workspace.diagnostics.refreshSupport, per explicit product direction). -fresh_publish_is_followed_by_a_diagnostic_refresh_request(Config) -> - AppDir = ?config(data_dir, Config), - File = filename:join(AppDir, "clean.erl"), - {ok, Content} = file:read_file(File), - gen_lsp_doc_server:document_opened(File, Content), - Version = gen_lsp_doc_server:get_document_version(File), - - {ServerSocket, ClientSocket} = open_socket_pair(), - lsp_handlers:maybe_send_diagnostics(ServerSocket, File, Version, []), - Raw = drain_raw(ClientSocket, 5000, <<>>), - gen_tcp:close(ServerSocket), - gen_tcp:close(ClientSocket), - - ?assertNotEqual(nomatch, binary:match(Raw, <<"textDocument\\/publishDiagnostics">>)), - ?assertNotEqual(nomatch, binary:match(Raw, <<"workspace\\/diagnostic\\/refresh">>)). - -%% Handing back the resultId the server just gave us means "I already have -%% this" - the server must say so rather than resend the items. Without a -%% resultId on the way out, the client can never say this, which is half of -%% why the workspace pull below used to spin. -pull_diagnostic_reports_unchanged_when_result_id_matches(Config) -> - #{resultId := ResultId} = pull(Config, "with_warning.erl"), - AppDir = ?config(data_dir, Config), - File = filename:join(AppDir, "with_warning.erl"), - Params = #{textDocument => #{uri => lsp_utils:file_to_file_uri(File)}, - previousResultId => ResultId}, - ?assertEqual(#{kind => <<"unchanged">>, resultId => ResultId}, - lsp_handlers:textDocument_diagnostic(undefined, Params)). - -%% Regression test for the workspace/diagnostic spin: vscode-languageclient -%% re-issues the pull as soon as it is answered, so a pull with nothing new to -%% report must NOT answer straight away - it parks until diagnostics actually -%% change. Before this, every round answered instantly with full reports and -%% re-linted every project file, forever. -workspace_diagnostic_long_polls_until_something_changes(_Config) -> - #{items := Reports} = lsp_handlers:workspace_diagnostic(undefined, #{}), - Previous = [#{uri => maps:get(uri, R), value => maps:get(resultId, R)} || R <- Reports], - Params = #{previousResultIds => Previous}, - - Self = self(), - Puller = spawn(fun () -> Self ! {pulled, lsp_handlers:workspace_diagnostic(undefined, Params)} end), - - %% Nothing changed, so the pull must still be parked. - receive {pulled, _} -> ct:fail("workspace_diagnostic answered an unchanged pull immediately") - after 1000 -> ok - end, - - lsp_diagnostics:notify_changed(), - receive - {pulled, #{items := Woken}} -> - ?assertEqual(length(Reports), length(Woken)) - after 5000 -> - exit(Puller, kill), - ct:fail("workspace_diagnostic stayed parked after a change was signalled") - end. - -%% One refresh per push restarts the client's whole workspace pull each time, -%% so a burst must collapse to one request - and each request needs its own -%% JSON-RPC id (they used to share a constant one, so three requests went out -%% and only one response ever came back). -diagnostic_refreshes_are_debounced_and_uniquely_identified(Config) -> - AppDir = ?config(data_dir, Config), - File = filename:join(AppDir, "clean.erl"), - {ok, Content} = file:read_file(File), - gen_lsp_doc_server:document_opened(File, Content), - Version = gen_lsp_doc_server:get_document_version(File), - - FirstIds = refresh_ids_of_a_push_burst(File, Version), - ?assertEqual(1, length(FirstIds)), - SecondIds = refresh_ids_of_a_push_burst(File, Version), - ?assertEqual(1, length(SecondIds)), - ?assertNotEqual(FirstIds, SecondIds). - %% workspace/configuration is answered more than once per session (the trace %% behind this change shows configuration/2 running twice at startup), and each %% run used to redo the whole project scan plus a validation pass per open @@ -353,35 +424,56 @@ concurrent_configuration_calls_do_not_clobber_a_later_edit(Config) -> %% helpers %% %%%%%%%%%%%%% -%% Three pushes back to back, then whatever refresh ids reached the client. -refresh_ids_of_a_push_burst(File, Version) -> +data_file(Config, Name) -> + filename:join(?config(data_dir, Config), Name). + +fixed_with_warning() -> + <<"-module(with_warning).\n-export([go/0]).\n\ngo() ->\n X = 1,\n X.\n">>. + +with_warning_module(Name) -> + iolist_to_binary(["-module(", atom_to_list(Name), ").\n-export([go/0]).\n\ngo() ->\n X = 1,\n ok.\n"]). + +%% A header with a syntax error, and a module including it on line 2. +write_includer(Config, HrlName, Module) -> + Dir = ?config(priv_dir, Config), + Hrl = filename:join(Dir, HrlName), + File = filename:join(Dir, Module ++ ".erl"), + ok = file:write_file(Hrl, <<"-record(r, {a = )}).\n">>), + ok = file:write_file(File, iolist_to_binary(["-module(", Module, ").\n-include(\"", HrlName, + "\").\n-export([go/0]).\n\ngo() -> ok.\n"])), + {Hrl, File}. + +watched_file_event(Socket, File, Type) -> + lsp_handlers:workspace_didChangeWatchedFiles(Socket, #{changes => + [#{uri => lsp_utils:file_to_file_uri(File), type => Type}]}). + +%% configuration/2 with the given search section and erlang settings, plus a +%% marker so it is never short-circuited as identical to the previous call; +%% returns everything pushed until the background lint goes quiet. +configure(SearchSection) -> + configure(SearchSection, #{}). + +configure(SearchSection, ErlangSettings) -> + Erlang = maps:merge(#{verbose => false, linting => true, marker => erlang:unique_integer()}, + ErlangSettings), {ServerSocket, ClientSocket} = open_socket_pair(), - lists:foreach(fun (_) -> - lsp_handlers:maybe_send_diagnostics(ServerSocket, File, Version, []) - end, [1, 2, 3]), + lsp_handlers:configuration(ServerSocket, [Erlang, #{}, #{}, #{}, SearchSection]), Raw = drain_raw(ClientSocket, 5000, <<>>), + close_socket_pair(ServerSocket, ClientSocket), + Raw. + +close_socket_pair(ServerSocket, ClientSocket) -> gen_tcp:close(ServerSocket), - gen_tcp:close(ClientSocket), - case re:run(Raw, "\"(workspace_diagnostic_refresh#[0-9]+)\"", - [global, {capture, all_but_first, binary}]) of - {match, Matches} -> lists:usort([Id || [Id] <- Matches]); - nomatch -> [] + gen_tcp:close(ClientSocket). + +wait_until_idle(0) -> + ct:fail(lsp_diagnostics_never_idle); +wait_until_idle(N) -> + case lsp_diagnostics:idle() of + true -> ok; + false -> timer:sleep(100), wait_until_idle(N - 1) end. -pull(Config, FileName) -> - AppDir = ?config(data_dir, Config), - File = filename:join(AppDir, FileName), - Params = #{textDocument => #{uri => lsp_utils:file_to_file_uri(File)}}, - lsp_handlers:textDocument_diagnostic(undefined, Params). - -%% One named file's report out of a fresh whole-workspace pull. Sending no -%% previousResultIds makes every report `full`, so the pull answers at once -%% rather than parking in its long poll. -workspace_report(FileName) -> - #{items := Reports} = lsp_handlers:workspace_diagnostic(undefined, #{}), - [Report] = [R || R <- Reports, binary:match(maps:get(uri, R), FileName) =/= nomatch], - Report. - open_socket_pair() -> {ok, LSock} = gen_tcp:listen(0, [binary, {active, false}, {packet, raw}, {ip, {127, 0, 0, 1}}]), {ok, Port} = inet:port(LSock), @@ -393,7 +485,7 @@ open_socket_pair() -> %% Raw bytes until the stream goes quiet, rather than a single %% Content-Length-framed message: more than one notification/request can %% legitimately be in flight per test case here (a dropped stale publish -%% alongside a fresh one, or a publish alongside its own refresh request), +%% alongside a fresh one, or one publish per project file after a scan), %% and a framed single-message reader would silently drop whatever %% trailing bytes of a *following* message arrived in the same TCP read. drain_raw(Socket, Timeout, Acc) -> diff --git a/apps/erlangbridge/test/lsp_diagnostic_SUITE_data/records.hrl b/apps/erlangbridge/test/lsp_diagnostic_SUITE_data/records.hrl new file mode 100644 index 00000000..87529316 --- /dev/null +++ b/apps/erlangbridge/test/lsp_diagnostic_SUITE_data/records.hrl @@ -0,0 +1,2 @@ +-record(point, {x = 0, y = 0}). +-record(user, {name, age}). diff --git a/apps/erlangbridge/test/lsp_protocol_SUITE.erl b/apps/erlangbridge/test/lsp_protocol_SUITE.erl index 34aa2bb3..8b64ac7f 100644 --- a/apps/erlangbridge/test/lsp_protocol_SUITE.erl +++ b/apps/erlangbridge/test/lsp_protocol_SUITE.erl @@ -28,7 +28,8 @@ all() -> [ shutdown_closes_the_connection, exit_closes_the_connection, initialized_requests_configuration_with_a_tagged_unique_id, - did_change_configuration_burst_collapses_to_one_configuration_call + did_change_configuration_burst_collapses_to_one_configuration_call, + did_change_burst_is_applied_in_order ]. init_per_testcase(_TestCase, Config) -> @@ -141,6 +142,36 @@ did_change_configuration_burst_collapses_to_one_configuration_call(Config) -> result => [#{verbose => false}, #{}, #{}, #{}, #{}]}), assert_connection_still_responsive(Socket). +%% Document sync must be applied in wire order. gen_lsp_server used to spawn +%% one process per incoming message, so a burst of incremental didChange +%% notifications - each one positioned against the buffer left by the +%% previous one - could be applied out of order, or race on the same buffer +%% (read, splice, write) and lose an edit. Diagnostics and every other +%% analysis were then computed on a text the editor never had. +did_change_burst_is_applied_in_order(Config) -> + Socket = ?config(socket, Config), + Peer = ?config(peer, Config), + %% Not an .erl: nothing to parse or lint, only the buffer is under test. + File = filename:join(?config(priv_dir, Config), "ordering.txt"), + Uri = peer:call(Peer, lsp_utils, file_to_file_uri, [File]), + Count = 300, + ok = send_notification(Socket, <<"textDocument/didOpen">>, + #{textDocument => #{uri => Uri, text => <<>>, version => 0}}), + %% Each change appends one character at the end of what the previous one + %% left, so any reordering or lost update shows up in the final text. + lists:foreach(fun (I) -> + ok = send_notification(Socket, <<"textDocument/didChange">>, #{ + textDocument => #{uri => Uri, version => I}, + contentChanges => [#{range => #{start => #{line => 0, character => I - 1}, + 'end' => #{line => 0, character => I - 1}}, + text => <<($a + (I rem 26))>>}]}) + end, lists:seq(1, Count)), + %% Requests are still answered concurrently, but only after every sync + %% notification ahead of them has been applied. + assert_connection_still_responsive(Socket), + Expected = << <<($a + (I rem 26))>> || I <- lists:seq(1, Count) >>, + ?assertEqual(Expected, peer:call(Peer, gen_lsp_doc_server, get_document_contents, [File])). + %%%%%%%%%%%%%%%%%%%%%% %% golden reference %% %%%%%%%%%%%%%%%%%%%%%% @@ -192,7 +223,7 @@ golden_initialize_result() -> typeHierarchyProvider => true, %% task 4.6 inlineValueProvider => true, inlayHintProvider => #{resolveProvider => true}, %% task 5.8 - diagnosticProvider => #{interFileDependencies => true, workspaceDiagnostics => true}, %% task 5.9 + %% no diagnosticProvider: push only workspaceSymbolProvider => #{resolveProvider => true}, %% task 4.1 workspace => #{ workspaceFolders => #{supported => true, changeNotifications => true} diff --git a/apps/erlangbridge/test/lsp_syntax_SUITE.erl b/apps/erlangbridge/test/lsp_syntax_SUITE.erl index 192e6981..89a47aeb 100644 --- a/apps/erlangbridge/test/lsp_syntax_SUITE.erl +++ b/apps/erlangbridge/test/lsp_syntax_SUITE.erl @@ -21,6 +21,11 @@ all() -> [ unused_variable_is_reported, unused_function_is_reported, missing_include_is_reported, + non_latin1_path_and_message_are_utf8, + error_in_included_file_is_reported_on_the_include_line, + module_and_header_problems_are_both_reported, + header_problem_is_on_the_include_line_with_crlf_endings, + header_problem_is_on_the_include_line_with_a_trailing_comment, bad_record_field_is_reported, parse_transform_from_rebar_config_is_applied, eunit_generator_is_not_reported_as_unused, @@ -81,6 +86,63 @@ missing_include_is_reported(Config) -> ?assertEqual(2, maps:get(line, Info)), assert_message_contains(Info, "does_not_exist.hrl"). +%% erl_lint reports a header's problems in a group of their own, at the +%% header's positions. They used to be shown at those positions in the module +%% (a syntax error on line 1 of the header underlined line 1 of the module); +%% they go on the -include line as one diagnostic naming the header, with no +%% correlation data - a quick fix would edit the module at the header's +%% positions - and a link to each problem in the header, carrying its message +%% (the summary does not repeat it, or the hover shows it twice). +error_in_included_file_is_reported_on_the_include_line(Config) -> + AppDir = ?config(data_dir, Config), + File = filename:join(AppDir, "include_with_error.erl"), + Hrl = filename:join(AppDir, "broken_include.hrl"), + #{errors_warnings := [Item]} = lsp_syntax:validate_parsed_source_file(File), + ?assertMatch(#{type := <<"error">>, info := #{line := 2, character := 1}}, Item), + #{info := #{message := Message}} = Item, + ?assertEqual(<<"1 error in included file broken_include.hrl">>, Message), + ?assertNot(maps:is_key(correlation_data, Item)), + ?assertMatch([#{file := Hrl, line := 1, character := 17, + message := <<"syntax error before: ')'">>}], maps:get(related, Item)). + +%% Two groups - the module's and the header's - used to fall through the +%% single-group patterns in lsp_syntax:lint/2 and drop every diagnostic. +module_and_header_problems_are_both_reported(Config) -> + AppDir = ?config(data_dir, Config), + File = filename:join(AppDir, "include_and_module_problems.erl"), + #{errors_warnings := Items} = lsp_syntax:validate_parsed_source_file(File), + ?assertMatch([#{type := <<"error">>, info := #{line := 2}}, + #{type := <<"warning">>, info := #{line := 6}}], Items). + +%% epp says where the module resumes after a header, which is the line after +%% the -include only when the `.` ending it is followed by a bare newline: +%% with CRLF endings or a comment after it, it is the -include line itself - +%% and the problem used to land on the line above. +header_problem_is_on_the_include_line_with_crlf_endings(Config) -> + assert_header_problem_on_line(Config, "include_with_error_crlf.erl", 3). + +header_problem_is_on_the_include_line_with_a_trailing_comment(Config) -> + assert_header_problem_on_line(Config, "include_with_error_comment.erl", 3). + +assert_header_problem_on_line(Config, FileName, Line) -> + File = filename:join(?config(data_dir, Config), FileName), + #{errors_warnings := [Item]} = lsp_syntax:validate_parsed_source_file(File), + ?assertMatch(#{info := #{line := Line}}, Item). + +%% File names and messages are lists of Unicode code points: turned into +%% binaries byte by byte, a character above 255 (a module under a folder +%% named in Japanese) crashed the whole lint, and one between 128 and 255 (an +%% accented variable name) came out as Latin-1, not the UTF-8 JSON expects. +%% Written at test time so that no non-ASCII name is committed. +non_latin1_path_and_message_are_utf8(Config) -> + Dir = filename:join(?config(priv_dir, Config), [26085, 26412]), % "日本" + File = filename:join(Dir, "unicode_path.erl"), + ok = filelib:ensure_dir(File), + ok = file:write_file(File, <<"-module(unicode_path).\n-export([go/0]).\n\ngo() ->\n Été = 1,\n ok.\n"/utf8>>), + #{errors_warnings := [Item]} = lsp_syntax:validate_parsed_source_file(File), + ?assertEqual(unicode:characters_to_binary(File), maps:get(file, Item)), + ?assertEqual(<<"variable 'Été' is unused"/utf8>>, maps:get(message, maps:get(info, Item))). + bad_record_field_is_reported(Config) -> Item = the_one_diagnostic(Config, "bad_record_field.erl"), ?assertMatch(#{type := <<"error">>}, Item), @@ -122,7 +184,7 @@ the_one_diagnostic(Config, FileName) -> ?assertMatch(#{parse_result := true, errors_warnings := [_ | _]}, Result), #{errors_warnings := [Item | _]} = Result, ?assertEqual(lists:sort([type, file, info, correlation_data]), lists:sort(maps:keys(Item))), - ?assertEqual(list_to_binary(File), maps:get(file, Item)), + ?assertEqual(unicode:characters_to_binary(File), maps:get(file, Item)), ?assertEqual(lists:sort([module, messageBody]), lists:sort(maps:keys(maps:get(correlation_data, Item)))), #{info := Info} = Item, ?assertEqual(lists:sort([line, character, message]), lists:sort(maps:keys(Info))), diff --git a/apps/erlangbridge/test/lsp_syntax_SUITE_data/broken_include.hrl b/apps/erlangbridge/test/lsp_syntax_SUITE_data/broken_include.hrl new file mode 100644 index 00000000..02f8d6a1 --- /dev/null +++ b/apps/erlangbridge/test/lsp_syntax_SUITE_data/broken_include.hrl @@ -0,0 +1 @@ +-record(r, {a = )}). diff --git a/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_and_module_problems.erl b/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_and_module_problems.erl new file mode 100644 index 00000000..42ba490d --- /dev/null +++ b/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_and_module_problems.erl @@ -0,0 +1,7 @@ +-module(include_and_module_problems). +-include("broken_include.hrl"). +-export([go/0]). + +go() -> + X = 1, + ok. diff --git a/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error.erl b/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error.erl new file mode 100644 index 00000000..bc2c00bf --- /dev/null +++ b/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error.erl @@ -0,0 +1,5 @@ +-module(include_with_error). +-include("broken_include.hrl"). +-export([go/0]). + +go() -> ok. diff --git a/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error_comment.erl b/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error_comment.erl new file mode 100644 index 00000000..1065118b --- /dev/null +++ b/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error_comment.erl @@ -0,0 +1,6 @@ +-module(include_with_error_comment). + +-include("broken_include.hrl"). % trailing comment +-export([go/0]). + +go() -> ok. diff --git a/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error_crlf.erl b/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error_crlf.erl new file mode 100644 index 00000000..84b6e63b --- /dev/null +++ b/apps/erlangbridge/test/lsp_syntax_SUITE_data/include_with_error_crlf.erl @@ -0,0 +1,6 @@ +-module(include_with_error_crlf). + +-include("broken_include.hrl"). +-export([go/0]). + +go() -> ok. diff --git a/lib/lsp/lspclientextension.ts b/lib/lsp/lspclientextension.ts index 0fe65a80..bd9f15c2 100644 --- a/lib/lsp/lspclientextension.ts +++ b/lib/lsp/lspclientextension.ts @@ -105,7 +105,7 @@ namespace Configuration { lspcodelens.configurationChanged(); client.sendNotification(DidChangeConfigurationNotification.type, { settings: null }); }); - fileSystemWatcher = workspace.createFileSystemWatcher('**/*.erl'); + fileSystemWatcher = workspace.createFileSystemWatcher('**/*.{erl,hrl}'); fileSystemWatcher.onDidCreate(uri => { client.sendNotification(DidChangeWatchedFilesNotification.type, { changes: [{ uri: uri.toString(), type: FileChangeType.Created }] }); @@ -261,24 +261,6 @@ export function activate(context: ExtensionContext) { documentSelector: [{ scheme: 'file', language: 'erlang' }], middleware: middleware, diagnosticCollectionName: 'Erlang Language Server', - // An open document's diagnostics come from the push channel - // (textDocument/publishDiagnostics); workspace/diagnostic covers the - // project files that are *not* open (lsp_handlers reports open ones as - // empty). Document pull would report an open file a second time, into - // the client's own DiagnosticCollection, and two collections holding - // the same problem means two Problems rows, two hover messages, and - // each quick fix offered twice - lsp_codeaction:code_actions/3 emits - // one action per diagnostic in the request context. - // `filter` is the actual off-switch: with only the trigger flags - // cleared, a server-sent workspace/diagnostic/refresh still makes the - // client re-pull open documents. - diagnosticPullOptions: { - onChange: false, - onSave: false, - onFocus: false, - onTabs: false, - filter: () => true // true = do not pull this document - }, outputChannel: lspOutputChannel } diff --git a/package-lock.json b/package-lock.json index 3efea6c1..56848af7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "erlang", - "version": "1.2.2", + "version": "1.2.4", "lockfileVersion": 2, "requires": true, "packages": { diff --git a/package.json b/package.json index 1b3d332a..c9311df9 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "erlang", "description": "Erlang language extension for Visual Studio Code", - "version": "1.2.2", + "version": "1.2.4", "icon": "images/icon.png", "publisher": "pgourlain", "engines": {