diff --git a/awsproviderlint/README.md b/awsproviderlint/README.md index 1cc7ca7550ec..773dc8f76f9c 100644 --- a/awsproviderlint/README.md +++ b/awsproviderlint/README.md @@ -28,6 +28,12 @@ The `awsproviderlint` tool extends the `tfproviderlint` tool and its checks. See | [AWSR001](passes/AWSR001/README.md) | check for `fmt.Sprintf()` calls using `.amazonaws.com` domain suffix | | [AWSR002](passes/AWSR002/README.md) | check for `d.Set()` of `tags` attribute that should include `IgnoreConfig()` | +### AWS Validation Checks + +| Check | Description | +|---|---| +| [AWSV001](passes/AWSV001) | check for `validation.StringInSlice()` calls using `[]string` parameter | + ## Development and Testing This project is built on the [`tfproviderlint`](https://github.com/bflad/tfproviderlint) project and the [`go/analysis`](https://godoc.org/golang.org/x/tools/go/analysis) framework. diff --git a/awsproviderlint/passes/AWSR001/AWSR001.go b/awsproviderlint/passes/AWSR001/AWSR001.go index 5027fe26e532..3aa2f39a67d9 100644 --- a/awsproviderlint/passes/AWSR001/AWSR001.go +++ b/awsproviderlint/passes/AWSR001/AWSR001.go @@ -6,7 +6,7 @@ import ( "github.com/bflad/tfproviderlint/helper/astutils" "github.com/bflad/tfproviderlint/passes/commentignore" - "github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/fmtsprintfcallexpr" + "github.com/bflad/tfproviderlint/passes/stdlib/fmtsprintfcallexpr" "golang.org/x/tools/go/analysis" ) diff --git a/awsproviderlint/passes/AWSV001/AWSV001.go b/awsproviderlint/passes/AWSV001/AWSV001.go new file mode 100644 index 000000000000..91c09b3fe952 --- /dev/null +++ b/awsproviderlint/passes/AWSV001/AWSV001.go @@ -0,0 +1,69 @@ +package AWSV001 + +import ( + "go/ast" + + "github.com/bflad/tfproviderlint/helper/astutils" + "github.com/bflad/tfproviderlint/passes/commentignore" + "github.com/bflad/tfproviderlint/passes/helper/validation/stringinslicecallexpr" + "golang.org/x/tools/go/analysis" +) + +const Doc = `check for validation.StringInSlice() using []string parameter + +The AWSV001 analyzer reports when a validation.StringInSlice() call has the +first parameter of a []string, which suggests either that AWS API model +constants are not available or that the usage is prior to the AWS Go SDK adding +functions that return all values for the enumeration type. + +If the API model constants are not available, this check can be ignored but it +is recommended to submit an AWS Support case to the AWS service team for adding +the constants. + +If the hardcoded strings are AWS Go SDK constants, this check reports when the +first parameter should be switched to the newer ENUM_Values() function. +` + +const analyzerName = "AWSV001" + +var Analyzer = &analysis.Analyzer{ + Name: analyzerName, + Doc: Doc, + Requires: []*analysis.Analyzer{ + commentignore.Analyzer, + stringinslicecallexpr.Analyzer, + }, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + callExprs := pass.ResultOf[stringinslicecallexpr.Analyzer].([]*ast.CallExpr) + commentIgnorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer) + + for _, callExpr := range callExprs { + if commentIgnorer.ShouldIgnore(analyzerName, callExpr) { + continue + } + + if len(callExpr.Args) < 2 { + continue + } + + ast.Inspect(callExpr.Args[0], func(n ast.Node) bool { + compositeLit, ok := n.(*ast.CompositeLit) + + if !ok { + return true + } + + if astutils.IsExprTypeArrayString(compositeLit.Type) { + pass.Reportf(callExpr.Args[0].Pos(), "%s: prefer AWS Go SDK ENUM_Values() function (ignore if not applicable)", analyzerName) + return false + } + + return true + }) + } + + return nil, nil +} diff --git a/awsproviderlint/passes/AWSV001/AWSV001_test.go b/awsproviderlint/passes/AWSV001/AWSV001_test.go new file mode 100644 index 000000000000..9d6db98b2030 --- /dev/null +++ b/awsproviderlint/passes/AWSV001/AWSV001_test.go @@ -0,0 +1,12 @@ +package AWSV001 + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAWSV001(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "a") +} diff --git a/awsproviderlint/passes/AWSV001/README.md b/awsproviderlint/passes/AWSV001/README.md new file mode 100644 index 000000000000..630e4072ed51 --- /dev/null +++ b/awsproviderlint/passes/AWSV001/README.md @@ -0,0 +1,38 @@ +# AWSV001 + +The `AWSV001` analyzer reports when a `validation.StringInSlice()` call has the first parameter of a `[]string`, which suggests either that AWS API model constants are not available or that the usage is prior to the AWS Go SDK adding functions that return all values for the enumeration type. + +If the API model constants are not available, this check can be ignored but it is recommended to submit an AWS Support case to the AWS service team for adding the constants. + +If the elements of the string slice are AWS Go SDK constants, this check reports when the parameter should be switched to the newer AWS Go SDK `ENUM_Values()` function. + +## Flagged Code + +```go +&schema.Schema{ + ValidateFunc: validation.StringInSlice([]string{ + service.EnumTypeExample1, + service.EnumTypeExample2, + }, false), +} +``` + +## Passing Code + +```go +&schema.Schema{ + ValidateFunc: validation.StringInSlice(service.EnumType_Values(), false), +} +``` + +## Ignoring Check + +The check can be ignored for a certain line via a `//lintignore:AWSV001` comment on the previous line or at the end of the offending line, e.g. + +```go +//lintignore:AWSV001 +ValidateFunc: validation.StringInSlice([]string{ + service.EnumTypeExample1, + service.EnumTypeExample2, +}, false), +``` diff --git a/awsproviderlint/passes/AWSV001/testdata/src/a/main.go b/awsproviderlint/passes/AWSV001/testdata/src/a/main.go new file mode 100644 index 000000000000..e7abdf179f8b --- /dev/null +++ b/awsproviderlint/passes/AWSV001/testdata/src/a/main.go @@ -0,0 +1,32 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" +) + +func f() { + testSlice := []string{"test"} + + /* Passing cases */ + + _ = validation.StringInSlice(testSlice, false) + + _ = validation.StringInSlice(testSlice, true) + + _ = validation.StringInSlice(testFunc(), false) + + /* Comment ignored cases */ + + //lintignore:AWSV001 + _ = validation.StringInSlice([]string{"test"}, false) + + _ = validation.StringInSlice([]string{"test"}, false) //lintignore:AWSV001 + + /* Failing cases */ + + _ = validation.StringInSlice([]string{"test"}, false) // want "prefer AWS Go SDK ENUM_Values\\(\\) function" +} + +func testFunc() []string { + return []string{"test"} +} diff --git a/awsproviderlint/passes/AWSV001/testdata/src/a/vendor b/awsproviderlint/passes/AWSV001/testdata/src/a/vendor new file mode 120000 index 000000000000..ce7b6d9c07fa --- /dev/null +++ b/awsproviderlint/passes/AWSV001/testdata/src/a/vendor @@ -0,0 +1 @@ +../../../../../../vendor \ No newline at end of file diff --git a/awsproviderlint/passes/checks.go b/awsproviderlint/passes/checks.go index 4a8f81410c73..d9e4ee0052a0 100644 --- a/awsproviderlint/passes/checks.go +++ b/awsproviderlint/passes/checks.go @@ -9,6 +9,7 @@ import ( "github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSAT006" "github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSR001" "github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSR002" + "github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSV001" "golang.org/x/tools/go/analysis" ) @@ -21,4 +22,5 @@ var AllChecks = []*analysis.Analyzer{ AWSAT006.Analyzer, AWSR001.Analyzer, AWSR002.Analyzer, + AWSV001.Analyzer, } diff --git a/awsproviderlint/passes/fmtsprintfcallexpr/fmtsprintfcallexpr.go b/awsproviderlint/passes/fmtsprintfcallexpr/fmtsprintfcallexpr.go deleted file mode 100644 index 231ceabd29c3..000000000000 --- a/awsproviderlint/passes/fmtsprintfcallexpr/fmtsprintfcallexpr.go +++ /dev/null @@ -1,55 +0,0 @@ -package fmtsprintfcallexpr - -import ( - "go/ast" - "go/types" - "reflect" - - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" - "golang.org/x/tools/go/ast/inspector" -) - -var Analyzer = &analysis.Analyzer{ - Name: "fmtsprintfcallexpr", - Doc: "find fmt.Sprintf() *ast.CallExpr for later passes", - Requires: []*analysis.Analyzer{ - inspect.Analyzer, - }, - Run: run, - ResultType: reflect.TypeOf([]*ast.CallExpr{}), -} - -func run(pass *analysis.Pass) (interface{}, error) { - inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) - nodeFilter := []ast.Node{ - (*ast.CallExpr)(nil), - } - var result []*ast.CallExpr - - inspect.Preorder(nodeFilter, func(n ast.Node) { - callExpr := n.(*ast.CallExpr) - - switch fun := callExpr.Fun.(type) { - case *ast.SelectorExpr: - if fun.Sel.Name != "Sprintf" { - return - } - - switch x := fun.X.(type) { - case *ast.Ident: - if pass.TypesInfo.ObjectOf(x).(*types.PkgName).Imported().Path() != "fmt" { - return - } - default: - return - } - default: - return - } - - result = append(result, callExpr) - }) - - return result, nil -} diff --git a/awsproviderlint/passes/fmtsprintfcallexpr/fmtsprintfcallexpr_test.go b/awsproviderlint/passes/fmtsprintfcallexpr/fmtsprintfcallexpr_test.go deleted file mode 100644 index 9c0683f658a2..000000000000 --- a/awsproviderlint/passes/fmtsprintfcallexpr/fmtsprintfcallexpr_test.go +++ /dev/null @@ -1,15 +0,0 @@ -package fmtsprintfcallexpr - -import ( - "testing" - - "golang.org/x/tools/go/analysis" -) - -func TestValidateAnalyzer(t *testing.T) { - err := analysis.Validate([]*analysis.Analyzer{Analyzer}) - - if err != nil { - t.Fatal(err) - } -} diff --git a/vendor/github.com/bflad/tfproviderlint/passes/helper/validation/stringinslicecallexpr/stringinslicecallexpr.go b/vendor/github.com/bflad/tfproviderlint/passes/helper/validation/stringinslicecallexpr/stringinslicecallexpr.go new file mode 100644 index 000000000000..447d5b9e2dab --- /dev/null +++ b/vendor/github.com/bflad/tfproviderlint/passes/helper/validation/stringinslicecallexpr/stringinslicecallexpr.go @@ -0,0 +1,13 @@ +package stringinslicecallexpr + +import ( + "github.com/bflad/tfproviderlint/helper/analysisutils" + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/validation" +) + +var Analyzer = analysisutils.FunctionCallExprAnalyzer( + "stringinslicecallexpr", + validation.IsFunc, + validation.PackagePath, + validation.FuncNameStringInSlice, +) diff --git a/vendor/github.com/bflad/tfproviderlint/passes/stdlib/fmtsprintfcallexpr/fmtsprintfcallexpr.go b/vendor/github.com/bflad/tfproviderlint/passes/stdlib/fmtsprintfcallexpr/fmtsprintfcallexpr.go new file mode 100644 index 000000000000..07f42f70546c --- /dev/null +++ b/vendor/github.com/bflad/tfproviderlint/passes/stdlib/fmtsprintfcallexpr/fmtsprintfcallexpr.go @@ -0,0 +1,11 @@ +package fmtsprintfcallexpr + +import ( + "github.com/bflad/tfproviderlint/helper/analysisutils" +) + +var Analyzer = analysisutils.StdlibFunctionCallExprAnalyzer( + "fmtsprintfcallexpr", + "fmt", + "Sprintf", +) diff --git a/vendor/modules.txt b/vendor/modules.txt index cfa6e1e8b5b1..68e3e5032b61 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -344,10 +344,12 @@ github.com/bflad/tfproviderlint/passes/helper/validation/iprangecallexpr github.com/bflad/tfproviderlint/passes/helper/validation/iprangeselectorexpr github.com/bflad/tfproviderlint/passes/helper/validation/singleipcallexpr github.com/bflad/tfproviderlint/passes/helper/validation/singleipselectorexpr +github.com/bflad/tfproviderlint/passes/helper/validation/stringinslicecallexpr github.com/bflad/tfproviderlint/passes/helper/validation/validatejsonstringselectorexpr github.com/bflad/tfproviderlint/passes/helper/validation/validatelistuniquestringsselectorexpr github.com/bflad/tfproviderlint/passes/helper/validation/validateregexpselectorexpr github.com/bflad/tfproviderlint/passes/helper/validation/validaterfc3339timestringselectorexpr +github.com/bflad/tfproviderlint/passes/stdlib/fmtsprintfcallexpr github.com/bflad/tfproviderlint/passes/testaccfuncdecl github.com/bflad/tfproviderlint/passes/testfuncdecl github.com/bflad/tfproviderlint/version