From 07a9e781a6b917fb616931c44ec25689f7e6b293 Mon Sep 17 00:00:00 2001 From: Qi Xiao Date: Mon, 13 Apr 2020 00:47:33 +0100 Subject: [PATCH] pkg/eval: Report detailed ArityMismatch errors. --- pkg/eval/closure.go | 18 +++++++++--------- pkg/eval/closure_test.go | 26 ++++++++++++++++++++++---- pkg/eval/compile_effect.go | 14 ++++++++------ pkg/eval/compile_effect_test.go | 18 +++++++++++++++--- pkg/eval/go_fn.go | 15 ++++++++++----- pkg/eval/go_fn_test.go | 22 +++++++++++++--------- 6 files changed, 77 insertions(+), 36 deletions(-) diff --git a/pkg/eval/closure.go b/pkg/eval/closure.go index dc5d460b..6efe14e5 100644 --- a/pkg/eval/closure.go +++ b/pkg/eval/closure.go @@ -1,10 +1,10 @@ package eval import ( - "errors" "fmt" "unsafe" + "github.com/elves/elvish/pkg/eval/errs" "github.com/elves/elvish/pkg/eval/vals" "github.com/elves/elvish/pkg/eval/vars" "github.com/elves/elvish/pkg/parse" @@ -12,10 +12,6 @@ import ( "github.com/xiaq/persistent/hash" ) -// ErrArityMismatch is thrown by a closure when the number of arguments the user -// supplies does not match with what is required. -var ErrArityMismatch = errors.New("arity mismatch") - // Closure is a closure defined in Elvish script. Each closure has its unique // identity. type Closure struct { @@ -93,12 +89,16 @@ func (c *Closure) IterateKeys(f func(interface{}) bool) { // Call calls a closure. func (c *Closure) Call(fm *Frame, args []interface{}, opts map[string]interface{}) error { if c.RestArg != "" { - if len(c.ArgNames) > len(args) { - return fmt.Errorf("need %d or more arguments, got %d", len(c.ArgNames), len(args)) + if len(args) < len(c.ArgNames) { + return errs.ArityMismatch{ + What: "arguments here", + ValidLow: len(c.ArgNames), ValidHigh: -1, Actual: len(args)} } } else { - if len(c.ArgNames) != len(args) { - return fmt.Errorf("need %d arguments, got %d", len(c.ArgNames), len(args)) + if len(args) != len(c.ArgNames) { + return errs.ArityMismatch{ + What: "arguments here", + ValidLow: len(c.ArgNames), ValidHigh: len(c.ArgNames), Actual: len(args)} } } diff --git a/pkg/eval/closure_test.go b/pkg/eval/closure_test.go index ca291136..7b420591 100644 --- a/pkg/eval/closure_test.go +++ b/pkg/eval/closure_test.go @@ -1,15 +1,33 @@ package eval -import "testing" +import ( + "testing" + + "github.com/elves/elvish/pkg/eval/errs" +) func TestClosure(t *testing.T) { Test(t, That("kind-of { }").Puts("fn"), That("eq { } { }").Puts(false), That("x = { }; put [&$x= foo][$x]").Puts("foo"), - That("[x]{ } a b").ThrowsAny(), - That("[x y]{ } a").ThrowsAny(), - That("[x y @rest]{ } a").ThrowsAny(), + + That("f = [x]{ }", "$f a b").Throws( + errs.ArityMismatch{ + What: "arguments here", + ValidLow: 1, ValidHigh: 1, Actual: 2}, + "$f a b"), + That("f = [x y]{ }", "$f a").Throws( + errs.ArityMismatch{ + What: "arguments here", + ValidLow: 2, ValidHigh: 2, Actual: 1}, + "$f a"), + That("f = [x y @rest]{ }", "$f a").Throws( + errs.ArityMismatch{ + What: "arguments here", + ValidLow: 2, ValidHigh: -1, Actual: 1}, + "$f a"), + That("[]{ } &k=v").ThrowsAny(), That("explode [a b]{ }[arg-names]").Puts("a", "b"), diff --git a/pkg/eval/compile_effect.go b/pkg/eval/compile_effect.go index b7028ac5..bc5eb6cf 100644 --- a/pkg/eval/compile_effect.go +++ b/pkg/eval/compile_effect.go @@ -8,6 +8,7 @@ import ( "sync" "github.com/elves/elvish/pkg/diag" + "github.com/elves/elvish/pkg/eval/errs" "github.com/elves/elvish/pkg/eval/vals" "github.com/elves/elvish/pkg/eval/vars" "github.com/elves/elvish/pkg/parse" @@ -372,9 +373,6 @@ func (op *formOp) invoke(fm *Frame) (errRet error) { } if headFn != nil { - if _, isClosure := headFn.(*Closure); isClosure { - fm.traceback = fm.addTraceback(op) - } return headFn.Call(fm, args, convertedOpts) } return op.spaceyAssignOp.exec(fm) @@ -432,12 +430,16 @@ func (op *assignmentOp) invoke(fm *Frame) (errRet error) { return ErrMoreThanOneRest } if len(rest) == 1 { - if len(variables) > len(values) { - return ErrArityMismatch + if len(values) < len(variables) { + return errs.ArityMismatch{ + What: "assignment right-hand-side", + ValidLow: len(variables), ValidHigh: -1, Actual: len(values)} } } else { if len(variables) != len(values) { - return ErrArityMismatch + return errs.ArityMismatch{ + What: "assignment right-hand-side", + ValidLow: len(variables), ValidHigh: len(variables), Actual: len(values)} } } diff --git a/pkg/eval/compile_effect_test.go b/pkg/eval/compile_effect_test.go index 53ef0623..8d10014c 100644 --- a/pkg/eval/compile_effect_test.go +++ b/pkg/eval/compile_effect_test.go @@ -97,9 +97,21 @@ func TestCompileEffect(t *testing.T) { // Assignment errors when the RHS errors. That("x = [][1]").Throws(errWithType{errs.OutOfRange{}}, "[][1]"), // Arity mismatch. - That("x = 1 2").ThrowsCause(ErrArityMismatch), - That("x y = 1").ThrowsCause(ErrArityMismatch), - That("x y @z = 1").ThrowsCause(ErrArityMismatch), + That("x = 1 2").Throws( + errs.ArityMismatch{ + What: "assignment right-hand-side", + ValidLow: 1, ValidHigh: 1, Actual: 2}, + "x = 1 2"), + That("x y = 1").Throws( + errs.ArityMismatch{ + What: "assignment right-hand-side", + ValidLow: 2, ValidHigh: 2, Actual: 1}, + "x y = 1"), + That("x y @z = 1").Throws( + errs.ArityMismatch{ + What: "assignment right-hand-side", + ValidLow: 2, ValidHigh: -1, Actual: 1}, + "x y @z = 1"), // Redirections // ------------ diff --git a/pkg/eval/go_fn.go b/pkg/eval/go_fn.go index 3c90d527..242b3566 100644 --- a/pkg/eval/go_fn.go +++ b/pkg/eval/go_fn.go @@ -6,6 +6,7 @@ import ( "reflect" "unsafe" + "github.com/elves/elvish/pkg/eval/errs" "github.com/elves/elvish/pkg/eval/vals" "github.com/xiaq/persistent/hash" ) @@ -155,16 +156,20 @@ var errNoOptions = errors.New("function does not accept any options") func (b *GoFn) Call(f *Frame, args []interface{}, opts map[string]interface{}) error { if b.variadicArg != nil { if len(args) < len(b.normalArgs) { - return fmt.Errorf("want %d or more arguments, got %d", - len(b.normalArgs), len(args)) + return errs.ArityMismatch{ + What: "arguments here", + ValidLow: len(b.normalArgs), ValidHigh: -1, Actual: len(args)} } } else if b.inputs { if len(args) != len(b.normalArgs) && len(args) != len(b.normalArgs)+1 { - return fmt.Errorf("want %d or %d arguments, got %d", - len(b.normalArgs), len(b.normalArgs)+1, len(args)) + return errs.ArityMismatch{ + What: "arguments here", + ValidLow: len(b.normalArgs), ValidHigh: len(b.normalArgs) + 1, Actual: len(args)} } } else if len(args) != len(b.normalArgs) { - return fmt.Errorf("want %d arguments, got %d", len(b.normalArgs), len(args)) + return errs.ArityMismatch{ + What: "arguments here", + ValidLow: len(b.normalArgs), ValidHigh: len(b.normalArgs), Actual: len(args)} } if !b.rawOptions && b.options == nil && len(opts) > 0 { return ErrNoOptAccepted diff --git a/pkg/eval/go_fn_test.go b/pkg/eval/go_fn_test.go index da32a044..1761f3ca 100644 --- a/pkg/eval/go_fn_test.go +++ b/pkg/eval/go_fn_test.go @@ -4,6 +4,7 @@ import ( "errors" "testing" + "github.com/elves/elvish/pkg/eval/errs" "github.com/elves/elvish/pkg/eval/vals" ) @@ -25,9 +26,9 @@ func TestGoFnCall(t *testing.T) { t.Errorf("Failed to call f: %v", err) } } - callBad := func(fm *Frame, args []interface{}, opts map[string]interface{}) { + callBad := func(fm *Frame, args []interface{}, opts map[string]interface{}, wantErr error) { err := f.Call(fm, args, opts) - if err == nil { + if !matchErr(wantErr, err) { t.Errorf("Calling f didn't return error") } } @@ -175,39 +176,42 @@ func TestGoFnCall(t *testing.T) { f = NewGoFn("f", func() { t.Errorf("Function called when there are too many arguments") }) - callBad(theFrame, []interface{}{"x"}, theOptions) + callBad(theFrame, []interface{}{"x"}, theOptions, errs.ArityMismatch{ + What: "arguments here", ValidLow: 0, ValidHigh: 0, Actual: 1}) // Too few arguments. f = NewGoFn("f", func(x string) { t.Errorf("Function called when there are too few arguments") }) - callBad(theFrame, nil, theOptions) + callBad(theFrame, nil, theOptions, errs.ArityMismatch{ + What: "arguments here", ValidLow: 1, ValidHigh: 1, Actual: 0}) f = NewGoFn("f", func(x string, y ...string) { t.Errorf("Function called when there are too few arguments") }) - callBad(theFrame, nil, theOptions) + callBad(theFrame, nil, theOptions, errs.ArityMismatch{ + What: "arguments here", ValidLow: 1, ValidHigh: -1, Actual: 0}) // Options when the function does not accept options. f = NewGoFn("f", func() { t.Errorf("Function called when there are extra options") }) - callBad(theFrame, nil, RawOptions{"foo": "bar"}) + callBad(theFrame, nil, RawOptions{"foo": "bar"}, errNoOptions) // Wrong argument type. f = NewGoFn("f", func(x string) { t.Errorf("Function called when arguments have wrong type") }) - callBad(theFrame, []interface{}{1}, theOptions) + callBad(theFrame, []interface{}{1}, theOptions, anyError{}) // Wrong argument type: cannot convert to int. f = NewGoFn("f", func(x int) { t.Errorf("Function called when arguments have wrong type") }) - callBad(theFrame, []interface{}{"x"}, theOptions) + callBad(theFrame, []interface{}{"x"}, theOptions, anyError{}) // Wrong argument type: cannot convert to float64. f = NewGoFn("f", func(x float64) { t.Errorf("Function called when arguments have wrong type") }) - callBad(theFrame, []interface{}{"x"}, theOptions) + callBad(theFrame, []interface{}{"x"}, theOptions, anyError{}) }