-
Notifications
You must be signed in to change notification settings - Fork 348
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
Add option --enable-all to enable all rules #1311
Conversation
Thank you for your contribution! First, this change only enables some of the Terraform rules built into the TFLint core, not rules provided by plugins. To enable all plugin rules, you need to add an RPC endpoint to plugin and call it from TFLint core. Second, we can't enable rules that require settings like For these reasons, I don't think we should add an option to enable all rules uniformly. |
Hi @wata727 thank you for maintaining tflint. I was looking for a way to find unused variables in Terraform files and found out that this should be possible with tflint via this issue hashicorp/terraform#11412 (comment) In my opinion it would be nice to have the the most common and provider independent checks built into the TFLlint core. This way using tflint would be super simple. You could download tflint and run it in the directory where you have your Terraform files without further configuration. And then, only if you have unwanted warnings or if you want to add additional checks, you can start to configure tflint to disable rules or add plugins. For me it would be ok if this option would only enable the built in rules but I understand that this could be confusing for people which use plugins. So I understand if you don't want this change in tflint and its OK if you close this PR. |
How to design the default rules is a difficult issue. Since linter's default rules tend to strongly reflect the opinionated thought of the author, I took the policy of disabling all but the rules that were clearly useful to all users. It focuses on making it easy to introduce by minimizing warnings for already working Terraform configurations. I learned from my experience in Go that detecting unused declarations is not always useful in a development environment, so I decided to disable this rule. But I still don't know if that was correct... In the end, the answer I arrived at was a design like ESLint where the user could choose the set of rules themselves. In your example, you can start an inspection right away by installing the "strict" Terraform ruleset, which includes the unused declaration rule. |
I'm just trying tflint on a large project and I was surprised that it did not detect anything wrong. It took me some time to realize that rules were not being enabled at all and I had to specify them one by one (either in the config file or command line). Had I not dug deeper into the issue I would have thought tflint didn't do anything at all. It might be worth to enable at least all core rules that don't require customization from users. |
@wata727 Somehow i missed your last reply. How do I install the strict rule set? I think your design decision is perfectly fine. But if you want to solve it like this i suggest that all rules are disabled by default and a warning is presented to the user if no rules are enabled or if no configuration was found. This way if a user just downloads tflint and expect it to check certain basic rules they get a warning that they have to configure the rules themself. At least for me such a hint would have been very helpful. |
Thank you for the advice. To be sure, the current UX is not ideal. The biggest problem is that the terraform rules have been built into the core for historical reasons, and I believe that we can improve it to design a better UX. |
The current design cannot accept this feature and will close it. Thank you for your contribution. |
It would be nice to have a way to enable all available rules.
It seems that there are also other people requesting this feature #1056