Move TypeMap Elem check to S006, add S001 check for TypeList or TypeSet

README.md

@@ -77,11 +77,12 @@ For additional information about each check, you can run `tfproviderlint help NA
 
 | Check | Description | Type |
 |---|---|---|
-| S001 | check for `Schema` of `TypeMap` missing `Elem` | AST |
+| S001 | check for `Schema` of `TypeList` or `TypeSet` missing `Elem` | AST |
 | S002 | check for `Schema` with both `Required` and `Optional` enabled | AST |
 | S003 | check for `Schema` with both `Required` and `Computed` enabled | AST |
 | S004 | check for `Schema` with both `Required` and `Default` configured | AST |
 | S005 | check for `Schema` with both `Computed` and `Default` configured | AST |
+| S006 | check for `Schema` of `TypeMap` missing `Elem` | AST |
 
 ## Development and Testing
 

cmd/tfproviderlint/tfproviderlint.go

@@ -21,6 +21,7 @@ import (
 	"github.com/bflad/tfproviderlint/passes/S003"
 	"github.com/bflad/tfproviderlint/passes/S004"
 	"github.com/bflad/tfproviderlint/passes/S005"
+	"github.com/bflad/tfproviderlint/passes/S006"
 )
 
 func main() {
@@ -37,5 +38,6 @@ func main() {
 		S003.Analyzer,
 		S004.Analyzer,
 		S005.Analyzer,
+		S006.Analyzer,
 	)
 }

passes/S001/S001.go

@@ -1,5 +1,5 @@
 // Package S001 defines an Analyzer that checks for
-// Schema of TypeMap missing Elem
+// Schema of TypeList or TypeSet missing Elem
 package S001
 
 import (
@@ -13,11 +13,10 @@ import (
 	"github.com/bflad/tfproviderlint/passes/schemaschema"
 )
 
-const Doc = `check for Schema of TypeMap missing Elem
+const Doc = `check for Schema of TypeList or TypeSet missing Elem
 
-The S001 analyzer reports cases of TypeMap schemas missing Elem,
-which currently passes Terraform schema validation, but breaks downstream tools
-and may be required in the future.`
+The S001 analyzer reports cases of TypeList or TypeSet schemas missing Elem,
+which will fail schema validation.`
 
 const analyzerName = "S001"
 
@@ -39,8 +38,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			continue
 		}
 
-		var elemFound bool
-		var typeMap bool
+		var elemFound, typeListOrSet bool
 
 		for _, elt := range schema.Elts {
 			switch v := elt.(type) {
@@ -63,7 +61,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 					continue
 				case *ast.SelectorExpr:
 					// Use AST over TypesInfo here as schema uses ValueType
-					if v.Sel.Name != "TypeMap" {
+					if v.Sel.Name != "TypeList" && v.Sel.Name != "TypeSet" {
 						continue
 					}
 
@@ -76,14 +74,14 @@ func run(pass *analysis.Pass) (interface{}, error) {
 							continue
 						}
 
-						typeMap = true
+						typeListOrSet = true
 					}
 				}
 			}
 		}
 
-		if typeMap && !elemFound {
-			pass.Reportf(schema.Type.(*ast.SelectorExpr).Sel.Pos(), "%s: schema of TypeMap should include Elem", analyzerName)
+		if typeListOrSet && !elemFound {
+			pass.Reportf(schema.Type.(*ast.SelectorExpr).Sel.Pos(), "%s: schema of TypeList or TypeSet should include Elem", analyzerName)
 		}
 	}
 

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

@@ -5,7 +5,11 @@ import (
 )
 
 func falias() {
-	_ = s.Schema{ // want "schema of TypeMap should include Elem"
-		Type: s.TypeMap,
+	_ = s.Schema{ // want "schema of TypeList or TypeSet should include Elem"
+		Type: s.TypeList,
+	}
+
+	_ = s.Schema{ // want "schema of TypeList or TypeSet should include Elem"
+		Type: s.TypeSet,
 	}
 }

passes/S001/testdata/src/a/comment_ignore.go

@@ -5,10 +5,17 @@ import (
 )
 
 func fcommentignore() {
-	_ = schema.Schema{Type: schema.TypeMap} //lintignore:S001
+	_ = schema.Schema{Type: schema.TypeList} //lintignore:S001
+
+	_ = schema.Schema{Type: schema.TypeSet} //lintignore:S001
+
+	//lintignore:S001
+	_ = schema.Schema{
+		Type: schema.TypeList,
+	}
 
 	//lintignore:S001
 	_ = schema.Schema{
-		Type: schema.TypeMap,
+		Type: schema.TypeSet,
 	}
 }

passes/S001/testdata/src/a/main.go

@@ -5,12 +5,21 @@ import (
 )
 
 func f() {
-	_ = schema.Schema{ // want "schema of TypeMap should include Elem"
-		Type: schema.TypeMap,
+	_ = schema.Schema{ // want "schema of TypeList or TypeSet should include Elem"
+		Type: schema.TypeList,
+	}
+
+	_ = schema.Schema{ // want "schema of TypeList or TypeSet should include Elem"
+		Type: schema.TypeSet,
+	}
+
+	_ = schema.Schema{
+		Type: schema.TypeList,
+		Elem: &schema.Schema{Type: schema.TypeString},
 	}
 
 	_ = schema.Schema{
-		Type: schema.TypeMap,
+		Type: schema.TypeSet,
 		Elem: &schema.Schema{Type: schema.TypeString},
 	}
 }

passes/S001/testdata/src/a/outside_package.go

@@ -6,6 +6,10 @@ import (
 
 func foutside() {
 	_ = schema.Schema{
-		Type: schema.TypeMap,
+		Type: schema.TypeList,
+	}
+
+	_ = schema.Schema{
+		Type: schema.TypeSet,
 	}
 }

passes/S006/S006.go

@@ -0,0 +1,91 @@
+// Package S006 defines an Analyzer that checks for
+// Schema of TypeMap missing Elem
+package S006
+
+import (
+	"go/ast"
+	"go/types"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/bflad/tfproviderlint/passes/commentignore"
+	"github.com/bflad/tfproviderlint/passes/schemaschema"
+)
+
+const Doc = `check for Schema of TypeMap missing Elem
+
+The S006 analyzer reports cases of TypeMap schemas missing Elem,
+which currently passes Terraform schema validation, but breaks downstream tools
+and may be required in the future.`
+
+const analyzerName = "S006"
+
+var Analyzer = &analysis.Analyzer{
+	Name: analyzerName,
+	Doc:  Doc,
+	Requires: []*analysis.Analyzer{
+		schemaschema.Analyzer,
+		commentignore.Analyzer,
+	},
+	Run: run,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
+	schemas := pass.ResultOf[schemaschema.Analyzer].([]*ast.CompositeLit)
+	for _, schema := range schemas {
+		if ignorer.ShouldIgnore(analyzerName, schema) {
+			continue
+		}
+
+		var elemFound bool
+		var typeMap bool
+
+		for _, elt := range schema.Elts {
+			switch v := elt.(type) {
+			default:
+				continue
+			case *ast.KeyValueExpr:
+				name := v.Key.(*ast.Ident).Name
+
+				if name == "Elem" {
+					elemFound = true
+					continue
+				}
+
+				if name != "Type" {
+					continue
+				}
+
+				switch v := v.Value.(type) {
+				default:
+					continue
+				case *ast.SelectorExpr:
+					// Use AST over TypesInfo here as schema uses ValueType
+					if v.Sel.Name != "TypeMap" {
+						continue
+					}
+
+					switch t := pass.TypesInfo.TypeOf(v).(type) {
+					default:
+						continue
+					case *types.Named:
+						// HasSuffix here due to vendoring
+						if !strings.HasSuffix(t.Obj().Pkg().Path(), "github.com/hashicorp/terraform/helper/schema") {
+							continue
+						}
+
+						typeMap = true
+					}
+				}
+			}
+		}
+
+		if typeMap && !elemFound {
+			pass.Reportf(schema.Type.(*ast.SelectorExpr).Sel.Pos(), "%s: schema of TypeMap should include Elem", analyzerName)
+		}
+	}
+
+	return nil, nil
+}

passes/S006/S006_test.go

@@ -0,0 +1,14 @@
+package S006_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/bflad/tfproviderlint/passes/S006"
+)
+
+func TestS006(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, S006.Analyzer, "a")
+}

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

@@ -0,0 +1,11 @@
+package a
+
+import (
+	s "github.com/hashicorp/terraform/helper/schema"
+)
+
+func falias() {
+	_ = s.Schema{ // want "schema of TypeMap should include Elem"
+		Type: s.TypeMap,
+	}
+}

passes/S006/testdata/src/a/comment_ignore.go

@@ -0,0 +1,14 @@
+package a
+
+import (
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+func fcommentignore() {
+	_ = schema.Schema{Type: schema.TypeMap} //lintignore:S006
+
+	//lintignore:S006
+	_ = schema.Schema{
+		Type: schema.TypeMap,
+	}
+}

passes/S006/testdata/src/a/main.go

@@ -0,0 +1,16 @@
+package a
+
+import (
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+func f() {
+	_ = schema.Schema{ // want "schema of TypeMap should include Elem"
+		Type: schema.TypeMap,
+	}
+
+	_ = schema.Schema{
+		Type: schema.TypeMap,
+		Elem: &schema.Schema{Type: schema.TypeString},
+	}
+}

passes/S006/testdata/src/a/outside_package.go

@@ -0,0 +1,11 @@
+package a
+
+import (
+	"a/schema"
+)
+
+func foutside() {
+	_ = schema.Schema{
+		Type: schema.TypeMap,
+	}
+}

passes/S006/testdata/src/a/schema/schema.go

@@ -0,0 +1,33 @@
+package schema
+
+import (
+	"strconv"
+)
+
+type Schema struct {
+	Type ValueType
+}
+
+type ValueType int
+
+const (
+	TypeInvalid ValueType = iota
+	TypeBool
+	TypeInt
+	TypeFloat
+	TypeString
+	TypeList
+	TypeMap
+	TypeSet
+)
+
+const _ValueType_name = "TypeInvalidTypeBoolTypeIntTypeFloatTypeStringTypeListTypeMapTypeSettypeObject"
+
+var _ValueType_index = [...]uint8{0, 11, 19, 26, 35, 45, 53, 60, 67, 77}
+
+func (i ValueType) String() string {
+	if i < 0 || i >= ValueType(len(_ValueType_index)-1) {
+		return "ValueType(" + strconv.FormatInt(int64(i), 10) + ")"
+	}
+	return _ValueType_name[_ValueType_index[i]:_ValueType_index[i+1]]
+}

passes/S006/testdata/src/vendor

@@ -0,0 +1 @@
+../../../../vendor