Repository: urfave/cli
Pull request #1354 (2022-04-19 23:44:10)

Merging mostynb report source of parse errors
(supersedes #1168)

## What type of PR is this?

See #1168 


Patch:
diff --git a/flag.go b/flag.go
index 0c61d00..b750a28 100644
--- a/flag.go
+++ b/flag.go
@@ -386,21 +386,24 @@ func hasFlag(flags []Flag, fl Flag) bool {
 	return false
 }
 
-func flagFromEnvOrFile(envVars []string, filePath string) (val string, ok bool) {
+// Return the first value from a list of environment variables and files
+// (which may or may not exist), a description of where the value was found,
+// and a boolean which is true if a value was found.
+func flagFromEnvOrFile(envVars []string, filePath string) (value string, fromWhere string, found bool) {
 	for _, envVar := range envVars {
 		envVar = strings.TrimSpace(envVar)
-		if val, ok := syscall.Getenv(envVar); ok {
-			return val, true
+		if value, found := syscall.Getenv(envVar); found {
+			return value, fmt.Sprintf("environment variable %q", envVar), true
 		}
 	}
 	for _, fileVar := range strings.Split(filePath, ",") {
 		if fileVar != "" {
 			if data, err := ioutil.ReadFile(fileVar); err == nil {
-				return string(data), true
+				return string(data), fmt.Sprintf("file %q", filePath), true
 			}
 		}
 	}
-	return "", false
+	return "", "", false
 }
 
 func flagSplitMultiValues(val string) []string {
diff --git a/flag_bool.go b/flag_bool.go
index 1277504..ff5eb7f 100644
--- a/flag_bool.go
+++ b/flag_bool.go
@@ -37,12 +37,12 @@ func (f *BoolFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *BoolFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valBool, err := strconv.ParseBool(val)
 
 			if err != nil {
-				return fmt.Errorf("could not parse %q as bool value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as bool value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 
 			f.Value = valBool
diff --git a/flag_duration.go b/flag_duration.go
index 236056c..bdc29b6 100644
--- a/flag_duration.go
+++ b/flag_duration.go
@@ -37,12 +37,12 @@ func (f *DurationFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *DurationFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valDuration, err := time.ParseDuration(val)
 
 			if err != nil {
-				return fmt.Errorf("could not parse %q as duration value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as duration value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 
 			f.Value = valDuration
diff --git a/flag_float64.go b/flag_float64.go
index aa2f359..9aa3f69 100644
--- a/flag_float64.go
+++ b/flag_float64.go
@@ -37,11 +37,11 @@ func (f *Float64Flag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *Float64Flag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valFloat, err := strconv.ParseFloat(val, 64)
 			if err != nil {
-				return fmt.Errorf("could not parse %q as float64 value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as float64 value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 
 			f.Value = valFloat
diff --git a/flag_float64_slice.go b/flag_float64_slice.go
index e2bfc4c..377b779 100644
--- a/flag_float64_slice.go
+++ b/flag_float64_slice.go
@@ -115,13 +115,13 @@ func (f *Float64SliceFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *Float64SliceFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			f.Value = &Float64Slice{}
 
 			for _, s := range flagSplitMultiValues(val) {
 				if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
-					return fmt.Errorf("could not parse %q as float64 slice value for flag %s: %s", f.Value, f.Name, err)
+					return fmt.Errorf("could not parse %q as float64 slice value from %s for flag %s: %s", f.Value, source, f.Name, err)
 				}
 			}
 
diff --git a/flag_generic.go b/flag_generic.go
index 8be32b8..d7c0ed4 100644
--- a/flag_generic.go
+++ b/flag_generic.go
@@ -46,10 +46,10 @@ func (f *GenericFlag) GetEnvVars() []string {
 // Apply takes the flagset and calls Set on the generic flag with the value
 // provided by the user for parsing by the flag
 func (f GenericFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			if err := f.Value.Set(val); err != nil {
-				return fmt.Errorf("could not parse %q as value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q from %s as value for flag %s: %s", val, source, f.Name, err)
 			}
 
 			f.HasBeenSet = true
diff --git a/flag_int.go b/flag_int.go
index 3f5dec5..e7e4077 100644
--- a/flag_int.go
+++ b/flag_int.go
@@ -37,12 +37,12 @@ func (f *IntFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *IntFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valInt, err := strconv.ParseInt(val, 0, 64)
 
 			if err != nil {
-				return fmt.Errorf("could not parse %q as int value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as int value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 
 			f.Value = int(valInt)
diff --git a/flag_int64.go b/flag_int64.go
index d005866..96a8e9a 100644
--- a/flag_int64.go
+++ b/flag_int64.go
@@ -37,12 +37,12 @@ func (f *Int64Flag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *Int64Flag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valInt, err := strconv.ParseInt(val, 0, 64)
 
 			if err != nil {
-				return fmt.Errorf("could not parse %q as int value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as int value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 
 			f.Value = valInt
diff --git a/flag_int64_slice.go b/flag_int64_slice.go
index b61bd7f..46e8e98 100644
--- a/flag_int64_slice.go
+++ b/flag_int64_slice.go
@@ -116,12 +116,12 @@ func (f *Int64SliceFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *Int64SliceFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		f.Value = &Int64Slice{}
 
 		for _, s := range flagSplitMultiValues(val) {
 			if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
-				return fmt.Errorf("could not parse %q as int64 slice value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as int64 slice value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 		}
 
diff --git a/flag_int_slice.go b/flag_int_slice.go
index f9713cc..2071cbf 100644
--- a/flag_int_slice.go
+++ b/flag_int_slice.go
@@ -127,12 +127,12 @@ func (f *IntSliceFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *IntSliceFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		f.Value = &IntSlice{}
 
 		for _, s := range flagSplitMultiValues(val) {
 			if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
-				return fmt.Errorf("could not parse %q as int slice value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as int slice value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 		}
 
diff --git a/flag_path.go b/flag_path.go
index b0c2215..2866f84 100644
--- a/flag_path.go
+++ b/flag_path.go
@@ -41,7 +41,7 @@ func (f *PathFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *PathFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, _, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		f.Value = val
 		f.HasBeenSet = true
 	}
diff --git a/flag_string.go b/flag_string.go
index 24adbe9..6333afe 100644
--- a/flag_string.go
+++ b/flag_string.go
@@ -39,7 +39,7 @@ func (f *StringFlag) GetEnvVars() []string {
 
 // Apply populates the flag given the flag set and environment
 func (f *StringFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, _, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		f.Value = val
 		f.HasBeenSet = true
 	}
diff --git a/flag_string_slice.go b/flag_string_slice.go
index d0195d5..34c8af1 100644
--- a/flag_string_slice.go
+++ b/flag_string_slice.go
@@ -117,7 +117,7 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error {
 
 	}
 
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if f.Value == nil {
 			f.Value = &StringSlice{}
 		}
@@ -128,7 +128,7 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error {
 
 		for _, s := range flagSplitMultiValues(val) {
 			if err := destination.Set(strings.TrimSpace(s)); err != nil {
-				return fmt.Errorf("could not parse %q as string value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as string value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 		}
 
diff --git a/flag_timestamp.go b/flag_timestamp.go
index ed480cf..e0dd912 100644
--- a/flag_timestamp.go
+++ b/flag_timestamp.go
@@ -104,9 +104,9 @@ func (f *TimestampFlag) Apply(set *flag.FlagSet) error {
 		f.Destination.SetLayout(f.Layout)
 	}
 
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if err := f.Value.Set(val); err != nil {
-			return fmt.Errorf("could not parse %q as timestamp value for flag %s: %s", val, f.Name, err)
+			return fmt.Errorf("could not parse %q as timestamp value from %s for flag %s: %s", val, source, f.Name, err)
 		}
 		f.HasBeenSet = true
 	}
diff --git a/flag_uint.go b/flag_uint.go
index 1ec9713..1936a9f 100644
--- a/flag_uint.go
+++ b/flag_uint.go
@@ -18,11 +18,11 @@ func (f *UintFlag) GetUsage() string {
 
 // Apply populates the flag given the flag set and environment
 func (f *UintFlag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valInt, err := strconv.ParseUint(val, 0, 64)
 			if err != nil {
-				return fmt.Errorf("could not parse %q as uint value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as uint value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 
 			f.Value = uint(valInt)
diff --git a/flag_uint64.go b/flag_uint64.go
index 55ba08a..59e1bb5 100644
--- a/flag_uint64.go
+++ b/flag_uint64.go
@@ -18,11 +18,11 @@ func (f *Uint64Flag) GetUsage() string {
 
 // Apply populates the flag given the flag set and environment
 func (f *Uint64Flag) Apply(set *flag.FlagSet) error {
-	if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
+	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
 		if val != "" {
 			valInt, err := strconv.ParseUint(val, 0, 64)
 			if err != nil {
-				return fmt.Errorf("could not parse %q as uint64 value for flag %s: %s", val, f.Name, err)
+				return fmt.Errorf("could not parse %q as uint64 value from %s for flag %s: %s", val, source, f.Name, err)
 			}
 
 			f.Value = valInt


Test patch:
diff --git a/flag_test.go b/flag_test.go
index 9433fc9..ba90e91 100644
--- a/flag_test.go
+++ b/flag_test.go
@@ -96,33 +96,33 @@ func TestFlagsFromEnv(t *testing.T) {
 		{"", false, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""},
 		{"1", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""},
 		{"false", false, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, ""},
-		{"foobar", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, `could not parse "foobar" as bool value for flag debug: .*`},
+		{"foobar", true, &BoolFlag{Name: "debug", EnvVars: []string{"DEBUG"}}, `could not parse "foobar" as bool value from environment variable "DEBUG" for flag debug: .*`},
 
 		{"1s", 1 * time.Second, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, ""},
-		{"foobar", false, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, `could not parse "foobar" as duration value for flag time: .*`},
+		{"foobar", false, &DurationFlag{Name: "time", EnvVars: []string{"TIME"}}, `could not parse "foobar" as duration value from environment variable "TIME" for flag time: .*`},
 
 		{"1.2", 1.2, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
 		{"1", 1.0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
-		{"foobar", 0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as float64 value for flag seconds: .*`},
+		{"foobar", 0, &Float64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as float64 value from environment variable "SECONDS" for flag seconds: .*`},
 
 		{"1", int64(1), &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
-		{"1.2", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value for flag seconds: .*`},
-		{"foobar", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value for flag seconds: .*`},
+		{"1.2", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value from environment variable "SECONDS" for flag seconds: .*`},
+		{"foobar", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value from environment variable "SECONDS" for flag seconds: .*`},
 
 		{"1", 1, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
-		{"1.2", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value for flag seconds: .*`},
-		{"foobar", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value for flag seconds: .*`},
+		{"1.2", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value from environment variable "SECONDS" for flag seconds: .*`},
+		{"foobar", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value from environment variable "SECONDS" for flag seconds: .*`},
 
 		{"1.0,2", newSetFloat64Slice(1, 2), &Float64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
-		{"foobar", newSetFloat64Slice(), &Float64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "\[\]float64{}" as float64 slice value for flag seconds: .*`},
+		{"foobar", newSetFloat64Slice(), &Float64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "\[\]float64{}" as float64 slice value from environment variable "SECONDS" for flag seconds: .*`},
 
 		{"1,2", newSetIntSlice(1, 2), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
-		{"1.2,2", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int slice value for flag seconds: .*`},
-		{"foobar", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int slice value for flag seconds: .*`},
+		{"1.2,2", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int slice value from environment variable "SECONDS" for flag seconds: .*`},
+		{"foobar", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int slice value from environment variable "SECONDS" for flag seconds: .*`},
 
 		{"1,2", newSetInt64Slice(1, 2), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
-		{"1.2,2", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int64 slice value for flag seconds: .*`},
-		{"foobar", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int64 slice value for flag seconds: .*`},
+		{"1.2,2", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int64 slice value from environment variable "SECONDS" for flag seconds: .*`},
+		{"foobar", newSetInt64Slice(), &Int64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int64 slice value from environment variable "SECONDS" for flag seconds: .*`},
 
 		{"foo", "foo", &StringFlag{Name: "name", EnvVars: []string{"NAME"}}, ""},
 		{"path", "path", &PathFlag{Name: "path", EnvVars: []string{"PATH"}}, ""},
@@ -130,12 +130,12 @@ func TestFlagsFromEnv(t *testing.T) {
 		{"foo,bar", newSetStringSlice("foo", "bar"), &StringSliceFlag{Name: "names", EnvVars: []string{"NAMES"}}, ""},
 
 		{"1", uint(1), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
-		{"1.2", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint value for flag seconds: .*`},
-		{"foobar", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint value for flag seconds: .*`},
+		{"1.2", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint value from environment variable "SECONDS" for flag seconds: .*`},
+		{"foobar", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint value from environment variable "SECONDS" for flag seconds: .*`},
 
 		{"1", uint64(1), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
-		{"1.2", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint64 value for flag seconds: .*`},
-		{"foobar", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint64 value for flag seconds: .*`},
+		{"1.2", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint64 value from environment variable "SECONDS" for flag seconds: .*`},
+		{"foobar", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint64 value from environment variable "SECONDS" for flag seconds: .*`},
 
 		{"foo,bar", &Parser{"foo", "bar"}, &GenericFlag{Name: "names", Value: &Parser{}, EnvVars: []string{"NAMES"}}, ""},
 	}
@@ -2157,7 +2157,7 @@ func TestFlagFromFile(t *testing.T) {
 	}
 
 	for _, filePathTest := range filePathTests {
-		got, _ := flagFromEnvOrFile(filePathTest.name, filePathTest.path)
+		got, _, _ := flagFromEnvOrFile(filePathTest.name, filePathTest.path)
 		if want := filePathTest.expected; got != want {
 			t.Errorf("Did not expect %v - Want %v", got, want)
 		}