add AZG012: separate the steps of a longer function body with blank lines

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Unreleased
 
+- add rule `AZG012`: in a body of more than five statements, an `if`/`for`/`switch`/`select` must be followed by a blank line before the next step; short bodies and runs of one-line guards are left alone; fixable with `-fix`; `statements` and `lines` set what counts as short ([#71](https://github.com/katbyte/azproviderlint/pull/71))
 - 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)

README.md

@@ -49,6 +49,7 @@ Rules are named `AZ`, a category letter, and a number. The letters follow [tfpro
 | [AZG008](checks/AZG/AZG008_unchecked_nil_dereference) | pointer dereferences must have a nil guard or use pointer.From |
 | [AZG009](checks/AZG/AZG009_pointer_from_enum_conversion) | use pointer.FromEnum for enum conversions |
 | [AZG010](checks/AZG/AZG010_unneeded_import_alias) | drop import aliases that are not needed |
+| [AZG012](checks/AZG/AZG012_missing_blank_line_between_steps) | separate the steps of a longer function body with blank lines |
 
 ### AZP - Provider-Wide Conventions
 

checks/AZG/AZG.go

@@ -14,6 +14,7 @@ import (
 	AZG008 "github.com/katbyte/azproviderlint/checks/AZG/AZG008_unchecked_nil_dereference"
 	AZG009 "github.com/katbyte/azproviderlint/checks/AZG/AZG009_pointer_from_enum_conversion"
 	AZG010 "github.com/katbyte/azproviderlint/checks/AZG/AZG010_unneeded_import_alias"
+	AZG012 "github.com/katbyte/azproviderlint/checks/AZG/AZG012_missing_blank_line_between_steps"
 )
 
 // Checks contains all AZG (general Go style & readability) analyzers.
@@ -28,4 +29,5 @@ var Checks = []*analysis.Analyzer{
 	AZG008.Analyzer,
 	AZG009.Analyzer,
 	AZG010.Analyzer,
+	AZG012.Analyzer,
 }

checks/AZG/AZG012_missing_blank_line_between_steps/AZG012.go

@@ -0,0 +1,131 @@
+// Package AZG012 defines an analyzer that reports a block statement run straight into the
+// next step of a longer function body without a blank line between them.
+package AZG012
+
+import (
+	"flag"
+	"go/ast"
+	"go/token"
+
+	"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"
+)
+
+// Analyzer checks that in a body with more than a few statements, an if, for, switch or select
+// is separated from the statement that follows it by a blank line, so a reader sees where one
+// step ends and the next begins. A short body is one group and is left alone, as is a run of
+// one-line ifs, which read as a single set of guards.
+var Analyzer = &analysis.Analyzer{
+	Name:     "AZG012",
+	Doc:      "check that the steps of a longer function body are separated by blank lines",
+	URL:      "https://github.com/katbyte/azproviderlint/blob/main/checks/AZG/AZG012_missing_blank_line_between_steps/README.md",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+// A body with at most this many statements, or spanning at most this many lines, is one
+// group: no blank lines are asked for inside it.
+var (
+	maxStatements = 5
+	maxLines      = 10
+)
+
+func init() {
+	Analyzer.Flags.Init("AZG012", flag.ContinueOnError)
+	Analyzer.Flags.IntVar(&maxStatements, "statements", maxStatements, "bodies with this many statements or fewer are one group")
+	Analyzer.Flags.IntVar(&maxLines, "lines", maxLines, "bodies spanning this many lines or fewer are one group")
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	insp, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, nil
+	}
+
+	line := func(p token.Pos) int { return pass.Fset.Position(p).Line }
+	check := func(stmts []ast.Stmt, open, closing token.Pos) {
+		if len(stmts) <= maxStatements || line(closing)-line(open)+1 <= maxLines {
+			return
+		}
+
+		for i := 0; i+1 < len(stmts); i++ {
+			a, s := stmts[i], stmts[i+1]
+			kind := blockKind(a)
+			if kind == "" {
+				continue
+			}
+
+			switch s.(type) {
+			case *ast.CaseClause, *ast.CommClause:
+				continue
+			}
+
+			if _, isIf := s.(*ast.IfStmt); isIf && oneLiner(a) && oneLiner(s) {
+				continue
+			}
+
+			// a comment between them belongs to the next step, so the blank goes before it
+			at := s.Pos()
+			for _, cg := range astx.EnclosingFile(pass, s.Pos()).Comments {
+				if cg.Pos() > a.End() && cg.End() < s.Pos() && line(cg.Pos()) > line(a.End()) && line(cg.Pos()) < line(at) {
+					at = cg.Pos()
+				}
+			}
+
+			if line(at) != line(a.End())+1 {
+				continue
+			}
+
+			tf := pass.Fset.File(at)
+			start := tf.LineStart(tf.Line(at))
+			pass.Report(analysis.Diagnostic{
+				Pos:     at,
+				Message: "a blank line should separate this from the " + kind + " above, which ends a step",
+				SuggestedFixes: []analysis.SuggestedFix{{
+					Message:   "Insert a blank line",
+					TextEdits: []analysis.TextEdit{{Pos: start, End: start, NewText: []byte("\n")}},
+				}},
+			})
+		}
+	}
+
+	insp.Preorder([]ast.Node{(*ast.BlockStmt)(nil), (*ast.CaseClause)(nil), (*ast.CommClause)(nil)}, func(n ast.Node) {
+		switch n := n.(type) {
+		case *ast.BlockStmt:
+			check(n.List, n.Lbrace, n.Rbrace)
+		case *ast.CaseClause:
+			check(n.Body, n.Pos(), n.End())
+		case *ast.CommClause:
+			check(n.Body, n.Pos(), n.End())
+		}
+	})
+
+	return nil, nil
+}
+
+// blockKind names a statement with a body, or is empty for anything else: only these end a
+// step. A literal assignment that happens to close with a brace does not.
+func blockKind(s ast.Stmt) string {
+	switch s.(type) {
+	case *ast.IfStmt:
+		return "if"
+	case *ast.ForStmt, *ast.RangeStmt:
+		return "for"
+	case *ast.SwitchStmt, *ast.TypeSwitchStmt:
+		return "switch"
+	case *ast.SelectStmt:
+		return "select"
+	case *ast.BlockStmt:
+		return "block"
+	}
+	return ""
+}
+
+// oneLiner reports whether s is an if without else whose body is a single statement: a guard.
+func oneLiner(s ast.Stmt) bool {
+	i, ok := s.(*ast.IfStmt)
+	return ok && i.Else == nil && len(i.Body.List) == 1
+}

checks/AZG/AZG012_missing_blank_line_between_steps/AZG012_test.go

@@ -0,0 +1,22 @@
+package AZG012
+
+import (
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestAZG012(t *testing.T) {
+	t.Parallel()
+
+	_, filename, _, _ := runtime.Caller(0)
+	dir := filepath.Join(filepath.Dir(filename), "testdata")
+
+	analysistest.RunWithSuggestedFixes(t, dir, Analyzer, "azg012")
+
+	maxStatements, maxLines = 2, 4
+	analysistest.Run(t, dir, Analyzer, "azg012short")
+	maxStatements, maxLines = 5, 10
+}

checks/AZG/AZG012_missing_blank_line_between_steps/README.md

@@ -0,0 +1,86 @@
+# AZG012 - separate the steps of a longer function body with blank lines
+
+AZG012 reports an `if`, `for`, `switch` or `select` in a longer body that is run straight into the statement after it, with no blank line between.
+
+A body of any length is read as a sequence of steps. A block ends one, and the statement after it starts the next: get something, check it, use it. A blank line between them is what shows the boundary. Without it a twenty-line loop body reads as one undifferentiated run.
+
+## Flagged Code
+
+```go
+for _, decl := range file.Decls {
+	fd, ok := decl.(*ast.FuncDecl)
+	if !ok || fd.Body == nil {
+		continue
+	}
+	fn, ok := pass.TypesInfo.Defs[fd.Name].(*types.Func)
+	if !ok || runs[fn] != nil {
+		continue
+	}
+	rest := strings.TrimPrefix(fn.Name(), "TestAcc")
+	if rest != "" && unicode.IsLower(rune(rest[0])) {
+		pass.Reportf(fd.Name.Pos(), "...")
+		continue
+	}
+	pass.Reportf(fd.Name.Pos(), "...")
+}
+```
+
+## Passing Code
+
+```go
+for _, decl := range file.Decls {
+	fd, ok := decl.(*ast.FuncDecl)
+	if !ok || fd.Body == nil {
+		continue
+	}
+
+	fn, ok := pass.TypesInfo.Defs[fd.Name].(*types.Func)
+	if !ok || runs[fn] != nil {
+		continue
+	}
+
+	rest := strings.TrimPrefix(fn.Name(), "TestAcc")
+	if rest != "" && unicode.IsLower(rune(rest[0])) {
+		pass.Reportf(fd.Name.Pos(), "...")
+		continue
+	}
+
+	pass.Reportf(fd.Name.Pos(), "...")
+}
+```
+
+## What counts
+
+Only a statement with a body ends a step: `if`, `for`, `switch`, `select`, or a bare block. A literal assignment that happens to close with a brace does not. Closures are checked like any other body, and so are the bodies of `case` clauses.
+
+Not reported:
+
+- a short body, which is one group: five statements or fewer, or ten lines or fewer, see [Options](#options)
+- two one-line `if`s in a row, which read as a single set of guards
+- a block followed by the next `case` or `default`
+- anywhere a blank line is already there
+
+A comment on the line after the block belongs to the next step, so the report, and the blank line, go before the comment.
+
+## The fix
+
+`-fix` inserts the blank line.
+
+## Options
+
+| Option | Default | Effect |
+|---|---|---|
+| `statements` | 5 | bodies with this many statements or fewer are one group |
+| `lines` | 10 | bodies spanning this many lines or fewer are one group |
+
+A body is left alone when either holds. Set with `-AZG012.<option>` on the CLI or under the rule name in the golangci settings; see the [root README](../../../README.md#options).
+
+## Ignoring Reports
+
+Put `//azignore:AZG012 - <reason>` at the end of the reported line, or on the line above it. The reason is required.
+
+```go
+report(rest) //azignore:AZG012 - <reason>
+```
+
+Under golangci-lint, `//nolint:azproviderlint` in the same place also works, but it silences every azproviderlint check on that line.

checks/AZG/AZG012_missing_blank_line_between_steps/testdata/src/azg012/azg012.go

@@ -0,0 +1,110 @@
+package azg012
+
+import "strings"
+
+type item struct {
+	name string
+	ok   bool
+}
+
+func lookup(name string) (item, bool) {
+	return item{name: name}, name != ""
+}
+
+func report(s string) {}
+
+// Should be flagged: a long body whose steps run together. The function body itself is three
+// statements, so the final return needs no blank line.
+func long(names []string) int {
+	count := 0
+	for _, name := range names {
+		if name == "" {
+			continue
+		}
+		it, ok := lookup(name) // want `a blank line should separate this from the if above, which ends a step`
+		if !ok || !it.ok {
+			continue
+		} // a comment on the next line belongs to the next step, so the blank goes before it
+		// want `a blank line should separate this from the if above, which ends a step`
+		rest := strings.TrimPrefix(it.name, "x")
+		if rest == "" {
+			report(rest)
+			continue
+		}
+		report(rest) // want `a blank line should separate this from the if above, which ends a step`
+		count++
+	}
+	return count
+}
+
+// A run of one-line guards stays together, but the statement after the last one starts a
+// step. A literal assignment does not end a step, and a blank line already there is enough.
+func guards(names []string) []string {
+	var out []string
+	for _, name := range names {
+		if name == "" {
+			continue
+		}
+		if strings.HasPrefix(name, "_") {
+			continue
+		}
+		if len(name) > 64 {
+			continue
+		}
+		it := item{ // want `a blank line should separate this from the if above, which ends a step`
+			name: name,
+		}
+		if it.name == "skip" {
+			continue
+		}
+
+		out = append(out, it.name)
+		switch it.name {
+		case "a":
+			if it.ok {
+				report("a")
+			}
+		case "b":
+			report("b")
+		}
+		count := len(out) // want `a blank line should separate this from the switch above, which ends a step`
+		report(string(rune(count)))
+	}
+
+	return out
+}
+
+// Should NOT be flagged: a short body is one group
+func short(allow string) map[string]bool {
+	allowed := map[string]bool{}
+	for form := range strings.SplitSeq(allow, ",") {
+		form = strings.TrimSpace(form)
+		if form == "" {
+			continue
+		}
+		if form != "x" {
+			return nil
+		}
+		allowed[form] = true
+	}
+	return allowed
+}
+
+// Should be flagged: a closure body is checked like any other
+func closure(names []string) func() int {
+	return func() int {
+		n := 0
+		for _, name := range names {
+			if name != "" {
+				n++
+			}
+		}
+		if n > 10 { // want `a blank line should separate this from the for above, which ends a step`
+			n = 10
+		}
+		report("n") // want `a blank line should separate this from the if above, which ends a step`
+		report("m")
+		report("o")
+		return n
+	}
+}