Change StarArgPassStyle::Default -> PosOrNamed

Summary: Determine wheter a parameter is named-only or pos-or-named earlier during parameter parsing.

Reviewed By: JakobDegen

Differential Revision: D63600508

fbshipit-source-id: 7de17ad602eccfc0f8340b6b1ee9aed6433906ee

starlark_derive/src/module/param_spec.rs

@@ -90,25 +90,15 @@ impl<'a> ParamSpec<'a> {
                     last_param_style = CurrentParamStyle::PosOnly;
                     pos_only.push(arg);
                 }
-                StarArgPassStyle::Default => {
-                    last_param_style = match last_param_style {
-                        CurrentParamStyle::PosOnly | CurrentParamStyle::PosOrNamed => {
-                            pos_or_named.push(arg);
-                            CurrentParamStyle::PosOrNamed
-                        }
-                        CurrentParamStyle::NamedOnly => {
-                            // After named parameters, following parameters cannot be positional,
-                            // so defaut means named-only.
-                            named_only.push(arg);
-                            CurrentParamStyle::NamedOnly
-                        }
-                        CurrentParamStyle::NoMore => {
-                            return Err(syn::Error::new(
-                                arg.span,
-                                "Regular parameter after **kwargs",
-                            ));
-                        }
+                StarArgPassStyle::PosOrNamed => {
+                    if last_param_style > CurrentParamStyle::PosOrNamed {
+                        return Err(syn::Error::new(
+                            arg.span,
+                            "Positional-or-named parameter after named-only",
+                        ));
                     }
+                    last_param_style = CurrentParamStyle::PosOrNamed;
+                    pos_or_named.push(arg);
                 }
                 StarArgPassStyle::NamedOnly => {
                     if last_param_style > CurrentParamStyle::NamedOnly {

starlark_derive/src/module/parse/fun.rs

@@ -309,11 +309,12 @@ pub(crate) fn parse_fun(func: ItemFn, module_kind: ModuleKind) -> syn::Result<St
     let mut eval = None;
     let mut heap = None;
 
-    let mut seen_star_args = false;
+    // Seen an equivalent of `*` or `*args`. Meaning default parameters are named-only after this.
+    let mut seen_star = false;
     let mut args: Option<RegularParams> = None;
     for (i, arg) in func.sig.inputs.into_iter().enumerate() {
         let span = arg.span();
-        let parsed_arg = parse_arg(arg, has_v, seen_star_args, module_kind, i)?;
+        let parsed_arg = parse_arg(arg, has_v, seen_star, module_kind, i)?;
         match parsed_arg {
             StarArgOrSpecial::Heap(special) => {
                 if heap.is_some() {
@@ -328,8 +329,10 @@ pub(crate) fn parse_fun(func: ItemFn, module_kind: ModuleKind) -> syn::Result<St
                 eval = Some(special);
             }
             StarArgOrSpecial::StarArg(arg) => {
-                if arg.pass_style == StarArgPassStyle::Args {
-                    seen_star_args = true;
+                if arg.pass_style == StarArgPassStyle::Args
+                    || arg.pass_style == StarArgPassStyle::NamedOnly
+                {
+                    seen_star = true;
                 }
                 let args = args.get_or_insert_with(|| RegularParams::Unpack(Vec::new()));
                 match args {
@@ -681,7 +684,7 @@ fn is_arguments(param: &SimpleParam, attrs: &FnParamAttrs) -> syn::Result<Option
 fn parse_arg(
     x: FnArg,
     has_v: bool,
-    seen_star_args: bool,
+    seen_star: bool,
     module_kind: ModuleKind,
     param_index: usize,
 ) -> syn::Result<StarArgOrSpecial> {
@@ -733,7 +736,7 @@ fn parse_arg(
     let pass_style = match (
         param_attrs.args,
         param_attrs.kwargs,
-        seen_star_args,
+        seen_star,
         param_attrs.pos_only,
         param_attrs.named_only,
     ) {
@@ -745,22 +748,22 @@ fn parse_arg(
                 "Positional-only arguments cannot follow *args",
             ));
         }
+        (_, _, _, true, true) => {
+            return Err(syn::Error::new(
+                span,
+                "Function parameter cannot be both positional-only and named-only",
+            ));
+        }
         (false, false, true, false, _) => StarArgPassStyle::NamedOnly,
         // TODO(nga): currently, without `#[starlark(require = named)]`
         //   and without `#[starlark(require = pos)]`, parameter is positional-or-named.
         //   We want to change that: either make it positional by default,
         //   or require explicit `#[starlark(pos, named)]`.
         //   Discussion there:
         //   https://fb.workplace.com/groups/1267349253953900/posts/1299495914072567
-        (false, false, false, false, false) => StarArgPassStyle::Default,
+        (false, false, false, false, false) => StarArgPassStyle::PosOrNamed,
         (false, false, false, true, false) => StarArgPassStyle::PosOnly,
         (false, false, false, false, true) => StarArgPassStyle::NamedOnly,
-        (false, false, false, true, true) => {
-            return Err(syn::Error::new(
-                span,
-                "Function parameter cannot be both positional-only and named-only",
-            ));
-        }
     };
     Ok(StarArgOrSpecial::StarArg(StarArg {
         span,

starlark_derive/src/module/typ.rs

@@ -116,9 +116,8 @@ pub(crate) struct StarAttr {
 pub(crate) enum StarArgPassStyle {
     /// Parameter can be filled only positionally.
     PosOnly,
-    /// Parameter can filled both positionally and by name by default,
-    /// or named only if followed by `*args` or named only.
-    Default,
+    /// Parameter can be filled positionally or by name.
+    PosOrNamed,
     /// Parameter can be filled by name.
     NamedOnly,
     /// `*args`.