Fix binding generation special cases (#1902)

* Make binding.go easier to test

* Fix non-deterministic namespace order for bindings

* Add binding tests

* Fix nested import structs, non-string map keys, and escape invalid variable names

Co-authored-by: Lea Anthony <lea.anthony@gmail.com>

v2/internal/binding/binding.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"reflect"
 	"runtime"
+	"sort"
 	"strings"
 
 	"github.com/wailsapp/wails/v2/internal/typescriptify"
@@ -83,7 +84,7 @@ func (b *Bindings) ToJSON() (string, error) {
 	return b.db.ToJSON()
 }
 
-func (b *Bindings) WriteModels(modelsDir string) error {
+func (b *Bindings) GenerateModels() ([]byte, error) {
 	models := map[string]string{}
 	var seen slicer.StringSlicer
 	allStructNames := b.getAllStructNames()
@@ -102,15 +103,23 @@ func (b *Bindings) WriteModels(modelsDir string) error {
 		}
 		str, err := w.Convert(nil)
 		if err != nil {
-			return err
+			return nil, err
 		}
 		thisPackageCode += str
 		seen.AddSlice(w.GetGeneratedStructs())
 		models[packageName] = thisPackageCode
 	}
 
+	// Sort the package names first to make the output deterministic
+	sortedPackageNames := make([]string, 0)
+	for packageName := range models {
+		sortedPackageNames = append(sortedPackageNames, packageName)
+	}
+	sort.Strings(sortedPackageNames)
+
 	var modelsData bytes.Buffer
-	for packageName, modelData := range models {
+	for _, packageName := range sortedPackageNames {
+		modelData := models[packageName]
 		if strings.TrimSpace(modelData) == "" {
 			continue
 		}
@@ -121,14 +130,22 @@ func (b *Bindings) WriteModels(modelsDir string) error {
 		}
 		modelsData.WriteString("\n}\n\n")
 	}
+	return modelsData.Bytes(), nil
+}
 
+func (b *Bindings) WriteModels(modelsDir string) error {
+
+	modelsData, err := b.GenerateModels()
+	if err != nil {
+		return err
+	}
 	// Don't write if we don't have anything
-	if len(modelsData.Bytes()) == 0 {
+	if len(modelsData) == 0 {
 		return nil
 	}
 
 	filename := filepath.Join(modelsDir, "models.ts")
-	err := os.WriteFile(filename, modelsData.Bytes(), 0755)
+	err = os.WriteFile(filename, modelsData, 0755)
 	if err != nil {
 		return err
 	}
@@ -147,7 +164,7 @@ func (b *Bindings) AddStructToGenerateTS(packageName string, structName string,
 
 	// Iterate this struct and add any struct field references
 	structType := reflect.TypeOf(s)
-	if structType.Kind() == reflect.Ptr {
+	if hasElements(structType) {
 		structType = structType.Elem()
 	}
 
@@ -169,11 +186,11 @@ func (b *Bindings) AddStructToGenerateTS(packageName string, structName string,
 				s := reflect.Indirect(a).Interface()
 				b.AddStructToGenerateTS(pName, sName, s)
 			}
-		} else if kind == reflect.Ptr && field.Type.Elem().Kind() == reflect.Struct {
+		} else if hasElements(field.Type) && field.Type.Elem().Kind() == reflect.Struct {
 			if !field.IsExported() {
 				continue
 			}
-			fqname := field.Type.String()
+			fqname := field.Type.Elem().String()
 			sName := strings.Split(fqname, ".")[1]
 			pName := getPackageName(fqname)
 			typ := field.Type.Elem()

v2/internal/binding/binding_test/binding_escapedname_test.go

@@ -0,0 +1,32 @@
+package binding_test
+
+type EscapedName struct {
+	Name string `json:"n.a.m.e"`
+}
+
+func (s EscapedName) Get() EscapedName {
+	return s
+}
+
+var EscapedNameTest = BindingTest{
+	name: "EscapedName",
+	structs: []interface{}{
+		&EscapedName{},
+	},
+	exemptions:  nil,
+	shouldError: false,
+	want: `
+export namespace binding_test {
+	export class EscapedName {
+		"n.a.m.e": string;
+		static createFrom(source: any = {}) {
+			return new EscapedName(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this["n.a.m.e"] = source["n.a.m.e"];
+		}
+	}
+}
+`,
+}

v2/internal/binding/binding_test/binding_importedmap_test.go

@@ -0,0 +1,94 @@
+package binding_test
+
+import "github.com/wailsapp/wails/v2/internal/binding/binding_test/binding_test_import"
+
+type ImportedMap struct {
+	AMapWrapperContainer binding_test_import.AMapWrapper `json:"AMapWrapperContainer"`
+}
+
+func (s ImportedMap) Get() ImportedMap {
+	return s
+}
+
+var ImportedMapTest = BindingTest{
+	name: "ImportedMap",
+	structs: []interface{}{
+		&ImportedMap{},
+	},
+	exemptions:  nil,
+	shouldError: false,
+	want: `
+export namespace binding_test {
+	export class ImportedMap {
+		AMapWrapperContainer: binding_test_import.AMapWrapper;
+		static createFrom(source: any = {}) {
+			return new ImportedMap(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.AMapWrapperContainer = this.convertValues(source["AMapWrapperContainer"], binding_test_import.AMapWrapper);
+		}
+		convertValues(a: any, classs: any, asMap: boolean = false): any {
+			if (!a) {
+				return a;
+			}
+			if (a.slice) {
+				return (a as any[]).map(elem => this.convertValues(elem, classs));
+			} else if ("object" === typeof a) {
+				if (asMap) {
+					for (const key of Object.keys(a)) {
+						a[key] = new classs(a[key]);
+					}
+					return a;
+				}
+				return new classs(a);
+			}
+			return a;
+		}
+	}
+}
+
+export namespace binding_test_import {
+	export class AMapWrapper {
+		AMap: {[key: string]: binding_test_nestedimport.A};
+		static createFrom(source: any = {}) {
+			return new AMapWrapper(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.AMap = this.convertValues(source["AMap"], binding_test_nestedimport.A, true);
+		}
+		convertValues(a: any, classs: any, asMap: boolean = false): any {
+			if (!a) {
+				return a;
+			}
+			if (a.slice) {
+				return (a as any[]).map(elem => this.convertValues(elem, classs));
+			} else if ("object" === typeof a) {
+				if (asMap) {
+					for (const key of Object.keys(a)) {
+						a[key] = new classs(a[key]);
+					}
+					return a;
+				}
+				return new classs(a);
+			}
+			return a;
+		}
+	}
+}
+
+export namespace binding_test_nestedimport {
+	export class A {
+		A: string;
+		static createFrom(source: any = {}) {
+			return new A(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.A = source["A"];
+		}
+	}
+}
+`,
+}

v2/internal/binding/binding_test/binding_importedslice_test.go

@@ -0,0 +1,94 @@
+package binding_test
+
+import "github.com/wailsapp/wails/v2/internal/binding/binding_test/binding_test_import"
+
+type ImportedSlice struct {
+	ASliceWrapperContainer binding_test_import.ASliceWrapper `json:"ASliceWrapperContainer"`
+}
+
+func (s ImportedSlice) Get() ImportedSlice {
+	return s
+}
+
+var ImportedSliceTest = BindingTest{
+	name: "ImportedSlice",
+	structs: []interface{}{
+		&ImportedSlice{},
+	},
+	exemptions:  nil,
+	shouldError: false,
+	want: `
+export namespace binding_test {
+	export class ImportedSlice {
+		ASliceWrapperContainer: binding_test_import.ASliceWrapper;
+		static createFrom(source: any = {}) {
+			return new ImportedSlice(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.ASliceWrapperContainer = this.convertValues(source["ASliceWrapperContainer"], binding_test_import.ASliceWrapper);
+		}
+		convertValues(a: any, classs: any, asMap: boolean = false): any {
+			if (!a) {
+				return a;
+			}
+			if (a.slice) {
+				return (a as any[]).map(elem => this.convertValues(elem, classs));
+			} else if ("object" === typeof a) {
+				if (asMap) {
+					for (const key of Object.keys(a)) {
+						a[key] = new classs(a[key]);
+					}
+					return a;
+				}
+				return new classs(a);
+			}
+			return a;
+		}
+	}
+}
+
+export namespace binding_test_import {
+	export class ASliceWrapper {
+		ASlice: binding_test_nestedimport.A[];
+		static createFrom(source: any = {}) {
+			return new ASliceWrapper(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.ASlice = this.convertValues(source["ASlice"], binding_test_nestedimport.A);
+		}
+		convertValues(a: any, classs: any, asMap: boolean = false): any {
+			if (!a) {
+				return a;
+			}
+			if (a.slice) {
+				return (a as any[]).map(elem => this.convertValues(elem, classs));
+			} else if ("object" === typeof a) {
+				if (asMap) {
+					for (const key of Object.keys(a)) {
+						a[key] = new classs(a[key]);
+					}
+					return a;
+				}
+				return new classs(a);
+			}
+			return a;
+		}
+	}
+}
+
+export namespace binding_test_nestedimport {
+	export class A {
+		A: string;
+		static createFrom(source: any = {}) {
+			return new A(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.A = source["A"];
+		}
+	}
+}
+`,
+}

v2/internal/binding/binding_test/binding_importedstruct_test.go

@@ -0,0 +1,95 @@
+package binding_test
+
+import "github.com/wailsapp/wails/v2/internal/binding/binding_test/binding_test_import"
+
+type ImportedStruct struct {
+	AWrapperContainer binding_test_import.AWrapper `json:"AWrapperContainer"`
+}
+
+func (s ImportedStruct) Get() ImportedStruct {
+	return s
+}
+
+var ImportedStructTest = BindingTest{
+	name: "ImportedStruct",
+	structs: []interface{}{
+		&ImportedStruct{},
+	},
+	exemptions:  nil,
+	shouldError: false,
+	want: `
+export namespace binding_test {
+	export class ImportedStruct {
+		AWrapperContainer: binding_test_import.AWrapper;
+		static createFrom(source: any = {}) {
+			return new ImportedStruct(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.AWrapperContainer = this.convertValues(source["AWrapperContainer"], binding_test_import.AWrapper);
+		}
+		convertValues(a: any, classs: any, asMap: boolean = false): any {
+			if (!a) {
+				return a;
+			}
+
+			if (a.slice) {
+				return (a as any[]).map(elem => this.convertValues(elem, classs));
+			} else if ("object" === typeof a) {
+				if (asMap) {
+					for (const key of Object.keys(a)) {
+						a[key] = new classs(a[key]);
+					}
+					return a;
+				}
+				return new classs(a);
+			}
+			return a;
+		}
+	}
+}
+
+export namespace binding_test_import {
+	export class AWrapper {
+		AWrapper: binding_test_nestedimport.A;
+		static createFrom(source: any = {}) {
+			return new AWrapper(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.AWrapper = this.convertValues(source["AWrapper"], binding_test_nestedimport.A);
+		}
+		convertValues(a: any, classs: any, asMap: boolean = false): any {
+			if (!a) {
+				return a;
+			}
+			if (a.slice) {
+				return (a as any[]).map(elem => this.convertValues(elem, classs));
+			} else if ("object" === typeof a) {
+				if (asMap) {
+					for (const key of Object.keys(a)) {
+						a[key] = new classs(a[key]);
+					}
+					return a;
+				}
+				return new classs(a);
+			}
+			return a;
+		}
+	}
+}
+
+export namespace binding_test_nestedimport {
+	export class A {
+		A: string;
+		static createFrom(source: any = {}) {
+			return new A(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.A = source["A"];
+		}
+	}
+}
+`,
+}

v2/internal/binding/binding_test/binding_nestedfield_test.go

@@ -0,0 +1,63 @@
+package binding_test
+
+type As struct {
+	B Bs `json:"b"`
+}
+
+type Bs struct {
+	Name string `json:"name"`
+}
+
+func (a As) Get() As {
+	return a
+}
+
+var NestedFieldTest = BindingTest{
+	name: "NestedField",
+	structs: []interface{}{
+		&As{},
+	},
+	exemptions:  nil,
+	shouldError: false,
+	want: `
+export namespace binding_test {
+	export class Bs {
+		name: string;
+		static createFrom(source: any = {}) {
+			return new Bs(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.name = source["name"];
+		}
+	}
+	export class As {
+		b: Bs;
+		static createFrom(source: any = {}) {
+			return new As(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.b = this.convertValues(source["b"], Bs);
+		}
+		convertValues(a: any, classs: any, asMap: boolean = false): any {
+			if (!a) {
+				return a;
+			}
+			if (a.slice) {
+				return (a as any[]).map(elem => this.convertValues(elem, classs));
+			} else if ("object" === typeof a) {
+				if (asMap) {
+					for (const key of Object.keys(a)) {
+						a[key] = new classs(a[key]);
+					}
+					return a;
+				}
+				return new classs(a);
+			}
+			return a;
+		}
+	}
+}
+`,
+}

v2/internal/binding/binding_test/binding_nonstringmapkey_test.go

@@ -0,0 +1,32 @@
+package binding_test
+
+type NonStringMapKey struct {
+	NumberMap map[uint]any `json:"numberMap"`
+}
+
+func (s NonStringMapKey) Get() NonStringMapKey {
+	return s
+}
+
+var NonStringMapKeyTest = BindingTest{
+	name: "NonStringMapKey",
+	structs: []interface{}{
+		&NonStringMapKey{},
+	},
+	exemptions:  nil,
+	shouldError: false,
+	want: `
+export namespace binding_test {
+	export class NonStringMapKey {
+		numberMap: {[key: number]: any};
+		static createFrom(source: any = {}) {
+			return new NonStringMapKey(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.numberMap = source["numberMap"];
+		}
+	}
+}
+`,
+}

v2/internal/binding/binding_test/binding_singlefield_test.go

@@ -0,0 +1,32 @@
+package binding_test
+
+type SingleField struct {
+	Name string `json:"name"`
+}
+
+func (s SingleField) Get() SingleField {
+	return s
+}
+
+var SingleFieldTest = BindingTest{
+	name: "SingleField",
+	structs: []interface{}{
+		&SingleField{},
+	},
+	exemptions:  nil,
+	shouldError: false,
+	want: `
+export namespace binding_test {
+	export class SingleField {
+		name: string;
+		static createFrom(source: any = {}) {
+			return new SingleField(source);
+		}
+		constructor(source: any = {}) {
+			if ('string' === typeof source) source = JSON.parse(source);
+			this.name = source["name"];
+		}
+	}
+}
+`,
+}

v2/internal/binding/binding_test/binding_test.go

@@ -0,0 +1,51 @@
+package binding_test
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/wailsapp/wails/v2/internal/binding"
+	"github.com/wailsapp/wails/v2/internal/logger"
+)
+
+type BindingTest struct {
+	name        string
+	structs     []interface{}
+	exemptions  []interface{}
+	want        string
+	shouldError bool
+}
+
+func TestBindings_GenerateModels(t *testing.T) {
+
+	tests := []BindingTest{
+		EscapedNameTest,
+		ImportedStructTest,
+		ImportedSliceTest,
+		ImportedMapTest,
+		NestedFieldTest,
+		NonStringMapKeyTest,
+		SingleFieldTest,
+	}
+
+	testLogger := &logger.Logger{}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b := binding.NewBindings(testLogger, tt.structs, tt.exemptions, false)
+			for _, s := range tt.structs {
+				err := b.Add(s)
+				require.NoError(t, err)
+			}
+			got, err := b.GenerateModels()
+			if (err != nil) != tt.shouldError {
+				t.Errorf("GenerateModels() error = %v, shouldError %v", err, tt.shouldError)
+				return
+			}
+			if !reflect.DeepEqual(strings.Fields(string(got)), strings.Fields(tt.want)) {
+				t.Errorf("GenerateModels() got = %v, want %v", string(got), tt.want)
+			}
+		})
+	}
+}

v2/internal/binding/binding_test/binding_test_import/binding_test_import.go

@@ -0,0 +1,15 @@
+package binding_test_import
+
+import "github.com/wailsapp/wails/v2/internal/binding/binding_test/binding_test_import/binding_test_nestedimport"
+
+type AWrapper struct {
+	AWrapper binding_test_nestedimport.A `json:"AWrapper"`
+}
+
+type ASliceWrapper struct {
+	ASlice []binding_test_nestedimport.A `json:"ASlice"`
+}
+
+type AMapWrapper struct {
+	AMap map[string]binding_test_nestedimport.A `json:"AMap"`
+}

v2/internal/binding/binding_test/binding_test_import/binding_test_nestedimport/binding_nestedimport.go

@@ -0,0 +1,5 @@
+package binding_test_nestedimport
+
+type A struct {
+	A string `json:"A"`
+}

v2/internal/binding/reflect.go

@@ -165,3 +165,8 @@ func getPackageName(in string) string {
 	result = strings.ReplaceAll(result, "*", "")
 	return result
 }
+
+func hasElements(typ reflect.Type) bool {
+	kind := typ.Kind()
+	return kind == reflect.Ptr || kind == reflect.Array || kind == reflect.Slice || kind == reflect.Map
+}