Skip to content

internal: Don't turn local names into strings in CompletionContext #10561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 17, 2021
Merged

internal: Don't turn local names into strings in CompletionContext #10561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions crates/hir_expand/src/name.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,30 @@ impl Name {
Name::new_text("[missing name]".into())
}

/// Returns the tuple index this name represents if it is a tuple field.
pub fn as_tuple_index(&self) -> Option<usize> {
match self.0 {
Repr::TupleField(idx) => Some(idx),
_ => None,
}
}

/// Returns the text this name represents if it isn't a tuple field.
pub fn as_text(&self) -> Option<SmolStr> {
match &self.0 {
Repr::Text(it) => Some(it.clone()),
_ => None,
}
}

/// Returns the textual representation of this name as a [`SmolStr`].
/// Prefer using this over [`ToString::to_string`] if possible as this conversion is cheaper.
pub fn to_smol_str(&self) -> SmolStr {
match &self.0 {
Repr::Text(it) => it.clone(),
Repr::TupleField(it) => SmolStr::new(&it.to_string()),
}
}
}

pub trait AsName {
Expand Down
36 changes: 14 additions & 22 deletions crates/ide_completion/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ pub(crate) struct CompletionContext<'a> {
pub(super) function_def: Option<ast::Fn>,
/// The parent impl of the cursor position if it exists.
pub(super) impl_def: Option<ast::Impl>,
/// The NameLike under the cursor in the original file if it exists.
pub(super) name_syntax: Option<ast::NameLike>,
pub(super) incomplete_let: bool,

pub(super) completion_location: Option<ImmediateLocation>,
pub(super) prev_sibling: Option<ImmediatePrevSibling>,
Expand All @@ -112,9 +114,7 @@ pub(crate) struct CompletionContext<'a> {
pub(super) lifetime_ctx: Option<LifetimeContext>,
pub(super) pattern_ctx: Option<PatternContext>,
pub(super) path_context: Option<PathCompletionContext>,
pub(super) locals: Vec<(String, Local)>,

pub(super) incomplete_let: bool,
pub(super) locals: Vec<(Name, Local)>,

no_completion_required: bool,
}
Expand Down Expand Up @@ -148,7 +148,7 @@ impl<'a> CompletionContext<'a> {
let mut locals = vec![];
scope.process_all_names(&mut |name, scope| {
if let ScopeDef::Local(local) = scope {
locals.push((name.to_string(), local));
locals.push((name, local));
}
});
let mut ctx = CompletionContext {
Expand Down Expand Up @@ -492,14 +492,6 @@ impl<'a> CompletionContext<'a> {
false
}

fn fill_impl_def(&mut self) {
self.impl_def = self
.sema
.token_ancestors_with_macros(self.token.clone())
.take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
.find_map(ast::Impl::cast);
}

fn expected_type_and_name(&self) -> (Option<Type>, Option<NameOrNameRef>) {
let mut node = match self.token.parent() {
Some(it) => it,
Expand Down Expand Up @@ -654,6 +646,16 @@ impl<'a> CompletionContext<'a> {
self.prev_sibling = determine_prev_sibling(&name_like);
self.name_syntax =
find_node_at_offset(original_file, name_like.syntax().text_range().start());
self.impl_def = self
.sema
.token_ancestors_with_macros(self.token.clone())
.take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
.find_map(ast::Impl::cast);
self.function_def = self
.sema
.token_ancestors_with_macros(self.token.clone())
.take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
.find_map(ast::Fn::cast);
match name_like {
ast::NameLike::Lifetime(lifetime) => {
self.classify_lifetime(original_file, lifetime, offset);
Expand Down Expand Up @@ -691,8 +693,6 @@ impl<'a> CompletionContext<'a> {
}

fn classify_name(&mut self, name: ast::Name) {
self.fill_impl_def();

if let Some(bind_pat) = name.syntax().parent().and_then(ast::IdentPat::cast) {
let is_name_in_field_pat = bind_pat
.syntax()
Expand Down Expand Up @@ -740,14 +740,6 @@ impl<'a> CompletionContext<'a> {
}

fn classify_name_ref(&mut self, original_file: &SyntaxNode, name_ref: ast::NameRef) {
self.fill_impl_def();

self.function_def = self
.sema
.token_ancestors_with_macros(self.token.clone())
.take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
.find_map(ast::Fn::cast);

let parent = match name_ref.syntax().parent() {
Some(it) => it,
None => return,
Expand Down
58 changes: 53 additions & 5 deletions crates/ide_completion/src/render/builder_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
//! Extensions for `Builder` structure required for item rendering.

use either::Either;
use itertools::Itertools;
use syntax::ast::{self, HasName};

use crate::{context::CallKind, item::Builder, patterns::ImmediateLocation, CompletionContext};

#[derive(Debug)]
pub(super) enum Params {
Named(Vec<String>),
Named(Vec<(Either<ast::SelfParam, ast::Param>, hir::Param)>),
Anonymous(usize),
}

Expand Down Expand Up @@ -76,10 +78,46 @@ impl Builder {
self.trigger_call_info();
let snippet = match (ctx.config.add_call_argument_snippets, params) {
(true, Params::Named(params)) => {
let function_params_snippet =
params.iter().enumerate().format_with(", ", |(index, param_name), f| {
f(&format_args!("${{{}:{}}}", index + 1, param_name))
});
let function_params_snippet = params.iter().enumerate().format_with(
", ",
|(index, (param_source, param)), f| {
let name;
let text;
let (ref_, name) = match param_source {
Either::Left(self_param) => (
match self_param.kind() {
ast::SelfParamKind::Owned => "",
ast::SelfParamKind::Ref => "&",
ast::SelfParamKind::MutRef => "&mut ",
},
"self",
),
Either::Right(it) => {
let n = (|| {
let mut pat = it.pat()?;
loop {
match pat {
ast::Pat::IdentPat(pat) => break pat.name(),
ast::Pat::RefPat(it) => pat = it.pat()?,
_ => return None,
}
}
})();
match n {
Some(n) => {
name = n;
text = name.text();
let text = text.as_str().trim_start_matches('_');
let ref_ = ref_of_param(ctx, text, param.ty());
(ref_, text)
}
None => ("", "_"),
}
}
};
f(&format_args!("${{{}:{}{}}}", index + 1, ref_, name))
},
);
format!("{}({})$0", name, function_params_snippet)
}
_ => {
Expand All @@ -93,3 +131,13 @@ impl Builder {
self.lookup_by(name).label(label).insert_snippet(cap, snippet)
}
}
fn ref_of_param(ctx: &CompletionContext, arg: &str, ty: &hir::Type) -> &'static str {
if let Some(derefed_ty) = ty.remove_ref() {
for (name, local) in ctx.locals.iter() {
if name.as_text().as_deref() == Some(arg) && local.ty(ctx.db) == derefed_ty {
return if ty.is_mutable_reference() { "&mut " } else { "&" };
}
}
}
""
}
46 changes: 12 additions & 34 deletions crates/ide_completion/src/render/function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Renderer for function calls.

use either::Either;
use hir::{AsAssocItem, HasSource, HirDisplay};
use ide_db::SymbolKind;
use itertools::Itertools;
Expand Down Expand Up @@ -160,47 +161,25 @@ impl<'a> FunctionRender<'a> {
format!("-> {}", ret_ty.display(self.ctx.db()))
}

fn add_arg(&self, arg: &str, ty: &hir::Type) -> String {
if let Some(derefed_ty) = ty.remove_ref() {
for (name, local) in self.ctx.completion.locals.iter() {
if name == arg && local.ty(self.ctx.db()) == derefed_ty {
let mutability = if ty.is_mutable_reference() { "&mut " } else { "&" };
return format!("{}{}", mutability, arg);
}
}
}
arg.to_string()
}

fn params(&self) -> Params {
let ast_params = match self.ast_node.param_list() {
Some(it) => it,
None => return Params::Named(Vec::new()),
};
let params = ast_params.params().map(Either::Right);

let mut params_pats = Vec::new();
let params_ty = if self.ctx.completion.has_dot_receiver() || self.receiver.is_some() {
self.func.method_params(self.ctx.db()).unwrap_or_default()
let params = if self.ctx.completion.has_dot_receiver() || self.receiver.is_some() {
params.zip(self.func.method_params(self.ctx.db()).unwrap_or_default()).collect()
} else {
if let Some(s) = ast_params.self_param() {
cov_mark::hit!(parens_for_method_call_as_assoc_fn);
params_pats.push(Some(s.to_string()));
}
self.func.assoc_fn_params(self.ctx.db())
ast_params
.self_param()
.map(Either::Left)
.into_iter()
.chain(params)
.zip(self.func.assoc_fn_params(self.ctx.db()))
.collect()
};
params_pats
.extend(ast_params.params().into_iter().map(|it| it.pat().map(|it| it.to_string())));

let params = params_pats
.into_iter()
.zip(params_ty)
.flat_map(|(pat, param_ty)| {
let pat = pat?;
let name = pat;
let arg = name.trim_start_matches("mut ").trim_start_matches('_');
Some(self.add_arg(arg, param_ty.ty()))
})
.collect();

Params::Named(params)
}

Expand Down Expand Up @@ -310,7 +289,6 @@ impl S {

#[test]
fn parens_for_method_call_as_assoc_fn() {
cov_mark::check!(parens_for_method_call_as_assoc_fn);
check_edit(
"foo",
r#"
Expand Down