From cfd89ce37302f1f53dd13fc498cd462660964397 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Sat, 3 Oct 2026 20:11:13 -0300 Subject: [PATCH] test: derive enum members and mocked interfaces as live in the dead-code check The check counted a declaration as used only where code named it, so two kinds of declarations that must stay had to be listed by hand in .deadcode-allow, and the list failed again the moment a test started naming one. Both kinds can be read off the types instead. An exported constant of a named type from its own package is an enum member. Its values often arrive by conversion, as Stripe's statuses do through SubscriptionStatus(sub.Status), so no code names the member that matches. It is now live while its type is used anywhere other than its own members' declarations, or while any sibling member is used: InvoiceStatus is never used as a type, only through string(InvoiceStatusOpen), and its uncollectible member is no less real for that. An enum whose type and members nobody uses is still reported. An interface that a type in a mocks/ package implements is mockery's input, and the generated mock never names it. It is now live, checked with types.Implements against every type the mocks declare, so .mockery.yaml is not read. An interface without methods is left out, since every mock would implement it. With both rules nothing in shellhub or cloud needs an entry, so shellhub's .deadcode-allow goes; cloud drops its own on the branch of the same name. The allowlist stays for a real exception. --- .deadcode-allow | 1 - tests/cmd/deadcode/main.go | 94 +++++++++++++++++++++++++++++++-- tests/cmd/deadcode/main_test.go | 68 ++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 5 deletions(-) delete mode 100644 .deadcode-allow diff --git a/.deadcode-allow b/.deadcode-allow deleted file mode 100644 index 0b157e2d4b9..00000000000 --- a/.deadcode-allow +++ /dev/null @@ -1 +0,0 @@ -pkg/api/scope/scope.go KindInvalid zero value of Kind: names the scope nobody constructed, which stores reject diff --git a/tests/cmd/deadcode/main.go b/tests/cmd/deadcode/main.go index 31f46534177..bc8a18a27a3 100644 --- a/tests/cmd/deadcode/main.go +++ b/tests/cmd/deadcode/main.go @@ -9,10 +9,13 @@ import ( "flag" "fmt" "go/ast" + "go/token" + "go/types" "io" "os" "os/exec" "path/filepath" + "slices" "sort" "strings" @@ -486,10 +489,21 @@ func unusedDeclarations(ctx context.Context, dir string, repos []repository, wor declared := map[string]finding{} used := map[string]bool{} + usedBeyondMembers := map[string]bool{} + memberTypeRefs := map[string]bool{} + enumTypeOf := map[string]string{} + membersOf := map[string][]string{} + interfacesAt := map[string][]*types.Interface{} + var mocks []types.Type packages.Visit(pkgs, nil, func(pkg *packages.Package) { for _, file := range pkg.Syntax { - repo, rel, ok := relate(repos, pkg.Fset.File(file.Pos()).Name()) + filename := pkg.Fset.File(file.Pos()).Name() + if strings.Contains(filename, "/mocks/") { + mocks = append(mocks, declaredTypes(pkg, file)...) + } + + repo, rel, ok := relate(repos, filename) if !ok || skipped(rel) || ast.IsGenerated(file) { continue } @@ -505,6 +519,15 @@ func unusedDeclarations(ctx context.Context, dir string, repos []repository, wor switch spec := spec.(type) { case *ast.ValueSpec: names = spec.Names + if gen.Tok == token.CONST && spec.Type != nil { + ast.Inspect(spec.Type, func(n ast.Node) bool { + if ident, ok := n.(*ast.Ident); ok { + memberTypeRefs[pkg.Fset.Position(ident.Pos()).String()] = true + } + + return true + }) + } case *ast.TypeSpec: names = []*ast.Ident{spec.Name} } @@ -516,23 +539,63 @@ func unusedDeclarations(ctx context.Context, dir string, repos []repository, wor pos := pkg.Fset.Position(name.Pos()) declared[pos.String()] = finding{repo: repo.name, path: rel, line: pos.Line, symbol: name.Name, kind: gen.Tok.String()} + + switch obj := pkg.TypesInfo.Defs[name].(type) { + case *types.Const: + if named, ok := obj.Type().(*types.Named); ok && named.Obj().Pkg() == obj.Pkg() { + typePos := pkg.Fset.Position(named.Obj().Pos()).String() + enumTypeOf[pos.String()] = typePos + membersOf[typePos] = append(membersOf[typePos], pos.String()) + } + case *types.TypeName: + if iface, ok := obj.Type().Underlying().(*types.Interface); ok && iface.NumMethods() > 0 { + interfacesAt[pos.String()] = append(interfacesAt[pos.String()], iface) + } + } } } } } for ident, obj := range pkg.TypesInfo.Uses { - if !obj.Pos().IsValid() || strings.Contains(pkg.Fset.Position(ident.Pos()).Filename, "/mocks/") { + at := pkg.Fset.Position(ident.Pos()) + if !obj.Pos().IsValid() || strings.Contains(at.Filename, "/mocks/") { continue } - used[pkg.Fset.Position(obj.Pos()).String()] = true + target := pkg.Fset.Position(obj.Pos()).String() + used[target] = true + if !memberTypeRefs[at.String()] { + usedBeyondMembers[target] = true + } } }) + live := func(pos string) bool { + if used[pos] { + return true + } + + if typePos, ok := enumTypeOf[pos]; ok { + if usedBeyondMembers[typePos] || slices.ContainsFunc(membersOf[typePos], func(member string) bool { return used[member] }) { + return true + } + } + + for _, iface := range interfacesAt[pos] { + for _, mock := range mocks { + if types.Implements(mock, iface) || types.Implements(types.NewPointer(mock), iface) { + return true + } + } + } + + return false + } + var found []finding for pos, f := range declared { - if !used[pos] { + if !live(pos) { found = append(found, f) } } @@ -540,6 +603,29 @@ func unusedDeclarations(ctx context.Context, dir string, repos []repository, wor return found, nil } +func declaredTypes(pkg *packages.Package, file *ast.File) []types.Type { + var declared []types.Type + for _, decl := range file.Decls { + gen, ok := decl.(*ast.GenDecl) + if !ok || gen.Tok != token.TYPE { + continue + } + + for _, spec := range gen.Specs { + typeSpec, ok := spec.(*ast.TypeSpec) + if !ok { + continue + } + + if obj, ok := pkg.TypesInfo.Defs[typeSpec.Name].(*types.TypeName); ok { + declared = append(declared, obj.Type()) + } + } + } + + return declared +} + func readAllowlist(repo repository, allowed map[string]string) error { file, err := os.Open(filepath.Join(repo.root, allowlistName)) if errors.Is(err, os.ErrNotExist) { diff --git a/tests/cmd/deadcode/main_test.go b/tests/cmd/deadcode/main_test.go index 69ea41c66b6..082ae7dcbe9 100644 --- a/tests/cmd/deadcode/main_test.go +++ b/tests/cmd/deadcode/main_test.go @@ -252,3 +252,71 @@ func TestMainCheckout(t *testing.T) { }) } } + +func TestUnusedDeclarations(t *testing.T) { + root := t.TempDir() + files := map[string]string{ + "go.mod": "module github.com/shellhub-io/fixture\n\ngo 1.22\n", + "status/status.go": `package status + +type Status string + +const ( + Active Status = "active" + Canceled Status = "canceled" +) + +func Parse(s string) Status { return Status(s) } +`, + "phase/phase.go": `package phase + +type Phase string + +const ( + PhaseOpen Phase = "open" + PhaseVoid Phase = "void" +) + +func IsOpen(s string) bool { return s == string(PhaseOpen) } +`, + "kind/kind.go": `package kind + +type Kind int + +const ( + KindA Kind = iota + KindB +) +`, + "backend/backend.go": `package backend + +type Backend interface{ Call() } + +type Orphan interface{ Run() } +`, + "backend/mocks/mock_backend.go": `package mocks + +type MockBackend struct{} + +func (MockBackend) Call() {} +`, + } + for name, content := range files { + require.NoError(t, os.MkdirAll(filepath.Join(root, filepath.Dir(name)), 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(root, name), []byte(content), 0o600)) + } + + found, err := unusedDeclarations(t.Context(), root, []repository{{name: "shellhub", root: root}}, "off", "") + require.NoError(t, err) + + var symbols []string + for _, f := range found { + symbols = append(symbols, f.path+" "+f.symbol) + } + + assert.ElementsMatch(t, []string{ + "kind/kind.go KindA", + "kind/kind.go KindB", + "backend/backend.go Orphan", + }, symbols, "an enum member is live while its type or a sibling is used, and a mocked interface is live") +}