diff --git a/crates/ide_completion/src/completions/postfix.rs b/crates/ide_completion/src/completions/postfix.rs index 44f2aec51b6..a640b7d0738 100644 --- a/crates/ide_completion/src/completions/postfix.rs +++ b/crates/ide_completion/src/completions/postfix.rs @@ -232,8 +232,8 @@ fn add_custom_postfix_completions( ImportScope::find_insert_use_container_with_macros(&ctx.token.parent()?, &ctx.sema)?; ctx.config.postfix_snippets.iter().for_each(|snippet| { let imports = match snippet.imports(ctx, &import_scope) { - Ok(imports) => imports, - Err(_) => return, + Some(imports) => imports, + None => return, }; let mut builder = postfix_snippet( &snippet.label, diff --git a/crates/ide_completion/src/completions/snippet.rs b/crates/ide_completion/src/completions/snippet.rs index 1dace20102c..3824bf8a9a0 100644 --- a/crates/ide_completion/src/completions/snippet.rs +++ b/crates/ide_completion/src/completions/snippet.rs @@ -105,8 +105,8 @@ fn add_custom_completions( ImportScope::find_insert_use_container_with_macros(&ctx.token.parent()?, &ctx.sema)?; ctx.config.snippets.iter().filter(|snip| snip.scope == scope).for_each(|snip| { let imports = match snip.imports(ctx, &import_scope) { - Ok(imports) => imports, - Err(_) => return, + Some(imports) => imports, + None => return, }; let mut builder = snippet(ctx, cap, &snip.label, &snip.snippet); for import in imports.into_iter() { diff --git a/crates/ide_completion/src/snippet.rs b/crates/ide_completion/src/snippet.rs index 9d6e18f1a7e..b7dd1b3297e 100644 --- a/crates/ide_completion/src/snippet.rs +++ b/crates/ide_completion/src/snippet.rs @@ -37,7 +37,6 @@ pub struct Snippet { pub description: Option, pub requires: Box<[String]>, } - impl Snippet { pub fn new( label: String, @@ -46,19 +45,7 @@ impl Snippet { requires: &[String], scope: SnippetScope, ) -> Option { - // validate that these are indeed simple paths - if requires.iter().any(|path| match ast::Path::parse(path) { - Ok(path) => path.segments().any(|seg| { - !matches!(seg.kind(), Some(ast::PathSegmentKind::Name(_))) - || seg.generic_arg_list().is_some() - }), - Err(_) => true, - }) { - return None; - } - let snippet = snippet.iter().join("\n"); - let description = description.iter().join("\n"); - let description = if description.is_empty() { None } else { Some(description) }; + let (snippet, description) = validate_snippet(snippet, description, requires)?; Some(Snippet { scope, label, @@ -68,12 +55,12 @@ impl Snippet { }) } - // FIXME: This shouldn't be fallible + /// Returns None if the required items do not resolve. pub(crate) fn imports( &self, ctx: &CompletionContext, import_scope: &ImportScope, - ) -> Result, ()> { + ) -> Option> { import_edits(ctx, import_scope, &self.requires) } @@ -94,19 +81,7 @@ impl PostfixSnippet { requires: &[String], scope: PostfixSnippetScope, ) -> Option { - // validate that these are indeed simple paths - if requires.iter().any(|path| match ast::Path::parse(path) { - Ok(path) => path.segments().any(|seg| { - !matches!(seg.kind(), Some(ast::PathSegmentKind::Name(_))) - || seg.generic_arg_list().is_some() - }), - Err(_) => true, - }) { - return None; - } - let snippet = snippet.iter().join("\n"); - let description = description.iter().join("\n"); - let description = if description.is_empty() { None } else { Some(description) }; + let (snippet, description) = validate_snippet(snippet, description, requires)?; Some(PostfixSnippet { scope, label, @@ -116,12 +91,12 @@ impl PostfixSnippet { }) } - // FIXME: This shouldn't be fallible + /// Returns None if the required items do not resolve. pub(crate) fn imports( &self, ctx: &CompletionContext, import_scope: &ImportScope, - ) -> Result, ()> { + ) -> Option> { import_edits(ctx, import_scope, &self.requires) } @@ -142,32 +117,52 @@ fn import_edits( ctx: &CompletionContext, import_scope: &ImportScope, requires: &[String], -) -> Result, ()> { +) -> Option> { let resolve = |import| { let path = ast::Path::parse(import).ok()?; - match ctx.scope.speculative_resolve(&path)? { - hir::PathResolution::Macro(_) => None, - hir::PathResolution::Def(def) => { - let item = def.into(); - let path = ctx.scope.module()?.find_use_path_prefixed( - ctx.db, - item, - ctx.config.insert_use.prefix_kind, - )?; - Some((path.len() > 1).then(|| ImportEdit { - import: LocatedImport::new(path.clone(), item, item, None), - scope: import_scope.clone(), - })) - } - _ => None, - } + let item = match ctx.scope.speculative_resolve(&path)? { + hir::PathResolution::Macro(mac) => mac.into(), + hir::PathResolution::Def(def) => def.into(), + _ => return None, + }; + let path = ctx.scope.module()?.find_use_path_prefixed( + ctx.db, + item, + ctx.config.insert_use.prefix_kind, + )?; + Some((path.len() > 1).then(|| ImportEdit { + import: LocatedImport::new(path.clone(), item, item, None), + scope: import_scope.clone(), + })) }; let mut res = Vec::with_capacity(requires.len()); for import in requires { match resolve(import) { Some(first) => res.extend(first), - None => return Err(()), + None => return None, } } - Ok(res) + Some(res) +} + +fn validate_snippet( + snippet: &[String], + description: &[String], + requires: &[String], +) -> Option<(String, Option)> { + // validate that these are indeed simple paths + // we can't save the paths unfortunately due to them not being Send+Sync + if requires.iter().any(|path| match ast::Path::parse(path) { + Ok(path) => path.segments().any(|seg| { + !matches!(seg.kind(), Some(ast::PathSegmentKind::Name(_))) + || seg.generic_arg_list().is_some() + }), + Err(_) => true, + }) { + return None; + } + let snippet = snippet.iter().join("\n"); + let description = description.iter().join("\n"); + let description = if description.is_empty() { None } else { Some(description) }; + Some((snippet, description)) }