Add support for tflint-ignore-file annotations in JSON

[isobit]

I have a use case where I'm generating .tf.json files for a bunch of different root modules; these .tf.json files define several locals for convenience, which may or may not be used by the root modules they're placed in. I wanted to be able to ignore the terraform_unused_declarations rule without disabling it globally, but I realized that there wasn't a way to do this since the annotation system didn't support JSON files at all. This change adds limited support for tflint-ignore-file annotations in a top-level comment property.

[bendrucker]

Are there any other examples you can offer of this map comment structure being used? The documentation illustrates comment properties with string values. In that sense, this proposal clashes with the official guidance, as a JSON document that already contains a comment property (string) would be unable to conform to this object syntax.

Seems like // could be parsed equivalently to an HCL document with a first-line comment. These would be equivalent:

# tflint-ignore-file: foo

{
  "//": "tflint-ignore-file: foo"
}

[isobit]

I have no examples of this being used "in the wild"; my reasoning for the map structure was only that, hypothetically, someone might want to be able to use the comment property to leave other information. It wasn't clear to me how that would work if the comment property was a string which was already occupied by the tflint-ignore-file annotation.

For example:

{
  "//": {
    "tflint-ignore-file": "foo",
    "_1": "we ignore foo because reasons",
    "_2": "this file does stuff"
  }
}

I'm realizing now though that the JSON spec may allow for multiple properties with the key //; that may be a cleaner way to have multiple comments, at the expense of being harder to parse here (encoding/json discards duplicate entries). I could look for a way of handling that though. E.g.

{
  "//": "this file does stuff",
  "//": "we ignore foo because reasons",
  "//": "tflint-ignore-file: foo"
}

I'm open to alternatives though, it might make sense to expect a escaped linefeed-delimited string or a list of strings. E.g.

{
  "//": "this file does stuff\nwe ignore foo because reasons\ntflint-ignore-file: foo"
}

or

{
  "//": [
    "this file does stuff",
    "we ignore foo because reasons",
    "tflint-ignore-file: foo"
  ]
}

[bendrucker]

This was my assumption, thanks. I'd treat it as a string then, with the same rules as HCL:

tflint/tflint/annotation.go

Line 49 in fcbcce5

if !(token.Range.Start.Line == 1 && token.Range.Start.Column == 1) {

So if you have some existing comment property in the config, tflint-ignore-file would have to be prepended to that. Followed by whitespace, then any other comment content.

The map or list patterns are not inherently unreasonable, but TFLint tries to avoid creating Terraform patterns of its own.

[isobit]

Makes sense to me; will update in a bit here.

[wata727]

The documentation illustrates comment properties with string values

I thought the same thing, but as mentioned in #2230 (comment), I thought a map would not be a bad choice to add other comments.

In fact, having looked at the HCL JSON parser, it doesn't seem to matter what the type of the comment value is:
https://github.com/hashicorp/hcl/blob/v2.23.0/json/structure.go#L55-L59

Personally, I'm concerned that comments like the following look like comments to the tflint-ignore-file annotation:

{
  "//": "tflint-ignore-file: aws_instance_invalid_type # This file is auto-generated by tools/terraform_gen.go. DO NOT EDIT",
}

I like the following style the most:

{
  "//": "tflint-ignore-file: aws_instance_invalid_type",
  "//": "This file is auto-generated by tools/terraform_gen.go. DO NOT EDIT"
}

However, given the difficulties involved when you have multiple keys with the same name, I believe we need to find some compromise.