Repository: compose-spec/compose-go
Pull request #393 (2023-04-19 10:15:37)

required variable should not trigger a generic InvalidTemplateError
closes https://github.com/docker/compose/issues/10293


Patch:
diff --git a/template/template.go b/template/template.go
index 27f7067..f9b52f6 100644
--- a/template/template.go
+++ b/template/template.go
@@ -47,6 +47,19 @@ func (e InvalidTemplateError) Error() string {
 	return fmt.Sprintf("Invalid template: %#v", e.Template)
 }
 
+// MissingRequiredError is returned when a variable template is missing
+type MissingRequiredError struct {
+	Variable string
+	Reason   string
+}
+
+func (e MissingRequiredError) Error() string {
+	if e.Reason != "" {
+		return fmt.Sprintf("required variable %s is missing a value: %s", e.Variable, e.Reason)
+	}
+	return fmt.Sprintf("required variable %s is missing a value", e.Variable)
+}
+
 // Mapping is a user-supplied function which maps from variable names to values.
 // Returns the value as a string and a bool indicating whether
 // the value is present, to distinguish between an empty string
@@ -351,8 +364,9 @@ func withRequired(substitution string, mapping Mapping, sep string, valid func(s
 	}
 	value, ok := mapping(name)
 	if !ok || !valid(value) {
-		return "", true, &InvalidTemplateError{
-			Template: fmt.Sprintf("required variable %s is missing a value: %s", name, errorMessage),
+		return "", true, &MissingRequiredError{
+			Reason:   errorMessage,
+			Variable: name,
 		}
 	}
 	return value, true, nil


Test patch:
diff --git a/dotenv/godotenv_var_expansion_test.go b/dotenv/godotenv_var_expansion_test.go
index f8a2ff7..a78fc59 100644
--- a/dotenv/godotenv_var_expansion_test.go
+++ b/dotenv/godotenv_var_expansion_test.go
@@ -92,13 +92,13 @@ func TestErrorIfEmptyOrUnset(t *testing.T) {
 			"Error empty or unset: UNSET_VAR",
 			"RESULT=${UNSET_VAR:?Test error}",
 			"RESULT=${UNSET_VAR:?Test error}",
-			&template.InvalidTemplateError{Template: "required variable UNSET_VAR is missing a value: Test error"},
+			&template.MissingRequiredError{Variable: "UNSET_VAR", Reason: "Test error"},
 		},
 		{
 			"Error empty or unset: EMPTY_VAR",
 			"RESULT=${EMPTY_VAR:?Test error}",
 			"RESULT=${EMPTY_VAR:?Test error}",
-			&template.InvalidTemplateError{Template: "required variable EMPTY_VAR is missing a value: Test error"},
+			&template.MissingRequiredError{Variable: "EMPTY_VAR", Reason: "Test error"},
 		},
 		{
 			"Error empty or unset: TEST_VAR",
@@ -128,7 +128,7 @@ func TestErrorIfUnset(t *testing.T) {
 			"Error on unset: UNSET_VAR",
 			"RESULT=${UNSET_VAR?Test error}",
 			"RESULT=${UNSET_VAR?Test error}",
-			&template.InvalidTemplateError{Template: "required variable UNSET_VAR is missing a value: Test error"},
+			&template.MissingRequiredError{Variable: "UNSET_VAR", Reason: "Test error"},
 		},
 		{
 			"Error on unset: EMPTY_VAR",
diff --git a/interpolation/interpolation_test.go b/interpolation/interpolation_test.go
index 8502731..ca8da0e 100644
--- a/interpolation/interpolation_test.go
+++ b/interpolation/interpolation_test.go
@@ -134,8 +134,7 @@ func TestValidUnexistentInterpolation(t *testing.T) {
 	}
 
 	getFullErrorMsg := func(msg string) string {
-		return fmt.Sprintf("invalid interpolation format for myservice.environment.TESTVAR.\n"+
-			"You may need to escape any $ with another $.\nrequired variable FOO is missing a value: %s", msg)
+		return fmt.Sprintf("error while interpolating myservice.environment.TESTVAR: required variable FOO is missing a value: %s", msg)
 	}
 
 	for _, testcase := range testcases {
diff --git a/loader/loader_test.go b/loader/loader_test.go
index f37e585..cf27efc 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -2351,6 +2351,6 @@ func TestDeviceWriteBps(t *testing.T) {
 
 func TestInvalidProjectNameType(t *testing.T) {
 	p, err := loadYAML(`name: 123`)
-	assert.Error(t, err, "name must be a string")
+	assert.Error(t, err, "validating filename0.yml: name must be a string")
 	assert.Assert(t, is.Nil(p))
 }
diff --git a/template/template_test.go b/template/template_test.go
index 27c0f0e..1a5c6a9 100644
--- a/template/template_test.go
+++ b/template/template_test.go
@@ -302,7 +302,7 @@ func TestMandatoryVariableErrors(t *testing.T) {
 	for _, tc := range testCases {
 		_, err := Substitute(tc.template, defaultMapping)
 		assert.ErrorContains(t, err, tc.expectedError)
-		assert.ErrorType(t, err, reflect.TypeOf(&InvalidTemplateError{}))
+		assert.ErrorType(t, err, reflect.TypeOf(&MissingRequiredError{}))
 	}
 }
 
@@ -324,7 +324,7 @@ func TestMandatoryVariableErrorsWithNestedExpansion(t *testing.T) {
 	for _, tc := range testCases {
 		_, err := Substitute(tc.template, defaultMapping)
 		assert.ErrorContains(t, err, tc.expectedError)
-		assert.ErrorType(t, err, reflect.TypeOf(&InvalidTemplateError{}))
+		assert.ErrorType(t, err, reflect.TypeOf(&MissingRequiredError{}))
 	}
 }
 
@@ -388,8 +388,9 @@ func TestPrecedence(t *testing.T) {
 		{
 			template: "${UNSET_VAR?bar-baz}", // Unexistent variable
 			expected: "",
-			err: &InvalidTemplateError{
-				Template: "required variable UNSET_VAR is missing a value: bar-baz",
+			err: &MissingRequiredError{
+				Variable: "UNSET_VAR",
+				Reason:   "bar-baz",
 			},
 		},
 		{