From 8a04372fec5f26a0650395a1e420fea062b3a7ab Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 20 Apr 2020 16:34:01 +0200 Subject: Fix panic in split_imports assist The fix is admittedly quit literally just papering over. Long-term, I see two more principled approaches: * we switch to a fully tree-based impl, without parse . to_string step; with this approach, there shouldn't be any panics. The results might be nonsensical, but so was the original input. * we preserve the invariant that re-parsing constructed node is an identity, and make all the `make_xxx` method return an `Option`. closes #4044 --- crates/ra_assists/src/handlers/split_import.rs | 7 ++++++- crates/ra_syntax/src/algo.rs | 8 ++++++-- crates/ra_syntax/src/ast/edit.rs | 6 +++++- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/ra_assists/src/handlers/split_import.rs b/crates/ra_assists/src/handlers/split_import.rs index d9244f22d..f25826796 100644 --- a/crates/ra_assists/src/handlers/split_import.rs +++ b/crates/ra_assists/src/handlers/split_import.rs @@ -37,7 +37,7 @@ pub(crate) fn split_import(ctx: AssistCtx) -> Option { #[cfg(test)] mod tests { - use crate::helpers::{check_assist, check_assist_target}; + use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target}; use super::*; @@ -63,4 +63,9 @@ mod tests { fn split_import_target() { check_assist_target(split_import, "use crate::<|>db::{RootDatabase, FileSymbol}", "::"); } + + #[test] + fn issue4044() { + check_assist_not_applicable(split_import, "use crate::<|>:::self;") + } } diff --git a/crates/ra_syntax/src/algo.rs b/crates/ra_syntax/src/algo.rs index ea41bf85d..06df8495c 100644 --- a/crates/ra_syntax/src/algo.rs +++ b/crates/ra_syntax/src/algo.rs @@ -10,8 +10,8 @@ use ra_text_edit::TextEditBuilder; use rustc_hash::FxHashMap; use crate::{ - AstNode, Direction, NodeOrToken, SyntaxElement, SyntaxNode, SyntaxNodePtr, SyntaxToken, - TextRange, TextUnit, + AstNode, Direction, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxNodePtr, + SyntaxToken, TextRange, TextUnit, }; /// Returns ancestors of the node at the offset, sorted by length. This should @@ -90,6 +90,10 @@ pub fn neighbor(me: &T, direction: Direction) -> Option { me.syntax().siblings(direction).skip(1).find_map(T::cast) } +pub fn has_errors(node: &SyntaxNode) -> bool { + node.children().any(|it| it.kind() == SyntaxKind::ERROR) +} + #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum InsertPosition { First, diff --git a/crates/ra_syntax/src/ast/edit.rs b/crates/ra_syntax/src/ast/edit.rs index 9e5411ee5..26e4576ff 100644 --- a/crates/ra_syntax/src/ast/edit.rs +++ b/crates/ra_syntax/src/ast/edit.rs @@ -307,7 +307,11 @@ impl ast::UseTree { fn split_path_prefix(prefix: &ast::Path) -> Option { let parent = prefix.parent_path()?; - let mut res = make::path_unqualified(parent.segment()?); + let segment = parent.segment()?; + if algo::has_errors(segment.syntax()) { + return None; + } + let mut res = make::path_unqualified(segment); for p in iter::successors(parent.parent_path(), |it| it.parent_path()) { res = make::path_qualified(res, p.segment()?); } -- cgit v1.2.3