Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle field name overlaps #11262

Merged
merged 2 commits into from Nov 7, 2022
+408 −35
Merged

Handle field name overlaps #11262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5142d94
Handle field name overlaps
iwahbe Nov 4, 2022
4abde4c
chore: fixup implicit optionals for mypy
kpitzen Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/pending/20221104--sdkgen-go--guard-against-conflicting-field-names.yaml
View file
Open in desktop
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: sdkgen/go
description: Guard against conflicting field names.
65 changes: 35 additions & 30 deletions pkg/codegen/go/gen.go
View file
Open in desktop
Expand Up @@ -1295,24 +1295,29 @@ func (pkg *pkgContext) assignProperty(w io.Writer, p *schema.Property, object, v
if t != "" {
value = fmt.Sprintf("%s(%s)", t, value)
}
fmt.Fprintf(w, "\t%s.%s = %s\n", object, Title(p.Name), value)
fmt.Fprintf(w, "\t%s.%s = %s\n", object, pkg.fieldName(nil, p), value)
} else if indirectAssign {
tmpName := cgstrings.Camel(p.Name) + "_"
fmt.Fprintf(w, "%s := %s\n", tmpName, value)
fmt.Fprintf(w, "%s.%s = &%s\n", object, Title(p.Name), tmpName)
fmt.Fprintf(w, "%s.%s = &%s\n", object, pkg.fieldName(nil, p), tmpName)
} else {
fmt.Fprintf(w, "%s.%s = %s\n", object, Title(p.Name), value)
fmt.Fprintf(w, "%s.%s = %s\n", object, pkg.fieldName(nil, p), value)
}
}

func (pkg *pkgContext) fieldName(r *schema.Resource, field *schema.Property) string {
contract.Assert(field != nil)
return fieldName(pkg, r, field)
}

func (pkg *pkgContext) genPlainType(w io.Writer, name, comment, deprecationMessage string,
properties []*schema.Property) {

printCommentWithDeprecationMessage(w, comment, deprecationMessage, false)
fmt.Fprintf(w, "type %s struct {\n", name)
for _, p := range properties {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", Title(p.Name), pkg.typeString(codegen.ResolvedType(p.Type)), p.Name)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", pkg.fieldName(nil, p), pkg.typeString(codegen.ResolvedType(p.Type)), p.Name)
}
fmt.Fprintf(w, "}\n\n")
}
Expand Down Expand Up @@ -1342,16 +1347,16 @@ func (pkg *pkgContext) genObjectDefaultFunc(w io.Writer, name string,
return err
}
pkg.needsUtils = true
fmt.Fprintf(w, "if isZero(tmp.%s) {\n", Title(p.Name))
fmt.Fprintf(w, "if isZero(tmp.%s) {\n", pkg.fieldName(nil, p))
pkg.assignProperty(w, p, "tmp", dv, !p.IsRequired())
fmt.Fprintf(w, "}\n")
} else if funcName := pkg.provideDefaultsFuncName(p.Type); funcName != "" {
var member string
if codegen.IsNOptionalInput(p.Type) {
// f := fmt.Sprintf("func(v %[1]s) %[1]s { return *v.%[2]s() }", name, funcName)
// member = fmt.Sprintf("tmp.%[1]s.ApplyT(%[2]s)", Title(p.Name), f)
// member = fmt.Sprintf("tmp.%[1]s.ApplyT(%[2]s)", pkg.fieldName(nil, p), f)
} else {
member = fmt.Sprintf("tmp.%[1]s.%[2]s()", Title(p.Name), funcName)
member = fmt.Sprintf("tmp.%[1]s.%[2]s()", pkg.fieldName(nil, p), funcName)
sigil := ""
if p.IsRequired() {
sigil = "*"
Expand Down Expand Up @@ -1441,7 +1446,7 @@ func (pkg *pkgContext) genInputArgsStruct(w io.Writer, typeName string, t *schem
fmt.Fprintf(w, "type %s struct {\n", typeName)
for _, p := range t.Properties {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", Title(p.Name), pkg.typeString(p.Type), p.Name)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", pkg.fieldName(nil, p), pkg.typeString(p.Type), p.Name)
}
fmt.Fprintf(w, "}\n\n")
}
Expand Down Expand Up @@ -1476,14 +1481,14 @@ func (pkg *pkgContext) genOutputTypes(w io.Writer, genArgs genOutputTypesArgs) {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, false)
outputType, applyType := pkg.outputType(p.Type), pkg.typeString(p.Type)

propName := Title(p.Name)
propName := pkg.fieldName(nil, p)
switch strings.ToLower(p.Name) {
case "elementtype", "issecret":
propName = "Get" + propName
}
fmt.Fprintf(w, "func (o %sOutput) %s() %s {\n", name, propName, outputType)
fmt.Fprintf(w, "\treturn o.ApplyT(func (v %s) %s { return v.%s }).(%s)\n",
name, applyType, Title(p.Name), outputType)
name, applyType, pkg.fieldName(nil, p), outputType)
fmt.Fprintf(w, "}\n\n")
}
}
Expand Down Expand Up @@ -1515,7 +1520,7 @@ func (pkg *pkgContext) genOutputTypes(w io.Writer, genArgs genOutputTypesArgs) {
fmt.Fprintf(w, "\t\tif v == nil {\n")
fmt.Fprintf(w, "\t\t\treturn nil\n")
fmt.Fprintf(w, "\t\t}\n")
fmt.Fprintf(w, "\t\treturn %sv.%s\n", deref, Title(p.Name))
fmt.Fprintf(w, "\t\treturn %sv.%s\n", deref, pkg.fieldName(nil, p))
fmt.Fprintf(w, "\t}).(%s)\n", outputType)
fmt.Fprintf(w, "}\n\n")
}
Expand Down Expand Up @@ -1638,7 +1643,7 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso

for _, p := range r.Properties {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", Title(p.Name), pkg.outputType(p.Type), p.Name)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", pkg.fieldName(r, p), pkg.outputType(p.Type), p.Name)

if p.Secret {
secretProps = append(secretProps, p)
Expand Down Expand Up @@ -1673,8 +1678,8 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
// Check all required inputs are present
for _, p := range r.InputProperties {
if p.IsRequired() && isNilType(p.Type) && p.DefaultValue == nil {
fmt.Fprintf(w, "\tif args.%s == nil {\n", Title(p.Name))
fmt.Fprintf(w, "\t\treturn nil, errors.New(\"invalid value for required argument '%s'\")\n", Title(p.Name))
fmt.Fprintf(w, "\tif args.%s == nil {\n", pkg.fieldName(r, p))
fmt.Fprintf(w, "\t\treturn nil, errors.New(\"invalid value for required argument '%s'\")\n", pkg.fieldName(r, p))
fmt.Fprintf(w, "\t}\n")
}

Expand All @@ -1700,7 +1705,7 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
return err
}
pkg.needsUtils = true
fmt.Fprintf(w, "\tif isZero(args.%s) {\n", Title(p.Name))
fmt.Fprintf(w, "\tif isZero(args.%s) {\n", pkg.fieldName(r, p))
assign(p, dv)
fmt.Fprintf(w, "\t}\n")
} else if name := pkg.provideDefaultsFuncName(p.Type); name != "" && !pkg.disableObjectDefaults {
Expand All @@ -1712,19 +1717,19 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
toOutputMethod := pkg.toOutputMethod(p.Type)
outputType := pkg.outputType(p.Type)
resolvedType := pkg.typeString(codegen.ResolvedType(p.Type))
originalValue := fmt.Sprintf("args.%s.%s()", Title(p.Name), toOutputMethod)
originalValue := fmt.Sprintf("args.%s.%s()", pkg.fieldName(r, p), toOutputMethod)
valueWithDefaults := fmt.Sprintf("%[1]v.ApplyT(func (v %[2]s) %[2]s { return %[3]sv.%[4]s() }).(%[5]s)",
originalValue, resolvedType, optionalDeref, name, outputType)
if p.Plain {
valueWithDefaults = fmt.Sprintf("args.%v.Defaults()", Title(p.Name))
valueWithDefaults = fmt.Sprintf("args.%v.Defaults()", pkg.fieldName(r, p))
}

if !p.IsRequired() {
fmt.Fprintf(w, "if args.%s != nil {\n", Title(p.Name))
fmt.Fprintf(w, "args.%[1]s = %s\n", Title(p.Name), valueWithDefaults)
fmt.Fprintf(w, "if args.%s != nil {\n", pkg.fieldName(r, p))
fmt.Fprintf(w, "args.%[1]s = %s\n", pkg.fieldName(r, p), valueWithDefaults)
fmt.Fprint(w, "}\n")
} else {
fmt.Fprintf(w, "args.%[1]s = %s\n", Title(p.Name), valueWithDefaults)
fmt.Fprintf(w, "args.%[1]s = %s\n", pkg.fieldName(r, p), valueWithDefaults)
}

}
Expand Down Expand Up @@ -1753,8 +1758,8 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso

// Setup secrets
for _, p := range secretInputProps {
fmt.Fprintf(w, "\tif args.%s != nil {\n", Title(p.Name))
fmt.Fprintf(w, "\t\targs.%[1]s = pulumi.ToSecret(args.%[1]s).(%[2]s)\n", Title(p.Name), pkg.outputType(p.Type))
fmt.Fprintf(w, "\tif args.%s != nil {\n", pkg.fieldName(r, p))
fmt.Fprintf(w, "\t\targs.%[1]s = pulumi.ToSecret(args.%[1]s).(%[2]s)\n", pkg.fieldName(r, p), pkg.outputType(p.Type))
fmt.Fprintf(w, "\t}\n")
}
if len(secretProps) > 0 {
Expand Down Expand Up @@ -1817,7 +1822,7 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
if r.StateInputs != nil {
for _, p := range r.StateInputs.Properties {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", Title(p.Name), pkg.typeString(codegen.ResolvedType(codegen.OptionalType(p))), p.Name)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", pkg.fieldName(r, p), pkg.typeString(codegen.ResolvedType(codegen.OptionalType(p))), p.Name)
}
}
fmt.Fprintf(w, "}\n\n")
Expand All @@ -1826,7 +1831,7 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
if r.StateInputs != nil {
for _, p := range r.StateInputs.Properties {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s\n", Title(p.Name), pkg.inputType(p.Type))
fmt.Fprintf(w, "\t%s %s\n", pkg.fieldName(r, p), pkg.inputType(p.Type))
}
}
fmt.Fprintf(w, "}\n\n")
Expand All @@ -1840,7 +1845,7 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
fmt.Fprintf(w, "type %sArgs struct {\n", cgstrings.Camel(name))
for _, p := range r.InputProperties {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", Title(p.Name), pkg.typeString(codegen.ResolvedType(p.Type)), p.Name)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", pkg.fieldName(r, p), pkg.typeString(codegen.ResolvedType(p.Type)), p.Name)
}
fmt.Fprintf(w, "}\n\n")

Expand All @@ -1858,7 +1863,7 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
}

printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s\n", Title(p.Name), pkg.typeString(typ))
fmt.Fprintf(w, "\t%s %s\n", pkg.fieldName(r, p), pkg.typeString(typ))
}
fmt.Fprintf(w, "}\n\n")

Expand Down Expand Up @@ -1948,7 +1953,7 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
fmt.Fprintf(w, "type %s%sArgs struct {\n", cgstrings.Camel(name), methodName)
for _, p := range args {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", Title(p.Name), pkg.typeString(codegen.ResolvedType(p.Type)),
fmt.Fprintf(w, "\t%s %s `pulumi:\"%s\"`\n", pkg.fieldName(nil, p), pkg.typeString(codegen.ResolvedType(p.Type)),
p.Name)
}
fmt.Fprintf(w, "}\n\n")
Expand All @@ -1957,7 +1962,7 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
fmt.Fprintf(w, "type %s%sArgs struct {\n", name, methodName)
for _, p := range args {
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, true)
fmt.Fprintf(w, "\t%s %s\n", Title(p.Name), pkg.typeString(p.Type))
fmt.Fprintf(w, "\t%s %s\n", pkg.fieldName(nil, p), pkg.typeString(p.Type))
}
fmt.Fprintf(w, "}\n\n")

Expand Down Expand Up @@ -2026,14 +2031,14 @@ func (pkg *pkgContext) genResource(w io.Writer, r *schema.Resource, generateReso
printCommentWithDeprecationMessage(w, p.Comment, p.DeprecationMessage, false)
outputType := pkg.outputType(p.Type)

propName := Title(p.Name)
propName := pkg.fieldName(r, p)
switch strings.ToLower(p.Name) {
case "elementtype", "issecret":
propName = "Get" + propName
}
fmt.Fprintf(w, "func (o %sOutput) %s() %s {\n", name, propName, outputType)
fmt.Fprintf(w, "\treturn o.ApplyT(func (v *%s) %s { return v.%s }).(%s)\n",
name, outputType, Title(p.Name), outputType)
name, outputType, pkg.fieldName(r, p), outputType)
fmt.Fprintf(w, "}\n\n")
}

Expand Down
33 changes: 33 additions & 0 deletions pkg/codegen/go/utilities.go
View file
Open in desktop
Expand Up @@ -22,6 +22,8 @@ import (

"github.com/pulumi/pulumi/pkg/v3/codegen"
"github.com/pulumi/pulumi/pkg/v3/codegen/cgstrings"
"github.com/pulumi/pulumi/pkg/v3/codegen/schema"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
)

// isReservedWord returns true if s is a Go reserved word as per
Expand All @@ -40,6 +42,21 @@ func isReservedWord(s string) bool {
}
}

// isReservedResourceField returns true if s would conflict with a method on a generated
// resource.
func isReservedResourceField(resourceName, s string) bool {
switch s {
case "ID", "URN", "GetProvider", "ElementType":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any Go reserved words we should be checking here as well?

return true
default:
if resourceName != "" {
toOutput := "To" + resourceName + "Output"
return s == toOutput || s == toOutput+"WithContext"
}
return false
}
}

// isLegalIdentifierStart returns true if it is legal for c to be the first character of a Go identifier as per
// https://golang.org/ref/spec#Identifiers
func isLegalIdentifierStart(c rune) bool {
Expand Down Expand Up @@ -120,3 +137,19 @@ func enumTitle(s string) string {
return "_" + cgstrings.UppercaseFirst(next)
})
}

// Calculate the name of a field in a resource
func fieldName(pkg *pkgContext, r *schema.Resource, p *schema.Property) string {
s := Title(p.Name)
var name string
if r != nil {
name = disambiguatedResourceName(r, pkg)
}
if !isReservedResourceField(name, s) {
return s
}

res := s + "_"
contract.Assert(!isReservedResourceField(name, res))
return res
}
3 changes: 3 additions & 0 deletions pkg/codegen/testing/test/testdata/go-nested-collections/go/codegen-manifest.json
View file
Open in desktop
@@ -1,6 +1,9 @@
{
"emittedFiles": [
"repro/doc.go",
"repro/elementtype/elementType.go",
"repro/elementtype/init.go",
"repro/elementtype/pulumiTypes.go",
"repro/foo.go",
"repro/init.go",
"repro/overlap/consumer.go",
Expand Down