add AZT007: acceptance test names must not carry a second Acc

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Unreleased
 
+- 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))
 - 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))
 

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 |
+| [AZT007](checks/AZT/AZT007_stray_acc_in_test_name) | acceptance test names must not carry a second Acc |
 | [AZT004](checks/AZT/AZT004_acceptance_test_not_named_test_acc) | acceptance tests must be named TestAcc |
 
 ### AZN - Naming Conventions

checks/AZT/AZT.go

@@ -8,12 +8,14 @@ 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"
+	AZT007 "github.com/katbyte/azproviderlint/checks/AZT/AZT007_stray_acc_in_test_name"
 )
 
 // Checks contains all AZT (acceptance testing) analyzers.
 var Checks = []*analysis.Analyzer{
 	AZT001.Analyzer,
 	AZT002.Analyzer,
 	AZT003.Analyzer,
+	AZT007.Analyzer,
 	AZT004.Analyzer,
 }

checks/AZT/AZT007_stray_acc_in_test_name/AZT007.go

@@ -0,0 +1,52 @@
+// Package AZT007 defines an analyzer that reports acceptance test names carrying a second
+// Acc after the TestAcc prefix.
+package AZT007
+
+import (
+	"go/ast"
+	"regexp"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// Analyzer checks that a TestAcc name does not contain Acc again as a word of its own:
+// TestAccThing_storageAccBehindFirewall. Acc is the marker that makes a function an
+// acceptance test, so a second one reads as a mistake, and as an abbreviation of account it
+// is unclear. Account and Access are whole words and are left alone.
+var Analyzer = &analysis.Analyzer{
+	Name: "AZT007",
+	Doc:  "check that acceptance test names do not carry a second Acc after the TestAcc prefix",
+	URL:  "https://github.com/katbyte/azproviderlint/blob/main/checks/AZT/AZT007_stray_acc_in_test_name/README.md",
+	Run:  run,
+}
+
+// strayAcc matches Acc as a whole word inside a camel-case name: followed by an upper-case
+// letter, an underscore or the end, so Account and Access are not matched.
+var strayAcc = regexp.MustCompile(`Acc([A-Z_]|$)`)
+
+func run(pass *analysis.Pass) (any, error) {
+	for _, file := range pass.Files {
+		if !strings.HasSuffix(pass.Fset.Position(file.Pos()).Filename, "_test.go") {
+			continue
+		}
+
+		for _, decl := range file.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 && strayAcc.MatchString(rest) {
+				pass.Reportf(fd.Name.Pos(), "%s has a second Acc in its name, spell the word out so it is not read as another acceptance marker", fd.Name.Name)
+			}
+		}
+	}
+
+	return nil, nil
+}

checks/AZT/AZT007_stray_acc_in_test_name/AZT007_test.go

@@ -0,0 +1,16 @@
+package AZT007
+
+import (
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestAZT007(t *testing.T) {
+	t.Parallel()
+
+	_, filename, _, _ := runtime.Caller(0)
+	analysistest.Run(t, filepath.Join(filepath.Dir(filename), "testdata"), Analyzer, "azt007")
+}

checks/AZT/AZT007_stray_acc_in_test_name/README.md

@@ -0,0 +1,37 @@
+# AZT007 - acceptance test names must not carry a second Acc
+
+AZT007 reports a `TestAcc` or `testAcc` function whose name has `Acc` in it again as a word of its own, as in `TestAccThing_storageAccBehindFireWall`.
+
+`Acc` is the marker that makes a function an acceptance test. A second one reads as a mistake, and as an abbreviation of account it is unclear. Write the word out.
+
+## Flagged Code
+
+```go
+func TestAccMsSqlServerExtendedAuditingPolicy_storageAccBehindFireWall(t *testing.T) {
+```
+
+## Passing Code
+
+```go
+func TestAccMsSqlServerExtendedAuditingPolicy_storageAccountBehindFirewall(t *testing.T) {
+```
+
+## What counts
+
+`Acc` counts when it is a whole camel-case word: followed by an upper-case letter, an underscore, or the end of the name. `Account`, `Access` and `accelerated` are left alone, and so is the `Acc` of the prefix itself.
+
+Only `TestAcc` and lowercase `testAcc` functions in `_test.go` files are checked. A plain `Test` name with `Acc` in the wrong place, like `TestWebAppAccActiveSlot_basic`, is [AZT004](../AZT004_acceptance_test_not_named_test_acc)'s report when it runs an acceptance test.
+
+## No fix
+
+AZT007 only reports: whether `Acc` meant account, access or something else is the author's call.
+
+## Ignoring Reports
+
+Put `//azignore:AZT007 - <reason>` at the end of the line, or on the line above it. The reason is required.
+
+```go
+func TestAccThing_storageAccBehindFireWall(t *testing.T) { //azignore:AZT007 - <reason>
+```
+
+Under golangci-lint, `//nolint:azproviderlint` in the same place also works, but it silences every azproviderlint check on that line.

checks/AZT/AZT007_stray_acc_in_test_name/testdata/src/azt007/thing.go

@@ -0,0 +1,6 @@
+package azt007
+
+import "testing"
+
+// Should NOT be flagged: go test only runs functions from _test.go files
+func TestAccThing_storageAccNotATestFile(t *testing.T) {}

checks/AZT/AZT007_stray_acc_in_test_name/testdata/src/azt007/thing_test.go

@@ -0,0 +1,23 @@
+package azt007_test
+
+import "testing"
+
+// Should be flagged: Acc as an abbreviation inside the step name, in a test or a sequential helper
+func TestAccThing_storageAccBehindFireWall(t *testing.T) {} // want `TestAccThing_storageAccBehindFireWall has a second Acc in its name, spell the word out so it is not read as another acceptance marker`
+func testAccThing_storageAccBehindFireWall(t *testing.T) {} // want `testAccThing_storageAccBehindFireWall has a second Acc in its name, spell the word out so it is not read as another acceptance marker`
+func TestAccThing_withAcc(t *testing.T)                 {} // want `TestAccThing_withAcc has a second Acc in its name, spell the word out so it is not read as another acceptance marker`
+func TestAccAcc_basic(t *testing.T)                     {} // want `TestAccAcc_basic has a second Acc in its name, spell the word out so it is not read as another acceptance marker`
+
+// Should NOT be flagged: whole words that start with Acc
+func TestAccThing_storageAccountBehindFirewall(t *testing.T) {}
+func TestAccThing_publicAccess(t *testing.T)                 {}
+func TestAccAccount_basic(t *testing.T)                      {}
+func TestAccThing_accelerated(t *testing.T)                  {}
+
+// Should NOT be flagged: not acceptance test names, AZT004 covers those that should be
+func TestWebAppAccActiveSlot_basic(t *testing.T) {}
+func TestThing_storageAcc(t *testing.T)          {}
+
+type ThingResource struct{}
+
+func (r ThingResource) TestAccThing_storageAccMethod(t *testing.T) {}