From d69a5fbe1a4f909b7eba0eac81dd111fb7229232 Mon Sep 17 00:00:00 2001 From: Satya Rohith Date: Tue, 18 May 2021 12:05:46 +0530 Subject: [PATCH] feat(lsp): support formatting json and markdown files (#10180) Resolves #9447 Resolves #9415 --- cli/lsp/diagnostics.rs | 22 ++++-- cli/lsp/language_server.rs | 30 +++---- cli/tests/integration_tests_lsp.rs | 123 +++++++++++++++++++++++++++++ cli/tools/fmt.rs | 60 ++++++-------- 4 files changed, 180 insertions(+), 55 deletions(-) diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index a5b8df879a..7ddb3ff7b4 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -218,6 +218,17 @@ impl<'a> From<&'a diagnostics::Position> for lsp::Position { } } +/// Check if diagnostics can be generated for the provided media type. +pub fn is_diagnosable(media_type: MediaType) -> bool { + matches!( + media_type, + MediaType::TypeScript + | MediaType::JavaScript + | MediaType::Tsx + | MediaType::Jsx + ) +} + fn get_diagnostic_message(diagnostic: &diagnostics::Diagnostic) -> String { if let Some(message) = diagnostic.message_text.clone() { message @@ -312,8 +323,8 @@ async fn generate_lint_diagnostics( .lock() .await .get_version(specifier, &DiagnosticSource::DenoLint); - if version != current_version { - let media_type = MediaType::from(specifier); + let media_type = MediaType::from(specifier); + if version != current_version && is_diagnosable(media_type) { if let Ok(Some(source_code)) = documents.content(specifier) { if let Ok(references) = analysis::get_lint_references( specifier, @@ -354,10 +365,11 @@ async fn generate_ts_diagnostics( let version = snapshot.documents.version(s); let current_version = collection.get_version(s, &DiagnosticSource::TypeScript); - if version == current_version { - None - } else { + let media_type = MediaType::from(s); + if version != current_version && is_diagnosable(media_type) { Some(s.clone()) + } else { + None } }) .collect() diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 9f5a7c84ad..fcf28dbf70 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -10,7 +10,6 @@ use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::serde_json::Value; use deno_core::ModuleSpecifier; -use dprint_plugin_typescript as dprint; use log::error; use log::info; use log::warn; @@ -30,13 +29,6 @@ use std::sync::atomic::Ordering; use std::sync::Arc; use tokio::fs; -use crate::config_file::ConfigFile; -use crate::config_file::TsConfig; -use crate::deno_dir; -use crate::import_map::ImportMap; -use crate::logger; -use crate::media_type::MediaType; - use super::analysis; use super::analysis::ts_changes_to_edit; use super::analysis::CodeActionCollection; @@ -62,6 +54,15 @@ use super::tsc::AssetDocument; use super::tsc::Assets; use super::tsc::TsServer; use super::urls; +use crate::config_file::ConfigFile; +use crate::config_file::TsConfig; +use crate::deno_dir; +use crate::import_map::ImportMap; +use crate::logger; +use crate::lsp::diagnostics::is_diagnosable; +use crate::media_type::MediaType; +use crate::tools::fmt::format_file; +use crate::tools::fmt::get_typescript_config; pub const REGISTRIES_PATH: &str = "registries"; const SOURCES_PATH: &str = "deps"; @@ -785,6 +786,11 @@ impl Inner { if !self.config.specifier_enabled(&specifier) { return Ok(None); } + let media_type = MediaType::from(&specifier); + if !is_diagnosable(media_type) { + return Ok(None); + } + let mark = self.performance.mark("document_symbol", Some(¶ms)); let line_index = @@ -845,12 +851,8 @@ impl Inner { // TODO(lucacasonato): handle error properly let text_edits = tokio::task::spawn_blocking(move || { - let config = dprint::configuration::ConfigurationBuilder::new() - .deno() - .build(); - // TODO(@kitsonk) this could be handled better in `cli/tools/fmt.rs` in the - // future. - match dprint::format_text(&file_path, &file_text, &config) { + let config = get_typescript_config(); + match format_file(&file_path, &file_text, config) { Ok(new_text) => { Some(text::get_edits(&file_text, &new_text, line_index)) } diff --git a/cli/tests/integration_tests_lsp.rs b/cli/tests/integration_tests_lsp.rs index 6b0f6b7920..fa88ad17d4 100644 --- a/cli/tests/integration_tests_lsp.rs +++ b/cli/tests/integration_tests_lsp.rs @@ -1659,3 +1659,126 @@ fn lsp_performance() { } shutdown(&mut client); } + +#[test] +fn lsp_format_json() { + let mut client = init("initialize_params.json"); + client + .write_notification( + "textDocument/didOpen", + json!({ + "textDocument": { + "uri": "file:///a/file.json", + "languageId": "json", + "version": 1, + "text": "{\"key\":\"value\"}" + } + }), + ) + .unwrap(); + + let (maybe_res, maybe_err) = client + .write_request::<_, _, Value>( + "textDocument/formatting", + json!({ + "textDocument": { + "uri": "file:///a/file.json" + }, + "options": { + "tabSize": 2, + "insertSpaces": true + } + }), + ) + .unwrap(); + + assert!(maybe_err.is_none()); + assert_eq!( + maybe_res, + Some(json!([ + { + "range": { + "start": { + "line": 0, + "character": 1 + }, + "end": { + "line": 0, + "character": 1 + } + }, + "newText": " " + }, + { + "range": { + "start": { "line": 0, "character": 7 }, + "end": { "line": 0, "character": 7 } + }, + "newText": " " + }, + { + "range": { + "start": { "line": 0, "character": 14 }, + "end": { "line": 0, "character": 15 } + }, + "newText": " }\n" + } + ])) + ); + shutdown(&mut client); +} + +#[test] +fn lsp_format_markdown() { + let mut client = init("initialize_params.json"); + client + .write_notification( + "textDocument/didOpen", + json!({ + "textDocument": { + "uri": "file:///a/file.md", + "languageId": "markdown", + "version": 1, + "text": "# Hello World" + } + }), + ) + .unwrap(); + + let (maybe_res, maybe_err) = client + .write_request::<_, _, Value>( + "textDocument/formatting", + json!({ + "textDocument": { + "uri": "file:///a/file.md" + }, + "options": { + "tabSize": 2, + "insertSpaces": true + } + }), + ) + .unwrap(); + + assert!(maybe_err.is_none()); + assert_eq!( + maybe_res, + Some(json!([ + { + "range": { + "start": { "line": 0, "character": 1 }, + "end": { "line": 0, "character": 3 } + }, + "newText": "" + }, + { + "range": { + "start": { "line": 0, "character": 15 }, + "end": { "line": 0, "character": 15 } + }, + "newText": "\n" + } + ])) + ); + shutdown(&mut client); +} diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index 9a16afecab..869403f071 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -152,6 +152,22 @@ fn format_json(file_text: &str) -> Result { dprint_plugin_json::format_text(&file_text, &json_config) } +/// Formats a single TS, TSX, JS, JSX, JSONC, JSON, or MD file. +pub fn format_file( + file_path: &Path, + file_text: &str, + config: dprint_plugin_typescript::configuration::Configuration, +) -> Result { + let ext = get_extension(file_path).unwrap_or_else(String::new); + if ext == "md" { + format_markdown(&file_text, config) + } else if matches!(ext.as_str(), "json" | "jsonc") { + format_json(&file_text) + } else { + dprint_plugin_typescript::format_text(&file_path, &file_text, &config) + } +} + async fn check_source_files( config: dprint_plugin_typescript::configuration::Configuration, paths: Vec, @@ -168,15 +184,8 @@ async fn check_source_files( move |file_path| { checked_files_count.fetch_add(1, Ordering::Relaxed); let file_text = read_file_contents(&file_path)?.text; - let ext = get_extension(&file_path).unwrap_or_else(String::new); - let r = if ext == "md" { - format_markdown(&file_text, config.clone()) - } else if matches!(ext.as_str(), "json" | "jsonc") { - format_json(&file_text) - } else { - dprint_plugin_typescript::format_text(&file_path, &file_text, &config) - }; - match r { + + match format_file(&file_path, &file_text, config) { Ok(formatted_text) => { if formatted_text != file_text { not_formatted_files_count.fetch_add(1, Ordering::Relaxed); @@ -229,19 +238,8 @@ async fn format_source_files( move |file_path| { checked_files_count.fetch_add(1, Ordering::Relaxed); let file_contents = read_file_contents(&file_path)?; - let ext = get_extension(&file_path).unwrap_or_else(String::new); - let r = if ext == "md" { - format_markdown(&file_contents.text, config.clone()) - } else if matches!(ext.as_str(), "json" | "jsonc") { - format_json(&file_contents.text) - } else { - dprint_plugin_typescript::format_text( - &file_path, - &file_contents.text, - &config, - ) - }; - match r { + + match format_file(&file_path, &file_contents.text, config) { Ok(formatted_text) => { if formatted_text != file_contents.text { write_file_contents( @@ -293,19 +291,9 @@ pub fn format_stdin(check: bool, ext: String) -> Result<(), AnyError> { return Err(generic_error("Failed to read from stdin")); } let config = get_typescript_config(); - let r = if ext.as_str() == "md" { - format_markdown(&source, config) - } else if matches!(ext.as_str(), "json" | "jsonc") { - format_json(&source) - } else { - // dprint will fallback to jsx parsing if parsing this as a .ts file doesn't work - dprint_plugin_typescript::format_text( - &PathBuf::from("_stdin.ts"), - &source, - &config, - ) - }; - match r { + let file_path = PathBuf::from(format!("_stdin.{}", ext)); + + match format_file(&file_path, &source, config) { Ok(formatted_text) => { if check { if formatted_text != source { @@ -330,7 +318,7 @@ fn files_str(len: usize) -> &'static str { } } -fn get_typescript_config( +pub fn get_typescript_config( ) -> dprint_plugin_typescript::configuration::Configuration { dprint_plugin_typescript::configuration::ConfigurationBuilder::new() .deno()