passes/R006: Add -package-aliases flag (#239)

Certain providers may wish to shim `helper/resource` functionality to prevent multiple module version issues while upgrading. When using type aliasing for `RetryError` and method expressions for `RetryableError` (or a passthrough implementation), the `R006` pass cannot be satisfied due to the mismatched package naming of `RetryableError`. Instead, we can allow callers to declare their alias/shim package paths to additionally ignore reports. The only requirement is that the shimmed method expression or function name must still be `RetryableError`.

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)
 }

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,20 +18,47 @@ 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,
 	},
 	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
 		})
 

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")
+}

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

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)
+}

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

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

@@ -0,0 +1,9 @@
+package a
+
+import (
+	"a/resource"
+)
+
+func foutside() *resource.RetryError {
+	return nil
+}

passes/R006/testdata/src/a/resource/resource.go

@@ -0,0 +1,3 @@
+package resource
+
+type RetryError struct{}

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(""))
+}

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(""))
+}

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)
+}

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

passes/R006/testdata/src/packagealiases/vendor

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