-
Notifications
You must be signed in to change notification settings - Fork 379
Add JSON configuration file support #2391
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
base: master
Are you sure you want to change the base?
Add JSON configuration file support #2391
Conversation
a963a04
to
29f637e
Compare
6c5caa0
to
c84a32a
Compare
This PR has been updated to include a compatibility shim for plugins using SDK versions < 0.23.0. The ProblemPlugins using SDK < 0.23.0 cannot parse JSON config files with .json extensions - they only recognize .tf.json as valid JSON. This causes errors when rules with custom parameters try to parse the config. The SolutionAdded a compatibility shim that:
Current Test StatusThe integration test is failing because the bundled terraform ruleset bypasses the init version checks that apply to third-party plugins. Rather than adding special-case handling for the bundled ruleset, the simpler solution is to wait for this PR to be merged and SDK v0.23.0 to be published. Once the SDK is updated, the bundled plugin will natively support .json extensions and all tests will pass. The compatibility shim will continue to ensure backward compatibility for any third-party plugins still using older SDK versions. |
ced163f
to
e3118f9
Compare
e3118f9
to
1335832
Compare
1335832
to
e443f8c
Compare
672d813
to
089b88d
Compare
…sion checking - Add support for JSON configuration files (.tflint.json) - Detect JSON configs by file extension, not filename pattern - Centralize all plugin version checking logic into plugin/plugin_version.go - Enforce SDK version 0.23+ for JSON configuration support - Split version checking: TFLint constraints checked before ApplyGlobalConfig, SDK versions checked after to avoid plugin initialization failures - Add comprehensive unit tests for SDK version validation - Update documentation to mention JSON configuration support This allows users to use JSON configuration files with any name when using the --config flag, while ensuring proper plugin compatibility.
089b88d
to
0807dfb
Compare
Sorry about the commit noise. I use Graphite to manage my CI in the backend and it gets a little pushy. This should be ready to review now. In summary: Changes by ComponentPlugin Version Checking (
Config Loading (
Inspection Flow (
Language Server (
Testing
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work here! Couple minor code readability comments, if you have time, it would be helpful. Otherwise I can make those edits.
switch { | ||
case strings.HasSuffix(strings.ToLower(file.Name()), ".json"): | ||
f, diags = parser.ParseJSON(src, file.Name()) | ||
default: | ||
f, diags = parser.ParseHCL(src, file.Name()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A switch expression with case expressions is easier to read than a bare switch:
switch { | |
case strings.HasSuffix(strings.ToLower(file.Name()), ".json"): | |
f, diags = parser.ParseJSON(src, file.Name()) | |
default: | |
f, diags = parser.ParseHCL(src, file.Name()) | |
} | |
switch filepath.Ext(file.Name()) { | |
case ".json": | |
f, diags = parser.ParseJSON(src, file.Name()) | |
default: | |
f, diags = parser.ParseHCL(src, file.Name()) | |
} |
You'll need to import filepath
too
config := EmptyConfig() | ||
config.sources = parser.Sources() | ||
// Set the flag if this is a JSON config file | ||
config.isJSONConfig = strings.HasSuffix(strings.ToLower(file.Name()), ".json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declare the empty config earlier so you can just do config.isJSONConfig = true
in the switch
statement rather than repeating the extension check.
|
||
// IsJSONConfig returns true if the configuration was loaded from a JSON file | ||
func (c *Config) IsJSONConfig() bool { | ||
return c.isJSONConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, you could just store the config path on the config object, unexported. Or expect it as the first/only key in sources
. And then call return filepath.Ext(filename) == ".json"
here
I'm travelling without my work computer til the 5th of November. Happy to make the changes on my return, else feel free to make them :). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Once you've addressed Ben's comments, we can merge this PR.
@@ -1,4 +1,4 @@ | |||
package main | |||
package inspection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why you changed the package name?
closes #2389
This change adds support for JSON-formatted TFLint configuration files (.tflint.json) in addition to the existing HCL format (.tflint.hcl).
Changes:
The implementation uses the existing hclparse.ParseJSON() from the HCL library, following the same JSON structure conventions as Terraform's .tf.json files. Existing functionality remains unchanged.
Other:
For full transparency, I used Claude Code to help generate this commit, but am responsible for reviews etc.