From 8d5aea4826d4cdf59ca1a16acc0bf3b018660c93 Mon Sep 17 00:00:00 2001 From: Florian Loitsch Date: Mon, 23 Jan 2023 02:19:20 -0800 Subject: [PATCH] Remove RPC filesystem. (#1358) This was a protocol we used with Toit v1. The 'toit' executable would proxy LSP calls to the LSP server and take over reading of the file system. This was necessary, as the Toit LSP server wasn't yet able to read files on Windows. --- tests/lsp/fail.cmake | 1 - tests/lsp/lsp_client.toit | 4 - tests/lsp/lsp_filesystem_compiler_test.toit | 186 -------------------- tools/lsp/server/client.toit | 12 +- tools/lsp/server/file_server.toit | 102 ----------- tools/lsp/server/replay.toit | 2 +- tools/lsp/server/server.toit | 58 +----- 7 files changed, 5 insertions(+), 360 deletions(-) delete mode 100644 tests/lsp/lsp_filesystem_compiler_test.toit diff --git a/tests/lsp/fail.cmake b/tests/lsp/fail.cmake index cbf2dbb8a..91cf1be71 100644 --- a/tests/lsp/fail.cmake +++ b/tests/lsp/fail.cmake @@ -86,7 +86,6 @@ if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Windows" OR "${CMAKE_SYSTEM_NAME}" STREQUAL tests/lsp/export_summary_compiler_test.toit tests/lsp/incomplete_compiler_test.toit tests/lsp/invalid_symbol_compiler_test.toit - tests/lsp/lsp_filesystem_compiler_test.toit tests/lsp/lsp_ubjson_rpc_compiler_test.toit tests/lsp/null_char_compiler_test.toit tests/lsp/open_many_compiler_test.toit diff --git a/tests/lsp/lsp_client.toit b/tests/lsp/lsp_client.toit index 12bee736d..979b305cb 100644 --- a/tests/lsp/lsp_client.toit +++ b/tests/lsp/lsp_client.toit @@ -25,7 +25,6 @@ run_client_test [test_fun] --supports_config=true --needs_server_args=(not supports_config) - --use_rpc_filesystem=false --use_mock=false --exit=true --spawn_process=true @@ -50,7 +49,6 @@ run_client_test --toitlsp_exe=toitlsp_exe --supports_config=supports_config --needs_server_args=needs_server_args - --use_rpc_filesystem=use_rpc_filesystem --spawn_process=spawn_process --pre_initialize=: | client args | client.configuration["reproDir"] = repro_dir @@ -64,7 +62,6 @@ run_client_test [test_fun] --supports_config=true --needs_server_args=(not supports_config) - --use_rpc_filesystem=false --use_mock=false --exit=true --spawn_process=true @@ -74,7 +71,6 @@ run_client_test test_fun --supports_config=supports_config --needs_server_args=needs_server_args - --use_rpc_filesystem=use_rpc_filesystem --pre_initialize=: null --use_mock=use_mock --spawn_process=spawn_process diff --git a/tests/lsp/lsp_filesystem_compiler_test.toit b/tests/lsp/lsp_filesystem_compiler_test.toit deleted file mode 100644 index df0bd2c93..000000000 --- a/tests/lsp/lsp_filesystem_compiler_test.toit +++ /dev/null @@ -1,186 +0,0 @@ -// Copyright (C) 2020 Toitware ApS. -// Use of this source code is governed by a Zero-Clause BSD license that can -// be found in the tests/LICENSE file. - -import expect show * -import .lsp_client show LspClient run_client_test - -import ...tools.lsp.server.uri_path_translator -import ...tools.lsp.server.documents -import ...tools.lsp.server.file_server - -main args: - run_client_test --use_rpc_filesystem args: test it - // Also check, that the rpc filesystem works without process. - run_client_test --use_rpc_filesystem --no-spawn_process args: test it - - run_client_test --use_rpc_filesystem args: test2 it - // Also check, that the rpc filesystem works without process. - run_client_test --use_rpc_filesystem --no-spawn_process args: test2 it - exit 0 - -PATH_PREFIX ::= "/non_existent/some path with spaces and :/toit_test" - -PATH1 ::= "$PATH_PREFIX/p1.toit" -PATH2 ::= "$PATH_PREFIX/p2.toit" -PATH3 ::= "$PATH_PREFIX/p3.toit" -PATH4 ::= "$PATH_PREFIX/p4.toit" - -PATH1_CONTENT ::= """ - import .p2 - - from_p1: - from_p2 1 - """ - -PATH2_CONTENT ::= """ - from_p2 x: return x - """ - -PATH3_CONTENT ::= """ - from_p3: unresolved - """ - -PATH4_CONTENT ::= """ - import RETURN_NON_EXISTENT - main: - """ - -SPECIAL_FILES1 ::= { - PATH1: PATH1_CONTENT, - PATH2: PATH2_CONTENT, - PATH3: PATH3_CONTENT, -} - -PACKAGE_CACHE ::= "$PATH_PREFIX/package_cache" -PATH5 ::= "$PATH_PREFIX/p5.toit" -PATH6A ::= "$PACKAGE_CACHE/pkg_foo" -PATH6B ::= "$PACKAGE_CACHE/pkg_foo/1.0.0" -PATH6C ::= "$PACKAGE_CACHE/pkg_foo/1.0.0/src" -PATH6D ::= "$PACKAGE_CACHE/pkg_foo/1.0.0/src/foo.toit" -PATH7 ::= "$PATH_PREFIX/package.lock" - -PATH5_CONTENT ::= """ - import pkg.foo as foo - main: - foo.say_hi - """ - -PATH6_CONTENT ::= """ - say_hi: return "hello" - """ - -PATH7_CONTENT ::= """ -prefixes: - pkg: pkg-foo - -packages: - pkg-foo: - url: pkg_foo - version: 1.0.0 -""" - -SPECIAL_FILES2 ::= { - PATH5: PATH5_CONTENT, - PATH6A: true, - PATH6B: true, - PATH6C: true, - PATH6D: PATH6_CONTENT, - PATH7: PATH7_CONTENT, -} - - -create_file_response local/FilesystemLocal special_files/Map param: - path := param["path"] - file := null - if special_files.contains path: - entry := special_files[path] - if entry is bool: - file = File path true true true null - else: - file = File path true true false entry.to_byte_array - else if path.contains "RETURN_NON_EXISTENT": - file = File path false false false null - else if path == PATH_PREFIX or path == "$PATH_PREFIX/": - file = File path true false true null - else: - file = local.create_file_entry path - content_string := file.content == null - ? null - : file.content.to_string - return { - "path": file.path, - "exists": file.exists, - "is_regular": file.is_regular, - "is_directory": file.is_directory, - "content": content_string, - } - -basename path -> string: - return (path.split "/").last - -create_list_response local/FilesystemLocal special_files/Map param: - path := param["path"] - if path == PATH_PREFIX or path == "$PATH_PREFIX/": - return special_files.keys.map: basename it - return local.directory_entries path - -test client/LspClient: - translator := UriPathTranslator - documents := Documents translator - sdk_path := sdk_path_from_compiler client.toitc - local := FilesystemLocal sdk_path - client.install_handler "toit/sdk_path":: local.sdk_path - client.install_handler "toit/file":: create_file_response local SPECIAL_FILES1 it - client.install_handler "toit/list":: create_list_response local SPECIAL_FILES1 it - - path := "$PATH_PREFIX/entry.toit" - client.send_did_open --path=path --text="import .p1" - diagnostics := client.diagnostics_for --path=path - expect_equals 0 diagnostics.size - diagnostics = client.diagnostics_for --path=PATH1 - expect_equals 0 diagnostics.size - diagnostics = client.diagnostics_for --path=PATH2 - expect_equals 0 diagnostics.size - - client.send_did_change --path=path "import .p3" - diagnostics = client.diagnostics_for --path=path - expect_equals 0 diagnostics.size - diagnostics = client.diagnostics_for --path=PATH3 - expect_equals 1 diagnostics.size - - client.send_did_change --path=path "import " - responses := client.send_completion_request --path=path 0 7 - expect responses is List - expect (responses.any: it["label"] == "core") - - client.send_did_change --path=path "import core." - responses = client.send_completion_request --path=path 0 12 - expect responses is List - expect (responses.any: it["label"] == "collections") - - client.send_did_change --path=path "import ." - responses = client.send_completion_request --path=path 0 8 - expect responses is List - expect (responses.any: it["label"] == "p1") - expect (responses.any: it["label"] == "p2") - expect (responses.any: it["label"] == "p3") - - path = PATH4 - client.send_did_open --path=path --text=PATH4_CONTENT - diagnostics = client.diagnostics_for --path=path - expect_equals 1 diagnostics.size - -test2 client/LspClient: - path := PATH5 - sdk_path := sdk_path_from_compiler client.toitc - local := FilesystemLocal sdk_path - client.install_handler "toit/sdk_path":: local.sdk_path - client.install_handler "toit/file":: create_file_response local SPECIAL_FILES2 it - client.install_handler "toit/list":: create_list_response local SPECIAL_FILES2 it - client.install_handler "toit/package_cache_paths":: [PACKAGE_CACHE] - - client.send_did_open --path=path --text=PATH5_CONTENT - diagnostics := client.diagnostics_for --path=path - diagnostics.do: print it - expect_equals 0 diagnostics.size diff --git a/tools/lsp/server/client.toit b/tools/lsp/server/client.toit index f21607d2d..22c837b3a 100644 --- a/tools/lsp/server/client.toit +++ b/tools/lsp/server/client.toit @@ -32,7 +32,6 @@ with_lsp_client [block] --toitlsp_exe/string?=null --supports_config=true --needs_server_args=(not supports_config) - --use_rpc_filesystem=false --spawn_process=false: with_lsp_client block --toitc=toitc @@ -40,7 +39,6 @@ with_lsp_client [block] --compiler_exe=compiler_exe --supports_config=supports_config --needs_server_args=needs_server_args - --use_rpc_filesystem=use_rpc_filesystem --spawn_process=spawn_process --pre_initialize=(:null) @@ -51,7 +49,6 @@ with_lsp_client [block] --compiler_exe/string = toitc --supports_config=true --needs_server_args=(not supports_config) - --use_rpc_filesystem=false --spawn_process=true [--pre_initialize]: server_args := [lsp_server] @@ -61,7 +58,6 @@ with_lsp_client [block] if toitlsp_exe: server_cmd = toitlsp_exe server_args = ["--toitc", compiler_exe, "--sdk", (sdk_path_from_compiler toitc)] - use_rpc_filesystem = false else: server_cmd = toitc @@ -69,7 +65,6 @@ with_lsp_client [block] server_cmd server_args --supports_config=supports_config - --use_rpc_filesystem=use_rpc_filesystem --compiler_exe=compiler_exe --spawn_process=spawn_process client.initialize pre_initialize @@ -122,9 +117,7 @@ class LspClient: "timeoutMs": 10_000, // Increase the timeout to avoid flaky tests. } - static start_server_ cmd args compiler_exe --spawn_process/bool --use_rpc_filesystem/bool -> List: - if use_rpc_filesystem: - args += ["--rpc-filesystem"] + static start_server_ cmd args compiler_exe --spawn_process/bool -> List: print "starting the server $cmd with $args" if spawn_process: pipes := pipe.fork @@ -143,7 +136,6 @@ class LspClient: server_to := FakePipe server_rpc_connection := RpcConnection (BufferedReader server_to) server_from server := LspServer server_rpc_connection compiler_exe UriPathTranslator - --use_rpc_filesystem=use_rpc_filesystem task:: server.run return [server_to, server_from, server, null] @@ -153,12 +145,10 @@ class LspClient: server_cmd/string server_args/List --supports_config/bool - --use_rpc_filesystem/bool --compiler_exe=server_cmd --spawn_process: start_result := start_server_ server_cmd server_args compiler_exe --spawn_process=spawn_process - --use_rpc_filesystem=use_rpc_filesystem server_to := start_result[0] server_from := start_result[1] server := start_result[2] diff --git a/tools/lsp/server/file_server.toit b/tools/lsp/server/file_server.toit index 41dbe7d59..37cc8ffd4 100644 --- a/tools/lsp/server/file_server.toit +++ b/tools/lsp/server/file_server.toit @@ -253,105 +253,3 @@ class FilesystemLocal extends FilesystemBase: entries.add entry stream.close return entries - - -class FilesystemLspRpc implements Filesystem: - rpc_connection_ / RpcConnection ::= ? - - constructor .rpc_connection_: - - sdk_path -> string: - // We expect a response of the form: - // `{ "id": , "result": }` - // See rpc.toit for the underlying format. - return rpc_connection_.request "toit/sdk_path" {:} - - package_cache_paths -> List: - // We expect a response of the form: - // `{ "id": , "result": }` - // See rpc.toit for the underlying format. - return rpc_connection_.request "toit/package_cache_paths" {:} - - create_file_entry path/string -> File: - // See $Filesystem.create_file_entry for a description on how the - // response should be computed. - // We expect a response of the form: - // ``` - // { "id": , - // "result": { - // "path": // For sanity checking. - // "exists": // Whether the path exists. - // "is_regular": // Whether this is a regular file. - // "is_directory": // Whether this is a directory. - // "content": // May be null. - // } - // } - // ``` - // See rpc.toit for the underlying format. - verbose: "Requesting $path through RPC protocol" - response := rpc_connection_.request "toit/file" {"path": path} - verbose: "Got answer for $path" - assert: response["path"] == path - // Content is string for json, ByteArray for ubjson. - content := response.get "content" - if content is string: content = content.to_byte_array - exists := ? - // TODO(florian): remove 'realpath' support. - if response.contains "exists": - exists = response["exists"] - else: - exists = response.get "realpath" != null - return File - response["path"] - exists - response["is_regular"] - response["is_directory"] - content - - directory_entries path/string -> List: - return rpc_connection_.request "toit/list" {"path": path} - - -class FilesystemHybrid implements Filesystem: - /// The placeholder (if any) for the SDK path. - /// If null, then the client's SDK library is used. - /// Otherwise the placeholder is replaced with the compiler's SDK library path. - rpc_sdk_path_placeholder_ /string ::= ? - sdk_path_ /string ::= ? - sdk_fs_ /Filesystem ::= ? - rpc_fs_ /Filesystem ::= ? - - constructor .rpc_sdk_path_placeholder_ compiler_path rpc_connection: - sdk_path_ = (sdk_path_from_compiler compiler_path) - rpc_fs_ = FilesystemLspRpc rpc_connection - sdk_fs_ = FilesystemLocal sdk_path_ - - is_rpc_sdk_path_ path/string -> bool: - return path.starts_with rpc_sdk_path_placeholder_ - - sdk_path -> string: - return rpc_sdk_path_placeholder_ - - package_cache_paths -> List: - return rpc_fs_.package_cache_paths - - convert_sdk_path_ path/string -> string: - return path.replace rpc_sdk_path_placeholder_ sdk_path_ - - create_file_entry path/string -> File: - if not is_rpc_sdk_path_ path: - return rpc_fs_.create_file_entry path - - verbose: "SDK path handled locally: $path" - // We always send a request to the rpc-filesystem, even if the path will be served from the - // sdk-filesystem. This makes it possible for the bridge to request the file from the server. - // However, we don't need to wait for the response and can serve the response faster. - task:: catch --trace: rpc_fs_.create_file_entry path - sdk_file := sdk_fs_.create_file_entry (convert_sdk_path_ path) - return File sdk_file.path sdk_file.exists sdk_file.is_regular sdk_file.is_directory sdk_file.content - - directory_entries path/string -> List: - if is_rpc_sdk_path_ path: - return sdk_fs_.directory_entries (convert_sdk_path_ path) - else: - return rpc_fs_.directory_entries path diff --git a/tools/lsp/server/replay.toit b/tools/lsp/server/replay.toit index ad382cff9..cb670e42a 100644 --- a/tools/lsp/server/replay.toit +++ b/tools/lsp/server/replay.toit @@ -83,7 +83,7 @@ main args: server_from = FakePipe server_to = FakePipe server_rpc_connection := RpcConnection (BufferedReader server_to) server_from - server := LspServer --no-use_rpc_filesystem server_rpc_connection null UriPathTranslator + server := LspServer server_rpc_connection null UriPathTranslator task:: catch --trace: server.run debug_file := parsed["debug-file"] diff --git a/tools/lsp/server/server.toit b/tools/lsp/server/server.toit index fe4d72f24..68cda697b 100644 --- a/tools/lsp/server/server.toit +++ b/tools/lsp/server/server.toit @@ -72,11 +72,6 @@ class LspServer: connection_ /RpcConnection ::= ? translator_ /UriPathTranslator ::= ? toit_path_override_ /string? ::= ? - uses_rpc_filesystem_ /bool ::= ? - /// The placeholder for the compiler's SDK path. - /// When null uses the client's SDK libraries. - /// Otherwise the placeholder is replaced with the compiler's SDK library path. - rpc_sdk_path_placeholder_ /string? ::= ? /// The root uri of the workspace. /// Rarely needed, as the server generally works with absolute paths. /// It's mainly used to find package.lock files. @@ -101,12 +96,8 @@ class LspServer: constructor .connection_ .toit_path_override_ - .translator_ - --use_rpc_filesystem/bool - --rpc_sdk_path_placeholder/string?=null: + .translator_: documents_ = Documents translator_ - uses_rpc_filesystem_ = use_rpc_filesystem - rpc_sdk_path_placeholder_ = rpc_sdk_path_placeholder run -> none: while true: @@ -150,7 +141,6 @@ class LspServer: "toit/reset_crash_rate_limit": (:: reset_crash_rate_limit), "toit/settings": (:: settings_.map_), "toit/didOpenMany": (:: did_open_many it), - "toit/fetchSdkFile": (:: fetch_sdk_file (FetchSdkFileParams it)), "toit/archive": (:: archive (ArchiveParams it)), "toit/snapshot_bundle": (:: snapshot_bundle (SnapshotBundleParams it)) } @@ -232,29 +222,6 @@ class LspServer: documents_.did_open --uri=it content content_revision analyze uris - fetch_sdk_file params/FetchSdkFileParams -> Map: - path := params.path - if not uses_rpc_filesystem_ or not rpc_sdk_path_placeholder_: - throw "fetch_sdk_file only permitted when running with rpc sdk path" - - if not path.starts_with rpc_sdk_path_placeholder_: - throw "fetch_sdk_file called with non sdk path: '$path'" - - sdk_path := (sdk_path_from_compiler compiler_path_) - local_path := path.replace rpc_sdk_path_placeholder_ sdk_path - - protocol := FileServerProtocol.local compiler_path_ sdk_path_ documents_ - file := protocol.get_file local_path - content/any := file.content - if content and connection_.uses_json: content = content.to_string - return { - "path": file.path, - "exists": file.exists, - "is_regular": file.is_regular, - "is_directory": file.is_directory, - "content": content, - } - archive params/ArchiveParams -> string: non_canonicalized_uris := params.uris or [params.uri] // If the request doesn't specify whether it wants the sdk we include it. @@ -545,16 +512,7 @@ class LspServer: should_write_repro := settings_.get "shouldWriteReproOnCrash" --if_absent=:false - protocol / FileServerProtocol := ? - if uses_rpc_filesystem_: - filesystem/Filesystem := ? - if rpc_sdk_path_placeholder_: - filesystem = FilesystemHybrid rpc_sdk_path_placeholder_ compiler_path connection_ - else: - filesystem = FilesystemLspRpc connection_ - protocol = FileServerProtocol documents_ filesystem - else: - protocol = FileServerProtocol.local compiler_path sdk_path documents_ + protocol := FileServerProtocol.local compiler_path sdk_path documents_ compiler := null // Let the 'compiler' local be visible in the lambda expression below. compiler = Compiler compiler_path translator_ timeout_ms @@ -594,10 +552,7 @@ main args -> none: cli.OptionString "toit-path-override", ] --options=[ - cli.Flag "rpc-filesystem" --default=false, - cli.OptionString "rpc-sdk-path", cli.OptionString "home-path", - cli.OptionString "uri_path_mapping", cli.Flag "verbose" --default=false, ] --run=:: parsed = it @@ -606,14 +561,9 @@ main args -> none: toit_path_override := parsed["toit-path-override"] - use_rpc_filesystem := parsed["rpc-filesystem"] - rpc_sdk_path_placeholder := parsed["rpc-sdk-path"] - json_mapping := parsed["uri_path_mapping"] is_verbose = parsed["verbose"] - uri_path_mapping := json_mapping == null or use_rpc_filesystem ? null : json.parse json_mapping - - uri_path_translator := UriPathTranslator uri_path_mapping + uri_path_translator := UriPathTranslator in_pipe := pipe.stdin out_pipe := pipe.stdout @@ -636,6 +586,4 @@ main args -> none: rpc_connection := RpcConnection reader writer server := LspServer rpc_connection toit_path_override uri_path_translator - --use_rpc_filesystem=use_rpc_filesystem - --rpc_sdk_path_placeholder=rpc_sdk_path_placeholder server.run