From 033c02d8504848f8b5a0eeead65fd62484817b00 Mon Sep 17 00:00:00 2001 From: Filbert Cia Date: Tue, 7 Oct 2025 00:03:27 +0800 Subject: [PATCH 1/2] cmd/deadcode: exclude marker interface methods from reports by default The deadcode tool now suppresses marker interface methods by default, as these are typically intentional interface implementations rather than dead code. Users can include them in reports using the new -marker flag. Also: in go/ssa/ssautil, Add IsMarkerMethod helper function to identify marker interface methods based on their properties. The function checks that a function: - is unexported - has no parameters beyond the receiver - has no results - has an empty body. Add Marker field to jsonFunction struct to indicate when a function is a marker interface method, allowing users to filter or identify these methods in using the -f template flag. Fixes golang/go#75628 --- cmd/deadcode/deadcode.go | 14 +++++ cmd/deadcode/doc.go | 4 ++ cmd/deadcode/testdata/issue65915.txtar | 17 +++--- cmd/deadcode/testdata/marker.txtar | 23 ++++++++ go/ssa/ssautil/marker.go | 59 +++++++++++++++++++ go/ssa/ssautil/marker_test.go | 79 ++++++++++++++++++++++++++ go/ssa/ssautil/testdata/marker.txtar | 30 ++++++++++ 7 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 cmd/deadcode/testdata/marker.txtar create mode 100644 go/ssa/ssautil/marker.go create mode 100644 go/ssa/ssautil/marker_test.go create mode 100644 go/ssa/ssautil/testdata/marker.txtar diff --git a/cmd/deadcode/deadcode.go b/cmd/deadcode/deadcode.go index 0c0b7ec394e..2e84aea2e10 100644 --- a/cmd/deadcode/deadcode.go +++ b/cmd/deadcode/deadcode.go @@ -45,6 +45,7 @@ var ( filterFlag = flag.String("filter", "", "report only packages matching this regular expression (default: module of first package)") generatedFlag = flag.Bool("generated", false, "include dead functions in generated Go files") + markerFlag = flag.Bool("marker", false, "include marker interface methods in the report") whyLiveFlag = flag.String("whylive", "", "show a path from main to the named function") formatFlag = flag.String("f", "", "format output records using template") jsonFlag = flag.Bool("json", false, "output JSON records") @@ -176,12 +177,16 @@ func main() { // invariably because the parent is unreachable. var sourceFuncs []*ssa.Function generated := make(map[string]bool) + markers := make(map[*ssa.Function]bool) packages.Visit(initial, nil, func(p *packages.Package) { for _, file := range p.Syntax { for _, decl := range file.Decls { if decl, ok := decl.(*ast.FuncDecl); ok { obj := p.TypesInfo.Defs[decl.Name].(*types.Func) fn := prog.FuncValue(obj) + if ssautil.IsMarkerMethod(fn) { + markers[fn] = true + } sourceFuncs = append(sourceFuncs, fn) } } @@ -334,10 +339,18 @@ func main() { continue } + // If the -marker flag is not set to true, + // marker methods should not be reported + marker := markers[fn] + if marker && !*markerFlag { + continue + } + functions = append(functions, jsonFunction{ Name: prettyName(fn, false), Position: toJSONPosition(posn), Generated: gen, + Marker: marker, }) } if len(functions) > 0 { @@ -546,6 +559,7 @@ type jsonFunction struct { Name string // name (sans package qualifier) Position jsonPosition // file/line/column of declaration Generated bool // function is declared in a generated .go file + Marker bool // function is a marker interface method } func (f jsonFunction) String() string { return f.Name } diff --git a/cmd/deadcode/doc.go b/cmd/deadcode/doc.go index bd474248e55..89ef185fb23 100644 --- a/cmd/deadcode/doc.go +++ b/cmd/deadcode/doc.go @@ -43,6 +43,9 @@ By default, the tool does not report dead functions in generated files, as determined by the special comment described in https://go.dev/s/generatedcode. Use the -generated flag to include them. +The tool also does not report marker interface methods by default. +Use the -marker flag to include them. + In any case, just because a function is reported as dead does not mean it is unconditionally safe to delete it. For example, a dead function may be referenced by another dead function, and a dead method may be @@ -121,6 +124,7 @@ is static or dynamic, and its source line number. For example: Name string // name (sans package qualifier) Position Position // file/line/column of function declaration Generated bool // function is declared in a generated .go file + Marker bool // function is a marker interface method } type Edge struct { diff --git a/cmd/deadcode/testdata/issue65915.txtar b/cmd/deadcode/testdata/issue65915.txtar index a7c15630bdd..3f86ec4bdd5 100644 --- a/cmd/deadcode/testdata/issue65915.txtar +++ b/cmd/deadcode/testdata/issue65915.txtar @@ -15,26 +15,27 @@ go 1.18 -- main.go -- package main +import "fmt" type example struct{} -func (e example) UnUsed() {} +func (e example) UnUsed() {fmt.Println("a")} -func (e example) Used() {} +func (e example) Used() {fmt.Println("a")} -func (e example) unUsed() {} +func (e example) unUsed() {fmt.Println("a")} -func (e example) used() {} +func (e example) used() {fmt.Println("a")} type PublicExample struct{} -func (p PublicExample) UnUsed() {} +func (p PublicExample) UnUsed() {fmt.Println("a")} -func (p PublicExample) Used() {} +func (p PublicExample) Used() {fmt.Println("a")} -func (p PublicExample) unUsed() {} +func (p PublicExample) unUsed() {fmt.Println("a")} -func (p PublicExample) used() {} +func (p PublicExample) used() {fmt.Println("a")} func main() { example{}.Used() diff --git a/cmd/deadcode/testdata/marker.txtar b/cmd/deadcode/testdata/marker.txtar new file mode 100644 index 00000000000..70a3163dd58 --- /dev/null +++ b/cmd/deadcode/testdata/marker.txtar @@ -0,0 +1,23 @@ +# Test of basic functionality. + + deadcode -marker -filter= example.com + + want "T.isMarker" +!want "T.Hello" + +-- go.mod -- +module example.com +go 1.18 + +-- main.go -- +package main +import "fmt" + +type T struct{} +func (t *T) isMarker() {} +func (t *T) Hello() {fmt.Println("Hello world")} + +func main () { + var t T + t.Hello() +} \ No newline at end of file diff --git a/go/ssa/ssautil/marker.go b/go/ssa/ssautil/marker.go new file mode 100644 index 00000000000..5300b7b1df2 --- /dev/null +++ b/go/ssa/ssautil/marker.go @@ -0,0 +1,59 @@ +package ssautil + +import ( + "go/ast" + + "golang.org/x/tools/go/ssa" +) + +// IsMarkerMethod returns true if the function is a method that implements a marker interface. +// A marker interface method is defined by the following properties: +// - Is a method (i.e. has a receiver) +// - Is unexported +// - Has no params (other than the receiver) and no results +// - Has an empty function body +func IsMarkerMethod(fn *ssa.Function) bool { + var sig = fn.Signature + + if isMethod := sig.Recv() != nil; !isMethod { + return false + } + if isUnexported := !ast.IsExported(fn.Name()); !isUnexported { + return false + } + if hasNoParams := sig.Params() == nil; !hasNoParams { + return false + } + if hasNoResults := sig.Results() == nil; !hasNoResults { + return false + } + if isEmpty := isFunctionEmpty(fn); !isEmpty { + return false + } + + return true +} + +func isFunctionEmpty(fun *ssa.Function) bool { + // SSA analyzes the source code + // if blocks is nil, it means it's an external (imported) function. This shouldn't be flagged as a marker method + if fun.Blocks == nil { + return false + } + + if len(fun.Blocks) != 1 { + return false + } + + blk := fun.Blocks[0] + if len(blk.Instrs) > 1 { + return false + } + + instr := blk.Instrs[0] + if _, ok := instr.(*ssa.Return); !ok { + return false + } + + return true +} diff --git a/go/ssa/ssautil/marker_test.go b/go/ssa/ssautil/marker_test.go new file mode 100644 index 00000000000..7265a16070f --- /dev/null +++ b/go/ssa/ssautil/marker_test.go @@ -0,0 +1,79 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package ssautil_test + +import ( + "testing" + + "golang.org/x/tools/go/ssa" + "golang.org/x/tools/go/ssa/ssautil" + "golang.org/x/tools/internal/testfiles" + "golang.org/x/tools/txtar" +) + +func TestIsMarkerMethod(t *testing.T) { + tests := []struct { + name string + funcName string + want bool + }{ + { + name: "marker method - unexported, no params, no results, empty body", + funcName: "(*example.com/test.T).isMarker", + want: true, + }, + { + name: "exported method - should not be marker", + funcName: "(*example.com/test.T).IsExported", + want: false, + }, + { + name: "method with parameters - should not be marker", + funcName: "(*example.com/test.T).withParams", + want: false, + }, + { + name: "method with results - should not be marker", + funcName: "(*example.com/test.T).withResults", + want: false, + }, + { + name: "method with non-empty body - should not be marker", + funcName: "(*example.com/test.T).nonEmpty", + want: false, + }, + { + name: "functionNotMethod function - should not be marker", + funcName: "example.com/test.functionNotMethod", + want: false, + }, + } + + testFile, err := txtar.ParseFile("testdata/marker.txtar") + if err != nil { + t.Fatal(err) + } + + ppkgs := testfiles.LoadPackages(t, testFile, ".") + prog, _ := ssautil.Packages(ppkgs, ssa.BuilderMode(0)) + pkg := prog.Package(ppkgs[0].Types) + pkg.Build() + + funcs := make(map[string]*ssa.Function) + for f := range ssautil.AllFunctions(prog) { + funcs[f.String()] = f + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fn, _ := funcs[test.funcName] + + got := ssautil.IsMarkerMethod(fn) + if got != test.want { + t.Errorf("IsMarkerMethod(%s) = %v, want %v", test.funcName, got, test.want) + } + }) + } +} diff --git a/go/ssa/ssautil/testdata/marker.txtar b/go/ssa/ssautil/testdata/marker.txtar new file mode 100644 index 00000000000..9a150e1ea78 --- /dev/null +++ b/go/ssa/ssautil/testdata/marker.txtar @@ -0,0 +1,30 @@ +-- go.mod -- +module example.com/test +go 1.24 + +-- marker.go -- +package main + +// This file is the input to TestIsMarkerMethod in marker_test.go. + +type T struct{} + +// Marker method: unexported, no params, no results, empty body +func (t *T) isMarker() {} + +// Exported method +func (t *T) IsExported() {} + +// Method with parameters +func (t *T) withParams(x int) {} + +// Method with results +func (t *T) withResults() int { return 0 } + +// Method with non-empty body +func (t *T) nonEmpty() { + print("This is not an empty method") +} + +// Function (not a method) +func functionNotMethod() {} From ae0fc950955f87187e5f3ef7cee25451dd50f66c Mon Sep 17 00:00:00 2001 From: Filbert Cia Date: Sat, 11 Oct 2025 10:27:46 +0800 Subject: [PATCH 2/2] feat: add check by interface implementation, move marker functions out of ssautil, cleanup on docs --- cmd/deadcode/deadcode.go | 89 ++++++++++++++++++++++++-- cmd/deadcode/doc.go | 6 ++ cmd/deadcode/testdata/issue65915.txtar | 17 +++-- cmd/deadcode/testdata/marker.txtar | 41 ++++++++++-- go/ssa/ssautil/marker.go | 59 ----------------- go/ssa/ssautil/marker_test.go | 79 ----------------------- go/ssa/ssautil/testdata/marker.txtar | 30 --------- 7 files changed, 132 insertions(+), 189 deletions(-) delete mode 100644 go/ssa/ssautil/marker.go delete mode 100644 go/ssa/ssautil/marker_test.go delete mode 100644 go/ssa/ssautil/testdata/marker.txtar diff --git a/cmd/deadcode/deadcode.go b/cmd/deadcode/deadcode.go index 2e84aea2e10..ac6d8067c16 100644 --- a/cmd/deadcode/deadcode.go +++ b/cmd/deadcode/deadcode.go @@ -177,16 +177,12 @@ func main() { // invariably because the parent is unreachable. var sourceFuncs []*ssa.Function generated := make(map[string]bool) - markers := make(map[*ssa.Function]bool) packages.Visit(initial, nil, func(p *packages.Package) { for _, file := range p.Syntax { for _, decl := range file.Decls { if decl, ok := decl.(*ast.FuncDecl); ok { obj := p.TypesInfo.Defs[decl.Name].(*types.Func) fn := prog.FuncValue(obj) - if ssautil.IsMarkerMethod(fn) { - markers[fn] = true - } sourceFuncs = append(sourceFuncs, fn) } } @@ -304,6 +300,16 @@ func main() { } } + // Collect interfaces by package for marker method identification + interfacesByPkg := make(map[*ssa.Package][]*types.Interface) + for _, fn := range sourceFuncs { + pkg := fn.Package() + if _, exists := interfacesByPkg[pkg]; exists { + continue + } + interfacesByPkg[pkg] = getTopLevelInterfaces(pkg) + } + // Build array of jsonPackage objects. var packages []any for _, pkgpath := range slices.Sorted(maps.Keys(byPkgPath)) { @@ -341,7 +347,7 @@ func main() { // If the -marker flag is not set to true, // marker methods should not be reported - marker := markers[fn] + marker := isMarkerMethod(fn, interfacesByPkg[fn.Package()]) if marker && !*markerFlag { continue } @@ -551,6 +557,79 @@ func cond[T any](cond bool, t, f T) T { } } +func getTopLevelInterfaces(p *ssa.Package) []*types.Interface { + var interfaces []*types.Interface + for _, member := range p.Members { + if typ, ok := member.(*ssa.Type); ok { + if intf, ok := typ.Type().Underlying().(*types.Interface); ok { + interfaces = append(interfaces, intf) + } + } + } + return interfaces +} + +// isMarkerMethod returns true if the function is a method that implements a marker interface. +// A marker interface method is defined by the following properties: +// - Is a method (i.e. has a receiver) +// - Its receiver type implements one of the top-level interfaces in its package +// - Is unexported +// - Has no params (other than the receiver) and no results +// - Has an empty function body +func isMarkerMethod(fn *ssa.Function, interfaces []*types.Interface) bool { + var ( + sig = fn.Signature + implements = func(intf *types.Interface) bool { + return types.Implements(fn.Signature.Recv().Type(), intf) + } + isFunctionEmpty = func(fun *ssa.Function) bool { + // SSA analyzes the source code + // if blocks is nil, it means it's an external (imported) function. + // This shouldn't be flagged as a marker method + if fun.Blocks == nil { + return false + } + + if len(fun.Blocks) != 1 { + return false + } + + blk := fun.Blocks[0] + if len(blk.Instrs) > 1 { + return false + } + + instr := blk.Instrs[0] + if _, ok := instr.(*ssa.Return); !ok { + return false + } + + return true + } + ) + + if isMethod := sig.Recv() != nil; !isMethod { + return false + } + if implementsInterface := slices.ContainsFunc(interfaces, implements); !implementsInterface { + return false + } + if isUnexported := !ast.IsExported(fn.Name()); !isUnexported { + return false + } + if hasNoParams := sig.Params() == nil; !hasNoParams { + return false + } + if hasNoResults := sig.Results() == nil; !hasNoResults { + return false + } + if isEmpty := isFunctionEmpty(fn); !isEmpty { + return false + } + + return true +} + // -- output protocol (for JSON or text/template) -- // Keep in sync with doc comment! diff --git a/cmd/deadcode/doc.go b/cmd/deadcode/doc.go index 89ef185fb23..8bb0ca613ce 100644 --- a/cmd/deadcode/doc.go +++ b/cmd/deadcode/doc.go @@ -44,6 +44,12 @@ as determined by the special comment described in https://go.dev/s/generatedcode. Use the -generated flag to include them. The tool also does not report marker interface methods by default. +Marker interface methods are typically used to create compile-time constraints +to ensure that only specific types can implement a particular interface. +These methods have no other functionality (empty function body) and are never invoked. +Although marker interface methods are technically unreachable, removing them would break +the interface implementation. +Hence, the default behavior is to exclude them from the report. Use the -marker flag to include them. In any case, just because a function is reported as dead does not mean diff --git a/cmd/deadcode/testdata/issue65915.txtar b/cmd/deadcode/testdata/issue65915.txtar index 3f86ec4bdd5..a7c15630bdd 100644 --- a/cmd/deadcode/testdata/issue65915.txtar +++ b/cmd/deadcode/testdata/issue65915.txtar @@ -15,27 +15,26 @@ go 1.18 -- main.go -- package main -import "fmt" type example struct{} -func (e example) UnUsed() {fmt.Println("a")} +func (e example) UnUsed() {} -func (e example) Used() {fmt.Println("a")} +func (e example) Used() {} -func (e example) unUsed() {fmt.Println("a")} +func (e example) unUsed() {} -func (e example) used() {fmt.Println("a")} +func (e example) used() {} type PublicExample struct{} -func (p PublicExample) UnUsed() {fmt.Println("a")} +func (p PublicExample) UnUsed() {} -func (p PublicExample) Used() {fmt.Println("a")} +func (p PublicExample) Used() {} -func (p PublicExample) unUsed() {fmt.Println("a")} +func (p PublicExample) unUsed() {} -func (p PublicExample) used() {fmt.Println("a")} +func (p PublicExample) used() {} func main() { example{}.Used() diff --git a/cmd/deadcode/testdata/marker.txtar b/cmd/deadcode/testdata/marker.txtar index 70a3163dd58..43a028ce972 100644 --- a/cmd/deadcode/testdata/marker.txtar +++ b/cmd/deadcode/testdata/marker.txtar @@ -1,9 +1,18 @@ -# Test of basic functionality. +# Test of -marker functionality. - deadcode -marker -filter= example.com +# Suppresses marker methods by default + deadcode -filter= example.com +!want "T.isMarker" +want "R.isNotMarker" +!want "P.greet" +want "Q.greet" - want "T.isMarker" -!want "T.Hello" +# Includes marker methods if flag is on + deadcode -marker -filter= example.com +want "T.isMarker" +want "R.isNotMarker" +!want "P.greet" +want "Q.greet" -- go.mod -- module example.com @@ -13,11 +22,29 @@ go 1.18 package main import "fmt" +// Marker method: implements interface, unexported, no params, no results, empty body +type Marker interface { + isMarker() +} type T struct{} + func (t *T) isMarker() {} -func (t *T) Hello() {fmt.Println("Hello world")} + +// Not marker method: does not implement interface +type R struct {} +func (r *R) isNotMarker() {} + +// Ensure that it still reports valid interface methods +// when unused +type Greeter interface { + greet() +} +type P struct {} +func (p *P) greet() {fmt.Println("P: hello")} +type Q struct {} +func (q *Q) greet() {fmt.Println("Q: hello")} // this method should be reported func main () { - var t T - t.Hello() + var p P + p.greet() } \ No newline at end of file diff --git a/go/ssa/ssautil/marker.go b/go/ssa/ssautil/marker.go deleted file mode 100644 index 5300b7b1df2..00000000000 --- a/go/ssa/ssautil/marker.go +++ /dev/null @@ -1,59 +0,0 @@ -package ssautil - -import ( - "go/ast" - - "golang.org/x/tools/go/ssa" -) - -// IsMarkerMethod returns true if the function is a method that implements a marker interface. -// A marker interface method is defined by the following properties: -// - Is a method (i.e. has a receiver) -// - Is unexported -// - Has no params (other than the receiver) and no results -// - Has an empty function body -func IsMarkerMethod(fn *ssa.Function) bool { - var sig = fn.Signature - - if isMethod := sig.Recv() != nil; !isMethod { - return false - } - if isUnexported := !ast.IsExported(fn.Name()); !isUnexported { - return false - } - if hasNoParams := sig.Params() == nil; !hasNoParams { - return false - } - if hasNoResults := sig.Results() == nil; !hasNoResults { - return false - } - if isEmpty := isFunctionEmpty(fn); !isEmpty { - return false - } - - return true -} - -func isFunctionEmpty(fun *ssa.Function) bool { - // SSA analyzes the source code - // if blocks is nil, it means it's an external (imported) function. This shouldn't be flagged as a marker method - if fun.Blocks == nil { - return false - } - - if len(fun.Blocks) != 1 { - return false - } - - blk := fun.Blocks[0] - if len(blk.Instrs) > 1 { - return false - } - - instr := blk.Instrs[0] - if _, ok := instr.(*ssa.Return); !ok { - return false - } - - return true -} diff --git a/go/ssa/ssautil/marker_test.go b/go/ssa/ssautil/marker_test.go deleted file mode 100644 index 7265a16070f..00000000000 --- a/go/ssa/ssautil/marker_test.go +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright 2025 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package ssautil_test - -import ( - "testing" - - "golang.org/x/tools/go/ssa" - "golang.org/x/tools/go/ssa/ssautil" - "golang.org/x/tools/internal/testfiles" - "golang.org/x/tools/txtar" -) - -func TestIsMarkerMethod(t *testing.T) { - tests := []struct { - name string - funcName string - want bool - }{ - { - name: "marker method - unexported, no params, no results, empty body", - funcName: "(*example.com/test.T).isMarker", - want: true, - }, - { - name: "exported method - should not be marker", - funcName: "(*example.com/test.T).IsExported", - want: false, - }, - { - name: "method with parameters - should not be marker", - funcName: "(*example.com/test.T).withParams", - want: false, - }, - { - name: "method with results - should not be marker", - funcName: "(*example.com/test.T).withResults", - want: false, - }, - { - name: "method with non-empty body - should not be marker", - funcName: "(*example.com/test.T).nonEmpty", - want: false, - }, - { - name: "functionNotMethod function - should not be marker", - funcName: "example.com/test.functionNotMethod", - want: false, - }, - } - - testFile, err := txtar.ParseFile("testdata/marker.txtar") - if err != nil { - t.Fatal(err) - } - - ppkgs := testfiles.LoadPackages(t, testFile, ".") - prog, _ := ssautil.Packages(ppkgs, ssa.BuilderMode(0)) - pkg := prog.Package(ppkgs[0].Types) - pkg.Build() - - funcs := make(map[string]*ssa.Function) - for f := range ssautil.AllFunctions(prog) { - funcs[f.String()] = f - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - fn, _ := funcs[test.funcName] - - got := ssautil.IsMarkerMethod(fn) - if got != test.want { - t.Errorf("IsMarkerMethod(%s) = %v, want %v", test.funcName, got, test.want) - } - }) - } -} diff --git a/go/ssa/ssautil/testdata/marker.txtar b/go/ssa/ssautil/testdata/marker.txtar deleted file mode 100644 index 9a150e1ea78..00000000000 --- a/go/ssa/ssautil/testdata/marker.txtar +++ /dev/null @@ -1,30 +0,0 @@ --- go.mod -- -module example.com/test -go 1.24 - --- marker.go -- -package main - -// This file is the input to TestIsMarkerMethod in marker_test.go. - -type T struct{} - -// Marker method: unexported, no params, no results, empty body -func (t *T) isMarker() {} - -// Exported method -func (t *T) IsExported() {} - -// Method with parameters -func (t *T) withParams(x int) {} - -// Method with results -func (t *T) withResults() int { return 0 } - -// Method with non-empty body -func (t *T) nonEmpty() { - print("This is not an empty method") -} - -// Function (not a method) -func functionNotMethod() {}