feat: make join lines behavior configurable

closes #9492

crates/ide/src/join_lines.rs

@@ -12,10 +12,20 @@ use syntax::{
 
 use text_edit::{TextEdit, TextEditBuilder};
 
+pub struct JoinLinesConfig {
+    pub join_else_if: bool,
+    pub remove_trailing_comma: bool,
+    pub unwrap_trivial_blocks: bool,
+}
+
 // Feature: Join Lines
 //
 // Join selected lines into one, smartly fixing up whitespace, trailing commas, and braces.
 //
+// See
+// https://user-images.githubusercontent.com/1711539/124515923-4504e800-dde9-11eb-8d58-d97945a1a785.gif[this gif]
+// for the cases handled specially by joined lines.
+//
 // |===
 // | Editor  | Action Name
 //
@@ -23,7 +33,11 @@ use text_edit::{TextEdit, TextEditBuilder};
 // |===
 //
 // image::https://user-images.githubusercontent.com/48062697/113020661-b6922200-917a-11eb-87c4-b75acc028f11.gif[]
-pub(crate) fn join_lines(file: &SourceFile, range: TextRange) -> TextEdit {
+pub(crate) fn join_lines(
+    config: &JoinLinesConfig,
+    file: &SourceFile,
+    range: TextRange,
+) -> TextEdit {
     let range = if range.is_empty() {
         let syntax = file.syntax();
         let text = syntax.text().slice(range.start()..);
@@ -40,15 +54,20 @@ pub(crate) fn join_lines(file: &SourceFile, range: TextRange) -> TextEdit {
     match file.syntax().covering_element(range) {
         NodeOrToken::Node(node) => {
             for token in node.descendants_with_tokens().filter_map(|it| it.into_token()) {
-                remove_newlines(&mut edit, &token, range)
+                remove_newlines(config, &mut edit, &token, range)
             }
         }
-        NodeOrToken::Token(token) => remove_newlines(&mut edit, &token, range),
+        NodeOrToken::Token(token) => remove_newlines(config, &mut edit, &token, range),
     };
     edit.finish()
 }
 
-fn remove_newlines(edit: &mut TextEditBuilder, token: &SyntaxToken, range: TextRange) {
+fn remove_newlines(
+    config: &JoinLinesConfig,
+    edit: &mut TextEditBuilder,
+    token: &SyntaxToken,
+    range: TextRange,
+) {
     let intersection = match range.intersect(token.text_range()) {
         Some(range) => range,
         None => return,
@@ -60,12 +79,17 @@ fn remove_newlines(edit: &mut TextEditBuilder, token: &SyntaxToken, range: TextR
         let pos: TextSize = (pos as u32).into();
         let offset = token.text_range().start() + range.start() + pos;
         if !edit.invalidates_offset(offset) {
-            remove_newline(edit, token, offset);
+            remove_newline(config, edit, token, offset);
         }
     }
 }
 
-fn remove_newline(edit: &mut TextEditBuilder, token: &SyntaxToken, offset: TextSize) {
+fn remove_newline(
+    config: &JoinLinesConfig,
+    edit: &mut TextEditBuilder,
+    token: &SyntaxToken,
+    offset: TextSize,
+) {
     if token.kind() != WHITESPACE || token.text().bytes().filter(|&b| b == b'\n').count() != 1 {
         let n_spaces_after_line_break = {
             let suff = &token.text()[TextRange::new(
@@ -102,13 +126,14 @@ fn remove_newline(edit: &mut TextEditBuilder, token: &SyntaxToken, offset: TextS
         _ => return,
     };
 
-    if is_trailing_comma(prev.kind(), next.kind()) {
+    if config.remove_trailing_comma && prev.kind() == T![,] {
+        match next.kind() {
+            T![')'] | T![']'] => {
                 // Removes: trailing comma, newline (incl. surrounding whitespace)
                 edit.delete(TextRange::new(prev.text_range().start(), token.text_range().end()));
                 return;
             }
-
+            T!['}'] => {
-    if prev.kind() == T![,] && next.kind() == T!['}'] {
                 // Removes: comma, newline (incl. surrounding whitespace)
                 let space = if let Some(left) = prev.prev_sibling_or_token() {
                     compute_ws(left.kind(), next.kind())
@@ -121,19 +146,11 @@ fn remove_newline(edit: &mut TextEditBuilder, token: &SyntaxToken, offset: TextS
                 );
                 return;
             }
-
+            _ => (),
-    if let (Some(_), Some(next)) = (
+        }
-        prev.as_token().cloned().and_then(ast::Comment::cast),
-        next.as_token().cloned().and_then(ast::Comment::cast),
-    ) {
-        // Removes: newline (incl. surrounding whitespace), start of the next comment
-        edit.delete(TextRange::new(
-            token.text_range().start(),
-            next.syntax().text_range().start() + TextSize::of(next.prefix()),
-        ));
-        return;
     }
 
+    if config.join_else_if {
         if let (Some(prev), Some(_next)) = (as_if_expr(&prev), as_if_expr(&next)) {
             match prev.else_token() {
                 Some(_) => cov_mark::hit!(join_two_ifs_with_existing_else),
@@ -144,7 +161,9 @@ fn remove_newline(edit: &mut TextEditBuilder, token: &SyntaxToken, offset: TextS
                 }
             }
         }
+    }
 
+    if config.unwrap_trivial_blocks {
         // Special case that turns something like:
         //
         // ```
@@ -167,6 +186,19 @@ fn remove_newline(edit: &mut TextEditBuilder, token: &SyntaxToken, offset: TextS
         if join_single_use_tree(edit, token).is_some() {
             return;
         }
+    }
+
+    if let (Some(_), Some(next)) = (
+        prev.as_token().cloned().and_then(ast::Comment::cast),
+        next.as_token().cloned().and_then(ast::Comment::cast),
+    ) {
+        // Removes: newline (incl. surrounding whitespace), start of the next comment
+        edit.delete(TextRange::new(
+            token.text_range().start(),
+            next.syntax().text_range().start() + TextSize::of(next.prefix()),
+        ));
+        return;
+    }
 
     // Remove newline but add a computed amount of whitespace characters
     edit.replace(token.text_range(), compute_ws(prev.kind(), next.kind()).to_string());
@@ -208,10 +240,6 @@ fn join_single_use_tree(edit: &mut TextEditBuilder, token: &SyntaxToken) -> Opti
     Some(())
 }
 
-fn is_trailing_comma(left: SyntaxKind, right: SyntaxKind) -> bool {
-    matches!((left, right), (T![,], T![')'] | T![']']))
-}
-
 fn as_if_expr(element: &SyntaxElement) -> Option<ast::IfExpr> {
     let mut node = element.as_node()?.clone();
     if let Some(stmt) = ast::ExprStmt::cast(node.clone()) {
@@ -251,11 +279,17 @@ mod tests {
     use super::*;
 
     fn check_join_lines(ra_fixture_before: &str, ra_fixture_after: &str) {
+        let config = JoinLinesConfig {
+            join_else_if: true,
+            remove_trailing_comma: true,
+            unwrap_trivial_blocks: true,
+        };
+
         let (before_cursor_pos, before) = extract_offset(ra_fixture_before);
         let file = SourceFile::parse(&before).ok().unwrap();
 
         let range = TextRange::empty(before_cursor_pos);
-        let result = join_lines(&file, range);
+        let result = join_lines(&config, &file, range);
 
         let actual = {
             let mut actual = before;
@@ -269,6 +303,24 @@ mod tests {
         assert_eq_text!(ra_fixture_after, &actual);
     }
 
+    fn check_join_lines_sel(ra_fixture_before: &str, ra_fixture_after: &str) {
+        let config = JoinLinesConfig {
+            join_else_if: true,
+            remove_trailing_comma: true,
+            unwrap_trivial_blocks: true,
+        };
+
+        let (sel, before) = extract_range(ra_fixture_before);
+        let parse = SourceFile::parse(&before);
+        let result = join_lines(&config, &parse.tree(), sel);
+        let actual = {
+            let mut actual = before;
+            result.apply(&mut actual);
+            actual
+        };
+        assert_eq_text!(ra_fixture_after, &actual);
+    }
+
     #[test]
     fn test_join_lines_comma() {
         check_join_lines(
@@ -657,18 +709,6 @@ fn foo() {
         );
     }
 
-    fn check_join_lines_sel(ra_fixture_before: &str, ra_fixture_after: &str) {
-        let (sel, before) = extract_range(ra_fixture_before);
-        let parse = SourceFile::parse(&before);
-        let result = join_lines(&parse.tree(), sel);
-        let actual = {
-            let mut actual = before;
-            result.apply(&mut actual);
-            actual
-        };
-        assert_eq_text!(ra_fixture_after, &actual);
-    }
-
     #[test]
     fn test_join_lines_selection_fn_args() {
         check_join_lines_sel(

crates/ide/src/lib.rs

@@ -57,12 +57,11 @@ mod view_item_tree;
 use std::sync::Arc;
 
 use cfg::CfgOptions;
-
+use ide_db::{
-use ide_db::base_db::{
+    base_db::{
         salsa::{self, ParallelDatabase},
         Env, FileLoader, FileSet, SourceDatabase, VfsPath,
-};
+    },
-use ide_db::{
     symbol_index::{self, FileSymbol},
     LineIndexDatabase,
 };
@@ -80,6 +79,7 @@ pub use crate::{
     highlight_related::HighlightedRange,
     hover::{HoverAction, HoverConfig, HoverDocFormat, HoverGotoTypeData, HoverResult},
     inlay_hints::{InlayHint, InlayHintsConfig, InlayKind},
+    join_lines::JoinLinesConfig,
     markup::Markup,
     move_item::Direction,
     prime_caches::PrimeCachesProgress,
@@ -308,10 +308,10 @@ impl Analysis {
 
     /// Returns an edit to remove all newlines in the range, cleaning up minor
     /// stuff like trailing commas.
-    pub fn join_lines(&self, frange: FileRange) -> Cancellable<TextEdit> {
+    pub fn join_lines(&self, config: &JoinLinesConfig, frange: FileRange) -> Cancellable<TextEdit> {
         self.with_db(|db| {
             let parse = db.parse(frange.file_id);
-            join_lines::join_lines(&parse.tree(), frange.range)
+            join_lines::join_lines(&config, &parse.tree(), frange.range)
         })
     }
 

crates/rust-analyzer/src/config.rs

@@ -12,7 +12,7 @@ use std::{ffi::OsString, iter, path::PathBuf};
 use flycheck::FlycheckConfig;
 use ide::{
     AssistConfig, CompletionConfig, DiagnosticsConfig, HoverConfig, HoverDocFormat,
-    InlayHintsConfig,
+    InlayHintsConfig, JoinLinesConfig,
 };
 use ide_db::helpers::{
     insert_use::{ImportGranularity, InsertUseConfig, PrefixKind},
@@ -186,6 +186,13 @@ config_data! {
         /// Whether to show inlay type hints for variables.
         inlayHints_typeHints: bool          = "true",
 
+        /// Join lines inserts else between consecutive ifs.
+        joinLines_joinElseIf: bool = "true",
+        /// Join lines removes trailing commas.
+        joinLines_removeTrailingComma: bool = "true",
+        /// Join lines unwraps trivial blocks.
+        joinLines_unwrapTrivialBlock: bool = "true",
+
         /// Whether to show `Debug` lens. Only applies when
         /// `#rust-analyzer.lens.enable#` is set.
         lens_debug: bool            = "true",
@@ -752,6 +759,13 @@ impl Config {
             insert_use: self.insert_use_config(),
         }
     }
+    pub fn join_lines(&self) -> JoinLinesConfig {
+        JoinLinesConfig {
+            join_else_if: self.data.joinLines_joinElseIf,
+            remove_trailing_comma: self.data.joinLines_removeTrailingComma,
+            unwrap_trivial_blocks: self.data.joinLines_unwrapTrivialBlock,
+        }
+    }
     pub fn call_info_full(&self) -> bool {
         self.data.callInfo_full
     }

crates/rust-analyzer/src/handlers.rs

@@ -233,12 +233,15 @@ pub(crate) fn handle_join_lines(
     params: lsp_ext::JoinLinesParams,
 ) -> Result<Vec<lsp_types::TextEdit>> {
     let _p = profile::span("handle_join_lines");
+
+    let config = snap.config.join_lines();
     let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
+
     let mut res = TextEdit::default();
     for range in params.ranges {
         let range = from_proto::text_range(&line_index, range);
-        let edit = snap.analysis.join_lines(FileRange { file_id, range })?;
+        let edit = snap.analysis.join_lines(&config, FileRange { file_id, range })?;
         match res.union(edit) {
             Ok(()) => (),
             Err(_edit) => {
@@ -246,8 +249,8 @@ pub(crate) fn handle_join_lines(
             }
         }
     }
-    let res = to_proto::text_edit_vec(&line_index, res);
+
-    Ok(res)
+    Ok(to_proto::text_edit_vec(&line_index, res))
 }
 
 pub(crate) fn handle_on_enter(

docs/user/generated_config.adoc

@@ -281,6 +281,21 @@ site.
 --
 Whether to show inlay type hints for variables.
 --
+[[rust-analyzer.joinLines.joinElseIf]]rust-analyzer.joinLines.joinElseIf (default: `true`)::
++
+--
+Join lines inserts else between consecutive ifs.
+--
+[[rust-analyzer.joinLines.removeTrailingComma]]rust-analyzer.joinLines.removeTrailingComma (default: `true`)::
++
+--
+Join lines removes trailing commas.
+--
+[[rust-analyzer.joinLines.unwrapTrivialBlock]]rust-analyzer.joinLines.unwrapTrivialBlock (default: `true`)::
++
+--
+Join lines unwraps trivial blocks.
+--
 [[rust-analyzer.lens.debug]]rust-analyzer.lens.debug (default: `true`)::
 +
 --

editors/code/package.json

@@ -724,6 +724,21 @@
                     "default": true,
                     "type": "boolean"
                 },
+                "rust-analyzer.joinLines.joinElseIf": {
+                    "markdownDescription": "Join lines inserts else between consecutive ifs.",
+                    "default": true,
+                    "type": "boolean"
+                },
+                "rust-analyzer.joinLines.removeTrailingComma": {
+                    "markdownDescription": "Join lines removes trailing commas.",
+                    "default": true,
+                    "type": "boolean"
+                },
+                "rust-analyzer.joinLines.unwrapTrivialBlock": {
+                    "markdownDescription": "Join lines unwraps trivial blocks.",
+                    "default": true,
+                    "type": "boolean"
+                },
                 "rust-analyzer.lens.debug": {
                     "markdownDescription": "Whether to show `Debug` lens. Only applies when\n`#rust-analyzer.lens.enable#` is set.",
                     "default": true,