Failure in code generation if nested attribute defines name with reserved Go keyword

[AgustinBettati]

Provider Spec definition

{
    "provider": {
        "name": "provider"
    },
    "resources": [
        {
            "name": "resource",
            "schema": {
                "attributes": [
                    {
                        "name": "scopes",
                        "list_nested": {
                            "computed_optional_required": "computed_optional",
                            "nested_object": {
                                "attributes": [
                                    {
                                        "name": "type",
                                        "string": {
                                            "computed_optional_required": "required"
                                        }
                                    }
                                ]
                            }
                        }
                    }
                ]
            }
        }
    ],
    "version": "0.1"
}

Output of generate command

tfplugingen-framework generate resources --input ./provider-spec.json

2023/10/23 15:11:41 26:5: expected 'IDENT', found ',' (and 5 more errors)

Issue can be reproduced using any reserved Go keyword (break, case, chan, const, continue, etc) as name of a nested object attribute.

[bendbennett]

Hi @AgustinBettati 👋

Thank you for reporting this, and sorry you ran into trouble here.

The bug you reported arises because the nested attribute names are used in the generated code as variable names.

Using the spec definition that you supplied results in the following generated code:

func (t ScopesType) ValueFromObject(ctx context.Context, in basetypes.ObjectValue) (basetypes.ObjectValuable, diag.Diagnostics) {
        /* ... */

	type, ok := attributes["type"]

	if !ok {
		diags.AddError(
			"Attribute Missing",
			`type is missing from object`)

		return nil, diags
	}

	typeVal, ok := type.(basetypes.StringValue)

	if !ok {
		diags.AddError(
			"Attribute Wrong Type",
			fmt.Sprintf(`type expected to be basetypes.StringValue, was: %T`, type))
	}
 
        /* ... */

func NewScopesValue(attributeTypes map[string]attr.Type, attributes map[string]attr.Value) (ScopesValue, diag.Diagnostics) {
        /* ... */

	type, ok := attributes["type"]

	if !ok {
		diags.AddError(
			"Attribute Missing",
			`type is missing from object`)

		return NewScopesValueUnknown(), diags
	}

	typeVal, ok := type.(basetypes.StringValue)

	if !ok {
		diags.AddError(
			"Attribute Wrong Type",
			fmt.Sprintf(`type expected to be basetypes.StringValue, was: %T`, type))
	}

        /* ... */

Adding a suffix to the attribute names in the generated code would allow for reserved Go keywords to be used as attribute names within nested attributes. For example type => typeAttribute.

I've opened a PR to address this. The generated code, now appears as:

func (t ScopesType) ValueFromObject(ctx context.Context, in basetypes.ObjectValue) (basetypes.ObjectValuable, diag.Diagnostics) {
        /* ... */

	typeAttribute, ok := attributes["type"]

	if !ok {
		diags.AddError(
			"Attribute Missing",
			`type is missing from object`)

		return nil, diags
	}

	typeVal, ok := typeAttribute.(basetypes.StringValue)

	if !ok {
		diags.AddError(
			"Attribute Wrong Type",
			fmt.Sprintf(`type expected to be basetypes.StringValue, was: %T`, typeAttribute))
	}

        /* ... */

func NewScopesValue(attributeTypes map[string]attr.Type, attributes map[string]attr.Value) (ScopesValue, diag.Diagnostics) {
        /* ... */
	
	typeAttribute, ok := attributes["type"]

	if !ok {
		diags.AddError(
			"Attribute Missing",
			`type is missing from object`)

		return NewScopesValueUnknown(), diags
	}

	typeVal, ok := typeAttribute.(basetypes.StringValue)

	if !ok {
		diags.AddError(
			"Attribute Wrong Type",
			fmt.Sprintf(`type expected to be basetypes.StringValue, was: %T`, typeAttribute))
	}

        /* ... */

[bendbennett]

@AgustinBettati the fix should be available in release v0.2.0.

[AgustinBettati]

Hi @bendbennett. Thanks for the quick response and release.

While this fix has improved the generation, I am still facing an issue particularly with the case of a nested attribute named "type" (original example in issue).

Error encountered: field and method with the same name Type.

The method func (v ScopesValue) Type(ctx context.Context) attr.Type has the same name as one of the attributes in

type ScopesValue struct {
	Name  basetypes.StringValue `tfsdk:"name"`
	Type  basetypes.StringValue `tfsdk:"type"`
	state attr.ValueState
}

We might need some particular handling for attribute names that have the same name as autogenerated Go functions associated to the nested type.

[bendbennett]

Hi @AgustinBettati

Apologies for the oversight.

I'm wondering whether, given that the generated code methods for custom value types have to implement the attr.Value interface, one option would be to introduce rules whereby nested attributes that use reserved Go keywords as their name (e.g., type) have these names manipulated in the generated code.

For example, we have the following in the generated code currently:

func (v ScopesValue) Type(ctx context.Context) attr.Type {
    ....

type ScopesValue struct {
	Type  basetypes.StringValue `tfsdk:"type"`
    ....

We could manipulate the name in the generated code so we have something like the following, where the name of the attribute that is using a reserved Go keyword as its name prefixes the name with the name of the attribute within which it is nested:

type ScopesValue struct {
	ScopesType  basetypes.StringValue `tfsdk:"type"`
    ....

I'll discuss this with the team and get back to you.

[AgustinBettati]

@bendbennett Given that we only need to handle this scenario for a limited set of names defined in attr.Value interface your proposal sounds reasonable. Appreciate the quick response.

[bendbennett]

@AgustinBettati

I've opened a PR which should address the issue of conflicts between nested attribute names and the names of generated custom value methods.

The approach taken is to inspect the attribute name, and determine whether it would conflict with any of the names of the methods that are generated for the custom value type. If there is a conflict, then the name used for the attribute within the generated code is prefixed with the name of the "parent" nested attribute.

For example, the spec definition that you supplied currently generates the following output which is problematic because both a struct field and a method on the struct have the same name, in this case Type:

/* ... */

type ScopesValue struct {
	Type  basetypes.StringValue `tfsdk:"type"`
	state attr.ValueState
}

/* ... */

func (v ScopesValue) Type(ctx context.Context) attr.Type {
	return ScopesType{
		basetypes.ObjectType{
			AttrTypes: v.AttributeTypes(ctx),
		},
	}
}

Code generation has been been modified so that, given a "parent" nested attribute with the name scopes, the output has been changed as follows:

/* ... */

type ScopesValue struct {
	ScopesType basetypes.StringValue `tfsdk:"type"`
	state      attr.ValueState
}

/* ... */

func (t ScopesType) ValueFromObject(ctx context.Context, in basetypes.ObjectValue) (basetypes.ObjectValuable, diag.Diagnostics) {
        /* ... */
	return ScopesValue{
		ScopesType: typeVal,
		state:      attr.ValueStateKnown,
	}, diags
}

func NewScopesValue(attributeTypes map[string]attr.Type, attributes map[string]attr.Value) (ScopesValue, diag.Diagnostics) {
        /* ... */
	return ScopesValue{
		ScopesType: typeVal,
		state:      attr.ValueStateKnown,
	}, diags
}

/* ... */

func (v ScopesValue) ToTerraformValue(ctx context.Context) (tftypes.Value, error) {
                /* ... */
		val, err = v.ScopesType.ToTerraformValue(ctx)
                /* ... */
}

/* ... */

func (v ScopesValue) ToObjectValue(ctx context.Context) (basetypes.ObjectValue, diag.Diagnostics) {
                /* ... */
		map[string]attr.Value{
			"type": v.ScopesType,
		})
                /* ... */
}

/* ... */

func (v ScopesValue) Equal(o attr.Value) bool {
        /* ... */
	if !v.ScopesType.Equal(other.ScopesType) {
		return false
	}
        /* ... */
}

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.