Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use a temporary file for tflint #3717

Merged
merged 2 commits into from
May 25, 2021
Merged

Don't use a temporary file for tflint #3717

merged 2 commits into from
May 25, 2021

Conversation

dimbleby
Copy link
Contributor

@dimbleby dimbleby commented May 9, 2021

Standard terraform module structure is to split files into main.tf, variables.tf, output.tf.

Current tflint linting creates a temporary file. But taking eg the main.tf in isolation prevents tflint from doing its job, ALEInfo shows output like this:

{"issues":[],"errors":[{"message":"Failed to check `azurerm_public_ip_invalid_sku` rule: Failed to eval an expression i
n /tmp/nvimcFkTcA/2/main.tf:49; Reference to undeclared input variable: An input variable with the name \"vm\" has not
been declared. This variable can be declared with a variable \"vm\" {} block."}]}

... where the vm was indeed declared, in the variables.tf file, where it should be.

Simply not using a temporary file resolves this.

@dimbleby
Copy link
Contributor Author

dimbleby commented May 9, 2021

Using a temporary file was deliberately introduced, at #1052.

Presumably either ALE or tflint has changed since then, the linting works fine without it now.

@dimbleby
Copy link
Contributor Author

dimbleby commented May 9, 2021

Ah, but then the linter become useless in a different way: tflint doesn't recurse into subdirectories. So you need to be in the working directory of the file being linted.

Therefore I've also set cwd for this linter.

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change seems correct and basic testing seems to work for me.

@hsanson hsanson merged commit 3f386ae into dense-analysis:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants