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]>

Author: bors[bot]

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 `{`");

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");
     }

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);
 }

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)

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")
     }

crates/ra_syntax/test_data/parser/inline/ok/0004_value_parameters_no_patterns.rs

@@ -1 +1 @@
-type F = Box<Fn(a: i32, &b: &i32, &mut c: &i32, ())>;
+type F = Box<Fn(i32, &i32, &i32, ())>;

crates/ra_syntax/test_data/parser/inline/ok/0004_value_parameters_no_patterns.txt

@@ -1,81 +1,60 @@
-SOURCE_FILE@[0; 54)
-  TYPE_ALIAS_DEF@[0; 53)
+SOURCE_FILE@[0; 39)
+  TYPE_ALIAS_DEF@[0; 38)
     TYPE_KW@[0; 4) "type"
     WHITESPACE@[4; 5) " "
     NAME@[5; 6)
       IDENT@[5; 6) "F"
     WHITESPACE@[6; 7) " "
     EQ@[7; 8) "="
     WHITESPACE@[8; 9) " "
-    PATH_TYPE@[9; 52)
-      PATH@[9; 52)
-        PATH_SEGMENT@[9; 52)
+    PATH_TYPE@[9; 37)
+      PATH@[9; 37)
+        PATH_SEGMENT@[9; 37)
           NAME_REF@[9; 12)
             IDENT@[9; 12) "Box"
-          TYPE_ARG_LIST@[12; 52)
+          TYPE_ARG_LIST@[12; 37)
             L_ANGLE@[12; 13) "<"
-            TYPE_ARG@[13; 51)
-              PATH_TYPE@[13; 51)
-                PATH@[13; 51)
-                  PATH_SEGMENT@[13; 51)
+            TYPE_ARG@[13; 36)
+              PATH_TYPE@[13; 36)
+                PATH@[13; 36)
+                  PATH_SEGMENT@[13; 36)
                     NAME_REF@[13; 15)
                       IDENT@[13; 15) "Fn"
-                    PARAM_LIST@[15; 51)
+                    PARAM_LIST@[15; 36)
                       L_PAREN@[15; 16) "("
-                      PARAM@[16; 22)
-                        BIND_PAT@[16; 17)
-                          NAME@[16; 17)
-                            IDENT@[16; 17) "a"
-                        COLON@[17; 18) ":"
-                        WHITESPACE@[18; 19) " "
-                        PATH_TYPE@[19; 22)
-                          PATH@[19; 22)
-                            PATH_SEGMENT@[19; 22)
-                              NAME_REF@[19; 22)
-                                IDENT@[19; 22) "i32"
-                      COMMA@[22; 23) ","
-                      WHITESPACE@[23; 24) " "
-                      PARAM@[24; 32)
-                        REF_PAT@[24; 26)
-                          AMP@[24; 25) "&"
-                          BIND_PAT@[25; 26)
-                            NAME@[25; 26)
-                              IDENT@[25; 26) "b"
-                        COLON@[26; 27) ":"
-                        WHITESPACE@[27; 28) " "
-                        REFERENCE_TYPE@[28; 32)
-                          AMP@[28; 29) "&"
-                          PATH_TYPE@[29; 32)
-                            PATH@[29; 32)
-                              PATH_SEGMENT@[29; 32)
-                                NAME_REF@[29; 32)
-                                  IDENT@[29; 32) "i32"
-                      COMMA@[32; 33) ","
-                      WHITESPACE@[33; 34) " "
-                      PARAM@[34; 46)
-                        REF_PAT@[34; 40)
-                          AMP@[34; 35) "&"
-                          MUT_KW@[35; 38) "mut"
-                          WHITESPACE@[38; 39) " "
-                          BIND_PAT@[39; 40)
-                            NAME@[39; 40)
-                              IDENT@[39; 40) "c"
-                        COLON@[40; 41) ":"
-                        WHITESPACE@[41; 42) " "
-                        REFERENCE_TYPE@[42; 46)
-                          AMP@[42; 43) "&"
-                          PATH_TYPE@[43; 46)
-                            PATH@[43; 46)
-                              PATH_SEGMENT@[43; 46)
-                                NAME_REF@[43; 46)
-                                  IDENT@[43; 46) "i32"
-                      COMMA@[46; 47) ","
-                      WHITESPACE@[47; 48) " "
-                      PARAM@[48; 50)
-                        TUPLE_TYPE@[48; 50)
-                          L_PAREN@[48; 49) "("
-                          R_PAREN@[49; 50) ")"
-                      R_PAREN@[50; 51) ")"
-            R_ANGLE@[51; 52) ">"
-    SEMI@[52; 53) ";"
-  WHITESPACE@[53; 54) "\n"
+                      PARAM@[16; 19)
+                        PATH_TYPE@[16; 19)
+                          PATH@[16; 19)
+                            PATH_SEGMENT@[16; 19)
+                              NAME_REF@[16; 19)
+                                IDENT@[16; 19) "i32"
+                      COMMA@[19; 20) ","
+                      WHITESPACE@[20; 21) " "
+                      PARAM@[21; 25)
+                        REFERENCE_TYPE@[21; 25)
+                          AMP@[21; 22) "&"
+                          PATH_TYPE@[22; 25)
+                            PATH@[22; 25)
+                              PATH_SEGMENT@[22; 25)
+                                NAME_REF@[22; 25)
+                                  IDENT@[22; 25) "i32"
+                      COMMA@[25; 26) ","
+                      WHITESPACE@[26; 27) " "
+                      PARAM@[27; 31)
+                        REFERENCE_TYPE@[27; 31)
+                          AMP@[27; 28) "&"
+                          PATH_TYPE@[28; 31)
+                            PATH@[28; 31)
+                              PATH_SEGMENT@[28; 31)
+                                NAME_REF@[28; 31)
+                                  IDENT@[28; 31) "i32"
+                      COMMA@[31; 32) ","
+                      WHITESPACE@[32; 33) " "
+                      PARAM@[33; 35)
+                        TUPLE_TYPE@[33; 35)
+                          L_PAREN@[33; 34) "("
+                          R_PAREN@[34; 35) ")"
+                      R_PAREN@[35; 36) ")"
+            R_ANGLE@[36; 37) ">"
+    SEMI@[37; 38) ";"
+  WHITESPACE@[38; 39) "\n"

crates/ra_syntax/test_data/parser/inline/ok/0104_path_fn_trait_args.rs

@@ -1 +1 @@
-type F = Box<Fn(x: i32) -> ()>;
+type F = Box<Fn(i32) -> ()>;