Skip to content

Commit

Permalink
Allow def(*args, x=1, y)
Browse files Browse the repository at this point in the history
Summary: Previously we banned this by accident, but it's legal in Python and the Java Starlark. Added a bunch of related tests which were useful for probing where the boundaries of this bug were, and all seem useful to keep.

Reviewed By: stepancheg

Differential Revision: D57151706

fbshipit-source-id: 2cdf61d3550c21c8f3c40a677385525d4eb174fd
  • Loading branch information
ndmitchell authored and facebook-github-bot committed May 9, 2024
1 parent a97048f commit 71bb77a
Showing 1 changed file with 20 additions and 1 deletion.
21 changes: 20 additions & 1 deletion starlark_syntax/src/syntax/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ impl<'a, P: AstPayload> DefParams<'a, P> {
let span = param.span;
match &param.node {
ParameterP::Normal(n, ty) => {
if seen_kwargs || seen_optional {
if seen_kwargs || (seen_optional && !seen_args) {
return Err(WithDiagnostic::new_spanned(
DefError::PositionalThenNonPositional,
param.span,
Expand Down Expand Up @@ -259,6 +259,10 @@ mod tests {
spanned(ParameterP::KwArgs(ident(i), None))
}

fn noargs() -> AstParameterP<AstNoPayload> {
spanned(ParameterP::NoArgs)
}

fn default(i: usize) -> AstParameterP<AstNoPayload> {
let default = spanned(ExprP::Tuple(Vec::new()));
spanned(ParameterP::WithDefaultValue(
Expand Down Expand Up @@ -288,4 +292,19 @@ mod tests {

passes(&[param(0), param(1), default(2), args(3), kwargs(4)]);
}

#[test]
fn test_params_noargs() {
fails(&[noargs(), noargs()], DefError::ArgsParameterAfterStars);
fails(
&[param(0), default(1), param(2)],
DefError::PositionalThenNonPositional,
);
passes(&[args(0), param(1)]);
passes(&[args(0), default(1)]);
passes(&[args(0), param(1), default(2)]);
passes(&[default(0), args(1), param(2)]);
passes(&[args(0), param(1), default(2), param(3)]);
passes(&[noargs(), param(0), default(1), param(2)]);
}
}

0 comments on commit 71bb77a

Please sign in to comment.