From 83cb8e252b6a85c162499b234ba7c3e088437924 Mon Sep 17 00:00:00 2001 From: kt Date: Fri, 9 Oct 2026 14:01:53 -0700 Subject: [PATCH 1/4] add AZT005: resources and data sources must have basic, requiresImport, complete and update acceptance tests --- CHANGELOG.md | 1 + README.md | 1 + .../AZP002.go | 221 +------------- checks/AZT/AZT.go | 2 + .../AZT005.go | 269 ++++++++++++++++++ .../AZT005_test.go | 16 ++ .../README.md | 70 +++++ .../testdata/src/azt005/cond_resource.go | 11 + .../testdata/src/azt005/fixed_resource.go | 11 + .../src/azt005/fixed_resource_test.go | 6 + .../testdata/src/azt005/legacy_resource.go | 11 + .../testdata/src/azt005/misc_test.go | 13 + .../testdata/src/azt005/named_data_source.go | 7 + .../testdata/src/azt005/nilupdate_resource.go | 12 + .../src/azt005/nilupdate_resource_test.go | 6 + .../testdata/src/azt005/orphan_data_source.go | 9 + .../src/azt005/pluginsdk/pluginsdk.go | 6 + .../testdata/src/azt005/pointer_resource.go | 9 + .../src/azt005/pointer_resource_test.go | 7 + .../testdata/src/azt005/registration.go | 67 +++++ .../testdata/src/azt005/schema/schema.go | 16 ++ .../testdata/src/azt005/sdk/sdk.go | 16 ++ .../testdata/src/azt005/seq_resource.go | 12 + .../testdata/src/azt005/seq_resource_test.go | 10 + .../testdata/src/azt005/split_resource.go | 12 + .../src/azt005/split_resource_auth_test.go | 6 + .../src/azt005/split_resource_other_test.go | 7 + .../testdata/src/azt005/thing_data_source.go | 9 + .../src/azt005/thing_data_source_test.go | 5 + .../testdata/src/azt005/thing_resource.go | 12 + .../src/azt005/thing_resource_test.go | 8 + .../src/azt005/typed_fixed_resource.go | 13 + .../src/azt005/typed_fixed_resource_test.go | 5 + .../testdata/src/azt005/typed_resource.go | 17 ++ .../src/azt005/typed_resource_test.go | 7 + lib/tf/registration.go | 211 ++++++++++++++ 36 files changed, 908 insertions(+), 213 deletions(-) create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/README.md create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/cond_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/fixed_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/fixed_resource_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/legacy_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/misc_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/named_data_source.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/nilupdate_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/nilupdate_resource_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/orphan_data_source.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pluginsdk/pluginsdk.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pointer_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pointer_resource_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/registration.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/schema/schema.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/sdk/sdk.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/seq_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/seq_resource_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource_auth_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource_other_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_data_source.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_data_source_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_resource_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_fixed_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_fixed_resource_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_resource_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index aecd6f7..a284187 100644 --- a/CHANGELOG.md +++ b/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; report only, no fix - add rule `AZT003`: test functions named `TestAcc` that never run an acceptance test, so are scheduled as one without touching Azure; report only, no fix ([#64](https://github.com/katbyte/azproviderlint/pull/64)) ## v0.11.0 (2026-09-28) diff --git a/README.md b/README.md index ba48d62..e15072b 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,7 @@ Rules are named `AZ`, a category letter, and a number. The letters follow [tfpro | [AZT001](checks/AZT/AZT001_acceptance_test_external_package) | acceptance tests must use a _test package | | [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 | +| [AZT005](checks/AZT/AZT005_resource_missing_acceptance_tests) | resources and data sources must have basic, requiresImport, complete and update acceptance tests | ### AZN - Naming Conventions diff --git a/checks/AZP/AZP002_resource_missing_data_source/AZP002.go b/checks/AZP/AZP002_resource_missing_data_source/AZP002.go index 898c6e4..a6a3189 100644 --- a/checks/AZP/AZP002_resource_missing_data_source/AZP002.go +++ b/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,7 +73,7 @@ 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 @@ -96,7 +81,7 @@ func run(pass *analysis.Pass) (any, error) { 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 -} diff --git a/checks/AZT/AZT.go b/checks/AZT/AZT.go index c400309..e99a229 100644 --- a/checks/AZT/AZT.go +++ b/checks/AZT/AZT.go @@ -7,6 +7,7 @@ import ( AZT001 "github.com/katbyte/azproviderlint/checks/AZT/AZT001_acceptance_test_external_package" AZT002 "github.com/katbyte/azproviderlint/checks/AZT/AZT002_credentials_from_environment" AZT003 "github.com/katbyte/azproviderlint/checks/AZT/AZT003_misnamed_acceptance_test" + AZT005 "github.com/katbyte/azproviderlint/checks/AZT/AZT005_resource_missing_acceptance_tests" ) // Checks contains all AZT (acceptance testing) analyzers. @@ -14,4 +15,5 @@ var Checks = []*analysis.Analyzer{ AZT001.Analyzer, AZT002.Analyzer, AZT003.Analyzer, + AZT005.Analyzer, } diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go new file mode 100644 index 0000000..8747948 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go @@ -0,0 +1,269 @@ +// Package AZT005 defines an analyzer that reports registered resources and data sources +// missing the acceptance tests every one of them should have. +package AZT005 + +import ( + "go/ast" + "go/parser" + "go/token" + "go/types" + "os" + "path/filepath" + "strings" + "unicode" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + + "github.com/katbyte/azproviderlint/lib/astx" + "github.com/katbyte/azproviderlint/lib/tf" +) + +// Analyzer checks that every registered data source has a `basic` acceptance test and every +// registered resource has `basic` and `requiresImport` ones, plus `complete` and `update` +// when the resource can be updated in place. A resource can be updated when it declares an +// Update handler: the plugin SDK requires one exactly when a field is not ForceNew. +// +// The tests live in the package's `_test.go` files, which are a separate package, so they +// are read from disk: every `TestAcc`/`testAcc` function in a test file named after the +// file declaring the resource, or whose name spells the resource's type name, counts. +var Analyzer = &analysis.Analyzer{ + Name: "AZT005", + Doc: "check that resources and data sources have basic, requiresImport, complete and update acceptance tests", + URL: "https://github.com/katbyte/azproviderlint/blob/main/checks/AZT/AZT005_resource_missing_acceptance_tests/README.md", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +// The registration keys that give an untyped resource an Update handler. +var updateKeys = map[string]bool{"Update": true, "UpdateContext": true, "UpdateWithoutTimeout": true} + +// testFunc is one acceptance test function found in a test file: TestAccFoo_list_basic has +// prefix Foo and segments [list basic]. +type testFunc struct { + file string // base name + prefix string + segments []string +} + +func run(pass *analysis.Pass) (any, error) { + insp, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, nil + } + + funcDecls := map[*types.Func]*ast.FuncDecl{} + for _, file := range pass.Files { + for _, decl := range file.Decls { + if fd, ok := decl.(*ast.FuncDecl); ok && fd.Body != nil { + if fn, ok := pass.TypesInfo.Defs[fd.Name].(*types.Func); ok { + funcDecls[fn] = fd + } + } + } + } + + type registered struct { + tf.Entry + dataSource bool + } + var entries []registered + insp.Preorder([]ast.Node{(*ast.FuncDecl)(nil)}, func(n ast.Node) { + fn, ok := n.(*ast.FuncDecl) + if !ok || fn.Recv == nil || fn.Body == nil { + return + } + isResource := tf.ResourceMethods[fn.Name.Name] + isDataSource := tf.DataSourceMethods[fn.Name.Name] + if (!isResource && !isDataSource) || !tf.RegistrationReturnShape(pass, fn) { + return + } + found, _ := tf.RegistrationEntries(pass, fn.Body) + for _, e := range found { + entries = append(entries, registered{Entry: e, dataSource: isDataSource}) + } + }) + + testsByDir := map[string][]testFunc{} + seen := map[string]bool{} + for _, e := range entries { + kind := "resource" + if e.dataSource { + kind = "data source" + } + if seen[kind+e.Name] || e.Expr == nil { + continue + } + seen[kind+e.Name] = true + + file, updatable, ok := declaration(pass, funcDecls, e.Expr) + if !ok { + continue + } + dir := filepath.Dir(file) + tests, cached := testsByDir[dir] + if !cached { + tests = acceptanceTests(dir) + testsByDir[dir] = tests + } + + steps := []string{"basic"} + if !e.dataSource { + steps = append(steps, "requiresImport") + if updatable { + steps = append(steps, "complete", "update") + } + } + + // the tests for this entry: in a test file named after the declaring file, or named + // after the type name itself + stem := strings.TrimSuffix(filepath.Base(file), ".go") + "_" + typeName := strings.TrimPrefix(e.Name, "azurerm_") + prefix := "" + covered := map[string]bool{} + for _, t := range tests { + if !strings.HasPrefix(t.file, stem) && !spells(t.prefix, typeName, e.dataSource) { + continue + } + if prefix == "" { + prefix = t.prefix + } + for _, seg := range t.segments { + for _, step := range steps { + if strings.HasPrefix(strings.ToLower(seg), strings.ToLower(step)) { + covered[step] = true + } + } + } + } + if prefix == "" { + prefix = camel(typeName) + } + + var missing []string + for _, step := range steps { + if !covered[step] { + missing = append(missing, "TestAcc"+prefix+"_"+step) + } + } + if len(missing) == 0 { + continue + } + pass.Reportf(e.Pos, "%s %q is missing acceptance tests: %s", kind, e.Name, strings.Join(missing, ", ")) + } + + return nil, nil +} + +// declaration resolves a registered value to the file declaring the resource and whether the +// resource has an Update handler. An untyped `resourceFoo()` call resolves to the function, +// whose schema.Resource literal is read for an Update key; a typed or framework `FooResource{}` +// resolves to the type, which has an Update method or not. Anything declared outside the +// package is not resolved. +func declaration(pass *analysis.Pass, funcDecls map[*types.Func]*ast.FuncDecl, expr ast.Expr) (file string, updatable, ok bool) { + expr = ast.Unparen(expr) + if call, isCall := expr.(*ast.CallExpr); isCall { + fd := funcDecls[astx.CalledFunc(pass, call)] + if fd == nil { + return "", false, false + } + ast.Inspect(fd.Body, func(n ast.Node) bool { + cl, isLit := n.(*ast.CompositeLit) + if !isLit || !tf.IsSchemaHelperType(pass, cl, "Resource") { + return true + } + for _, elt := range cl.Elts { + kv, isKV := elt.(*ast.KeyValueExpr) + if !isKV { + continue + } + if key, isIdent := kv.Key.(*ast.Ident); isIdent && updateKeys[key.Name] && !astx.IsNilValue(pass, kv.Value) { + updatable = true + } + } + return true + }) + return pass.Fset.Position(fd.Pos()).Filename, updatable, true + } + + t := types.Unalias(pass.TypesInfo.TypeOf(expr)) + if ptr, isPtr := t.(*types.Pointer); isPtr { + t = types.Unalias(ptr.Elem()) + } + named, isNamed := t.(*types.Named) + if !isNamed || named.Obj().Pkg() != pass.Pkg { + return "", false, false + } + update, _, _ := types.LookupFieldOrMethod(named, true, pass.Pkg, "Update") + _, updatable = update.(*types.Func) + return pass.Fset.Position(named.Obj().Pos()).Filename, updatable, true +} + +// acceptanceTests parses every _test.go file in dir and returns its TestAcc/testAcc +// functions. Test files belong to a separate package, so they are read from disk rather than +// from the pass. A file that does not parse contributes nothing. +func acceptanceTests(dir string) []testFunc { + names, err := os.ReadDir(dir) + if err != nil { + return nil + } + var tests []testFunc + fset := token.NewFileSet() + for _, entry := range names { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), "_test.go") { + continue + } + f, err := parser.ParseFile(fset, filepath.Join(dir, entry.Name()), nil, parser.SkipObjectResolution) + if err != nil { + continue + } + for _, decl := range f.Decls { + fd, ok := decl.(*ast.FuncDecl) + if !ok || fd.Recv != nil { + continue + } + rest, ok := strings.CutPrefix(fd.Name.Name, "TestAcc") + if !ok { + rest, ok = strings.CutPrefix(fd.Name.Name, "testAcc") + } + if !ok { + continue + } + parts := strings.Split(rest, "_") + tests = append(tests, testFunc{file: entry.Name(), prefix: parts[0], segments: parts[1:]}) + } + } + return tests +} + +// spells reports whether a test name prefix spells the terraform type name (without its +// provider prefix), ignoring case and underscores: TestAccMsSqlServer for mssql_server. A +// leading AzureRM, as older tests carry, and a Resource suffix are allowed; a data source +// may carry DataSource at either end. +func spells(prefix, typeName string, dataSource bool) bool { + p := strings.TrimPrefix(strings.ToLower(prefix), "azurerm") + want := strings.ReplaceAll(typeName, "_", "") + if dataSource { + return p == want || p == "datasource"+want || p == want+"datasource" + } + return p == want || p == want+"resource" +} + +// camel turns foo_bar into FooBar, the test name prefix a new resource would use. +func camel(typeName string) string { + var b strings.Builder + upper := true + for _, r := range typeName { + if r == '_' { + upper = true + continue + } + if upper { + r = unicode.ToUpper(r) + upper = false + } + b.WriteRune(r) + } + return b.String() +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005_test.go new file mode 100644 index 0000000..5f6760f --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005_test.go @@ -0,0 +1,16 @@ +package AZT005 + +import ( + "path/filepath" + "runtime" + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAZT005(t *testing.T) { + t.Parallel() + + _, filename, _, _ := runtime.Caller(0) + analysistest.Run(t, filepath.Join(filepath.Dir(filename), "testdata"), Analyzer, "azt005") +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/README.md b/checks/AZT/AZT005_resource_missing_acceptance_tests/README.md new file mode 100644 index 0000000..f01b1d4 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/README.md @@ -0,0 +1,70 @@ +# AZT005 - resources and data sources must have basic, requiresImport, complete and update acceptance tests + +AZT005 reports a registered data source with no `basic` acceptance test, and a registered resource missing any of `basic`, `requiresImport`, and, when the resource can be updated in place, `complete` and `update`. + +These are the tests every resource is expected to ship with. `basic` proves it can be created and read back, `requiresImport` proves an existing object is not silently adopted, `complete` proves every field round-trips, and `update` proves changes apply without recreating the object. + +## Flagged Code + +```go +func (r Registration) SupportedResources() map[string]*pluginsdk.Resource { + return map[string]*pluginsdk.Resource{ + "azurerm_example": resourceExample(), // example_resource_test.go has only TestAccExample_basic + } +} + +func resourceExample() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Create: resourceExampleCreate, + Read: resourceExampleRead, + Update: resourceExampleUpdate, + Delete: resourceExampleDelete, + } +} +``` + +```text +resource "azurerm_example" is missing acceptance tests: TestAccExample_requiresImport, TestAccExample_complete, TestAccExample_update +``` + +## Passing Code + +```go +// example_resource_test.go +func TestAccExample_basic(t *testing.T) { /* ... */ } +func TestAccExample_requiresImport(t *testing.T) { /* ... */ } +func TestAccExample_complete(t *testing.T) { /* ... */ } +func TestAccExample_update(t *testing.T) { /* ... */ } + +// example_data_source_test.go +func TestAccExampleDataSource_basic(t *testing.T) { /* ... */ } +``` + +## What counts + +Every registration style is read: untyped `SupportedResources()` / `SupportedDataSources()` map keys, typed `Resources()` / `DataSources()` elements, and `FrameworkResources()` elements, named by their `ResourceType()` methods. Entries registered behind a feature flag count too. + +A resource can be updated when it declares an Update handler: an `Update`, `UpdateContext` or `UpdateWithoutTimeout` key on an untyped resource, or an `Update` method on a typed or framework one. The plugin SDK only allows an Update handler when some field is not ForceNew, so the two are the same thing. A resource without one needs only `basic` and `requiresImport`. + +Test files are a separate package, so they are read from disk. Every `TestAcc` or lowercase `testAcc` function counts when it is in a test file named after the file declaring the resource (`example_resource.go` covers `example_resource_test.go` and `example_resource_other_test.go`), or when its name spells the type name without `azurerm_`, ignoring case and underscores (`TestAccMsSqlServer` for `azurerm_mssql_server`, with an older `AzureRM` prefix, a `Resource` suffix, or `DataSource` at either end of a data source name also accepted). + +The test name's segments after the first underscore name its steps. A segment that starts with the step name counts, so `TestAccExample_completeWithTags`, `TestAccExample_updateSku` and `TestAccExample_list_basic` all cover their steps. + +Not reported: + +- entries whose value is declared in another package, or cannot be resolved +- a resource whose Update handler is set to `nil`, which is treated as having none + +## The fix + +AZT005 only reports. Add the missing tests, named for the steps they cover. + +## Ignoring Reports + +Put `//azignore:AZT005 - ` at the end of the registration line, or on the line above it. The reason is required. + +```go +"azurerm_example": resourceExample(), //azignore:AZT005 - +``` + +Under golangci-lint, `//nolint:azproviderlint` in the same place also works, but it silences every azproviderlint check on that line. diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/cond_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/cond_resource.go new file mode 100644 index 0000000..a28eb1e --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/cond_resource.go @@ -0,0 +1,11 @@ +package azt005 + +import "azt005/pluginsdk" + +func resourceCond() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Create: noop, + Read: noop, + Delete: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/fixed_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/fixed_resource.go new file mode 100644 index 0000000..918d99f --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/fixed_resource.go @@ -0,0 +1,11 @@ +package azt005 + +import "azt005/pluginsdk" + +func resourceFixed() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Create: noop, + Read: noop, + Delete: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/fixed_resource_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/fixed_resource_test.go new file mode 100644 index 0000000..37b7245 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/fixed_resource_test.go @@ -0,0 +1,6 @@ +package azt005_test + +import "testing" + +func TestAccFixed_basic(t *testing.T) {} +func TestAccFixed_RequiresImport(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/legacy_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/legacy_resource.go new file mode 100644 index 0000000..2b62777 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/legacy_resource.go @@ -0,0 +1,11 @@ +package azt005 + +import "azt005/pluginsdk" + +func resourceLegacy() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Create: noop, + Read: noop, + Delete: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/misc_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/misc_test.go new file mode 100644 index 0000000..821636b --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/misc_test.go @@ -0,0 +1,13 @@ +package azt005_test + +import "testing" + +// found by name: the older AzureRM prefix and a Resource suffix are both accepted +func TestAccAzureRMLegacy_basic(t *testing.T) {} +func TestAccLegacyResource_requiresImport(t *testing.T) {} + +// a data source may carry DataSource at either end +func TestAccDataSourceNamed_basic(t *testing.T) {} + +// a different resource's tests never count for another +func TestAccTypedOther_list_update(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/named_data_source.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/named_data_source.go new file mode 100644 index 0000000..7836a23 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/named_data_source.go @@ -0,0 +1,7 @@ +package azt005 + +type NamedDataSource struct{} + +func (r NamedDataSource) ResourceType() string { + return "azurerm_named" +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/nilupdate_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/nilupdate_resource.go new file mode 100644 index 0000000..535de4d --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/nilupdate_resource.go @@ -0,0 +1,12 @@ +package azt005 + +import "azt005/pluginsdk" + +func resourceNilUpdate() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Create: noop, + Read: noop, + Update: nil, + Delete: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/nilupdate_resource_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/nilupdate_resource_test.go new file mode 100644 index 0000000..678ccf3 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/nilupdate_resource_test.go @@ -0,0 +1,6 @@ +package azt005_test + +import "testing" + +func TestAccNilUpdate_basic(t *testing.T) {} +func TestAccNilUpdate_requiresImport(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/orphan_data_source.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/orphan_data_source.go new file mode 100644 index 0000000..aa2c64e --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/orphan_data_source.go @@ -0,0 +1,9 @@ +package azt005 + +import "azt005/pluginsdk" + +func dataSourceOrphan() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Read: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pluginsdk/pluginsdk.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pluginsdk/pluginsdk.go new file mode 100644 index 0000000..c52d12a --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pluginsdk/pluginsdk.go @@ -0,0 +1,6 @@ +// Package pluginsdk mirrors terraform-provider-azurerm's internal/tf/pluginsdk type aliases. +package pluginsdk + +import "azt005/schema" + +type Resource = schema.Resource diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pointer_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pointer_resource.go new file mode 100644 index 0000000..569b3ee --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pointer_resource.go @@ -0,0 +1,9 @@ +package azt005 + +type PointerResource struct{} + +func (r *PointerResource) ResourceType() string { + return "azurerm_pointer" +} + +func (r *PointerResource) Update() {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pointer_resource_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pointer_resource_test.go new file mode 100644 index 0000000..8885e7a --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/pointer_resource_test.go @@ -0,0 +1,7 @@ +package azt005_test + +import "testing" + +func TestAccPointer_basic(t *testing.T) {} +func TestAccPointer_requiresImport(t *testing.T) {} +func TestAccPointer_update(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/registration.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/registration.go new file mode 100644 index 0000000..0d44906 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/registration.go @@ -0,0 +1,67 @@ +package azt005 + +import ( + "azt005/pluginsdk" + "azt005/schema" + "azt005/sdk" +) + +type Registration struct{} + +func (r Registration) SupportedResources() map[string]*pluginsdk.Resource { + resources := map[string]*pluginsdk.Resource{ + "azurerm_thing": resourceThing(), // basic, requiresImport, complete, update all present + "azurerm_fixed": resourceFixed(), // no Update: basic and requiresImport are enough + "azurerm_nilupdate": resourceNilUpdate(), // Update: nil counts as no Update + "azurerm_split": resourceSplit(), // tests split over several files named after the resource file + "azurerm_legacy": resourceLegacy(), // tests in an unrelated file, found by name + "azurerm_seq": resourceSeq(), // sequential: lowercase testAcc subtests + "azurerm_external": schema.External(), // declared elsewhere: skipped + } + + if featureFlag() { + resources["azurerm_cond"] = resourceCond() // want `resource "azurerm_cond" is missing acceptance tests: TestAccCond_basic, TestAccCond_requiresImport` + } + + return resources +} + +func (r Registration) SupportedDataSources() map[string]*pluginsdk.Resource { + return map[string]*pluginsdk.Resource{ + "azurerm_thing": dataSourceThing(), + "azurerm_orphan": dataSourceOrphan(), // want `data source "azurerm_orphan" is missing acceptance tests: TestAccOrphan_basic` + } +} + +func (r Registration) Resources() []sdk.Resource { + out := []sdk.Resource{ + TypedResource{}, // want `resource "azurerm_typed" is missing acceptance tests: TestAccTyped_update` + TypedFixedResource{}, // want `resource "azurerm_typed_fixed" is missing acceptance tests: TestAccTypedFixed_requiresImport` + } + + // an element whose static type has no declaration here is skipped + var extra sdk.Resource + out = append(out, extra) + + return out +} + +func (r Registration) DataSources() []sdk.DataSource { + return []sdk.DataSource{ + NamedDataSource{}, + } +} + +func (r Registration) FrameworkResources() []sdk.FrameworkWrappedResource { + return []sdk.FrameworkWrappedResource{ + &PointerResource{}, // want `resource "azurerm_pointer" is missing acceptance tests: TestAccPointer_complete` + } +} + +func featureFlag() bool { + return false +} + +func noop() error { + return nil +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/schema/schema.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/schema/schema.go new file mode 100644 index 0000000..d2a9f8f --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/schema/schema.go @@ -0,0 +1,16 @@ +// Package schema is a minimal stub of the plugin SDK's helper/schema for the analyzer tests. +package schema + +type ResourceFunc func() error + +type Resource struct { + Create ResourceFunc + Read ResourceFunc + Update ResourceFunc + Delete ResourceFunc +} + +// External stands in for a resource declared in another package. +func External() *Resource { + return &Resource{} +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/sdk/sdk.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/sdk/sdk.go new file mode 100644 index 0000000..b6a4c72 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/sdk/sdk.go @@ -0,0 +1,16 @@ +// Package sdk is a minimal stand-in for azurerm's typed and framework SDK types. +package sdk + +type ResourceFunc struct{} + +type Resource interface { + ResourceType() string +} + +type DataSource interface { + ResourceType() string +} + +type FrameworkWrappedResource interface { + ResourceType() string +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/seq_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/seq_resource.go new file mode 100644 index 0000000..61c2036 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/seq_resource.go @@ -0,0 +1,12 @@ +package azt005 + +import "azt005/pluginsdk" + +func resourceSeq() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Create: noop, + Read: noop, + Update: noop, + Delete: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/seq_resource_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/seq_resource_test.go new file mode 100644 index 0000000..3f6c618 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/seq_resource_test.go @@ -0,0 +1,10 @@ +package azt005_test + +import "testing" + +func TestAccSeqSequential(t *testing.T) {} + +func testAccSeq_basic(t *testing.T) {} +func testAccSeq_requiresImport(t *testing.T) {} +func testAccSeq_complete(t *testing.T) {} +func testAccSeq_update(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource.go new file mode 100644 index 0000000..d7f6192 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource.go @@ -0,0 +1,12 @@ +package azt005 + +import "azt005/pluginsdk" + +func resourceSplit() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Create: noop, + Read: noop, + Update: noop, + Delete: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource_auth_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource_auth_test.go new file mode 100644 index 0000000..fb8bed4 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource_auth_test.go @@ -0,0 +1,6 @@ +package azt005_test + +import "testing" + +func TestAccSplit_basic(t *testing.T) {} +func TestAccSplit_requiresImport(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource_other_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource_other_test.go new file mode 100644 index 0000000..d86d25b --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/split_resource_other_test.go @@ -0,0 +1,7 @@ +package azt005_test + +import "testing" + +// a step name that starts the segment counts: completeWithTags, updateSku, list_update +func TestAccSplit_completeWithTags(t *testing.T) {} +func TestAccSplit_list_updateSku(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_data_source.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_data_source.go new file mode 100644 index 0000000..4a2d7f5 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_data_source.go @@ -0,0 +1,9 @@ +package azt005 + +import "azt005/pluginsdk" + +func dataSourceThing() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Read: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_data_source_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_data_source_test.go new file mode 100644 index 0000000..050f633 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_data_source_test.go @@ -0,0 +1,5 @@ +package azt005_test + +import "testing" + +func TestAccThingDataSource_basic(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_resource.go new file mode 100644 index 0000000..e6cabd5 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_resource.go @@ -0,0 +1,12 @@ +package azt005 + +import "azt005/pluginsdk" + +func resourceThing() *pluginsdk.Resource { + return &pluginsdk.Resource{ + Create: noop, + Read: noop, + Update: noop, + Delete: noop, + } +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_resource_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_resource_test.go new file mode 100644 index 0000000..d833d90 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/thing_resource_test.go @@ -0,0 +1,8 @@ +package azt005_test + +import "testing" + +func TestAccThing_basic(t *testing.T) {} +func TestAccThing_requiresImport(t *testing.T) {} +func TestAccThing_complete(t *testing.T) {} +func TestAccThing_update(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_fixed_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_fixed_resource.go new file mode 100644 index 0000000..279919d --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_fixed_resource.go @@ -0,0 +1,13 @@ +package azt005 + +import "azt005/sdk" + +type TypedFixedResource struct{} + +func (r TypedFixedResource) ResourceType() string { + return "azurerm_typed_fixed" +} + +func (r TypedFixedResource) Create() sdk.ResourceFunc { + return sdk.ResourceFunc{} +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_fixed_resource_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_fixed_resource_test.go new file mode 100644 index 0000000..edb6841 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_fixed_resource_test.go @@ -0,0 +1,5 @@ +package azt005_test + +import "testing" + +func TestAccTypedFixed_basic(t *testing.T) {} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_resource.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_resource.go new file mode 100644 index 0000000..a4efaba --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_resource.go @@ -0,0 +1,17 @@ +package azt005 + +import "azt005/sdk" + +type TypedResource struct{} + +func (r TypedResource) ResourceType() string { + return "azurerm_typed" +} + +func (r TypedResource) Create() sdk.ResourceFunc { + return sdk.ResourceFunc{} +} + +func (r TypedResource) Update() sdk.ResourceFunc { + return sdk.ResourceFunc{} +} diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_resource_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_resource_test.go new file mode 100644 index 0000000..a690362 --- /dev/null +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005/typed_resource_test.go @@ -0,0 +1,7 @@ +package azt005_test + +import "testing" + +func TestAccTyped_basic(t *testing.T) {} +func TestAccTyped_requiresImport(t *testing.T) {} +func TestAccTyped_complete(t *testing.T) {} diff --git a/lib/tf/registration.go b/lib/tf/registration.go index e1cf225..da2a918 100644 --- a/lib/tf/registration.go +++ b/lib/tf/registration.go @@ -4,11 +4,20 @@ package tf import ( "go/ast" + "go/constant" + "go/token" "go/types" "golang.org/x/tools/go/analysis" ) +// Registration method names for each side; the untyped map methods and the typed/framework +// slice methods are distinguished by their return shape, see RegistrationReturnShape. +var ( + ResourceMethods = map[string]bool{"SupportedResources": true, "Resources": true, "FrameworkResources": true} + DataSourceMethods = map[string]bool{"SupportedDataSources": true, "DataSources": true, "FrameworkDataSources": true} +) + // RegistrationReturnShape reports whether fn returns either a map[string]*Resource (untyped // registration) or a slice of a named Resource/DataSource/FrameworkWrapped* type // (typed/framework registration), which guards against unrelated methods sharing the names. @@ -43,3 +52,205 @@ func RegistrationReturnShape(pass *analysis.Pass, fn *ast.FuncDecl) bool { return false } + +// Entry is one registered resource or data source. +type Entry struct { + Name string // the terraform type name, "azurerm_foo" + Pos token.Pos // the map key or slice element that registers it + Expr ast.Expr // the registered value: the call in `"azurerm_foo": resourceFoo()`, the element `FooResource{}`; nil when unknown +} + +// RegistrationEntries 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. Typed and framework elements are named by +// their `ResourceType()` method. It reports whether every encountered entry resolved to a +// name. +func RegistrationEntries(pass *analysis.Pass, body *ast.BlockStmt) ([]Entry, bool) { + var entries []Entry + allResolved := true + + add := func(name string, pos token.Pos, expr ast.Expr, ok bool) { + if !ok { + allResolved = false + return + } + entries = append(entries, Entry{Name: name, Pos: pos, Expr: expr}) + } + + 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(), kv.Value, ok) + } + return false + } + if _, isSlice := t.(*types.Slice); isSlice { + for _, elt := range node.Elts { + name, ok := resourceTypeOf(pass, elt) + add(name, elt.Pos(), elt, ok) + } + return false + } + case *ast.AssignStmt: + // m["azurerm_x"] = resourceX() — conditional registration behind feature flags + for i, 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 + } + var value ast.Expr + if len(node.Rhs) == len(node.Lhs) { + value = node.Rhs[i] + } + name, ok := constantString(pass, idx.Index) + add(name, idx.Index.Pos(), value, 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(), arg, 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 +} From 26095734986c4a550ff766a8c46b6ebd14be689e Mon Sep 17 00:00:00 2001 From: kt Date: Fri, 9 Oct 2026 14:02:12 -0700 Subject: [PATCH 2/4] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a284187..0e14efd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +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; report only, no fix +- 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; report only, no fix ([#68](https://github.com/katbyte/azproviderlint/pull/68)) - add rule `AZT003`: test functions named `TestAcc` that never run an acceptance test, so are scheduled as one without touching Azure; report only, no fix ([#64](https://github.com/katbyte/azproviderlint/pull/64)) ## v0.11.0 (2026-09-28) From 96463e833cfd7cd5db53777309767c686cbabbe6 Mon Sep 17 00:00:00 2001 From: kt Date: Fri, 9 Oct 2026 14:17:13 -0700 Subject: [PATCH 3/4] join split lines, blank lines between steps --- .../AZT005.go | 13 +++++++++++++ lib/tf/registration.go | 9 +++++++++ 2 files changed, 22 insertions(+) diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go index 8747948..bab5ae8 100644 --- a/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go @@ -74,11 +74,13 @@ func run(pass *analysis.Pass) (any, error) { if !ok || fn.Recv == nil || fn.Body == nil { return } + isResource := tf.ResourceMethods[fn.Name.Name] isDataSource := tf.DataSourceMethods[fn.Name.Name] if (!isResource && !isDataSource) || !tf.RegistrationReturnShape(pass, fn) { return } + found, _ := tf.RegistrationEntries(pass, fn.Body) for _, e := range found { entries = append(entries, registered{Entry: e, dataSource: isDataSource}) @@ -95,12 +97,14 @@ func run(pass *analysis.Pass) (any, error) { if seen[kind+e.Name] || e.Expr == nil { continue } + seen[kind+e.Name] = true file, updatable, ok := declaration(pass, funcDecls, e.Expr) if !ok { continue } + dir := filepath.Dir(file) tests, cached := testsByDir[dir] if !cached { @@ -137,6 +141,7 @@ func run(pass *analysis.Pass) (any, error) { } } } + if prefix == "" { prefix = camel(typeName) } @@ -147,9 +152,11 @@ func run(pass *analysis.Pass) (any, error) { missing = append(missing, "TestAcc"+prefix+"_"+step) } } + if len(missing) == 0 { continue } + pass.Reportf(e.Pos, "%s %q is missing acceptance tests: %s", kind, e.Name, strings.Join(missing, ", ")) } @@ -191,10 +198,12 @@ func declaration(pass *analysis.Pass, funcDecls map[*types.Func]*ast.FuncDecl, e if ptr, isPtr := t.(*types.Pointer); isPtr { t = types.Unalias(ptr.Elem()) } + named, isNamed := t.(*types.Named) if !isNamed || named.Obj().Pkg() != pass.Pkg { return "", false, false } + update, _, _ := types.LookupFieldOrMethod(named, true, pass.Pkg, "Update") _, updatable = update.(*types.Func) return pass.Fset.Position(named.Obj().Pos()).Filename, updatable, true @@ -208,6 +217,7 @@ func acceptanceTests(dir string) []testFunc { if err != nil { return nil } + var tests []testFunc fset := token.NewFileSet() for _, entry := range names { @@ -223,6 +233,7 @@ func acceptanceTests(dir string) []testFunc { if !ok || fd.Recv != nil { continue } + rest, ok := strings.CutPrefix(fd.Name.Name, "TestAcc") if !ok { rest, ok = strings.CutPrefix(fd.Name.Name, "testAcc") @@ -230,10 +241,12 @@ func acceptanceTests(dir string) []testFunc { if !ok { continue } + parts := strings.Split(rest, "_") tests = append(tests, testFunc{file: entry.Name(), prefix: parts[0], segments: parts[1:]}) } } + return tests } diff --git a/lib/tf/registration.go b/lib/tf/registration.go index da2a918..fc47a8d 100644 --- a/lib/tf/registration.go +++ b/lib/tf/registration.go @@ -33,10 +33,12 @@ func RegistrationReturnShape(pass *analysis.Pass, fn *ast.FuncDecl) bool { if !ok || basic.Kind() != types.String { return false } + ptr, ok := types.Unalias(t.Elem()).(*types.Pointer) if !ok { return false } + named, ok := types.Unalias(ptr.Elem()).(*types.Named) return ok && named.Obj().Name() == "Resource" case *types.Slice: @@ -109,10 +111,12 @@ func RegistrationEntries(pass *analysis.Pass, body *ast.BlockStmt) ([]Entry, boo if _, isMap := types.Unalias(pass.TypesInfo.TypeOf(idx.X)).(*types.Map); !isMap { continue } + var value ast.Expr if len(node.Rhs) == len(node.Lhs) { value = node.Rhs[i] } + name, ok := constantString(pass, idx.Index) add(name, idx.Index.Pos(), value, ok) } @@ -146,14 +150,17 @@ func delegatesToRegistrationMethod(pass *analysis.Pass, expr ast.Expr) bool { 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 } @@ -171,6 +178,7 @@ func constantString(pass *analysis.Pass, expr ast.Expr) (string, bool) { 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 @@ -210,6 +218,7 @@ func resourceTypeOf(pass *analysis.Pass, elem ast.Expr) (string, bool) { if ptr, ok := t.(*types.Pointer); ok { t = types.Unalias(ptr.Elem()) } + named, ok := t.(*types.Named) if !ok { return "", false From 6af86acd3c6483ec9f6722545f599be5256a1cae Mon Sep 17 00:00:00 2001 From: kt Date: Fri, 9 Oct 2026 16:37:53 -0700 Subject: [PATCH 4/4] AZT005 - disable option to enforce one step at a time --- CHANGELOG.md | 2 +- .../AZT005.go | 31 +++++++++++++++++++ .../AZT005_test.go | 8 ++++- .../README.md | 15 +++++++++ .../src/azt005disable/partial_resource.go | 12 +++++++ .../azt005disable/partial_resource_test.go | 5 +++ .../src/azt005disable/pluginsdk/pluginsdk.go | 6 ++++ .../src/azt005disable/registration.go | 17 ++++++++++ .../src/azt005disable/schema/schema.go | 16 ++++++++++ .../src/azt005disable/untested_resource.go | 12 +++++++ 10 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005disable/partial_resource.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005disable/partial_resource_test.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005disable/pluginsdk/pluginsdk.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005disable/registration.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005disable/schema/schema.go create mode 100644 checks/AZT/AZT005_resource_missing_acceptance_tests/testdata/src/azt005disable/untested_resource.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e14efd..f21ce7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +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; report only, no fix ([#68](https://github.com/katbyte/azproviderlint/pull/68)) +- 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 `AZT003`: test functions named `TestAcc` that never run an acceptance test, so are scheduled as one without touching Azure; report only, no fix ([#64](https://github.com/katbyte/azproviderlint/pull/64)) ## v0.11.0 (2026-09-28) diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go index bab5ae8..250133e 100644 --- a/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005.go @@ -3,12 +3,15 @@ package AZT005 import ( + "flag" + "fmt" "go/ast" "go/parser" "go/token" "go/types" "os" "path/filepath" + "slices" "strings" "unicode" @@ -39,6 +42,29 @@ var Analyzer = &analysis.Analyzer{ // The registration keys that give an untyped resource an Update handler. var updateKeys = map[string]bool{"Update": true, "UpdateContext": true, "UpdateWithoutTimeout": true} +// The steps a test can be named for, in the order they are reported. +var allSteps = []string{"basic", "requiresImport", "complete", "update"} + +// disabled holds the steps the `disable` option turned off, so a consumer can enforce the +// rule one step at a time. +var disabled = map[string]bool{} + +func init() { + Analyzer.Flags.Init("AZT005", flag.ContinueOnError) + Analyzer.Flags.Func("disable", "a step not to ask for: basic, requiresImport, complete or update; repeat or comma-separate for several", func(s string) error { + for step := range strings.SplitSeq(s, ",") { + step = strings.TrimSpace(step) + if !slices.Contains(allSteps, step) { + return fmt.Errorf("unknown step %q, expected one of %s", step, strings.Join(allSteps, ", ")) + } + + disabled[step] = true + } + + return nil + }) +} + // testFunc is one acceptance test function found in a test file: TestAccFoo_list_basic has // prefix Foo and segments [list basic]. type testFunc struct { @@ -120,6 +146,11 @@ func run(pass *analysis.Pass) (any, error) { } } + steps = slices.DeleteFunc(steps, func(step string) bool { return disabled[step] }) + if len(steps) == 0 { + continue + } + // the tests for this entry: in a test file named after the declaring file, or named // after the type name itself stem := strings.TrimSuffix(filepath.Base(file), ".go") + "_" diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005_test.go b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005_test.go index 5f6760f..d3c27e9 100644 --- a/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005_test.go +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/AZT005_test.go @@ -12,5 +12,11 @@ func TestAZT005(t *testing.T) { t.Parallel() _, filename, _, _ := runtime.Caller(0) - analysistest.Run(t, filepath.Join(filepath.Dir(filename), "testdata"), Analyzer, "azt005") + dir := filepath.Join(filepath.Dir(filename), "testdata") + + analysistest.Run(t, dir, Analyzer, "azt005") + + disabled = map[string]bool{"requiresImport": true, "complete": true, "update": true} + analysistest.Run(t, dir, Analyzer, "azt005disable") + disabled = map[string]bool{} } diff --git a/checks/AZT/AZT005_resource_missing_acceptance_tests/README.md b/checks/AZT/AZT005_resource_missing_acceptance_tests/README.md index f01b1d4..49e56e4 100644 --- a/checks/AZT/AZT005_resource_missing_acceptance_tests/README.md +++ b/checks/AZT/AZT005_resource_missing_acceptance_tests/README.md @@ -59,6 +59,21 @@ Not reported: AZT005 only reports. Add the missing tests, named for the steps they cover. +## Options + +| Option | Default | Effect | +|---|---|---| +| `disable` | | a step not to ask for: `basic`, `requiresImport`, `complete` or `update` | + +`disable` takes several values: a list in the golangci settings, or the flag repeated or comma-separated on the command line. Every step is asked for by default; turning the others off lets a provider enforce `basic` first and add the rest as the backlog clears. + +```yaml + AZT005: + disable: [complete, update] +``` + +Set with `-AZT005.