fix: Fix auto-ref completions inserting into wrong locations

This commit is contained in:
Lukas Wirth 2022-06-20 18:59:57 +02:00
parent 1f028403cd
commit 06ee4d6222
10 changed files with 98 additions and 94 deletions

View file

@ -169,6 +169,8 @@ pub(crate) fn import_on_the_fly_pat(
has_macro_bang: false, has_macro_bang: false,
qualified: Qualified::No, qualified: Qualified::No,
parent: None, parent: None,
// FIXME
path: syntax::ast::make::ext::ident_path("dummy__"),
kind: crate::context::PathKind::Pat { pat_ctx: pat_ctx.clone() }, kind: crate::context::PathKind::Pat { pat_ctx: pat_ctx.clone() },
has_type_args: false, has_type_args: false,
use_tree_parent: false, use_tree_parent: false,

View file

@ -88,6 +88,8 @@ pub(crate) fn complete_pattern(
has_call_parens: false, has_call_parens: false,
has_macro_bang: false, has_macro_bang: false,
qualified: Qualified::No, qualified: Qualified::No,
// FIXME
path: syntax::ast::make::ext::ident_path("dummy__"),
parent: None, parent: None,
kind: crate::context::PathKind::Pat { pat_ctx: pattern_ctx.clone() }, kind: crate::context::PathKind::Pat { pat_ctx: pattern_ctx.clone() },
has_type_args: false, has_type_args: false,
@ -95,7 +97,7 @@ pub(crate) fn complete_pattern(
}, },
mac, mac,
name, name,
) );
} }
_ => false, _ => false,
}, },

View file

@ -61,6 +61,8 @@ pub(crate) struct PathCompletionCtx {
pub(super) qualified: Qualified, pub(super) qualified: Qualified,
/// The parent of the path we are completing. /// The parent of the path we are completing.
pub(super) parent: Option<ast::Path>, pub(super) parent: Option<ast::Path>,
/// The path of which we are completing the segment
pub(super) path: ast::Path,
pub(super) kind: PathKind, pub(super) kind: PathKind,
/// Whether the path segment has type args or not. /// Whether the path segment has type args or not.
pub(super) has_type_args: bool, pub(super) has_type_args: bool,

View file

@ -556,6 +556,7 @@ impl<'a> CompletionContext<'a> {
has_macro_bang: false, has_macro_bang: false,
qualified: Qualified::No, qualified: Qualified::No,
parent: path.parent_path(), parent: path.parent_path(),
path: path.clone(),
kind: PathKind::Item { kind: ItemListKind::SourceFile }, kind: PathKind::Item { kind: ItemListKind::SourceFile },
has_type_args: false, has_type_args: false,
use_tree_parent: false, use_tree_parent: false,

View file

@ -6,7 +6,7 @@ use hir::{Documentation, Mutability};
use ide_db::{imports::import_assets::LocatedImport, SnippetCap, SymbolKind}; use ide_db::{imports::import_assets::LocatedImport, SnippetCap, SymbolKind};
use smallvec::SmallVec; use smallvec::SmallVec;
use stdx::{impl_from, never}; use stdx::{impl_from, never};
use syntax::{SmolStr, TextRange}; use syntax::{SmolStr, TextRange, TextSize};
use text_edit::TextEdit; use text_edit::TextEdit;
use crate::{ use crate::{
@ -68,7 +68,7 @@ pub struct CompletionItem {
/// Indicates that a reference or mutable reference to this variable is a /// Indicates that a reference or mutable reference to this variable is a
/// possible match. /// possible match.
ref_match: Option<Mutability>, ref_match: Option<(Mutability, TextSize)>,
/// The import data to add to completion's edits. /// The import data to add to completion's edits.
import_to_add: SmallVec<[LocatedImport; 1]>, import_to_add: SmallVec<[LocatedImport; 1]>,
@ -104,8 +104,8 @@ impl fmt::Debug for CompletionItem {
s.field("relevance", &self.relevance); s.field("relevance", &self.relevance);
} }
if let Some(mutability) = &self.ref_match { if let Some((mutability, offset)) = &self.ref_match {
s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref())); s.field("ref_match", &format!("&{}@{offset:?}", mutability.as_keyword_for_ref()));
} }
if self.trigger_call_info { if self.trigger_call_info {
s.field("trigger_call_info", &true); s.field("trigger_call_info", &true);
@ -395,14 +395,14 @@ impl CompletionItem {
self.trigger_call_info self.trigger_call_info
} }
pub fn ref_match(&self) -> Option<(Mutability, CompletionRelevance)> { pub fn ref_match(&self) -> Option<(Mutability, TextSize, CompletionRelevance)> {
// Relevance of the ref match should be the same as the original // Relevance of the ref match should be the same as the original
// match, but with exact type match set because self.ref_match // match, but with exact type match set because self.ref_match
// is only set if there is an exact type match. // is only set if there is an exact type match.
let mut relevance = self.relevance; let mut relevance = self.relevance;
relevance.type_match = Some(CompletionRelevanceTypeMatch::Exact); relevance.type_match = Some(CompletionRelevanceTypeMatch::Exact);
self.ref_match.map(|mutability| (mutability, relevance)) self.ref_match.map(|(mutability, offset)| (mutability, offset, relevance))
} }
pub fn imports_to_add(&self) -> &[LocatedImport] { pub fn imports_to_add(&self) -> &[LocatedImport] {
@ -428,7 +428,7 @@ pub(crate) struct Builder {
deprecated: bool, deprecated: bool,
trigger_call_info: bool, trigger_call_info: bool,
relevance: CompletionRelevance, relevance: CompletionRelevance,
ref_match: Option<Mutability>, ref_match: Option<(Mutability, TextSize)>,
} }
impl Builder { impl Builder {
@ -548,8 +548,8 @@ impl Builder {
self.imports_to_add.push(import_to_add); self.imports_to_add.push(import_to_add);
self self
} }
pub(crate) fn ref_match(&mut self, mutability: Mutability) -> &mut Builder { pub(crate) fn ref_match(&mut self, mutability: Mutability, offset: TextSize) -> &mut Builder {
self.ref_match = Some(mutability); self.ref_match = Some((mutability, offset));
self self
} }
} }

View file

@ -14,7 +14,7 @@ use hir::{AsAssocItem, HasAttrs, HirDisplay, ScopeDef};
use ide_db::{ use ide_db::{
helpers::item_name, imports::import_assets::LocatedImport, RootDatabase, SnippetCap, SymbolKind, helpers::item_name, imports::import_assets::LocatedImport, RootDatabase, SnippetCap, SymbolKind,
}; };
use syntax::{SmolStr, SyntaxKind, TextRange}; use syntax::{AstNode, SmolStr, SyntaxKind, TextRange};
use crate::{ use crate::{
context::{PathCompletionCtx, PathKind}, context::{PathCompletionCtx, PathKind},
@ -167,7 +167,7 @@ pub(crate) fn render_resolution_simple(
local_name: hir::Name, local_name: hir::Name,
resolution: ScopeDef, resolution: ScopeDef,
) -> Builder { ) -> Builder {
render_resolution_simple_(ctx, local_name, None, resolution) render_resolution_simple_(ctx, None, local_name, None, resolution)
} }
pub(crate) fn render_resolution_with_import( pub(crate) fn render_resolution_with_import(
@ -235,7 +235,8 @@ fn render_resolution_simple_type(
let db = ctx.completion.db; let db = ctx.completion.db;
let config = ctx.completion.config; let config = ctx.completion.config;
let name = local_name.to_smol_str(); let name = local_name.to_smol_str();
let mut item = render_resolution_simple_(ctx, local_name, import_to_add, resolution); let mut item =
render_resolution_simple_(ctx, Some(path_ctx), local_name, import_to_add, resolution);
// Add `<>` for generic types // Add `<>` for generic types
let type_path_no_ty_args = matches!( let type_path_no_ty_args = matches!(
path_ctx, path_ctx,
@ -264,6 +265,7 @@ fn render_resolution_simple_type(
fn render_resolution_simple_( fn render_resolution_simple_(
ctx: RenderContext<'_>, ctx: RenderContext<'_>,
path_ctx: Option<&PathCompletionCtx>,
local_name: hir::Name, local_name: hir::Name,
import_to_add: Option<LocatedImport>, import_to_add: Option<LocatedImport>,
resolution: ScopeDef, resolution: ScopeDef,
@ -317,8 +319,10 @@ fn render_resolution_simple_(
..CompletionRelevance::default() ..CompletionRelevance::default()
}); });
if let Some(ref_match) = compute_ref_match(ctx.completion, &ty) { if let Some(path_ctx) = path_ctx {
item.ref_match(ref_match); if let Some(ref_match) = compute_ref_match(ctx.completion, &ty) {
item.ref_match(ref_match, path_ctx.path.syntax().text_range().start());
}
} }
}; };
@ -455,7 +459,7 @@ mod tests {
let relevance = display_relevance(it.relevance()); let relevance = display_relevance(it.relevance());
items.push(format!("{} {} {}\n", tag, it.label(), relevance)); items.push(format!("{} {} {}\n", tag, it.label(), relevance));
if let Some((mutability, relevance)) = it.ref_match() { if let Some((mutability, _offset, relevance)) = it.ref_match() {
let label = format!("&{}{}", mutability.as_keyword_for_ref(), it.label()); let label = format!("&{}{}", mutability.as_keyword_for_ref(), it.label());
let relevance = display_relevance(relevance); let relevance = display_relevance(relevance);
@ -1495,7 +1499,7 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
"#, "#,
&[CompletionItemKind::Method, CompletionItemKind::SymbolKind(SymbolKind::Field)], &[CompletionItemKind::Method, CompletionItemKind::SymbolKind(SymbolKind::Field)],
// FIXME // FIXME
// Ideally we'd also suggest &f.bar and &f.baz() as exact // Ideally we'd also suggest &f.bar as exact
// type matches. See #8058. // type matches. See #8058.
expect![[r#" expect![[r#"
[ [
@ -1507,6 +1511,7 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
kind: Method, kind: Method,
lookup: "baz", lookup: "baz",
detail: "fn(&self) -> u32", detail: "fn(&self) -> u32",
ref_match: "&@96",
}, },
CompletionItem { CompletionItem {
label: "bar", label: "bar",
@ -1525,7 +1530,6 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
#[test] #[test]
fn qualified_path_ref() { fn qualified_path_ref() {
// disabled right now because it doesn't render correctly, #8058
check_kinds( check_kinds(
r#" r#"
struct S; struct S;
@ -1554,6 +1558,7 @@ fn main() {
), ),
lookup: "foo", lookup: "foo",
detail: "fn() -> S", detail: "fn() -> S",
ref_match: "&@92",
}, },
] ]
"#]], "#]],

View file

@ -4,20 +4,19 @@ use hir::{db::HirDatabase, AsAssocItem, HirDisplay};
use ide_db::{SnippetCap, SymbolKind}; use ide_db::{SnippetCap, SymbolKind};
use itertools::Itertools; use itertools::Itertools;
use stdx::{format_to, to_lower_snake_case}; use stdx::{format_to, to_lower_snake_case};
use syntax::SmolStr; use syntax::{AstNode, SmolStr};
use crate::{ use crate::{
context::{ context::{CompletionContext, DotAccess, DotAccessKind, PathCompletionCtx, PathKind},
CompletionContext, DotAccess, DotAccessKind, PathCompletionCtx, PathKind, Qualified,
},
item::{Builder, CompletionItem, CompletionItemKind, CompletionRelevance}, item::{Builder, CompletionItem, CompletionItemKind, CompletionRelevance},
render::{compute_exact_name_match, compute_ref_match, compute_type_match, RenderContext}, render::{compute_exact_name_match, compute_ref_match, compute_type_match, RenderContext},
CallableSnippets, CallableSnippets,
}; };
enum FuncKind { #[derive(Debug)]
Function, enum FuncKind<'ctx> {
Method(Option<hir::Name>), Function(&'ctx PathCompletionCtx),
Method(&'ctx DotAccess, Option<hir::Name>),
} }
pub(crate) fn render_fn( pub(crate) fn render_fn(
@ -27,29 +26,7 @@ pub(crate) fn render_fn(
func: hir::Function, func: hir::Function,
) -> Builder { ) -> Builder {
let _p = profile::span("render_fn"); let _p = profile::span("render_fn");
let func_kind = FuncKind::Function; render(ctx, local_name, func, FuncKind::Function(path_ctx))
let params = match ctx.completion.config.snippet_cap {
Some(_) => {
if !matches!(
path_ctx,
PathCompletionCtx { kind: PathKind::Expr { .. }, has_call_parens: true, .. }
| PathCompletionCtx { kind: PathKind::Use | PathKind::Type { .. }, .. }
) {
params(ctx.completion, func, &func_kind, false)
} else {
None
}
}
_ => None,
};
render(
ctx,
local_name,
func,
func_kind,
params,
matches!(path_ctx.qualified, Qualified::With { .. }),
)
} }
pub(crate) fn render_method( pub(crate) fn render_method(
@ -60,16 +37,7 @@ pub(crate) fn render_method(
func: hir::Function, func: hir::Function,
) -> Builder { ) -> Builder {
let _p = profile::span("render_method"); let _p = profile::span("render_method");
let func_kind = FuncKind::Method(receiver); render(ctx, local_name, func, FuncKind::Method(dot_access, receiver))
let params = match ctx.completion.config.snippet_cap {
Some(_) => match dot_access {
DotAccess { kind: DotAccessKind::Method { has_parens: true }, .. } => None,
_ => params(ctx.completion, func, &func_kind, true),
},
_ => None,
};
render(ctx, local_name, func, func_kind, params, false)
} }
fn render( fn render(
@ -77,15 +45,13 @@ fn render(
local_name: Option<hir::Name>, local_name: Option<hir::Name>,
func: hir::Function, func: hir::Function,
func_kind: FuncKind, func_kind: FuncKind,
params: Option<(Option<hir::SelfParam>, Vec<hir::Param>)>,
qualified_path: bool,
) -> Builder { ) -> Builder {
let db = completion.db; let db = completion.db;
let name = local_name.unwrap_or_else(|| func.name(db)); let name = local_name.unwrap_or_else(|| func.name(db));
let call = match &func_kind { let call = match &func_kind {
FuncKind::Method(Some(receiver)) => format!("{}.{}", receiver, &name).into(), FuncKind::Method(_, Some(receiver)) => format!("{}.{}", receiver, &name).into(),
_ => name.to_smol_str(), _ => name.to_smol_str(),
}; };
let mut item = CompletionItem::new( let mut item = CompletionItem::new(
@ -111,11 +77,14 @@ fn render(
}); });
if let Some(ref_match) = compute_ref_match(completion, &ret_type) { if let Some(ref_match) = compute_ref_match(completion, &ret_type) {
// FIXME For now we don't properly calculate the edits for ref match match func_kind {
// completions on methods or qualified paths, so we've disabled them. FuncKind::Function(path_ctx) => {
// See #8058. item.ref_match(ref_match, path_ctx.path.syntax().text_range().start());
if matches!(func_kind, FuncKind::Function) && !qualified_path { }
item.ref_match(ref_match); FuncKind::Method(DotAccess { receiver: Some(receiver), .. }, _) => {
item.ref_match(ref_match, receiver.syntax().text_range().start());
}
_ => (),
} }
} }
@ -124,12 +93,34 @@ fn render(
.detail(detail(db, func)) .detail(detail(db, func))
.lookup_by(name.to_smol_str()); .lookup_by(name.to_smol_str());
match completion.config.snippet_cap.zip(params) { match ctx.completion.config.snippet_cap {
Some((cap, (self_param, params))) => { Some(cap) => {
add_call_parens(&mut item, completion, cap, call, self_param, params); let complete_params = match func_kind {
FuncKind::Function(PathCompletionCtx {
kind: PathKind::Expr { .. },
has_call_parens: false,
..
}) => Some(false),
FuncKind::Method(
DotAccess {
kind:
DotAccessKind::Method { has_parens: false } | DotAccessKind::Field { .. },
..
},
_,
) => Some(true),
_ => None,
};
if let Some(has_dot_receiver) = complete_params {
if let Some((self_param, params)) =
params(ctx.completion, func, &func_kind, has_dot_receiver)
{
add_call_parens(&mut item, completion, cap, call, self_param, params);
}
}
} }
_ => (), _ => (),
} };
match ctx.import_to_add { match ctx.import_to_add {
Some(import_to_add) => { Some(import_to_add) => {
@ -291,7 +282,7 @@ fn params(
} }
} }
let self_param = if has_dot_receiver || matches!(func_kind, FuncKind::Method(Some(_))) { let self_param = if has_dot_receiver || matches!(func_kind, FuncKind::Method(_, Some(_))) {
None None
} else { } else {
func.self_param(ctx.db) func.self_param(ctx.db)

View file

@ -2,6 +2,7 @@
use hir::{db::HirDatabase, Documentation, HasAttrs, StructKind}; use hir::{db::HirDatabase, Documentation, HasAttrs, StructKind};
use ide_db::SymbolKind; use ide_db::SymbolKind;
use syntax::AstNode;
use crate::{ use crate::{
context::{CompletionContext, PathCompletionCtx, PathKind}, context::{CompletionContext, PathCompletionCtx, PathKind},
@ -117,7 +118,7 @@ fn render(
..ctx.completion_relevance() ..ctx.completion_relevance()
}); });
if let Some(ref_match) = compute_ref_match(completion, &ty) { if let Some(ref_match) = compute_ref_match(completion, &ty) {
item.ref_match(ref_match); item.ref_match(ref_match, path_ctx.path.syntax().text_range().start());
} }
if let Some(import_to_add) = ctx.import_to_add { if let Some(import_to_add) = ctx.import_to_add {

View file

@ -399,6 +399,7 @@ fn foo() {
#[test] #[test]
fn completes_no_delims_if_existing() { fn completes_no_delims_if_existing() {
// FIXME: We should not complete functions here
check_empty( check_empty(
r#" r#"
struct Bar(u32); struct Bar(u32);
@ -409,7 +410,7 @@ fn foo() {
} }
"#, "#,
expect![[r#" expect![[r#"
fn foo() fn() fn foo fn()
st Bar st Bar
bt u32 bt u32
kw crate:: kw crate::
@ -427,7 +428,7 @@ fn foo() {
} }
"#, "#,
expect![[r#" expect![[r#"
fn foo() fn() fn foo fn()
st Foo st Foo
bt u32 bt u32
kw crate:: kw crate::

View file

@ -224,6 +224,7 @@ fn completion_item(
max_relevance: u32, max_relevance: u32,
item: CompletionItem, item: CompletionItem,
) { ) {
let insert_replace_support = config.insert_replace_support().then(|| tdpp.position);
let mut additional_text_edits = Vec::new(); let mut additional_text_edits = Vec::new();
// LSP does not allow arbitrary edits in completion, so we have to do a // LSP does not allow arbitrary edits in completion, so we have to do a
@ -233,7 +234,6 @@ fn completion_item(
let source_range = item.source_range(); let source_range = item.source_range();
for indel in item.text_edit().iter() { for indel in item.text_edit().iter() {
if indel.delete.contains_range(source_range) { if indel.delete.contains_range(source_range) {
let insert_replace_support = config.insert_replace_support().then(|| tdpp.position);
text_edit = Some(if indel.delete == source_range { text_edit = Some(if indel.delete == source_range {
self::completion_text_edit(line_index, insert_replace_support, indel.clone()) self::completion_text_edit(line_index, insert_replace_support, indel.clone())
} else { } else {
@ -254,6 +254,14 @@ fn completion_item(
text_edit.unwrap() text_edit.unwrap()
}; };
let insert_text_format = item.is_snippet().then(|| lsp_types::InsertTextFormat::SNIPPET);
let tags = item.deprecated().then(|| vec![lsp_types::CompletionItemTag::DEPRECATED]);
let command = if item.trigger_call_info() && config.client_commands().trigger_parameter_hints {
Some(command::trigger_parameter_hints())
} else {
None
};
let mut lsp_item = lsp_types::CompletionItem { let mut lsp_item = lsp_types::CompletionItem {
label: item.label().to_string(), label: item.label().to_string(),
detail: item.detail().map(|it| it.to_string()), detail: item.detail().map(|it| it.to_string()),
@ -263,22 +271,14 @@ fn completion_item(
additional_text_edits: Some(additional_text_edits), additional_text_edits: Some(additional_text_edits),
documentation: item.documentation().map(documentation), documentation: item.documentation().map(documentation),
deprecated: Some(item.deprecated()), deprecated: Some(item.deprecated()),
tags,
command,
insert_text_format,
..Default::default() ..Default::default()
}; };
set_score(&mut lsp_item, max_relevance, item.relevance()); set_score(&mut lsp_item, max_relevance, item.relevance());
if item.deprecated() {
lsp_item.tags = Some(vec![lsp_types::CompletionItemTag::DEPRECATED])
}
if item.trigger_call_info() && config.client_commands().trigger_parameter_hints {
lsp_item.command = Some(command::trigger_parameter_hints());
}
if item.is_snippet() {
lsp_item.insert_text_format = Some(lsp_types::InsertTextFormat::SNIPPET);
}
if config.completion().enable_imports_on_the_fly { if config.completion().enable_imports_on_the_fly {
if let imports @ [_, ..] = item.imports_to_add() { if let imports @ [_, ..] = item.imports_to_add() {
let imports: Vec<_> = imports let imports: Vec<_> = imports
@ -299,18 +299,17 @@ fn completion_item(
} }
} }
if let Some((mutability, relevance)) = item.ref_match() { if let Some((mutability, offset, relevance)) = item.ref_match() {
let mut lsp_item_with_ref = lsp_item.clone(); let mut lsp_item_with_ref = lsp_item.clone();
set_score(&mut lsp_item_with_ref, max_relevance, relevance); set_score(&mut lsp_item_with_ref, max_relevance, relevance);
lsp_item_with_ref.label = lsp_item_with_ref.label =
format!("&{}{}", mutability.as_keyword_for_ref(), lsp_item_with_ref.label); format!("&{}{}", mutability.as_keyword_for_ref(), lsp_item_with_ref.label);
if let Some(it) = &mut lsp_item_with_ref.text_edit { lsp_item_with_ref.additional_text_edits.get_or_insert_with(Default::default).push(
let new_text = match it { self::text_edit(
lsp_types::CompletionTextEdit::Edit(it) => &mut it.new_text, line_index,
lsp_types::CompletionTextEdit::InsertAndReplace(it) => &mut it.new_text, Indel::insert(offset, format!("&{}", mutability.as_keyword_for_ref())),
}; ),
*new_text = format!("&{}{}", mutability.as_keyword_for_ref(), new_text); );
}
acc.push(lsp_item_with_ref); acc.push(lsp_item_with_ref);
}; };