Merge branch 'magodo-unsed_timeout'

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)

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
 

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 {

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

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

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

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

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

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

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

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