create terraform_standard_module_structure rule

docs/rules/README.md

@@ -78,4 +78,5 @@ These rules suggest to better ways.
 |[terraform_naming_convention](terraform_naming_convention.md)||
 |[terraform_required_version](terraform_required_version.md)||
 |[terraform_required_providers](terraform_required_providers.md)||
+|[terraform_standard_module_structure](terraform_standard_module_structure.md)||
 |[terraform_workspace_remote](terraform_workspace_remote.md)|✔|

docs/rules/terraform_standard_module_structure.md

@@ -0,0 +1,31 @@
+# terraform_standard_module_structure
+
+Ensure that a module complies with the Terraform [Standard Module Structure](https://www.terraform.io/docs/modules/index.html#standard-module-structure)
+
+## Example
+
+_main.tf_
+```hcl
+variable "v" {}
+```
+
+```
+$ tflint
+1 issue(s) found:
+
+Warning: variable "v" should be moved from main.tf to variables.tf (terraform_standard_module_structure)
+
+  on main.tf line 1:
+   1: variable "v" {}
+
+Reference: https://github.com/terraform-linters/tflint/blob/v0.16.0/docs/rules/terraform_standard_module_structure.md
+```
+
+## Why
+
+Terraform's documentation outlines a [Standard Module Structure](https://www.terraform.io/docs/modules/index.html#standard-module-structure). A minimal module should have a `main.tf`, `variables.tf`, and `outputs.tf` file. Variable and output blocks should be included in the corresponding file.
+
+## How To Fix
+
+* Move blocks to their conventional files as needed
+* Create empty files even if no `variable` or `output` blocks are defined

rules/provider.go

@@ -43,6 +43,7 @@ var manualDefaultRules = []Rule{
 	terraformrules.NewTerraformDocumentedVariablesRule(),
 	terraformrules.NewTerraformModulePinnedSourceRule(),
 	terraformrules.NewTerraformNamingConventionRule(),
+	terraformrules.NewTerraformStandardModuleStructureRule(),
 	terraformrules.NewTerraformTypedVariablesRule(),
 	terraformrules.NewTerraformRequiredVersionRule(),
 	terraformrules.NewTerraformRequiredProvidersRule(),

rules/terraformrules/terraform_standard_module_structure.go

@@ -0,0 +1,149 @@
+package terraformrules
+
+import (
+	"fmt"
+	"log"
+	"path"
+	"path/filepath"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/terraform-linters/tflint/tflint"
+)
+
+const (
+	filenameMain      = "main.tf"
+	filenameVariables = "variables.tf"
+	filenameOutputs   = "outputs.tf"
+)
+
+// TerraformStandardModuleStructureRule checks whether modules adhere to Terraform's standard module structure
+type TerraformStandardModuleStructureRule struct{}
+
+// NewTerraformStandardModuleStructureRule returns a new rule
+func NewTerraformStandardModuleStructureRule() *TerraformStandardModuleStructureRule {
+	return &TerraformStandardModuleStructureRule{}
+}
+
+// Name returns the rule name
+func (r *TerraformStandardModuleStructureRule) Name() string {
+	return "terraform_standard_module_structure"
+}
+
+// Enabled returns whether the rule is enabled by default
+func (r *TerraformStandardModuleStructureRule) Enabled() bool {
+	return false
+}
+
+// Severity returns the rule severity
+func (r *TerraformStandardModuleStructureRule) Severity() string {
+	return tflint.WARNING
+}
+
+// Link returns the rule reference link
+func (r *TerraformStandardModuleStructureRule) Link() string {
+	return tflint.ReferenceLink(r.Name())
+}
+
+// Check emits errors for any missing files and any block types that are included in the wrong file
+func (r *TerraformStandardModuleStructureRule) Check(runner *tflint.Runner) error {
+	log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath())
+
+	r.checkFiles(runner)
+	r.checkVariables(runner)
+	r.checkOutputs(runner)
+
+	return nil
+}
+
+func (r *TerraformStandardModuleStructureRule) checkFiles(runner *tflint.Runner) {
+	if r.onlyJSON(runner) {
+		return
+	}
+
+	files := runner.Files()
+	for name, file := range files {
+		files[path.Base(name)] = file
+	}
+
+	if files[filenameMain] == nil {
+		runner.EmitIssue(
+			r,
+			fmt.Sprintf("Module should include a %s file as the primary entrypoint", filenameMain),
+			hcl.Range{
+				Filename: path.Join(runner.TFConfig.Module.SourceDir, filenameMain),
+				Start:    hcl.InitialPos,
+			},
+		)
+	}
+
+	if files[filenameVariables] == nil && len(runner.TFConfig.Module.Variables) == 0 {
+		runner.EmitIssue(
+			r,
+			fmt.Sprintf("Module should include an empty %s file", filenameVariables),
+			hcl.Range{
+				Filename: path.Join(runner.TFConfig.Module.SourceDir, filenameVariables),
+				Start:    hcl.InitialPos,
+			},
+		)
+	}
+
+	if files[filenameOutputs] == nil && len(runner.TFConfig.Module.Outputs) == 0 {
+		runner.EmitIssue(
+			r,
+			fmt.Sprintf("Module should include an empty %s file", filenameOutputs),
+			hcl.Range{
+				Filename: path.Join(runner.TFConfig.Module.SourceDir, filenameOutputs),
+				Start:    hcl.InitialPos,
+			},
+		)
+	}
+}
+
+func (r *TerraformStandardModuleStructureRule) checkVariables(runner *tflint.Runner) {
+	for _, variable := range runner.TFConfig.Module.Variables {
+		if filename := variable.DeclRange.Filename; r.shouldMove(filename, filenameVariables) {
+			runner.EmitIssue(
+				r,
+				fmt.Sprintf("variable %q should be moved from %s to %s", variable.Name, filename, filenameVariables),
+				variable.DeclRange,
+			)
+		}
+	}
+}
+
+func (r *TerraformStandardModuleStructureRule) checkOutputs(runner *tflint.Runner) {
+	for _, variable := range runner.TFConfig.Module.Outputs {
+		if filename := variable.DeclRange.Filename; r.shouldMove(filename, filenameOutputs) {
+			runner.EmitIssue(
+				r,
+				fmt.Sprintf("output %q should be moved from %s to %s", variable.Name, filename, filenameOutputs),
+				variable.DeclRange,
+			)
+		}
+	}
+}
+
+func (r *TerraformStandardModuleStructureRule) onlyJSON(runner *tflint.Runner) bool {
+	files := runner.Files()
+
+	if len(files) == 0 {
+		return false
+	}
+
+	for filename := range files {
+		if filepath.Ext(filename) != ".json" {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (r *TerraformStandardModuleStructureRule) shouldMove(path string, expected string) bool {
+	// json files are likely generated and conventional filenames do not apply
+	if filepath.Ext(path) == ".json" {
+		return false
+	}
+
+	return path != expected
+}

rules/terraformrules/terraform_standard_module_structure_test.go

@@ -0,0 +1,156 @@
+package terraformrules
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+	"github.com/terraform-linters/tflint/tflint"
+)
+
+func Test_TerraformStandardModuleStructureRule(t *testing.T) {
+	cases := []struct {
+		Name     string
+		Content  map[string]string
+		Expected tflint.Issues
+	}{
+		{
+			Name:    "empty module",
+			Content: map[string]string{},
+			Expected: tflint.Issues{
+				{
+					Rule:    NewTerraformStandardModuleStructureRule(),
+					Message: "Module should include a main.tf file as the primary entrypoint",
+					Range: hcl.Range{
+						Filename: "main.tf",
+						Start:    hcl.InitialPos,
+					},
+				},
+				{
+					Rule:    NewTerraformStandardModuleStructureRule(),
+					Message: "Module should include an empty variables.tf file",
+					Range: hcl.Range{
+						Filename: "variables.tf",
+						Start:    hcl.InitialPos,
+					},
+				},
+				{
+					Rule:    NewTerraformStandardModuleStructureRule(),
+					Message: "Module should include an empty outputs.tf file",
+					Range: hcl.Range{
+						Filename: "outputs.tf",
+						Start:    hcl.InitialPos,
+					},
+				},
+			},
+		},
+		{
+			Name: "directory in path",
+			Content: map[string]string{
+				"foo/main.tf":      "",
+				"foo/variables.tf": "",
+			},
+			Expected: tflint.Issues{
+				{
+					Rule:    NewTerraformStandardModuleStructureRule(),
+					Message: "Module should include an empty outputs.tf file",
+					Range: hcl.Range{
+						Filename: "foo/outputs.tf",
+						Start:    hcl.InitialPos,
+					},
+				},
+			},
+		},
+		{
+			Name: "move variable",
+			Content: map[string]string{
+				"main.tf": `
+variable "v" {}
+`,
+				"variables.tf": "",
+				"outputs.tf":   "",
+			},
+			Expected: tflint.Issues{
+				{
+					Rule:    NewTerraformStandardModuleStructureRule(),
+					Message: `variable "v" should be moved from main.tf to variables.tf`,
+					Range: hcl.Range{
+						Filename: "main.tf",
+						Start: hcl.Pos{
+							Line:   2,
+							Column: 1,
+						},
+						End: hcl.Pos{
+							Line:   2,
+							Column: 13,
+						},
+					},
+				},
+			},
+		},
+		{
+			Name: "move output",
+			Content: map[string]string{
+				"main.tf": `
+output "o" { value = null }
+`,
+				"variables.tf": "",
+				"outputs.tf":   "",
+			},
+			Expected: tflint.Issues{
+				{
+					Rule:    NewTerraformStandardModuleStructureRule(),
+					Message: `output "o" should be moved from main.tf to outputs.tf`,
+					Range: hcl.Range{
+						Filename: "main.tf",
+						Start: hcl.Pos{
+							Line:   2,
+							Column: 1,
+						},
+						End: hcl.Pos{
+							Line:   2,
+							Column: 11,
+						},
+					},
+				},
+			},
+		},
+		{
+			Name: "json only",
+			Content: map[string]string{
+				"main.tf.json": "{}",
+			},
+			Expected: tflint.Issues{},
+		},
+		{
+			Name: "json variable",
+			Content: map[string]string{
+				"main.tf.json": `{"variable": {"v": {}}}`,
+			},
+			Expected: tflint.Issues{},
+		},
+		{
+			Name: "json output",
+			Content: map[string]string{
+				"main.tf.json": `{"output": {"o": {"value": null}}}`,
+			},
+			Expected: tflint.Issues{},
+		},
+	}
+
+	rule := NewTerraformStandardModuleStructureRule()
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.Name, func(t *testing.T) {
+			runner := tflint.TestRunnerWithConfig(t, tc.Content, &tflint.Config{
+				Module: true,
+			})
+
+			if err := rule.Check(runner); err != nil {
+				t.Fatalf("Unexpected error occurred: %s", err)
+			}
+
+			tflint.AssertIssues(t, tc.Expected, runner.Issues)
+		})
+	}
+}

tflint/testing.go

@@ -2,6 +2,7 @@ package tflint
 
 import (
 	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -33,7 +34,28 @@ func TestRunnerWithConfig(t *testing.T, files map[string]string, config *Config)
 		t.Fatal(err)
 	}
 
-	cfg, err := loader.LoadConfig(".")
+	dirMap := map[string]*struct{}{}
+	for file := range files {
+		dirMap[filepath.Dir(file)] = nil
+	}
+	dirs := make([]string, 0)
+	for dir := range dirMap {
+		dirs = append(dirs, dir)
+	}
+
+	if len(dirs) > 1 {
+		t.Fatalf("All test files must be in the same directory, got %d directories: %v", len(dirs), dirs)
+		return nil
+	}
+
+	var dir string
+	if len(dirs) == 0 {
+		dir = "."
+	} else {
+		dir = dirs[0]
+	}
+
+	cfg, err := loader.LoadConfig(dir)
 	if err != nil {
 		t.Fatal(err)
 	}