Skip to content

Commit d66ea29

Browse files
committed
fix: improve fn_param adds comma location
Example --- ```rust fn bar(file_id: usize) {} fn baz( foo: () file$0 ) {} ``` **Before this PR** ```rust fn bar(file_id: usize) {} fn baz( foo: () , file_id: usize ) {} ``` **After this PR** ```rust fn bar(file_id: usize) {} fn baz( foo: (), file_id: usize, ) {} ```
1 parent 09ee4e6 commit d66ea29

2 files changed

Lines changed: 106 additions & 15 deletions

File tree

crates/ide-completion/src/completions/fn_param.rs

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! See [`complete_fn_param`].
22
33
use hir::HirDisplay;
4-
use ide_db::FxHashMap;
4+
use ide_db::{FxHashMap, text_edit::TextEdit};
55
use itertools::Either;
66
use syntax::{
77
AstNode, Direction, SmolStr, SyntaxKind, TextRange, TextSize, ToSmolStr, algo,
@@ -40,17 +40,17 @@ pub(crate) fn complete_fn_param(
4040
} else {
4141
format_smolstr!("{qualifier}{label}")
4242
};
43-
let mk_item = |insert_text: &str, range: TextRange| {
43+
let mk_item = |edit, range: TextRange| {
4444
let mut item =
4545
CompletionItem::new(CompletionItemKind::Binding, range, label, ctx.edition);
46-
if insert_text != label {
47-
item.insert_text(insert_text);
48-
}
46+
item.text_edit(edit);
4947
item
5048
};
5149
let item = match &comma_wrapper {
52-
Some((fmt, range)) => mk_item(&fmt(&insert), *range),
53-
None => mk_item(&insert, ctx.source_range()),
50+
Some((fmt, range)) => mk_item(fmt(&insert), *range),
51+
None => {
52+
mk_item(TextEdit::replace(ctx.source_range(), insert.into()), ctx.source_range())
53+
}
5454
};
5555
// Completion lookup is omitted intentionally here.
5656
// See the full discussion: https://github.com/rust-lang/rust-analyzer/issues/12073
@@ -196,7 +196,9 @@ fn should_add_self_completions(
196196
}
197197
}
198198

199-
fn comma_wrapper(ctx: &CompletionContext<'_, '_>) -> Option<(impl Fn(&str) -> SmolStr, TextRange)> {
199+
fn comma_wrapper(
200+
ctx: &CompletionContext<'_, '_>,
201+
) -> Option<(impl Fn(&str) -> TextEdit, TextRange)> {
200202
let param =
201203
ctx.original_token.parent_ancestors().find(|node| node.kind() == SyntaxKind::PARAM)?;
202204

@@ -210,16 +212,45 @@ fn comma_wrapper(ctx: &CompletionContext<'_, '_>) -> Option<(impl Fn(&str) -> Sm
210212
let t = algo::skip_whitespace_token(t, Direction::Prev)?;
211213
t.kind()
212214
};
215+
let prev_param = param.prev_sibling().and_then(ast::Param::cast);
213216

214-
let has_trailing_comma =
215-
matches!(next_token_kind, SyntaxKind::COMMA | SyntaxKind::R_PAREN | SyntaxKind::PIPE);
216-
let trailing = if has_trailing_comma { "" } else { "," };
217+
let needs_comma_before =
218+
!matches!(prev_token_kind, SyntaxKind::COMMA | SyntaxKind::L_PAREN | SyntaxKind::PIPE);
219+
let needs_comma_after = match next_token_kind {
220+
SyntaxKind::COMMA => false,
221+
SyntaxKind::R_PAREN | SyntaxKind::PIPE => param
222+
.next_sibling_or_token()
223+
.and_then(|it| it.into_token())
224+
.is_some_and(|it| it.text().contains("\n")),
225+
_ => true,
226+
};
217227

218-
let has_leading_comma =
219-
matches!(prev_token_kind, SyntaxKind::COMMA | SyntaxKind::L_PAREN | SyntaxKind::PIPE);
220-
let leading = if has_leading_comma { "" } else { ", " };
228+
let insert_comma_before = prev_param.filter(|_| needs_comma_before).map(|prev_param| {
229+
let needs_space_before =
230+
prev_param.syntax().next_sibling_or_token().is_none_or(|it| !it.kind().is_trivia());
231+
(prev_param.syntax().text_range().end(), if needs_space_before { ", " } else { "," })
232+
});
221233

222-
Some((move |label: &_| format_smolstr!("{leading}{label}{trailing}"), param.text_range()))
234+
let needs_space_after = param.next_sibling_or_token().is_none_or(|it| !it.kind().is_trivia());
235+
let trailing = match (needs_comma_after, needs_space_after) {
236+
(true, true) => ", ",
237+
(true, false) => ",",
238+
(false, _) => "",
239+
};
240+
let range = param.text_range();
241+
242+
Some((
243+
move |label: &_| {
244+
let insert_text = format!("{label}{trailing}");
245+
let mut edit = TextEdit::builder();
246+
if let Some((offset, comma)) = insert_comma_before {
247+
edit.insert(offset, comma.to_owned());
248+
}
249+
edit.replace(range, insert_text);
250+
edit.finish()
251+
},
252+
range,
253+
))
223254
}
224255

225256
fn is_simple_param(param: &ast::Param) -> bool {

crates/ide-completion/src/tests/fn_param.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,46 @@ fn baz(foo: (), file$0) {}
3232
fn foo(file_id: usize) {}
3333
fn bar(file_id: usize) {}
3434
fn baz(foo: (), file_id: usize) {}
35+
"#,
36+
);
37+
38+
check_edit(
39+
"file_id: usize",
40+
r#"
41+
fn foo(file_id: usize) {}
42+
fn bar(file_id: usize) {}
43+
fn baz(
44+
foo: ()
45+
file$0
46+
) {}
47+
"#,
48+
r#"
49+
fn foo(file_id: usize) {}
50+
fn bar(file_id: usize) {}
51+
fn baz(
52+
foo: (),
53+
file_id: usize,
54+
) {}
55+
"#,
56+
);
57+
58+
check_edit(
59+
"file_id: usize",
60+
r#"
61+
fn foo(file_id: usize) {}
62+
fn bar(file_id: usize) {}
63+
fn baz(
64+
foo: (),
65+
file$0
66+
) {}
67+
"#,
68+
r#"
69+
fn foo(file_id: usize) {}
70+
fn bar(file_id: usize) {}
71+
fn baz(
72+
foo: (),
73+
file_id: usize,
74+
) {}
3575
"#,
3676
);
3777
}
@@ -49,6 +89,26 @@ fn baz(file$0 id: u32) {}
4989
fn foo(file_id: usize) {}
5090
fn bar(file_id: usize) {}
5191
fn baz(file_id: usize, id: u32) {}
92+
"#,
93+
);
94+
95+
check_edit(
96+
"file_id: usize",
97+
r#"
98+
fn foo(file_id: usize) {}
99+
fn bar(file_id: usize) {}
100+
fn baz(
101+
file$0
102+
id: u32
103+
) {}
104+
"#,
105+
r#"
106+
fn foo(file_id: usize) {}
107+
fn bar(file_id: usize) {}
108+
fn baz(
109+
file_id: usize,
110+
id: u32
111+
) {}
52112
"#,
53113
);
54114
}

0 commit comments

Comments
 (0)