From ca262a34acdde2f72d15535e9f535021a9cb8d62 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sun, 1 Sep 2024 07:02:40 +0300 Subject: [PATCH] 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. --- .../ide-completion/src/completions/dot.rs | 2 +- .../crates/ide-completion/src/config.rs | 1 + .../crates/ide-completion/src/render.rs | 18 +- .../ide-completion/src/render/function.rs | 184 ++++++++++++++++-- .../crates/ide-completion/src/tests.rs | 1 + .../ide-completion/src/tests/flyimport.rs | 16 +- .../src/tests/raw_identifiers.rs | 8 +- .../ide-completion/src/tests/special.rs | 2 +- .../crates/rust-analyzer/src/config.rs | 5 + .../src/integrated_benchmarks.rs | 3 + .../docs/user/generated_config.adoc | 7 + .../rust-analyzer/editors/code/package.json | 10 + 12 files changed, 217 insertions(+), 40 deletions(-) diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/completions/dot.rs b/src/tools/rust-analyzer/crates/ide-completion/src/completions/dot.rs index d55bc3ea5d03..f2c360a9d5bf 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/completions/dot.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/completions/dot.rs @@ -600,7 +600,7 @@ fn foo(a: A) { a.$0 } struct A {} trait Trait { fn the_method(&self); } impl Trait for A {} -fn foo(a: A) { a.the_method()$0 } +fn foo(a: A) { a.the_method();$0 } "#, ); } diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/config.rs b/src/tools/rust-analyzer/crates/ide-completion/src/config.rs index d885b82ec90d..0d403f49b7a8 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/config.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/config.rs @@ -19,6 +19,7 @@ pub struct CompletionConfig { pub term_search_fuel: u64, pub full_function_signatures: bool, pub callable: Option, + pub add_semicolon_to_unit: bool, pub snippet_cap: Option, pub insert_use: InsertUseConfig, pub prefer_no_std: bool, diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/render.rs b/src/tools/rust-analyzer/crates/ide-completion/src/render.rs index 2bec2eb87bc4..f2e9de9382c6 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/render.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/render.rs @@ -1185,7 +1185,7 @@ fn main() { fo$0 } label: "main()", source_range: 68..70, delete: 68..70, - insert: "main()$0", + insert: "main();$0", kind: SymbolKind( Function, ), @@ -1244,7 +1244,7 @@ fn main() { let _: m::Spam = S$0 } label: "main()", source_range: 75..76, delete: 75..76, - insert: "main()$0", + insert: "main();$0", kind: SymbolKind( Function, ), @@ -1331,7 +1331,7 @@ fn main() { som$0 } label: "main()", source_range: 56..59, delete: 56..59, - insert: "main()$0", + insert: "main();$0", kind: SymbolKind( Function, ), @@ -1342,7 +1342,7 @@ fn main() { som$0 } label: "something_deprecated()", source_range: 56..59, delete: 56..59, - insert: "something_deprecated()$0", + insert: "something_deprecated();$0", kind: SymbolKind( Function, ), @@ -1413,7 +1413,7 @@ impl S { label: "bar()", source_range: 94..94, delete: 94..94, - insert: "bar()$0", + insert: "bar();$0", kind: SymbolKind( Method, ), @@ -1540,7 +1540,7 @@ fn foo(s: S) { s.$0 } label: "the_method()", source_range: 81..81, delete: 81..81, - insert: "the_method()$0", + insert: "the_method();$0", kind: SymbolKind( Method, ), @@ -2789,7 +2789,7 @@ fn main() { r#" mod m { pub fn r#type {} } fn main() { - m::r#type()$0 + m::r#type();$0 } "#, ) @@ -2963,7 +2963,7 @@ fn main() { label: "flush()", source_range: 193..193, delete: 193..193, - insert: "flush()$0", + insert: "flush();$0", kind: SymbolKind( Method, ), @@ -2990,7 +2990,7 @@ fn main() { label: "write()", source_range: 193..193, delete: 193..193, - insert: "write()$0", + insert: "write();$0", kind: SymbolKind( Method, ), diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs b/src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs index 29820cb2036d..7e5e69665f1f 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs @@ -1,10 +1,12 @@ //! Renderer for function calls. +use std::ops::ControlFlow; + use hir::{db::HirDatabase, AsAssocItem, HirDisplay}; use ide_db::{SnippetCap, SymbolKind}; use itertools::Itertools; 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::{ context::{CompletionContext, DotAccess, DotAccessKind, PathCompletionCtx, PathKind}, @@ -160,7 +162,16 @@ fn render( .lookup_by(name.unescaped().display(db).to_smolstr()); 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 { @@ -217,10 +228,11 @@ pub(super) fn add_call_parens<'b>( escaped_name: SmolStr, self_param: Option, params: Vec, + ret_type: &hir::Type, ) -> &'b mut Builder { 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"), "()") } else { builder.trigger_call_info(); @@ -265,6 +277,35 @@ pub(super) fn add_call_parens<'b>( (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) } @@ -393,7 +434,7 @@ fn main() { no_$0 } "#, r#" fn no_args() {} -fn main() { no_args()$0 } +fn main() { no_args();$0 } "#, ); @@ -405,7 +446,7 @@ fn main() { with_$0 } "#, r#" 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#" struct S; impl S { - fn foo(&self) {} + fn foo(&self) -> i32 { 0 } } fn bar(s: &S) { s.f$0 } "#, r#" struct S; impl S { - fn foo(&self) {} + fn foo(&self) -> i32 { 0 } } fn bar(s: &S) { s.foo()$0 } "#, @@ -444,7 +485,7 @@ impl S { fn foo(&self, x: i32) {} } fn bar(s: &S) { - s.foo(${1:x})$0 + s.foo(${1:x});$0 } "#, ); @@ -463,7 +504,7 @@ impl S { struct S {} impl S { fn foo(&self, x: i32) { - self.foo(${1:x})$0 + self.foo(${1:x});$0 } } "#, @@ -486,7 +527,7 @@ struct S; impl S { 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#" 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#" 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 main() { 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 main() { let x = Foo {}; - ref_arg(${1:&mut x})$0 + ref_arg(${1:&mut x});$0 } "#, ); @@ -596,7 +637,7 @@ impl Bar { fn main() { let x = Foo {}; 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 main() { - take_mutably(${1:x})$0 + take_mutably(${1:x});$0 } "#, ); @@ -650,7 +691,7 @@ fn qux(Foo { bar }: Foo) { } fn main() { - qux(${1:foo})$0 + qux(${1:foo});$0 } "#, ); @@ -736,6 +777,115 @@ fn g(foo: ()#[baz = "qux"] mut ba$0) r#" fn f(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, + } +} "#, ); } diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/tests.rs b/src/tools/rust-analyzer/crates/ide-completion/src/tests.rs index 04ba7e1f41b6..9d77d9700718 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/tests.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/tests.rs @@ -70,6 +70,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig { term_search_fuel: 200, full_function_signatures: false, callable: Some(CallableSnippets::FillArguments), + add_semicolon_to_unit: true, snippet_cap: SnippetCap::new(true), insert_use: InsertUseConfig { granularity: ImportGranularity::Crate, diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs b/src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs index 8350fdcc4c07..0b532064fb2b 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/tests/flyimport.rs @@ -53,7 +53,7 @@ fn main() { use dep::io::stdin; fn main() { - stdin()$0 + stdin();$0 } "#, ); @@ -274,7 +274,7 @@ fn trait_function_fuzzy_completion() { use dep::test_mod::TestTrait; 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() { 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 { fn main() { let t = fundamental::Box(TestStruct); - t.some_method()$0 + t.some_method();$0 } "#, ); @@ -466,7 +466,7 @@ impl foo::TestTrait for &TestStruct { fn main() { let t = &TestStruct; - t.some_method()$0 + t.some_method();$0 } "#, ); @@ -507,7 +507,7 @@ fn completion(whatever: T) { use foo::{NotInScope, Wrapper}; fn completion(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; 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; fn main() { - dep::test_mod::TestStruct::another_function()$0 + dep::test_mod::TestStruct::another_function();$0 } "#, ); diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/tests/raw_identifiers.rs b/src/tools/rust-analyzer/crates/ide-completion/src/tests/raw_identifiers.rs index bc630189edc9..d81b3d697aa8 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/tests/raw_identifiers.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/tests/raw_identifiers.rs @@ -30,7 +30,7 @@ fn foo() { "#, expect![[r#" fn foo() { - a::dyn()$0 + a::dyn();$0 "#]], ); @@ -45,7 +45,7 @@ fn foo() { "#, expect![[r#" fn foo() { - a::dyn()$0 + a::dyn();$0 "#]], ); } @@ -63,7 +63,7 @@ fn foo() { "#, expect![[r#" fn foo() { - a::r#dyn()$0 + a::r#dyn();$0 "#]], ); @@ -78,7 +78,7 @@ fn foo() { "#, expect![[r#" fn foo() { - a::r#dyn()$0 + a::r#dyn();$0 "#]], ); } diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/tests/special.rs b/src/tools/rust-analyzer/crates/ide-completion/src/tests/special.rs index 2ae7d37889e5..508f6248dd41 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/tests/special.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/tests/special.rs @@ -63,7 +63,7 @@ fn _alpha() {} "#, r#" fn main() { - _alpha()$0 + _alpha();$0 } fn _alpha() {} "#, diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs index e4e8c9c6d9f9..35be6d0cf7a1 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs @@ -420,6 +420,10 @@ config_data! { 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. /// Note that your client must specify the `additionalTextEdits` LSP client capability to truly have this feature enabled. completion_autoimport_enable: bool = true, @@ -1441,6 +1445,7 @@ impl Config { CallableCompletionDef::AddParentheses => Some(CallableSnippets::AddParentheses), CallableCompletionDef::None => None, }, + add_semicolon_to_unit: *self.completion_addSemicolonToUnit(source_root), snippet_cap: SnippetCap::new(self.completion_snippet()), insert_use: self.insert_use_config(source_root), prefer_no_std: self.imports_preferNoStd(source_root).to_owned(), diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/integrated_benchmarks.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/integrated_benchmarks.rs index 28f4b809d6c8..118469df730c 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/integrated_benchmarks.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/integrated_benchmarks.rs @@ -167,6 +167,7 @@ fn integrated_completion_benchmark() { prefer_absolute: false, snippets: Vec::new(), limit: None, + add_semicolon_to_unit: true, }; let position = FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; @@ -213,6 +214,7 @@ fn integrated_completion_benchmark() { prefer_absolute: false, snippets: Vec::new(), limit: None, + add_semicolon_to_unit: true, }; let position = FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; @@ -257,6 +259,7 @@ fn integrated_completion_benchmark() { prefer_absolute: false, snippets: Vec::new(), limit: None, + add_semicolon_to_unit: true, }; let position = FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; diff --git a/src/tools/rust-analyzer/docs/user/generated_config.adoc b/src/tools/rust-analyzer/docs/user/generated_config.adoc index 4fcf580e75f5..f37fd7f4ab3c 100644 --- a/src/tools/rust-analyzer/docs/user/generated_config.adoc +++ b/src/tools/rust-analyzer/docs/user/generated_config.adoc @@ -261,6 +261,13 @@ Aliased as `"checkOnSave.targets"`. Whether `--workspace` should be passed to `cargo check`. If false, `-p ` 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`):: + -- diff --git a/src/tools/rust-analyzer/editors/code/package.json b/src/tools/rust-analyzer/editors/code/package.json index 0b029460b322..b66f0a64d575 100644 --- a/src/tools/rust-analyzer/editors/code/package.json +++ b/src/tools/rust-analyzer/editors/code/package.json @@ -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", "properties": {