-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix miscellaneous bindings and typescript export bugs #3978
Changes from 9 commits
42c3e07
f9d053a
e8110bc
a4345e5
3fbb230
48a8268
82a4952
2d7ffaa
9e26774
75fc0f5
2813db4
bf6867f
c0f22a7
203abce
7ac3c8c
4e17993
676e6ed
0248bd8
c91b54e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -166,7 +166,7 @@ func getPackageName(in string) string { | |||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
func getSplitReturn(in string) (string, string) { | ||||||||||||||||||||||
result := strings.Split(in, ".") | ||||||||||||||||||||||
result := strings.SplitN(in, ".", 2) | ||||||||||||||||||||||
return result[0], result[1] | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+169
to
171
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add input validation to prevent panic The current implementation will panic if the input string doesn't contain a "." character. Consider adding validation: func getSplitReturn(in string) (string, string) {
result := strings.SplitN(in, ".", 2)
+ if len(result) != 2 {
+ return in, ""
+ }
return result[0], result[1]
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,20 @@ const ( | |
jsVariableNameRegex = `^([A-Z]|[a-z]|\$|_)([A-Z]|[a-z]|[0-9]|\$|_)*$` | ||
) | ||
|
||
var ( | ||
jsVariableUnsafeChars = regexp.MustCompile(`[^A-Za-z0-9_]`) | ||
) | ||
|
||
func nameTypeOf(typeOf reflect.Type) string { | ||
tname := typeOf.Name() | ||
gidx := strings.IndexRune(tname, '[') | ||
if gidx > 0 { // its a generic type | ||
rem := strings.SplitN(tname, "[", 2) | ||
tname = rem[0] + "_" + jsVariableUnsafeChars.ReplaceAllLiteralString(rem[1], "_") | ||
} | ||
return tname | ||
} | ||
|
||
// TypeOptions overrides options set by `ts_*` tags. | ||
type TypeOptions struct { | ||
TSType string | ||
|
@@ -261,15 +275,22 @@ func (t *TypeScriptify) AddType(typeOf reflect.Type) *TypeScriptify { | |
func (t *typeScriptClassBuilder) AddMapField(fieldName string, field reflect.StructField) { | ||
keyType := field.Type.Key() | ||
valueType := field.Type.Elem() | ||
valueTypeName := valueType.Name() | ||
valueTypeName := nameTypeOf(valueType) | ||
if valueType.Kind() == reflect.Ptr { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. matches Ptr unwrapping everywhere else in this file without this, a map value that is a pointer to a struct looses its namespace #3709 |
||
valueType = valueType.Elem() | ||
} | ||
if name, ok := t.types[valueType.Kind()]; ok { | ||
valueTypeName = name | ||
} | ||
if valueType.Kind() == reflect.Array || valueType.Kind() == reflect.Slice { | ||
valueTypeName = valueType.Elem().Name() + "[]" | ||
valueTypeName = nameTypeOf(valueType.Elem()) + "[]" | ||
} | ||
if valueType.Kind() == reflect.Ptr { | ||
valueTypeName = valueType.Elem().Name() | ||
valueTypeName = nameTypeOf(valueType.Elem()) | ||
} | ||
if valueType.Kind() == reflect.Map { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. previously there was no reflect.Map here, so valueTypeName defaulted to "" and caused invalid typescript with e.g. nested map[string]map[string]string. any here gives valid code at least, future work to recursively generate? |
||
// TODO: support nested maps | ||
valueTypeName = "any" // valueType.Elem().Name() | ||
} | ||
if valueType.Kind() == reflect.Struct && differentNamespaces(t.namespace, valueType) { | ||
valueTypeName = valueType.String() | ||
|
@@ -296,9 +317,11 @@ func (t *typeScriptClassBuilder) AddMapField(fieldName string, field reflect.Str | |
} | ||
t.fields = append(t.fields, fmt.Sprintf("%s%s: {[key: %s]: %s};", t.indent, fieldName, keyTypeStr, valueTypeName)) | ||
if valueType.Kind() == reflect.Struct { | ||
t.constructorBody = append(t.constructorBody, fmt.Sprintf("%s%sthis%s = this.convertValues(source[\"%s\"], %s, true);", t.indent, t.indent, dotField, strippedFieldName, t.prefix+valueTypeName+t.suffix)) | ||
t.constructorBody = append(t.constructorBody, fmt.Sprintf("%s%sthis%s = this.convertValues(source[\"%s\"], %s, true);", | ||
t.indent, t.indent, dotField, strippedFieldName, t.prefix+valueTypeName+t.suffix)) | ||
} else { | ||
t.constructorBody = append(t.constructorBody, fmt.Sprintf("%s%sthis%s = source[\"%s\"];", t.indent, t.indent, dotField, strippedFieldName)) | ||
t.constructorBody = append(t.constructorBody, fmt.Sprintf("%s%sthis%s = source[\"%s\"];", | ||
t.indent, t.indent, dotField, strippedFieldName)) | ||
} | ||
} | ||
|
||
|
@@ -501,7 +524,7 @@ func (t *TypeScriptify) convertEnum(depth int, typeOf reflect.Type, elements []e | |
} | ||
t.alreadyConverted[typeOf.String()] = true | ||
|
||
entityName := t.Prefix + typeOf.Name() + t.Suffix | ||
entityName := t.Prefix + nameTypeOf(typeOf) + t.Suffix | ||
result := "enum " + entityName + " {\n" | ||
|
||
for _, val := range elements { | ||
|
@@ -553,6 +576,14 @@ func (t *TypeScriptify) getFieldOptions(structType reflect.Type, field reflect.S | |
|
||
func (t *TypeScriptify) getJSONFieldName(field reflect.StructField, isPtr bool) string { | ||
jsonFieldName := "" | ||
// function, complex, and channel types cannot be json-encoded | ||
if field.Type.Kind() == reflect.Chan || | ||
field.Type.Kind() == reflect.Func || | ||
field.Type.Kind() == reflect.UnsafePointer || | ||
field.Type.Kind() == reflect.Complex128 || | ||
field.Type.Kind() == reflect.Complex64 { | ||
return "" | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM! Added essential JSON encoding safety checks. The changes prevent JSON serialization of types that cannot be properly encoded (channels, functions, complex numbers, etc.), improving type safety and preventing runtime errors. |
||
jsonTag, hasTag := field.Tag.Lookup("json") | ||
if !hasTag && field.IsExported() { | ||
jsonFieldName = field.Name | ||
|
@@ -599,7 +630,7 @@ func (t *TypeScriptify) convertType(depth int, typeOf reflect.Type, customCode m | |
|
||
t.alreadyConverted[typeOf.String()] = true | ||
|
||
entityName := t.Prefix + typeOf.Name() + t.Suffix | ||
entityName := t.Prefix + nameTypeOf(typeOf) + t.Suffix | ||
|
||
if typeClashWithReservedKeyword(entityName) { | ||
warnAboutTypesClash(entityName) | ||
|
@@ -659,8 +690,10 @@ func (t *TypeScriptify) convertType(depth int, typeOf reflect.Type, customCode m | |
} | ||
|
||
isKnownType := t.KnownStructs.Contains(getStructFQN(field.Type.String())) | ||
println("KnownStructs:", t.KnownStructs.Join("\t")) | ||
println(getStructFQN(field.Type.String())) | ||
if !isKnownType { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is very loud with a lot of structs. Quieting it unless it misses something |
||
println("KnownStructs:", t.KnownStructs.Join("\t")) | ||
println("Not found:", getStructFQN(field.Type.String())) | ||
} | ||
builder.AddStructField(jsonFieldName, field, !isKnownType) | ||
} else if field.Type.Kind() == reflect.Map { | ||
t.logf(depth, "- map field %s.%s", typeOf.Name(), field.Name) | ||
|
@@ -800,7 +833,8 @@ type typeScriptClassBuilder struct { | |
} | ||
|
||
func (t *typeScriptClassBuilder) AddSimpleArrayField(fieldName string, field reflect.StructField, arrayDepth int, opts TypeOptions) error { | ||
fieldType, kind := field.Type.Elem().Name(), field.Type.Elem().Kind() | ||
fieldType := nameTypeOf(field.Type.Elem()) | ||
kind := field.Type.Elem().Kind() | ||
typeScriptType := t.types[kind] | ||
|
||
if len(fieldName) > 0 { | ||
|
@@ -820,7 +854,8 @@ func (t *typeScriptClassBuilder) AddSimpleArrayField(fieldName string, field ref | |
} | ||
|
||
func (t *typeScriptClassBuilder) AddSimpleField(fieldName string, field reflect.StructField, opts TypeOptions) error { | ||
fieldType, kind := field.Type.Name(), field.Type.Kind() | ||
fieldType := nameTypeOf(field.Type) | ||
kind := field.Type.Kind() | ||
|
||
typeScriptType := t.types[kind] | ||
if len(opts.TSType) > 0 { | ||
|
@@ -844,7 +879,7 @@ func (t *typeScriptClassBuilder) AddSimpleField(fieldName string, field reflect. | |
} | ||
|
||
func (t *typeScriptClassBuilder) AddEnumField(fieldName string, field reflect.StructField) { | ||
fieldType := field.Type.Name() | ||
fieldType := nameTypeOf(field.Type) | ||
t.addField(fieldName, t.prefix+fieldType+t.suffix, false) | ||
strippedFieldName := strings.ReplaceAll(fieldName, "?", "") | ||
t.addInitializerFieldLine(strippedFieldName, fmt.Sprintf("source[\"%s\"]", strippedFieldName)) | ||
|
@@ -854,7 +889,7 @@ func (t *typeScriptClassBuilder) AddStructField(fieldName string, field reflect. | |
strippedFieldName := strings.ReplaceAll(fieldName, "?", "") | ||
classname := "null" | ||
namespace := strings.Split(field.Type.String(), ".")[0] | ||
fqname := t.prefix + field.Type.Name() + t.suffix | ||
fqname := t.prefix + nameTypeOf(field.Type) + t.suffix | ||
if namespace != t.namespace { | ||
fqname = namespace + "." + fqname | ||
} | ||
|
@@ -873,7 +908,7 @@ func (t *typeScriptClassBuilder) AddStructField(fieldName string, field reflect. | |
} | ||
|
||
func (t *typeScriptClassBuilder) AddArrayOfStructsField(fieldName string, field reflect.StructField, arrayDepth int) { | ||
fieldType := field.Type.Elem().Name() | ||
fieldType := nameTypeOf(field.Type.Elem()) | ||
if differentNamespaces(t.namespace, field.Type.Elem()) { | ||
fieldType = field.Type.Elem().String() | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 284 throws away the rest, we want to keep them