diff --git a/README.md b/README.md index 747e6119..f2b6b9df 100644 --- a/README.md +++ b/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 diff --git a/cmd/tfproviderlint/tfproviderlint.go b/cmd/tfproviderlint/tfproviderlint.go index e2bc8b0e..8f885f28 100644 --- a/cmd/tfproviderlint/tfproviderlint.go +++ b/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, ) } diff --git a/passes/S001/S001.go b/passes/S001/S001.go index 4a3dd52a..9397ebf4 100644 --- a/passes/S001/S001.go +++ b/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) } } diff --git a/passes/S001/testdata/src/a/alias.go b/passes/S001/testdata/src/a/alias.go index c0cf4397..bbfe13e4 100644 --- a/passes/S001/testdata/src/a/alias.go +++ b/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, } } diff --git a/passes/S001/testdata/src/a/comment_ignore.go b/passes/S001/testdata/src/a/comment_ignore.go index 86b295bd..b35a4495 100644 --- a/passes/S001/testdata/src/a/comment_ignore.go +++ b/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, } } diff --git a/passes/S001/testdata/src/a/main.go b/passes/S001/testdata/src/a/main.go index 460fc850..c113b6e0 100644 --- a/passes/S001/testdata/src/a/main.go +++ b/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}, } } diff --git a/passes/S001/testdata/src/a/outside_package.go b/passes/S001/testdata/src/a/outside_package.go index d9eb1a65..1826436b 100644 --- a/passes/S001/testdata/src/a/outside_package.go +++ b/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, } } diff --git a/passes/S006/S006.go b/passes/S006/S006.go new file mode 100644 index 00000000..aacfff33 --- /dev/null +++ b/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 +} diff --git a/passes/S006/S006_test.go b/passes/S006/S006_test.go new file mode 100644 index 00000000..ef50f4de --- /dev/null +++ b/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") +} diff --git a/passes/S006/testdata/src/a/alias.go b/passes/S006/testdata/src/a/alias.go new file mode 100644 index 00000000..c0cf4397 --- /dev/null +++ b/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, + } +} diff --git a/passes/S006/testdata/src/a/comment_ignore.go b/passes/S006/testdata/src/a/comment_ignore.go new file mode 100644 index 00000000..08c02d4a --- /dev/null +++ b/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, + } +} diff --git a/passes/S006/testdata/src/a/main.go b/passes/S006/testdata/src/a/main.go new file mode 100644 index 00000000..460fc850 --- /dev/null +++ b/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}, + } +} diff --git a/passes/S006/testdata/src/a/outside_package.go b/passes/S006/testdata/src/a/outside_package.go new file mode 100644 index 00000000..d9eb1a65 --- /dev/null +++ b/passes/S006/testdata/src/a/outside_package.go @@ -0,0 +1,11 @@ +package a + +import ( + "a/schema" +) + +func foutside() { + _ = schema.Schema{ + Type: schema.TypeMap, + } +} diff --git a/passes/S006/testdata/src/a/schema/schema.go b/passes/S006/testdata/src/a/schema/schema.go new file mode 100644 index 00000000..719e5625 --- /dev/null +++ b/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]] +} diff --git a/passes/S006/testdata/src/vendor b/passes/S006/testdata/src/vendor new file mode 120000 index 00000000..4a212190 --- /dev/null +++ b/passes/S006/testdata/src/vendor @@ -0,0 +1 @@ +../../../../vendor \ No newline at end of file