Skip to content

Commit fe6fdb7

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 fe6fdb7

2 files changed

Lines changed: 140 additions & 19 deletions

File tree

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

Lines changed: 44 additions & 19 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

@@ -205,21 +207,44 @@ fn comma_wrapper(ctx: &CompletionContext<'_, '_>) -> Option<(impl Fn(&str) -> Sm
205207
let t = algo::skip_whitespace_token(t, Direction::Next)?;
206208
t.kind()
207209
};
208-
let prev_token_kind = {
209-
let t = param.first_token()?.prev_token()?;
210-
let t = algo::skip_whitespace_token(t, Direction::Prev)?;
211-
t.kind()
210+
let prev_param = param.prev_sibling().and_then(ast::Param::cast);
211+
212+
let needs_comma_before = prev_param
213+
.as_ref()
214+
.and_then(|it| {
215+
algo::skip_trivia_token(it.syntax().last_token()?.next_token()?, Direction::Next)
216+
})
217+
.is_some_and(|it| it.kind() != SyntaxKind::COMMA);
218+
let needs_comma_after = match next_token_kind {
219+
SyntaxKind::COMMA => false,
220+
SyntaxKind::R_PAREN | SyntaxKind::PIPE => param
221+
.next_sibling_or_token()
222+
.and_then(|it| it.into_token())
223+
.is_some_and(|it| it.text().contains("\n")),
224+
_ => true,
212225
};
213226

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 { "," };
227+
let insert_comma_before = prev_param.filter(|_| needs_comma_before).map(|prev_param| {
228+
let needs_space_before =
229+
prev_param.syntax().next_sibling_or_token().is_none_or(|it| !it.kind().is_trivia());
230+
(prev_param.syntax().text_range().end(), if needs_space_before { ", " } else { "," })
231+
});
217232

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

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

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

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

Lines changed: 96 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,68 @@ 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+
) {}
89+
"#,
90+
);
91+
92+
check_edit(
93+
"file_id: usize",
94+
r#"
95+
fn foo(file_id: usize) {}
96+
fn bar(file_id: usize) {}
97+
fn baz(
98+
foo: (),
99+
// comment
100+
file$0
101+
) {}
102+
"#,
103+
r#"
104+
fn foo(file_id: usize) {}
105+
fn bar(file_id: usize) {}
106+
fn baz(
107+
foo: (),
108+
// comment
109+
file_id: usize,
110+
) {}
35111
"#,
36112
);
37113
}
@@ -49,6 +125,26 @@ fn baz(file$0 id: u32) {}
49125
fn foo(file_id: usize) {}
50126
fn bar(file_id: usize) {}
51127
fn baz(file_id: usize, id: u32) {}
128+
"#,
129+
);
130+
131+
check_edit(
132+
"file_id: usize",
133+
r#"
134+
fn foo(file_id: usize) {}
135+
fn bar(file_id: usize) {}
136+
fn baz(
137+
file$0
138+
id: u32
139+
) {}
140+
"#,
141+
r#"
142+
fn foo(file_id: usize) {}
143+
fn bar(file_id: usize) {}
144+
fn baz(
145+
file_id: usize,
146+
id: u32
147+
) {}
52148
"#,
53149
);
54150
}

0 commit comments

Comments
 (0)