From 5b4f6fdded9a1bd471fb32e4e3e66f7838d79fa2 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 5 Aug 2020 08:25:29 -0400 Subject: [PATCH] helper/terraformtype/helper/schema: Ensure CustomizeDiffFunc handling supports Terraform Plugin SDK v2 Reference: https://github.com/bflad/tfproviderlint/issues/178 --- .../helper/schema/type_customizedifffunc.go | 48 ++++++++-- .../schema/type_customizedifffunc_test.go | 90 +++++++++++++++---- 2 files changed, 114 insertions(+), 24 deletions(-) diff --git a/helper/terraformtype/helper/schema/type_customizedifffunc.go b/helper/terraformtype/helper/schema/type_customizedifffunc.go index c90186db..8ff8c633 100644 --- a/helper/terraformtype/helper/schema/type_customizedifffunc.go +++ b/helper/terraformtype/helper/schema/type_customizedifffunc.go @@ -20,11 +20,28 @@ func IsFuncTypeCustomizeDiffFunc(node ast.Node, info *types.Info) bool { return false } + return isFuncTypeCustomizeDiffFuncV1(funcType, info) || isFuncTypeCustomizeDiffFuncV2(funcType, info) +} + +// IsTypeCustomizeDiffFunc returns if the type is CustomizeDiffFunc from the customdiff package +func IsTypeCustomizeDiffFunc(t types.Type) bool { + switch t := t.(type) { + case *types.Named: + return IsNamedType(t, TypeNameCustomizeDiffFunc) + case *types.Pointer: + return IsTypeCustomizeDiffFunc(t.Elem()) + default: + return false + } +} + +// isFuncTypeCustomizeDiffFuncV1 returns true if the FuncType matches expected parameters and results types of V1 +func isFuncTypeCustomizeDiffFuncV1(funcType *ast.FuncType, info *types.Info) bool { if !astutils.HasFieldListLength(funcType.Params, 2) { return false } - if !astutils.IsFieldListTypePackageType(funcType.Params, 0, info, PackagePath, TypeNameResourceDiff) { + if !astutils.IsFieldListTypePackageType(funcType.Params, 0, info, PackagePathVersion(1), TypeNameResourceDiff) { return false } @@ -39,16 +56,29 @@ func IsFuncTypeCustomizeDiffFunc(node ast.Node, info *types.Info) bool { return astutils.IsFieldListType(funcType.Results, 0, astutils.IsExprTypeError) } -// IsTypeCustomizeDiffFunc returns if the type is CustomizeDiffFunc from the customdiff package -func IsTypeCustomizeDiffFunc(t types.Type) bool { - switch t := t.(type) { - case *types.Named: - return IsNamedType(t, TypeNameCustomizeDiffFunc) - case *types.Pointer: - return IsTypeCustomizeDiffFunc(t.Elem()) - default: +// isFuncTypeCustomizeDiffFuncV2 returns true if the FuncType matches expected parameters and results types of V2 +func isFuncTypeCustomizeDiffFuncV2(funcType *ast.FuncType, info *types.Info) bool { + if !astutils.HasFieldListLength(funcType.Params, 3) { + return false + } + + if !astutils.IsFieldListTypePackageType(funcType.Params, 0, info, "context", "Context") { + return false + } + + if !astutils.IsFieldListTypePackageType(funcType.Params, 1, info, PackagePathVersion(2), TypeNameResourceDiff) { return false } + + if !astutils.IsFieldListType(funcType.Params, 2, astutils.IsExprTypeInterface) { + return false + } + + if !astutils.HasFieldListLength(funcType.Results, 1) { + return false + } + + return astutils.IsFieldListType(funcType.Results, 0, astutils.IsExprTypeError) } // CustomizeDiffFuncInfo represents all gathered CustomizeDiffFunc data for easier access diff --git a/helper/terraformtype/helper/schema/type_customizedifffunc_test.go b/helper/terraformtype/helper/schema/type_customizedifffunc_test.go index 18048bf9..58b68711 100644 --- a/helper/terraformtype/helper/schema/type_customizedifffunc_test.go +++ b/helper/terraformtype/helper/schema/type_customizedifffunc_test.go @@ -12,41 +12,70 @@ func TestIsFuncTypeCustomizeDiffFunc(t *testing.T) { boolIdent := &ast.Ident{ Name: types.Typ[types.Bool].String(), } + contextIdent := &ast.Ident{ + Name: "context", + } + contextContextIdent := &ast.Ident{ + Name: "Context", + } errorIdent := &ast.Ident{ Name: "error", } interfaceAst := &ast.InterfaceType{ Methods: &ast.FieldList{}, } - packageIdent := &ast.Ident{ + packageIdentV1 := &ast.Ident{ Name: PackageName, } - packageType := types.NewPackage(PackagePath, PackageName) - packageNameType := types.NewPkgName(token.NoPos, packageType, packageType.Name(), packageType) + packageIdentV2 := &ast.Ident{ + Name: PackageName + `2`, + } + contextType := types.NewPackage("context", "context") + packageTypeV1 := types.NewPackage(PackagePathVersion(1), PackageName) + packageTypeV2 := types.NewPackage(PackagePathVersion(2), PackageName) + contextNameType := types.NewPkgName(token.NoPos, contextType, contextType.Name(), contextType) + packageNameTypeV1 := types.NewPkgName(token.NoPos, packageTypeV1, packageTypeV1.Name(), packageTypeV1) + packageNameTypeV2 := types.NewPkgName(token.NoPos, packageTypeV2, packageTypeV2.Name(), packageTypeV2) packageFuncIdent := &ast.Ident{ Name: TypeNameResourceDiff, } - packageSelectorExpr := &ast.SelectorExpr{ + contextContextSelectorExpr := &ast.SelectorExpr{ + Sel: contextContextIdent, + X: contextIdent, + } + packageSelectorExprV1 := &ast.SelectorExpr{ Sel: packageFuncIdent, - X: packageIdent, + X: packageIdentV1, + } + packageSelectorExprV2 := &ast.SelectorExpr{ + Sel: packageFuncIdent, + X: packageIdentV2, } typesInfo := &types.Info{ Types: map[ast.Expr]types.TypeAndValue{ - boolIdent: types.TypeAndValue{ + boolIdent: { Type: types.Typ[types.Bool], }, - errorIdent: types.TypeAndValue{ + contextContextSelectorExpr: { + Type: types.NewNamed(types.NewTypeName(token.NoPos, contextType, contextContextIdent.Name, nil), nil, nil), + }, + errorIdent: { Type: types.NewNamed(types.NewTypeName(token.NoPos, nil, "error", nil), nil, nil), }, - interfaceAst: types.TypeAndValue{ + interfaceAst: { Type: types.NewInterfaceType(nil, nil), }, - packageSelectorExpr: types.TypeAndValue{ - Type: types.NewNamed(types.NewTypeName(token.NoPos, packageType, TypeNameResourceDiff, nil), nil, nil), + packageSelectorExprV1: { + Type: types.NewNamed(types.NewTypeName(token.NoPos, packageTypeV1, TypeNameResourceDiff, nil), nil, nil), + }, + packageSelectorExprV2: { + Type: types.NewNamed(types.NewTypeName(token.NoPos, packageTypeV2, TypeNameResourceDiff, nil), nil, nil), }, }, Uses: map[*ast.Ident]types.Object{ - packageIdent: packageNameType, + contextIdent: contextNameType, + packageIdentV1: packageNameTypeV1, + packageIdentV2: packageNameTypeV2, }, } @@ -57,14 +86,14 @@ func TestIsFuncTypeCustomizeDiffFunc(t *testing.T) { Expected bool }{ { - Name: fmt.Sprintf("func(*%s.%s, interface{}) bool", PackagePath, TypeNameResourceDiff), + Name: fmt.Sprintf("func(*%s.%s, interface{}) bool", PackagePathVersion(1), TypeNameResourceDiff), Node: &ast.FuncLit{ Type: &ast.FuncType{ Params: &ast.FieldList{ List: []*ast.Field{ { Type: &ast.StarExpr{ - X: packageSelectorExpr, + X: packageSelectorExprV1, }, }, { @@ -85,14 +114,45 @@ func TestIsFuncTypeCustomizeDiffFunc(t *testing.T) { Expected: false, }, { - Name: fmt.Sprintf("func(*%s.%s, interface{}) error", PackagePath, TypeNameResourceDiff), + Name: fmt.Sprintf("func(*%s.%s, interface{}) error", PackagePathVersion(1), TypeNameResourceDiff), + Node: &ast.FuncLit{ + Type: &ast.FuncType{ + Params: &ast.FieldList{ + List: []*ast.Field{ + { + Type: &ast.StarExpr{ + X: packageSelectorExprV1, + }, + }, + { + Type: interfaceAst, + }, + }, + }, + Results: &ast.FieldList{ + List: []*ast.Field{ + { + Type: errorIdent, + }, + }, + }, + }, + }, + Info: typesInfo, + Expected: true, + }, + { + Name: fmt.Sprintf("func(context.Context, *%s.%s, interface{}) error", PackagePathVersion(2), TypeNameResourceDiff), Node: &ast.FuncLit{ Type: &ast.FuncType{ Params: &ast.FieldList{ List: []*ast.Field{ + { + Type: contextContextSelectorExpr, + }, { Type: &ast.StarExpr{ - X: packageSelectorExpr, + X: packageSelectorExprV2, }, }, {