fix: terraform linter - handled multiple top level attributes. closes #…

…87 (#91)
abcxyz · Mar 17, 2023 · 8cfb3be · 8cfb3be
1 parent 6b92457
commit 8cfb3be
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 18 deletions.
diff --git a/internal/tools/terraformlinter/terraform_linter.go b/internal/tools/terraformlinter/terraform_linter.go
@@ -43,7 +43,8 @@ type tokenPosition int32
 
 const (
 	None tokenPosition = iota
-	Leading
+	LeadingStart
+	LeadingEnd
 	ProviderStart
 	ProviderCenter
 	ProviderEnd
@@ -77,10 +78,10 @@ const (
 
 // mapping of attributes to their expected position.
 var positionMap = map[string]tokenPosition{
-	attrForEach:                Leading,
-	attrCount:                  Leading,
-	attrProvider:               Leading,
-	attrSource:                 Leading,
+	attrForEach:                LeadingStart,
+	attrCount:                  LeadingStart,
+	attrSource:                 LeadingStart,
+	attrProvider:               LeadingEnd,
 	attrProviderProject:        ProviderEnd,
 	attrProviderProjectID:      ProviderEnd,
 	attrProviderFolder:         ProviderCenter,
@@ -268,21 +269,16 @@ func generateViolations(idents []tokenAttr) []*ViolationInstance {
 	for pos, token := range idents {
 		contents := string(token.token.Bytes)
 		switch contents {
-		// for_each and count should be at the top
-		case attrForEach, attrCount:
-			if pos != 0 {
+		// for_each, count and source should be at the top
+		case attrForEach, attrCount, attrSource:
+			if pos != 0 && lastAttr.tokenPos != LeadingStart {
 				instances = append(instances, &ViolationInstance{ViolationType: fmt.Sprintf(violationLeadingMetaBlockAttribute, contents), Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
 		// provider is at the top but below for_each or count if they exist
 		case attrProvider:
-			if pos > 0 && lastAttr.tokenPos != Leading {
+			if pos > 0 && lastAttr.tokenPos != LeadingStart {
 				instances = append(instances, &ViolationInstance{ViolationType: fmt.Sprintf(violationLeadingMetaBlockAttribute, attrProvider), Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
-		// source should be the top attribute in a module
-		case attrSource:
-			if pos != 0 {
-				instances = append(instances, &ViolationInstance{ViolationType: fmt.Sprintf(violationLeadingMetaBlockAttribute, attrSource), Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
-			}
 		case attrDependsOn:
 			// depends_on somewhere above where it should be
 			if pos < len(idents)-1 && idents[len(idents)-1].tokenPos != Trailing {
@@ -308,31 +304,31 @@ func generateViolations(idents []tokenAttr) []*ViolationInstance {
 			if lastAttr.tokenPos > ProviderStart {
 				instances = append(instances, &ViolationInstance{ViolationType: fmt.Sprintf(violationProviderAttributes, contents), Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
-			if lastAttr.tokenPos == Leading && !lastAttr.trailingNewline {
+			if (lastAttr.tokenPos == LeadingStart || lastAttr.tokenPos == LeadingEnd) && !lastAttr.trailingNewline {
 				instances = append(instances, &ViolationInstance{ViolationType: violationMetaBlockNewline, Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
 		case attrProviderFolder,
 			attrProviderFolderID:
 			if lastAttr.tokenPos > ProviderCenter {
 				instances = append(instances, &ViolationInstance{ViolationType: fmt.Sprintf(violationProviderAttributes, contents), Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
-			if lastAttr.tokenPos == Leading && !lastAttr.trailingNewline {
+			if (lastAttr.tokenPos == LeadingStart || lastAttr.tokenPos == LeadingEnd) && !lastAttr.trailingNewline {
 				instances = append(instances, &ViolationInstance{ViolationType: violationMetaBlockNewline, Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
 		case attrProviderProject,
 			attrProviderProjectID:
 			if lastAttr.tokenPos > ProviderEnd {
 				instances = append(instances, &ViolationInstance{ViolationType: fmt.Sprintf(violationProviderAttributes, contents), Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
-			if lastAttr.tokenPos == Leading && !lastAttr.trailingNewline {
+			if (lastAttr.tokenPos == LeadingStart || lastAttr.tokenPos == LeadingEnd) && !lastAttr.trailingNewline {
 				instances = append(instances, &ViolationInstance{ViolationType: violationMetaBlockNewline, Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
 		// Check for trailing newlines where required
 		default:
 			if lastAttr.tokenPos == ProviderEnd && !lastAttr.trailingNewline {
 				instances = append(instances, &ViolationInstance{ViolationType: violationProviderNewline, Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
-			if lastAttr.tokenPos == Leading && !lastAttr.trailingNewline {
+			if (lastAttr.tokenPos == LeadingStart || lastAttr.tokenPos == LeadingEnd) && !lastAttr.trailingNewline {
 				instances = append(instances, &ViolationInstance{ViolationType: violationMetaBlockNewline, Path: token.token.Range.Filename, Line: token.token.Range.Start.Line})
 			}
 		}

diff --git a/internal/tools/terraformlinter/terraform_linter_test.go b/internal/tools/terraformlinter/terraform_linter_test.go
@@ -658,6 +658,23 @@ func TestTerraformLinter_FindViolations(t *testing.T) {
 			},
 			wantError: false,
 		},
+		// Issue #87 - source and for_each are both valid at the top and shouldn't
+		// cause violations if both are present.
+		{
+			name: "for_each_and_source_both_present_repro",
+			content: `
+				module "some_module" {
+					source = "git://https://github.com/abc/def"
+					for_each = local.mylocal
+				}
+
+				module "some_module" {
+					for_each = local.mylocal
+					source = "git://https://github.com/abc/def"
+				}
+			`,
+			wantError: false,
+		},
 	}
 
 	for _, tc := range cases {