From 1b9d0e7119bc2aca0cd6be14ea0b8b184dc12bd2 Mon Sep 17 00:00:00 2001
From: Alan Donovan <adonovan@google.com>
Date: Tue, 28 Nov 2017 14:19:10 -0500
Subject: [PATCH] resolve: add missing check for order of parameters and
 arguments

Parameters: required before optional
Arguments: positional before named
---
 resolve/resolve.go           | 12 ++++++++----
 resolve/testdata/resolve.sky |  7 +++++++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/resolve/resolve.go b/resolve/resolve.go
index 4649914..14fdcdd 100644
--- a/resolve/resolve.go
+++ b/resolve/resolve.go
@@ -584,8 +584,7 @@ func (r *resolver) expr(e syntax.Expr) {
 
 	case *syntax.CallExpr:
 		r.expr(e.Fn)
-		seenVarargs := false
-		seenKwargs := false
+		var seenVarargs, seenKwargs, seenNamed bool
 		for _, arg := range e.Args {
 			pos, _ := arg.Span()
 			if unop, ok := arg.(*syntax.UnaryExpr); ok && unop.Op == syntax.STARSTAR {
@@ -611,12 +610,15 @@ func (r *resolver) expr(e syntax.Expr) {
 				}
 				// ignore binop.X
 				r.expr(binop.Y)
+				seenNamed = true
 			} else {
 				// positional argument
 				if seenVarargs {
 					r.errorf(pos, "argument may not follow *args")
 				} else if seenKwargs {
 					r.errorf(pos, "argument may not follow **kwargs")
+				} else if seenNamed {
+					r.errorf(pos, "positional argument may not follow named")
 				}
 				r.expr(arg)
 			}
@@ -646,8 +648,7 @@ func (r *resolver) function(pos syntax.Position, name string, function *syntax.F
 	r.push(b)
 
 	const allowRebind = false
-	seenVarargs := false
-	seenKwargs := false
+	var seenVarargs, seenKwargs, seenOptional bool
 	for _, param := range function.Params {
 		switch param := param.(type) {
 		case *syntax.Ident:
@@ -656,6 +657,8 @@ func (r *resolver) function(pos syntax.Position, name string, function *syntax.F
 				r.errorf(pos, "parameter may not follow **kwargs")
 			} else if seenVarargs {
 				r.errorf(pos, "parameter may not follow *args")
+			} else if seenOptional {
+				r.errorf(pos, "required parameter may not follow optional")
 			}
 			if r.bind(param, allowRebind) {
 				r.errorf(pos, "duplicate parameter: %s", param.Name)
@@ -671,6 +674,7 @@ func (r *resolver) function(pos syntax.Position, name string, function *syntax.F
 			if id := param.X.(*syntax.Ident); r.bind(id, allowRebind) {
 				r.errorf(pos, "duplicate parameter: %s", id.Name)
 			}
+			seenOptional = true
 
 		case *syntax.UnaryExpr:
 			// *args or **kwargs
diff --git a/resolve/testdata/resolve.sky b/resolve/testdata/resolve.sky
index f8c4c9a..6feaf67 100644
--- a/resolve/testdata/resolve.sky
+++ b/resolve/testdata/resolve.sky
@@ -165,6 +165,13 @@ continue ### "continue not in a loop"
 
 pass
 
+---
+# Positional arguments (and required parameters)
+# must appear before named arguments (and optional parameters).
+
+G(x=1, 2) ### `positional argument may not follow named`
+
+def f(x=1, y): pass ### `required parameter may not follow optional`
 ---
 # No parameters may follow **kwargs