From 40e9af420ca5fde7c8fc7961be4cb60395baf32c Mon Sep 17 00:00:00 2001 From: Peyton Spencer Date: Thu, 9 Jul 2026 00:48:45 -0400 Subject: [PATCH] Add lookup table lint analyzer --- go.work | 1 + lookuptablelint/analyzer.go | 313 ++++++++++++++++++++ lookuptablelint/analyzer_test.go | 11 + lookuptablelint/cmd/lookuptablelint/main.go | 10 + lookuptablelint/doc.go | 3 + lookuptablelint/go.mod | 12 + lookuptablelint/go.sum | 8 + lookuptablelint/testdata/src/a/a.go | 163 ++++++++++ 8 files changed, 521 insertions(+) create mode 100644 lookuptablelint/analyzer.go create mode 100644 lookuptablelint/analyzer_test.go create mode 100644 lookuptablelint/cmd/lookuptablelint/main.go create mode 100644 lookuptablelint/doc.go create mode 100644 lookuptablelint/go.mod create mode 100644 lookuptablelint/go.sum create mode 100644 lookuptablelint/testdata/src/a/a.go diff --git a/go.work b/go.work index f85867b..a3ed35b 100644 --- a/go.work +++ b/go.work @@ -9,6 +9,7 @@ use ( ./errs ./fieldalign ./itemcache + ./lookuptablelint ./mapcache ./net ./pgconv diff --git a/lookuptablelint/analyzer.go b/lookuptablelint/analyzer.go new file mode 100644 index 0000000..76f9dc2 --- /dev/null +++ b/lookuptablelint/analyzer.go @@ -0,0 +1,313 @@ +package lookuptablelint + +import ( + "flag" + "fmt" + "go/ast" + "go/constant" + "go/token" + "go/types" + "strings" + "unicode" + "unicode/utf8" + + "golang.org/x/tools/go/analysis" +) + +const ( + analyzerName = "lookuptablelint" + minLookupEntries = 2 + defaultMaxEntries = 64 +) + +var maxEntries = defaultMaxEntries + +// Analyzer reports static lookup maps that should be predicate helpers. +var Analyzer = &analysis.Analyzer{ + Name: analyzerName, + Doc: "check for small static lookup maps that should be single-case switch predicates", + URL: "https://pkg.go.dev/github.com/omniaura/go-kit/lookuptablelint", + Run: run, +} + +func init() { + Analyzer.Flags.Init(analyzerName, flag.ExitOnError) + Analyzer.Flags.IntVar(&maxEntries, "max_entries", defaultMaxEntries, "maximum lookup-table entries to report") +} + +type tableKind int + +const ( + boolTable tableKind = iota + 1 + structTable +) + +type candidate struct { + name *ast.Ident + obj *types.Var + kind tableKind + count int + + safe bool + lookupUses int +} + +func run(pass *analysis.Pass) (any, error) { + if maxEntries < minLookupEntries { + return nil, nil + } + + candidates := collectCandidates(pass) + if len(candidates) == 0 { + return nil, nil + } + + checkUses(pass, candidates) + for _, cand := range candidates { + if !cand.safe || cand.lookupUses == 0 { + continue + } + pass.Report(analysis.Diagnostic{ + Pos: cand.name.Pos(), + End: cand.name.End(), + Message: fmt.Sprintf("static lookup table %s has %d entries; prefer predicate %s with a single-case switch", cand.name.Name, cand.count, predicateName(cand.name.Name)), + }) + } + return nil, nil +} + +func collectCandidates(pass *analysis.Pass) map[*types.Var]*candidate { + candidates := make(map[*types.Var]*candidate) + for _, file := range pass.Files { + if generated(file) { + continue + } + for _, decl := range file.Decls { + gen, ok := decl.(*ast.GenDecl) + if !ok || gen.Tok != token.VAR { + continue + } + for _, spec := range gen.Specs { + values, ok := spec.(*ast.ValueSpec) + if !ok || len(values.Values) != len(values.Names) { + continue + } + for i, name := range values.Names { + if name == nil || name.Name == "_" || ast.IsExported(name.Name) { + continue + } + lit, ok := values.Values[i].(*ast.CompositeLit) + if !ok { + continue + } + kind, count, ok := lookupTable(pass, lit) + if !ok { + continue + } + obj, ok := pass.TypesInfo.Defs[name].(*types.Var) + if !ok { + continue + } + candidates[obj] = &candidate{ + name: name, + obj: obj, + kind: kind, + count: count, + safe: true, + } + } + } + } + } + return candidates +} + +func lookupTable(pass *analysis.Pass, lit *ast.CompositeLit) (tableKind, int, bool) { + tv, ok := pass.TypesInfo.Types[lit] + if !ok { + return 0, 0, false + } + mapType, ok := tv.Type.(*types.Map) + if !ok || !switchableKeyType(mapType.Key()) { + return 0, 0, false + } + + var kind tableKind + switch { + case isBoolType(mapType.Elem()): + kind = boolTable + case isUnnamedEmptyStruct(mapType.Elem()): + kind = structTable + default: + return 0, 0, false + } + + if len(lit.Elts) < minLookupEntries || len(lit.Elts) > maxEntries { + return 0, 0, false + } + for _, elt := range lit.Elts { + kv, ok := elt.(*ast.KeyValueExpr) + if !ok || !constantKey(pass, kv.Key) { + return 0, 0, false + } + switch kind { + case boolTable: + if !constantTrue(pass, kv.Value) { + return 0, 0, false + } + case structTable: + if !emptyStructValue(pass, kv.Value) { + return 0, 0, false + } + } + } + return kind, len(lit.Elts), true +} + +func checkUses(pass *analysis.Pass, candidates map[*types.Var]*candidate) { + for _, file := range pass.Files { + parents := parentMap(file) + ast.Inspect(file, func(node ast.Node) bool { + id, ok := node.(*ast.Ident) + if !ok { + return true + } + obj, ok := pass.TypesInfo.Uses[id].(*types.Var) + if !ok { + return true + } + cand := candidates[obj] + if cand == nil { + return true + } + if lookupUse(cand, id, parents) { + cand.lookupUses++ + } else { + cand.safe = false + } + return true + }) + } +} + +func lookupUse(cand *candidate, id *ast.Ident, parents map[ast.Node]ast.Node) bool { + index, ok := parents[id].(*ast.IndexExpr) + if !ok || index.X != id || indexWrite(index, parents) { + return false + } + if cand.kind == structTable && !twoValueLookup(index, parents) { + return false + } + return true +} + +func indexWrite(index *ast.IndexExpr, parents map[ast.Node]ast.Node) bool { + switch parent := parents[index].(type) { + case *ast.AssignStmt: + for _, lhs := range parent.Lhs { + if lhs == index { + return true + } + } + case *ast.IncDecStmt: + return parent.X == index + } + return false +} + +func twoValueLookup(index *ast.IndexExpr, parents map[ast.Node]ast.Node) bool { + switch parent := parents[index].(type) { + case *ast.AssignStmt: + return len(parent.Rhs) == 1 && parent.Rhs[0] == index && len(parent.Lhs) == 2 + case *ast.ValueSpec: + return len(parent.Values) == 1 && parent.Values[0] == index && len(parent.Names) == 2 + default: + return false + } +} + +func parentMap(file *ast.File) map[ast.Node]ast.Node { + parents := make(map[ast.Node]ast.Node) + var stack []ast.Node + ast.Inspect(file, func(node ast.Node) bool { + if node == nil { + stack = stack[:len(stack)-1] + return true + } + if len(stack) > 0 { + parents[node] = stack[len(stack)-1] + } + stack = append(stack, node) + return true + }) + return parents +} + +func switchableKeyType(t types.Type) bool { + basic, ok := t.Underlying().(*types.Basic) + if !ok { + return false + } + switch basic.Kind() { + case types.String, + types.Int, types.Int8, types.Int16, types.Int32, types.Int64, + types.Uint, types.Uint8, types.Uint16, types.Uint32, types.Uint64, types.Uintptr: + return true + default: + return false + } +} + +func isBoolType(t types.Type) bool { + basic, ok := t.Underlying().(*types.Basic) + return ok && basic.Kind() == types.Bool +} + +func isUnnamedEmptyStruct(t types.Type) bool { + st, ok := t.(*types.Struct) + return ok && st.NumFields() == 0 +} + +func constantKey(pass *analysis.Pass, expr ast.Expr) bool { + tv, ok := pass.TypesInfo.Types[expr] + return ok && tv.Value != nil +} + +func constantTrue(pass *analysis.Pass, expr ast.Expr) bool { + tv, ok := pass.TypesInfo.Types[expr] + return ok && tv.Value != nil && tv.Value.Kind() == constant.Bool && constant.BoolVal(tv.Value) +} + +func emptyStructValue(pass *analysis.Pass, expr ast.Expr) bool { + lit, ok := expr.(*ast.CompositeLit) + if !ok || len(lit.Elts) != 0 { + return false + } + tv, ok := pass.TypesInfo.Types[lit] + return ok && isUnnamedEmptyStruct(tv.Type) +} + +func predicateName(name string) string { + prefix := "is" + if ast.IsExported(name) { + prefix = "Is" + } + r, size := utf8.DecodeRuneInString(name) + if r == utf8.RuneError && size == 0 { + return prefix + } + return prefix + string(unicode.ToUpper(r)) + name[size:] +} + +func generated(file *ast.File) bool { + for _, group := range file.Comments { + if group.Pos() > file.Package { + break + } + text := group.Text() + if strings.Contains(text, "Code generated") && strings.Contains(text, "DO NOT EDIT") { + return true + } + } + return false +} diff --git a/lookuptablelint/analyzer_test.go b/lookuptablelint/analyzer_test.go new file mode 100644 index 0000000..26269cd --- /dev/null +++ b/lookuptablelint/analyzer_test.go @@ -0,0 +1,11 @@ +package lookuptablelint + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), Analyzer, "a") +} diff --git a/lookuptablelint/cmd/lookuptablelint/main.go b/lookuptablelint/cmd/lookuptablelint/main.go new file mode 100644 index 0000000..12acc80 --- /dev/null +++ b/lookuptablelint/cmd/lookuptablelint/main.go @@ -0,0 +1,10 @@ +package main + +import ( + "github.com/omniaura/go-kit/lookuptablelint" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { + singlechecker.Main(lookuptablelint.Analyzer) +} diff --git a/lookuptablelint/doc.go b/lookuptablelint/doc.go new file mode 100644 index 0000000..893d94a --- /dev/null +++ b/lookuptablelint/doc.go @@ -0,0 +1,3 @@ +// Package lookuptablelint reports small static lookup maps that are +// better expressed as predicate helpers with single-case switch statements. +package lookuptablelint diff --git a/lookuptablelint/go.mod b/lookuptablelint/go.mod new file mode 100644 index 0000000..2478fff --- /dev/null +++ b/lookuptablelint/go.mod @@ -0,0 +1,12 @@ +module github.com/omniaura/go-kit/lookuptablelint + +go 1.25.5 + +toolchain go1.26.1 + +require golang.org/x/tools v0.44.0 + +require ( + golang.org/x/mod v0.35.0 // indirect + golang.org/x/sync v0.20.0 // indirect +) diff --git a/lookuptablelint/go.sum b/lookuptablelint/go.sum new file mode 100644 index 0000000..1050b3b --- /dev/null +++ b/lookuptablelint/go.sum @@ -0,0 +1,8 @@ +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= +golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= +golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= diff --git a/lookuptablelint/testdata/src/a/a.go b/lookuptablelint/testdata/src/a/a.go new file mode 100644 index 0000000..e8b46f7 --- /dev/null +++ b/lookuptablelint/testdata/src/a/a.go @@ -0,0 +1,163 @@ +package a + +const opus = "opus" + +var googleAIOpenAICompatAudioFormats = map[string]bool{ // want `static lookup table googleAIOpenAICompatAudioFormats has 2 entries; prefer predicate isGoogleAIOpenAICompatAudioFormats with a single-case switch` + "mp3": true, + "wav": true, +} + +var supportedImageFormats = map[string]struct{}{ // want `static lookup table supportedImageFormats has 3 entries; prefer predicate isSupportedImageFormats with a single-case switch` + "gif": {}, + "jpeg": {}, + "png": {}, +} + +var constKeyFormats = map[string]bool{ // want `static lookup table constKeyFormats has 2 entries; prefer predicate isConstKeyFormats with a single-case switch` + opus: true, + "wav": true, +} + +var hasFalseValue = map[string]bool{ + "mp3": true, + "wav": false, +} + +var mutableFormats = map[string]bool{ + "mp3": true, + "wav": true, +} + +var rangedFormats = map[string]struct{}{ + "gif": {}, + "png": {}, +} + +var lenFormats = map[string]bool{ + "gif": true, + "png": true, +} + +var structValueUse = map[string]struct{}{ + "gif": {}, + "png": {}, +} + +var intLookup = map[int]bool{ // want `static lookup table intLookup has 2 entries; prefer predicate isIntLookup with a single-case switch` + 1: true, + 2: true, +} + +var ExportedLookup = map[string]bool{ + "gif": true, + "png": true, +} + +var tooSmall = map[string]bool{ + "gif": true, +} + +var tooLarge = map[string]bool{ + "000": true, + "001": true, + "002": true, + "003": true, + "004": true, + "005": true, + "006": true, + "007": true, + "008": true, + "009": true, + "010": true, + "011": true, + "012": true, + "013": true, + "014": true, + "015": true, + "016": true, + "017": true, + "018": true, + "019": true, + "020": true, + "021": true, + "022": true, + "023": true, + "024": true, + "025": true, + "026": true, + "027": true, + "028": true, + "029": true, + "030": true, + "031": true, + "032": true, + "033": true, + "034": true, + "035": true, + "036": true, + "037": true, + "038": true, + "039": true, + "040": true, + "041": true, + "042": true, + "043": true, + "044": true, + "045": true, + "046": true, + "047": true, + "048": true, + "049": true, + "050": true, + "051": true, + "052": true, + "053": true, + "054": true, + "055": true, + "056": true, + "057": true, + "058": true, + "059": true, + "060": true, + "061": true, + "062": true, + "063": true, + "064": true, +} + +func boolLookup(format string) bool { + return googleAIOpenAICompatAudioFormats[format] +} + +func structLookup(format string) bool { + _, ok := supportedImageFormats[format] + return ok +} + +func constKeyLookup(format string) bool { + return constKeyFormats[format] +} + +func mutate(format string) { + mutableFormats[format] = true +} + +func rangeLookup() int { + count := 0 + for range rangedFormats { + count++ + } + return count +} + +func lenLookup() int { + return len(lenFormats) +} + +func structValue(format string) struct{} { + return structValueUse[format] +} + +func intLookupUse(v int) bool { + return intLookup[v] +}