6 files changed, 155 insertions, 88 deletions

diff --git a/crates/ra_editor/Cargo.toml b/crates/ra_editor/Cargo.toml
index 40e3254ff..91cefc8d7 100644
--- a/crates/ra_editor/Cargo.toml
+++ b/crates/ra_editor/Cargo.toml
@@ -8,6 +8,7 @@ publish = false
 itertools = "0.7.8"
 superslice = "0.1.0"
 join_to_string = "0.1.1"
+rustc-hash = "1.0"
 
 ra_syntax = { path = "../ra_syntax" }
 
diff --git a/crates/ra_editor/src/completion.rs b/crates/ra_editor/src/completion.rs
index 570d72d66..20b8484b3 100644
--- a/crates/ra_editor/src/completion.rs
+++ b/crates/ra_editor/src/completion.rs
@@ -1,4 +1,4 @@
-use std::collections::{HashSet, HashMap};
+use rustc_hash::{FxHashMap, FxHashSet};
 
 use ra_syntax::{
     File, TextUnit, AstNode, SyntaxNodeRef, SyntaxKind::*,
@@ -96,7 +96,7 @@ fn complete_name_ref(file: &File, name_ref: ast::NameRef, acc: &mut Vec<Completi
 }
 
 fn param_completions(ctx: SyntaxNodeRef, acc: &mut Vec<CompletionItem>) {
-    let mut params = HashMap::new();
+    let mut params = FxHashMap::default();
     for node in ctx.ancestors() {
         let _ = visitor_ctx(&mut params)
             .visit::<ast::Root, _>(process)
@@ -114,7 +114,7 @@ fn param_completions(ctx: SyntaxNodeRef, acc: &mut Vec<CompletionItem>) {
             })
         });
 
-    fn process<'a, N: ast::FnDefOwner<'a>>(node: N, params: &mut HashMap<String, (u32, ast::Param<'a>)>) {
+    fn process<'a, N: ast::FnDefOwner<'a>>(node: N, params: &mut FxHashMap<String, (u32, ast::Param<'a>)>) {
         node.functions()
             .filter_map(|it| it.param_list())
             .flat_map(|it| it.params())
@@ -232,7 +232,7 @@ fn complete_mod_item_snippets(acc: &mut Vec<CompletionItem>) {
 }
 
 fn complete_fn(name_ref: ast::NameRef, scopes: &FnScopes, acc: &mut Vec<CompletionItem>) {
-    let mut shadowed = HashSet::new();
+    let mut shadowed = FxHashSet::default();
     acc.extend(
         scopes.scope_chain(name_ref.syntax())
             .flat_map(|scope| scopes.entries(scope).iter())
diff --git a/crates/ra_editor/src/folding_ranges.rs b/crates/ra_editor/src/folding_ranges.rs
index 733512368..3aabd54ae 100644
--- a/crates/ra_editor/src/folding_ranges.rs
+++ b/crates/ra_editor/src/folding_ranges.rs
@@ -1,4 +1,4 @@
-use std::collections::HashSet;
+use rustc_hash::FxHashSet;
 
 use ra_syntax::{
     File, TextRange, SyntaxNodeRef,
@@ -20,7 +20,7 @@ pub struct Fold {
 
 pub fn folding_ranges(file: &File) -> Vec<Fold> {
     let mut res = vec![];
-    let mut visited = HashSet::new();
+    let mut visited = FxHashSet::default();
 
     for node in file.syntax().descendants() {
         if visited.contains(&node) {
@@ -56,7 +56,7 @@ pub fn folding_ranges(file: &File) -> Vec<Fold> {
 fn contiguous_range_for<'a>(
     kind: SyntaxKind,
     node: SyntaxNodeRef<'a>,
-    visited: &mut HashSet<SyntaxNodeRef<'a>>,
+    visited: &mut FxHashSet<SyntaxNodeRef<'a>>,
 ) -> Option<TextRange> {
     visited.insert(node);
 
diff --git a/crates/ra_editor/src/lib.rs b/crates/ra_editor/src/lib.rs
index fe0045378..710afc65d 100644
--- a/crates/ra_editor/src/lib.rs
+++ b/crates/ra_editor/src/lib.rs
@@ -2,6 +2,7 @@ extern crate ra_syntax;
 extern crate superslice;
 extern crate itertools;
 extern crate join_to_string;
+extern crate rustc_hash;
 #[cfg(test)]
 #[macro_use]
 extern crate test_utils as _test_utils;
@@ -88,7 +89,6 @@ pub fn highlight(file: &File) -> Vec<HighlightedRange> {
     let mut res = Vec::new();
     for node in file.syntax().descendants() {
         let tag = match node.kind() {
-            ERROR => "error",
             COMMENT | DOC_COMMENT => "comment",
             STRING | RAW_STRING | RAW_BYTE_STRING | BYTE_STRING => "string",
             ATTR => "attribute",
@@ -108,21 +108,10 @@ pub fn highlight(file: &File) -> Vec<HighlightedRange> {
 }
 
 pub fn diagnostics(file: &File) -> Vec<Diagnostic> {
-    let mut res = Vec::new();
-
-    for node in file.syntax().descendants() {
-        if node.kind() == ERROR {
-            res.push(Diagnostic {
-                range: node.range(),
-                msg: "Syntax Error".to_string(),
-            });
-        }
-    }
-    res.extend(file.errors().into_iter().map(|err| Diagnostic {
+    file.errors().into_iter().map(|err| Diagnostic {
         range: TextRange::offset_len(err.offset, 1.into()),
-        msg: err.msg,
-    }));
-    res
+        msg: "Syntax Error: ".to_string() + &err.msg,
+    }).collect()
 }
 
 pub fn syntax_tree(file: &File) -> String {
diff --git a/crates/ra_editor/src/scope/fn_scope.rs b/crates/ra_editor/src/scope/fn_scope.rs
index 65d85279f..9a48bda02 100644
--- a/crates/ra_editor/src/scope/fn_scope.rs
+++ b/crates/ra_editor/src/scope/fn_scope.rs
@@ -1,7 +1,5 @@
-use std::{
-    fmt,
-    collections::HashMap,
-};
+use std::fmt;
+use rustc_hash::FxHashMap;
 
 use ra_syntax::{
     SyntaxNodeRef, SyntaxNode, SmolStr, AstNode,
@@ -15,7 +13,7 @@ type ScopeId = usize;
 pub struct FnScopes {
     pub self_param: Option<SyntaxNode>,
     scopes: Vec<ScopeData>,
-    scope_for: HashMap<SyntaxNode, ScopeId>,
+    scope_for: FxHashMap<SyntaxNode, ScopeId>,
 }
 
 impl FnScopes {
@@ -25,7 +23,7 @@ impl FnScopes {
                 .and_then(|it| it.self_param())
                 .map(|it| it.syntax().owned()),
             scopes: Vec::new(),
-            scope_for: HashMap::new()
+            scope_for: FxHashMap::default()
         };
         let root = scopes.root_scope();
         scopes.add_params_bindings(root, fn_def.param_list());
@@ -242,9 +240,9 @@ struct ScopeData {
 }
 
 pub fn resolve_local_name<'a>(name_ref: ast::NameRef, scopes: &'a FnScopes) -> Option<&'a ScopeEntry> {
-    use std::collections::HashSet;
+    use rustc_hash::FxHashSet;
 
-    let mut shadowed = HashSet::new();
+    let mut shadowed = FxHashSet::default();
     let ret = scopes.scope_chain(name_ref.syntax())
         .flat_map(|scope| scopes.entries(scope).iter())
         .filter(|entry| shadowed.insert(entry.name()))
diff --git a/crates/ra_editor/src/typing.rs b/crates/ra_editor/src/typing.rs
index 3384389d1..1dc658f9b 100644
--- a/crates/ra_editor/src/typing.rs
+++ b/crates/ra_editor/src/typing.rs
@@ -30,6 +30,7 @@ pub fn join_lines(file: &File, range: TextRange) -> LocalEdit {
     } else {
         range
     };
+
     let node = find_covering_node(file.syntax(), range);
     let mut edit = EditBuilder::new();
     for node in node.descendants() {
@@ -57,14 +58,19 @@ pub fn join_lines(file: &File, range: TextRange) -> LocalEdit {
 }
 
 pub fn on_enter(file: &File, offset: TextUnit) -> Option<LocalEdit> {
-    let comment = find_leaf_at_offset(file.syntax(), offset).left_biased().filter(|it| it.kind() == COMMENT)?;
-    let prefix = comment_preffix(comment)?;
-    if offset < comment.range().start() + TextUnit::of_str(prefix) {
+    let comment = find_leaf_at_offset(file.syntax(), offset).left_biased().and_then(|it| ast::Comment::cast(it))?;
+
+    if let ast::CommentFlavor::Multiline = comment.flavor() {
+        return None;
+    }
+
+    let prefix = comment.prefix();
+    if offset < comment.syntax().range().start() + TextUnit::of_str(prefix) + TextUnit::from(1) {
         return None;
     }
 
-    let indent = node_indent(file, comment)?;
-    let inserted = format!("\n{}{}", indent, prefix);
+    let indent = node_indent(file, comment.syntax())?;
+    let inserted = format!("\n{}{} ", indent, prefix);
     let cursor_position = offset + TextUnit::of_str(&inserted);
     let mut edit = EditBuilder::new();
     edit.insert(offset, inserted);
@@ -74,20 +80,6 @@ pub fn on_enter(file: &File, offset: TextUnit) -> Option<LocalEdit> {
     })
 }
 
-fn comment_preffix(comment: SyntaxNodeRef) -> Option<&'static str> {
-    let text = comment.leaf_text().unwrap();
-    let res = if text.starts_with("///") {
-        "/// "
-    } else if text.starts_with("//!") {
-        "//! "
-    } else if text.starts_with("//") {
-        "// "
-    } else {
-        return None;
-    };
-    Some(res)
-}
-
 fn node_indent<'a>(file: &'a File, node: SyntaxNodeRef) -> Option<&'a str> {
     let ws = match find_leaf_at_offset(file.syntax(), node.range().start()) {
         LeafAtOffset::Between(l, r) => {
@@ -139,41 +131,60 @@ fn remove_newline(
     node_text: &str,
     offset: TextUnit,
 ) {
-    if node.kind() == WHITESPACE && node_text.bytes().filter(|&b| b == b'\n').count() == 1 {
-        if join_single_expr_block(edit, node).is_some() {
-            return
-        }
-        match (node.prev_sibling(), node.next_sibling()) {
-            (Some(prev), Some(next)) => {
-                let range = TextRange::from_to(prev.range().start(), node.range().end());
-                if is_trailing_comma(prev.kind(), next.kind()) {
-                    edit.delete(range);
-                } else if no_space_required(prev.kind(), next.kind()) {
-                    edit.delete(node.range());
-                } else if prev.kind() == COMMA && next.kind() == R_CURLY {
-                    edit.replace(range, " ".to_string());
-                } else {
-                    edit.replace(
-                        node.range(),
-                        compute_ws(prev, next).to_string(),
-                    );
-                }
-                return;
-            }
-            _ => (),
-        }
+    if node.kind() != WHITESPACE || node_text.bytes().filter(|&b| b == b'\n').count() != 1 {
+        // The node is either the first or the last in the file
+        let suff = &node_text[TextRange::from_to(
+            offset - node.range().start() + TextUnit::of_char('\n'),
+            TextUnit::of_str(node_text),
+        )];
+        let spaces = suff.bytes().take_while(|&b| b == b' ').count();
+
+        edit.replace(
+            TextRange::offset_len(offset, ((spaces + 1) as u32).into()),
+            " ".to_string(),
+        );
+        return;
     }
 
-    let suff = &node_text[TextRange::from_to(
-        offset - node.range().start() + TextUnit::of_char('\n'),
-        TextUnit::of_str(node_text),
-    )];
-    let spaces = suff.bytes().take_while(|&b| b == b' ').count();
+    // Special case that turns something like:
+    //
+    // ```
+    // my_function({<|>
+    //    <some-expr>
+    // })
+    // ```
+    //
+    // into `my_function(<some-expr>)`
+    if join_single_expr_block(edit, node).is_some() {
+        return
+    }
 
-    edit.replace(
-        TextRange::offset_len(offset, ((spaces + 1) as u32).into()),
-        " ".to_string(),
-    );
+    // The node is between two other nodes
+    let prev = node.prev_sibling().unwrap();
+    let next = node.next_sibling().unwrap();
+    if is_trailing_comma(prev.kind(), next.kind()) {
+        // Removes: trailing comma, newline (incl. surrounding whitespace)
+        edit.delete(TextRange::from_to(prev.range().start(), node.range().end()));
+    } else if prev.kind() == COMMA && next.kind() == R_CURLY {
+        // Removes: comma, newline (incl. surrounding whitespace)
+        // Adds: a single whitespace
+        edit.replace(
+            TextRange::from_to(prev.range().start(), node.range().end()),
+            " ".to_string()
+        );
+    } else if let (Some(_), Some(next)) = (ast::Comment::cast(prev), ast::Comment::cast(next)) {
+        // Removes: newline (incl. surrounding whitespace), start of the next comment
+        edit.delete(TextRange::from_to(
+            node.range().start(),
+            next.syntax().range().start() + TextUnit::of_str(next.prefix())
+        ));
+    } else {
+        // Remove newline but add a computed amount of whitespace characters
+        edit.replace(
+            node.range(),
+            compute_ws(prev, next).to_string(),
+        );
+    }
 }
 
 fn is_trailing_comma(left: SyntaxKind, right: SyntaxKind) -> bool {
@@ -183,13 +194,6 @@ fn is_trailing_comma(left: SyntaxKind, right: SyntaxKind) -> bool {
     }
 }
 
-fn no_space_required(left: SyntaxKind, right: SyntaxKind) -> bool {
-    match (left, right) {
-       (_, DOT) => true,
-        _ => false
-    }
-}
-
 fn join_single_expr_block(
     edit: &mut EditBuilder,
     node: SyntaxNodeRef,
@@ -231,6 +235,7 @@ fn compute_ws(left: SyntaxNodeRef, right: SyntaxNodeRef) -> &'static str {
     }
     match right.kind() {
         R_PAREN | R_BRACK => return "",
+        DOT => return "",
         _ => (),
     }
     " "
@@ -291,6 +296,80 @@ fn foo() {
 }");
     }
 
+    #[test]
+    fn test_join_lines_normal_comments() {
+        check_join_lines(r"
+fn foo() {
+    // Hello<|>
+    // world!
+}
+", r"
+fn foo() {
+    // Hello<|> world!
+}
+");
+    }
+
+    #[test]
+    fn test_join_lines_doc_comments() {
+        check_join_lines(r"
+fn foo() {
+    /// Hello<|>
+    /// world!
+}
+", r"
+fn foo() {
+    /// Hello<|> world!
+}
+");
+    }
+
+    #[test]
+    fn test_join_lines_mod_comments() {
+        check_join_lines(r"
+fn foo() {
+    //! Hello<|>
+    //! world!
+}
+", r"
+fn foo() {
+    //! Hello<|> world!
+}
+");
+    }
+
+    #[test]
+    fn test_join_lines_multiline_comments_1() {
+        check_join_lines(r"
+fn foo() {
+    // Hello<|>
+    /* world! */
+}
+", r"
+fn foo() {
+    // Hello<|> world! */
+}
+");
+    }
+
+    #[test]
+    fn test_join_lines_multiline_comments_2() {
+        check_join_lines(r"
+fn foo() {
+    // The<|>
+    /* quick
+    brown
+    fox! */
+}
+", r"
+fn foo() {
+    // The<|> quick
+    brown
+    fox! */
+}
+");
+    }
+
     fn check_join_lines_sel(before: &str, after: &str) {
         let (sel, before) = extract_range(before);
         let file = File::parse(&before);