Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Jul 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions helper/astutils/package.go
Expand Up @@ -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)
}
42 changes: 39 additions & 3 deletions passes/R006/R006.go
Expand Up @@ -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"
Expand All @@ -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)
Expand All @@ -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
})

Expand Down
7 changes: 7 additions & 0 deletions passes/R006/R006_test.go
Expand Up @@ -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")
}
4 changes: 4 additions & 0 deletions passes/R006/README.md
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions 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)
}
@@ -0,0 +1,9 @@
package methodexpression

import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
)

type RetryError = resource.RetryError

var RetryableError = resource.RetryableError
9 changes: 9 additions & 0 deletions passes/R006/testdata/src/a/outside_package.go
@@ -0,0 +1,9 @@
package a

import (
"a/resource"
)

func foutside() *resource.RetryError {
return nil
}
3 changes: 3 additions & 0 deletions passes/R006/testdata/src/a/resource/resource.go
@@ -0,0 +1,3 @@
package resource

type RetryError struct{}
44 changes: 44 additions & 0 deletions 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(""))
}
44 changes: 44 additions & 0 deletions 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(""))
}
9 changes: 9 additions & 0 deletions 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)
}
@@ -0,0 +1,9 @@
package methodexpression

import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
)

type RetryError = resource.RetryError

var RetryableError = resource.RetryableError
1 change: 1 addition & 0 deletions passes/R006/testdata/src/packagealiases/vendor