[linter-miner] Add slice-make-zero-length linter #60310
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # ADR-60310: Add Slice Make Zero Length Linter | ||
|
|
||
| **Date**: 2026-09-11 | ||
| **Status**: Draft | ||
| **Deciders**: gh-aw maintainers | ||
|
|
||
| --- | ||
|
|
||
| ### Context | ||
|
|
||
| This pull request adds a new custom Go analyzer under `pkg/linters/` and registers it in the shared linter registry, which makes the change part of the repository's standard static-analysis policy rather than an isolated utility. The implementation targets calls of the form `make([]T, 0)` without a capacity argument and treats them as a performance-oriented code smell, with tests showing both expected findings and allowed cases such as explicit capacity, non-zero length, and `nolint` suppression. Because the linter becomes part of the central analyzer suite, the architectural decision is whether this repository should enforce this allocation pattern through automated linting. The available PR evidence emphasizes performance and repeated review feedback as the primary drivers for codifying the rule. | ||
|
|
||
| ### Decision | ||
|
|
||
| We will add a custom `slicemakezerolength` analyzer to the repository's shared linter registry. It flags `make([]T, 0)` only when the next statement is a range loop over a value with a known length and the loop appends exactly one element to that slice per iteration. This makes `len(rangeValue)` a useful capacity hint while avoiding diagnostics when the slice does not grow or its growth cannot be derived. The analyzer supports existing repository conventions such as generated-file skipping, coverage gating, and `nolint` suppression. | ||
|
|
||
| ### Alternatives Considered | ||
|
|
||
| #### Alternative 1: Keep this as a code review guideline only | ||
|
|
||
| The team could continue treating `make([]T, 0)` without capacity as an informal review suggestion instead of building a dedicated analyzer. This was considered because the PR body explicitly describes the pattern as a common review comment, and manual review avoids growing the custom linter suite. It was not chosen because the diff shows the pattern occurs in multiple locations across the codebase and the change aims to make the guidance consistent and automatically enforceable. | ||
|
|
||
| #### Alternative 2: Rely on existing third-party linters | ||
|
|
||
| Another option would be to depend on an upstream linter or broader performance lint package instead of adding a repository-specific analyzer. This was considered because it could reduce local maintenance and reuse community tooling. It was not chosen because the PR implements the rule directly in `pkg/linters/`, integrates it with the local registry and helper utilities, and therefore indicates the repository wants targeted behavior aligned with its existing custom-linter framework. | ||
|
|
||
| #### Alternative 3: Flag every zero-length slice allocation without capacity | ||
|
|
||
| The analyzer could report every `make([]T, 0)` without analyzing subsequent use. This simpler syntactic rule was not chosen because slices that never grow need no backing allocation, and slices with indeterminate growth have no defensible capacity hint. Restricting the rule to a direct, known-size append loop provides a precise optimization with predictable enforcement. | ||
|
|
||
| ### Consequences | ||
|
|
||
| #### Positive | ||
| - The repository will enforce this slice-allocation convention consistently across reviews and automation. | ||
| - Developers get fast feedback for a repeated performance-oriented pattern without waiting for reviewer intervention. | ||
| - The analyzer fits the existing custom linter architecture, including registry-based activation, testdata-driven verification, coverage gating, and `nolint` support. | ||
|
|
||
| #### Negative | ||
| - The deliberately narrow pattern leaves more complex growth patterns to manual performance analysis. | ||
| - Maintaining another custom analyzer increases long-term cost for compatibility, testing, and linter-suite complexity. | ||
| - Codifying this recommendation as a lint rule may push style and micro-optimization policy into CI, which can increase friction for contributors. | ||
|
|
||
| #### Neutral | ||
| - The implementation only adds diagnostics; it does not include an automatic fix or rewrite. | ||
| - Existing suppression mechanisms remain available through `//nolint:slicemakezerolength`. | ||
| - The decision extends the current custom-linter framework rather than introducing a new enforcement mechanism. | ||
|
|
||
| --- | ||
|
|
||
| *ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ import ( | |
| "github.com/github/gh-aw/pkg/linters/regexpcompileinfunction" | ||
| "github.com/github/gh-aw/pkg/linters/regexpdynamicpattern" | ||
| "github.com/github/gh-aw/pkg/linters/seenmapbool" | ||
| "github.com/github/gh-aw/pkg/linters/slicemakezerolength" | ||
| "github.com/github/gh-aw/pkg/linters/sortslice" | ||
| "github.com/github/gh-aw/pkg/linters/sprintfbool" | ||
| "github.com/github/gh-aw/pkg/linters/sprintferrdot" | ||
|
|
@@ -125,6 +126,7 @@ var allAnalyzers = []*analysis.Analyzer{ | |
| regexpdynamicpattern.Analyzer, | ||
| ssljson.Analyzer, | ||
| seenmapbool.Analyzer, | ||
| slicemakezerolength.Analyzer, | ||
|
Contributor
Author
There was a problem hiding this comment. [/codebase-design] This PR registers 💡 FixAdd a bullet to the Overview list and a row to the Public API table in - `slicemakezerolength` — reports `make([]T, 0)` calls without a capacity argument when the final slice length is known, which can be optimized.And in the Public API table: | `slicemakezerolength` | Custom `go/analysis` analyzer that flags `make([]T, 0)` calls without capacity when the final length is known |Also consider whether this should be added to the coverage-gated linter list in the README (it registers a @copilot please address this.
Contributor
There was a problem hiding this comment. Addressed in |
||
| sortslice.Analyzer, | ||
| sprintferrdot.Analyzer, | ||
| sprintferrorsnew.Analyzer, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| // Package slicemakezerolength implements a Go analysis linter that flags | ||
| // make([]T, 0) calls without a capacity argument when the final slice length | ||
| // can be statically determined, suggesting the capacity should be specified | ||
| // to avoid allocation overhead. | ||
| package slicemakezerolength | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "go/ast" | ||
| "go/constant" | ||
| "go/types" | ||
|
|
||
| "golang.org/x/tools/go/analysis" | ||
|
|
||
| "github.com/github/gh-aw/pkg/linters/internal/analyzerutil" | ||
| "github.com/github/gh-aw/pkg/linters/internal/astutil" | ||
| "github.com/github/gh-aw/pkg/linters/internal/coverage" | ||
| "github.com/github/gh-aw/pkg/linters/internal/filecheck" | ||
| "github.com/github/gh-aw/pkg/linters/internal/nolint" | ||
| ) | ||
|
|
||
| // Analyzer is the slice-make-zero-length analysis pass. | ||
| var Analyzer = analyzerutil.New("slicemakezerolength", "reports make([]T, 0) calls without capacity when the final length is known, which can be optimized", run) | ||
|
|
||
| // hotThreshold gates findings on coverage data; see coverage package docs. | ||
| var hotThreshold *int | ||
|
|
||
| func init() { | ||
| hotThreshold = coverage.RegisterHotThresholdFlag(Analyzer) | ||
| } | ||
|
|
||
| func run(pass *analysis.Pass) (any, error) { | ||
| noLintIndex, generatedFiles, err := analyzerutil.Indexes(pass) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| nodeFilter := []ast.Node{(*ast.BlockStmt)(nil)} | ||
| return analyzerutil.Preorder(pass, nodeFilter, func(n ast.Node) { | ||
| analyzeBlock(pass, n, generatedFiles, noLintIndex) | ||
| }) | ||
| } | ||
|
|
||
| func analyzeBlock(pass *analysis.Pass, n ast.Node, generatedFiles filecheck.GeneratedIndex, noLintIndex nolint.DirectiveIndex) { | ||
| block, ok := n.(*ast.BlockStmt) | ||
| if !ok { | ||
| return | ||
| } | ||
|
|
||
| for i := 0; i+1 < len(block.List); i++ { | ||
| target, call, sliceType, ok := zeroLengthSliceAssignment(pass, block.List[i]) | ||
| if !ok { | ||
| continue | ||
| } | ||
| rangeStmt, ok := block.List[i+1].(*ast.RangeStmt) | ||
| if !ok || !hasKnownRangeSize(pass, rangeStmt.X) || containsReference(pass, rangeStmt.X, target) { | ||
| continue | ||
| } | ||
| if !appendsOneElement(pass, rangeStmt, target) { | ||
| continue | ||
| } | ||
|
|
||
| reportDiagnostic(pass, call, sliceType, rangeStmt.X, generatedFiles, noLintIndex) | ||
| } | ||
| } | ||
|
|
||
| func zeroLengthSliceAssignment(pass *analysis.Pass, stmt ast.Stmt) (types.Object, *ast.CallExpr, *ast.ArrayType, bool) { | ||
| var target *ast.Ident | ||
| var value ast.Expr | ||
|
|
||
| switch stmt := stmt.(type) { | ||
| case *ast.AssignStmt: | ||
| if len(stmt.Lhs) != 1 || len(stmt.Rhs) != 1 { | ||
| return nil, nil, nil, false | ||
| } | ||
| target, _ = stmt.Lhs[0].(*ast.Ident) | ||
| value = stmt.Rhs[0] | ||
| case *ast.DeclStmt: | ||
| decl, ok := stmt.Decl.(*ast.GenDecl) | ||
| if !ok || len(decl.Specs) != 1 { | ||
| return nil, nil, nil, false | ||
| } | ||
| spec, ok := decl.Specs[0].(*ast.ValueSpec) | ||
| if !ok || len(spec.Names) != 1 || len(spec.Values) != 1 { | ||
| return nil, nil, nil, false | ||
| } | ||
| target = spec.Names[0] | ||
| value = spec.Values[0] | ||
| default: | ||
| return nil, nil, nil, false | ||
| } | ||
|
|
||
| if target == nil { | ||
| return nil, nil, nil, false | ||
| } | ||
| targetObject := pass.TypesInfo.ObjectOf(target) | ||
| if targetObject == nil { | ||
| return nil, nil, nil, false | ||
| } | ||
|
|
||
| call, ok := value.(*ast.CallExpr) | ||
| if !ok || len(call.Args) != 2 || call.Ellipsis.IsValid() { | ||
| return nil, nil, nil, false | ||
| } | ||
| ident, ok := call.Fun.(*ast.Ident) | ||
| if !ok || ident.Name != "make" { | ||
| return nil, nil, nil, false | ||
| } | ||
| if pass.TypesInfo.ObjectOf(ident) != types.Universe.Lookup("make") { | ||
| return nil, nil, nil, false | ||
| } | ||
|
|
||
| sliceType, ok := call.Args[0].(*ast.ArrayType) | ||
| if !ok || sliceType.Len != nil { | ||
| return nil, nil, nil, false | ||
| } | ||
| if !isConstantZero(pass, call.Args[1]) { | ||
| return nil, nil, nil, false | ||
| } | ||
|
|
||
| return targetObject, call, sliceType, true | ||
| } | ||
|
|
||
| func hasKnownRangeSize(pass *analysis.Pass, expr ast.Expr) bool { | ||
| typ := pass.TypesInfo.TypeOf(expr) | ||
| if typ == nil { | ||
| return false | ||
| } | ||
| switch underlying := typ.Underlying().(type) { | ||
| case *types.Array, *types.Slice, *types.Map: | ||
| return true | ||
| case *types.Basic: | ||
| return underlying.Info()&types.IsString != 0 | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| func appendsOneElement(pass *analysis.Pass, rangeStmt *ast.RangeStmt, target types.Object) bool { | ||
| if len(rangeStmt.Body.List) != 1 { | ||
| return false | ||
| } | ||
| assign, ok := rangeStmt.Body.List[0].(*ast.AssignStmt) | ||
| if !ok || len(assign.Lhs) != 1 || len(assign.Rhs) != 1 || !refersTo(pass, assign.Lhs[0], target) { | ||
| return false | ||
| } | ||
| call, ok := assign.Rhs[0].(*ast.CallExpr) | ||
| if !ok || len(call.Args) != 2 || call.Ellipsis.IsValid() || !refersTo(pass, call.Args[0], target) { | ||
| return false | ||
| } | ||
| ident, ok := call.Fun.(*ast.Ident) | ||
| return ok && pass.TypesInfo.ObjectOf(ident) == types.Universe.Lookup("append") | ||
| } | ||
|
|
||
| func refersTo(pass *analysis.Pass, expr ast.Expr, target types.Object) bool { | ||
| ident, ok := expr.(*ast.Ident) | ||
| return ok && pass.TypesInfo.ObjectOf(ident) == target | ||
| } | ||
|
|
||
| func containsReference(pass *analysis.Pass, expr ast.Expr, target types.Object) bool { | ||
| found := false | ||
| ast.Inspect(expr, func(node ast.Node) bool { | ||
| ident, ok := node.(*ast.Ident) | ||
| if ok && pass.TypesInfo.ObjectOf(ident) == target { | ||
| found = true | ||
| return false | ||
| } | ||
| return !found | ||
| }) | ||
| return found | ||
| } | ||
|
|
||
| func reportDiagnostic(pass *analysis.Pass, call *ast.CallExpr, sliceType *ast.ArrayType, rangeExpr ast.Expr, generatedFiles filecheck.GeneratedIndex, noLintIndex nolint.DirectiveIndex) { | ||
| pos := pass.Fset.PositionFor(call.Pos(), false) | ||
| if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { | ||
| return | ||
| } | ||
| if nolint.HasDirectiveForLinter(pos, noLintIndex, "slicemakezerolength") { | ||
| return | ||
| } | ||
| if !coverage.ShouldApply(pass, call.Pos(), *hotThreshold) { | ||
| return | ||
| } | ||
|
|
||
| sliceTypeText := astutil.NodeText(pass.Fset, sliceType) | ||
| lenText := astutil.NodeText(pass.Fset, call.Args[1]) | ||
| rangeText := astutil.NodeText(pass.Fset, rangeExpr) | ||
| if sliceTypeText == "" || lenText == "" || rangeText == "" { | ||
| return | ||
| } | ||
|
|
||
| pass.Report(analysis.Diagnostic{ | ||
| Pos: call.Pos(), | ||
| End: call.End(), | ||
| Message: fmt.Sprintf("make(%s, %s) before this range loop can use capacity len(%s)", sliceTypeText, lenText, rangeText), | ||
| }) | ||
| } | ||
|
|
||
| func isConstantZero(pass *analysis.Pass, expr ast.Expr) bool { | ||
| value := pass.TypesInfo.Types[expr].Value | ||
| return value != nil && constant.Sign(value) == 0 | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in
9ca42be. The analyzer now reports only an immediately following known-size range loop that appends exactly one element per iteration, and its isolated repository-wide run passes without findings.