0.14.0-rc regression: opinionated formatting creates syntax errors with parentheses

[aaronsteers]

Terraform Version

Terraform v0.14.0-rc1
+ provider registry.terraform.io/hashicorp/archive v2.0.0
+ provider registry.terraform.io/hashicorp/aws v3.17.0
+ provider registry.terraform.io/hashicorp/http v2.0.0
+ provider registry.terraform.io/hashicorp/local v2.0.0
+ provider registry.terraform.io/hashicorp/null v3.0.0
+ provider registry.terraform.io/hashicorp/random v3.0.0```

Terraform Configuration Files

locals {
  value_a = ([
    [1,2,3],
    [1]
  ])
  value_b = 4
}

Actual Behavior

Code is formatted into uncompilable code which will subsequently fail basic code validation. Specifically this seems to occur when code has surrounding parenthesis. Those parenthesis are likely not necessary but they otherwise would not block compilation until after auto-format is applied.

The below output is generated from the above sample. Note the paren incorrectly placed ahead and inline with the next statement.

locals {
  value_a = ([
    [1, 2, 3],
    [1]
    ]
  ) value_b = 4
}

Expected Behavior

Applying terraform fmt should not create syntax errors or otherwise create code that then would fail validation.

One or (ideally) both of the following should occur:
1 - In general, terraform fmt would render everything as expected, without error.
2 - Adopting a pessimistic approach to (1), terraform fmt should attempt a compile check post-operation - and the format operation should fail if code cannot compile. This check already is performed beforehand and terraform fmt will abort if code cannot be initially compiled. Ideally, the same check would occur afterwards which already is performed beforehand.

Steps to Reproduce

1. Apply terraform fmt to code containing parenthetical groupings such as in the example above.

Additional Context

Also documented here in Discuss thread: https://discuss.hashicorp.com/t/terraform-v0-14-0-rc1-released/15752/15

[alisdair]

Hi @aaronsteers, thanks for reporting this. I've confirmed the issue on 0.14.0-rc1.

[aaronsteers]

@alisdair - thanks so much!!

[aaronsteers]

One thing I just realized here - since in most (maybe all?) instances where I found this the parenthesis were for readability and not necessary required, it might be a reasonable fix for the formatter to simply remove the parentheses. This would be similar behavior to what the python "black" formatter does with extraneous parenthetical groups.

[apparentlymart]

I expect (but have not yet confirmed) that this bug is in the hclwrite package in the HCL module, with that final newline somehow ending up being incorporated into the expression and thus being removed. Either way, I expect we'd want to start here by fixing the immediate problem (it's making the configuration invalid by removing a significant newline) and save further adjustments that it might otherwise make for a separate change.

[aaronsteers]

@apparentlymart - Agreed 💯 - the immediate fix should just be if we can resolve the newline compile error.

I don't know anything about this module but I did some digging and found this issue tagged to hclwrite: hashicorp/hcl#402

Seems like same root cause. I've cross-linked just in case they are same/related. I see also there's a PR linked there on the thread. I'm unsure though if it is a full/correct fix, but it looks like there's some discussion on this there on the above thread.

[apparentlymart]

The change in hashicorp/hcl#426 should address this, once it's included in an HCL release and we've upgraded our dependency here in Terraform.

[wyardley]

Another example of something that seems to be affected by this would be something like:
https://github.com/forseti-security/terraform-google-forseti/blob/524e684a4bac0b3af9ed8e3035bcb12a992e3d1b/modules/server_config/main.tf#L35

Is there any workaround? Since we enforce terraform fmt in CI, and have 2 files affected by this in our codebase, I'm thinking this means we can't update to 0.14 until a patched version is released?

Based on the above, I think for a simple use case like:
count = (var.xyz == "" ? 1 : 0)

it would probably work (and be preferable) to just get rid of the parens entirely, right?

[aaronsteers]

@wyardley - In your very simple example (and also in the example you linked to), yes, I believe removal of the outer parenthesis is a good fix.

The guideline I've started using for myself is to not use parenthesis unless I am trying to force order of operations, or unless the expression covers multiple lines and the parenthesis are needed for logical grouping. (I'm still not entirely sure how to properly deal with multi-line expressions in hcl.)

I'm curious if others have best practice guidelines for when to use parenthesis - I haven't found a lot of documentation on this topic, tbh.

[apparentlymart]

It is a valid thing to use parentheses solely to allow an expression to wrap over multiple lines, but of course that's the very situation that this bug affects so for now you'll either need to hold off for 0.14.1 or not wrap your expressions over multiple lines.

There are some other ways to get into the mode where the parser will accept newlines -- any sort of brackets will do it -- but the result will look weird so I'd probably not do it just to avoid the confusion it might cause if left that way for too long. For example:

  example = [
    # any single expression here, to make a single-element list
  ][0] # then take the first element of the list

[aaronsteers]

@apparentlymart. That's very helpful; thank you!

For our purposes, and in case it helps anyone else, we've decided to move forward using 0.14.0 with the following mitigations: (1) we've simply remove parenthesis in simple examples when not needed, and (2) for more complex expressions, we either add workarounds like the above or just refrain from auto-formatting those files.

It turns out most of our cases fit into the first category, and only a small handful strictly do require parenthesis as multi-line expressions.

[ghost]

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.