internal/cmd/deadcode: support -json, -format=template

This change adds support for JSON output and text/template
formatting of output records, in the manner of 'go list (-f|-json)'.
Plus test.

Also, the -generated flag is now enabled by default,
and it affects all output modes.  Plus test.

Updates golang/go#63501

Change-Id: I1374abad78d800f92739de5c75b28e6e5189caa1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/539661
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

internal/cmd/deadcode/deadcode.go

@@ -7,11 +7,14 @@
 package main
 
 import (
+	"bytes"
 	_ "embed"
+	"encoding/json"
 	"flag"
 	"fmt"
 	"go/ast"
 	"go/token"
+	"html/template"
 	"io"
 	"log"
 	"os"
@@ -36,8 +39,9 @@ var (
 	tagsFlag = flag.String("tags", "", "comma-separated list of extra build tags (see: go help buildconstraint)")
 
 	filterFlag    = flag.String("filter", "<module>", "report only packages matching this regular expression (default: module of first package)")
-	generatedFlag = flag.Bool("generated", true, "report dead functions in generated Go files")
-	lineFlag      = flag.Bool("line", false, "show output in a line-oriented format")
+	generatedFlag = flag.Bool("generated", false, "include dead functions in generated Go files")
+	formatFlag    = flag.String("format", "", "format output records using template")
+	jsonFlag      = flag.Bool("json", false, "output JSON records")
 	cpuProfile    = flag.String("cpuprofile", "", "write CPU profile to this file")
 	memProfile    = flag.String("memprofile", "", "write memory profile to this file")
 )
@@ -91,6 +95,18 @@ func main() {
 		}()
 	}
 
+	var tmpl *template.Template
+	if *formatFlag != "" {
+		if *jsonFlag {
+			log.Fatalf("you cannot specify both -format=template and -json")
+		}
+		var err error
+		tmpl, err = template.New("deadcode").Parse(*formatFlag)
+		if err != nil {
+			log.Fatalf("invalid -format: %v", err)
+		}
+	}
+
 	// Load, parse, and type-check the complete program(s).
 	cfg := &packages.Config{
 		BuildFlags: []string{"-tags=" + *tagsFlag},
@@ -108,17 +124,15 @@ func main() {
 		log.Fatalf("packages contain errors")
 	}
 
-	// (Optionally) gather names of generated files.
+	// Gather names of generated files.
 	generated := make(map[string]bool)
-	if !*generatedFlag {
-		packages.Visit(initial, nil, func(p *packages.Package) {
-			for _, file := range p.Syntax {
-				if isGenerated(file) {
-					generated[p.Fset.File(file.Pos()).Name()] = true
-				}
+	packages.Visit(initial, nil, func(p *packages.Package) {
+		for _, file := range p.Syntax {
+			if isGenerated(file) {
+				generated[p.Fset.File(file.Pos()).Name()] = true
 			}
-		})
-	}
+		}
+	})
 
 	// If -filter is unset, use first module (if available).
 	if *filterFlag == "<module>" {
@@ -193,12 +207,6 @@ func main() {
 
 		posn := prog.Fset.Position(fn.Pos())
 
-		// If -generated=false, skip functions declared in generated Go files.
-		// (Functions called by them may still be reported as dead.)
-		if generated[posn.Filename] {
-			continue
-		}
-
 		if !reachablePosn[posn] {
 			reachablePosn[posn] = true // suppress dups with same pos
 
@@ -212,6 +220,8 @@ func main() {
 		}
 	}
 
+	var packages []jsonPackage
+
 	// Report dead functions grouped by packages.
 	// TODO(adonovan): use maps.Keys, twice.
 	pkgpaths := make([]string, 0, len(byPkgPath))
@@ -243,18 +253,68 @@ func main() {
 			return xposn.Line < yposn.Line
 		})
 
-		if *lineFlag {
-			// line-oriented output
-			for _, fn := range fns {
-				fmt.Println(fn)
+		var functions []jsonFunction
+		for _, fn := range fns {
+			posn := prog.Fset.Position(fn.Pos())
+
+			// Without -generated, skip functions declared in
+			// generated Go files.
+			// (Functions called by them may still be reported.)
+			gen := generated[posn.Filename]
+			if gen && !*generatedFlag {
+				continue
 			}
-		} else {
-			// functions grouped by package
-			fmt.Printf("package %q\n", pkgpath)
-			for _, fn := range fns {
-				fmt.Printf("\tfunc %s\n", fn.RelString(fn.Pkg.Pkg))
+
+			functions = append(functions, jsonFunction{
+				Name:      fn.String(),
+				RelName:   fn.RelString(fn.Pkg.Pkg),
+				Posn:      posn.String(),
+				Generated: gen,
+			})
+		}
+		packages = append(packages, jsonPackage{
+			Path:  pkgpath,
+			Funcs: functions,
+		})
+	}
+
+	// Format the output, in the manner of 'go list (-json|-f=template)'.
+	switch {
+	case *jsonFlag:
+		// -json
+		out, err := json.MarshalIndent(packages, "", "\t")
+		if err != nil {
+			log.Fatalf("internal error: %v", err)
+		}
+		os.Stdout.Write(out)
+
+	case tmpl != nil:
+		// -format=template
+		for _, p := range packages {
+			var buf bytes.Buffer
+			if err := tmpl.Execute(&buf, p); err != nil {
+				log.Fatal(err)
+			}
+			if n := buf.Len(); n == 0 || buf.Bytes()[n-1] != '\n' {
+				buf.WriteByte('\n')
+			}
+			os.Stdout.Write(buf.Bytes())
+		}
+
+	default:
+		// functions grouped by package
+		for _, pkg := range packages {
+			seen := false
+			for _, fn := range pkg.Funcs {
+				if !seen {
+					seen = true
+					fmt.Printf("package %q\n", pkg.Path)
+				}
+				fmt.Printf("\tfunc %s\n", fn.RelName)
+			}
+			if seen {
+				fmt.Println()
 			}
-			fmt.Println()
 		}
 	}
 }
@@ -297,3 +357,23 @@ func generator(file *ast.File) (string, bool) {
 	}
 	return "", false
 }
+
+// -- output protocol (for JSON or text/template) --
+
+// Keep in sync with doc comment!
+
+type jsonFunction struct {
+	Name      string // name (with package qualifier)
+	RelName   string // name (sans package qualifier)
+	Posn      string // position in form "filename:line:col"
+	Generated bool   // function is declared in a generated .go file
+}
+
+func (f jsonFunction) String() string { return f.Name }
+
+type jsonPackage struct {
+	Path  string
+	Funcs []jsonFunction
+}
+
+func (p jsonPackage) String() string { return p.Path }

internal/cmd/deadcode/deadcode_test.go

@@ -48,8 +48,9 @@ func Test(t *testing.T) {
 			// Parse archive comment as directives of these forms:
 			//
 			//    deadcode args...		command-line arguments
-			//  [!]want "quoted"		expected/unwanted string in output
+			//  [!]want arg		expected/unwanted string in output
 			//
+			// Args may be Go-quoted strings.
 			var args []string
 			want := make(map[string]bool) // string -> sense
 			for _, line := range strings.Split(string(ar.Comment), "\n") {
@@ -58,17 +59,18 @@ func Test(t *testing.T) {
 					continue // skip blanks and comments
 				}
 
-				fields := strings.Fields(line)
-				switch kind := fields[0]; kind {
+				words, err := words(line)
+				if err != nil {
+					t.Fatalf("cannot break line into words: %v (%s)", err, line)
+				}
+				switch kind := words[0]; kind {
 				case "deadcode":
-					args = fields[1:] // lossy wrt spaces
+					args = words[1:]
 				case "want", "!want":
-					rest := line[len(kind):]
-					str, err := strconv.Unquote(strings.TrimSpace(rest))
-					if err != nil {
-						t.Fatalf("bad %s directive <<%s>>", kind, line)
+					if len(words) != 2 {
+						t.Fatalf("'want' directive needs argument <<%s>>", line)
 					}
-					want[str] = kind[0] != '!'
+					want[words[1]] = kind[0] != '!'
 				default:
 					t.Fatalf("%s: invalid directive %q", filename, kind)
 				}
@@ -129,3 +131,30 @@ func buildDeadcode(t *testing.T) string {
 	}
 	return bin
 }
+
+// words breaks a string into words, respecting
+// Go string quotations around words with spaces.
+func words(s string) ([]string, error) {
+	var words []string
+	for s != "" {
+		if s[0] == ' ' {
+			s = s[1:]
+			continue
+		}
+		var word string
+		if s[0] == '"' || s[0] == '`' {
+			prefix, err := strconv.QuotedPrefix(s)
+			if err != nil {
+				return nil, err
+			}
+			s = s[len(prefix):]
+			word, _ = strconv.Unquote(prefix)
+		} else {
+			prefix, rest, _ := strings.Cut(s, " ")
+			s = rest
+			word = prefix
+		}
+		words = append(words, word)
+	}
+	return words, nil
+}

internal/cmd/deadcode/doc.go

@@ -39,6 +39,10 @@ will fail to recognize that calls to a linkname-annotated function
 with no body in fact dispatch to the function named in the annotation.
 This may result in the latter function being spuriously reported as dead.
 
+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.
+
 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
@@ -48,9 +52,40 @@ Some judgement is required.
 The analysis is valid only for a single GOOS/GOARCH/-tags configuration,
 so a function reported as dead may be live in a different configuration.
 Consider running the tool once for each configuration of interest.
-Use the -line flag to emit a line-oriented output that makes it
+Consider using a line-oriented output format (see below) to make it
 easier to compute the intersection of results across all runs.
 
+# Output
+
+The command supports three output formats.
+
+With no flags, the command prints dead functions grouped by package.
+
+With the -json flag, the command prints an array of JSON Package
+objects, as defined by this schema:
+
+	type Package struct {
+		Path  string
+		Funcs []Function
+	}
+
+	type Function struct {
+		Name      string // name (with package qualifier)
+		RelName   string // name (sans package qualifier)
+		Posn      string // position in form "filename:line:col"
+		Generated bool   // function is declared in a generated .go file
+	}
+
+With the -format=template flag, the command executes the specified template
+on each Package record. So, this template produces a result similar to the
+default format:
+
+	-format='{{printf "package %q\n" .Path}}{{range .Funcs}}{{println "\tfunc " .RelName}}{{end}}{{println}}'
+
+And this template shows only the list of source positions of dead functions:
+
+	-format='{{range .Funcs}}{{println .Posn}}{{end}}'
+
 THIS TOOL IS EXPERIMENTAL and its interface may change.
 At some point it may be published at cmd/deadcode.
 In the meantime, please give us feedback at github.com/golang/go/issues.

internal/cmd/deadcode/testdata/generated.txtar

@@ -0,0 +1,28 @@
+# Test of -generated flag output.
+
+ deadcode example.com
+!want "main"
+ want "Dead1"
+!want "Dead2"
+
+ deadcode -generated example.com
+!want "main"
+ want "Dead1"
+ want "Dead2"
+
+-- go.mod --
+module example.com
+go 1.18
+
+-- main.go --
+package main
+
+func main() {}
+func Dead1() {}
+
+-- gen.go --
+// Code generated by hand. DO NOT EDIT.
+
+package main
+
+func Dead2() {}

internal/cmd/deadcode/testdata/jsonflag.txtar

@@ -0,0 +1,20 @@
+# Very minimal test of -json flag.
+
+deadcode -json example.com/p
+
+ want `"Path": "example.com/p",`
+ want `"Name": "example.com/p.Dead",`
+ want `"RelName": "Dead",`
+ want `"Generated": false`
+
+-- go.mod --
+module example.com
+go 1.18
+
+-- p/p.go --
+package main
+
+func Dead() {}
+
+func main() {}
+

internal/cmd/deadcode/testdata/lineflag.txtar

@@ -1,6 +1,6 @@
-# Test of -line output.
+# Test of line-oriented output.
 
- deadcode -line -filter= example.com
+ deadcode "-format={{range .Funcs}}{{println .Name}}{{end}}" -filter= example.com
 
  want "(example.com.T).Goodbye"
 !want "(example.com.T).Hello"