Fix the interpretation of vararg separator

[kngwyu]

Resolves #790
* does not mean accept_args and we should reject fn(10) for def fn(*, var=10).

[oconnor663]

Thanks for the quick fix!

[pganssle]

Should you also add a test for args(a, "*", c=10)? (or whatever the correct syntax is for a required argument, I'm not sure).

[kngwyu]

Thanks.
Now this allows args(a, "*", c=10), args(a, b, "*", c=10), and args("*", c=10), which seems confusing.
So it may be good to restrict the syntax 🤔

[pganssle]

I don't think it's a good idea to restrict things, and in fact we should also add support for positional-only arguments (in a separate change, IMO). The full spec for a python function definition can be found here: https://docs.python.org/3/tutorial/controlflow.html#special-parameters

def f(pos1, pos2, /, pos_or_kwd, *, kwd1, kwd2):
      -----------    ----------     ----------
        |             |                  |
        |        Positional or keyword   |
        |                                - Keyword only
         -- Positional only

That doesn't include *args and **kwargs.

[kngwyu]

Yeah, but our proc-macro can do somewhat more flexible things like

    #[args(a, "*", d=10)]
    fn get_pos_arg_kw_sep(&self, a: i32, b: i32, c: i32) -> PyResult<i32>

, which are really confusing.
And it looks like now we allow #[args("*", d)], which looks like a bug.

[davidhewitt]

If we're working on this, should we also change to accept * instead of "*" ? e.g. #[args(a, *, d=10)]

[davidhewitt]

(Maybe we should note these down each as separate bug issues.)

[kngwyu]

syn doesn't accept it.

[davidhewitt]

I could be wrong, can't it parse it as syn::Token![*] ?

[kngwyu]

https://docs.rs/syn/1.0.16/syn/enum.NestedMeta.html

[kngwyu]

I pushed a change that raises a compile error for #[pyfunction(a, "*", b)].
For the problem of ambiguity, I opened another issue #794

[kngwyu]

I separated this function for refactoring.
The logic is not changed except *, **, name=lit is inhibited(the old code does

        if self.has_varargs {
           // No check here
           ...
        } else {
            self.kw_arg_is_ok(item)?;
            ...
        }

).

[kngwyu]

I removed this function since just quote! { #bool_value } works.

[davidhewitt]

Will give this a full review tomorrow 👍

[davidhewitt]

LGTM. One nitpick about the changelog. Feel free to re-word what I suggest as you like.

[davidhewitt]

Nice :)

[kngwyu]

@davidhewitt
Thanks!