[componenttest] Improve config struct checks

.chloggen/improve-config-tag-checks.yaml

@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: componenttest
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Improve config struct mapstructure field tag checks
+
+# One or more tracking issues or pull requests related to the change
+issues: [12590]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: "`remain` tags and `omitempty` tags without a custom field name will now pass validation."
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

component/componenttest/configtest.go

@@ -65,8 +65,8 @@ func validateConfigDataType(t reflect.Type) error {
 
 // checkStructFieldTags inspects the tags of a struct field.
 func checkStructFieldTags(f reflect.StructField) error {
-	tagValue := f.Tag.Get("mapstructure")
-	if tagValue == "" {
+	tagValue, ok := f.Tag.Lookup("mapstructure")
+	if !ok {
 		// Ignore special types.
 		switch f.Type.Kind() {
 		case reflect.Interface, reflect.Chan, reflect.Func, reflect.Uintptr, reflect.UnsafePointer:
@@ -85,6 +85,10 @@ func checkStructFieldTags(f reflect.StructField) error {
 		return nil
 	}
 
+	if tagValue == "" {
+		return fmt.Errorf("mapstructure tag on field %q is empty", f.Name)
+	}
+
 	tagParts := strings.Split(tagValue, ",")
 	if tagParts[0] != "" {
 		if tagParts[0] == "-" {
@@ -93,20 +97,17 @@ func checkStructFieldTags(f reflect.StructField) error {
 		}
 	}
 
-	// Check if squash is specified.
-	squash := false
 	for _, tag := range tagParts[1:] {
-		if tag == "squash" {
-			squash = true
-			break
-		}
-	}
-
-	if squash {
-		// Field was squashed.
-		if (f.Type.Kind() != reflect.Struct) && (f.Type.Kind() != reflect.Ptr || f.Type.Elem().Kind() != reflect.Struct) {
-			return fmt.Errorf(
-				"attempt to squash non-struct type on field %q", f.Name)
+		switch tag {
+		case "squash":
+			if (f.Type.Kind() != reflect.Struct) && (f.Type.Kind() != reflect.Ptr || f.Type.Elem().Kind() != reflect.Struct) {
+				return fmt.Errorf(
+					"attempt to squash non-struct type on field %q", f.Name)
+			}
+		case "remain":
+			if f.Type.Kind() != reflect.Map && f.Type.Kind() != reflect.Interface {
+				return fmt.Errorf(`attempt to use "remain" on non-map or interface type field %q`, f.Name)
+			}
 		}
 	}
 
@@ -121,10 +122,7 @@ func checkStructFieldTags(f reflect.StructField) error {
 
 	default:
 		fieldTag := tagParts[0]
-		if !configFieldTagRegExp.MatchString(fieldTag) {
-			if f.Name == "AdditionalProperties" {
-				return nil
-			}
+		if fieldTag != "" && !configFieldTagRegExp.MatchString(fieldTag) {
 			return fmt.Errorf(
 				"field %q has config tag %q which doesn't satisfy %q",
 				f.Name,

component/componenttest/configtest_test.go

@@ -51,6 +51,30 @@ func TestCheckConfigStruct(t *testing.T) {
 				_someInt        int
 			}{},
 		},
+		{
+			name: "remain_mapstructure_tag",
+			config: struct {
+				AdditionalProperties map[string]any `mapstructure:",remain"`
+			}{},
+		},
+		{
+			name: "remain_with_interface_type",
+			config: struct {
+				AdditionalProperties any `mapstructure:",remain"`
+			}{},
+		},
+		{
+			name: "omitempty_mapstructure_tag",
+			config: struct {
+				MyPublicString string `mapstructure:",omitempty"`
+			}{},
+		},
+		{
+			name: "named_omitempty_mapstructure_tag",
+			config: struct {
+				MyPublicString string `mapstructure:"my_public_string,omitempty"`
+			}{},
+		},
 		{
 			name: "not_struct_nor_pointer",
 			config: func(x int) int {
@@ -65,6 +89,20 @@ func TestCheckConfigStruct(t *testing.T) {
 			}{},
 			wantErrMsgSubStr: "attempt to squash non-struct type on field \"MyInt\"",
 		},
+		{
+			name: "remain_on_non_map",
+			config: struct {
+				AdditionalProperties string `mapstructure:",remain"`
+			}{},
+			wantErrMsgSubStr: `attempt to use "remain" on non-map or interface type field "AdditionalProperties"`,
+		},
+		{
+			name: "bad_custom_field_name",
+			config: struct {
+				AdditionalProperties any `mapstructure:"Additional_Properties"`
+			}{},
+			wantErrMsgSubStr: `field "AdditionalProperties" has config tag "Additional_Properties" which doesn't satisfy "^[a-z0-9][a-z0-9_]*$"`,
+		},
 		{
 			name:             "invalid_tag_detected",
 			config:           BadConfigTag{},
@@ -77,6 +115,13 @@ func TestCheckConfigStruct(t *testing.T) {
 			}{},
 			wantErrMsgSubStr: "mapstructure tag not present on field \"PublicFieldWithoutMapstructureTag\"",
 		},
+		{
+			name: "public_field_must_have_nonempty_tag",
+			config: struct {
+				PublicFieldWithoutMapstructureTag string `mapstructure:""`
+			}{},
+			wantErrMsgSubStr: "mapstructure tag on field \"PublicFieldWithoutMapstructureTag\" is empty",
+		},
 		{
 			name: "invalid_map_item",
 			config: struct {