[linter-miner] add lenstringzero linter (#37618)

Author: github-actions[bot]

cmd/linters/main.go

@@ -26,6 +26,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/fprintlnsprintf"
 	"github.com/github/gh-aw/pkg/linters/jsonmarshalignoredeerror"
 	"github.com/github/gh-aw/pkg/linters/largefunc"
+	"github.com/github/gh-aw/pkg/linters/lenstringzero"
 	"github.com/github/gh-aw/pkg/linters/manualmutexunlock"
 	"github.com/github/gh-aw/pkg/linters/osexitinlibrary"
 	"github.com/github/gh-aw/pkg/linters/ossetenvlibrary"
@@ -60,6 +61,7 @@ func main() {
 		seenmapbool.Analyzer,
 		strconvparseignorederror.Analyzer,
 		jsonmarshalignoredeerror.Analyzer,
+		lenstringzero.Analyzer,
 		tolowerequalfold.Analyzer,
 		uncheckedtypeassertion.Analyzer,
 	)

docs/adr/37618-add-lenstringzero-linter.md

@@ -0,0 +1,41 @@
+# ADR-37618: Add a dedicated `lenstringzero` analysis-pass linter
+
+**Date**: 2026-06-07
+**Status**: Draft
+
+## Context
+
+The codebase maintains a set of custom `golang.org/x/tools/go/analysis` linters under `pkg/linters/`, each implemented as its own single-responsibility package and registered centrally in `cmd/linters/main.go`. A recurring non-idiomatic pattern was observed: `len(s) == 0` and `len(s) != 0` comparisons on **string** values, where the idiomatic Go form is `s == ""` / `s != ""`. The off-the-shelf linters that would normally catch this — `gocritic`'s `sloppyLen` and `revive` — are both explicitly disabled in `.golangci.yml` due to golangci-lint v2 configuration bugs, so the pattern is currently unguarded. Code-pattern scanning surfaced the idiom across 20+ non-test source files in `pkg/`.
+
+## Decision
+
+We will add a new standalone analysis-pass linter, `lenstringzero`, in its own package `pkg/linters/lenstringzero/`, and register it in the central analyzer list in `cmd/linters/main.go`. The analyzer inspects every `*ast.BinaryExpr` using `==` / `!=`, detects when one operand is a single-argument `len(...)` call and the other is the integer literal `0` (handling both operand orders), and reports a diagnostic only when the `len()` argument type-checks (via `pass.TypesInfo`) to an underlying `string`. `[]byte`, arrays, and other non-string types are excluded, as are comparisons that are not against `0`. Test files are skipped via `filecheck.IsTestFile`, and the package ships `analysistest`-based fixtures with `// want` annotations, consistent with sibling linters.
+
+## Alternatives Considered
+
+### Alternative 1: Re-enable `gocritic`/`revive` in `.golangci.yml`
+The `sloppyLen` (gocritic) and `revive` rules already implement this check. They were not chosen because both are currently disabled due to golangci-lint v2 configuration bugs; re-enabling them is blocked on an upstream/config fix outside this PR's scope, and doing so would re-activate a broad set of other rules rather than this single targeted check. *[TODO: verify whether a narrower re-enable was attempted and rejected.]*
+
+### Alternative 2: Fold the check into an existing linter package
+The string-empty check could have been added to a broader existing analyzer rather than introducing a new package. It was not chosen because each rule in `pkg/linters/` is intentionally a single-responsibility, independently testable, and individually toggleable package, which keeps rules discoverable and consistent with the established one-package-per-linter convention.
+
+## Consequences
+
+### Positive
+- Automated, repeatable prevention of the `len(s) == 0` / `len(s) != 0`-on-string idiom, restoring coverage lost when `gocritic`/`revive` were disabled.
+- Type-aware: the `string` underlying-type check avoids false positives on `[]byte`, arrays, slices, and maps, which legitimately use `len(x) == 0`.
+- Consistent with the established one-package-per-linter convention, so the addition is low-friction for maintainers.
+
+### Negative
+- Adds another custom linter to maintain in-house rather than leveraging the upstream-maintained `gocritic`/`revive` equivalents (duplication that should be retired if those are ever re-enabled).
+- Detection is limited to direct `len(...)` comparisons against the literal `0`; an equivalent length stored in an intermediate variable (`n := len(s); n == 0`) is not flagged (false negatives).
+- The existing 20+ occurrences are not auto-fixed by this PR; they must be remediated separately once the linter is enforced in CI.
+
+### Neutral
+- The analyzer requires the `inspect` pass and type information, matching the resource profile of sibling linters.
+- The diagnostic is report-only (no suggested-fix/autofix is emitted in this PR).
+- The rule deliberately does not flag length comparisons using other operators (`>`, `>=`, `< `) or against non-zero literals, preserving genuine length-threshold checks.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27100525416) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/linters/lenstringzero/lenstringzero.go

@@ -0,0 +1,98 @@
+// Package lenstringzero implements a Go analysis linter that flags len(s) == 0
+// and len(s) != 0 comparisons on string values that should use == "" or != "" instead.
+package lenstringzero
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+)
+
+var Analyzer = &analysis.Analyzer{
+	Name:     "lenstringzero",
+	Doc:      "reports len(s) == 0 and len(s) != 0 comparisons on string values that should use == \"\" or != \"\" instead",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/lenstringzero",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	insp, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", pass.ResultOf[inspect.Analyzer])
+	}
+
+	nodeFilter := []ast.Node{(*ast.BinaryExpr)(nil)}
+
+	insp.Preorder(nodeFilter, func(n ast.Node) {
+		expr, ok := n.(*ast.BinaryExpr)
+		if !ok {
+			return
+		}
+		if expr.Op != token.EQL && expr.Op != token.NEQ {
+			return
+		}
+
+		pos := pass.Fset.PositionFor(expr.Pos(), false)
+		if filecheck.IsTestFile(pos.Filename) {
+			return
+		}
+
+		var lenArg ast.Expr
+		if isLenCall(expr.X) && isIntZero(expr.Y) {
+			lenArg = lenCallArg(expr.X)
+		} else if isIntZero(expr.X) && isLenCall(expr.Y) {
+			lenArg = lenCallArg(expr.Y)
+		}
+		if lenArg == nil {
+			return
+		}
+
+		t := pass.TypesInfo.TypeOf(lenArg)
+		if t == nil {
+			return
+		}
+		basic, ok := t.Underlying().(*types.Basic)
+		if !ok || basic.Kind() != types.String {
+			return
+		}
+
+		op := expr.Op.String()
+		var cmpVerb string
+		if expr.Op == token.EQL {
+			cmpVerb = "empty"
+		} else {
+			cmpVerb = "non-empty"
+		}
+		pass.ReportRangef(expr,
+			`use s %s "" to check for %s string instead of len(s) %s 0`,
+			op, cmpVerb, op)
+	})
+
+	return nil, nil
+}
+
+func isLenCall(expr ast.Expr) bool {
+	call, ok := expr.(*ast.CallExpr)
+	if !ok || len(call.Args) != 1 {
+		return false
+	}
+	ident, ok := call.Fun.(*ast.Ident)
+	return ok && ident.Name == "len"
+}
+
+func lenCallArg(expr ast.Expr) ast.Expr {
+	return expr.(*ast.CallExpr).Args[0]
+}
+
+func isIntZero(expr ast.Expr) bool {
+	lit, ok := expr.(*ast.BasicLit)
+	return ok && lit.Kind == token.INT && lit.Value == "0"
+}

pkg/linters/lenstringzero/lenstringzero_test.go

@@ -0,0 +1,16 @@
+//go:build !integration
+
+package lenstringzero_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/lenstringzero"
+)
+
+func TestLenStringZero(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, lenstringzero.Analyzer, "lenstringzero")
+}

pkg/linters/lenstringzero/testdata/src/lenstringzero/lenstringzero.go

@@ -0,0 +1,41 @@
+package lenstringzero
+
+func isEmpty(s string) bool {
+return len(s) == 0 // want `use s == "" to check for empty string instead of len\(s\) == 0`
+}
+
+func isNotEmpty(s string) bool {
+return len(s) != 0 // want `use s != "" to check for non-empty string instead of len\(s\) != 0`
+}
+
+func flippedEmpty(s string) bool {
+return 0 == len(s) // want `use s == "" to check for empty string instead of len\(s\) == 0`
+}
+
+func flippedNotEmpty(s string) bool {
+return 0 != len(s) // want `use s != "" to check for non-empty string instead of len\(s\) != 0`
+}
+
+func alreadyGoodEmpty(s string) bool {
+return s == ""
+}
+
+func alreadyGoodNotEmpty(s string) bool {
+return s != ""
+}
+
+func sliceNotFlagged(s []byte) bool {
+return len(s) == 0
+}
+
+func arrayNotFlagged(s [1]byte) bool {
+return len(s) != 0
+}
+
+func lenNotZeroOp(s string) bool {
+return len(s) > 0
+}
+
+func lenNotComparedToZero(s string) bool {
+return len(s) == 1
+}