Merge pull request #356 from jbenet/fix/option-cast

Fix Option Validation

commands/cli/parse.go

@@ -46,7 +46,10 @@ func Parse(input []string, stdin *os.File, root *cmds.Command) (cmds.Request, *c
 		}
 	}
 
-	req := cmds.NewRequest(path, opts, args, cmd, optDefs)
+	req, err := cmds.NewRequest(path, opts, args, cmd, optDefs)
+	if err != nil {
+		return nil, cmd, path, err
+	}
 
 	err = cmd.CheckArguments(req)
 	if err != nil {

commands/command_test.go

@@ -17,21 +17,21 @@ func TestOptionValidation(t *testing.T) {
 
 	opts, _ := cmd.GetOptions(nil)
 
-	req := NewRequest(nil, nil, nil, nil, opts)
+	req, _ := NewRequest(nil, nil, nil, nil, opts)
 	req.SetOption("beep", true)
 	res := cmd.Call(req)
 	if res.Error() == nil {
 		t.Error("Should have failed (incorrect type)")
 	}
 
-	req = NewRequest(nil, nil, nil, nil, opts)
+	req, _ = NewRequest(nil, nil, nil, nil, opts)
 	req.SetOption("beep", 5)
 	res = cmd.Call(req)
 	if res.Error() != nil {
 		t.Error(res.Error(), "Should have passed")
 	}
 
-	req = NewRequest(nil, nil, nil, nil, opts)
+	req, _ = NewRequest(nil, nil, nil, nil, opts)
 	req.SetOption("beep", 5)
 	req.SetOption("boop", "test")
 	res = cmd.Call(req)
@@ -39,7 +39,7 @@ func TestOptionValidation(t *testing.T) {
 		t.Error("Should have passed")
 	}
 
-	req = NewRequest(nil, nil, nil, nil, opts)
+	req, _ = NewRequest(nil, nil, nil, nil, opts)
 	req.SetOption("b", 5)
 	req.SetOption("B", "test")
 	res = cmd.Call(req)
@@ -47,28 +47,28 @@ func TestOptionValidation(t *testing.T) {
 		t.Error("Should have passed")
 	}
 
-	req = NewRequest(nil, nil, nil, nil, opts)
+	req, _ = NewRequest(nil, nil, nil, nil, opts)
 	req.SetOption("foo", 5)
 	res = cmd.Call(req)
 	if res.Error() != nil {
 		t.Error("Should have passed")
 	}
 
-	req = NewRequest(nil, nil, nil, nil, opts)
+	req, _ = NewRequest(nil, nil, nil, nil, opts)
 	req.SetOption(EncShort, "json")
 	res = cmd.Call(req)
 	if res.Error() != nil {
 		t.Error("Should have passed")
 	}
 
-	req = NewRequest(nil, nil, nil, nil, opts)
+	req, _ = NewRequest(nil, nil, nil, nil, opts)
 	req.SetOption("b", "100")
 	res = cmd.Call(req)
 	if res.Error() != nil {
 		t.Error("Should have passed")
 	}
 
-	req = NewRequest(nil, nil, nil, nil, opts)
+	req, _ = NewRequest(nil, nil, nil, nil, opts)
 	req.SetOption("b", ":)")
 	res = cmd.Call(req)
 	if res.Error() == nil {

commands/http/parse.go

@@ -94,7 +94,10 @@ func Parse(r *http.Request, root *cmds.Command) (cmds.Request, error) {
 		return nil, err
 	}
 
-	req := cmds.NewRequest(path, opts, args, cmd, optDefs)
+	req, err := cmds.NewRequest(path, opts, args, cmd, optDefs)
+	if err != nil {
+		return nil, err
+	}
 
 	err = cmd.CheckArguments(req)
 	if err != nil {

commands/request.go

@@ -223,8 +223,12 @@ func (r *request) ConvertOptions() error {
 				}
 				val, err := convert(str)
 				if err != nil {
-					return fmt.Errorf("Could not convert string value '%s' to type '%s'",
-						v, opt.Type.String())
+					value := fmt.Sprintf("value '%v'", v)
+					if len(str) == 0 {
+						value = "empty value"
+					}
+					return fmt.Errorf("Could not convert %s to type '%s' (for option '-%s')",
+						value, opt.Type.String(), k)
 				}
 				r.options[k] = val
 
@@ -248,12 +252,13 @@ func (r *request) ConvertOptions() error {
 }
 
 // NewEmptyRequest initializes an empty request
-func NewEmptyRequest() Request {
+func NewEmptyRequest() (Request, error) {
 	return NewRequest(nil, nil, nil, nil, nil)
 }
 
 // NewRequest returns a request initialized with given arguments
-func NewRequest(path []string, opts optMap, args []interface{}, cmd *Command, optDefs map[string]Option) Request {
+// An non-nil error will be returned if the provided option values are invalid
+func NewRequest(path []string, opts optMap, args []interface{}, cmd *Command, optDefs map[string]Option) (Request, error) {
 	if path == nil {
 		path = make([]string, 0)
 	}
@@ -268,7 +273,10 @@ func NewRequest(path []string, opts optMap, args []interface{}, cmd *Command, op
 	}
 
 	req := &request{path, opts, args, cmd, Context{}, optDefs}
-	req.ConvertOptions()
+	err := req.ConvertOptions()
+	if err != nil {
+		return nil, err
+	}
 
-	return req
+	return req, nil
 }

commands/response_test.go

@@ -15,7 +15,7 @@ func TestMarshalling(t *testing.T) {
 	cmd := &Command{}
 	opts, _ := cmd.GetOptions(nil)
 
-	req := NewRequest(nil, nil, nil, nil, opts)
+	req, _ := NewRequest(nil, nil, nil, nil, opts)
 
 	res := NewResponse(req)
 	res.SetOutput(TestOutput{"beep", "boop", 1337})