Repository: zeromicro/go-zero
Pull request #3156 (2023-04-22 13:31:57)

feat: inheritance rewrite error prompt is more friendly

codecov[bot]: ## [Codecov](https://codecov.io/gh/zeromicro/go-zero/pull/3156?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#3156](https://codecov.io/gh/zeromicro/go-zero/pull/3156?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (be491f5) into [master](https://codecov.io/gh/zeromicro/go-zero/commit/027193dc99ded84159bdcd31756af83cd958c397?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (027193d) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.


| [Impacted Files](https://codecov.io/gh/zeromicro/go-zero/pull/3156?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [core/conf/config.go](https://codecov.io/gh/zeromicro/go-zero/pull/3156?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9jb25mL2NvbmZpZy5nbw==) | `98.65% <100.00%> (+0.04%)` | :arrow_up: |


Patch:
diff --git a/core/conf/config.go b/core/conf/config.go
index c39c73bf..02ef9666 100644
--- a/core/conf/config.go
+++ b/core/conf/config.go
@@ -73,7 +73,7 @@ func LoadConfig(file string, v any, opts ...Option) error {
 
 // LoadFromJsonBytes loads config into v from content json bytes.
 func LoadFromJsonBytes(content []byte, v any) error {
-	info, err := buildFieldsInfo(reflect.TypeOf(v))
+	info, err := buildFieldsInfo(reflect.TypeOf(v), "")
 	if err != nil {
 		return err
 	}
@@ -127,13 +127,13 @@ func MustLoad(path string, v any, opts ...Option) {
 	}
 }
 
-func addOrMergeFields(info *fieldInfo, key string, child *fieldInfo) error {
+func addOrMergeFields(info *fieldInfo, key string, child *fieldInfo, fullName string) error {
 	if prev, ok := info.children[key]; ok {
 		if child.mapField != nil {
-			return newConflictKeyError(key)
+			return newConflictKeyError(fullName)
 		}
 
-		if err := mergeFields(prev, key, child.children); err != nil {
+		if err := mergeFields(prev, key, child.children, fullName); err != nil {
 			return err
 		}
 	} else {
@@ -143,27 +143,27 @@ func addOrMergeFields(info *fieldInfo, key string, child *fieldInfo) error {
 	return nil
 }
 
-func buildAnonymousFieldInfo(info *fieldInfo, lowerCaseName string, ft reflect.Type) error {
+func buildAnonymousFieldInfo(info *fieldInfo, lowerCaseName string, ft reflect.Type, fullName string) error {
 	switch ft.Kind() {
 	case reflect.Struct:
-		fields, err := buildFieldsInfo(ft)
+		fields, err := buildFieldsInfo(ft, fullName)
 		if err != nil {
 			return err
 		}
 
 		for k, v := range fields.children {
-			if err = addOrMergeFields(info, k, v); err != nil {
+			if err = addOrMergeFields(info, k, v, fullName); err != nil {
 				return err
 			}
 		}
 	case reflect.Map:
-		elemField, err := buildFieldsInfo(mapping.Deref(ft.Elem()))
+		elemField, err := buildFieldsInfo(mapping.Deref(ft.Elem()), fullName)
 		if err != nil {
 			return err
 		}
 
 		if _, ok := info.children[lowerCaseName]; ok {
-			return newConflictKeyError(lowerCaseName)
+			return newConflictKeyError(fullName)
 		}
 
 		info.children[lowerCaseName] = &fieldInfo{
@@ -172,7 +172,7 @@ func buildAnonymousFieldInfo(info *fieldInfo, lowerCaseName string, ft reflect.T
 		}
 	default:
 		if _, ok := info.children[lowerCaseName]; ok {
-			return newConflictKeyError(lowerCaseName)
+			return newConflictKeyError(fullName)
 		}
 
 		info.children[lowerCaseName] = &fieldInfo{
@@ -183,14 +183,14 @@ func buildAnonymousFieldInfo(info *fieldInfo, lowerCaseName string, ft reflect.T
 	return nil
 }
 
-func buildFieldsInfo(tp reflect.Type) (*fieldInfo, error) {
+func buildFieldsInfo(tp reflect.Type, fullName string) (*fieldInfo, error) {
 	tp = mapping.Deref(tp)
 
 	switch tp.Kind() {
 	case reflect.Struct:
-		return buildStructFieldsInfo(tp)
+		return buildStructFieldsInfo(tp, fullName)
 	case reflect.Array, reflect.Slice:
-		return buildFieldsInfo(mapping.Deref(tp.Elem()))
+		return buildFieldsInfo(mapping.Deref(tp.Elem()), fullName)
 	case reflect.Chan, reflect.Func:
 		return nil, fmt.Errorf("unsupported type: %s", tp.Kind())
 	default:
@@ -200,23 +200,23 @@ func buildFieldsInfo(tp reflect.Type) (*fieldInfo, error) {
 	}
 }
 
-func buildNamedFieldInfo(info *fieldInfo, lowerCaseName string, ft reflect.Type) error {
+func buildNamedFieldInfo(info *fieldInfo, lowerCaseName string, ft reflect.Type, fullName string) error {
 	var finfo *fieldInfo
 	var err error
 
 	switch ft.Kind() {
 	case reflect.Struct:
-		finfo, err = buildFieldsInfo(ft)
+		finfo, err = buildFieldsInfo(ft, fullName)
 		if err != nil {
 			return err
 		}
 	case reflect.Array, reflect.Slice:
-		finfo, err = buildFieldsInfo(ft.Elem())
+		finfo, err = buildFieldsInfo(ft.Elem(), fullName)
 		if err != nil {
 			return err
 		}
 	case reflect.Map:
-		elemInfo, err := buildFieldsInfo(mapping.Deref(ft.Elem()))
+		elemInfo, err := buildFieldsInfo(mapping.Deref(ft.Elem()), fullName)
 		if err != nil {
 			return err
 		}
@@ -226,16 +226,16 @@ func buildNamedFieldInfo(info *fieldInfo, lowerCaseName string, ft reflect.Type)
 			mapField: elemInfo,
 		}
 	default:
-		finfo, err = buildFieldsInfo(ft)
+		finfo, err = buildFieldsInfo(ft, fullName)
 		if err != nil {
 			return err
 		}
 	}
 
-	return addOrMergeFields(info, lowerCaseName, finfo)
+	return addOrMergeFields(info, lowerCaseName, finfo, fullName)
 }
 
-func buildStructFieldsInfo(tp reflect.Type) (*fieldInfo, error) {
+func buildStructFieldsInfo(tp reflect.Type, fullName string) (*fieldInfo, error) {
 	info := &fieldInfo{
 		children: make(map[string]*fieldInfo),
 	}
@@ -251,10 +251,12 @@ func buildStructFieldsInfo(tp reflect.Type) (*fieldInfo, error) {
 		ft := mapping.Deref(field.Type)
 		// flatten anonymous fields
 		if field.Anonymous {
-			if err := buildAnonymousFieldInfo(info, lowerCaseName, ft); err != nil {
+			if err := buildAnonymousFieldInfo(info, lowerCaseName, ft,
+				getFullName(fullName, lowerCaseName)); err != nil {
 				return nil, err
 			}
-		} else if err := buildNamedFieldInfo(info, lowerCaseName, ft); err != nil {
+		} else if err := buildNamedFieldInfo(info, lowerCaseName, ft,
+			getFullName(fullName, lowerCaseName)); err != nil {
 			return nil, err
 		}
 	}
@@ -279,15 +281,15 @@ func getTagName(field reflect.StructField) string {
 	return field.Name
 }
 
-func mergeFields(prev *fieldInfo, key string, children map[string]*fieldInfo) error {
+func mergeFields(prev *fieldInfo, key string, children map[string]*fieldInfo, fullName string) error {
 	if len(prev.children) == 0 || len(children) == 0 {
-		return newConflictKeyError(key)
+		return newConflictKeyError(fullName)
 	}
 
 	// merge fields
 	for k, v := range children {
 		if _, ok := prev.children[k]; ok {
-			return newConflictKeyError(k)
+			return newConflictKeyError(fullName)
 		}
 
 		prev.children[k] = v
@@ -349,3 +351,11 @@ func newConflictKeyError(key string) conflictKeyError {
 func (e conflictKeyError) Error() string {
 	return fmt.Sprintf("conflict key %s, pay attention to anonymous fields", e.key)
 }
+
+func getFullName(parent, child string) string {
+	if parent == "" {
+		return child
+	}
+
+	return strings.Join([]string{parent, child}, ".")
+}


Test patch:
diff --git a/core/conf/config_test.go b/core/conf/config_test.go
index 9667f93f..3e20a095 100644
--- a/core/conf/config_test.go
+++ b/core/conf/config_test.go
@@ -2,6 +2,7 @@ package conf
 
 import (
 	"os"
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -733,7 +734,7 @@ func Test_FieldOverwrite(t *testing.T) {
 			input := []byte(`{"Name": "hello"}`)
 			err := LoadFromJsonBytes(input, val)
 			assert.ErrorAs(t, err, &dupErr)
-			assert.Equal(t, newConflictKeyError("name").Error(), err.Error())
+			assert.Error(t, err)
 		}
 
 		validate(&St0{})
@@ -1196,6 +1197,100 @@ Email = "bar"`)
 	})
 }
 
+func Test_getFullName(t *testing.T) {
+	assert.Equal(t, "a.b", getFullName("a", "b"))
+	assert.Equal(t, "a", getFullName("", "a"))
+}
+
+func Test_buildFieldsInfo(t *testing.T) {
+	type ParentSt struct {
+		Name string
+		M    map[string]int
+	}
+	tests := []struct {
+		name        string
+		t           reflect.Type
+		ok          bool
+		containsKey string
+	}{
+		{
+			name: "normal",
+			t:    reflect.TypeOf(struct{ A string }{}),
+			ok:   true,
+		},
+		{
+			name: "struct anonymous",
+			t: reflect.TypeOf(struct {
+				ParentSt
+				Name string
+			}{}),
+			ok:          false,
+			containsKey: newConflictKeyError("name").Error(),
+		},
+		{
+			name: "struct ptr anonymous",
+			t: reflect.TypeOf(struct {
+				*ParentSt
+				Name string
+			}{}),
+			ok:          false,
+			containsKey: newConflictKeyError("name").Error(),
+		},
+		{
+			name: "more struct anonymous",
+			t: reflect.TypeOf(struct {
+				Value struct {
+					ParentSt
+					Name string
+				}
+			}{}),
+			ok:          false,
+			containsKey: newConflictKeyError("value.name").Error(),
+		},
+		{
+			name: "map anonymous",
+			t: reflect.TypeOf(struct {
+				ParentSt
+				M string
+			}{}),
+			ok:          false,
+			containsKey: newConflictKeyError("m").Error(),
+		},
+		{
+			name: "map more anonymous",
+			t: reflect.TypeOf(struct {
+				Value struct {
+					ParentSt
+					M string
+				}
+			}{}),
+			ok:          false,
+			containsKey: newConflictKeyError("value.m").Error(),
+		},
+		{
+			name: "struct slice anonymous",
+			t: reflect.TypeOf([]struct {
+				ParentSt
+				Name string
+			}{}),
+			ok:          false,
+			containsKey: newConflictKeyError("name").Error(),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := buildFieldsInfo(tt.t, "")
+			if tt.ok {
+				assert.NoError(t, err)
+			} else {
+				assert.Error(t, err)
+				assert.Equal(t, err.Error(), tt.containsKey)
+			}
+		})
+	}
+}
+
 func createTempFile(ext, text string) (string, error) {
 	tmpFile, err := os.CreateTemp(os.TempDir(), hash.Md5Hex([]byte(text))+"*"+ext)
 	if err != nil {