New Check: S038 for invalid attribute reference semantically (AtLeast…

…OneOf)
bflad · Apr 12, 2020 · bfc47a3 · bfc47a3
1 parent dde8451
commit bfc47a3
Show file tree

Hide file tree

Showing 13 changed files with 363 additions and 10 deletions.
diff --git a/helper/analysisutils/schema_analyzers.go b/helper/analysisutils/schema_analyzers.go
@@ -2,15 +2,16 @@ package analysisutils
 
 import (
 	"fmt"
+	"github.com/bflad/tfproviderlint/passes/helper/schema/resourceinfo"
 
 	"github.com/bflad/tfproviderlint/passes/commentignore"
 	"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfo"
 	"golang.org/x/tools/go/analysis"
 )
 
-// SchemaAttributeReferencesAnalyzer returns an Analyzer for fields that use schema attribute references
-func SchemaAttributeReferencesAnalyzer(analyzerName string, fieldName string) *analysis.Analyzer {
-	doc := fmt.Sprintf(`check for Schema with invalid %[2]s references
+// SchemaAttributeReferencesSyntaxAnalyzer returns an Analyzer for fields that use schema attribute references
+func SchemaAttributeReferencesSyntaxAnalyzer(analyzerName string, fieldName string) *analysis.Analyzer {
+	doc := fmt.Sprintf(`check for Schema %[2]s references with invalid syntax
 
 The %[1]s analyzer ensures schema attribute references in the Schema %[2]s
 field use valid syntax. The Terraform Plugin SDK can unit test attribute
@@ -24,6 +25,25 @@ references to verify the references against the full schema.
 			commentignore.Analyzer,
 			schemainfo.Analyzer,
 		},
-		Run: SchemaAttributeReferencesRunner(analyzerName, fieldName),
+		Run: SchemaAttributeReferencesSyntaxRunner(analyzerName, fieldName),
+}
+}
+
+// SchemaAttributeReferencesSemanticsAnalyzer returns an Analyzer for fields that use schema attribute references
+func SchemaAttributeReferencesSemanticsAnalyzer(analyzerName string, fieldName string) *analysis.Analyzer {
+	doc := fmt.Sprintf(`check for Schema %[2]s references with invalid semantics
+
+The %[1]s analyzer ensures schema attribute references in the Schema %[2]s
+field refers to valid attribute.
+`, analyzerName, fieldName)
+
+	return &analysis.Analyzer{
+		Name: analyzerName,
+		Doc:  doc,
+		Requires: []*analysis.Analyzer{
+			commentignore.Analyzer,
+			resourceinfo.Analyzer,
+		},
+		Run: SchemaAttributeReferencesSemanticsRunner(analyzerName, fieldName),
 	}
 }
diff --git a/helper/analysisutils/schema_runners.go b/helper/analysisutils/schema_runners.go
@@ -1,6 +1,7 @@
 package analysisutils
 
 import (
+	"github.com/bflad/tfproviderlint/passes/helper/schema/resourceinfo"
 	"go/ast"
 
 	"github.com/bflad/tfproviderlint/helper/astutils"
@@ -10,8 +11,8 @@ import (
 	"golang.org/x/tools/go/analysis"
 )
 
-// SchemaAttributeReferencesRunner returns an Analyzer runner for fields that use schema attribute references
-func SchemaAttributeReferencesRunner(analyzerName string, fieldName string) func(*analysis.Pass) (interface{}, error) {
+// SchemaAttributeReferencesSyntaxRunner returns an Analyzer runner for fields that use schema attribute references
+func SchemaAttributeReferencesSyntaxRunner(analyzerName string, fieldName string) func(*analysis.Pass) (interface{}, error) {
 	return func(pass *analysis.Pass) (interface{}, error) {
 		ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
 		schemaInfos := pass.ResultOf[schemainfo.Analyzer].([]*schema.SchemaInfo)
@@ -39,7 +40,74 @@ func SchemaAttributeReferencesRunner(analyzerName string, fieldName string) func
 					}
 
 					if _, err := schema.ParseAttributeReference(*attributeReference); err != nil {
-						pass.Reportf(elt.Pos(), "%s: invalid %s attribute reference: %s", analyzerName, fieldName, err)
+						pass.Reportf(elt.Pos(), "%s: invalid %s attribute reference syntax: %s", analyzerName, fieldName, err)
+					}
+				}
+			}
+		}
+
+		return nil, nil
+	}
+}
+
+// SchemaAttributeReferencesSemanticsRunner returns an Analyzer runner for fields that use schema attribute references
+func SchemaAttributeReferencesSemanticsRunner(analyzerName string, fieldName string) func(*analysis.Pass) (interface{}, error) {
+	return func(pass *analysis.Pass) (interface{}, error) {
+		ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
+		resourceInfos := pass.ResultOf[resourceinfo.Analyzer].([]*schema.ResourceInfo)
+
+		for _, resourceInfo := range resourceInfos  {
+			// Handle "root" schema.Resource only since we will use the resourceInfo of that to validate attribute reference.
+			if !resourceInfo.IsDataSource() && !resourceInfo.IsResource() {
+				continue
+			}
+
+			// Collection schemaInfo under current resourceInfo
+			var schemaInfos []*schema.SchemaInfo
+			ast.Inspect(resourceInfo.AstCompositeLit, func(n ast.Node) bool {
+				compositeLit, ok := n.(*ast.CompositeLit)
+
+				if !ok {
+					return true
+				}
+
+				if schema.IsMapStringSchema(compositeLit, pass.TypesInfo) {
+					for _, mapSchema := range schema.GetSchemaMapSchemas(compositeLit) {
+						schemaInfos = append(schemaInfos, schema.NewSchemaInfo(mapSchema, pass.TypesInfo))
+					}
+				} else if schema.IsTypeSchema(pass.TypesInfo.TypeOf(compositeLit.Type)) {
+					schemaInfos = append(schemaInfos, schema.NewSchemaInfo(compositeLit, pass.TypesInfo))
+				}
+
+				return true
+			})
+
+			// Validate each schemaInfo under current resourceInfo
+			for _, schemaInfo := range schemaInfos {
+				if ignorer.ShouldIgnore(analyzerName, schemaInfo.AstCompositeLit) {
+					continue
+				}
+
+				if !schemaInfo.DeclaresField(fieldName) {
+					continue
+				}
+
+				switch value := schemaInfo.Fields[fieldName].Value.(type) {
+				case *ast.CompositeLit:
+					if !astutils.IsExprTypeArrayString(value.Type) {
+						continue
+					}
+
+					for _, elt := range value.Elts {
+						attributeReference := astutils.ExprStringValue(elt)
+
+						if attributeReference == nil {
+							continue
+						}
+
+						if _, err := schema.ValidateAttributeReference(resourceInfo.Resource, *attributeReference); err != nil {
+							pass.Reportf(elt.Pos(), "%s: invalid %s attribute reference semantics: %s", analyzerName, fieldName, err)
+						}
 					}
 				}
 			}

diff --git a/helper/terraformtype/helper/schema/attributes.go b/helper/terraformtype/helper/schema/attributes.go
@@ -2,6 +2,7 @@ package schema
 
 import (
 	"fmt"
+	tfschema "github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 	"math"
 	"regexp"
 	"strconv"
@@ -59,3 +60,42 @@ func ParseAttributeReference(reference string) ([]string, error) {
 
 	return attributeReferenceParts, nil
 }
+
+// ValidateAttributeReference validates schema attribute reference.
+// Attribute references are used in Schema fields such as AtLeastOneOf, ConflictsWith, and ExactlyOneOf.
+func ValidateAttributeReference(tfresource *tfschema.Resource, reference string) ([]string, error) {
+	var curSchemaOrResource interface{} = tfresource
+	attributeReferenceParts := strings.Split(reference, ".")
+	for idx, attributeReferencePart := range attributeReferenceParts {
+		attributeReference := strings.Join(attributeReferenceParts[:idx+1], ".")
+		if math.Mod(float64(idx), 2) == 1 {
+			// For odd part, ensure it is a 0 and the containing schema has `MaxItems` set to `1`. This is because
+			// reference among multiple instances of the same nested block is not supported in current plugin SDK.
+			attributeReferencePartInt, err := strconv.Atoi(attributeReferencePart)
+
+			configurationBlockReferenceErr := fmt.Errorf("%q configuration block attribute references must be separated by .0", attributeReference)
+			if err != nil {
+				return nil, configurationBlockReferenceErr
+			}
+			if attributeReferencePartInt != 0 {
+				return nil, configurationBlockReferenceErr
+			}
+
+			curSchema := curSchemaOrResource.(*tfschema.Schema)
+			if curSchema.MaxItems != 1 || curSchema.Type != tfschema.TypeList {
+				return nil, fmt.Errorf("%q configuration block attribute references are only valid for TypeList and MaxItems: 1 attributes", attributeReference)
+			}
+			curSchemaOrResource = curSchema.Elem
+		} else {
+			// For even part, ensure it references to defined attribute
+			schema := curSchemaOrResource.(*tfschema.Resource).Schema[attributeReferencePart]
+			if schema == nil {
+				return nil, fmt.Errorf("%q references to unknown attribute", attributeReference)
+			}
+			curSchemaOrResource = schema
+		}
+	}
+
+	return attributeReferenceParts, nil
+}
+
diff --git a/passes/S035/S035.go b/passes/S035/S035.go
@@ -5,4 +5,4 @@ import (
 	"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
 )
 
-var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S035", schema.SchemaFieldAtLeastOneOf)
+var Analyzer = analysisutils.SchemaAttributeReferencesSyntaxAnalyzer("S035", schema.SchemaFieldAtLeastOneOf)
diff --git a/passes/S036/S036.go b/passes/S036/S036.go
@@ -5,4 +5,4 @@ import (
 	"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
 )
 
-var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S036", schema.SchemaFieldConflictsWith)
+var Analyzer = analysisutils.SchemaAttributeReferencesSyntaxAnalyzer("S036", schema.SchemaFieldConflictsWith)
diff --git a/passes/S037/S037.go b/passes/S037/S037.go
@@ -5,4 +5,4 @@ import (
 	"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
 )
 
-var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S037", schema.SchemaFieldExactlyOneOf)
+var Analyzer = analysisutils.SchemaAttributeReferencesSyntaxAnalyzer("S037", schema.SchemaFieldExactlyOneOf)
diff --git a/passes/S038/README.md b/passes/S038/README.md
@@ -0,0 +1,86 @@
+# S038
+
+The S038 analyzer reports cases of Schemas which include `ConflictsWith` and have invalid schema attribute references.
+
+NOTE: This pass only works with Terraform resources that are fully defined in a single function.
+
+## Flagged Code
+
+```go
+// Attribute reference in multi nested block is not supported
+&schema.Resource{
+    Schema: map[string]*schema.Schema{
+        "x": {
+            Type: schema.TypeList,
+            Elem: &schema.Resource{
+                Schema: map[string]*schema.Schema{
+                    "foo": {
+                        AtLeastOneOf: []string{"x.0.bar"},
+                    },
+                    "bar": {
+                        AtLeastOneOf: []string{"x.0.foo"},
+                    },
+                },
+            },
+        },
+    },
+}
+```
+
+or
+
+```go
+// Non-existed attribute reference
+&schema.Resource{
+    Schema: map[string]*schema.Schema{
+        "x": {
+            Type: schema.TypeList,
+            MaxItems: 1,
+            Elem: &schema.Resource{
+                Schema: map[string]*schema.Schema{
+                    "foo": {
+                        AtLeastOneOf: []string{"x.1.bar"},
+                    },
+                    "bar": {
+                        AtLeastOneOf: []string{"x.1.foo"},
+                    },
+                },
+            },
+        },
+    },
+}
+```
+
+## Passing Code
+
+```go
+&schema.Resource{
+    Schema: map[string]*schema.Schema{
+        "x": {
+            Type: schema.TypeList,
+            MaxItems: 1,
+            Elem: &schema.Resource{
+                Schema: map[string]*schema.Schema{
+                    "foo": {
+                        AtLeastOneOf: []string{"x.0.bar"},
+                    },
+                    "bar": {
+                        AtLeastOneOf: []string{"x.0.foo"},
+                    },
+                },
+            },
+        },
+    },
+}
+```
+
+## Ignoring Reports
+
+Singular reports can be ignored by adding the a `//lintignore:S038` Go code comment at the end of the offending line or on the line immediately proceding, e.g.
+
+```go
+//lintignore:S038
+&schema.Resource{
+    // ...
+}
+```
diff --git a/passes/S038/S038.go b/passes/S038/S038.go
@@ -0,0 +1,8 @@
+package S038
+
+import (
+	"github.com/bflad/tfproviderlint/helper/analysisutils"
+	"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
+)
+
+var Analyzer = analysisutils.SchemaAttributeReferencesSemanticsAnalyzer("S038", schema.SchemaFieldAtLeastOneOf)
diff --git a/passes/S038/S038_test.go b/passes/S038/S038_test.go
@@ -0,0 +1,12 @@
+package S038
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestS038(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, Analyzer, "a")
+}
diff --git a/passes/S038/testdata/src/a/alias.go b/passes/S038/testdata/src/a/alias.go
@@ -0,0 +1,26 @@
+package a
+
+import (
+	s "github.com/hashicorp/terraform-plugin-sdk/helper/schema"
+)
+
+func falias() {
+	_ = &s.Resource{
+		Read: func(*s.ResourceData, interface{}) error { return nil },
+		Schema: map[string]*s.Schema{
+			"x": {
+				Type: s.TypeList,
+				Elem: &s.Resource{
+					Schema: map[string]*s.Schema{
+						"foo": {
+							AtLeastOneOf: []string{"x.0.bar"}, // want `S038: invalid AtLeastOneOf attribute reference semantics: "x.0" configuration block attribute references are only valid for TypeList and MaxItems: 1 attributes`
+						},
+						"bar": {
+							AtLeastOneOf: []string{"x.0.foo"}, // want `S038: invalid AtLeastOneOf attribute reference semantics: "x.0" configuration block attribute references are only valid for TypeList and MaxItems: 1 attributes`
+						},
+					},
+				},
+			},
+		},
+	}
+}