From 154d373d20c6073e47a0aeda03ba285d097d29cb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Jun 2026 05:40:56 +0000 Subject: [PATCH 1/4] Initial plan From 0ac57f25e5dd2a27da8c939b117defb439ad5c77 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Jun 2026 05:46:40 +0000 Subject: [PATCH 2/4] Add lenstringzero alias tracking for len(s) comparisons Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/linters/lenstringzero/lenstringzero.go | 152 ++++++++++++++++++ .../src/lenstringzero/lenstringzero.go | 52 ++++-- 2 files changed, 194 insertions(+), 10 deletions(-) diff --git a/pkg/linters/lenstringzero/lenstringzero.go b/pkg/linters/lenstringzero/lenstringzero.go index d41c7942c9c..5310e011b6a 100644 --- a/pkg/linters/lenstringzero/lenstringzero.go +++ b/pkg/linters/lenstringzero/lenstringzero.go @@ -28,6 +28,7 @@ func run(pass *analysis.Pass) (any, error) { if !ok { return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", pass.ResultOf[inspect.Analyzer]) } + lenStringAliases := collectLenStringAliases(pass) nodeFilter := []ast.Node{(*ast.BinaryExpr)(nil)} @@ -50,6 +51,12 @@ func run(pass *analysis.Pass) (any, error) { lenArg = lenCallArg(expr.X) } else if isIntZero(expr.X) && isLenCall(expr.Y) { lenArg = lenCallArg(expr.Y) + } else if arg, ok := lenAliasArg(pass, expr.X, lenStringAliases); ok && isIntZero(expr.Y) { + lenArg = arg + } else if isIntZero(expr.X) { + if arg, ok := lenAliasArg(pass, expr.Y, lenStringAliases); ok { + lenArg = arg + } } if lenArg == nil { return @@ -96,3 +103,148 @@ func isIntZero(expr ast.Expr) bool { lit, ok := expr.(*ast.BasicLit) return ok && lit.Kind == token.INT && lit.Value == "0" } + +func collectLenStringAliases(pass *analysis.Pass) map[types.Object]ast.Expr { + aliases := make(map[types.Object]ast.Expr) + for _, file := range pass.Files { + ast.Inspect(file, func(node ast.Node) bool { + switch n := node.(type) { + case *ast.AssignStmt: + collectLenStringAliasesFromAssignStmt(pass, n, aliases) + case *ast.ValueSpec: + collectLenStringAliasesFromValueSpec(pass, n, aliases) + case *ast.IncDecStmt: + if ident, ok := n.X.(*ast.Ident); ok { + delete(aliases, pass.TypesInfo.ObjectOf(ident)) + } + case *ast.RangeStmt: + if n.Tok == token.ASSIGN { + deleteLenStringAliasForExpr(pass, aliases, n.Key) + deleteLenStringAliasForExpr(pass, aliases, n.Value) + } + } + return true + }) + } + return aliases +} + +func collectLenStringAliasesFromAssignStmt(pass *analysis.Pass, stmt *ast.AssignStmt, aliases map[types.Object]ast.Expr) { + for i, lhs := range stmt.Lhs { + ident, ok := lhs.(*ast.Ident) + if !ok || ident.Name == "_" { + continue + } + obj := pass.TypesInfo.ObjectOf(ident) + if obj == nil || !isLocalObject(obj) { + continue + } + + switch stmt.Tok { + case token.DEFINE: + if obj.Pos() != ident.Pos() { + delete(aliases, obj) + continue + } + rhs, ok := rhsExprForIndex(stmt.Rhs, i) + if !ok { + delete(aliases, obj) + continue + } + if arg, ok := lenStringArg(pass, rhs); ok { + aliases[obj] = arg + } else { + delete(aliases, obj) + } + case token.ASSIGN: + delete(aliases, obj) + } + } +} + +func collectLenStringAliasesFromValueSpec(pass *analysis.Pass, spec *ast.ValueSpec, aliases map[types.Object]ast.Expr) { + for i, name := range spec.Names { + if name.Name == "_" { + continue + } + obj := pass.TypesInfo.ObjectOf(name) + if obj == nil || !isLocalObject(obj) { + continue + } + rhs, ok := rhsExprForIndex(spec.Values, i) + if !ok { + delete(aliases, obj) + continue + } + if arg, ok := lenStringArg(pass, rhs); ok { + aliases[obj] = arg + } else { + delete(aliases, obj) + } + } +} + +func lenAliasArg(pass *analysis.Pass, expr ast.Expr, aliases map[types.Object]ast.Expr) (ast.Expr, bool) { + ident, ok := expr.(*ast.Ident) + if !ok { + return nil, false + } + obj := pass.TypesInfo.ObjectOf(ident) + if obj == nil { + return nil, false + } + arg, ok := aliases[obj] + if !ok { + return nil, false + } + return arg, true +} + +func lenStringArg(pass *analysis.Pass, expr ast.Expr) (ast.Expr, bool) { + if !isLenCall(expr) { + return nil, false + } + arg := lenCallArg(expr) + t := pass.TypesInfo.TypeOf(arg) + if t == nil { + return nil, false + } + basic, ok := t.Underlying().(*types.Basic) + if !ok || basic.Kind() != types.String { + return nil, false + } + return arg, true +} + +func rhsExprForIndex(rhs []ast.Expr, idx int) (ast.Expr, bool) { + switch { + case len(rhs) == 0: + return nil, false + case len(rhs) == 1 && idx == 0: + return rhs[0], true + case idx < len(rhs): + return rhs[idx], true + default: + return nil, false + } +} + +func deleteLenStringAliasForExpr(pass *analysis.Pass, aliases map[types.Object]ast.Expr, expr ast.Expr) { + ident, ok := expr.(*ast.Ident) + if !ok { + return + } + delete(aliases, pass.TypesInfo.ObjectOf(ident)) +} + +func isLocalObject(obj types.Object) bool { + if obj == nil { + return false + } + parent := obj.Parent() + if parent == nil { + return false + } + pkg := obj.Pkg() + return pkg == nil || parent != pkg.Scope() +} diff --git a/pkg/linters/lenstringzero/testdata/src/lenstringzero/lenstringzero.go b/pkg/linters/lenstringzero/testdata/src/lenstringzero/lenstringzero.go index d2001beb119..ad23e80e31a 100644 --- a/pkg/linters/lenstringzero/testdata/src/lenstringzero/lenstringzero.go +++ b/pkg/linters/lenstringzero/testdata/src/lenstringzero/lenstringzero.go @@ -1,41 +1,73 @@ package lenstringzero func isEmpty(s string) bool { -return len(s) == 0 // want `use s == "" to check for empty string instead of len\(s\) == 0` + 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` + 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` + 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` + 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 == "" + return s == "" } func alreadyGoodNotEmpty(s string) bool { -return s != "" + return s != "" } func sliceNotFlagged(s []byte) bool { -return len(s) == 0 + return len(s) == 0 } func arrayNotFlagged(s [1]byte) bool { -return len(s) != 0 + return len(s) != 0 } func lenNotZeroOp(s string) bool { -return len(s) > 0 + return len(s) > 0 } func lenNotComparedToZero(s string) bool { -return len(s) == 1 + return len(s) == 1 +} + +func aliasEmpty(s string) bool { + n := len(s) + return n == 0 // want `use s == "" to check for empty string instead of len\(s\) == 0` +} + +func aliasNotEmpty(s string) bool { + n := len(s) + return n != 0 // want `use s != "" to check for non-empty string instead of len\(s\) != 0` +} + +func aliasReassignedNotFlagged(s string) bool { + n := len(s) + n = 1 + return n == 0 +} + +func aliasIncrementedNotFlagged(s string) bool { + n := len(s) + n++ + return n == 0 +} + +func sliceAliasNotFlagged(s []byte) bool { + n := len(s) + return n == 0 +} + +func arrayAliasNotFlagged(s [1]byte) bool { + n := len(s) + return n == 0 } From c74368ab37bda76581be340badb40ce9f3a8c09e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Jun 2026 05:50:07 +0000 Subject: [PATCH 3/4] Refine lenstringzero alias comparison flow Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/linters/lenstringzero/lenstringzero.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/linters/lenstringzero/lenstringzero.go b/pkg/linters/lenstringzero/lenstringzero.go index 5310e011b6a..668b29c652c 100644 --- a/pkg/linters/lenstringzero/lenstringzero.go +++ b/pkg/linters/lenstringzero/lenstringzero.go @@ -51,8 +51,10 @@ func run(pass *analysis.Pass) (any, error) { lenArg = lenCallArg(expr.X) } else if isIntZero(expr.X) && isLenCall(expr.Y) { lenArg = lenCallArg(expr.Y) - } else if arg, ok := lenAliasArg(pass, expr.X, lenStringAliases); ok && isIntZero(expr.Y) { - lenArg = arg + } else if isIntZero(expr.Y) { + if arg, ok := lenAliasArg(pass, expr.X, lenStringAliases); ok { + lenArg = arg + } } else if isIntZero(expr.X) { if arg, ok := lenAliasArg(pass, expr.Y, lenStringAliases); ok { lenArg = arg @@ -220,8 +222,6 @@ func rhsExprForIndex(rhs []ast.Expr, idx int) (ast.Expr, bool) { switch { case len(rhs) == 0: return nil, false - case len(rhs) == 1 && idx == 0: - return rhs[0], true case idx < len(rhs): return rhs[idx], true default: From 52af09577a9c1993b6abbf9e50f9a18ef01cc966 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 8 Jun 2026 06:03:55 +0000 Subject: [PATCH 4/4] docs(adr): add draft ADR-37750 for lenstringzero alias tracking Co-Authored-By: Claude Opus 4.8 (1M context) --- ...ack-len-string-aliases-in-lenstringzero.md | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 docs/adr/37750-track-len-string-aliases-in-lenstringzero.md diff --git a/docs/adr/37750-track-len-string-aliases-in-lenstringzero.md b/docs/adr/37750-track-len-string-aliases-in-lenstringzero.md new file mode 100644 index 00000000000..a3a3039c450 --- /dev/null +++ b/docs/adr/37750-track-len-string-aliases-in-lenstringzero.md @@ -0,0 +1,41 @@ +# ADR-37750: Track local `len(string)` aliases in the `lenstringzero` linter + +**Date**: 2026-06-08 +**Status**: Draft + +## Context + +The `lenstringzero` analysis-pass linter (introduced in [ADR-37618](37618-add-lenstringzero-linter.md)) flags the non-idiomatic `len(s) == 0` / `len(s) != 0` pattern on **string** values, recommending `s == ""` / `s != ""` instead. ADR-37618 explicitly recorded a known false-negative as a Negative consequence: an equivalent length stored in an intermediate variable (`n := len(s); n == 0`) was not detected. This PR closes that gap. The challenge is that an aliased length is only equivalent to the direct form until the alias is mutated, so naive aliasing would produce false positives on reassigned or incremented variables. + +## Decision + +We will extend `lenstringzero` with a pre-pass that records local objects initialized from `len()` — via short declarations (`:=`) and `var` specs (`ValueSpec`) — into a `map[types.Object]ast.Expr` keyed by the resolved type-checker object. The existing binary-expression matcher then treats a tracked alias compared against the integer literal `0` (in both operand orders) the same as a direct `len(s)` comparison, emitting identical diagnostics. To preserve soundness, aliases are invalidated (deleted from the map) on any mutation: reassignment (`=`), increment/decrement (`++`/`--`), and range-assignment targets. Type restriction to underlying `string` is reused so non-string aliases (`[]byte`, arrays) remain unflagged. + +## Alternatives Considered + +### Alternative 1: Full dataflow / SSA-based analysis +A control-flow-aware analysis (e.g. `go/ssa`) could track value provenance precisely across branches and scopes. It was not chosen because it is substantially heavier than the existing single-`inspect`-pass design of every linter in `pkg/linters/`, and the targeted alias-then-compare pattern is handled adequately by a syntactic pre-pass with conservative mutation invalidation. + +### Alternative 2: Leave the limitation documented and unaddressed +ADR-37618 already disclosed the alias false-negative as an accepted trade-off, so doing nothing was a valid option. It was not chosen because the alias form is a common, easily-detectable variant whose omission noticeably weakens the linter's value, and the conservative pre-pass approach adds the coverage without meaningful false-positive risk. + +## Consequences + +### Positive +- Detects the previously-missed `n := len(s); n == 0` alias form, closing the false-negative documented in ADR-37618. +- Mutation invalidation (`=`, `++`/`--`, range targets) prevents stale-alias false positives after the value changes. +- Reuses the existing underlying-`string` type check, so `[]byte`/array aliases stay correctly unflagged; diagnostic wording is unchanged. + +### Negative +- Adds ~150 lines of alias-collection and invalidation logic to maintain, increasing the linter's surface area and complexity. +- The pre-pass is syntactic, not flow-sensitive: it does not model branch-conditional reassignment or aliasing through additional intermediate variables, so some edge cases remain out of scope. +- Alias tracking is whole-file and keyed by `types.Object`; correctness depends on accurate object resolution and the completeness of the mutation-invalidation cases. + +### Neutral +- Detection remains string-only and limited to comparisons against the literal `0`; other operators and non-zero literals are still intentionally excluded. +- The diagnostic stays report-only (no suggested-fix/autofix is emitted). +- Test fixtures were extended with positive (alias `== 0`/`!= 0`) and negative (reassigned, incremented, non-string) cases, consistent with sibling linters. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27118730160) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*