Support Terraform Validate Command

[appilon]

This PR adds support to call terraform validate via executeCommand. It expects a uri to the dir to run it from to be passed. It will error if the specified dir is an uninitialized module.

Closes #27

[appilon]

Just need a release of terraform-exec, as well as a PR for terraform-schema, then will bump the dependency references

[radeksimko]

I left you some mostly minor comments inline. Aside from these it LGTM.

[radeksimko]

Just for readability, would you mind creating some custom types here and compose the final diagsCache type from these?

For example:

type fileURI string
type diagnosticSource string
type fileDiagnostics map[diagnosticSource][]lsp.Diagnostic

diagsCache map[fileURI]fileDiagnostics

[radeksimko]

I reckon we'll need to find a more flexible way of integrating tfexec and the rootmodule functionality as adding proxy method for every command probably won't scale. Which is certainly not a criticism of your change here - This is fine in this PR as it maintains consistency, I'm just thinking out loud about the future. 😄

[radeksimko]

It may be convertible now, but even Terraform itself has its own type for interpreting diags, so I wouldn't be surprised if at some point we find out that hcl types just aren't sufficient to interpret validate diags.

I think that returning []tfjson.Diagnostic and then merging it into []lsp.Diagnostic outside of this package only when we need to, may be safer approach.

[appilon]

Could we punt on that for a future PR? It helped to only work in hcl until it gets to the diagnostics notifier package. But if you felt strongly I could create a second Publish method.

[radeksimko]

Yeah, I suppose it's fine - I reckon we'll need to revisit this logic soon anyway to deliver on the promise of integrating tflint and somehow more "generalizing" the interface, or at least making it extendable.

So I am fairly confident this will need to change in the future, but I'm ok with leaving it for another PR. 👌

[radeksimko]

I remember we had a conversation about this, but what was the reasoning behind ignoring the error here? Would we not want to differentiate e.g. between a state where the module doesn't exist at all and when it exists, but isn't initialized?

[appilon]

Is there really a difference? As long as the URI is a folder, it is either initialized and a place where you can call terraform validate, or it isn't? I'm still foggy on these distinctions even after our numerous sessions 😄

[radeksimko]

Well, in most cases there probably won't be a difference, but that's just because we rely on the client always calling didOpen (which automatically creates/loads the parent folder as a module) on a file from any folder that we run validate on.

In theory a client might integrate the command in such a way that it allows user to run it in folders which were never opened in the editor (but may in fact be initialized, just not discovered yet?).

We should probably somehow account for this edge case and automatically load the module and make it known, but I think that raising a correct error is a solid start?

[appilon]

Ahh okay makes sense now, I will raise the error

[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 context necessary to investigate further.