diff --git a/helper/astutils/package.go b/helper/astutils/package.go index 1ce1d7f2..91aae941 100644 --- a/helper/astutils/package.go +++ b/helper/astutils/package.go @@ -293,6 +293,10 @@ func IsStdlibPackageType(t types.Type, packagePath string, typeName string) bool func isModulePackagePath(module string, packageSuffix string, path string) bool { // Only check end of path due to vendoring + if packageSuffix == "" { + return strings.HasSuffix(path, module) + } + r := regexp.MustCompile(fmt.Sprintf("%s(/v[1-9][0-9]*)?/%s$", module, packageSuffix)) return r.MatchString(path) } diff --git a/passes/R006/R006.go b/passes/R006/R006.go index da27a3e6..b66b59d5 100644 --- a/passes/R006/R006.go +++ b/passes/R006/R006.go @@ -3,8 +3,12 @@ package R006 import ( + "flag" "go/ast" + "go/types" + "strings" + "github.com/bflad/tfproviderlint/helper/astutils" "github.com/bflad/tfproviderlint/helper/terraformtype/helper/resource" "github.com/bflad/tfproviderlint/passes/commentignore" "github.com/bflad/tfproviderlint/passes/helper/resource/retryfuncinfo" @@ -14,13 +18,28 @@ import ( const Doc = `check for RetryFunc that omit retryable errors The R006 analyzer reports when RetryFunc declarations are missing -retryable errors and should not be used as RetryFunc.` +retryable errors and should not be used as RetryFunc. + +Optional parameters: + - package-aliases Comma-separated list of additional Go import paths to consider as aliases for helper/resource, defaults to none. +` const analyzerName = "R006" +var ( + packageAliases string +) + +func parseFlags() flag.FlagSet { + var flags = flag.NewFlagSet(analyzerName, flag.ExitOnError) + flags.StringVar(&packageAliases, "package-aliases", "", "Comma-separated list of additional Go import paths to consider as aliases for helper/resource") + return *flags +} + var Analyzer = &analysis.Analyzer{ - Name: analyzerName, - Doc: Doc, + Name: analyzerName, + Doc: Doc, + Flags: parseFlags(), Requires: []*analysis.Analyzer{ commentignore.Analyzer, retryfuncinfo.Analyzer, @@ -28,6 +47,18 @@ var Analyzer = &analysis.Analyzer{ Run: run, } +func isPackageAliasIgnored(e ast.Expr, info *types.Info, packageAliasesList string) bool { + packageAliases := strings.Split(packageAliasesList, ",") + + for _, packageAlias := range packageAliases { + if astutils.IsModulePackageFunc(e, info, packageAlias, "", resource.FuncNameRetryableError) { + return true + } + } + + return false +} + func run(pass *analysis.Pass) (interface{}, error) { ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer) retryFuncs := pass.ResultOf[retryfuncinfo.Analyzer].([]*resource.RetryFuncInfo) @@ -51,6 +82,11 @@ func run(pass *analysis.Pass) (interface{}, error) { return false } + if packageAliases != "" && isPackageAliasIgnored(callExpr.Fun, pass.TypesInfo, packageAliases) { + retryableErrorFound = true + return false + } + return true }) diff --git a/passes/R006/R006_test.go b/passes/R006/R006_test.go index 526bae78..261ccf28 100644 --- a/passes/R006/R006_test.go +++ b/passes/R006/R006_test.go @@ -11,3 +11,10 @@ func TestR006(t *testing.T) { testdata := analysistest.TestData() analysistest.Run(t, testdata, R006.Analyzer, "a") } + +func TestR006PackageAliases(t *testing.T) { + testdata := analysistest.TestData() + analyzer := R006.Analyzer + analyzer.Flags.Set("package-aliases", "a/methodexpression") + analysistest.Run(t, testdata, analyzer, "packagealiases") +} diff --git a/passes/R006/README.md b/passes/R006/README.md index 4afb2997..af744156 100644 --- a/passes/R006/README.md +++ b/passes/R006/README.md @@ -2,6 +2,10 @@ The R006 analyzer reports when `RetryFunc` declarations are missing retryable errors (e.g. `RetryableError()` calls) and should not be used as `RetryFunc`. +Optional parameters: + +- `-package-aliases` Comma-separated list of additional Go import paths to consider as aliases for helper/resource, defaults to none. + ## Flagged Code ```go diff --git a/passes/R006/testdata/src/a/method_expression.go b/passes/R006/testdata/src/a/method_expression.go new file mode 100644 index 00000000..61f2c4ba --- /dev/null +++ b/passes/R006/testdata/src/a/method_expression.go @@ -0,0 +1,9 @@ +package a + +import ( + "a/methodexpression" +) + +func fmethodexpression() *methodexpression.RetryError { // want "RetryFunc should include RetryableError\\(\\) handling or be removed" + return methodexpression.RetryableError(nil) +} diff --git a/passes/R006/testdata/src/a/methodexpression/methodexpression.go b/passes/R006/testdata/src/a/methodexpression/methodexpression.go new file mode 100644 index 00000000..60417b1a --- /dev/null +++ b/passes/R006/testdata/src/a/methodexpression/methodexpression.go @@ -0,0 +1,9 @@ +package methodexpression + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +type RetryError = resource.RetryError + +var RetryableError = resource.RetryableError diff --git a/passes/R006/testdata/src/a/outside_package.go b/passes/R006/testdata/src/a/outside_package.go new file mode 100644 index 00000000..73f6d2e5 --- /dev/null +++ b/passes/R006/testdata/src/a/outside_package.go @@ -0,0 +1,9 @@ +package a + +import ( + "a/resource" +) + +func foutside() *resource.RetryError { + return nil +} diff --git a/passes/R006/testdata/src/a/resource/resource.go b/passes/R006/testdata/src/a/resource/resource.go new file mode 100644 index 00000000..6b614765 --- /dev/null +++ b/passes/R006/testdata/src/a/resource/resource.go @@ -0,0 +1,3 @@ +package resource + +type RetryError struct{} diff --git a/passes/R006/testdata/src/packagealiases/main.go b/passes/R006/testdata/src/packagealiases/main.go new file mode 100644 index 00000000..1cdc4da2 --- /dev/null +++ b/passes/R006/testdata/src/packagealiases/main.go @@ -0,0 +1,44 @@ +package a + +import ( + "errors" + "time" + + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" +) + +//lintignore:R006 +func commentIgnore() *resource.RetryError { + return nil +} + +func failingAnonymousRetryFunc() { + _ = resource.Retry(1*time.Minute, func() *resource.RetryError { // want "RetryFunc should include RetryableError\\(\\) handling or be removed" + return nil + }) +} + +func failingNoCallExpr() *resource.RetryError { // want "RetryFunc should include RetryableError\\(\\) handling or be removed" + return nil +} + +func failingOnlyNonRetryableError() *resource.RetryError { // want "RetryFunc should include RetryableError\\(\\) handling or be removed" + return resource.NonRetryableError(errors.New("")) +} + +func passingAnonymousRetryFunc() { + _ = resource.Retry(1*time.Minute, func() *resource.RetryError { + return resource.RetryableError(errors.New("")) + }) +} + +func passingMultipleCallExpr() *resource.RetryError { + _ = resource.RetryableError(errors.New("")) + _ = resource.NonRetryableError(errors.New("")) + + return nil +} + +func passingRetryableError() *resource.RetryError { + return resource.RetryableError(errors.New("")) +} diff --git a/passes/R006/testdata/src/packagealiases/main_v2.go b/passes/R006/testdata/src/packagealiases/main_v2.go new file mode 100644 index 00000000..4fedfdef --- /dev/null +++ b/passes/R006/testdata/src/packagealiases/main_v2.go @@ -0,0 +1,44 @@ +package a + +import ( + "errors" + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +//lintignore:R006 +func commentIgnore_v2() *resource.RetryError { + return nil +} + +func failingAnonymousRetryFunc_v2() { + _ = resource.Retry(1*time.Minute, func() *resource.RetryError { // want "RetryFunc should include RetryableError\\(\\) handling or be removed" + return nil + }) +} + +func failingNoCallExpr_v2() *resource.RetryError { // want "RetryFunc should include RetryableError\\(\\) handling or be removed" + return nil +} + +func failingOnlyNonRetryableError_v2() *resource.RetryError { // want "RetryFunc should include RetryableError\\(\\) handling or be removed" + return resource.NonRetryableError(errors.New("")) +} + +func passingAnonymousRetryFunc_v2() { + _ = resource.Retry(1*time.Minute, func() *resource.RetryError { + return resource.RetryableError(errors.New("")) + }) +} + +func passingMultipleCallExpr_v2() *resource.RetryError { + _ = resource.RetryableError(errors.New("")) + _ = resource.NonRetryableError(errors.New("")) + + return nil +} + +func passingRetryableError_v2() *resource.RetryError { + return resource.RetryableError(errors.New("")) +} diff --git a/passes/R006/testdata/src/packagealiases/method_expression.go b/passes/R006/testdata/src/packagealiases/method_expression.go new file mode 100644 index 00000000..f5525fc4 --- /dev/null +++ b/passes/R006/testdata/src/packagealiases/method_expression.go @@ -0,0 +1,9 @@ +package a + +import ( + "a/methodexpression" +) + +func fmethodexpression() *methodexpression.RetryError { + return methodexpression.RetryableError(nil) +} diff --git a/passes/R006/testdata/src/packagealiases/methodexpression/methodexpression.go b/passes/R006/testdata/src/packagealiases/methodexpression/methodexpression.go new file mode 100644 index 00000000..60417b1a --- /dev/null +++ b/passes/R006/testdata/src/packagealiases/methodexpression/methodexpression.go @@ -0,0 +1,9 @@ +package methodexpression + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +type RetryError = resource.RetryError + +var RetryableError = resource.RetryableError diff --git a/passes/R006/testdata/src/packagealiases/vendor b/passes/R006/testdata/src/packagealiases/vendor new file mode 120000 index 00000000..776800d2 --- /dev/null +++ b/passes/R006/testdata/src/packagealiases/vendor @@ -0,0 +1 @@ +../../../../../vendor \ No newline at end of file