Preferring early loop exits to reduce indentation

stylecheck/analysis.go

@@ -81,4 +81,9 @@ var Analyzers = lint.InitializeAnalyzers(Docs, map[string]*analysis.Analyzer{
 		Requires: []*analysis.Analyzer{generated.Analyzer, inspect.Analyzer},
 	},
 	"ST1023": sharedcheck.RedundantTypeInDeclarationChecker("should", false),
+	"ST1024": {
+		Run:      CheckEarlyLoopReturns,
+		Requires: []*analysis.Analyzer{generated.Analyzer, inspect.Analyzer},
+	},
+
 })

stylecheck/doc.go

@@ -259,4 +259,38 @@ information on how to write good documentation.`,
 		NonDefault: true,
 		MergeIf:    lint.MergeIfAll,
 	},
+
+	"ST1024": {
+		Title: 		"Prefer Early loop returns",
+		Since:		"2022.12",
+		Text:  `if a for loop has only one if condition in it and
+it is longer than 10 statements then its better to have a simple
+if with a break early on instead.
+
+Instead of 
+    for {
+	if n > 5 {
+	    ...
+	    ...
+	    ...
+	    (more than 10 lines)
+	}
+    }
+use
+    for {
+	if n <= 5 {
+	    break
+	}
+	...
+	...
+	...
+	(more than 10 lines)
+	}
+    }
+
+This reduces the indent which makes it more readable.`,
+		NonDefault:	true,
+		MergeIf: lint.MergeIfAll,
+	},
+
 })

stylecheck/lint.go

@@ -988,3 +988,30 @@ func CheckExportedVarDocs(pass *analysis.Pass) (interface{}, error) {
 	pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Nodes([]ast.Node{(*ast.GenDecl)(nil), (*ast.ValueSpec)(nil), (*ast.FuncLit)(nil), (*ast.FuncDecl)(nil)}, fn)
 	return nil, nil
 }
+
+func CheckEarlyLoopReturns(pass *analysis.Pass) (interface{}, error) {
+	fn := func(node ast.Node, push bool) bool {
+		switch node := node.(type) {
+		case *ast.ForStmt:
+			if push {
+				// If a for statement has only one statement in its body
+				// and it is an if statement
+				// and there are more than 10 statements inside the if stmt
+				// then we could return earlier by reversing the if condition
+				if len(node.Body.List) == 1 {
+					switch ifstmt := node.Body.List[0].(type) {
+					case *ast.IfStmt:
+						if len(ifstmt.Body.List) > 10 {
+							report.Report(pass, node, fmt.Sprintf(`for loop with a single if statement in the body is a candidate for early return`), report.FilterGenerated())
+							return true
+						}
+					}
+				}
+			}
+		}
+		return true
+	}
+	pass.ResultOf[inspect.Analyzer].(*inspector.Inspector).Nodes([]ast.Node{(*ast.ForStmt)(nil)}, fn)
+
+	return nil, nil
+}

stylecheck/lint_test.go

@@ -26,6 +26,7 @@ func TestAll(t *testing.T) {
 		"ST1021": {{Dir: "CheckExportedTypeDocs"}},
 		"ST1022": {{Dir: "CheckExportedVarDocs"}},
 		"ST1023": {{Dir: "CheckRedundantTypeInDeclaration"}, {Dir: "CheckRedundantTypeInDeclaration_syscall"}},
+		"ST1024": {{Dir: "CheckEarlyLoopReturns"}},
 	}
 
 	testutil.Run(t, Analyzers, checks)

stylecheck/testdata/src/CheckEarlyLoopReturns/CheckEarlyLoopReturns.go

@@ -0,0 +1,89 @@
+package pkg
+
+import (
+	"math/rand"
+	"fmt"
+)
+
+func fn() {
+	n := rand.Intn(10)
+	for { //@ diag(`for loop with a single if statement in the body is a candidate for early return`)
+		if n > 5 {
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+		}
+	}
+
+	// Comments should not affect the warning.
+	for { //@ diag(`for loop with a single if statement in the body is a candidate for early return`)
+		// Just an uninteresting comment.
+		if n > 5 {
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+		}
+	}
+
+	// The if statement has < 10 lines so no warning should be shown.
+	for {
+		if n > 5 {
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+		}
+	}
+	for {
+		// This if condition should NOT be caught because there is another statement after
+		// the if condition within the for loop.
+		if n > 5 {
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+		}
+		fmt.Println("Extra line")
+	}
+	for {
+		fmt.Println("Extra line")
+		// This if condition should NOT be caught because there is another statement before
+		// the if condition within the for loop.
+		if n > 5 {
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+			fmt.Println("1")
+		}
+	}
+
+}