From 38150df115d8ec9bf8438fdef73c181c23e15bd4 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 11 Aug 2020 22:07:26 -0400 Subject: [PATCH] passes: New Checks for common unstable d.SetId() values Reference: https://github.com/bflad/tfproviderlint/issues/191 Reference: https://github.com/bflad/tfproviderlint/issues/192 --- CHANGELOG.md | 7 +++ README.md | 3 + helper/terraformtype/helper/resource/funcs.go | 2 + passes/R015/R015.go | 58 +++++++++++++++++++ passes/R015/R015_test.go | 14 +++++ passes/R015/README.md | 24 ++++++++ passes/R015/testdata/src/a/alias.go | 14 +++++ passes/R015/testdata/src/a/alias_v2.go | 14 +++++ passes/R015/testdata/src/a/comment_ignore.go | 15 +++++ .../R015/testdata/src/a/comment_ignore_v2.go | 15 +++++ passes/R015/testdata/src/a/main.go | 24 ++++++++ passes/R015/testdata/src/a/main_v2.go | 24 ++++++++ passes/R015/testdata/src/a/outside_package.go | 12 ++++ .../R015/testdata/src/a/resource/resource.go | 5 ++ passes/R015/testdata/src/a/schema/schema.go | 7 +++ passes/R015/testdata/src/a/vendor | 1 + passes/R016/R016.go | 58 +++++++++++++++++++ passes/R016/R016_test.go | 14 +++++ passes/R016/README.md | 24 ++++++++ passes/R016/testdata/src/a/alias.go | 14 +++++ passes/R016/testdata/src/a/alias_v2.go | 14 +++++ passes/R016/testdata/src/a/comment_ignore.go | 15 +++++ .../R016/testdata/src/a/comment_ignore_v2.go | 15 +++++ passes/R016/testdata/src/a/main.go | 24 ++++++++ passes/R016/testdata/src/a/main_v2.go | 24 ++++++++ passes/R016/testdata/src/a/outside_package.go | 12 ++++ .../R016/testdata/src/a/resource/resource.go | 5 ++ passes/R016/testdata/src/a/schema/schema.go | 7 +++ passes/R016/testdata/src/a/vendor | 1 + passes/R017/R017.go | 58 +++++++++++++++++++ passes/R017/R017_test.go | 14 +++++ passes/R017/README.md | 24 ++++++++ passes/R017/testdata/src/a/alias.go | 15 +++++ passes/R017/testdata/src/a/alias_v2.go | 15 +++++ passes/R017/testdata/src/a/comment_ignore.go | 16 +++++ .../R017/testdata/src/a/comment_ignore_v2.go | 16 +++++ passes/R017/testdata/src/a/main.go | 25 ++++++++ passes/R017/testdata/src/a/main_v2.go | 25 ++++++++ passes/R017/testdata/src/a/outside_package.go | 12 ++++ passes/R017/testdata/src/a/schema/schema.go | 7 +++ passes/R017/testdata/src/a/time/time.go | 5 ++ passes/R017/testdata/src/a/vendor | 1 + passes/checks.go | 6 ++ .../resourcedatasetidcallexpr.go | 14 +++++ .../resourcedatasetidcallexpr_test.go | 15 +++++ 45 files changed, 734 insertions(+) create mode 100644 passes/R015/R015.go create mode 100644 passes/R015/R015_test.go create mode 100644 passes/R015/README.md create mode 100644 passes/R015/testdata/src/a/alias.go create mode 100644 passes/R015/testdata/src/a/alias_v2.go create mode 100644 passes/R015/testdata/src/a/comment_ignore.go create mode 100644 passes/R015/testdata/src/a/comment_ignore_v2.go create mode 100644 passes/R015/testdata/src/a/main.go create mode 100644 passes/R015/testdata/src/a/main_v2.go create mode 100644 passes/R015/testdata/src/a/outside_package.go create mode 100644 passes/R015/testdata/src/a/resource/resource.go create mode 100644 passes/R015/testdata/src/a/schema/schema.go create mode 120000 passes/R015/testdata/src/a/vendor create mode 100644 passes/R016/R016.go create mode 100644 passes/R016/R016_test.go create mode 100644 passes/R016/README.md create mode 100644 passes/R016/testdata/src/a/alias.go create mode 100644 passes/R016/testdata/src/a/alias_v2.go create mode 100644 passes/R016/testdata/src/a/comment_ignore.go create mode 100644 passes/R016/testdata/src/a/comment_ignore_v2.go create mode 100644 passes/R016/testdata/src/a/main.go create mode 100644 passes/R016/testdata/src/a/main_v2.go create mode 100644 passes/R016/testdata/src/a/outside_package.go create mode 100644 passes/R016/testdata/src/a/resource/resource.go create mode 100644 passes/R016/testdata/src/a/schema/schema.go create mode 120000 passes/R016/testdata/src/a/vendor create mode 100644 passes/R017/R017.go create mode 100644 passes/R017/R017_test.go create mode 100644 passes/R017/README.md create mode 100644 passes/R017/testdata/src/a/alias.go create mode 100644 passes/R017/testdata/src/a/alias_v2.go create mode 100644 passes/R017/testdata/src/a/comment_ignore.go create mode 100644 passes/R017/testdata/src/a/comment_ignore_v2.go create mode 100644 passes/R017/testdata/src/a/main.go create mode 100644 passes/R017/testdata/src/a/main_v2.go create mode 100644 passes/R017/testdata/src/a/outside_package.go create mode 100644 passes/R017/testdata/src/a/schema/schema.go create mode 100644 passes/R017/testdata/src/a/time/time.go create mode 120000 passes/R017/testdata/src/a/vendor create mode 100644 passes/helper/schema/resourcedatasetidcallexpr/resourcedatasetidcallexpr.go create mode 100644 passes/helper/schema/resourcedatasetidcallexpr/resourcedatasetidcallexpr_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 462a1ca0..234a513b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,16 @@ # v0.17.0 +FEATURES + +* **New Check:** `R015`: check for `(*schema.ResourceData).SetId()` receiver method usage with unstable `resource.UniqueId()` value +* **New Check:** `R016`: check for `(*schema.ResourceData).SetId()` receiver method usage with unstable `resource.PrefixedUniqueId()` value +* **New Check:** `R017`: check for `(*schema.ResourceData).SetId()` receiver method usage with unstable `time.Now()` value + ENHANCEMENTS * helper/analysisutils: Add `StdlibFunctionCallExprAnalyzer` with associated runner * helper/astutils: Support standard library handling equivalents (no vendoring) for package functions +* passes/helper/schema: Pass for collecting `(*schema.ResourceData).SetId()` calls * passes/stdlib: Pass for collecting `fmt.Sprintf()` calls # v0.16.0 diff --git a/README.md b/README.md index d4673296..7cdb9948 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,9 @@ Standard lint checks are enabled by default in the `tfproviderlint` tool. Opt-in | [R012](passes/R012/README.md) | check for data source `Resource` that configure `CustomizeDiff` | AST | | [R013](passes/R013/README.md) | check for `map[string]*Resource` that resource names contain at least one underscore | AST | | [R014](passes/R014/README.md) | check for `CreateFunc`, `DeleteFunc`, `ReadFunc`, and `UpdateFunc` parameter naming | AST | +| [R015](passes/R015/README.md) | check for `(*schema.ResourceData).SetId()` receiver method usage with unstable `resource.UniqueId()` value | AST | +| [R016](passes/R016/README.md) | check for `(*schema.ResourceData).SetId()` receiver method usage with unstable `resource.PrefixedUniqueId()` value | AST | +| [R017](passes/R017/README.md) | check for `(*schema.ResourceData).SetId()` receiver method usage with unstable `time.Now()` value | AST | ### Standard Schema Checks diff --git a/helper/terraformtype/helper/resource/funcs.go b/helper/terraformtype/helper/resource/funcs.go index 81f78185..1a9324a0 100644 --- a/helper/terraformtype/helper/resource/funcs.go +++ b/helper/terraformtype/helper/resource/funcs.go @@ -5,6 +5,7 @@ const ( FuncNameComposeTestCheckFunc = `ComposeTestCheckFunc` FuncNameNonRetryableError = `NonRetryableError` FuncNameParallelTest = `ParallelTest` + FuncNamePrefixedUniqueId = `PrefixedUniqueId` FuncNameRetryableError = `RetryableError` FuncNameTest = `Test` FuncNameTestCheckNoResourceAttr = `TestCheckNoResourceAttr` @@ -14,4 +15,5 @@ const ( FuncNameTestCheckResourceAttrPtr = `TestCheckResourceAttrPtr` FuncNameTestCheckResourceAttrSet = `TestCheckResourceAttrSet` FuncNameTestMatchResourceAttr = `TestMatchResourceAttr` + FuncNameUniqueId = `UniqueId` ) diff --git a/passes/R015/R015.go b/passes/R015/R015.go new file mode 100644 index 00000000..0430b0df --- /dev/null +++ b/passes/R015/R015.go @@ -0,0 +1,58 @@ +package R015 + +import ( + "go/ast" + + "golang.org/x/tools/go/analysis" + + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/resource" + "github.com/bflad/tfproviderlint/passes/commentignore" + "github.com/bflad/tfproviderlint/passes/helper/schema/resourcedatasetidcallexpr" +) + +const Doc = `check for (*schema.ResourceData).SetId() usage with unstable resource.UniqueId() value + +Schema attributes should be stable across Terraform runs.` + +const analyzerName = "R015" + +var Analyzer = &analysis.Analyzer{ + Name: analyzerName, + Doc: Doc, + Requires: []*analysis.Analyzer{ + commentignore.Analyzer, + resourcedatasetidcallexpr.Analyzer, + }, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer) + callExprs := pass.ResultOf[resourcedatasetidcallexpr.Analyzer].([]*ast.CallExpr) + for _, callExpr := range callExprs { + if ignorer.ShouldIgnore(analyzerName, callExpr) { + continue + } + + if len(callExpr.Args) < 1 { + continue + } + + ast.Inspect(callExpr.Args[0], func(n ast.Node) bool { + callExpr, ok := n.(*ast.CallExpr) + + if !ok { + return true + } + + if resource.IsFunc(callExpr.Fun, pass.TypesInfo, resource.FuncNameUniqueId) { + pass.Reportf(callExpr.Pos(), "%s: schema attributes should be stable across Terraform runs", analyzerName) + return false + } + + return true + }) + } + + return nil, nil +} diff --git a/passes/R015/R015_test.go b/passes/R015/R015_test.go new file mode 100644 index 00000000..e572c8e3 --- /dev/null +++ b/passes/R015/R015_test.go @@ -0,0 +1,14 @@ +package R015_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/bflad/tfproviderlint/passes/R015" +) + +func TestR015(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, R015.Analyzer, "a") +} diff --git a/passes/R015/README.md b/passes/R015/README.md new file mode 100644 index 00000000..8d77815f --- /dev/null +++ b/passes/R015/README.md @@ -0,0 +1,24 @@ +# R015 + +The R015 analyzer reports [`(*schema.ResourceData).SetId()`](https://godoc.org/github.com/hashicorp/terraform-plugin-sdk/helper/schema#ResourceData.Set) usage with unstable `resource.UniqueId()` value. Schema attributes should be stable across Terraform runs. + +## Flagged Code + +```go +d.SetId(resource.UniqueId()) +``` + +## Passing Code + +```go +d.SetId("stablestring") +``` + +## Ignoring Reports + +Singular reports can be ignored by adding the a `//lintignore:R015` Go code comment at the end of the offending line or on the line immediately proceding, e.g. + +```go +//lintignore:R015 +d.SetId(resource.UniqueId()) +``` diff --git a/passes/R015/testdata/src/a/alias.go b/passes/R015/testdata/src/a/alias.go new file mode 100644 index 00000000..3bbc533b --- /dev/null +++ b/passes/R015/testdata/src/a/alias.go @@ -0,0 +1,14 @@ +package a + +import ( + r "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func falias() { + var d schema.ResourceData + + d.SetId(r.UniqueId()) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") +} diff --git a/passes/R015/testdata/src/a/alias_v2.go b/passes/R015/testdata/src/a/alias_v2.go new file mode 100644 index 00000000..aae08ff6 --- /dev/null +++ b/passes/R015/testdata/src/a/alias_v2.go @@ -0,0 +1,14 @@ +package a + +import ( + r "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func falias_v2() { + var d schema.ResourceData + + d.SetId(r.UniqueId()) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") +} diff --git a/passes/R015/testdata/src/a/comment_ignore.go b/passes/R015/testdata/src/a/comment_ignore.go new file mode 100644 index 00000000..ecb0a8cc --- /dev/null +++ b/passes/R015/testdata/src/a/comment_ignore.go @@ -0,0 +1,15 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func fcommentignore() { + var d schema.ResourceData + + d.SetId(resource.UniqueId()) //lintignore:R015 + + //lintignore:R015 + d.SetId(resource.UniqueId()) +} diff --git a/passes/R015/testdata/src/a/comment_ignore_v2.go b/passes/R015/testdata/src/a/comment_ignore_v2.go new file mode 100644 index 00000000..18cea11a --- /dev/null +++ b/passes/R015/testdata/src/a/comment_ignore_v2.go @@ -0,0 +1,15 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func fcommentignore_v2() { + var d schema.ResourceData + + d.SetId(resource.UniqueId()) //lintignore:R015 + + //lintignore:R015 + d.SetId(resource.UniqueId()) +} diff --git a/passes/R015/testdata/src/a/main.go b/passes/R015/testdata/src/a/main.go new file mode 100644 index 00000000..13e5d4d8 --- /dev/null +++ b/passes/R015/testdata/src/a/main.go @@ -0,0 +1,24 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func f() { + var d schema.ResourceData + + d.SetId(resource.UniqueId()) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + fResourceFunc(&d, nil) +} + +func fResourceFunc(d *schema.ResourceData, meta interface{}) error { + d.SetId(resource.UniqueId()) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + return nil +} diff --git a/passes/R015/testdata/src/a/main_v2.go b/passes/R015/testdata/src/a/main_v2.go new file mode 100644 index 00000000..c4d6efbe --- /dev/null +++ b/passes/R015/testdata/src/a/main_v2.go @@ -0,0 +1,24 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func f_v2() { + var d schema.ResourceData + + d.SetId(resource.UniqueId()) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + fResourceFunc_v2(&d, nil) +} + +func fResourceFunc_v2(d *schema.ResourceData, meta interface{}) error { + d.SetId(resource.UniqueId()) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + return nil +} diff --git a/passes/R015/testdata/src/a/outside_package.go b/passes/R015/testdata/src/a/outside_package.go new file mode 100644 index 00000000..aec4adf0 --- /dev/null +++ b/passes/R015/testdata/src/a/outside_package.go @@ -0,0 +1,12 @@ +package a + +import ( + "a/resource" + "a/schema" +) + +func foutside() { + var d schema.ResourceData + + d.SetId(resource.UniqueId()) +} diff --git a/passes/R015/testdata/src/a/resource/resource.go b/passes/R015/testdata/src/a/resource/resource.go new file mode 100644 index 00000000..7fea68b1 --- /dev/null +++ b/passes/R015/testdata/src/a/resource/resource.go @@ -0,0 +1,5 @@ +package resource + +func UniqueId() string { + return "" +} diff --git a/passes/R015/testdata/src/a/schema/schema.go b/passes/R015/testdata/src/a/schema/schema.go new file mode 100644 index 00000000..c5103cc3 --- /dev/null +++ b/passes/R015/testdata/src/a/schema/schema.go @@ -0,0 +1,7 @@ +package schema + +type ResourceData struct{} + +func (d *ResourceData) SetId(value interface{}) error { + return nil +} diff --git a/passes/R015/testdata/src/a/vendor b/passes/R015/testdata/src/a/vendor new file mode 120000 index 00000000..776800d2 --- /dev/null +++ b/passes/R015/testdata/src/a/vendor @@ -0,0 +1 @@ +../../../../../vendor \ No newline at end of file diff --git a/passes/R016/R016.go b/passes/R016/R016.go new file mode 100644 index 00000000..fdac29c3 --- /dev/null +++ b/passes/R016/R016.go @@ -0,0 +1,58 @@ +package R016 + +import ( + "go/ast" + + "golang.org/x/tools/go/analysis" + + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/resource" + "github.com/bflad/tfproviderlint/passes/commentignore" + "github.com/bflad/tfproviderlint/passes/helper/schema/resourcedatasetidcallexpr" +) + +const Doc = `check for (*schema.ResourceData).SetId() usage with unstable resource.PrefixedUniqueId() value + +Schema attributes should be stable across Terraform runs.` + +const analyzerName = "R016" + +var Analyzer = &analysis.Analyzer{ + Name: analyzerName, + Doc: Doc, + Requires: []*analysis.Analyzer{ + commentignore.Analyzer, + resourcedatasetidcallexpr.Analyzer, + }, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer) + callExprs := pass.ResultOf[resourcedatasetidcallexpr.Analyzer].([]*ast.CallExpr) + for _, callExpr := range callExprs { + if ignorer.ShouldIgnore(analyzerName, callExpr) { + continue + } + + if len(callExpr.Args) < 1 { + continue + } + + ast.Inspect(callExpr.Args[0], func(n ast.Node) bool { + callExpr, ok := n.(*ast.CallExpr) + + if !ok { + return true + } + + if resource.IsFunc(callExpr.Fun, pass.TypesInfo, resource.FuncNamePrefixedUniqueId) { + pass.Reportf(callExpr.Pos(), "%s: schema attributes should be stable across Terraform runs", analyzerName) + return false + } + + return true + }) + } + + return nil, nil +} diff --git a/passes/R016/R016_test.go b/passes/R016/R016_test.go new file mode 100644 index 00000000..18f11571 --- /dev/null +++ b/passes/R016/R016_test.go @@ -0,0 +1,14 @@ +package R016_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/bflad/tfproviderlint/passes/R016" +) + +func TestR016(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, R016.Analyzer, "a") +} diff --git a/passes/R016/README.md b/passes/R016/README.md new file mode 100644 index 00000000..461a847e --- /dev/null +++ b/passes/R016/README.md @@ -0,0 +1,24 @@ +# R016 + +The R016 analyzer reports [`(*schema.ResourceData).SetId()`](https://godoc.org/github.com/hashicorp/terraform-plugin-sdk/helper/schema#ResourceData.Set) usage with unstable `resource.PrefixedUniqueId()` value. Schema attributes should be stable across Terraform runs. + +## Flagged Code + +```go +d.SetId(resource.PrefixedUniqueId("example")) +``` + +## Passing Code + +```go +d.SetId("stablestring") +``` + +## Ignoring Reports + +Singular reports can be ignored by adding the a `//lintignore:R016` Go code comment at the end of the offending line or on the line immediately proceding, e.g. + +```go +//lintignore:R016 +d.SetId(resource.PrefixedUniqueId("example")) +``` diff --git a/passes/R016/testdata/src/a/alias.go b/passes/R016/testdata/src/a/alias.go new file mode 100644 index 00000000..03d47df0 --- /dev/null +++ b/passes/R016/testdata/src/a/alias.go @@ -0,0 +1,14 @@ +package a + +import ( + r "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func falias() { + var d schema.ResourceData + + d.SetId(r.PrefixedUniqueId("test")) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") +} diff --git a/passes/R016/testdata/src/a/alias_v2.go b/passes/R016/testdata/src/a/alias_v2.go new file mode 100644 index 00000000..2117ade1 --- /dev/null +++ b/passes/R016/testdata/src/a/alias_v2.go @@ -0,0 +1,14 @@ +package a + +import ( + r "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func falias_v2() { + var d schema.ResourceData + + d.SetId(r.PrefixedUniqueId("test")) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") +} diff --git a/passes/R016/testdata/src/a/comment_ignore.go b/passes/R016/testdata/src/a/comment_ignore.go new file mode 100644 index 00000000..91e0b86c --- /dev/null +++ b/passes/R016/testdata/src/a/comment_ignore.go @@ -0,0 +1,15 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func fcommentignore() { + var d schema.ResourceData + + d.SetId(resource.PrefixedUniqueId("test")) //lintignore:R016 + + //lintignore:R016 + d.SetId(resource.PrefixedUniqueId("test")) +} diff --git a/passes/R016/testdata/src/a/comment_ignore_v2.go b/passes/R016/testdata/src/a/comment_ignore_v2.go new file mode 100644 index 00000000..9fb89a5c --- /dev/null +++ b/passes/R016/testdata/src/a/comment_ignore_v2.go @@ -0,0 +1,15 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func fcommentignore_v2() { + var d schema.ResourceData + + d.SetId(resource.PrefixedUniqueId("test")) //lintignore:R016 + + //lintignore:R016 + d.SetId(resource.PrefixedUniqueId("test")) +} diff --git a/passes/R016/testdata/src/a/main.go b/passes/R016/testdata/src/a/main.go new file mode 100644 index 00000000..ae9531ae --- /dev/null +++ b/passes/R016/testdata/src/a/main.go @@ -0,0 +1,24 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func f() { + var d schema.ResourceData + + d.SetId(resource.PrefixedUniqueId("test")) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + fResourceFunc(&d, nil) +} + +func fResourceFunc(d *schema.ResourceData, meta interface{}) error { + d.SetId(resource.PrefixedUniqueId("test")) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + return nil +} diff --git a/passes/R016/testdata/src/a/main_v2.go b/passes/R016/testdata/src/a/main_v2.go new file mode 100644 index 00000000..9611007c --- /dev/null +++ b/passes/R016/testdata/src/a/main_v2.go @@ -0,0 +1,24 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func f_v2() { + var d schema.ResourceData + + d.SetId(resource.PrefixedUniqueId("test")) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + fResourceFunc_v2(&d, nil) +} + +func fResourceFunc_v2(d *schema.ResourceData, meta interface{}) error { + d.SetId(resource.PrefixedUniqueId("test")) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + return nil +} diff --git a/passes/R016/testdata/src/a/outside_package.go b/passes/R016/testdata/src/a/outside_package.go new file mode 100644 index 00000000..e49caee8 --- /dev/null +++ b/passes/R016/testdata/src/a/outside_package.go @@ -0,0 +1,12 @@ +package a + +import ( + "a/resource" + "a/schema" +) + +func foutside() { + var d schema.ResourceData + + d.SetId(resource.PrefixedUniqueId("test")) +} diff --git a/passes/R016/testdata/src/a/resource/resource.go b/passes/R016/testdata/src/a/resource/resource.go new file mode 100644 index 00000000..37f941e8 --- /dev/null +++ b/passes/R016/testdata/src/a/resource/resource.go @@ -0,0 +1,5 @@ +package resource + +func PrefixedUniqueId(prefix string) string { + return "" +} diff --git a/passes/R016/testdata/src/a/schema/schema.go b/passes/R016/testdata/src/a/schema/schema.go new file mode 100644 index 00000000..c5103cc3 --- /dev/null +++ b/passes/R016/testdata/src/a/schema/schema.go @@ -0,0 +1,7 @@ +package schema + +type ResourceData struct{} + +func (d *ResourceData) SetId(value interface{}) error { + return nil +} diff --git a/passes/R016/testdata/src/a/vendor b/passes/R016/testdata/src/a/vendor new file mode 120000 index 00000000..776800d2 --- /dev/null +++ b/passes/R016/testdata/src/a/vendor @@ -0,0 +1 @@ +../../../../../vendor \ No newline at end of file diff --git a/passes/R017/R017.go b/passes/R017/R017.go new file mode 100644 index 00000000..d8b6c490 --- /dev/null +++ b/passes/R017/R017.go @@ -0,0 +1,58 @@ +package R017 + +import ( + "go/ast" + + "golang.org/x/tools/go/analysis" + + "github.com/bflad/tfproviderlint/helper/astutils" + "github.com/bflad/tfproviderlint/passes/commentignore" + "github.com/bflad/tfproviderlint/passes/helper/schema/resourcedatasetidcallexpr" +) + +const Doc = `check for (*schema.ResourceData).SetId() usage with unstable time.Now() value + +Schema attributes should be stable across Terraform runs.` + +const analyzerName = "R017" + +var Analyzer = &analysis.Analyzer{ + Name: analyzerName, + Doc: Doc, + Requires: []*analysis.Analyzer{ + commentignore.Analyzer, + resourcedatasetidcallexpr.Analyzer, + }, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer) + callExprs := pass.ResultOf[resourcedatasetidcallexpr.Analyzer].([]*ast.CallExpr) + for _, callExpr := range callExprs { + if ignorer.ShouldIgnore(analyzerName, callExpr) { + continue + } + + if len(callExpr.Args) < 1 { + continue + } + + ast.Inspect(callExpr.Args[0], func(n ast.Node) bool { + callExpr, ok := n.(*ast.CallExpr) + + if !ok { + return true + } + + if astutils.IsStdlibPackageFunc(callExpr.Fun, pass.TypesInfo, "time", "Now") { + pass.Reportf(callExpr.Pos(), "%s: schema attributes should be stable across Terraform runs", analyzerName) + return false + } + + return true + }) + } + + return nil, nil +} diff --git a/passes/R017/R017_test.go b/passes/R017/R017_test.go new file mode 100644 index 00000000..6536d91d --- /dev/null +++ b/passes/R017/R017_test.go @@ -0,0 +1,14 @@ +package R017_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/bflad/tfproviderlint/passes/R017" +) + +func TestR017(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, R017.Analyzer, "a") +} diff --git a/passes/R017/README.md b/passes/R017/README.md new file mode 100644 index 00000000..1987c971 --- /dev/null +++ b/passes/R017/README.md @@ -0,0 +1,24 @@ +# R017 + +The R017 analyzer reports [`(*schema.ResourceData).SetId()`](https://godoc.org/github.com/hashicorp/terraform-plugin-sdk/helper/schema#ResourceData.Set) usage with unstable `time.Now()` value. Schema attributes should be stable across Terraform runs. + +## Flagged Code + +```go +d.SetId(time.Now().Format(time.RFC3339)) +``` + +## Passing Code + +```go +d.SetId("stablestring") +``` + +## Ignoring Reports + +Singular reports can be ignored by adding the a `//lintignore:R017` Go code comment at the end of the offending line or on the line immediately proceding, e.g. + +```go +//lintignore:R017 +d.SetId(time.Now().Format(time.RFC3339)) +``` diff --git a/passes/R017/testdata/src/a/alias.go b/passes/R017/testdata/src/a/alias.go new file mode 100644 index 00000000..de6be9e2 --- /dev/null +++ b/passes/R017/testdata/src/a/alias.go @@ -0,0 +1,15 @@ +package a + +import ( + t "time" + + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func falias() { + var d schema.ResourceData + + d.SetId(t.Now().Format(t.RFC3339)) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") +} diff --git a/passes/R017/testdata/src/a/alias_v2.go b/passes/R017/testdata/src/a/alias_v2.go new file mode 100644 index 00000000..e6465c6a --- /dev/null +++ b/passes/R017/testdata/src/a/alias_v2.go @@ -0,0 +1,15 @@ +package a + +import ( + t "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func falias_v2() { + var d schema.ResourceData + + d.SetId(t.Now().Format(t.RFC3339)) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") +} diff --git a/passes/R017/testdata/src/a/comment_ignore.go b/passes/R017/testdata/src/a/comment_ignore.go new file mode 100644 index 00000000..651114e8 --- /dev/null +++ b/passes/R017/testdata/src/a/comment_ignore.go @@ -0,0 +1,16 @@ +package a + +import ( + "time" + + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func fcommentignore() { + var d schema.ResourceData + + d.SetId(time.Now().Format(time.RFC3339)) //lintignore:R017 + + //lintignore:R017 + d.SetId(time.Now().Format(time.RFC3339)) +} diff --git a/passes/R017/testdata/src/a/comment_ignore_v2.go b/passes/R017/testdata/src/a/comment_ignore_v2.go new file mode 100644 index 00000000..9d51dd36 --- /dev/null +++ b/passes/R017/testdata/src/a/comment_ignore_v2.go @@ -0,0 +1,16 @@ +package a + +import ( + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func fcommentignore_v2() { + var d schema.ResourceData + + d.SetId(time.Now().Format(time.RFC3339)) //lintignore:R017 + + //lintignore:R017 + d.SetId(time.Now().Format(time.RFC3339)) +} diff --git a/passes/R017/testdata/src/a/main.go b/passes/R017/testdata/src/a/main.go new file mode 100644 index 00000000..0ff20f15 --- /dev/null +++ b/passes/R017/testdata/src/a/main.go @@ -0,0 +1,25 @@ +package a + +import ( + "time" + + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func f() { + var d schema.ResourceData + + d.SetId(time.Now().Format(time.RFC3339)) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + fResourceFunc(&d, nil) +} + +func fResourceFunc(d *schema.ResourceData, meta interface{}) error { + d.SetId(time.Now().Format(time.RFC3339)) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + return nil +} diff --git a/passes/R017/testdata/src/a/main_v2.go b/passes/R017/testdata/src/a/main_v2.go new file mode 100644 index 00000000..66f9a48c --- /dev/null +++ b/passes/R017/testdata/src/a/main_v2.go @@ -0,0 +1,25 @@ +package a + +import ( + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func f_v2() { + var d schema.ResourceData + + d.SetId(time.Now().Format(time.RFC3339)) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + fResourceFunc_v2(&d, nil) +} + +func fResourceFunc_v2(d *schema.ResourceData, meta interface{}) error { + d.SetId(time.Now().Format(time.RFC3339)) // want "schema attributes should be stable across Terraform runs" + + d.SetId("test") + + return nil +} diff --git a/passes/R017/testdata/src/a/outside_package.go b/passes/R017/testdata/src/a/outside_package.go new file mode 100644 index 00000000..f4653455 --- /dev/null +++ b/passes/R017/testdata/src/a/outside_package.go @@ -0,0 +1,12 @@ +package a + +import ( + "a/schema" + "a/time" +) + +func foutside() { + var d schema.ResourceData + + d.SetId(time.Now()) +} diff --git a/passes/R017/testdata/src/a/schema/schema.go b/passes/R017/testdata/src/a/schema/schema.go new file mode 100644 index 00000000..c5103cc3 --- /dev/null +++ b/passes/R017/testdata/src/a/schema/schema.go @@ -0,0 +1,7 @@ +package schema + +type ResourceData struct{} + +func (d *ResourceData) SetId(value interface{}) error { + return nil +} diff --git a/passes/R017/testdata/src/a/time/time.go b/passes/R017/testdata/src/a/time/time.go new file mode 100644 index 00000000..465b1112 --- /dev/null +++ b/passes/R017/testdata/src/a/time/time.go @@ -0,0 +1,5 @@ +package time + +func Now() string { + return "" +} diff --git a/passes/R017/testdata/src/a/vendor b/passes/R017/testdata/src/a/vendor new file mode 120000 index 00000000..776800d2 --- /dev/null +++ b/passes/R017/testdata/src/a/vendor @@ -0,0 +1 @@ +../../../../../vendor \ No newline at end of file diff --git a/passes/checks.go b/passes/checks.go index e42e9570..847e4cae 100644 --- a/passes/checks.go +++ b/passes/checks.go @@ -23,6 +23,9 @@ import ( "github.com/bflad/tfproviderlint/passes/R012" "github.com/bflad/tfproviderlint/passes/R013" "github.com/bflad/tfproviderlint/passes/R014" + "github.com/bflad/tfproviderlint/passes/R015" + "github.com/bflad/tfproviderlint/passes/R016" + "github.com/bflad/tfproviderlint/passes/R017" "github.com/bflad/tfproviderlint/passes/S001" "github.com/bflad/tfproviderlint/passes/S002" "github.com/bflad/tfproviderlint/passes/S003" @@ -97,6 +100,9 @@ var AllChecks = []*analysis.Analyzer{ R012.Analyzer, R013.Analyzer, R014.Analyzer, + R015.Analyzer, + R016.Analyzer, + R017.Analyzer, S001.Analyzer, S002.Analyzer, S003.Analyzer, diff --git a/passes/helper/schema/resourcedatasetidcallexpr/resourcedatasetidcallexpr.go b/passes/helper/schema/resourcedatasetidcallexpr/resourcedatasetidcallexpr.go new file mode 100644 index 00000000..0c4c909c --- /dev/null +++ b/passes/helper/schema/resourcedatasetidcallexpr/resourcedatasetidcallexpr.go @@ -0,0 +1,14 @@ +package resourcedatasetidcallexpr + +import ( + "github.com/bflad/tfproviderlint/helper/analysisutils" + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema" +) + +var Analyzer = analysisutils.ReceiverMethodCallExprAnalyzer( + "resourcedatasetidcallexpr", + schema.IsReceiverMethod, + schema.PackagePath, + schema.TypeNameResourceData, + "SetId", +) diff --git a/passes/helper/schema/resourcedatasetidcallexpr/resourcedatasetidcallexpr_test.go b/passes/helper/schema/resourcedatasetidcallexpr/resourcedatasetidcallexpr_test.go new file mode 100644 index 00000000..1e0edd5b --- /dev/null +++ b/passes/helper/schema/resourcedatasetidcallexpr/resourcedatasetidcallexpr_test.go @@ -0,0 +1,15 @@ +package resourcedatasetidcallexpr + +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) + } +}