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.

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

hclwrite: Add block label quoting to format #390

Closed
wants to merge 1 commit into from

Conversation

alisdair
Copy link
Contributor

Blocks consist of an identifier, followed by zero or more labels, and a body. Each label can be either an identifier or a quoted string literal.

This commit updates hclwrite's format logic to replace any identifier labels with quoted strings. That is, for this input:

resource null_resource test { }

hclwrite will prduce the following output:

resource "null_resource" "test" { }

Blocks consist of an identifier, followed by zero or more labels, and a
body. Each label can be either an identifier or a quoted string literal.

This commit updates hclwrite's format logic to replace any identifier
labels with quoted strings. That is, for this input:

  resource null_resource test { }

hclwrite will prduce the following output:

  resource "null_resource" "test" { }
@alisdair alisdair requested a review from a team June 16, 2020 20:25
Copy link
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this, @alisdair!

I feel a bit nervous about the idea of doing this purely with token manipulation... I worry that there may be edge cases where it might apply quoting to something that isn't a label just because it happens to match this token-based heuristic.

I had originally imagined having applications like Terraform implement this transformation using the syntax-tree-based API rather than using Format, with Format being primarily concerned with whitespace fiddling.

The pull request #340 contains a building block that could potentially be used for this -- the caller could potentially call block.SetLabels(block.Labels()) and have HCL rewrite them in canonical form. Of course, ideally there'd be a higher-level API wrapped around that for formatting purposes!

I've not been able to dig into that PR properly yet because I've been focused on the Terraform 0.13 work, but I'd like to take a look at it soon. What do you think about leaning on the label-updating functionality added in that PR as a different way to get this done, with terraform fmt then being rewritten to use the full hclwrite API to do its work rather than just calling hclwrite.Format? (I'd hesitated to do that at first because hclwrite had not been very battle-tested, but between my terraform-clean-syntax side-project and the Terraform 0.13 upgrade tool I expect we'll soon have greater confidence in it.)

@alisdair
Copy link
Contributor Author

Ah, I hadn't even considered that approach! Yes, that seems much less likely to be a disaster. Closing this and I'll try to help out with #340 when I can.

@alisdair alisdair closed this Jun 29, 2020
@alisdair alisdair deleted the format-quote-labels branch June 29, 2020 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants