From abdf725c558f860b47883a6843bfd2a5a7c633bd Mon Sep 17 00:00:00 2001
From: IceSentry <c.giguere42@gmail.com>
Date: Sun, 12 Apr 2020 21:29:14 -0400
Subject: Fix double comma when merge imports on second line

This fixes the a bug when merging imports from the second line when it already has a comma it would previously insert a comma.

There's probably a better way to check for a COMMA.

This also ends up with a weird indentation, but rust-fmt can easily deal with it so I'm not sure how to resolve that.

Closes #3832
---
 crates/ra_assists/src/handlers/merge_imports.rs | 40 +++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 3 deletions(-)

(limited to 'crates/ra_assists')

diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs
index 0958f52f1..ab1fb2a0b 100644
--- a/crates/ra_assists/src/handlers/merge_imports.rs
+++ b/crates/ra_assists/src/handlers/merge_imports.rs
@@ -3,7 +3,8 @@ use std::iter::successors;
 use ra_syntax::{
     algo::{neighbor, SyntaxRewriter},
     ast::{self, edit::AstNodeEdit, make},
-    AstNode, Direction, InsertPosition, SyntaxElement, T,
+    AstNode, Direction, InsertPosition, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxToken, Token,
+    T,
 };
 
 use crate::{Assist, AssistCtx, AssistId};
@@ -72,9 +73,24 @@ fn try_merge_trees(old: &ast::UseTree, new: &ast::UseTree) -> Option<ast::UseTre
     let lhs = old.split_prefix(&lhs_prefix);
     let rhs = new.split_prefix(&rhs_prefix);
 
+    let mut should_insert_comma = true;
+
+    lhs.syntax()
+        .last_child()
+        .and_then(|child| child.children().last())
+        .and_then(|last| last.next_sibling_or_token())
+        .map(|next_sibling| {
+            // FIXME: there's probably a better way to check if it's a COMMA
+            if let NodeOrToken::Token(maybe_comma) = next_sibling {
+                should_insert_comma = maybe_comma.to_string() != ",";
+            }
+        });
+
     let mut to_insert: Vec<SyntaxElement> = Vec::new();
-    to_insert.push(make::token(T![,]).into());
-    to_insert.push(make::tokens::single_space().into());
+    if should_insert_comma {
+        to_insert.push(make::token(T![,]).into());
+        to_insert.push(make::tokens::single_space().into());
+    }
     to_insert.extend(
         rhs.use_tree_list()?
             .syntax()
@@ -247,4 +263,22 @@ use {
 ",
         );
     }
+
+    #[test]
+    fn test_double_comma() {
+        check_assist(
+            merge_imports,
+            r"
+use hyper::service::make_service_fn;
+use hyper::<|>{
+    StatusCode,
+};
+",
+            r"
+use hyper::{<|>
+    StatusCode,
+service::make_service_fn};
+",
+        )
+    }
 }
-- 
cgit v1.2.3


From 2e279ca031a52e90cd19dff513ee9d5b80cee8aa Mon Sep 17 00:00:00 2001
From: IceSentry <c.giguere42@gmail.com>
Date: Sun, 12 Apr 2020 21:34:01 -0400
Subject: Generalize test and clean up imports

---
 crates/ra_assists/src/handlers/merge_imports.rs | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

(limited to 'crates/ra_assists')

diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs
index ab1fb2a0b..b3101f3ab 100644
--- a/crates/ra_assists/src/handlers/merge_imports.rs
+++ b/crates/ra_assists/src/handlers/merge_imports.rs
@@ -3,8 +3,7 @@ use std::iter::successors;
 use ra_syntax::{
     algo::{neighbor, SyntaxRewriter},
     ast::{self, edit::AstNodeEdit, make},
-    AstNode, Direction, InsertPosition, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxToken, Token,
-    T,
+    AstNode, Direction, InsertPosition, NodeOrToken, SyntaxElement, T,
 };
 
 use crate::{Assist, AssistCtx, AssistId};
@@ -269,15 +268,15 @@ use {
         check_assist(
             merge_imports,
             r"
-use hyper::service::make_service_fn;
-use hyper::<|>{
-    StatusCode,
+use foo::bar::baz;
+use foo::<|>{
+    FooBar,
 };
 ",
             r"
-use hyper::{<|>
-    StatusCode,
-service::make_service_fn};
+use foo::{<|>
+    FooBar,
+bar::baz};
 ",
         )
     }
-- 
cgit v1.2.3


From ed0eedb1dda9ae2a46735bb469aeaf5cf8a28601 Mon Sep 17 00:00:00 2001
From: IceSentry <c.giguere42@gmail.com>
Date: Mon, 13 Apr 2020 13:59:30 -0400
Subject: Fix PR

---
 crates/ra_assists/src/handlers/merge_imports.rs | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

(limited to 'crates/ra_assists')

diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs
index b3101f3ab..ef0ce0586 100644
--- a/crates/ra_assists/src/handlers/merge_imports.rs
+++ b/crates/ra_assists/src/handlers/merge_imports.rs
@@ -1,9 +1,9 @@
 use std::iter::successors;
 
 use ra_syntax::{
-    algo::{neighbor, SyntaxRewriter},
+    algo::{neighbor, skip_trivia_token, SyntaxRewriter},
     ast::{self, edit::AstNodeEdit, make},
-    AstNode, Direction, InsertPosition, NodeOrToken, SyntaxElement, T,
+    AstNode, Direction, InsertPosition, SyntaxElement, T,
 };
 
 use crate::{Assist, AssistCtx, AssistId};
@@ -72,18 +72,12 @@ fn try_merge_trees(old: &ast::UseTree, new: &ast::UseTree) -> Option<ast::UseTre
     let lhs = old.split_prefix(&lhs_prefix);
     let rhs = new.split_prefix(&rhs_prefix);
 
-    let mut should_insert_comma = true;
-
-    lhs.syntax()
-        .last_child()
-        .and_then(|child| child.children().last())
-        .and_then(|last| last.next_sibling_or_token())
-        .map(|next_sibling| {
-            // FIXME: there's probably a better way to check if it's a COMMA
-            if let NodeOrToken::Token(maybe_comma) = next_sibling {
-                should_insert_comma = maybe_comma.to_string() != ",";
-            }
-        });
+    let should_insert_comma = lhs
+        .use_tree_list()?
+        .r_curly_token()
+        .and_then(|it| skip_trivia_token(it.prev_token()?, Direction::Prev))
+        .map(|it| it.kind() != T![,])
+        .unwrap_or(true);
 
     let mut to_insert: Vec<SyntaxElement> = Vec::new();
     if should_insert_comma {
-- 
cgit v1.2.3