go.tools/oracle: change -mode argument into subcommand.

e.g. "oracle callgraph <package>"

Also: simplified error handling.
Eliminated oracle.errorf because it prepends "file:line:col: "
to the error message so the main function can't safely prepend "Error: ".
The position wasn't interesting though: it was just -pos, more or less.

R=crawshaw, dominik.honnef, r
CC=golang-dev
https://golang.org/cl/13864044

cmd/oracle/emacs-test.bash

@@ -24,7 +24,7 @@ trap "rm -f $log" EXIT
 go get code.google.com/p/go.tools/cmd/oracle || die "'go get' failed"
 mv -f $GOPATH/bin/oracle $GOROOT/bin/
 $GOROOT/bin/oracle >$log 2>&1 || true # (prints usage and exits 1)
-grep -q "Usage: oracle" $log || die "$GOROOT/bin/oracle not installed"
+grep -q "Run.*help" $log || die "$GOROOT/bin/oracle not installed"
 
 
 # Run Emacs, set the scope to the oracle tool itself,

cmd/oracle/main.go

@@ -6,9 +6,7 @@
 // http://golang.org/s/oracle-design
 // http://golang.org/s/oracle-user-manual
 //
-// Run with -help for usage information.
+// Run with -help flag or help subcommand for usage information.
-//
-// TODO(adonovan): perhaps -mode should be an args[1] verb, e.g. 'oracle callgraph ...'
 //
 package main
 
@@ -33,9 +31,6 @@ var posFlag = flag.String("pos", "",
 	"Filename and byte offset or extent of a syntax element about which to query, "+
 		"e.g. foo.go:#123,#456, bar.go:#123.")
 
-var modeFlag = flag.String("mode", "",
-	"Mode of query to perform: e.g. callers, describe, etc.")
-
 var ptalogFlag = flag.String("ptalog", "",
 	"Location of the points-to analysis log file, or empty to disable logging.")
 
@@ -47,7 +42,7 @@ var reflectFlag = flag.Bool("reflect", true, "Analyze reflection soundly (slow).
 const useHelp = "Run 'oracle -help' for more information.\n"
 
 const helpMessage = `Go source code oracle.
-Usage: oracle [<flag> ...] <args> ...
+Usage: oracle [<flag> ...] <mode> <args> ...
 
 The -format flag controls the output format:
 	plain	an editor-friendly format in which every line of output
@@ -57,7 +52,8 @@ The -format flag controls the output format:
 
 The -pos flag is required in all modes except 'callgraph'.
 
-The -mode flag determines the query to perform:
+The mode argument determines the query to perform:
+
 	callees	  	show possible targets of selected function call
 	callers	  	show possible callers of selected function
 	callgraph 	show complete callgraph of program
@@ -73,11 +69,11 @@ The user manual is available here:  http://golang.org/s/oracle-user-manual
 Examples:
 
 Describe the syntax at offset 530 in this file (an import spec):
-% oracle -mode=describe -pos=src/code.google.com/p/go.tools/cmd/oracle/main.go:#530 \
+% oracle -pos=src/code.google.com/p/go.tools/cmd/oracle/main.go:#530 describe \
    code.google.com/p/go.tools/cmd/oracle
 
 Print the callgraph of the trivial web-server in JSON format:
-% oracle -mode=callgraph -format=json src/pkg/net/http/triv.go
+% oracle -format=json src/pkg/net/http/triv.go callgraph
 ` + importer.InitialPackagesUsage
 
 var cpuprofile = flag.String("cpuprofile", "", "write cpu profile to file")
@@ -94,6 +90,12 @@ func init() {
 	}
 }
 
+func printHelp() {
+	fmt.Println(helpMessage)
+	fmt.Println("Flags:")
+	flag.PrintDefaults()
+}
+
 func main() {
 	// Don't print full help unless -help was requested.
 	// Just gently remind users that it's there.
@@ -102,13 +104,23 @@ func main() {
 	if err := flag.CommandLine.Parse(os.Args[1:]); err != nil {
 		// (err has already been printed)
 		if err == flag.ErrHelp {
-			fmt.Println(helpMessage)
+			printHelp()
-			fmt.Println("Flags:")
-			flag.PrintDefaults()
 		}
 		os.Exit(2)
 	}
+
 	args := flag.Args()
+	if len(args) == 0 || args[0] == "" {
+		fmt.Fprint(os.Stderr, "Error: a mode argument is required.\n"+useHelp)
+		os.Exit(2)
+	}
+
+	mode := args[0]
+	args = args[1:]
+	if mode == "help" {
+		printHelp()
+		os.Exit(2)
+	}
 
 	if len(args) == 0 {
 		fmt.Fprint(os.Stderr, "Error: no package arguments.\n"+useHelp)
@@ -145,20 +157,14 @@ func main() {
 	case "json", "plain", "xml":
 		// ok
 	default:
-		fmt.Fprintf(os.Stderr, "Error: illegal -format value: %q\n"+useHelp, *formatFlag)
+		fmt.Fprintf(os.Stderr, "Error: illegal -format value: %q.\n"+useHelp, *formatFlag)
-		os.Exit(2)
-	}
-
-	// -mode flag
-	if *modeFlag == "" {
-		fmt.Fprintf(os.Stderr, "Error: a query -mode is required.\n"+useHelp)
 		os.Exit(2)
 	}
 
 	// Ask the oracle.
-	res, err := oracle.Query(args, *modeFlag, *posFlag, ptalog, &build.Default, *reflectFlag)
+	res, err := oracle.Query(args, mode, *posFlag, ptalog, &build.Default, *reflectFlag)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "%s\n"+useHelp, err)
+		fmt.Fprintf(os.Stderr, "Error: %s.\n", err)
 		os.Exit(1)
 	}
 
@@ -167,7 +173,7 @@ func main() {
 	case "json":
 		b, err := json.MarshalIndent(res.Serial(), "", "\t")
 		if err != nil {
-			fmt.Fprintf(os.Stderr, "JSON error: %s\n", err)
+			fmt.Fprintf(os.Stderr, "JSON error: %s.\n", err)
 			os.Exit(1)
 		}
 		os.Stdout.Write(b)
@@ -175,7 +181,7 @@ func main() {
 	case "xml":
 		b, err := xml.MarshalIndent(res.Serial(), "", "\t")
 		if err != nil {
-			fmt.Fprintf(os.Stderr, "XML error: %s\n", err)
+			fmt.Fprintf(os.Stderr, "XML error: %s.\n", err)
 			os.Exit(1)
 		}
 		os.Stdout.Write(b)

cmd/oracle/oracle.el

@@ -111,9 +111,7 @@ result."
       (setq buffer-read-only nil)
       (erase-buffer)
       (insert "Go Oracle\n")
-      (let ((args (append (list go-oracle-command nil t nil
+      (let ((args (append (list go-oracle-command nil t nil posflag mode)
-                                posflag
-                                (format "-mode=%s" mode))
                           (split-string go-oracle-scope " " t))))
         ;; Log the command to *Messages*, for debugging.
         (message "Command: %s:" args)

cmd/oracle/oracle.vim

@@ -68,14 +68,14 @@ func! s:RunOracle(mode, selected) range abort
   if a:selected != -1
     let pos1 = s:getpos(line("'<"), col("'<"))
     let pos2 = s:getpos(line("'>"), col("'>"))
-    let cmd = printf('%s -mode=%s -pos=%s:#%d,#%d %s',
+    let cmd = printf('%s -pos=%s:#%d,#%d %s %s',
-      \  s:go_oracle, a:mode,
+      \  s:go_oracle,
-      \  shellescape(fname), pos1, pos2, shellescape(sname))
+      \  shellescape(fname), pos1, pos2, a:mode, shellescape(sname))
   else
     let pos = s:getpos(line('.'), col('.'))
-    let cmd = printf('%s -mode=%s -pos=%s:#%d %s',
+    let cmd = printf('%s -pos=%s:#%d %s %s',
-      \  s:go_oracle, a:mode,
+      \  s:go_oracle,
-      \  shellescape(fname), pos, shellescape(sname))
+      \  shellescape(fname), pos, a:mode, shellescape(sname))
   endif
   call s:qflist(system(cmd))
 endfun

oracle/callees.go

@@ -5,6 +5,7 @@
 package oracle
 
 import (
+	"fmt"
 	"go/ast"
 	"go/token"
 	"sort"
@@ -29,7 +30,7 @@ func callees(o *Oracle, qpos *QueryPos) (queryResult, error) {
 		}
 	}
 	if call == nil {
-		return nil, o.errorf(qpos.path[0], "there is no function call here")
+		return nil, fmt.Errorf("there is no function call here")
 	}
 	// TODO(adonovan): issue an error if the call is "too far
 	// away" from the current selection, as this most likely is
@@ -37,13 +38,13 @@ func callees(o *Oracle, qpos *QueryPos) (queryResult, error) {
 
 	// Reject type conversions.
 	if qpos.info.IsType(call.Fun) {
-		return nil, o.errorf(call, "this is a type conversion, not a function call")
+		return nil, fmt.Errorf("this is a type conversion, not a function call")
 	}
 
 	// Reject calls to built-ins.
 	if id, ok := unparen(call.Fun).(*ast.Ident); ok {
 		if b, ok := qpos.info.ObjectOf(id).(*types.Builtin); ok {
-			return nil, o.errorf(call, "this is a call to the built-in '%s' operator", b.Name())
+			return nil, fmt.Errorf("this is a call to the built-in '%s' operator", b.Name())
 		}
 	}
 
@@ -74,7 +75,7 @@ func callees(o *Oracle, qpos *QueryPos) (queryResult, error) {
 	ptrAnalysis(o)
 
 	if arbitrarySite == nil {
-		return nil, o.errorf(call.Lparen, "this call site is unreachable in this analysis")
+		return nil, fmt.Errorf("this call site is unreachable in this analysis")
 	}
 
 	// Compute union of callees across all contexts.

oracle/callers.go

@@ -5,6 +5,7 @@
 package oracle
 
 import (
+	"fmt"
 	"go/token"
 
 	"code.google.com/p/go.tools/oracle/serial"
@@ -20,17 +21,17 @@ import (
 func callers(o *Oracle, qpos *QueryPos) (queryResult, error) {
 	pkg := o.prog.Package(qpos.info.Pkg)
 	if pkg == nil {
-		return nil, o.errorf(qpos.path[0], "no SSA package")
+		return nil, fmt.Errorf("no SSA package")
 	}
 	if !ssa.HasEnclosingFunction(pkg, qpos.path) {
-		return nil, o.errorf(qpos.path[0], "this position is not inside a function")
+		return nil, fmt.Errorf("this position is not inside a function")
 	}
 
 	buildSSA(o)
 
 	target := ssa.EnclosingFunction(pkg, qpos.path)
 	if target == nil {
-		return nil, o.errorf(qpos.path[0], "no SSA function built for this location (dead code?)")
+		return nil, fmt.Errorf("no SSA function built for this location (dead code?)")
 	}
 
 	// Run the pointer analysis, recording each

oracle/callstack.go

@@ -5,6 +5,7 @@
 package oracle
 
 import (
+	"fmt"
 	"go/token"
 
 	"code.google.com/p/go.tools/oracle/serial"
@@ -25,19 +26,18 @@ import (
 func callstack(o *Oracle, qpos *QueryPos) (queryResult, error) {
 	pkg := o.prog.Package(qpos.info.Pkg)
 	if pkg == nil {
-		return nil, o.errorf(qpos.path[0], "no SSA package")
+		return nil, fmt.Errorf("no SSA package")
 	}
 
 	if !ssa.HasEnclosingFunction(pkg, qpos.path) {
-		return nil, o.errorf(qpos.path[0], "this position is not inside a function")
+		return nil, fmt.Errorf("this position is not inside a function")
 	}
 
 	buildSSA(o)
 
 	target := ssa.EnclosingFunction(pkg, qpos.path)
 	if target == nil {
-		return nil, o.errorf(qpos.path[0],
+		return nil, fmt.Errorf("no SSA function built for this location (dead code?)")
-			"no SSA function built for this location (dead code?)")
 	}
 
 	// Run the pointer analysis and build the complete call graph.

oracle/describe.go

@@ -351,7 +351,7 @@ func describeValue(o *Oracle, qpos *QueryPos, path []ast.Node) (*describeValueRe
 	switch n := path[0].(type) {
 	case *ast.ValueSpec:
 		// ambiguous ValueSpec containing multiple names
-		return nil, o.errorf(n, "multiple value specification")
+		return nil, fmt.Errorf("multiple value specification")
 	case *ast.Ident:
 		obj = qpos.info.ObjectOf(n)
 		expr = n
@@ -359,7 +359,7 @@ func describeValue(o *Oracle, qpos *QueryPos, path []ast.Node) (*describeValueRe
 		expr = n
 	default:
 		// Is this reachable?
-		return nil, o.errorf(n, "unexpected AST for expr: %T", n)
+		return nil, fmt.Errorf("unexpected AST for expr: %T", n)
 	}
 
 	typ := qpos.info.TypeOf(expr)
@@ -649,7 +649,7 @@ func describeType(o *Oracle, qpos *QueryPos, path []ast.Node) (*describeTypeResu
 
 	default:
 		// Unreachable?
-		return nil, o.errorf(n, "unexpected AST for type: %T", n)
+		return nil, fmt.Errorf("unexpected AST for type: %T", n)
 	}
 
 	return &describeTypeResult{
@@ -737,7 +737,7 @@ func describePackage(o *Oracle, qpos *QueryPos, path []ast.Node) (*describePacka
 
 	default:
 		// Unreachable?
-		return nil, o.errorf(n, "unexpected AST for package: %T", n)
+		return nil, fmt.Errorf("unexpected AST for package: %T", n)
 	}
 
 	var members []*describeMember

oracle/freevars.go

@@ -52,7 +52,7 @@ func freevars(o *Oracle, qpos *QueryPos) (queryResult, error) {
 		obj := qpos.info.ObjectOf(n)
 		if obj == nil {
 			return nil // TODO(adonovan): fix: this fails for *types.Label.
-			panic(o.errorf(n, "no types.Object for ast.Ident"))
+			panic("no types.Object for ast.Ident")
 		}
 		if _, ok := obj.(*types.PkgName); ok {
 			return nil // imported package

oracle/oracle.go

@@ -18,7 +18,6 @@ package oracle
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"go/ast"
 	"go/build"
@@ -257,7 +256,7 @@ func newOracle(imp *importer.Importer, args []string, ptalog io.Writer, needs in
 
 		// Create SSA packages.
 		if err := o.prog.CreatePackages(imp); err != nil {
-			return nil, o.errorf(nil, "%s", err)
+			return nil, err
 		}
 
 		// Initial packages (specified on command line)
@@ -270,7 +269,7 @@ func newOracle(imp *importer.Importer, args []string, ptalog io.Writer, needs in
 				// should build a single synthetic testmain package,
 				// not synthetic main functions to many packages.
 				if initialPkg.CreateTestMainFunction() == nil {
-					return nil, o.errorf(nil, "analysis scope has no main() entry points")
+					return nil, fmt.Errorf("analysis scope has no main() entry points")
 				}
 			}
 			o.config.Mains = append(o.config.Mains, initialPkg)
@@ -324,7 +323,7 @@ func ParseQueryPos(imp *importer.Importer, pos string, needExact bool) (*QueryPo
 	}
 	info, path, exact := imp.PathEnclosingInterval(start, end)
 	if path == nil {
-		return nil, errors.New("no syntax here")
+		return nil, fmt.Errorf("no syntax here")
 	}
 	if needExact && !exact {
 		return nil, fmt.Errorf("ambiguous selection within %s", importer.NodeDescription(path[0]))
@@ -537,13 +536,6 @@ func (o *Oracle) fprintf(w io.Writer, pos interface{}, format string, args ...in
 	io.WriteString(w, "\n")
 }
 
-// errorf is like fprintf, but returns a formatted error string.
-func (o *Oracle) errorf(pos interface{}, format string, args ...interface{}) error {
-	var buf bytes.Buffer
-	o.fprintf(&buf, pos, format, args...)
-	return errors.New(buf.String())
-}
-
 // printNode returns the pretty-printed syntax of n.
 func (o *Oracle) printNode(n ast.Node) string {
 	var buf bytes.Buffer

oracle/peers.go

@@ -5,6 +5,7 @@
 package oracle
 
 import (
+	"fmt"
 	"go/ast"
 	"go/token"
 	"sort"
@@ -25,7 +26,7 @@ import (
 func peers(o *Oracle, qpos *QueryPos) (queryResult, error) {
 	arrowPos := findArrow(qpos)
 	if arrowPos == token.NoPos {
-		return nil, o.errorf(qpos.path[0], "there is no send/receive here")
+		return nil, fmt.Errorf("there is no send/receive here")
 	}
 
 	buildSSA(o)
@@ -49,7 +50,7 @@ func peers(o *Oracle, qpos *QueryPos) (queryResult, error) {
 		}
 	}
 	if queryOp.ch == nil {
-		return nil, o.errorf(arrowPos, "ssa.Instruction for send/receive not found")
+		return nil, fmt.Errorf("ssa.Instruction for send/receive not found")
 	}
 
 	// Discard operations of wrong channel element type.

oracle/referrers.go

@@ -5,6 +5,7 @@
 package oracle
 
 import (
+	"fmt"
 	"go/ast"
 	"go/token"
 	"sort"
@@ -19,13 +20,13 @@ import (
 func referrers(o *Oracle, qpos *QueryPos) (queryResult, error) {
 	id, _ := qpos.path[0].(*ast.Ident)
 	if id == nil {
-		return nil, o.errorf(qpos, "no identifier here")
+		return nil, fmt.Errorf("no identifier here")
 	}
 
 	obj := qpos.info.ObjectOf(id)
 	if obj == nil {
 		// Happens for y in "switch y := x.(type)", but I think that's all.
-		return nil, o.errorf(qpos, "no object for identifier")
+		return nil, fmt.Errorf("no object for identifier")
 	}
 
 	// Iterate over all go/types' resolver facts for the entire program.

oracle/testdata/src/main/calls.golden

@@ -68,22 +68,18 @@ value may point to these labels:
 -------- @callees callees-err-no-call --------
 
 Error: there is no function call here
-
 -------- @callees callees-err-builtin --------
 
 Error: this is a call to the built-in 'print' operator
-
 -------- @callees callees-err-conversion --------
 
 Error: this is a type conversion, not a function call
-
 -------- @callees callees-err-bad-selection --------
 
 Error: ambiguous selection within function call (or conversion)
 -------- @callees callees-err-deadcode1 --------
 
 Error: this call site is unreachable in this analysis
-
 -------- @callees callees-err-nil-func --------
 dynamic function call on nil value
 
@@ -93,7 +89,6 @@ dynamic method call on nil value
 -------- @callees callees-err-deadcode2 --------
 
 Error: this call site is unreachable in this analysis
-
 -------- @callstack callstack-err-deadcode --------
 main.deadcode is unreachable in this analysis scope