From c284041f043cbbd5bd72421812b10b2ef89a7f5f Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 9 Oct 2023 23:43:32 +0100 Subject: [PATCH] fix(lsp): allow formatting vendor files (#20844) --- cli/lsp/language_server.rs | 21 ++++++--- cli/tests/integration/lsp_tests.rs | 68 ++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 08ebfcceef52aa..9e553b8b9c407a 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1632,23 +1632,30 @@ impl Inner { &self, params: DocumentFormattingParams, ) -> LspResult>> { - let specifier = self + let mut specifier = self .url_map .normalize_url(¶ms.text_document.uri, LspUrlKind::File); + // skip formatting any files ignored by the config file + if !self.fmt_options.files.matches_specifier(&specifier) { + return Ok(None); + } let document = match self.documents.get(&specifier) { Some(doc) if doc.is_open() => doc, _ => return Ok(None), }; - let mark = self.performance.mark("formatting", Some(¶ms)); + // Detect vendored paths. Vendor file URLs will normalize to their remote + // counterparts, but for formatting we want to favour the file URL. + // TODO(nayeemrmn): Implement `Document::file_resource_path()` or similar. + if specifier.scheme() != "file" + && params.text_document.uri.scheme() == "file" + { + specifier = params.text_document.uri.clone(); + } let file_path = specifier_to_file_path(&specifier).map_err(|err| { error!("{}", err); LspError::invalid_request() })?; - - // skip formatting any files ignored by the config file - if !self.fmt_options.files.matches_specifier(&specifier) { - return Ok(None); - } + let mark = self.performance.mark("formatting", Some(¶ms)); // spawn a blocking task to allow doing other work while this is occurring let text_edits = deno_core::unsync::spawn_blocking({ diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 433088fe96caa5..1fb1e0bc277b33 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -688,6 +688,74 @@ fn lsp_import_map_node_specifiers() { client.shutdown(); } +#[test] +fn lsp_format_vendor_path() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); + let temp_dir = context.temp_dir(); + temp_dir.write("deno.json", json!({ "vendor": true }).to_string()); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + client.did_open(json!({ + "textDocument": { + "uri": "file:///a/file.ts", + "languageId": "typescript", + "version": 1, + "text": r#"import "http://localhost:4545/run/002_hello.ts";"#, + }, + })); + client.write_request( + "workspace/executeCommand", + json!({ + "command": "deno.cache", + "arguments": [[], "file:///a/file.ts"], + }), + ); + assert!(temp_dir + .path() + .join("vendor/http_localhost_4545/run/002_hello.ts") + .exists()); + client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("vendor/http_localhost_4545/run/002_hello.ts").unwrap(), + "languageId": "typescript", + "version": 1, + "text": r#"console.log("Hello World");"#, + }, + })); + let res = client.write_request( + "textDocument/formatting", + json!({ + "textDocument": { + "uri": temp_dir.uri().join("vendor/http_localhost_4545/run/002_hello.ts").unwrap(), + }, + "options": { + "tabSize": 2, + "insertSpaces": true, + } + }), + ); + assert_eq!( + res, + json!([{ + "range": { + "start": { + "line": 0, + "character": 27, + }, + "end": { + "line": 0, + "character": 27, + }, + }, + "newText": "\n", + }]), + ); + client.shutdown(); +} + // Regression test for https://github.com/denoland/deno/issues/19802. // Disable the `workspace/configuration` capability. Ensure the LSP falls back // to using `enablePaths` from the `InitializationOptions`.