New Checks: S035 through S037 for invalid attribute references (#105)

Reference: bflad/tfproviderlint#104

CHANGELOG.md

@@ -1,3 +1,15 @@
+# v0.12.0
+
+BREAKING CHANGES
+
+* helper/astutils: The `IsFunctionParameterType*` functions have been renamed `IsExprType*` to better denote their purpose.
+
+FEATURES
+
+* **New Check:** `S035`: check for `Schema` with invalid `AtLeastOneOf` attribute references
+* **New Check:** `S036`: check for `Schema` with invalid `ConflictsWith` attribute references
+* **New Check:** `S037`: check for `Schema` with invalid `ExactlyOneOf` attribute references
+
 # v0.11.0
 
 ENHANCEMENTS

README.md

@@ -140,6 +140,9 @@ Standard lint checks are enabled by default in the `tfproviderlint` tool. Opt-in
 | [S032](passes/S032/README.md) | check for `Schema` of `Computed` only with `MinItems` configured | AST |
 | [S033](passes/S033/README.md) | check for `Schema` of `Computed` only with `StateFunc` configured | AST |
 | [S034](passes/S034/README.md) | check for `Schema` that configure `PromoteSingle` | AST |
+| [S035](passes/S035/README.md) | check for `Schema` with invalid `AtLeastOneOf` attribute references | AST |
+| [S036](passes/S036/README.md) | check for `Schema` with invalid `ConflictsWith` attribute references | AST |
+| [S037](passes/S037/README.md) | check for `Schema` with invalid `ExactlyOneOf` attribute references | AST |
 
 ### Standard Validation Checks
 

helper/analysisutils/schema_analyzers.go

@@ -0,0 +1,29 @@
+package analysisutils
+
+import (
+	"fmt"
+
+	"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
+
+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
+references to verify the references against the full schema.
+`, analyzerName, fieldName)
+
+	return &analysis.Analyzer{
+		Name: analyzerName,
+		Doc:  doc,
+		Requires: []*analysis.Analyzer{
+			commentignore.Analyzer,
+			schemainfo.Analyzer,
+		},
+		Run: SchemaAttributeReferencesRunner(analyzerName, fieldName),
+	}
+}

helper/analysisutils/schema_runners.go

@@ -0,0 +1,50 @@
+package analysisutils
+
+import (
+	"go/ast"
+
+	"github.com/bflad/tfproviderlint/helper/astutils"
+	"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
+	"github.com/bflad/tfproviderlint/passes/commentignore"
+	"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfo"
+	"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) {
+	return func(pass *analysis.Pass) (interface{}, error) {
+		ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
+		schemaInfos := pass.ResultOf[schemainfo.Analyzer].([]*schema.SchemaInfo)
+
+		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.ParseAttributeReference(*attributeReference); err != nil {
+						pass.Reportf(elt.Pos(), "%s: invalid %s attribute reference: %s", analyzerName, fieldName, err)
+					}
+				}
+			}
+		}
+
+		return nil, nil
+	}
+}

helper/astutils/expr.go

@@ -0,0 +1,40 @@
+package astutils
+
+import (
+	"go/ast"
+)
+
+// IsExprTypeArrayString returns true if the expression matches []string
+func IsExprTypeArrayString(e ast.Expr) bool {
+	arrayType, ok := e.(*ast.ArrayType)
+
+	return ok && IsExprTypeString(arrayType.Elt)
+}
+
+// IsExprTypeArrayError returns true if the expression matches []error
+func IsExprTypeArrayError(e ast.Expr) bool {
+	arrayType, ok := e.(*ast.ArrayType)
+
+	return ok && IsExprTypeError(arrayType.Elt)
+}
+
+// IsExprTypeError returns true if the expression matches string
+func IsExprTypeError(e ast.Expr) bool {
+	ident, ok := e.(*ast.Ident)
+
+	return ok && ident.Name == "error"
+}
+
+// IsExprTypeInterface returns true if the expression matches interface{}
+func IsExprTypeInterface(e ast.Expr) bool {
+	_, ok := e.(*ast.InterfaceType)
+
+	return ok
+}
+
+// IsExprTypeString returns true if the expression matches string
+func IsExprTypeString(e ast.Expr) bool {
+	ident, ok := e.(*ast.Ident)
+
+	return ok && ident.Name == "string"
+}

helper/terraformtype/helper/schema/attributes.go

@@ -0,0 +1,61 @@
+package schema
+
+import (
+	"fmt"
+	"math"
+	"regexp"
+	"strconv"
+	"strings"
+)
+
+const (
+	// Pattern for schema attribute names
+	AttributeNameRegexpPattern = `^[a-z0-9_]+$`
+
+	// Pattern for schema references to attributes, such as ConflictsWith values
+	AttributeReferenceRegexpPattern = `^[a-z0-9_]+(\.[a-z0-9_]+)*$`
+)
+
+var (
+	AttributeNameRegexp      = regexp.MustCompile(AttributeNameRegexpPattern)
+	AttributeReferenceRegexp = regexp.MustCompile(AttributeReferenceRegexpPattern)
+)
+
+// ParseAttributeReference validates and returns the split representation of schema attribute reference.
+// Attribute references are used in Schema fields such as AtLeastOneOf, ConflictsWith, and ExactlyOneOf.
+func ParseAttributeReference(reference string) ([]string, error) {
+	if !AttributeReferenceRegexp.MatchString(reference) {
+		return nil, fmt.Errorf("%q must contain only valid attribute names, separated by periods", reference)
+	}
+
+	attributeReferenceParts := strings.Split(reference, ".")
+
+	if len(attributeReferenceParts) == 1 {
+		return attributeReferenceParts, nil
+	}
+
+	configurationBlockReferenceErr := fmt.Errorf("%q configuration block attribute references are only valid for TypeList and MaxItems: 1 attributes and nested attributes must be separated by .0.", reference)
+
+	if math.Mod(float64(len(attributeReferenceParts)), 2) == 0 {
+		return attributeReferenceParts, configurationBlockReferenceErr
+	}
+
+	// All even parts of an attribute reference must be 0
+	for idx, attributeReferencePart := range attributeReferenceParts {
+		if math.Mod(float64(idx), 2) == 0 {
+			continue
+		}
+
+		attributeReferencePartInt, err := strconv.Atoi(attributeReferencePart)
+
+		if err != nil {
+			return attributeReferenceParts, configurationBlockReferenceErr
+		}
+
+		if attributeReferencePartInt != 0 {
+			return attributeReferenceParts, configurationBlockReferenceErr
+		}
+	}
+
+	return attributeReferenceParts, nil
+}

passes/S015/S015.go

@@ -5,7 +5,6 @@ package S015
 
 import (
 	"go/ast"
-	"regexp"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
@@ -37,8 +36,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
 	schemamapcompositelits := pass.ResultOf[schemamapcompositelit.Analyzer].([]*ast.CompositeLit)
 
-	attributeNameRegex := regexp.MustCompile(`^[a-z0-9_]+$`)
-
 	for _, smap := range schemamapcompositelits {
 		if ignorer.ShouldIgnore(analyzerName, smap) {
 			continue
@@ -51,7 +48,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			case *ast.BasicLit:
 				value := strings.Trim(t.Value, `"`)
 
-				if !attributeNameRegex.MatchString(value) {
+				if !schema.AttributeNameRegexp.MatchString(value) {
 					pass.Reportf(t.Pos(), "%s: schema attribute names should only be lowercase alphanumeric characters or underscores", analyzerName)
 				}
 			}

passes/S035/README.md

@@ -0,0 +1,32 @@
+# S035
+
+The S035 analyzer reports cases of Schemas which include `AtLeastOneOf` and have invalid schema attribute references.
+
+NOTE: This only verifies the syntax of attribute references. The Terraform Plugin SDK can unit test attribute references to verify the references against the full schema.
+
+## Flagged Code
+
+```go
+&schema.Schema{
+    AtLeastOneOf: []string{"config_block_attr.nested_attr"},
+}
+```
+
+## Passing Code
+
+```go
+&schema.Schema{
+    AtLeastOneOf: []string{"config_block_attr.0.nested_attr"},
+}
+```
+
+## Ignoring Reports
+
+Singular reports can be ignored by adding the a `//lintignore:S035` Go code comment at the end of the offending line or on the line immediately proceding, e.g.
+
+```go
+//lintignore:S035
+&schema.Schema{
+    AtLeastOneOf: []string{"config_block_attr.nested_attr"},
+}
+```

passes/S035/S035.go

@@ -0,0 +1,8 @@
+package S035
+
+import (
+	"github.com/bflad/tfproviderlint/helper/analysisutils"
+	"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
+)
+
+var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S035", schema.SchemaFieldAtLeastOneOf)

passes/S035/S035_test.go

@@ -0,0 +1,12 @@
+package S035
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestS035(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, Analyzer, "a")
+}

passes/S035/testdata/src/a/alias.go

@@ -0,0 +1,21 @@
+package a
+
+import (
+	s "github.com/hashicorp/terraform-plugin-sdk/helper/schema"
+)
+
+func falias() {
+	_ = s.Schema{
+		AtLeastOneOf: []string{
+			".invalidreference", // want "invalid AtLeastOneOf attribute reference"
+		},
+	}
+
+	_ = map[string]*s.Schema{
+		"name": {
+			AtLeastOneOf: []string{
+				".invalidreference", // want "invalid AtLeastOneOf attribute reference"
+			},
+		},
+	}
+}