Automatically add semicolon when completing unit-returning functions

But provide a config to suppress that.

I didn't check whether we are in statement expression position, because this is hard in completion (due to the natural incompleteness of source code when completion is invoked), and anyway using function returning unit as an argument to something seems... dubious.
This commit is contained in:
Chayim Refael Friedman 2024-09-01 07:02:40 +03:00
parent 08c7bbc2db
commit 779a7cb0e3
12 changed files with 217 additions and 40 deletions

View file

@ -600,7 +600,7 @@ fn foo(a: A) { a.$0 }
struct A {} struct A {}
trait Trait { fn the_method(&self); } trait Trait { fn the_method(&self); }
impl Trait for A {} impl Trait for A {}
fn foo(a: A) { a.the_method()$0 } fn foo(a: A) { a.the_method();$0 }
"#, "#,
); );
} }

View file

@ -19,6 +19,7 @@ pub struct CompletionConfig {
pub term_search_fuel: u64, pub term_search_fuel: u64,
pub full_function_signatures: bool, pub full_function_signatures: bool,
pub callable: Option<CallableSnippets>, pub callable: Option<CallableSnippets>,
pub add_semicolon_to_unit: bool,
pub snippet_cap: Option<SnippetCap>, pub snippet_cap: Option<SnippetCap>,
pub insert_use: InsertUseConfig, pub insert_use: InsertUseConfig,
pub prefer_no_std: bool, pub prefer_no_std: bool,

View file

@ -1185,7 +1185,7 @@ fn main() { fo$0 }
label: "main()", label: "main()",
source_range: 68..70, source_range: 68..70,
delete: 68..70, delete: 68..70,
insert: "main()$0", insert: "main();$0",
kind: SymbolKind( kind: SymbolKind(
Function, Function,
), ),
@ -1244,7 +1244,7 @@ fn main() { let _: m::Spam = S$0 }
label: "main()", label: "main()",
source_range: 75..76, source_range: 75..76,
delete: 75..76, delete: 75..76,
insert: "main()$0", insert: "main();$0",
kind: SymbolKind( kind: SymbolKind(
Function, Function,
), ),
@ -1331,7 +1331,7 @@ fn main() { som$0 }
label: "main()", label: "main()",
source_range: 56..59, source_range: 56..59,
delete: 56..59, delete: 56..59,
insert: "main()$0", insert: "main();$0",
kind: SymbolKind( kind: SymbolKind(
Function, Function,
), ),
@ -1342,7 +1342,7 @@ fn main() { som$0 }
label: "something_deprecated()", label: "something_deprecated()",
source_range: 56..59, source_range: 56..59,
delete: 56..59, delete: 56..59,
insert: "something_deprecated()$0", insert: "something_deprecated();$0",
kind: SymbolKind( kind: SymbolKind(
Function, Function,
), ),
@ -1413,7 +1413,7 @@ impl S {
label: "bar()", label: "bar()",
source_range: 94..94, source_range: 94..94,
delete: 94..94, delete: 94..94,
insert: "bar()$0", insert: "bar();$0",
kind: SymbolKind( kind: SymbolKind(
Method, Method,
), ),
@ -1540,7 +1540,7 @@ fn foo(s: S) { s.$0 }
label: "the_method()", label: "the_method()",
source_range: 81..81, source_range: 81..81,
delete: 81..81, delete: 81..81,
insert: "the_method()$0", insert: "the_method();$0",
kind: SymbolKind( kind: SymbolKind(
Method, Method,
), ),
@ -2789,7 +2789,7 @@ fn main() {
r#" r#"
mod m { pub fn r#type {} } mod m { pub fn r#type {} }
fn main() { fn main() {
m::r#type()$0 m::r#type();$0
} }
"#, "#,
) )
@ -2963,7 +2963,7 @@ fn main() {
label: "flush()", label: "flush()",
source_range: 193..193, source_range: 193..193,
delete: 193..193, delete: 193..193,
insert: "flush()$0", insert: "flush();$0",
kind: SymbolKind( kind: SymbolKind(
Method, Method,
), ),
@ -2990,7 +2990,7 @@ fn main() {
label: "write()", label: "write()",
source_range: 193..193, source_range: 193..193,
delete: 193..193, delete: 193..193,
insert: "write()$0", insert: "write();$0",
kind: SymbolKind( kind: SymbolKind(
Method, Method,
), ),

View file

@ -1,10 +1,12 @@
//! Renderer for function calls. //! Renderer for function calls.
use std::ops::ControlFlow;
use hir::{db::HirDatabase, AsAssocItem, HirDisplay}; use hir::{db::HirDatabase, AsAssocItem, HirDisplay};
use ide_db::{SnippetCap, SymbolKind}; use ide_db::{SnippetCap, SymbolKind};
use itertools::Itertools; use itertools::Itertools;
use stdx::{format_to, to_lower_snake_case}; use stdx::{format_to, to_lower_snake_case};
use syntax::{format_smolstr, AstNode, Edition, SmolStr, ToSmolStr}; use syntax::{ast, format_smolstr, AstNode, Edition, SmolStr, SyntaxKind, ToSmolStr, T};
use crate::{ use crate::{
context::{CompletionContext, DotAccess, DotAccessKind, PathCompletionCtx, PathKind}, context::{CompletionContext, DotAccess, DotAccessKind, PathCompletionCtx, PathKind},
@ -160,7 +162,16 @@ fn render(
.lookup_by(name.unescaped().display(db).to_smolstr()); .lookup_by(name.unescaped().display(db).to_smolstr());
if let Some((cap, (self_param, params))) = complete_call_parens { if let Some((cap, (self_param, params))) = complete_call_parens {
add_call_parens(&mut item, completion, cap, call, escaped_call, self_param, params); add_call_parens(
&mut item,
completion,
cap,
call,
escaped_call,
self_param,
params,
&ret_type,
);
} }
match ctx.import_to_add { match ctx.import_to_add {
@ -217,10 +228,11 @@ pub(super) fn add_call_parens<'b>(
escaped_name: SmolStr, escaped_name: SmolStr,
self_param: Option<hir::SelfParam>, self_param: Option<hir::SelfParam>,
params: Vec<hir::Param>, params: Vec<hir::Param>,
ret_type: &hir::Type,
) -> &'b mut Builder { ) -> &'b mut Builder {
cov_mark::hit!(inserts_parens_for_function_calls); cov_mark::hit!(inserts_parens_for_function_calls);
let (snippet, label_suffix) = if self_param.is_none() && params.is_empty() { let (mut snippet, label_suffix) = if self_param.is_none() && params.is_empty() {
(format!("{escaped_name}()$0"), "()") (format!("{escaped_name}()$0"), "()")
} else { } else {
builder.trigger_call_info(); builder.trigger_call_info();
@ -265,6 +277,35 @@ pub(super) fn add_call_parens<'b>(
(snippet, "(…)") (snippet, "(…)")
}; };
if ret_type.is_unit() && ctx.config.add_semicolon_to_unit {
let next_non_trivia_token =
std::iter::successors(ctx.token.next_token(), |it| it.next_token())
.find(|it| !it.kind().is_trivia());
let in_match_arm = ctx.token.parent_ancestors().try_for_each(|ancestor| {
if ast::MatchArm::can_cast(ancestor.kind()) {
ControlFlow::Break(true)
} else if matches!(ancestor.kind(), SyntaxKind::EXPR_STMT | SyntaxKind::BLOCK_EXPR) {
ControlFlow::Break(false)
} else {
ControlFlow::Continue(())
}
});
// FIXME: This will assume expr macros are not inside match, we need to somehow go to the "parent" of the root node.
let in_match_arm = match in_match_arm {
ControlFlow::Continue(()) => false,
ControlFlow::Break(it) => it,
};
let complete_token = if in_match_arm { T![,] } else { T![;] };
if next_non_trivia_token.map(|it| it.kind()) != Some(complete_token) {
cov_mark::hit!(complete_semicolon);
let ch = if in_match_arm { ',' } else { ';' };
if snippet.ends_with("$0") {
snippet.insert(snippet.len() - "$0".len(), ch);
} else {
snippet.push(ch);
}
}
}
builder.label(SmolStr::from_iter([&name, label_suffix])).insert_snippet(cap, snippet) builder.label(SmolStr::from_iter([&name, label_suffix])).insert_snippet(cap, snippet)
} }
@ -393,7 +434,7 @@ fn main() { no_$0 }
"#, "#,
r#" r#"
fn no_args() {} fn no_args() {}
fn main() { no_args()$0 } fn main() { no_args();$0 }
"#, "#,
); );
@ -405,7 +446,7 @@ fn main() { with_$0 }
"#, "#,
r#" r#"
fn with_args(x: i32, y: String) {} fn with_args(x: i32, y: String) {}
fn main() { with_args(${1:x}, ${2:y})$0 } fn main() { with_args(${1:x}, ${2:y});$0 }
"#, "#,
); );
@ -414,14 +455,14 @@ fn main() { with_args(${1:x}, ${2:y})$0 }
r#" r#"
struct S; struct S;
impl S { impl S {
fn foo(&self) {} fn foo(&self) -> i32 { 0 }
} }
fn bar(s: &S) { s.f$0 } fn bar(s: &S) { s.f$0 }
"#, "#,
r#" r#"
struct S; struct S;
impl S { impl S {
fn foo(&self) {} fn foo(&self) -> i32 { 0 }
} }
fn bar(s: &S) { s.foo()$0 } fn bar(s: &S) { s.foo()$0 }
"#, "#,
@ -444,7 +485,7 @@ impl S {
fn foo(&self, x: i32) {} fn foo(&self, x: i32) {}
} }
fn bar(s: &S) { fn bar(s: &S) {
s.foo(${1:x})$0 s.foo(${1:x});$0
} }
"#, "#,
); );
@ -463,7 +504,7 @@ impl S {
struct S {} struct S {}
impl S { impl S {
fn foo(&self, x: i32) { fn foo(&self, x: i32) {
self.foo(${1:x})$0 self.foo(${1:x});$0
} }
} }
"#, "#,
@ -486,7 +527,7 @@ struct S;
impl S { impl S {
fn foo(&self) {} fn foo(&self) {}
} }
fn main() { S::foo(${1:&self})$0 } fn main() { S::foo(${1:&self});$0 }
"#, "#,
); );
} }
@ -503,7 +544,7 @@ fn main() { with_$0 }
"#, "#,
r#" r#"
fn with_args(x: i32, y: String) {} fn with_args(x: i32, y: String) {}
fn main() { with_args($0) } fn main() { with_args($0); }
"#, "#,
); );
} }
@ -518,7 +559,7 @@ fn main() { f$0 }
"#, "#,
r#" r#"
fn foo(_foo: i32, ___bar: bool, ho_ge_: String) {} fn foo(_foo: i32, ___bar: bool, ho_ge_: String) {}
fn main() { foo(${1:foo}, ${2:bar}, ${3:ho_ge_})$0 } fn main() { foo(${1:foo}, ${2:bar}, ${3:ho_ge_});$0 }
"#, "#,
); );
} }
@ -540,7 +581,7 @@ struct Foo {}
fn ref_arg(x: &Foo) {} fn ref_arg(x: &Foo) {}
fn main() { fn main() {
let x = Foo {}; let x = Foo {};
ref_arg(${1:&x})$0 ref_arg(${1:&x});$0
} }
"#, "#,
); );
@ -563,7 +604,7 @@ struct Foo {}
fn ref_arg(x: &mut Foo) {} fn ref_arg(x: &mut Foo) {}
fn main() { fn main() {
let x = Foo {}; let x = Foo {};
ref_arg(${1:&mut x})$0 ref_arg(${1:&mut x});$0
} }
"#, "#,
); );
@ -596,7 +637,7 @@ impl Bar {
fn main() { fn main() {
let x = Foo {}; let x = Foo {};
let y = Bar {}; let y = Bar {};
y.apply_foo(${1:&x})$0 y.apply_foo(${1:&x});$0
} }
"#, "#,
); );
@ -617,7 +658,7 @@ fn main() {
fn take_mutably(mut x: &i32) {} fn take_mutably(mut x: &i32) {}
fn main() { fn main() {
take_mutably(${1:x})$0 take_mutably(${1:x});$0
} }
"#, "#,
); );
@ -650,7 +691,7 @@ fn qux(Foo { bar }: Foo) {
} }
fn main() { fn main() {
qux(${1:foo})$0 qux(${1:foo});$0
} }
"#, "#,
); );
@ -736,6 +777,115 @@ fn g(foo: ()#[baz = "qux"] mut ba$0)
r#" r#"
fn f(foo: (), #[baz = "qux"] mut bar: u32) {} fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
fn g(foo: (), #[baz = "qux"] mut bar: u32) fn g(foo: (), #[baz = "qux"] mut bar: u32)
"#,
);
}
#[test]
fn complete_semicolon_for_unit() {
cov_mark::check!(complete_semicolon);
check_edit(
r#"foo"#,
r#"
fn foo() {}
fn bar() {
foo$0
}
"#,
r#"
fn foo() {}
fn bar() {
foo();$0
}
"#,
);
check_edit(
r#"foo"#,
r#"
fn foo(a: i32) {}
fn bar() {
foo$0
}
"#,
r#"
fn foo(a: i32) {}
fn bar() {
foo(${1:a});$0
}
"#,
);
check_edit(
r#"foo"#,
r#"
fn foo(a: i32) {}
fn bar() {
foo$0;
}
"#,
r#"
fn foo(a: i32) {}
fn bar() {
foo(${1:a})$0;
}
"#,
);
check_edit_with_config(
CompletionConfig { add_semicolon_to_unit: false, ..TEST_CONFIG },
r#"foo"#,
r#"
fn foo(a: i32) {}
fn bar() {
foo$0
}
"#,
r#"
fn foo(a: i32) {}
fn bar() {
foo(${1:a})$0
}
"#,
);
}
#[test]
fn complete_comma_for_unit_match_arm() {
cov_mark::check!(complete_semicolon);
check_edit(
r#"foo"#,
r#"
fn foo() {}
fn bar() {
match Some(false) {
v => fo$0
}
}
"#,
r#"
fn foo() {}
fn bar() {
match Some(false) {
v => foo(),$0
}
}
"#,
);
check_edit(
r#"foo"#,
r#"
fn foo() {}
fn bar() {
match Some(false) {
v => fo$0,
}
}
"#,
r#"
fn foo() {}
fn bar() {
match Some(false) {
v => foo()$0,
}
}
"#, "#,
); );
} }

View file

@ -70,6 +70,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig {
term_search_fuel: 200, term_search_fuel: 200,
full_function_signatures: false, full_function_signatures: false,
callable: Some(CallableSnippets::FillArguments), callable: Some(CallableSnippets::FillArguments),
add_semicolon_to_unit: true,
snippet_cap: SnippetCap::new(true), snippet_cap: SnippetCap::new(true),
insert_use: InsertUseConfig { insert_use: InsertUseConfig {
granularity: ImportGranularity::Crate, granularity: ImportGranularity::Crate,

View file

@ -53,7 +53,7 @@ fn main() {
use dep::io::stdin; use dep::io::stdin;
fn main() { fn main() {
stdin()$0 stdin();$0
} }
"#, "#,
); );
@ -274,7 +274,7 @@ fn trait_function_fuzzy_completion() {
use dep::test_mod::TestTrait; use dep::test_mod::TestTrait;
fn main() { fn main() {
dep::test_mod::TestStruct::weird_function()$0 dep::test_mod::TestStruct::weird_function();$0
} }
"#, "#,
); );
@ -368,7 +368,7 @@ use dep::test_mod::TestTrait;
fn main() { fn main() {
let test_struct = dep::test_mod::TestStruct {}; let test_struct = dep::test_mod::TestStruct {};
test_struct.random_method()$0 test_struct.random_method();$0
} }
"#, "#,
); );
@ -419,7 +419,7 @@ impl foo::TestTrait for fundamental::Box<TestStruct> {
fn main() { fn main() {
let t = fundamental::Box(TestStruct); let t = fundamental::Box(TestStruct);
t.some_method()$0 t.some_method();$0
} }
"#, "#,
); );
@ -466,7 +466,7 @@ impl foo::TestTrait for &TestStruct {
fn main() { fn main() {
let t = &TestStruct; let t = &TestStruct;
t.some_method()$0 t.some_method();$0
} }
"#, "#,
); );
@ -507,7 +507,7 @@ fn completion<T: Wrapper>(whatever: T) {
use foo::{NotInScope, Wrapper}; use foo::{NotInScope, Wrapper};
fn completion<T: Wrapper>(whatever: T) { fn completion<T: Wrapper>(whatever: T) {
whatever.inner().not_in_scope()$0 whatever.inner().not_in_scope();$0
} }
"#, "#,
); );
@ -579,7 +579,7 @@ fn main() {
use dep::test_mod::TestTrait; use dep::test_mod::TestTrait;
fn main() { fn main() {
dep::test_mod::TestAlias::random_method()$0 dep::test_mod::TestAlias::random_method();$0
} }
"#, "#,
); );
@ -702,7 +702,7 @@ fn main() {
use dep::test_mod::TestTrait; use dep::test_mod::TestTrait;
fn main() { fn main() {
dep::test_mod::TestStruct::another_function()$0 dep::test_mod::TestStruct::another_function();$0
} }
"#, "#,
); );

View file

@ -30,7 +30,7 @@ fn foo() {
"#, "#,
expect![[r#" expect![[r#"
fn foo() { fn foo() {
a::dyn()$0 a::dyn();$0
"#]], "#]],
); );
@ -45,7 +45,7 @@ fn foo() {
"#, "#,
expect![[r#" expect![[r#"
fn foo() { fn foo() {
a::dyn()$0 a::dyn();$0
"#]], "#]],
); );
} }
@ -63,7 +63,7 @@ fn foo() {
"#, "#,
expect![[r#" expect![[r#"
fn foo() { fn foo() {
a::r#dyn()$0 a::r#dyn();$0
"#]], "#]],
); );
@ -78,7 +78,7 @@ fn foo() {
"#, "#,
expect![[r#" expect![[r#"
fn foo() { fn foo() {
a::r#dyn()$0 a::r#dyn();$0
"#]], "#]],
); );
} }

View file

@ -63,7 +63,7 @@ fn _alpha() {}
"#, "#,
r#" r#"
fn main() { fn main() {
_alpha()$0 _alpha();$0
} }
fn _alpha() {} fn _alpha() {}
"#, "#,

View file

@ -420,6 +420,10 @@ config_data! {
assist_termSearch_fuel: usize = 1800, assist_termSearch_fuel: usize = 1800,
/// Whether to automatically add a semicolon when completing unit-returning functions.
///
/// In `match` arms it completes a comma instead.
completion_addSemicolonToUnit: bool = true,
/// Toggles the additional completions that automatically add imports when completed. /// Toggles the additional completions that automatically add imports when completed.
/// Note that your client must specify the `additionalTextEdits` LSP client capability to truly have this feature enabled. /// Note that your client must specify the `additionalTextEdits` LSP client capability to truly have this feature enabled.
completion_autoimport_enable: bool = true, completion_autoimport_enable: bool = true,
@ -1441,6 +1445,7 @@ impl Config {
CallableCompletionDef::AddParentheses => Some(CallableSnippets::AddParentheses), CallableCompletionDef::AddParentheses => Some(CallableSnippets::AddParentheses),
CallableCompletionDef::None => None, CallableCompletionDef::None => None,
}, },
add_semicolon_to_unit: *self.completion_addSemicolonToUnit(source_root),
snippet_cap: SnippetCap::new(self.completion_snippet()), snippet_cap: SnippetCap::new(self.completion_snippet()),
insert_use: self.insert_use_config(source_root), insert_use: self.insert_use_config(source_root),
prefer_no_std: self.imports_preferNoStd(source_root).to_owned(), prefer_no_std: self.imports_preferNoStd(source_root).to_owned(),

View file

@ -167,6 +167,7 @@ fn integrated_completion_benchmark() {
prefer_absolute: false, prefer_absolute: false,
snippets: Vec::new(), snippets: Vec::new(),
limit: None, limit: None,
add_semicolon_to_unit: true,
}; };
let position = let position =
FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() };
@ -213,6 +214,7 @@ fn integrated_completion_benchmark() {
prefer_absolute: false, prefer_absolute: false,
snippets: Vec::new(), snippets: Vec::new(),
limit: None, limit: None,
add_semicolon_to_unit: true,
}; };
let position = let position =
FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() };
@ -257,6 +259,7 @@ fn integrated_completion_benchmark() {
prefer_absolute: false, prefer_absolute: false,
snippets: Vec::new(), snippets: Vec::new(),
limit: None, limit: None,
add_semicolon_to_unit: true,
}; };
let position = let position =
FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() };

View file

@ -261,6 +261,13 @@ Aliased as `"checkOnSave.targets"`.
Whether `--workspace` should be passed to `cargo check`. Whether `--workspace` should be passed to `cargo check`.
If false, `-p <package>` will be passed instead. If false, `-p <package>` will be passed instead.
-- --
[[rust-analyzer.completion.addSemicolonToUnit]]rust-analyzer.completion.addSemicolonToUnit (default: `true`)::
+
--
Whether to automatically add a semicolon when completing unit-returning functions.
In `match` arms it completes a comma instead.
--
[[rust-analyzer.completion.autoimport.enable]]rust-analyzer.completion.autoimport.enable (default: `true`):: [[rust-analyzer.completion.autoimport.enable]]rust-analyzer.completion.autoimport.enable (default: `true`)::
+ +
-- --

View file

@ -1027,6 +1027,16 @@
} }
} }
}, },
{
"title": "completion",
"properties": {
"rust-analyzer.completion.addSemicolonToUnit": {
"markdownDescription": "Whether to automatically add a semicolon when completing unit-returning functions.\n\nIn `match` arms it completes a comma instead.",
"default": true,
"type": "boolean"
}
}
},
{ {
"title": "completion", "title": "completion",
"properties": { "properties": {