diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b477ca3..560c68d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # v0.22.0 +FEATURES + +* **New Check:** `XR006`: check for `Resource` that implements `Timeouts` for missing `Create`, `Delete`, `Read`, or `Update` implementation (#222) + ENHANCEMENTS * passes/helper/schema/crudfuncinfo: Support `CreateContextFunc`, `DeleteContextFunc`, `ReadContextFunc`, and `UpdateContextFunc` matches (#221) diff --git a/README.md b/README.md index d206f4ad..a40bac7e 100644 --- a/README.md +++ b/README.md @@ -198,6 +198,7 @@ Extra lint checks are not included in the `tfproviderlint` tool and must be acce | [XR003](xpasses/XR003) | check for `Resource` that should implement `Timeouts` | AST | | [XR004](xpasses/XR004) | check for `ResourceData.Set()` calls that should implement error checking with complex values | AST | | [XR005](xpasses/XR005) | check for `Resource` that `Description` is configured | AST | +| [XR006](xpasses/XR006) | check for `Resource` that implements `Timeouts` for missing `Create`, `Delete`, `Read`, or `Update` implementation | AST | ### Extra Schema Checks diff --git a/helper/terraformtype/helper/schema/type_resource.go b/helper/terraformtype/helper/schema/type_resource.go index 334a66af..2bac427d 100644 --- a/helper/terraformtype/helper/schema/type_resource.go +++ b/helper/terraformtype/helper/schema/type_resource.go @@ -3,25 +3,30 @@ package schema import ( "go/ast" "go/types" + "time" "github.com/bflad/tfproviderlint/helper/astutils" ) const ( ResourceFieldCreate = `Create` + ResourceFieldCreateContext = `CreateContext` ResourceFieldCustomizeDiff = `CustomizeDiff` ResourceFieldDelete = `Delete` + ResourceFieldDeleteContext = `DeleteContext` ResourceFieldDeprecationMessage = `DeprecationMessage` ResourceFieldDescription = `Description` ResourceFieldExists = `Exists` ResourceFieldImporter = `Importer` ResourceFieldMigrateState = `MigrateState` ResourceFieldRead = `Read` + ResourceFieldReadContext = `ReadContext` ResourceFieldSchema = `Schema` ResourceFieldSchemaVersion = `SchemaVersion` ResourceFieldStateUpgraders = `StateUpgraders` ResourceFieldTimeouts = `Timeouts` ResourceFieldUpdate = `Update` + ResourceFieldUpdateContext = `UpdateContext` TypeNameResource = `Resource` ) @@ -35,6 +40,7 @@ type resourceType struct { Description string MigrateState func(int, interface{}, interface{}) (interface{}, error) Schema map[string]*schemaType + Timeouts resourceTimeoutType } // ResourceInfo represents all gathered Resource data for easier access @@ -62,6 +68,43 @@ func NewResourceInfo(cl *ast.CompositeLit, info *types.Info) *ResourceInfo { result.Resource.MigrateState = func(int, interface{}, interface{}) (interface{}, error) { return nil, nil } } + if kvExpr := result.Fields[ResourceFieldTimeouts]; kvExpr != nil && astutils.ExprValue(kvExpr.Value) != nil { + if timeoutUexpr, ok := kvExpr.Value.(*ast.UnaryExpr); ok { + if timeoutClit, ok := timeoutUexpr.X.(*ast.CompositeLit); ok { + for _, expr := range timeoutClit.Elts { + switch elt := expr.(type) { + case *ast.KeyValueExpr: + var key string + + switch keyExpr := elt.Key.(type) { + case *ast.Ident: + key = keyExpr.Name + } + + if key == "" { + continue + } + + if astutils.ExprValue(elt.Value) != nil { + switch key { + case "Create": + result.Resource.Timeouts.Create = new(time.Duration) + case "Read": + result.Resource.Timeouts.Read = new(time.Duration) + case "Update": + result.Resource.Timeouts.Update = new(time.Duration) + case "Delete": + result.Resource.Timeouts.Delete = new(time.Duration) + case "Default": + result.Resource.Timeouts.Default = new(time.Duration) + } + } + } + } + } + } + } + if kvExpr := result.Fields[ResourceFieldSchema]; kvExpr != nil && astutils.ExprValue(kvExpr.Value) != nil { result.Resource.Schema = map[string]*schemaType{} if smap, ok := kvExpr.Value.(*ast.CompositeLit); ok { diff --git a/helper/terraformtype/helper/schema/type_resourcetimeout.go b/helper/terraformtype/helper/schema/type_resourcetimeout.go new file mode 100644 index 00000000..689c0ed4 --- /dev/null +++ b/helper/terraformtype/helper/schema/type_resourcetimeout.go @@ -0,0 +1,20 @@ +package schema + +import "time" + +const ( + ResourceTimeoutTypeCreateField = `Create` + ResourceTimeoutTypeDefaultField = `Default` + ResourceTimeoutTypeDeleteField = `Delete` + ResourceTimeoutTypeReadField = `Read` + ResourceTimeoutTypeUpdateField = `Update` +) + +// resourceTimeoutType is an internal representation of the SDK helper/schema.ResourceTimeout type +type resourceTimeoutType struct { + Create *time.Duration + Default *time.Duration + Delete *time.Duration + Read *time.Duration + Update *time.Duration +} diff --git a/xpasses/XR006/README.md b/xpasses/XR006/README.md new file mode 100644 index 00000000..e9a9fb09 --- /dev/null +++ b/xpasses/XR006/README.md @@ -0,0 +1,49 @@ +# XR006 + +The XR006 analyzer reports extraneous `Timeouts` fields in resources where the corresponding `Create`/`CreateContext`, `Delete`/`DeleteContext`, `Read`/`ReadContext`, or `Update`/`UpdateContext` implementation does not exist. + +## Flagged Code + +```go +&schema.Resource{ + /* ... no Create ... */ + Read: /* ... */, + Timeouts: schema.ResourceTimesout{ + Create: schema.DefaultTimeout(10 * time.Minute), + }, +} +``` + +## Passing Code + +```go +// Fixed Timeouts field alignment +&schema.Resource{ + /* ... no Create ... */ + Read: /* ... */, + Timeouts: schema.ResourceTimesout{ + Read: schema.DefaultTimeout(10 * time.Minute), + }, +} + +// Removed Timeouts +&schema.Resource{ + /* ... no Create ... */ + Read: /* ... */, +} +``` + +## Ignoring Reports + +Singular reports can be ignored by adding the a `//lintignore:XR006` Go code comment at the end of the offending line or on the line immediately proceding, e.g. + +```go +//lintignore:XR006 +&schema.Resource{ + /* ... no Create ... */ + Read: /* ... */, + Timeouts: schema.ResourceTimesout{ + Create: schema.DefaultTimeout(10 * time.Minute), + }, +} +``` diff --git a/xpasses/XR006/XR006.go b/xpasses/XR006/XR006.go new file mode 100644 index 00000000..d9c8d405 --- /dev/null +++ b/xpasses/XR006/XR006.go @@ -0,0 +1,53 @@ +package XR006 + +import ( + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema" + "github.com/bflad/tfproviderlint/passes/commentignore" + "github.com/bflad/tfproviderlint/passes/helper/schema/resourceinfo" + "golang.org/x/tools/go/analysis" +) + +const Doc = `check for Resource that implements Timeouts for missing Create, Delete, Read, or Update implementation + +The XR006 analyzer reports extraneous Timeouts fields in resources where the +corresponding Create, Delete, Read, or Update implementation does not exist.` + +const analyzerName = "XR006" + +var Analyzer = &analysis.Analyzer{ + Name: analyzerName, + Doc: Doc, + Requires: []*analysis.Analyzer{ + commentignore.Analyzer, + resourceinfo.Analyzer, + }, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer) + resources := pass.ResultOf[resourceinfo.Analyzer].([]*schema.ResourceInfo) + for _, resource := range resources { + if ignorer.ShouldIgnore(analyzerName, resource.AstCompositeLit) { + continue + } + + if !resource.DeclaresField(schema.ResourceFieldCreate) && !resource.DeclaresField(schema.ResourceFieldCreateContext) && resource.Resource.Timeouts.Create != nil { + pass.Reportf(resource.AstCompositeLit.Pos(), "%s: resource should not configure Timeouts.Create without Create implementation", analyzerName) + } + + if !resource.DeclaresField(schema.ResourceFieldDelete) && !resource.DeclaresField(schema.ResourceFieldDeleteContext) && resource.Resource.Timeouts.Delete != nil { + pass.Reportf(resource.AstCompositeLit.Pos(), "%s: resource should not configure Timeouts.Delete without Delete implementation", analyzerName) + } + + if !resource.DeclaresField(schema.ResourceFieldRead) && !resource.DeclaresField(schema.ResourceFieldReadContext) && resource.Resource.Timeouts.Read != nil { + pass.Reportf(resource.AstCompositeLit.Pos(), "%s: resource should not configure Timeouts.Read without Read implementation", analyzerName) + } + + if !resource.DeclaresField(schema.ResourceFieldUpdate) && !resource.DeclaresField(schema.ResourceFieldUpdateContext) && resource.Resource.Timeouts.Update != nil { + pass.Reportf(resource.AstCompositeLit.Pos(), "%s: resource should not configure Timeouts.Update without Update implementation", analyzerName) + } + } + + return nil, nil +} diff --git a/xpasses/XR006/XR006_test.go b/xpasses/XR006/XR006_test.go new file mode 100644 index 00000000..2dc83fea --- /dev/null +++ b/xpasses/XR006/XR006_test.go @@ -0,0 +1,13 @@ +package XR006_test + +import ( + "testing" + + "github.com/bflad/tfproviderlint/xpasses/XR006" + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestXR006(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, XR006.Analyzer, "a") +} diff --git a/xpasses/XR006/testdata/src/a/alias.go b/xpasses/XR006/testdata/src/a/alias.go new file mode 100644 index 00000000..cc9970ee --- /dev/null +++ b/xpasses/XR006/testdata/src/a/alias.go @@ -0,0 +1,33 @@ +package a + +import ( + "time" + + s "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func falias() { + _ = s.Resource{ // want "resource should not configure Timeouts.Create without Create implementation" + Timeouts: &s.ResourceTimeout{ + Create: s.DefaultTimeout(time.Minute), + }, + } + + _ = s.Resource{ // want "resource should not configure Timeouts.Delete without Delete implementation" + Timeouts: &s.ResourceTimeout{ + Delete: s.DefaultTimeout(time.Minute), + }, + } + + _ = s.Resource{ // want "resource should not configure Timeouts.Read without Read implementation" + Timeouts: &s.ResourceTimeout{ + Read: s.DefaultTimeout(time.Minute), + }, + } + + _ = s.Resource{ // want "resource should not configure Timeouts.Update without Update implementation" + Timeouts: &s.ResourceTimeout{ + Update: s.DefaultTimeout(time.Minute), + }, + } +} diff --git a/xpasses/XR006/testdata/src/a/alias_v2.go b/xpasses/XR006/testdata/src/a/alias_v2.go new file mode 100644 index 00000000..80f912f2 --- /dev/null +++ b/xpasses/XR006/testdata/src/a/alias_v2.go @@ -0,0 +1,33 @@ +package a + +import ( + "time" + + s "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func falias_v2() { + _ = s.Resource{ // want "resource should not configure Timeouts.Create without Create implementation" + Timeouts: &s.ResourceTimeout{ + Create: s.DefaultTimeout(time.Minute), + }, + } + + _ = s.Resource{ // want "resource should not configure Timeouts.Delete without Delete implementation" + Timeouts: &s.ResourceTimeout{ + Delete: s.DefaultTimeout(time.Minute), + }, + } + + _ = s.Resource{ // want "resource should not configure Timeouts.Read without Read implementation" + Timeouts: &s.ResourceTimeout{ + Read: s.DefaultTimeout(time.Minute), + }, + } + + _ = s.Resource{ // want "resource should not configure Timeouts.Update without Update implementation" + Timeouts: &s.ResourceTimeout{ + Update: s.DefaultTimeout(time.Minute), + }, + } +} diff --git a/xpasses/XR006/testdata/src/a/comment_ignore.go b/xpasses/XR006/testdata/src/a/comment_ignore.go new file mode 100644 index 00000000..13ae9eed --- /dev/null +++ b/xpasses/XR006/testdata/src/a/comment_ignore.go @@ -0,0 +1,37 @@ +package a + +import ( + "time" + + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func fcommentignore() { + //lintignore:XR006 + _ = schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(time.Minute), + }, + } + + //lintignore:XR006 + _ = schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Delete: schema.DefaultTimeout(time.Minute), + }, + } + + //lintignore:XR006 + _ = schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Read: schema.DefaultTimeout(time.Minute), + }, + } + + //lintignore:XR006 + _ = schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Update: schema.DefaultTimeout(time.Minute), + }, + } +} diff --git a/xpasses/XR006/testdata/src/a/comment_ignore_v2.go b/xpasses/XR006/testdata/src/a/comment_ignore_v2.go new file mode 100644 index 00000000..a3eaf8e4 --- /dev/null +++ b/xpasses/XR006/testdata/src/a/comment_ignore_v2.go @@ -0,0 +1,37 @@ +package a + +import ( + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func fcommentignore_v2() { + //lintignore:XR006 + _ = schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(time.Minute), + }, + } + + //lintignore:XR006 + _ = schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Delete: schema.DefaultTimeout(time.Minute), + }, + } + + //lintignore:XR006 + _ = schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Read: schema.DefaultTimeout(time.Minute), + }, + } + + //lintignore:XR006 + _ = schema.Resource{ + Timeouts: &schema.ResourceTimeout{ + Update: schema.DefaultTimeout(time.Minute), + }, + } +} diff --git a/xpasses/XR006/testdata/src/a/main.go b/xpasses/XR006/testdata/src/a/main.go new file mode 100644 index 00000000..9034123f --- /dev/null +++ b/xpasses/XR006/testdata/src/a/main.go @@ -0,0 +1,77 @@ +package a + +import ( + "time" + + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func f() { + _ = schema.Resource{ // want "resource should not configure Timeouts.Create without Create implementation" + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ // want "resource should not configure Timeouts.Delete without Delete implementation" + Timeouts: &schema.ResourceTimeout{ + Delete: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ // want "resource should not configure Timeouts.Read without Read implementation" + Timeouts: &schema.ResourceTimeout{ + Read: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ // want "resource should not configure Timeouts.Update without Update implementation" + Timeouts: &schema.ResourceTimeout{ + Update: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + Create: createFunc, + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + Delete: deleteFunc, + Timeouts: &schema.ResourceTimeout{ + Delete: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + Read: readFunc, + Timeouts: &schema.ResourceTimeout{ + Read: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + Update: updateFunc, + Timeouts: &schema.ResourceTimeout{ + Update: schema.DefaultTimeout(time.Minute), + }, + } +} + +func createFunc(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func deleteFunc(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func readFunc(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func updateFunc(d *schema.ResourceData, meta interface{}) error { + return nil +} diff --git a/xpasses/XR006/testdata/src/a/main_v2.go b/xpasses/XR006/testdata/src/a/main_v2.go new file mode 100644 index 00000000..a1589e88 --- /dev/null +++ b/xpasses/XR006/testdata/src/a/main_v2.go @@ -0,0 +1,123 @@ +package a + +import ( + "context" + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func f_v2() { + _ = schema.Resource{ // want "resource should not configure Timeouts.Create without Create implementation" + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ // want "resource should not configure Timeouts.Delete without Delete implementation" + Timeouts: &schema.ResourceTimeout{ + Delete: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ // want "resource should not configure Timeouts.Read without Read implementation" + Timeouts: &schema.ResourceTimeout{ + Read: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ // want "resource should not configure Timeouts.Update without Update implementation" + Timeouts: &schema.ResourceTimeout{ + Update: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + Create: createFunc_v2, + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + Delete: deleteFunc_v2, + Timeouts: &schema.ResourceTimeout{ + Delete: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + Read: readFunc_v2, + Timeouts: &schema.ResourceTimeout{ + Read: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + Update: updateFunc_v2, + Timeouts: &schema.ResourceTimeout{ + Update: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + CreateContext: createContextFunc_v2, + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + DeleteContext: deleteContextFunc_v2, + Timeouts: &schema.ResourceTimeout{ + Delete: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + ReadContext: readContextFunc_v2, + Timeouts: &schema.ResourceTimeout{ + Read: schema.DefaultTimeout(time.Minute), + }, + } + + _ = schema.Resource{ + UpdateContext: updateContextFunc_v2, + Timeouts: &schema.ResourceTimeout{ + Update: schema.DefaultTimeout(time.Minute), + }, + } +} + +func createFunc_v2(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func createContextFunc_v2(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + return nil +} + +func deleteFunc_v2(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func deleteContextFunc_v2(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + return nil +} + +func readFunc_v2(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func readContextFunc_v2(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + return nil +} + +func updateFunc_v2(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func updateContextFunc_v2(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + return nil +} diff --git a/xpasses/XR006/testdata/src/a/outside_package.go b/xpasses/XR006/testdata/src/a/outside_package.go new file mode 100644 index 00000000..f10eb0ac --- /dev/null +++ b/xpasses/XR006/testdata/src/a/outside_package.go @@ -0,0 +1,20 @@ +package a + +import ( + "a/schema" + "time" +) + +func foutside() { + oneMinute := time.Minute + _ = schema.Resource{ + Create: outsideCreateFunc, + Timeouts: &schema.ResourceTimeout{ + Create: &oneMinute, + }, + } +} + +func outsideCreateFunc(d *schema.ResourceData, meta interface{}) error { + return nil +} diff --git a/xpasses/XR006/testdata/src/a/schema/schema.go b/xpasses/XR006/testdata/src/a/schema/schema.go new file mode 100644 index 00000000..4bca7b83 --- /dev/null +++ b/xpasses/XR006/testdata/src/a/schema/schema.go @@ -0,0 +1,16 @@ +package schema + +import "time" + +type Resource struct { + Create CreateFunc + Timeouts *ResourceTimeout +} + +type ResourceTimeout struct { + Create *time.Duration +} + +type ResourceData struct{} + +type CreateFunc func(*ResourceData, interface{}) error diff --git a/xpasses/XR006/testdata/src/a/vendor b/xpasses/XR006/testdata/src/a/vendor new file mode 120000 index 00000000..776800d2 --- /dev/null +++ b/xpasses/XR006/testdata/src/a/vendor @@ -0,0 +1 @@ +../../../../../vendor \ No newline at end of file diff --git a/xpasses/checks.go b/xpasses/checks.go index 7efdc2e6..f08386bd 100644 --- a/xpasses/checks.go +++ b/xpasses/checks.go @@ -6,6 +6,7 @@ import ( "github.com/bflad/tfproviderlint/xpasses/XR003" "github.com/bflad/tfproviderlint/xpasses/XR004" "github.com/bflad/tfproviderlint/xpasses/XR005" + "github.com/bflad/tfproviderlint/xpasses/XR006" "github.com/bflad/tfproviderlint/xpasses/XS001" "github.com/bflad/tfproviderlint/xpasses/XS002" "golang.org/x/tools/go/analysis" @@ -20,6 +21,7 @@ var AllChecks = []*analysis.Analyzer{ XR003.Analyzer, XR004.Analyzer, XR005.Analyzer, + XR006.Analyzer, XS001.Analyzer, XS002.Analyzer, }