Update moved module references (output) with new module identifier

Klaitos commented 1 year ago

Hello,

For now, when you rename module which has some outputs and other terraform resources using existing outputs, you can't run tfautomv without first update those references.

module "module1" {
 # i have an output1
}

resource "resource2" "i_use_module_1" {
  props = module1.output1
}

module "renamed_module1" {
}

resource "resource2" "i_use_module_1" {
     props = renamed_module1.output1
}

What do you think if tfautomv could handle this kind of refactor on our behalf ?

busser commented 1 year ago

Hello, thanks for opening this issue. I'll do my best to answer :)

My understanding of the problem

If I understand you correctly, you want to be able to make this change:

+ module "renamed_module1" 
- module "module1" {
    // ...
  }

and for tfautomv to automatically make this change for you:

  resource "resource2" "i_use_module_1" {
+   props = module.renamed_module1.output1
-   props = module.module1.output1
  }

Correct me if that's not the case. The rest of this comment assumes that I understood your goal correctly :)

Short answer

I think this can only be done by a language server. I am eager for the Terraform language server to support this.

Even if tfautomv could theoretically do it, which I don't think it could, I believe this capability is outside of tfautomv's scope.

Long answer

Let me start by saying that this isn't specific to modules. The exact same reasoning applies to simple resources, except that instead of outputs they have attributes.

In the example you provided, once we make the first change, the Terraform code is no longer valid. In fact, terraform validate will fail. Validation is one of the first steps Terraform performs, so terraform plan will also fail. tfautomv works by parsing the output of terraform plan, which is the first obstacle to implementing this feature. In order to do what you describe, tfautomv would need to parse the source code directly, which it doesn't currently do.

Even if we improved tfautomv to read the source code, I don't think it would have enough information to compute the desired change. In your example, tfautomv would run with the source code like this:

module "renamed_module1" { // <-- renamed manually
  // ...
}

resource "resource2" "i_use_module_1" {
     props = module.module1.output1 // <-- not renamed yet
}

How is tfautomv supposed to guess that renamed_module1's previous name was module1? I can think of a situation where it definitely couldn't:

module "renamed_module1" { // <-- renamed manually
  // ...
}

module "module2" { // <-- already existed, not renamed, same outputs
  // ...
}

resource "resource2" "i_use_module_1" {
  props = module.module1.output1 // <-- change to `renamed_module1` or `module2`?
}

The way I see it, the only way to rename module1 to renamed_module1 correctly is to do it everywhere at the same time. This seems like it would be the responsibility of an IDE, helped by a language server.

I found two relevant issues, but there may be more:

These issues are still open, and I don't know if they are being worked on. I hope that support for renaming symbols arrives in the language server soon. In the mean time, the question is: should we add this capability to tfautomv?

I believe the answer is no. I am open to discussing it, but here's my reasoning: tfautomv's role is to generate moved blocks, and here we are talking about editing existing source code. This seems to fall outside of tfautomv's scope.

I am happy to discuss this further, so I'll leave this issue open. If we can conceive of a design that would allow tfautomv to do this, then I will reconsider whether we want to extend tfautomv's scope :)

Klaitos commented 1 year ago

Thank you Arthur for such a complete answer. I was expecting that this could be an out of scope feature but I wasn't aware that it could be so difficult (impossible) for the tool to guess which module has to be rewrite. You are totally right, we will wait the tf language server to achieve this.

busser / tfautomv