Repository: urfave/cli
Pull request #1708 (2023-03-19 19:12:09)

Fix `incorrectTypeForFlagError` for unknowns
The `reflect` package doesn't always know what to call a type in `(reflect.Type).Name()`, but almost always gets it right in `(reflect.Type).String()`. Fall back to that so we get actionable error messages.

## What type of PR is this?

- bug

## What this PR does / why we need it:

When configs parsed into `MapInputSource` are looked up, they report type mismatch errors, as expected. However, if the type is mismatched for any reason, the error message isn't particularly useful - it reports an "actual type" of `''`. This is a known limitation of the way `(reflect.Type).Name()` works (it only provides names for package-defined types, not built-in composites or pointers or a handful of other things). Falling back to `(reflect.Type).String()` in these cases provides enough data for a developer to fix the actual issue (in this case, using the correct type in their `InputSourceContext` implementation), which is the entire point of returning a type name in that specific error message.

v3 doesn't have support for `InputSourceContext`s (or any equivalent I could find), so this fix currently only targets v2.

## Testing

A test was added to ensure this change works as expected. 

## Release Notes

```release-note
NONE
```

skelouse: Why don't we just do:

```go
fmt.Errorf("Mismatched type for flag '%s'. Expected '%s' but actual is '%T'", name, expectedTypeName, value)
```
danhunsaker: That's a good idea. Wonder if `%T` existed when this was originally written...
skelouse: > That's a good idea. Wonder if `%T` existed when this was originally written...

Looks like it has been around since go1, https://pkg.go.dev/fmt@go1

Patch:
diff --git a/altsrc/map_input_source.go b/altsrc/map_input_source.go
index abd989f..1a76051 100644
--- a/altsrc/map_input_source.go
+++ b/altsrc/map_input_source.go
@@ -3,7 +3,6 @@ package altsrc
 import (
 	"fmt"
 	"math"
-	"reflect"
 	"strings"
 	"time"
 
@@ -471,11 +470,5 @@ func (fsm *MapInputSource) isSet(name string) bool {
 }
 
 func incorrectTypeForFlagError(name, expectedTypeName string, value interface{}) error {
-	valueType := reflect.TypeOf(value)
-	valueTypeName := ""
-	if valueType != nil {
-		valueTypeName = valueType.Name()
-	}
-
-	return fmt.Errorf("Mismatched type for flag '%s'. Expected '%s' but actual is '%s'", name, expectedTypeName, valueTypeName)
+	return fmt.Errorf("Mismatched type for flag '%s'. Expected '%s' but actual is '%T'", name, expectedTypeName, value)
 }


Test patch:
diff --git a/altsrc/map_input_source_test.go b/altsrc/map_input_source_test.go
index cf399b5..4d3514e 100644
--- a/altsrc/map_input_source_test.go
+++ b/altsrc/map_input_source_test.go
@@ -1,6 +1,7 @@
 package altsrc
 
 import (
+	"fmt"
 	"testing"
 	"time"
 )
@@ -33,3 +34,8 @@ func TestMapInputSource_Int64Slice(t *testing.T) {
 	expect(t, []int64{1, 2, 3}, d)
 	expect(t, nil, err)
 }
+
+func TestMapInputSource_IncorrectFlagTypeError(t *testing.T) {
+	var testVal *bool
+	expect(t, incorrectTypeForFlagError("test", "bool", testVal), fmt.Errorf("Mismatched type for flag 'test'. Expected 'bool' but actual is '*bool'"))
+}