Merge #3040

3040: Rework value parameter parsing r=matklad a=tobz1000

Fixes #2847.

- `Fn__(...)` parameters with idents/patterns no longer parse
- Trait function parameters with arbitrary patterns parse
- Trait function parameters without idents/patterns no longer parse
- `fn(...)` parameters no longer parse with patterns other than a single ident

__Question__: The pre-existing test `param_list_opt_patterns` has been kept as-is, although the name no longer makes sense (it's testing `Fn__(...)` params, which aren't allowed patterns any more). What would be best to do about this?

Co-authored-by: Toby Dimmick <[email protected]>

5 files changed, 78 insertions, 59 deletions

diff --git a/crates/ra_parser/src/grammar/expressions/atom.rs b/crates/ra_parser/src/grammar/expressions/atom.rs
index 2cc321473..f154077a8 100644
--- a/crates/ra_parser/src/grammar/expressions/atom.rs
+++ b/crates/ra_parser/src/grammar/expressions/atom.rs
@@ -229,7 +229,7 @@ fn lambda_expr(p: &mut Parser) -> CompletedMarker {
     let m = p.start();
     p.eat(T![async]);
     p.eat(T![move]);
-    params::param_list_opt_types(p);
+    params::param_list_closure(p);
     if opt_fn_ret_type(p) {
         if !p.at(T!['{']) {
             p.error("expected `{`");
diff --git a/crates/ra_parser/src/grammar/items.rs b/crates/ra_parser/src/grammar/items.rs
index 6e23d9b72..7263c4d69 100644
--- a/crates/ra_parser/src/grammar/items.rs
+++ b/crates/ra_parser/src/grammar/items.rs
@@ -164,7 +164,7 @@ pub(super) fn maybe_item(p: &mut Parser, m: Marker, flavor: ItemFlavor) -> Resul
         // async unsafe fn foo() {}
         // unsafe const fn bar() {}
         T![fn] => {
-            fn_def(p, flavor);
+            fn_def(p);
             m.complete(p, FN_DEF);
         }
 
@@ -301,7 +301,7 @@ pub(crate) fn extern_item_list(p: &mut Parser) {
     m.complete(p, EXTERN_ITEM_LIST);
 }
 
-fn fn_def(p: &mut Parser, flavor: ItemFlavor) {
+fn fn_def(p: &mut Parser) {
     assert!(p.at(T![fn]));
     p.bump(T![fn]);
 
@@ -311,10 +311,7 @@ fn fn_def(p: &mut Parser, flavor: ItemFlavor) {
     type_params::opt_type_param_list(p);
 
     if p.at(T!['(']) {
-        match flavor {
-            ItemFlavor::Mod => params::param_list(p),
-            ItemFlavor::Trait => params::param_list_opt_patterns(p),
-        }
+        params::param_list_fn_def(p);
     } else {
         p.error("expected function arguments");
     }
diff --git a/crates/ra_parser/src/grammar/params.rs b/crates/ra_parser/src/grammar/params.rs
index c10b53316..94edc7f35 100644
--- a/crates/ra_parser/src/grammar/params.rs
+++ b/crates/ra_parser/src/grammar/params.rs
@@ -7,54 +7,60 @@ use super::*;
 // fn b(x: i32) {}
 // fn c(x: i32, ) {}
 // fn d(x: i32, y: ()) {}
-pub(super) fn param_list(p: &mut Parser) {
-    list_(p, Flavor::Normal)
+pub(super) fn param_list_fn_def(p: &mut Parser) {
+    list_(p, Flavor::FnDef)
 }
 
 // test param_list_opt_patterns
 // fn foo<F: FnMut(&mut Foo<'a>)>(){}
-pub(super) fn param_list_opt_patterns(p: &mut Parser) {
-    list_(p, Flavor::OptionalPattern)
+pub(super) fn param_list_fn_trait(p: &mut Parser) {
+    list_(p, Flavor::FnTrait)
 }
 
-pub(super) fn param_list_opt_types(p: &mut Parser) {
-    list_(p, Flavor::OptionalType)
+pub(super) fn param_list_fn_ptr(p: &mut Parser) {
+    list_(p, Flavor::FnPointer)
 }
 
-#[derive(Clone, Copy, Eq, PartialEq)]
-enum Flavor {
-    OptionalType,
-    OptionalPattern,
-    Normal,
+pub(super) fn param_list_closure(p: &mut Parser) {
+    list_(p, Flavor::Closure)
 }
 
-impl Flavor {
-    fn type_required(self) -> bool {
-        match self {
-            Flavor::OptionalType => false,
-            _ => true,
-        }
-    }
+#[derive(Debug, Clone, Copy)]
+enum Flavor {
+    FnDef,   // Includes trait fn params; omitted param idents are not supported
+    FnTrait, // Params for `Fn(...)`/`FnMut(...)`/`FnOnce(...)` annotations
+    FnPointer,
+    Closure,
 }
 
 fn list_(p: &mut Parser, flavor: Flavor) {
-    let (bra, ket) = if flavor.type_required() { (T!['('], T![')']) } else { (T![|], T![|]) };
-    assert!(p.at(bra));
+    use Flavor::*;
+
+    let (bra, ket) = match flavor {
+        Closure => (T![|], T![|]),
+        FnDef | FnTrait | FnPointer => (T!['('], T![')']),
+    };
+
     let m = p.start();
     p.bump(bra);
-    if flavor.type_required() {
+
+    if let FnDef = flavor {
         // test self_param_outer_attr
         // fn f(#[must_use] self) {}
         attributes::outer_attributes(p);
         opt_self_param(p);
     }
+
     while !p.at(EOF) && !p.at(ket) {
         // test param_outer_arg
         // fn f(#[attr1] pat: Type) {}
         attributes::outer_attributes(p);
 
-        if flavor.type_required() && p.at(T![...]) {
-            break;
+        // test param_list_vararg
+        // extern "C" { fn printf(format: *const i8, ...) -> i32; }
+        match flavor {
+            FnDef | FnPointer if p.eat(T![...]) => break,
+            _ => (),
         }
 
         if !p.at_ts(VALUE_PARAMETER_FIRST) {
@@ -66,11 +72,7 @@ fn list_(p: &mut Parser, flavor: Flavor) {
             p.expect(T![,]);
         }
     }
-    // test param_list_vararg
-    // extern "C" { fn printf(format: *const i8, ...) -> i32; }
-    if flavor.type_required() {
-        p.eat(T![...]);
-    }
+
     p.expect(ket);
     m.complete(p, PARAM_LIST);
 }
@@ -80,36 +82,56 @@ const VALUE_PARAMETER_FIRST: TokenSet = patterns::PATTERN_FIRST.union(types::TYP
 fn value_parameter(p: &mut Parser, flavor: Flavor) {
     let m = p.start();
     match flavor {
-        Flavor::OptionalType | Flavor::Normal => {
+        // test trait_fn_placeholder_parameter
+        // trait Foo {
+        //     fn bar(_: u64, mut x: i32);
+        // }
+
+        // test trait_fn_patterns
+        // trait T {
+        //     fn f1((a, b): (usize, usize)) {}
+        //     fn f2(S { a, b }: S) {}
+        //     fn f3(NewType(a): NewType) {}
+        //     fn f4(&&a: &&usize) {}
+        // }
+
+        // test fn_patterns
+        // impl U {
+        //     fn f1((a, b): (usize, usize)) {}
+        //     fn f2(S { a, b }: S) {}
+        //     fn f3(NewType(a): NewType) {}
+        //     fn f4(&&a: &&usize) {}
+        // }
+        Flavor::FnDef => {
             patterns::pattern(p);
-            if p.at(T![:]) && !p.at(T![::]) || flavor.type_required() {
-                types::ascription(p)
-            }
+            types::ascription(p);
         }
         // test value_parameters_no_patterns
-        // type F = Box<Fn(a: i32, &b: &i32, &mut c: &i32, ())>;
-        Flavor::OptionalPattern => {
-            let la0 = p.current();
-            let la1 = p.nth(1);
-            let la2 = p.nth(2);
-            let la3 = p.nth(3);
-
-            // test trait_fn_placeholder_parameter
-            // trait Foo {
-            //     fn bar(_: u64, mut x: i32);
-            // }
-            if (la0 == IDENT || la0 == T![_]) && la1 == T![:] && !p.nth_at(1, T![::])
-                || la0 == T![mut] && la1 == IDENT && la2 == T![:]
-                || la0 == T![&]
-                    && (la1 == IDENT && la2 == T![:] && !p.nth_at(2, T![::])
-                        || la1 == T![mut] && la2 == IDENT && la3 == T![:] && !p.nth_at(3, T![::]))
-            {
+        // type F = Box<Fn(i32, &i32, &i32, ())>;
+        Flavor::FnTrait => {
+            types::type_(p);
+        }
+        // test fn_pointer_param_ident_path
+        // type Foo = fn(Bar::Baz);
+        // type Qux = fn(baz: Bar::Baz);
+        Flavor::FnPointer => {
+            if p.at(IDENT) && p.nth(1) == T![:] && !p.nth_at(1, T![::]) {
                 patterns::pattern(p);
                 types::ascription(p);
             } else {
                 types::type_(p);
             }
         }
+        // test closure_params
+        // fn main() {
+        //    let foo = |bar, baz: Baz, qux: Qux::Quux| ();
+        // }
+        Flavor::Closure => {
+            patterns::pattern(p);
+            if p.at(T![:]) && !p.at(T![::]) {
+                types::ascription(p);
+            }
+        }
     }
     m.complete(p, PARAM);
 }
diff --git a/crates/ra_parser/src/grammar/paths.rs b/crates/ra_parser/src/grammar/paths.rs
index ca8e075a1..f5bf3d7ce 100644
--- a/crates/ra_parser/src/grammar/paths.rs
+++ b/crates/ra_parser/src/grammar/paths.rs
@@ -97,9 +97,9 @@ fn opt_path_type_args(p: &mut Parser, mode: Mode) {
         Mode::Use => return,
         Mode::Type => {
             // test path_fn_trait_args
-            // type F = Box<Fn(x: i32) -> ()>;
+            // type F = Box<Fn(i32) -> ()>;
             if p.at(T!['(']) {
-                params::param_list_opt_patterns(p);
+                params::param_list_fn_trait(p);
                 opt_fn_ret_type(p);
             } else {
                 type_args::opt_type_arg_list(p, false)
diff --git a/crates/ra_parser/src/grammar/types.rs b/crates/ra_parser/src/grammar/types.rs
index 9b2e440fb..2c00bce80 100644
--- a/crates/ra_parser/src/grammar/types.rs
+++ b/crates/ra_parser/src/grammar/types.rs
@@ -183,7 +183,7 @@ fn fn_pointer_type(p: &mut Parser) {
         return;
     }
     if p.at(T!['(']) {
-        params::param_list_opt_patterns(p);
+        params::param_list_fn_ptr(p);
     } else {
         p.error("expected parameters")
     }