Skip to content

Commit 9eb0850

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 9eb0850

2 files changed

Lines changed: 115 additions & 15 deletions

File tree

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

Lines changed: 41 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,40 @@ 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+
};
227+
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+
});
217233

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 { ", " };
234+
let trailing = if needs_comma_after { "," } else { "" };
235+
let range = param.text_range();
221236

222-
Some((move |label: &_| format_smolstr!("{leading}{label}{trailing}"), param.text_range()))
237+
Some((
238+
move |label: &_| {
239+
let insert_text = format!("{label}{trailing}");
240+
let mut edit = TextEdit::builder();
241+
if let Some((offset, comma)) = insert_comma_before {
242+
edit.insert(offset, comma.to_owned());
243+
}
244+
edit.replace(range, insert_text);
245+
edit.finish()
246+
},
247+
range,
248+
))
223249
}
224250

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

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,20 @@ fn baz(file$0) {}
1515
fn foo(file_id: usize) {}
1616
fn bar(file_id: usize) {}
1717
fn baz(file_id: usize) {}
18+
"#,
19+
);
20+
21+
check_edit(
22+
"file_id: usize",
23+
r#"
24+
fn foo(file_id: usize) {}
25+
fn bar(file_id: usize) {}
26+
fn baz(/*...*/ file$0) {}
27+
"#,
28+
r#"
29+
fn foo(file_id: usize) {}
30+
fn bar(file_id: usize) {}
31+
fn baz(/*...*/ file_id: usize) {}
1832
"#,
1933
);
2034
}
@@ -32,6 +46,46 @@ fn baz(foo: (), file$0) {}
3246
fn foo(file_id: usize) {}
3347
fn bar(file_id: usize) {}
3448
fn baz(foo: (), file_id: usize) {}
49+
"#,
50+
);
51+
52+
check_edit(
53+
"file_id: usize",
54+
r#"
55+
fn foo(file_id: usize) {}
56+
fn bar(file_id: usize) {}
57+
fn baz(
58+
foo: ()
59+
file$0
60+
) {}
61+
"#,
62+
r#"
63+
fn foo(file_id: usize) {}
64+
fn bar(file_id: usize) {}
65+
fn baz(
66+
foo: (),
67+
file_id: usize,
68+
) {}
69+
"#,
70+
);
71+
72+
check_edit(
73+
"file_id: usize",
74+
r#"
75+
fn foo(file_id: usize) {}
76+
fn bar(file_id: usize) {}
77+
fn baz(
78+
foo: (),
79+
file$0
80+
) {}
81+
"#,
82+
r#"
83+
fn foo(file_id: usize) {}
84+
fn bar(file_id: usize) {}
85+
fn baz(
86+
foo: (),
87+
file_id: usize,
88+
) {}
3589
"#,
3690
);
3791
}
@@ -49,6 +103,26 @@ fn baz(file$0 id: u32) {}
49103
fn foo(file_id: usize) {}
50104
fn bar(file_id: usize) {}
51105
fn baz(file_id: usize, id: u32) {}
106+
"#,
107+
);
108+
109+
check_edit(
110+
"file_id: usize",
111+
r#"
112+
fn foo(file_id: usize) {}
113+
fn bar(file_id: usize) {}
114+
fn baz(
115+
file$0
116+
id: u32
117+
) {}
118+
"#,
119+
r#"
120+
fn foo(file_id: usize) {}
121+
fn bar(file_id: usize) {}
122+
fn baz(
123+
file_id: usize,
124+
id: u32
125+
) {}
52126
"#,
53127
);
54128
}

0 commit comments

Comments
 (0)