Repository: rust-analyzer/rust-analyzer
Pull request #264 (2018-12-08 17:28:46)

check for empty range when extending in comment
fix for #140 


Patch:
diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index 99bcf0666e..975afc1456 100644
--- a/crates/ra_analysis/src/imp.rs
+++ b/crates/ra_analysis/src/imp.rs
@@ -144,7 +144,7 @@ impl AnalysisImpl {
         } else {
             let files = &self.db.source_root(WORKSPACE).files;
 
-            /// Need to wrap Snapshot to provide `Clone` impl for `map_with`
+            /// Need to wrap Snapshot to provide `Clon` impl for `map_with`
             struct Snap(salsa::Snapshot<db::RootDatabase>);
             impl Clone for Snap {
                 fn clone(&self) -> Snap {
@@ -164,7 +164,7 @@ impl AnalysisImpl {
             .sweep(salsa::SweepStrategy::default().discard_values());
         Ok(query.search(&buf))
     }
-    /// This returns `Vec` because a module may be included from several places. We
+    /// This return `Vec`: a module may be included from several places. We
     /// don't handle this case yet though, so the Vec has length at most one.
     pub fn parent_module(&self, position: FilePosition) -> Cancelable<Vec<(FileId, FileSymbol)>> {
         let descr = match source_binder::module_from_position(&*self.db, position)? {
@@ -206,7 +206,7 @@ impl AnalysisImpl {
     pub fn approximately_resolve_symbol(
         &self,
         position: FilePosition,
-    ) -> Cancelable<Option<(TextRange, Vec<(FileId, FileSymbol)>)>> {
+    ) -> Cancelable<Vec<(FileId, FileSymbol)>> {
         let file = self.db.source_file(position.file_id);
         let syntax = file.syntax();
         if let Some(name_ref) = find_node_at_offset::<ast::NameRef>(syntax, position.offset) {
@@ -218,22 +218,20 @@ impl AnalysisImpl {
                 let scope = fn_descr.scope(&*self.db);
                 // First try to resolve the symbol locally
                 if let Some(entry) = scope.resolve_local_name(name_ref) {
-                    let vec = vec![(
+                    let mut vec = vec![];
+                    vec.push((
                         position.file_id,
                         FileSymbol {
                             name: entry.name().clone(),
                             node_range: entry.ptr().range(),
                             kind: NAME,
                         },
-                    )];
-                    return Ok(Some((name_ref.syntax().range(), vec)));
+                    ));
+                    return Ok(vec);
                 };
             }
             // If that fails try the index based approach.
-            return Ok(Some((
-                name_ref.syntax().range(),
-                self.index_resolve(name_ref)?,
-            )));
+            return self.index_resolve(name_ref);
         }
         if let Some(name) = find_node_at_offset::<ast::Name>(syntax, position.offset) {
             if let Some(module) = name.syntax().parent().and_then(ast::Module::cast) {
@@ -250,7 +248,7 @@ impl AnalysisImpl {
                                     node_range: TextRange::offset_len(0.into(), 0.into()),
                                     kind: MODULE,
                                 };
-                                return Ok(Some((name.syntax().range(), vec![(file_id, symbol)])));
+                                return Ok(vec![(file_id, symbol)]);
                             }
                         }
                         _ => (),
@@ -258,10 +256,7 @@ impl AnalysisImpl {
                 }
             }
         }
-        let range =
-            ctry!(ra_syntax::algo::find_leaf_at_offset(syntax, position.offset).left_biased())
-                .range();
-        Ok(Some((range, vec![])))
+        Ok(vec![])
     }
 
     pub fn find_all_refs(&self, position: FilePosition) -> Cancelable<Vec<(FileId, TextRange)>> {
@@ -323,19 +318,6 @@ impl AnalysisImpl {
 
         Ok(symbol.docs(&file))
     }
-    pub fn doc_text_for(&self, file_id: FileId, symbol: FileSymbol) -> Cancelable<Option<String>> {
-        let file = self.db.source_file(file_id);
-        let result = match (symbol.description(&file), symbol.docs(&file)) {
-            (Some(desc), Some(docs)) => {
-                Some("```rust\n".to_string() + &*desc + "\n```\n\n" + &*docs)
-            }
-            (Some(desc), None) => Some("```rust\n".to_string() + &*desc + "\n```"),
-            (None, Some(docs)) => Some(docs),
-            _ => None,
-        };
-
-        Ok(result)
-    }
 
     pub fn diagnostics(&self, file_id: FileId) -> Cancelable<Vec<Diagnostic>> {
         let syntax = self.db.source_file(file_id);
diff --git a/crates/ra_analysis/src/lib.rs b/crates/ra_analysis/src/lib.rs
index d33f3e4ca8..4b8b108167 100644
--- a/crates/ra_analysis/src/lib.rs
+++ b/crates/ra_analysis/src/lib.rs
@@ -236,7 +236,7 @@ impl Analysis {
     pub fn approximately_resolve_symbol(
         &self,
         position: FilePosition,
-    ) -> Cancelable<Option<(TextRange, Vec<(FileId, FileSymbol)>)>> {
+    ) -> Cancelable<Vec<(FileId, FileSymbol)>> {
         self.imp.approximately_resolve_symbol(position)
     }
     pub fn find_all_refs(&self, position: FilePosition) -> Cancelable<Vec<(FileId, TextRange)>> {
@@ -249,9 +249,6 @@ impl Analysis {
     ) -> Cancelable<Option<String>> {
         self.imp.doc_comment_for(file_id, symbol)
     }
-    pub fn doc_text_for(&self, file_id: FileId, symbol: FileSymbol) -> Cancelable<Option<String>> {
-        self.imp.doc_text_for(file_id, symbol)
-    }
     pub fn parent_module(&self, position: FilePosition) -> Cancelable<Vec<(FileId, FileSymbol)>> {
         self.imp.parent_module(position)
     }
diff --git a/crates/ra_editor/src/extend_selection.rs b/crates/ra_editor/src/extend_selection.rs
index 8f11d5364b..b4227716d5 100644
--- a/crates/ra_editor/src/extend_selection.rs
+++ b/crates/ra_editor/src/extend_selection.rs
@@ -54,7 +54,12 @@ fn extend_single_word_in_comment(leaf: SyntaxNodeRef, offset: TextUnit) -> Optio
     let from: TextUnit = (start_idx + 1).into();
     let to: TextUnit = (cursor_position + end_idx).into();
 
-    Some(TextRange::from_to(from, to) + leaf.range().start())
+    let range = TextRange::from_to(from, to);
+    if range.is_empty() {
+        None
+    } else {
+        Some(range + leaf.range().start())
+    }
 }
 
 fn extend_ws(root: SyntaxNodeRef, ws: SyntaxNodeRef, offset: TextUnit) -> TextRange {
@@ -181,6 +186,20 @@ fn bar(){}
     "#,
             &["// 1 + 1", "// fn foo() {\n// 1 + 1\n// }"],
         );
+
+        do_check(
+            r#"
+// #[derive(Debug, Clone, Copy, PartialEq, Eq)]
+// pub enum Direction {
+//  <|>   Next,
+//     Prev
+// }
+"#,
+            &[
+                "//     Next,",
+                "// #[derive(Debug, Clone, Copy, PartialEq, Eq)]\n// pub enum Direction {\n//     Next,\n//     Prev\n// }",
+            ],
+        );
     }
 
     #[test]
diff --git a/crates/ra_editor/src/symbols.rs b/crates/ra_editor/src/symbols.rs
index 9e25decfbc..6d3b0514a2 100644
--- a/crates/ra_editor/src/symbols.rs
+++ b/crates/ra_editor/src/symbols.rs
@@ -50,61 +50,6 @@ impl FileSymbol {
             })
             .nth(0)
     }
-    /// Get a description of this node.
-    ///
-    /// e.g. `struct Name`, `enum Name`, `fn Name`
-    pub fn description(&self, file: &SourceFileNode) -> Option<String> {
-        // TODO: After type inference is done, add type information to improve the output
-        file.syntax()
-            .descendants()
-            .filter(|node| node.kind() == self.kind && node.range() == self.node_range)
-            .filter_map(|node: SyntaxNodeRef| {
-                // TODO: Refactor to be have less repetition
-                visitor()
-                    .visit(|node: ast::FnDef| {
-                        let mut string = "fn ".to_string();
-                        node.name()?.syntax().text().push_to(&mut string);
-                        Some(string)
-                    })
-                    .visit(|node: ast::StructDef| {
-                        let mut string = "struct ".to_string();
-                        node.name()?.syntax().text().push_to(&mut string);
-                        Some(string)
-                    })
-                    .visit(|node: ast::EnumDef| {
-                        let mut string = "enum ".to_string();
-                        node.name()?.syntax().text().push_to(&mut string);
-                        Some(string)
-                    })
-                    .visit(|node: ast::TraitDef| {
-                        let mut string = "trait ".to_string();
-                        node.name()?.syntax().text().push_to(&mut string);
-                        Some(string)
-                    })
-                    .visit(|node: ast::Module| {
-                        let mut string = "mod ".to_string();
-                        node.name()?.syntax().text().push_to(&mut string);
-                        Some(string)
-                    })
-                    .visit(|node: ast::TypeDef| {
-                        let mut string = "type ".to_string();
-                        node.name()?.syntax().text().push_to(&mut string);
-                        Some(string)
-                    })
-                    .visit(|node: ast::ConstDef| {
-                        let mut string = "const ".to_string();
-                        node.name()?.syntax().text().push_to(&mut string);
-                        Some(string)
-                    })
-                    .visit(|node: ast::StaticDef| {
-                        let mut string = "static ".to_string();
-                        node.name()?.syntax().text().push_to(&mut string);
-                        Some(string)
-                    })
-                    .accept(node)?
-            })
-            .nth(0)
-    }
 }
 
 pub fn file_symbols(file: &SourceFileNode) -> Vec<FileSymbol> {
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index f18a1305da..af21254e43 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -204,10 +204,7 @@ pub fn handle_goto_definition(
 ) -> Result<Option<req::GotoDefinitionResponse>> {
     let position = params.try_conv_with(&world)?;
     let mut res = Vec::new();
-    for (file_id, symbol) in match world.analysis().approximately_resolve_symbol(position)? {
-        None => return Ok(None),
-        Some(it) => it.1,
-    } {
+    for (file_id, symbol) in world.analysis().approximately_resolve_symbol(position)? {
         let line_index = world.analysis().file_line_index(file_id);
         let location = to_location(file_id, symbol.node_range, &world, &line_index)?;
         res.push(location)
@@ -507,30 +504,26 @@ pub fn handle_hover(
     world: ServerWorld,
     params: req::TextDocumentPositionParams,
 ) -> Result<Option<Hover>> {
-    // TODO: Cut down on number of allocations
     let position = params.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(position.file_id);
-    let (range, resolved) = match world.analysis().approximately_resolve_symbol(position)? {
-        None => return Ok(None),
-        Some(it) => it,
-    };
-    let mut result = Vec::new();
-    for (file_id, symbol) in resolved {
-        if let Some(docs) = world.analysis().doc_text_for(file_id, symbol)? {
-            result.push(docs);
+
+    for (file_id, symbol) in world.analysis().approximately_resolve_symbol(position)? {
+        let range = symbol.node_range.conv_with(&line_index);
+        let comment = world.analysis.doc_comment_for(file_id, symbol)?;
+
+        if comment.is_some() {
+            let contents = HoverContents::Scalar(MarkedString::String(comment.unwrap()));
+
+            return Ok(Some(Hover {
+                contents,
+                range: Some(range),
+            }));
         }
     }
-    let range = range.conv_with(&line_index);
-    if result.len() > 0 {
-        return Ok(Some(Hover {
-            contents: HoverContents::Scalar(MarkedString::String(result.join("\n\n---\n"))),
-            range: Some(range),
-        }));
-    }
+
     Ok(None)
 }
 
-/// Test doc comment
 pub fn handle_prepare_rename(
     world: ServerWorld,
     params: req::TextDocumentPositionParams,
diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron
index eed67637e0..53cd2118f3 100644
--- a/crates/ra_syntax/src/grammar.ron
+++ b/crates/ra_syntax/src/grammar.ron
@@ -25,9 +25,9 @@ Grammar(
         ["^", "CARET"],
         ["%", "PERCENT"],
     ],
-    // Tokens for which the longest match must be chosen (e.g. `..` is a DOTDOT, but `.` is a DOT)
+    // TODO: Confirm surmision: the tokens which cannot be recorded in a single UTF-8 byte
     multi_byte_tokens: [
-        [".", "DOT"],
+        [".", "DOT"], // Note: DOT is here because <TODO: REASON>
         ["..", "DOTDOT"],
         ["...", "DOTDOTDOT"],
         ["..=", "DOTDOTEQ"],
diff --git a/crates/ra_syntax/src/syntax_kinds/generated.rs b/crates/ra_syntax/src/syntax_kinds/generated.rs
index 3a869ad34b..c43a8bf653 100644
--- a/crates/ra_syntax/src/syntax_kinds/generated.rs
+++ b/crates/ra_syntax/src/syntax_kinds/generated.rs
@@ -560,3 +560,4 @@ impl SyntaxKind {
         Some(tok)
     }
 }
+
diff --git a/crates/ra_syntax/src/syntax_kinds/generated.rs.tera b/crates/ra_syntax/src/syntax_kinds/generated.rs.tera
index 21f9444b1e..c37e577022 100644
--- a/crates/ra_syntax/src/syntax_kinds/generated.rs.tera
+++ b/crates/ra_syntax/src/syntax_kinds/generated.rs.tera
@@ -75,3 +75,4 @@ impl SyntaxKind {
         Some(tok)
     }
 }
+


Test patch:
diff --git a/crates/ra_analysis/tests/tests.rs b/crates/ra_analysis/tests/tests.rs
index 05ad687aeb..4ce2c5c85d 100644
--- a/crates/ra_analysis/tests/tests.rs
+++ b/crates/ra_analysis/tests/tests.rs
@@ -21,9 +21,9 @@ fn approximate_resolve_works_in_items() {
     ",
     );
 
-    let symbols = analysis.approximately_resolve_symbol(pos).unwrap().unwrap();
+    let symbols = analysis.approximately_resolve_symbol(pos).unwrap();
     assert_eq_dbg(
-        r#"([23; 26), [(FileId(1), FileSymbol { name: "Foo", node_range: [0; 11), kind: STRUCT_DEF })])"#,
+        r#"[(FileId(1), FileSymbol { name: "Foo", node_range: [0; 11), kind: STRUCT_DEF })]"#,
         &symbols,
     );
 }
@@ -39,9 +39,9 @@ fn test_resolve_module() {
     ",
     );
 
-    let symbols = analysis.approximately_resolve_symbol(pos).unwrap().unwrap();
+    let symbols = analysis.approximately_resolve_symbol(pos).unwrap();
     assert_eq_dbg(
-        r#"([4; 7), [(FileId(2), FileSymbol { name: "foo", node_range: [0; 0), kind: MODULE })])"#,
+        r#"[(FileId(2), FileSymbol { name: "foo", node_range: [0; 0), kind: MODULE })]"#,
         &symbols,
     );
 
@@ -54,9 +54,9 @@ fn test_resolve_module() {
     ",
     );
 
-    let symbols = analysis.approximately_resolve_symbol(pos).unwrap().unwrap();
+    let symbols = analysis.approximately_resolve_symbol(pos).unwrap();
     assert_eq_dbg(
-        r#"([4; 7), [(FileId(2), FileSymbol { name: "foo", node_range: [0; 0), kind: MODULE })])"#,
+        r#"[(FileId(2), FileSymbol { name: "foo", node_range: [0; 0), kind: MODULE })]"#,
         &symbols,
     );
 }