From cc01f0aeff4232afe5d5967ce09233c2ca505e03 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 18 Nov 2024 12:27:57 +0100 Subject: [PATCH 01/18] Support `RangeFormatting` LSP request --- crates/lsp/src/handlers.rs | 7 ------- crates/lsp/src/handlers_format.rs | 29 ++++++++++++++++++++++++++++- crates/lsp/src/handlers_state.rs | 1 + crates/lsp/src/main_loop.rs | 3 +++ crates/lsp/src/to_proto.rs | 10 ++++++++++ crates/lsp/src/tower_lsp.rs | 13 +++++++++++++ 6 files changed, 55 insertions(+), 8 deletions(-) diff --git a/crates/lsp/src/handlers.rs b/crates/lsp/src/handlers.rs index fff87adf..53cb5ebc 100644 --- a/crates/lsp/src/handlers.rs +++ b/crates/lsp/src/handlers.rs @@ -46,13 +46,6 @@ pub(crate) async fn handle_initialized( regs.append(&mut config_diagnostics_regs); } - // TODO! Abstract this in a method - regs.push(lsp_types::Registration { - id: String::from("air_formatting"), - method: String::from("textDocument/formatting"), - register_options: None, - }); - client .register_capability(regs) .instrument(span.exit()) diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index ada97f36..8006ad7b 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -10,7 +10,7 @@ use biome_formatter::{IndentStyle, LineWidth}; use tower_lsp::lsp_types; use crate::state::WorldState; -use crate::to_proto; +use crate::{from_proto, to_proto}; #[tracing::instrument(level = "info", skip_all)] pub(crate) fn document_formatting( @@ -41,6 +41,33 @@ pub(crate) fn document_formatting( Ok(Some(edits)) } +#[tracing::instrument(level = "info", skip_all)] +pub(crate) fn document_range_formatting( + params: lsp_types::DocumentRangeFormattingParams, + state: &WorldState, +) -> anyhow::Result<Option<Vec<lsp_types::TextEdit>>> { + let doc = state.get_document(¶ms.text_document.uri)?; + + let line_width = LineWidth::try_from(80).map_err(|err| anyhow::anyhow!("{err}"))?; + let range = + from_proto::text_range(&doc.line_index.index, params.range, doc.line_index.encoding)?; + + // TODO: Handle FormattingOptions + let options = RFormatOptions::default() + .with_indent_style(IndentStyle::Space) + .with_line_width(line_width); + + let output = biome_formatter::format_range( + &doc.parse.syntax(), + range, + air_r_formatter::RFormatLanguage::new(options), + )? + .into_code(); + + let edits = to_proto::replace_all_edit(&doc.line_index, &doc.contents, &output)?; + Ok(Some(edits)) +} + #[cfg(test)] mod tests { use crate::{ diff --git a/crates/lsp/src/handlers_state.rs b/crates/lsp/src/handlers_state.rs index 211eb1b2..a570cf31 100644 --- a/crates/lsp/src/handlers_state.rs +++ b/crates/lsp/src/handlers_state.rs @@ -133,6 +133,7 @@ pub(crate) fn initialize( file_operations: None, }), document_formatting_provider: Some(OneOf::Left(true)), + document_range_formatting_provider: Some(OneOf::Left(true)), ..ServerCapabilities::default() }, }) diff --git a/crates/lsp/src/main_loop.rs b/crates/lsp/src/main_loop.rs index 4f07eb21..780a635b 100644 --- a/crates/lsp/src/main_loop.rs +++ b/crates/lsp/src/main_loop.rs @@ -338,6 +338,9 @@ impl GlobalState { LspRequest::DocumentFormatting(params) => { respond(tx, handlers_format::document_formatting(params, &self.world), LspResponse::DocumentFormatting)?; }, + LspRequest::DocumentRangeFormatting(params) => { + respond(tx, handlers_format::document_range_formatting(params, &self.world), LspResponse::DocumentRangeFormatting)?; + }, LspRequest::AirViewFile(params) => { respond(tx, handlers_ext::view_file(params, &self.world), LspResponse::AirViewFile)?; }, diff --git a/crates/lsp/src/to_proto.rs b/crates/lsp/src/to_proto.rs index e0e9f638..c7bbc771 100644 --- a/crates/lsp/src/to_proto.rs +++ b/crates/lsp/src/to_proto.rs @@ -10,6 +10,7 @@ pub(crate) use rust_analyzer::to_proto::text_edit_vec; use crate::rust_analyzer::{self, line_index::LineIndex, text_edit::TextEdit}; +use biome_text_size::TextRange; use tower_lsp::lsp_types; pub(crate) fn doc_edit_vec( @@ -28,6 +29,15 @@ pub(crate) fn doc_edit_vec( .collect()) } +pub(crate) fn replace_range_edit( + line_index: &LineIndex, + range: TextRange, + replace_with: String, +) -> anyhow::Result<Vec<lsp_types::TextEdit>> { + let edit = TextEdit::replace(range, replace_with); + text_edit_vec(line_index, edit) +} + pub(crate) fn replace_all_edit( line_index: &LineIndex, text: &str, diff --git a/crates/lsp/src/tower_lsp.rs b/crates/lsp/src/tower_lsp.rs index 5e824475..ba114988 100644 --- a/crates/lsp/src/tower_lsp.rs +++ b/crates/lsp/src/tower_lsp.rs @@ -62,6 +62,7 @@ pub(crate) enum LspRequest { Initialize(InitializeParams), DocumentFormatting(DocumentFormattingParams), Shutdown, + DocumentRangeFormatting(DocumentRangeFormattingParams), AirViewFile(ViewFileParams), } @@ -70,6 +71,7 @@ pub(crate) enum LspRequest { pub(crate) enum LspResponse { Initialize(InitializeResult), DocumentFormatting(Option<Vec<TextEdit>>), + DocumentRangeFormatting(Option<Vec<TextEdit>>), Shutdown(()), AirViewFile(String), } @@ -250,6 +252,17 @@ impl LanguageServer for Backend { LspResponse::DocumentFormatting ) } + + async fn range_formatting( + &self, + params: DocumentRangeFormattingParams, + ) -> Result<Option<Vec<TextEdit>>> { + cast_response!( + self.request(LspRequest::DocumentRangeFormatting(params)) + .await, + LspResponse::DocumentRangeFormatting + ) + } } pub async fn start_lsp<I, O>(read: I, write: O) From 77022781529763640e55b709e067eac7d39b30cd Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 9 Dec 2024 11:09:43 +0100 Subject: [PATCH 02/18] Respect output range --- crates/lsp/src/handlers_format.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 8006ad7b..32faff45 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -57,14 +57,17 @@ pub(crate) fn document_range_formatting( .with_indent_style(IndentStyle::Space) .with_line_width(line_width); - let output = biome_formatter::format_range( + let format_info = biome_formatter::format_range( &doc.parse.syntax(), range, air_r_formatter::RFormatLanguage::new(options), - )? - .into_code(); + )?; - let edits = to_proto::replace_all_edit(&doc.line_index, &doc.contents, &output)?; + // TODO! When can this be None? + let format_range = format_info.range().unwrap(); + let format_text = format_info.into_code(); + + let edits = to_proto::replace_range_edit(&doc.line_index, format_range, format_text)?; Ok(Some(edits)) } From f7fe3ab20115e1789bce3e37a1ff74cd1e5b2f7e Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 9 Dec 2024 12:20:53 +0100 Subject: [PATCH 03/18] Add failing test for formatting range --- crates/lsp/src/handlers_format.rs | 21 ++++++++ crates/lsp/src/rust_analyzer/to_proto.rs | 2 +- ...s_format__tests__format_range_minimal.snap | 6 +++ crates/lsp/src/to_proto.rs | 3 ++ crates/lsp/src/tower_lsp_test_client.rs | 50 ++++++++++++++++++- crates/lsp_test/src/lsp_client.rs | 8 +++ 6 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 32faff45..27a7c1fc 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -73,6 +73,8 @@ pub(crate) fn document_range_formatting( #[cfg(test)] mod tests { + use biome_text_size::{TextRange, TextSize}; + use crate::{ documents::Document, tower_lsp::init_test_client, tower_lsp_test_client::TestClientExt, }; @@ -117,4 +119,23 @@ mod tests { client } + + #[tests_macros::lsp_test] + async fn test_format_range_minimal() { + // FIXME: This currently fails. Line 0 should not be affected by formatting line 1. + let mut client = init_test_client().await; + + #[rustfmt::skip] + let doc = Document::doodle( +"1+1 +2+2 +", + ); + let range = TextRange::new(TextSize::from(4), TextSize::from(7)); + + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + + client + } } diff --git a/crates/lsp/src/rust_analyzer/to_proto.rs b/crates/lsp/src/rust_analyzer/to_proto.rs index 4ec929da..1eab5351 100644 --- a/crates/lsp/src/rust_analyzer/to_proto.rs +++ b/crates/lsp/src/rust_analyzer/to_proto.rs @@ -1,7 +1,7 @@ // --- source // authors = ["rust-analyzer team"] // license = "MIT OR Apache-2.0" -// origin = "https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/lsp/from_proto.rs" +// origin = "https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/lsp/to_proto.rs" // --- //! Conversion of rust-analyzer specific types to lsp_types equivalents. diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap new file mode 100644 index 00000000..30e46b0d --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap @@ -0,0 +1,6 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1 + 1 +2 + 2 diff --git a/crates/lsp/src/to_proto.rs b/crates/lsp/src/to_proto.rs index c7bbc771..4b4c5f5e 100644 --- a/crates/lsp/src/to_proto.rs +++ b/crates/lsp/src/to_proto.rs @@ -9,6 +9,9 @@ pub(crate) use rust_analyzer::to_proto::text_edit_vec; +#[cfg(test)] +pub(crate) use biome_lsp_converters::to_proto::range; + use crate::rust_analyzer::{self, line_index::LineIndex, text_edit::TextEdit}; use biome_text_size::TextRange; use tower_lsp::lsp_types; diff --git a/crates/lsp/src/tower_lsp_test_client.rs b/crates/lsp/src/tower_lsp_test_client.rs index 6d423718..e74f050e 100644 --- a/crates/lsp/src/tower_lsp_test_client.rs +++ b/crates/lsp/src/tower_lsp_test_client.rs @@ -1,12 +1,20 @@ +use biome_text_size::TextRange; use lsp_test::lsp_client::TestClient; use tower_lsp::lsp_types; -use crate::{documents::Document, from_proto}; +use crate::{documents::Document, from_proto, to_proto}; pub(crate) trait TestClientExt { async fn open_document(&mut self, doc: &Document) -> lsp_types::TextDocumentItem; + async fn format_document(&mut self, doc: &Document) -> String; + async fn format_document_range(&mut self, doc: &Document, range: TextRange) -> String; async fn format_document_edits(&mut self, doc: &Document) -> Option<Vec<lsp_types::TextEdit>>; + async fn format_document_range_edits( + &mut self, + doc: &Document, + range: TextRange, + ) -> Option<Vec<lsp_types::TextEdit>>; } impl TestClientExt for TestClient { @@ -34,6 +42,11 @@ impl TestClientExt for TestClient { from_proto::apply_text_edits(doc, edits).unwrap() } + async fn format_document_range(&mut self, doc: &Document, range: TextRange) -> String { + let edits = self.format_document_range_edits(doc, range).await.unwrap(); + from_proto::apply_text_edits(doc, edits).unwrap() + } + async fn format_document_edits(&mut self, doc: &Document) -> Option<Vec<lsp_types::TextEdit>> { let lsp_doc = self.open_document(doc).await; @@ -61,4 +74,39 @@ impl TestClientExt for TestClient { value } + + async fn format_document_range_edits( + &mut self, + doc: &Document, + range: TextRange, + ) -> Option<Vec<lsp_types::TextEdit>> { + let lsp_doc = self.open_document(doc).await; + + let options = lsp_types::FormattingOptions { + tab_size: 4, + insert_spaces: false, + ..Default::default() + }; + + let range = to_proto::range(&doc.line_index.index, range, doc.line_index.encoding).unwrap(); + + self.range_formatting(lsp_types::DocumentRangeFormattingParams { + text_document: lsp_types::TextDocumentIdentifier { + uri: lsp_doc.uri.clone(), + }, + range, + options, + work_done_progress_params: Default::default(), + }) + .await; + + let response = self.recv_response().await; + + let value: Option<Vec<lsp_types::TextEdit>> = + serde_json::from_value(response.result().unwrap().clone()).unwrap(); + + self.close_document(lsp_doc.uri).await; + + value + } } diff --git a/crates/lsp_test/src/lsp_client.rs b/crates/lsp_test/src/lsp_client.rs index 4d086590..dc6cd1da 100644 --- a/crates/lsp_test/src/lsp_client.rs +++ b/crates/lsp_test/src/lsp_client.rs @@ -152,4 +152,12 @@ impl TestClient { pub async fn formatting(&mut self, params: lsp_types::DocumentFormattingParams) -> i64 { self.request::<lsp_types::request::Formatting>(params).await } + + pub async fn range_formatting( + &mut self, + params: lsp_types::DocumentRangeFormattingParams, + ) -> i64 { + self.request::<lsp_types::request::RangeFormatting>(params) + .await + } } From c78ee7e42cd48399d1f0dba5f99e6808f2821dd3 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 9 Dec 2024 14:04:07 +0100 Subject: [PATCH 04/18] Add tests for no formatting changes --- crates/lsp/src/handlers_format.rs | 36 +++++++++++++++++++ ...rs_format__tests__format_range_none-2.snap | 5 +++ ...rs_format__tests__format_range_none-3.snap | 5 +++ ...lers_format__tests__format_range_none.snap | 5 +++ 4 files changed, 51 insertions(+) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none-2.snap create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none-3.snap create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none.snap diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 27a7c1fc..bbcebb80 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -120,6 +120,42 @@ mod tests { client } + #[tests_macros::lsp_test] + async fn test_format_range_none() { + let mut client = init_test_client().await; + + #[rustfmt::skip] + let doc = Document::doodle( +"", + ); + let range = TextRange::new(TextSize::from(0), TextSize::from(0)); + + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + + #[rustfmt::skip] + let doc = Document::doodle( +" +", + ); + let range = TextRange::new(TextSize::from(0), TextSize::from(1)); + + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + + #[rustfmt::skip] + let doc = Document::doodle( +"1 +", + ); + let range = TextRange::new(TextSize::from(0), TextSize::from(1)); + + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + + client + } + #[tests_macros::lsp_test] async fn test_format_range_minimal() { // FIXME: This currently fails. Line 0 should not be affected by formatting line 1. diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none-2.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none-2.snap new file mode 100644 index 00000000..f3257fac --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none-2.snap @@ -0,0 +1,5 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- + diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none-3.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none-3.snap new file mode 100644 index 00000000..0fb74fc7 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none-3.snap @@ -0,0 +1,5 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1 diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none.snap new file mode 100644 index 00000000..f3257fac --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_none.snap @@ -0,0 +1,5 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- + From 02643f020e4c439841ab671e5bc215836f98bb61 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 9 Dec 2024 14:23:49 +0100 Subject: [PATCH 05/18] Handle null range Happens in edge cases when biome returns a `Printed::new_empty()` --- crates/lsp/src/handlers_format.rs | 9 ++++++--- crates/lsp/src/tower_lsp_test_client.rs | 12 +++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index bbcebb80..8eab0f85 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -63,11 +63,14 @@ pub(crate) fn document_range_formatting( air_r_formatter::RFormatLanguage::new(options), )?; - // TODO! When can this be None? - let format_range = format_info.range().unwrap(); - let format_text = format_info.into_code(); + let Some(format_range) = format_info.range() else { + // Happens in edge cases when biome returns a `Printed::new_empty()` + return Ok(None); + }; + let format_text = format_info.into_code(); let edits = to_proto::replace_range_edit(&doc.line_index, format_range, format_text)?; + Ok(Some(edits)) } diff --git a/crates/lsp/src/tower_lsp_test_client.rs b/crates/lsp/src/tower_lsp_test_client.rs index e74f050e..cae23ed9 100644 --- a/crates/lsp/src/tower_lsp_test_client.rs +++ b/crates/lsp/src/tower_lsp_test_client.rs @@ -43,7 +43,9 @@ impl TestClientExt for TestClient { } async fn format_document_range(&mut self, doc: &Document, range: TextRange) -> String { - let edits = self.format_document_range_edits(doc, range).await.unwrap(); + let Some(edits) = self.format_document_range_edits(doc, range).await else { + return doc.contents.clone(); + }; from_proto::apply_text_edits(doc, edits).unwrap() } @@ -67,6 +69,10 @@ impl TestClientExt for TestClient { let response = self.recv_response().await; + if let Some(err) = response.error() { + panic!("Unexpected error: {}", err.message); + }; + let value: Option<Vec<lsp_types::TextEdit>> = serde_json::from_value(response.result().unwrap().clone()).unwrap(); @@ -102,6 +108,10 @@ impl TestClientExt for TestClient { let response = self.recv_response().await; + if let Some(err) = response.error() { + panic!("Unexpected error: {}", err.message); + }; + let value: Option<Vec<lsp_types::TextEdit>> = serde_json::from_value(response.result().unwrap().clone()).unwrap(); From 1ab2fd9ef3261da5508b3cc549167595193d9b99 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 9 Dec 2024 14:31:31 +0100 Subject: [PATCH 06/18] Add test for range bounded by curly braces --- crates/lsp/src/handlers_format.rs | 13 +++++++++++++ ...dlers_format__tests__format_range_minimal-2.snap | 6 ++++++ 2 files changed, 19 insertions(+) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal-2.snap diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 8eab0f85..09ff1396 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -175,6 +175,19 @@ mod tests { let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); + // This currently works because biome selects the curly brace expression + // as node to format, so `1+1` is left alone + #[rustfmt::skip] + let doc = Document::doodle( +"1+1 +{2+2} +", + ); + let range = TextRange::new(TextSize::from(5), TextSize::from(8)); + + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + client } } diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal-2.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal-2.snap new file mode 100644 index 00000000..f5bf150f --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal-2.snap @@ -0,0 +1,6 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1+1 +{2 + 2} From f5e6c508548f26b572f6cd71ccb1487e9879e239 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Tue, 10 Dec 2024 11:28:14 +0100 Subject: [PATCH 07/18] Find deepest enclosing logical line --- Cargo.lock | 1 + crates/lsp/Cargo.toml | 1 + crates/lsp/src/handlers_format.rs | 100 ++++++++++++++++-- ...s_format__tests__format_range_minimal.snap | 2 +- 4 files changed, 94 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 226e1976..90bdb3c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1315,6 +1315,7 @@ dependencies = [ "biome_formatter", "biome_lsp_converters", "biome_parser", + "biome_rowan", "biome_text_size", "bytes", "cargo_metadata", diff --git a/crates/lsp/Cargo.toml b/crates/lsp/Cargo.toml index 5bf1e568..c5e581dc 100644 --- a/crates/lsp/Cargo.toml +++ b/crates/lsp/Cargo.toml @@ -19,6 +19,7 @@ anyhow.workspace = true biome_formatter.workspace = true biome_lsp_converters.workspace = true biome_parser.workspace = true +biome_rowan.workspace = true biome_text_size.workspace = true crossbeam.workspace = true dissimilar.workspace = true diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 09ff1396..1bbd96c2 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -6,7 +6,10 @@ // use air_r_formatter::{context::RFormatOptions, format_node}; +use air_r_syntax::{RSyntaxKind, RSyntaxNode, WalkEvent}; use biome_formatter::{IndentStyle, LineWidth}; +use biome_rowan::{Language, SyntaxElement}; +use biome_text_size::TextRange; use tower_lsp::lsp_types; use crate::state::WorldState; @@ -57,23 +60,90 @@ pub(crate) fn document_range_formatting( .with_indent_style(IndentStyle::Space) .with_line_width(line_width); - let format_info = biome_formatter::format_range( - &doc.parse.syntax(), - range, - air_r_formatter::RFormatLanguage::new(options), - )?; + let Some(node) = find_deepest_enclosing_logical_line(doc.parse.syntax(), range) else { + return Ok(None); + }; + - let Some(format_range) = format_info.range() else { + let format_info = + biome_formatter::format_sub_tree(&node, air_r_formatter::RFormatLanguage::new(options))?; + + let Some(_format_range) = format_info.range() else { // Happens in edge cases when biome returns a `Printed::new_empty()` return Ok(None); }; + let format_range = text_non_whitespace_range(&node); + let format_text = format_info.into_code(); let edits = to_proto::replace_range_edit(&doc.line_index, format_range, format_text)?; Ok(Some(edits)) } +// From biome_formatter +fn text_non_whitespace_range<E, L>(elem: &E) -> TextRange +where + E: Into<SyntaxElement<L>> + Clone, + L: Language, +{ + let elem: SyntaxElement<L> = elem.clone().into(); + + let start = elem + .leading_trivia() + .into_iter() + .flat_map(|trivia| trivia.pieces()) + .find_map(|piece| { + if piece.is_whitespace() || piece.is_newline() { + None + } else { + Some(piece.text_range().start()) + } + }) + .unwrap_or_else(|| elem.text_trimmed_range().start()); + + let end = elem + .trailing_trivia() + .into_iter() + .flat_map(|trivia| trivia.pieces().rev()) + .find_map(|piece| { + if piece.is_whitespace() || piece.is_newline() { + None + } else { + Some(piece.text_range().end()) + } + }) + .unwrap_or_else(|| elem.text_trimmed_range().end()); + + TextRange::new(start, end) +} + +fn find_deepest_enclosing_logical_line(node: RSyntaxNode, range: TextRange) -> Option<RSyntaxNode> { + let mut preorder = node.preorder(); + let mut statement: Option<RSyntaxNode> = None; + + while let Some(event) = preorder.next() { + match event { + WalkEvent::Enter(node) => { + if !node.text_range().contains_range(range) { + preorder.skip_subtree(); + continue; + } + + if let Some(parent) = node.parent() { + if parent.kind() == RSyntaxKind::R_EXPRESSION_LIST { + statement = Some(node.clone()); + } + } + } + + WalkEvent::Leave(_) => {} + } + } + + statement +} + #[cfg(test)] mod tests { use biome_text_size::{TextRange, TextSize}; @@ -161,9 +231,9 @@ mod tests { #[tests_macros::lsp_test] async fn test_format_range_minimal() { - // FIXME: This currently fails. Line 0 should not be affected by formatting line 1. let mut client = init_test_client().await; + // 2+2 is the logical line to format #[rustfmt::skip] let doc = Document::doodle( "1+1 @@ -171,12 +241,24 @@ mod tests { ", ); let range = TextRange::new(TextSize::from(4), TextSize::from(7)); + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + + // FIXME: With a leading comment this currently fails. Why is the + // addition broken up? + #[rustfmt::skip] + let doc = Document::doodle( +"1+1 +# +2+2 +", + ); + let range = TextRange::new(TextSize::from(6), TextSize::from(9)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); - // This currently works because biome selects the curly brace expression - // as node to format, so `1+1` is left alone + // The whole braced expression is a logical line #[rustfmt::skip] let doc = Document::doodle( "1+1 diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap index 30e46b0d..cf743f29 100644 --- a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap @@ -2,5 +2,5 @@ source: crates/lsp/src/handlers_format.rs expression: output --- -1 + 1 +1+1 2 + 2 From 73e39cbb6780ecdaccf0157a8cdb5ca2857d93c3 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Wed, 11 Dec 2024 11:44:17 +0100 Subject: [PATCH 08/18] Wrap in root node to fix wrong comment attachment --- Cargo.lock | 1 + crates/air_r_factory/src/lib.rs | 3 +- .../src/r/auxiliary/binary_expression.rs | 1 + crates/lsp/Cargo.toml | 1 + crates/lsp/src/handlers_format.rs | 42 +++++++++++++++---- ...__tests__format_range_logical_lines-2.snap | 7 ++++ ..._tests__format_range_logical_lines-3.snap} | 0 ...__tests__format_range_logical_lines-4.snap | 8 ++++ ...t__tests__format_range_logical_lines.snap} | 0 9 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-2.snap rename crates/lsp/src/snapshots/{lsp__handlers_format__tests__format_range_minimal-2.snap => lsp__handlers_format__tests__format_range_logical_lines-3.snap} (100%) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-4.snap rename crates/lsp/src/snapshots/{lsp__handlers_format__tests__format_range_minimal.snap => lsp__handlers_format__tests__format_range_logical_lines.snap} (100%) diff --git a/Cargo.lock b/Cargo.lock index 90bdb3c9..a17078ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1307,6 +1307,7 @@ checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" name = "lsp" version = "0.0.0" dependencies = [ + "air_r_factory", "air_r_formatter", "air_r_parser", "air_r_syntax", diff --git a/crates/air_r_factory/src/lib.rs b/crates/air_r_factory/src/lib.rs index 8203124b..ed0f7a8d 100644 --- a/crates/air_r_factory/src/lib.rs +++ b/crates/air_r_factory/src/lib.rs @@ -1,2 +1,3 @@ mod generated; -pub use crate::generated::RSyntaxFactory; +pub use crate::generated::node_factory::*; +pub use crate::generated::*; diff --git a/crates/air_r_formatter/src/r/auxiliary/binary_expression.rs b/crates/air_r_formatter/src/r/auxiliary/binary_expression.rs index cd069a41..8d0cf3d3 100644 --- a/crates/air_r_formatter/src/r/auxiliary/binary_expression.rs +++ b/crates/air_r_formatter/src/r/auxiliary/binary_expression.rs @@ -117,6 +117,7 @@ fn fmt_binary( ) } +#[derive(Debug)] struct TailPiece { operator: SyntaxToken<RLanguage>, right: AnyRExpression, diff --git a/crates/lsp/Cargo.toml b/crates/lsp/Cargo.toml index c5e581dc..3c726341 100644 --- a/crates/lsp/Cargo.toml +++ b/crates/lsp/Cargo.toml @@ -12,6 +12,7 @@ repository.workspace = true rust-version.workspace = true [dependencies] +air_r_factory.workspace = true air_r_formatter.workspace = true air_r_parser.workspace = true air_r_syntax.workspace = true diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 1bbd96c2..f2e42edf 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -8,7 +8,7 @@ use air_r_formatter::{context::RFormatOptions, format_node}; use air_r_syntax::{RSyntaxKind, RSyntaxNode, WalkEvent}; use biome_formatter::{IndentStyle, LineWidth}; -use biome_rowan::{Language, SyntaxElement}; +use biome_rowan::{AstNode, Language, SyntaxElement}; use biome_text_size::TextRange; use tower_lsp::lsp_types; @@ -61,21 +61,42 @@ pub(crate) fn document_range_formatting( .with_line_width(line_width); let Some(node) = find_deepest_enclosing_logical_line(doc.parse.syntax(), range) else { + tracing::warn!("Can't find logical line"); return Ok(None); }; + let format_range = text_non_whitespace_range(&node); + + // We need to wrap in an `RRoot` otherwise the comments get attached too + // deep in the tree. See `CommentsBuilderVisitor` in biome_formatter and the + // `is_root` logic. Note that `node` needs to be wrapped in at least two + // other nodes in order to fix this problem, and here we have an `RRoot` and + // `RExpressionList` that do the job. + // + // Since we only format logical lines, it should be okay to wrap in an expression list. + let Some(expr) = air_r_syntax::AnyRExpression::cast(node) else { + tracing::warn!("Can't cast to `AnyRExpression`"); + return Ok(None); + }; + let list = air_r_factory::r_expression_list(vec![expr]); + + let eof = air_r_syntax::RSyntaxToken::new_detached(RSyntaxKind::EOF, "", vec![], vec![]); + let root = air_r_factory::r_root(list, eof).build(); - let format_info = - biome_formatter::format_sub_tree(&node, air_r_formatter::RFormatLanguage::new(options))?; + let format_info = biome_formatter::format_sub_tree( + root.syntax(), + air_r_formatter::RFormatLanguage::new(options), + )?; - let Some(_format_range) = format_info.range() else { + if format_info.range().is_none() { // Happens in edge cases when biome returns a `Printed::new_empty()` return Ok(None); }; - let format_range = text_non_whitespace_range(&node); + let mut format_text = format_info.into_code(); - let format_text = format_info.into_code(); + // Remove last hard break line from our artifical expression list + format_text.pop(); let edits = to_proto::replace_range_edit(&doc.line_index, format_range, format_text)?; Ok(Some(edits)) @@ -230,7 +251,7 @@ mod tests { } #[tests_macros::lsp_test] - async fn test_format_range_minimal() { + async fn test_format_range_logical_lines() { let mut client = init_test_client().await; // 2+2 is the logical line to format @@ -244,8 +265,6 @@ mod tests { let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); - // FIXME: With a leading comment this currently fails. Why is the - // addition broken up? #[rustfmt::skip] let doc = Document::doodle( "1+1 @@ -259,14 +278,19 @@ mod tests { insta::assert_snapshot!(output); // The whole braced expression is a logical line + // FIXME: Should this be the whole `{2+2}` instead? #[rustfmt::skip] let doc = Document::doodle( "1+1 {2+2} ", ); + let range = TextRange::new(TextSize::from(5), TextSize::from(8)); + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + let range = TextRange::new(TextSize::from(4), TextSize::from(9)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-2.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-2.snap new file mode 100644 index 00000000..ec60bfb9 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-2.snap @@ -0,0 +1,7 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1+1 +# +2 + 2 diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal-2.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-3.snap similarity index 100% rename from crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal-2.snap rename to crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-3.snap diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-4.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-4.snap new file mode 100644 index 00000000..cda77a86 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-4.snap @@ -0,0 +1,8 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1+1 +{ + 2 + 2 +} diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines.snap similarity index 100% rename from crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_minimal.snap rename to crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines.snap From 06d04e8081b9bf4590d7a75383374565d79f80d8 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Wed, 11 Dec 2024 13:03:55 +0100 Subject: [PATCH 09/18] Add test for mismatched indentation --- crates/lsp/src/handlers_format.rs | 26 +++++++++++++++++++ ...tests__format_range_mismatched_indent.snap | 6 +++++ 2 files changed, 32 insertions(+) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_mismatched_indent.snap diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index f2e42edf..c6910d0c 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -290,10 +290,36 @@ mod tests { let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); + // Including braces let range = TextRange::new(TextSize::from(4), TextSize::from(9)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); client } + + #[tests_macros::lsp_test] + async fn test_format_range_mismatched_indent() { + let mut client = init_test_client().await; + + #[rustfmt::skip] + let doc = Document::doodle( +"1 + 2+2 +", + ); + + // We don't change indentation when `2+2` is formatted + let range = TextRange::new(TextSize::from(4), TextSize::from(7)); + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + + // Debatable: Should we make an effort to remove unneeded indentation + // when it's part of the range? + let range_wide = TextRange::new(TextSize::from(2), TextSize::from(7)); + let output_wide = client.format_document_range(&doc, range_wide).await; + assert_eq!(output, output_wide); + + client + } } diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_mismatched_indent.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_mismatched_indent.snap new file mode 100644 index 00000000..9681b5cd --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_mismatched_indent.snap @@ -0,0 +1,6 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1 + 2 + 2 From 1a2e1cb01f05e346413794f9c7469db93f933065 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Wed, 11 Dec 2024 13:22:28 +0100 Subject: [PATCH 10/18] Format all logical lines included in range --- crates/lsp/src/handlers_format.rs | 70 +++++++++++++++---- ..._tests__format_range_multiple_lines-2.snap | 7 ++ ...t__tests__format_range_multiple_lines.snap | 7 ++ 3 files changed, 70 insertions(+), 14 deletions(-) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines-2.snap create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines.snap diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index c6910d0c..12b7e7bd 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -60,12 +60,19 @@ pub(crate) fn document_range_formatting( .with_indent_style(IndentStyle::Space) .with_line_width(line_width); - let Some(node) = find_deepest_enclosing_logical_line(doc.parse.syntax(), range) else { + let logical_lines = find_deepest_enclosing_logical_lines(doc.parse.syntax(), range); + if logical_lines.is_empty() { tracing::warn!("Can't find logical line"); return Ok(None); }; - let format_range = text_non_whitespace_range(&node); + // Find the overall formatting range by concatenating the ranges of the logical lines. + // We use the "non-whitespace-range" as that corresponds to what Biome will format. + let format_range = logical_lines + .iter() + .map(|line| text_non_whitespace_range(line)) + .reduce(|acc, new| acc.cover(new)) + .expect("`logical_lines` is non-empty"); // We need to wrap in an `RRoot` otherwise the comments get attached too // deep in the tree. See `CommentsBuilderVisitor` in biome_formatter and the @@ -73,13 +80,17 @@ pub(crate) fn document_range_formatting( // other nodes in order to fix this problem, and here we have an `RRoot` and // `RExpressionList` that do the job. // - // Since we only format logical lines, it should be okay to wrap in an expression list. - let Some(expr) = air_r_syntax::AnyRExpression::cast(node) else { + // Since we only format logical lines, it is fine to wrap in an expression list. + let Some(exprs): Option<Vec<air_r_syntax::AnyRExpression>> = logical_lines + .into_iter() + .map(|node| air_r_syntax::AnyRExpression::cast(node)) + .collect() + else { tracing::warn!("Can't cast to `AnyRExpression`"); return Ok(None); }; - let list = air_r_factory::r_expression_list(vec![expr]); + let list = air_r_factory::r_expression_list(exprs); let eof = air_r_syntax::RSyntaxToken::new_detached(RSyntaxKind::EOF, "", vec![], vec![]); let root = air_r_factory::r_root(list, eof).build(); @@ -139,22 +150,28 @@ where TextRange::new(start, end) } -fn find_deepest_enclosing_logical_line(node: RSyntaxNode, range: TextRange) -> Option<RSyntaxNode> { +/// Finds consecutive logical lines. Currently that's only expressions at +/// top-level or in a braced list. +fn find_deepest_enclosing_logical_lines(node: RSyntaxNode, range: TextRange) -> Vec<RSyntaxNode> { let mut preorder = node.preorder(); - let mut statement: Option<RSyntaxNode> = None; + let mut logical_lines: Vec<RSyntaxNode> = vec![]; while let Some(event) = preorder.next() { match event { WalkEvent::Enter(node) => { - if !node.text_range().contains_range(range) { - preorder.skip_subtree(); + let Some(parent) = node.parent() else { + continue; + }; + + let node_range = node.text_trimmed_range(); + if !range.contains_range(node_range) { continue; } - if let Some(parent) = node.parent() { - if parent.kind() == RSyntaxKind::R_EXPRESSION_LIST { - statement = Some(node.clone()); - } + if parent.kind() == RSyntaxKind::R_EXPRESSION_LIST { + logical_lines.push(node.clone()); + preorder.skip_subtree(); + continue; } } @@ -162,7 +179,7 @@ fn find_deepest_enclosing_logical_line(node: RSyntaxNode, range: TextRange) -> O } } - statement + logical_lines } #[cfg(test)] @@ -322,4 +339,29 @@ mod tests { client } + + #[tests_macros::lsp_test] + async fn test_format_range_multiple_lines() { + let mut client = init_test_client().await; + + #[rustfmt::skip] + let doc = Document::doodle( +"1+1 +# +2+2 +", + ); + + // Selecting the last two lines + let range = TextRange::new(TextSize::from(4), TextSize::from(9)); + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + + // Selecting all three lines + let range = TextRange::new(TextSize::from(0), TextSize::from(9)); + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); + + client + } } diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines-2.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines-2.snap new file mode 100644 index 00000000..abddb407 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines-2.snap @@ -0,0 +1,7 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1 + 1 +# +2 + 2 diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines.snap new file mode 100644 index 00000000..ec60bfb9 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines.snap @@ -0,0 +1,7 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1+1 +# +2 + 2 From 757790802afb02fc0a1cc9644007a879e901a0a0 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Wed, 11 Dec 2024 15:23:19 +0100 Subject: [PATCH 11/18] Add test for deep logical line --- crates/lsp/src/handlers_format.rs | 18 +++++++++++++++++- ...t__tests__format_range_logical_lines-5.snap | 11 +++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-5.snap diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 12b7e7bd..92fb4cea 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -294,7 +294,7 @@ mod tests { let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); - // The whole braced expression is a logical line + // The element in the braced expression is a logical line // FIXME: Should this be the whole `{2+2}` instead? #[rustfmt::skip] let doc = Document::doodle( @@ -312,6 +312,22 @@ mod tests { let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); + // The deepest element in the braced expression is our target + #[rustfmt::skip] + let doc = Document::doodle( +"1+1 +{ + 2+2 + { + 3+3 + } +} +", + ); + + let range = TextRange::new(TextSize::from(20), TextSize::from(23)); + let output = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output); client } diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-5.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-5.snap new file mode 100644 index 00000000..df9e8931 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_logical_lines-5.snap @@ -0,0 +1,11 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output +--- +1+1 +{ + 2+2 + { + 3 + 3 + } +} From dc8787855ae3aa3116795c1eee53e310ba301fa9 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Wed, 11 Dec 2024 15:28:20 +0100 Subject: [PATCH 12/18] Add changelog --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..2aaa41a0 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,12 @@ +# Changelog + +# 0.1.9000 + +- The LSP gains range formatting support (#63). + +- The `air format` command has been improved and is now able to take multiple files and directories. + + +# 0.1.0 + +- Initial release. From a754eff3882f24ea9f16efca02da006ba54a0e37 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Thu, 12 Dec 2024 13:07:59 +0100 Subject: [PATCH 13/18] Rework algorithm to handle ranges over nested lists --- crates/lsp/src/handlers_format.rs | 107 ++++++++++++++++-- ..._tests__format_range_multiple_lines-2.snap | 2 +- ...t__tests__format_range_multiple_lines.snap | 2 +- ...tests__format_range_unmatched_lists-2.snap | 10 ++ ...tests__format_range_unmatched_lists-3.snap | 10 ++ ...tests__format_range_unmatched_lists-4.snap | 10 ++ ...__tests__format_range_unmatched_lists.snap | 10 ++ 7 files changed, 137 insertions(+), 14 deletions(-) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-2.snap create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-3.snap create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-4.snap create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists.snap diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 92fb4cea..96b3e843 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -6,10 +6,10 @@ // use air_r_formatter::{context::RFormatOptions, format_node}; -use air_r_syntax::{RSyntaxKind, RSyntaxNode, WalkEvent}; +use air_r_syntax::{RExpressionList, RSyntaxKind, RSyntaxNode, WalkEvent}; use biome_formatter::{IndentStyle, LineWidth}; use biome_rowan::{AstNode, Language, SyntaxElement}; -use biome_text_size::TextRange; +use biome_text_size::{TextRange, TextSize}; use tower_lsp::lsp_types; use crate::state::WorldState; @@ -153,8 +153,46 @@ where /// Finds consecutive logical lines. Currently that's only expressions at /// top-level or in a braced list. fn find_deepest_enclosing_logical_lines(node: RSyntaxNode, range: TextRange) -> Vec<RSyntaxNode> { + let start_lists = find_expression_lists(&node, range.start(), false); + let end_lists = find_expression_lists(&node, range.end(), true); + + // Both vectors of lists should have a common prefix, starting from the + // program's expression list. As soon as the lists diverge we stop. + let Some(list) = start_lists + .into_iter() + .zip(end_lists.into_iter()) + .take_while(|pair| pair.0 == pair.1) + .map(|pair| pair.0) + .last() + else { + // Should not happen as the range is always included in the program's expression list + tracing::warn!("Can't find common list parent"); + return vec![]; + }; + + let Some(list) = RExpressionList::cast(list) else { + tracing::warn!("Can't cast to expression list"); + return vec![]; + }; + + let iter = list.into_iter(); + + let logical_lines: Vec<RSyntaxNode> = iter + .map(|expr| expr.into_syntax()) + .skip_while(|node| !node.text_range().contains(range.start())) + .take_while(|node| node.text_range().start() <= range.end()) + .collect(); + + logical_lines +} + +fn find_expression_lists( + node: &RSyntaxNode, + offset: TextSize, + inclusive: bool, +) -> Vec<RSyntaxNode> { let mut preorder = node.preorder(); - let mut logical_lines: Vec<RSyntaxNode> = vec![]; + let mut nodes: Vec<RSyntaxNode> = vec![]; while let Some(event) = preorder.next() { match event { @@ -163,14 +201,21 @@ fn find_deepest_enclosing_logical_lines(node: RSyntaxNode, range: TextRange) -> continue; }; - let node_range = node.text_trimmed_range(); - if !range.contains_range(node_range) { + let node_range = node.text_range(); + + let is_contained = if inclusive { + node_range.contains_inclusive(offset) + } else { + node_range.contains(offset) + }; + + if !is_contained { + preorder.skip_subtree(); continue; } if parent.kind() == RSyntaxKind::R_EXPRESSION_LIST { - logical_lines.push(node.clone()); - preorder.skip_subtree(); + nodes.push(parent.clone()); continue; } } @@ -179,7 +224,7 @@ fn find_deepest_enclosing_logical_lines(node: RSyntaxNode, range: TextRange) -> } } - logical_lines + nodes } #[cfg(test)] @@ -370,13 +415,51 @@ mod tests { // Selecting the last two lines let range = TextRange::new(TextSize::from(4), TextSize::from(9)); - let output = client.format_document_range(&doc, range).await; - insta::assert_snapshot!(output); + let output1 = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output1); // Selecting all three lines let range = TextRange::new(TextSize::from(0), TextSize::from(9)); - let output = client.format_document_range(&doc, range).await; - insta::assert_snapshot!(output); + let output2 = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output2); + + client + } + + #[tests_macros::lsp_test] + async fn test_format_range_unmatched_lists() { + let mut client = init_test_client().await; + + #[rustfmt::skip] + let doc = Document::doodle( +"0+0 +1+1 +{ + 2+2 +} +3+3 +", + ); + + // Selecting lines 2-4 + let range = TextRange::new(TextSize::from(4), TextSize::from(15)); + let output1 = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output1); + + // Selecting lines 2-4 with partial selection on line 4 + let range = TextRange::new(TextSize::from(4), TextSize::from(10)); + let output2 = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output2); + + // Selecting lines 2-6 + let range = TextRange::new(TextSize::from(4), TextSize::from(18)); + let output3 = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output3); + + // Selecting lines 4-6 + let range = TextRange::new(TextSize::from(10), TextSize::from(18)); + let output4 = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output4); client } diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines-2.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines-2.snap index abddb407..cc0b2f04 100644 --- a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines-2.snap +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines-2.snap @@ -1,6 +1,6 @@ --- source: crates/lsp/src/handlers_format.rs -expression: output +expression: output2 --- 1 + 1 # diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines.snap index ec60bfb9..f63c36ef 100644 --- a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines.snap +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_multiple_lines.snap @@ -1,6 +1,6 @@ --- source: crates/lsp/src/handlers_format.rs -expression: output +expression: output1 --- 1+1 # diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-2.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-2.snap new file mode 100644 index 00000000..776dab17 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-2.snap @@ -0,0 +1,10 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output2 +--- +0+0 +1 + 1 +{ + 2 + 2 +} +3+3 diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-3.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-3.snap new file mode 100644 index 00000000..5d9e27d0 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-3.snap @@ -0,0 +1,10 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output3 +--- +0+0 +1 + 1 +{ + 2 + 2 +} +3 + 3 diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-4.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-4.snap new file mode 100644 index 00000000..4234f400 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-4.snap @@ -0,0 +1,10 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output4 +--- +0+0 +1+1 +{ + 2 + 2 +} +3 + 3 diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists.snap new file mode 100644 index 00000000..33752478 --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists.snap @@ -0,0 +1,10 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output1 +--- +0+0 +1 + 1 +{ + 2 + 2 +} +3+3 From cdc491ad7a99b3feae8269a56e4b396ed6d73a57 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Thu, 12 Dec 2024 14:34:46 +0100 Subject: [PATCH 14/18] Rename dev version header --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aaa41a0..8ed57efa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -# 0.1.9000 +# Development version - The LSP gains range formatting support (#63). From 523004b1735d159b8f4063d0c7e5f954874b3381 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Thu, 12 Dec 2024 16:44:37 +0100 Subject: [PATCH 15/18] Fix clippy notes --- crates/lsp/src/handlers_format.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 96b3e843..5c9d2551 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -70,7 +70,7 @@ pub(crate) fn document_range_formatting( // We use the "non-whitespace-range" as that corresponds to what Biome will format. let format_range = logical_lines .iter() - .map(|line| text_non_whitespace_range(line)) + .map(text_non_whitespace_range) .reduce(|acc, new| acc.cover(new)) .expect("`logical_lines` is non-empty"); @@ -83,7 +83,7 @@ pub(crate) fn document_range_formatting( // Since we only format logical lines, it is fine to wrap in an expression list. let Some(exprs): Option<Vec<air_r_syntax::AnyRExpression>> = logical_lines .into_iter() - .map(|node| air_r_syntax::AnyRExpression::cast(node)) + .map(air_r_syntax::AnyRExpression::cast) .collect() else { tracing::warn!("Can't cast to `AnyRExpression`"); @@ -160,7 +160,7 @@ fn find_deepest_enclosing_logical_lines(node: RSyntaxNode, range: TextRange) -> // program's expression list. As soon as the lists diverge we stop. let Some(list) = start_lists .into_iter() - .zip(end_lists.into_iter()) + .zip(end_lists) .take_while(|pair| pair.0 == pair.1) .map(|pair| pair.0) .last() From 4be4b618b593696d4388ba3847ef59cc2b39e0d7 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 16 Dec 2024 10:25:57 +0100 Subject: [PATCH 16/18] Add `doodle_and_range()` for easier testing --- crates/lsp/src/documents.rs | 7 ++ crates/lsp/src/handlers_format.rs | 124 ++++++++++++++++++------------ crates/lsp/src/lib.rs | 2 + crates/lsp/src/test_utils.rs | 27 +++++++ 4 files changed, 110 insertions(+), 50 deletions(-) create mode 100644 crates/lsp/src/test_utils.rs diff --git a/crates/lsp/src/documents.rs b/crates/lsp/src/documents.rs index 56ed960e..d2406dae 100644 --- a/crates/lsp/src/documents.rs +++ b/crates/lsp/src/documents.rs @@ -89,6 +89,13 @@ impl Document { Self::new(contents.into(), None, PositionEncoding::Utf8) } + #[cfg(test)] + pub fn doodle_and_range(contents: &str) -> (Self, biome_text_size::TextRange) { + let (contents, range) = crate::test_utils::extract_marked_range(contents); + let doc = Self::new(contents, None, PositionEncoding::Utf8); + (doc, range) + } + pub fn on_did_change(&mut self, mut params: lsp_types::DidChangeTextDocumentParams) { let new_version = params.text_document.version; diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 5c9d2551..09503ced 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -229,8 +229,6 @@ fn find_expression_lists( #[cfg(test)] mod tests { - use biome_text_size::{TextRange, TextSize}; - use crate::{ documents::Document, tower_lsp::init_test_client, tower_lsp_test_client::TestClientExt, }; @@ -281,30 +279,27 @@ mod tests { let mut client = init_test_client().await; #[rustfmt::skip] - let doc = Document::doodle( -"", + let (doc, range) = Document::doodle_and_range( +"<<>>", ); - let range = TextRange::new(TextSize::from(0), TextSize::from(0)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); #[rustfmt::skip] - let doc = Document::doodle( -" -", + let (doc, range) = Document::doodle_and_range( +"<< +>>", ); - let range = TextRange::new(TextSize::from(0), TextSize::from(1)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); #[rustfmt::skip] - let doc = Document::doodle( -"1 -", + let (doc, range) = Document::doodle_and_range( +"<<1 +>>", ); - let range = TextRange::new(TextSize::from(0), TextSize::from(1)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); @@ -318,23 +313,21 @@ mod tests { // 2+2 is the logical line to format #[rustfmt::skip] - let doc = Document::doodle( + let (doc, range) = Document::doodle_and_range( "1+1 -2+2 +<<2+2>> ", ); - let range = TextRange::new(TextSize::from(4), TextSize::from(7)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); #[rustfmt::skip] - let doc = Document::doodle( + let (doc, range) = Document::doodle_and_range( "1+1 # -2+2 +<<2+2>> ", ); - let range = TextRange::new(TextSize::from(6), TextSize::from(9)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); @@ -342,35 +335,37 @@ mod tests { // The element in the braced expression is a logical line // FIXME: Should this be the whole `{2+2}` instead? #[rustfmt::skip] - let doc = Document::doodle( + let (doc, range) = Document::doodle_and_range( "1+1 -{2+2} +{<<2+2>>} ", ); - let range = TextRange::new(TextSize::from(5), TextSize::from(8)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); - // Including braces - let range = TextRange::new(TextSize::from(4), TextSize::from(9)); + #[rustfmt::skip] + let (doc, range) = Document::doodle_and_range( +"1+1 +<<{2+2}>> +", + ); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); // The deepest element in the braced expression is our target #[rustfmt::skip] - let doc = Document::doodle( + let (doc, range) = Document::doodle_and_range( "1+1 { 2+2 { - 3+3 + <<3+3>> } } ", ); - let range = TextRange::new(TextSize::from(20), TextSize::from(23)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); client @@ -381,21 +376,25 @@ mod tests { let mut client = init_test_client().await; #[rustfmt::skip] - let doc = Document::doodle( + let (doc, range) = Document::doodle_and_range( "1 - 2+2 + <<2+2>> ", ); // We don't change indentation when `2+2` is formatted - let range = TextRange::new(TextSize::from(4), TextSize::from(7)); let output = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output); // Debatable: Should we make an effort to remove unneeded indentation // when it's part of the range? - let range_wide = TextRange::new(TextSize::from(2), TextSize::from(7)); - let output_wide = client.format_document_range(&doc, range_wide).await; + #[rustfmt::skip] + let (doc, range) = Document::doodle_and_range( +"1 +<< 2+2>> +", + ); + let output_wide = client.format_document_range(&doc, range).await; assert_eq!(output, output_wide); client @@ -406,20 +405,23 @@ mod tests { let mut client = init_test_client().await; #[rustfmt::skip] - let doc = Document::doodle( + let (doc, range) = Document::doodle_and_range( "1+1 -# -2+2 +<<# +2+2>> ", ); - // Selecting the last two lines - let range = TextRange::new(TextSize::from(4), TextSize::from(9)); let output1 = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output1); - // Selecting all three lines - let range = TextRange::new(TextSize::from(0), TextSize::from(9)); + #[rustfmt::skip] + let (doc, range) = Document::doodle_and_range( +"<<1+1 +# +2+2>> +", + ); let output2 = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output2); @@ -431,33 +433,55 @@ mod tests { let mut client = init_test_client().await; #[rustfmt::skip] - let doc = Document::doodle( + let (doc, range) = Document::doodle_and_range( "0+0 -1+1 +<<1+1 { - 2+2 + 2+2>> } 3+3 ", ); - // Selecting lines 2-4 - let range = TextRange::new(TextSize::from(4), TextSize::from(15)); let output1 = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output1); - // Selecting lines 2-4 with partial selection on line 4 - let range = TextRange::new(TextSize::from(4), TextSize::from(10)); + #[rustfmt::skip] + let (doc, range) = Document::doodle_and_range( +"0+0 +<<1+1 +{ +>> 2+2 +} +3+3 +", + ); let output2 = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output2); - // Selecting lines 2-6 - let range = TextRange::new(TextSize::from(4), TextSize::from(18)); + #[rustfmt::skip] + let (doc, range) = Document::doodle_and_range( +"0+0 +<<1+1 +{ + 2+2 +} +>>3+3 +", + ); let output3 = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output3); - // Selecting lines 4-6 - let range = TextRange::new(TextSize::from(10), TextSize::from(18)); + #[rustfmt::skip] + let (doc, range) = Document::doodle_and_range( +"0+0 +1+1 +{ +<< 2+2 +} +>>3+3 +", + ); let output4 = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output4); diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index 30c20aac..65451b9f 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -19,5 +19,7 @@ pub mod state; pub mod to_proto; pub mod tower_lsp; +#[cfg(test)] +pub mod test_utils; #[cfg(test)] pub mod tower_lsp_test_client; diff --git a/crates/lsp/src/test_utils.rs b/crates/lsp/src/test_utils.rs new file mode 100644 index 00000000..fe60c0c9 --- /dev/null +++ b/crates/lsp/src/test_utils.rs @@ -0,0 +1,27 @@ +use biome_text_size::{TextRange, TextSize}; + +pub(crate) fn extract_marked_range(input: &str) -> (String, TextRange) { + let mut output = String::new(); + let mut start = None; + let mut end = None; + let mut chars = input.chars().peekable(); + + while let Some(c) = chars.next() { + if c == '<' && chars.peek() == Some(&'<') { + chars.next(); + start = Some(TextSize::from(output.len() as u32)); + } else if c == '>' && chars.peek() == Some(&'>') { + chars.next(); + end = Some(TextSize::from(output.len() as u32)); + } else { + output.push(c); + } + } + + let range = match (start, end) { + (Some(start), Some(end)) => TextRange::new(start, end), + _ => panic!("Missing range markers"), + }; + + (output, range) +} From 5e8dfd88ccfd87ef4dc851fc9e4d119aaca6dd09 Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 16 Dec 2024 11:33:27 +0100 Subject: [PATCH 17/18] Trim range when inspecting end positions --- crates/lsp/src/handlers_format.rs | 36 +++++++++++++------ ...tests__format_range_unmatched_lists-5.snap | 6 ++++ ...tests__format_range_unmatched_lists-6.snap | 6 ++++ 3 files changed, 38 insertions(+), 10 deletions(-) create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-5.snap create mode 100644 crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-6.snap diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index 09503ced..de765e61 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -180,17 +180,13 @@ fn find_deepest_enclosing_logical_lines(node: RSyntaxNode, range: TextRange) -> let logical_lines: Vec<RSyntaxNode> = iter .map(|expr| expr.into_syntax()) .skip_while(|node| !node.text_range().contains(range.start())) - .take_while(|node| node.text_range().start() <= range.end()) + .take_while(|node| node.text_trimmed_range().start() <= range.end()) .collect(); logical_lines } -fn find_expression_lists( - node: &RSyntaxNode, - offset: TextSize, - inclusive: bool, -) -> Vec<RSyntaxNode> { +fn find_expression_lists(node: &RSyntaxNode, offset: TextSize, end: bool) -> Vec<RSyntaxNode> { let mut preorder = node.preorder(); let mut nodes: Vec<RSyntaxNode> = vec![]; @@ -201,11 +197,11 @@ fn find_expression_lists( continue; }; - let node_range = node.text_range(); - - let is_contained = if inclusive { - node_range.contains_inclusive(offset) + let is_contained = if end { + let trimmed_node_range = node.text_trimmed_range(); + trimmed_node_range.contains_inclusive(offset) } else { + let node_range = node.text_range(); node_range.contains(offset) }; @@ -485,6 +481,26 @@ mod tests { let output4 = client.format_document_range(&doc, range).await; insta::assert_snapshot!(output4); + #[rustfmt::skip] + let (doc, range) = Document::doodle_and_range( +"<<1+1>> +2+2 +", + ); + + let output5 = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output5); + + #[rustfmt::skip] + let (doc, range) = Document::doodle_and_range( +"1+1 +<<2+2>> +", + ); + + let output6 = client.format_document_range(&doc, range).await; + insta::assert_snapshot!(output6); + client } } diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-5.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-5.snap new file mode 100644 index 00000000..2055d4bb --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-5.snap @@ -0,0 +1,6 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output5 +--- +1 + 1 +2+2 diff --git a/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-6.snap b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-6.snap new file mode 100644 index 00000000..2388991a --- /dev/null +++ b/crates/lsp/src/snapshots/lsp__handlers_format__tests__format_range_unmatched_lists-6.snap @@ -0,0 +1,6 @@ +--- +source: crates/lsp/src/handlers_format.rs +expression: output6 +--- +1+1 +2 + 2 From 8f425bcc148db6ca4028c4f4db27dde3099cbbbe Mon Sep 17 00:00:00 2001 From: Lionel Henry <lionel.hry@proton.me> Date: Mon, 16 Dec 2024 12:06:31 +0100 Subject: [PATCH 18/18] Comment on our choice to always widen selections --- crates/lsp/src/handlers_format.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/lsp/src/handlers_format.rs b/crates/lsp/src/handlers_format.rs index de765e61..c9de94b3 100644 --- a/crates/lsp/src/handlers_format.rs +++ b/crates/lsp/src/handlers_format.rs @@ -177,6 +177,14 @@ fn find_deepest_enclosing_logical_lines(node: RSyntaxNode, range: TextRange) -> let iter = list.into_iter(); + // We've chosen to be liberal about user selections and always widen the + // range to include the selection bounds. If we wanted to be conservative + // instead, we could use this `filter()` instead of the `skip_while()` and + // `take_while()`: + // + // ```rust + // .filter(|node| range.contains_range(node.text_trimmed_range())) + // ``` let logical_lines: Vec<RSyntaxNode> = iter .map(|expr| expr.into_syntax()) .skip_while(|node| !node.text_range().contains(range.start()))