Merge pull request #556 from urfave/use-assertions-over-reflection

Use type assertions rather than reflection to determine how to call the `Action`

app.go

@@ -6,9 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"reflect"
 	"sort"
-	"strings"
 	"time"
 )
 
@@ -19,11 +17,8 @@ var (
 
 	contactSysadmin = "This is an error in the application.  Please contact the distributor of this application if this is not you."
 
-	errNonFuncAction = NewExitError("ERROR invalid Action type.  "+
+	errInvalidActionType = NewExitError("ERROR invalid Action type. "+
-		fmt.Sprintf("Must be a func of type `cli.ActionFunc`.  %s", contactSysadmin)+
+		fmt.Sprintf("Must be `func(*Context`)` or `func(*Context) error).  %s", contactSysadmin)+
-		fmt.Sprintf("See %s", appActionDeprecationURL), 2)
-	errInvalidActionSignature = NewExitError("ERROR invalid Action signature.  "+
-		fmt.Sprintf("Must be `cli.ActionFunc`.  %s", contactSysadmin)+
 		fmt.Sprintf("See %s", appActionDeprecationURL), 2)
 )
 
@@ -468,41 +463,16 @@ func (a Author) String() string {
 	return fmt.Sprintf("%v%v", a.Name, e)
 }
 
-// HandleAction uses ✧✧✧reflection✧✧✧ to figure out if the given Action is an
+// HandleAction attempts to figure out which Action signature was used.  If
-// ActionFunc, a func with the legacy signature for Action, or some other
+// it's an ActionFunc or a func with the legacy signature for Action, the func
-// invalid thing.  If it's an ActionFunc or a func with the legacy signature for
+// is run!
-// Action, the func is run!
 func HandleAction(action interface{}, context *Context) (err error) {
-	defer func() {
+	if a, ok := action.(func(*Context) error); ok {
-		if r := recover(); r != nil {
+		return a(context)
-			// Try to detect a known reflection error from *this scope*, rather than
+	} else if a, ok := action.(func(*Context)); ok { // deprecated function signature
-			// swallowing all panics that may happen when calling an Action func.
+		a(context)
-			s := fmt.Sprintf("%v", r)
-			if strings.HasPrefix(s, "reflect: ") && strings.Contains(s, "too many input arguments") {
-				err = NewExitError(fmt.Sprintf("ERROR unknown Action error: %v.", r), 2)
-			} else {
-				panic(r)
-			}
-		}
-	}()
-
-	if reflect.TypeOf(action).Kind() != reflect.Func {
-		return errNonFuncAction
-	}
-
-	vals := reflect.ValueOf(action).Call([]reflect.Value{reflect.ValueOf(context)})
-
-	if len(vals) == 0 {
 		return nil
+	} else {
+		return errInvalidActionType
 	}
-
-	if len(vals) > 1 {
-		return errInvalidActionSignature
-	}
-
-	if retErr, ok := vals[0].Interface().(error); vals[0].IsValid() && ok {
-		return retErr
-	}
-
-	return err
 }

app_test.go

@@ -1492,7 +1492,7 @@ func TestHandleAction_WithNonFuncAction(t *testing.T) {
 		t.Fatalf("expected to receive a *ExitError")
 	}
 
-	if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") {
+	if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type.") {
 		t.Fatalf("expected an unknown Action error, but got: %v", exitErr.Error())
 	}
 
@@ -1516,7 +1516,7 @@ func TestHandleAction_WithInvalidFuncSignature(t *testing.T) {
 		t.Fatalf("expected to receive a *ExitError")
 	}
 
-	if !strings.HasPrefix(exitErr.Error(), "ERROR unknown Action error") {
+	if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") {
 		t.Fatalf("expected an unknown Action error, but got: %v", exitErr.Error())
 	}
 
@@ -1540,7 +1540,7 @@ func TestHandleAction_WithInvalidFuncReturnSignature(t *testing.T) {
 		t.Fatalf("expected to receive a *ExitError")
 	}
 
-	if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action signature") {
+	if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") {
 		t.Fatalf("expected an invalid Action signature error, but got: %v", exitErr.Error())
 	}