Skip to content

Commit

Permalink
Auto merge of #18031 - roife:suggest-name-in-completion, r=Veykril
Browse files Browse the repository at this point in the history
feat: Suggest name in completion for let_stmt and fn_param

fix #17780

1. Refactor: move `ide_assist::utils::suggest_name` to `ide-db::syntax_helpers::suggest_name` for reuse.
2. When completing `IdentPat`, detecte if the current node is a `let_stmt` or `fn_param`, and suggesting a new name based on the context.
  • Loading branch information
bors committed Sep 3, 2024
2 parents c2f9b47 + ef491f2 commit 6faf409
Show file tree
Hide file tree
Showing 12 changed files with 142 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use hir::TypeInfo;
use ide_db::syntax_helpers::suggest_name;
use syntax::{
ast::{self, edit::IndentLevel, edit_in_place::Indent, make, AstNode, HasName},
ted, NodeOrToken,
SyntaxKind::{BLOCK_EXPR, BREAK_EXPR, COMMENT, LOOP_EXPR, MATCH_GUARD, PATH_EXPR, RETURN_EXPR},
SyntaxNode, T,
};

use crate::{utils::suggest_name, AssistContext, AssistId, AssistKind, Assists};
use crate::{AssistContext, AssistId, AssistKind, Assists};

// Assist: extract_variable
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ use std::ops::Not;

use crate::{
assist_context::{AssistContext, Assists},
utils::{convert_param_list_to_arg_list, suggest_name},
utils::convert_param_list_to_arg_list,
};
use either::Either;
use hir::{db::HirDatabase, HasVisibility};
use ide_db::{
assists::{AssistId, GroupLabel},
path_transform::PathTransform,
syntax_helpers::suggest_name,
FxHashMap, FxHashSet,
};
use itertools::Itertools;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use ide_db::syntax_helpers::suggest_name;
use syntax::{
ast::{self, edit_in_place::GenericParamsOwnerEdit, make, AstNode, HasGenericParams},
ted,
};

use crate::{utils::suggest_name, AssistContext, AssistId, AssistKind, Assists};
use crate::{AssistContext, AssistId, AssistKind, Assists};

// Assist: introduce_named_generic
//
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use ide_db::syntax_helpers::suggest_name;
use syntax::{
ast::{self, make, AstNode},
ted,
};

use crate::{utils::suggest_name, AssistContext, AssistId, AssistKind, Assists};
use crate::{AssistContext, AssistId, AssistKind, Assists};

// Assist: replace_is_some_with_if_let_some
//
Expand Down
1 change: 0 additions & 1 deletion src/tools/rust-analyzer/crates/ide-assists/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use crate::assist_context::{AssistContext, SourceChangeBuilder};

mod gen_trait_fn_body;
pub(crate) mod ref_field_expr;
pub(crate) mod suggest_name;

pub(crate) fn unwrap_trivial_block(block_expr: ast::BlockExpr) -> ast::Expr {
extract_trivial_expression(&block_expr)
Expand Down
10 changes: 10 additions & 0 deletions src/tools/rust-analyzer/crates/ide-completion/src/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,16 @@ impl Completions {
}
self.add_opt(render_struct_pat(RenderContext::new(ctx), pattern_ctx, strukt, local_name));
}

pub(crate) fn suggest_name(&mut self, ctx: &CompletionContext<'_>, name: &str) {
let item = CompletionItem::new(
CompletionItemKind::Binding,
ctx.source_range(),
SmolStr::from(name),
ctx.edition,
);
item.add_to(self, ctx.db);
}
}

/// Calls the callback for each variant of the provided enum with the path to the variant.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Completes constants and paths in unqualified patterns.

use hir::{db::DefDatabase, AssocItem, ScopeDef};
use ide_db::syntax_helpers::suggest_name;
use syntax::ast::Pat;

use crate::{
Expand Down Expand Up @@ -45,6 +46,18 @@ pub(crate) fn complete_pattern(
return;
}

// Suggest name only in let-stmt and fn param
if pattern_ctx.should_suggest_name {
if let Some(suggested) = ctx
.expected_type
.as_ref()
.map(|ty| ty.strip_references())
.and_then(|ty| suggest_name::for_type(&ty, ctx.db, ctx.edition))
{
acc.suggest_name(ctx, &suggested);
}
}

let refutable = pattern_ctx.refutability == PatternRefutability::Refutable;
let single_variant_enum = |enum_: hir::Enum| ctx.db.enum_data(enum_.into()).variants.len() == 1;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ pub(crate) struct PatternContext {
pub(crate) refutability: PatternRefutability,
pub(crate) param_ctx: Option<ParamContext>,
pub(crate) has_type_ascription: bool,
pub(crate) should_suggest_name: bool,
pub(crate) parent_pat: Option<ast::Pat>,
pub(crate) ref_token: Option<SyntaxToken>,
pub(crate) mut_token: Option<SyntaxToken>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1430,10 +1430,23 @@ fn pattern_context_for(
_ => (None, None),
};

// Only suggest name in let-stmt or fn param
let should_suggest_name = matches!(
&pat,
ast::Pat::IdentPat(it)
if it.syntax()
.parent()
.map_or(false, |node| {
let kind = node.kind();
ast::LetStmt::can_cast(kind) || ast::Param::can_cast(kind)
})
);

PatternContext {
refutability,
param_ctx,
has_type_ascription,
should_suggest_name,
parent_pat: pat.syntax().parent().and_then(ast::Pat::cast),
mut_token,
ref_token,
Expand Down
73 changes: 73 additions & 0 deletions src/tools/rust-analyzer/crates/ide-completion/src/tests/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ fn foo(a$0: Tuple) {
st Unit
bn Record {…} Record { field$1 }$0
bn Tuple(…) Tuple($1)$0
bn tuple
kw mut
kw ref
"#]],
Expand Down Expand Up @@ -850,3 +851,75 @@ fn foo() {
"#,
);
}

#[test]
fn suggest_name_for_pattern() {
check_edit(
"s1",
r#"
struct S1;
fn foo() {
let $0 = S1;
}
"#,
r#"
struct S1;
fn foo() {
let s1 = S1;
}
"#,
);

check_edit(
"s1",
r#"
struct S1;
fn foo(s$0: S1) {
}
"#,
r#"
struct S1;
fn foo(s1: S1) {
}
"#,
);

// Tests for &adt
check_edit(
"s1",
r#"
struct S1;
fn foo() {
let $0 = &S1;
}
"#,
r#"
struct S1;
fn foo() {
let s1 = &S1;
}
"#,
);

// Do not suggest reserved keywords
check_empty(
r#"
struct Struct;
fn foo() {
let $0 = Struct;
}
"#,
expect![[r#"
st Struct
kw mut
kw ref
"#]],
);
}
1 change: 1 addition & 0 deletions src/tools/rust-analyzer/crates/ide-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub mod syntax_helpers {
pub mod format_string_exprs;
pub use hir::insert_whitespace_into_node;
pub mod node_ext;
pub mod suggest_name;

pub use parser::LexedStr;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
//! This module contains functions to suggest names for expressions, functions and other items

use hir::Semantics;
use ide_db::{FxHashSet, RootDatabase};
use itertools::Itertools;
use rustc_hash::FxHashSet;
use stdx::to_lower_snake_case;
use syntax::{
ast::{self, HasName},
match_ast, AstNode, Edition, SmolStr,
};

use crate::RootDatabase;

/// Trait names, that will be ignored when in `impl Trait` and `dyn Trait`
const USELESS_TRAITS: &[&str] = &["Send", "Sync", "Copy", "Clone", "Eq", "PartialEq"];

Expand Down Expand Up @@ -58,6 +60,21 @@ const USELESS_METHODS: &[&str] = &[
"into_future",
];

/// Suggest a name for given type.
///
/// The function will strip references first, and suggest name from the inner type.
///
/// - If `ty` is an ADT, it will suggest the name of the ADT.
/// + If `ty` is wrapped in `Box`, `Option` or `Result`, it will suggest the name from the inner type.
/// - If `ty` is a trait, it will suggest the name of the trait.
/// - If `ty` is an `impl Trait`, it will suggest the name of the first trait.
///
/// If the suggested name conflicts with reserved keywords, it will return `None`.
pub fn for_type(ty: &hir::Type, db: &RootDatabase, edition: Edition) -> Option<String> {
let ty = ty.strip_references();
name_of_type(&ty, db, edition)
}

/// Suggest a unique name for generic parameter.
///
/// `existing_params` is used to check if the name conflicts with existing
Expand All @@ -66,10 +83,7 @@ const USELESS_METHODS: &[&str] = &[
/// The function checks if the name conflicts with existing generic parameters.
/// If so, it will try to resolve the conflict by adding a number suffix, e.g.
/// `T`, `T0`, `T1`, ...
pub(crate) fn for_unique_generic_name(
name: &str,
existing_params: &ast::GenericParamList,
) -> SmolStr {
pub fn for_unique_generic_name(name: &str, existing_params: &ast::GenericParamList) -> SmolStr {
let param_names = existing_params
.generic_params()
.map(|param| match param {
Expand Down Expand Up @@ -101,7 +115,7 @@ pub(crate) fn for_unique_generic_name(
///
/// If the name conflicts with existing generic parameters, it will try to
/// resolve the conflict with `for_unique_generic_name`.
pub(crate) fn for_impl_trait_as_generic(
pub fn for_impl_trait_as_generic(
ty: &ast::ImplTraitType,
existing_params: &ast::GenericParamList,
) -> SmolStr {
Expand Down Expand Up @@ -132,7 +146,7 @@ pub(crate) fn for_impl_trait_as_generic(
///
/// Currently it sticks to the first name found.
// FIXME: Microoptimize and return a `SmolStr` here.
pub(crate) fn for_variable(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> String {
pub fn for_variable(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> String {
// `from_param` does not benefit from stripping
// it need the largest context possible
// so we check firstmost
Expand Down Expand Up @@ -184,7 +198,7 @@ fn normalize(name: &str) -> Option<String> {

fn is_valid_name(name: &str) -> bool {
matches!(
ide_db::syntax_helpers::LexedStr::single_token(syntax::Edition::CURRENT_FIXME, name),
super::LexedStr::single_token(syntax::Edition::CURRENT_FIXME, name),
Some((syntax::SyntaxKind::IDENT, _error))
)
}
Expand Down Expand Up @@ -270,10 +284,9 @@ fn var_name_from_pat(pat: &ast::Pat) -> Option<ast::Name> {

fn from_type(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> Option<String> {
let ty = sema.type_of_expr(expr)?.adjusted();
let ty = ty.remove_ref().unwrap_or(ty);
let edition = sema.scope(expr.syntax())?.krate().edition(sema.db);

name_of_type(&ty, sema.db, edition)
for_type(&ty, sema.db, edition)
}

fn name_of_type(ty: &hir::Type, db: &RootDatabase, edition: Edition) -> Option<String> {
Expand Down

0 comments on commit 6faf409

Please sign in to comment.