terraform_unused_declarations triggers on data sources with pre/postconditions

alexjurkiewicz commented 2 years ago

Introduction

Sometimes I use new Terraform 1.2 pre/postconditions for runtime assertions on data sources with no other references:

data "aws_region" "this" {
  lifecycle {
    # Sanity check we are running in dev
    postcondition {
      condition     = self.name == "eu-central-1"
      error_message = "This stack only works in eu-central-1."
    }
  }
}

Expected Behavior

No error should be generated.

Actual behavior

main.tf:1:1: Warning - data "aws_region" "this" is declared but not used (terraform_unused_declarations)

Additional Context

$ tflint -v
TFLint version 0.40.0
+ ruleset.terraform (0.1.0-bundled)
+ ruleset.aws (0.17.0)
$ terraform version
Terraform v1.2.9
on darwin_amd64

PS, I love tflint! Keep it up! 😍

wata727 commented 2 years ago

It's a technical limitation that we can't completely detect unused data sources with side effects. https://github.com/terraform-linters/tflint-ruleset-terraform/blob/v0.1.1/docs/rules/terraform_unused_declarations.md#how-to-fix But in this example, we might be able to improve the check based on the presence of a self reference.

PS, I love tflint! Keep it up! 😍

Thanks! I'm glad to hear that :)

alexjurkiewicz commented 2 years ago

After thinking about it overnight, the current behaviour is probably the best.

Many pre/post condition blocks will not be assertions about the whole stack state and shouldn't prevent the data source from being deleted. For example the official Terraform docs use data "aws_ami" as an example, and assert on the image's architecture.

So, I don't think anything should change. My usage should continue to require ignore directives 😊

On Thu, 22 Sept 2022, 00:03 Kazuma Watanabe, @.***> wrote:

It's a technical limitation that we can't completely detect unused data sources with side effects.

https://github.com/terraform-linters/tflint-ruleset-terraform/blob/v0.1.1/docs/rules/terraform_unused_declarations.md#how-to-fix But in this example, we might be able to improve the check based on the presence of a self reference.

PS, I love tflint! Keep it up! 😍

Thanks! I'm glad to hear that :)

— Reply to this email directly, view it on GitHub https://github.com/terraform-linters/tflint-ruleset-terraform/issues/28#issuecomment-1253759256, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC4U5PKMQDDB54ALVFQZLLV7MIR7ANCNFSM6AAAAAAQSDFWWM . You are receiving this because you authored the thread.Message ID: @.*** com>

terraform-linters / tflint-ruleset-terraform