add AZT005: resources and data sources must have basic, requiresImport, complete and update acceptance tests

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Unreleased
 
+- add rule `AZT005`: every data source needs a `basic` acceptance test and every resource `basic` and `requiresImport`, plus `complete` and `update` when it has an Update handler; tests are found in files named after the resource file or by the resource's type name; `disable` turns a step off so the rule can be enforced one step at a time; report only, no fix ([#68](https://github.com/katbyte/azproviderlint/pull/68))
 - add rule `AZT006`: tests named for an update step must apply two different configs; one config, or the same config twice, never runs the update; report only, no fix ([#69](https://github.com/katbyte/azproviderlint/pull/69))
 - add rule `AZT007`: a `TestAcc` name must not carry `Acc` again as a word of its own (`_storageAccBehindFireWall`); `Account` and `Access` are fine; report only, no fix ([#72](https://github.com/katbyte/azproviderlint/pull/72))
 - add rule `AZT004`: test functions that run an acceptance test but are not named `TestAcc`, so acceptance runs never pick them up; report only, no fix ([#67](https://github.com/katbyte/azproviderlint/pull/67))

README.md

@@ -106,6 +106,7 @@ Rules are named `AZ`, a category letter, and a number. The letters follow [tfpro
 | [AZT002](checks/AZT/AZT002_credentials_from_environment) | acceptance tests must not read credentials from the environment |
 | [AZT003](checks/AZT/AZT003_misnamed_acceptance_test) | TestAcc functions must run an acceptance test |
 | [AZT004](checks/AZT/AZT004_acceptance_test_not_named_test_acc) | acceptance tests must be named TestAcc |
+| [AZT005](checks/AZT/AZT005_resource_missing_acceptance_tests) | resources and data sources must have basic, requiresImport, complete and update acceptance tests |
 | [AZT006](checks/AZT/AZT006_update_test_single_config) | update tests must apply two different configs |
 | [AZT007](checks/AZT/AZT007_stray_acc_in_test_name) | acceptance test names must not carry a second Acc |
 

checks/AZP/AZP002_resource_missing_data_source/AZP002.go

@@ -5,9 +5,6 @@ package AZP002
 
 import (
 	"go/ast"
-	"go/constant"
-	"go/token"
-	"go/types"
 	"strings"
 
 	"github.com/katbyte/azproviderlint/lib/tf"
@@ -36,13 +33,6 @@ var Analyzer = &analysis.Analyzer{
 	Run:      run,
 }
 
-// registration method names for each side; the untyped map methods and the typed/framework
-// slice methods are distinguished by their return shape when collecting entries.
-var (
-	resourceMethods   = map[string]bool{"SupportedResources": true, "Resources": true, "FrameworkResources": true}
-	dataSourceMethods = map[string]bool{"SupportedDataSources": true, "DataSources": true, "FrameworkDataSources": true}
-)
-
 // actionStyleSuffixes marks invoke-style resources — ones that perform an operation or mint a
 // credential rather than manage a durable object (the kind that will eventually become
 // framework Actions). These have no meaningful data source form, so they are never reported.
@@ -53,18 +43,13 @@ var actionStyleSuffixes = []string{
 	"_sas_token",
 }
 
-type entry struct {
-	name string
-	pos  token.Pos
-}
-
 func run(pass *analysis.Pass) (any, error) {
 	insp, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 	if !ok {
 		return nil, nil
 	}
 
-	var resources []entry
+	var resources []tf.Entry
 	dataSources := map[string]bool{}
 	resolvable := true
 
@@ -78,8 +63,8 @@ func run(pass *analysis.Pass) (any, error) {
 			return
 		}
 
-		isResource := resourceMethods[fn.Name.Name]
-		isDataSource := dataSourceMethods[fn.Name.Name]
+		isResource := tf.ResourceMethods[fn.Name.Name]
+		isDataSource := tf.DataSourceMethods[fn.Name.Name]
 		if !isResource && !isDataSource {
 			return
 		}
@@ -88,15 +73,15 @@ func run(pass *analysis.Pass) (any, error) {
 			return
 		}
 
-		entries, allResolved := collectEntries(pass, fn.Body)
+		entries, allResolved := tf.RegistrationEntries(pass, fn.Body)
 		if isDataSource {
 			if !allResolved {
 				// an unresolvable data source name could hide a match — skip the package
 				resolvable = false
 				return
 			}
 			for _, e := range entries {
-				dataSources[e.name] = true
+				dataSources[e.Name] = true
 			}
 			return
 		}
@@ -111,11 +96,11 @@ func run(pass *analysis.Pass) (any, error) {
 
 	reported := map[string]bool{}
 	for _, r := range resources {
-		if dataSources[r.name] || reported[r.name] || isActionStyle(r.name) {
+		if dataSources[r.Name] || reported[r.Name] || isActionStyle(r.Name) {
 			continue
 		}
-		reported[r.name] = true
-		pass.Reportf(r.pos, "resource %q has no corresponding data source", r.name)
+		reported[r.Name] = true
+		pass.Reportf(r.Pos, "resource %q has no corresponding data source", r.Name)
 	}
 
 	return nil, nil
@@ -131,193 +116,3 @@ func isActionStyle(name string) bool {
 	}
 	return false
 }
-
-// collectEntries walks a registration method body and collects the terraform type name of
-// every registered entry: map literal keys, `m[key] = ...` assignments, slice literal
-// elements and `append(...)` arguments. It reports whether every encountered entry resolved
-// to a name.
-func collectEntries(pass *analysis.Pass, body *ast.BlockStmt) ([]entry, bool) {
-	var entries []entry
-	allResolved := true
-
-	add := func(name string, pos token.Pos, ok bool) {
-		if !ok {
-			allResolved = false
-			return
-		}
-		entries = append(entries, entry{name: name, pos: pos})
-	}
-
-	ast.Inspect(body, func(n ast.Node) bool {
-		switch node := n.(type) {
-		case *ast.CompositeLit:
-			t := types.Unalias(pass.TypesInfo.TypeOf(node))
-			if _, isMap := t.(*types.Map); isMap {
-				for _, elt := range node.Elts {
-					kv, ok := elt.(*ast.KeyValueExpr)
-					if !ok {
-						continue
-					}
-					name, ok := constantString(pass, kv.Key)
-					add(name, kv.Key.Pos(), ok)
-				}
-				return false
-			}
-			if _, isSlice := t.(*types.Slice); isSlice {
-				for _, elt := range node.Elts {
-					name, ok := resourceTypeOf(pass, elt)
-					add(name, elt.Pos(), ok)
-				}
-				return false
-			}
-		case *ast.AssignStmt:
-			// m["azurerm_x"] = resourceX() — conditional registration behind feature flags
-			for _, lhs := range node.Lhs {
-				idx, ok := lhs.(*ast.IndexExpr)
-				if !ok {
-					continue
-				}
-				if _, isMap := types.Unalias(pass.TypesInfo.TypeOf(idx.X)).(*types.Map); !isMap {
-					continue
-				}
-				name, ok := constantString(pass, idx.Index)
-				add(name, idx.Index.Pos(), ok)
-			}
-		case *ast.CallExpr:
-			// out = append(out, FooResource{}) — conditional registration behind feature flags
-			if fun, ok := node.Fun.(*ast.Ident); ok && fun.Name == "append" && len(node.Args) > 1 {
-				for _, arg := range node.Args[1:] {
-					// append(out, r.autoRegistration.DataSources()...) — delegation to another
-					// registration method in the same package (generated auto-registration);
-					// its entries are collected when that method's declaration is visited
-					if delegatesToRegistrationMethod(pass, arg) {
-						continue
-					}
-					name, ok := resourceTypeOf(pass, arg)
-					add(name, arg.Pos(), ok)
-				}
-				return false
-			}
-		}
-		return true
-	})
-
-	return entries, allResolved
-}
-
-// delegatesToRegistrationMethod reports whether expr is a call to another registration method
-// (`r.autoRegistration.DataSources()` etc.), whose entries are collected independently from
-// that method's own declaration in the package.
-func delegatesToRegistrationMethod(pass *analysis.Pass, expr ast.Expr) bool {
-	call, ok := ast.Unparen(expr).(*ast.CallExpr)
-	if !ok {
-		return false
-	}
-	sel, ok := call.Fun.(*ast.SelectorExpr)
-	if !ok {
-		return false
-	}
-	name := sel.Sel.Name
-	if !resourceMethods[name] && !dataSourceMethods[name] {
-		return false
-	}
-	_, isFunc := pass.TypesInfo.ObjectOf(sel.Sel).(*types.Func)
-	return isFunc
-}
-
-// constantString resolves expr to a constant string via the type checker, so literals and
-// named constants both work. As a fallback, a reference to a package-level `var` with a
-// constant string initializer (`var FooResourceName = "azurerm_foo"`) resolves to that
-// initializer, since some resources declare their type name that way for use with locks.
-func constantString(pass *analysis.Pass, expr ast.Expr) (string, bool) {
-	if tv, ok := pass.TypesInfo.Types[expr]; ok && tv.Value != nil && tv.Value.Kind() == constant.String {
-		return constant.StringVal(tv.Value), true
-	}
-
-	id, ok := ast.Unparen(expr).(*ast.Ident)
-	if !ok {
-		return "", false
-	}
-	v, ok := pass.TypesInfo.ObjectOf(id).(*types.Var)
-	if !ok || v.Pkg() != pass.Pkg || v.Parent() != pass.Pkg.Scope() {
-		return "", false
-	}
-
-	for _, file := range pass.Files {
-		for _, decl := range file.Decls {
-			gen, ok := decl.(*ast.GenDecl)
-			if !ok || gen.Tok != token.VAR {
-				continue
-			}
-			for _, spec := range gen.Specs {
-				vs, ok := spec.(*ast.ValueSpec)
-				if !ok || len(vs.Names) != len(vs.Values) {
-					continue
-				}
-				for i, name := range vs.Names {
-					if name.Name != id.Name {
-						continue
-					}
-					if tv, ok := pass.TypesInfo.Types[vs.Values[i]]; ok && tv.Value != nil && tv.Value.Kind() == constant.String {
-						return constant.StringVal(tv.Value), true
-					}
-					return "", false
-				}
-			}
-		}
-	}
-
-	return "", false
-}
-
-// resourceTypeOf resolves a typed/framework registration element (`FooResource{}` or
-// `&FooResource{}`) to the constant string its ResourceType() method returns.
-func resourceTypeOf(pass *analysis.Pass, elem ast.Expr) (string, bool) {
-	t := types.Unalias(pass.TypesInfo.TypeOf(elem))
-	if ptr, ok := t.(*types.Pointer); ok {
-		t = types.Unalias(ptr.Elem())
-	}
-	named, ok := t.(*types.Named)
-	if !ok {
-		return "", false
-	}
-
-	decl := resourceTypeMethod(pass, named.Obj().Name())
-	if decl == nil {
-		return "", false
-	}
-
-	var name string
-	found := false
-	ast.Inspect(decl.Body, func(n ast.Node) bool {
-		ret, ok := n.(*ast.ReturnStmt)
-		if !ok || found || len(ret.Results) != 1 {
-			return true
-		}
-		name, found = constantString(pass, ret.Results[0])
-		return !found
-	})
-
-	return name, found
-}
-
-// resourceTypeMethod finds the `func (r TypeName) ResourceType() string` declaration for the
-// named receiver type in the package being analysed.
-func resourceTypeMethod(pass *analysis.Pass, typeName string) *ast.FuncDecl {
-	for _, file := range pass.Files {
-		for _, decl := range file.Decls {
-			fn, ok := decl.(*ast.FuncDecl)
-			if !ok || fn.Name.Name != "ResourceType" || fn.Recv == nil || len(fn.Recv.List) != 1 || fn.Body == nil {
-				continue
-			}
-			recv := fn.Recv.List[0].Type
-			if star, ok := recv.(*ast.StarExpr); ok {
-				recv = star.X
-			}
-			if id, ok := recv.(*ast.Ident); ok && id.Name == typeName {
-				return fn
-			}
-		}
-	}
-	return nil
-}

checks/AZT/AZT.go

@@ -8,6 +8,7 @@ import (
 	AZT002 "github.com/katbyte/azproviderlint/checks/AZT/AZT002_credentials_from_environment"
 	AZT003 "github.com/katbyte/azproviderlint/checks/AZT/AZT003_misnamed_acceptance_test"
 	AZT004 "github.com/katbyte/azproviderlint/checks/AZT/AZT004_acceptance_test_not_named_test_acc"
+	AZT005 "github.com/katbyte/azproviderlint/checks/AZT/AZT005_resource_missing_acceptance_tests"
 	AZT006 "github.com/katbyte/azproviderlint/checks/AZT/AZT006_update_test_single_config"
 	AZT007 "github.com/katbyte/azproviderlint/checks/AZT/AZT007_stray_acc_in_test_name"
 )
@@ -18,6 +19,7 @@ var Checks = []*analysis.Analyzer{
 	AZT002.Analyzer,
 	AZT003.Analyzer,
 	AZT004.Analyzer,
+	AZT005.Analyzer,
 	AZT006.Analyzer,
 	AZT007.Analyzer,
 }