hclwrite: Add RenameAttribute on Body

[raymyers]

Adding the ability to change the name of an attribute already set on a block body. This can be used to automate renaming of terraform locals.

Without this, the only obvious approach is to remove the current attribute and add a new one, but this loses the order and comments which isn't desirable.

Usage

attr := block.Body().GetAttribute(fromName)
if attr != nil {
  attr.SetName(toName)
}

UPDATE

wasChanged := block.Body().RenameAttribute(fromName, toName)

[hashicorp-cla]

All committers have signed the CLA.

[apparentlymart]

Hi @raymyers! Thanks for working on this.

I agree that this seems like a valuable thing for hclwrite to be able to represent! My lone concern about it is that so far we intentionally put all of the attribute-manipulation methods up on Body rather than on individual Attribute so that we could maintain the required invariant that attribute names always be unique within a particular body. For example, SetAttribute will either update the value of an existing attribute in-place or append a new one, depending on whether the given name is already in use.

A hypothetical block.Body.RenameAttribute(fromName, toName) could in principle return a bool to indicate whether it succeeded or not and return false if the toName is already in use. That's a slightly-less-intuitive API design, but I think it would be a drop-in replacement for your current approach if you already know that toName isn't conflicted. I assume you'd already be checking that the given name isn't conflicted within Terraform's local value namespace, which requires that the names be unique across the entire module and therefore implies uniqueness within a particular locals block too.

What do you think? :thinking:

[raymyers]

@apparentlymart

A hypothetical block.Body.RenameAttribute(fromName, toName) could in principle return a bool to indicate whether it succeeded or not and return false if the toName is already in use.

This makes sense, I've renamed SetName to private setName and added a RenameAttribute() on body that preserves the invariant and returns a bool as you suggested. I considered making it an error, but that seemed less consistent with the rest of the API - open to either.

Thanks for reviewing!

[raymyers]

Looks like CI has some linux-only failures in TestTokenGenerateConsistency now which didn't show up locally on my OSX and seem unrelated to the change. Is this a known issue?

[raymyers]

@apparentlymart Would you be able to approve the workflow to run the build?

[minamijoyo]

Hi @raymyers @apparentlymart, Thank you for working on this!

Is there anything I can help move this forward? I have another use case. Upgrading Terraform AWS provider v3 to v4 requires renaming some attributes. I’m currently removing an old attribute and adding a new one, which causes a loss of comments. https://github.com/minamijoyo/tfedit/issues/49