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
diff --git a/cmd/oracle/emacs-test.bash b/cmd/oracle/emacs-test.bash index b525897..b3f7af6 100755 --- a/cmd/oracle/emacs-test.bash +++ b/cmd/oracle/emacs-test.bash
@@ -24,7 +24,7 @@ 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,
diff --git a/cmd/oracle/main.go b/cmd/oracle/main.go index 5dff054..3336f63 100644 --- a/cmd/oracle/main.go +++ b/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. -// -// TODO(adonovan): perhaps -mode should be an args[1] verb, e.g. 'oracle callgraph ...' +// Run with -help flag or help subcommand for usage information. // package main @@ -33,9 +31,6 @@ "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 @@ 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 -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 @@ 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 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 @@ if err := flag.CommandLine.Parse(os.Args[1:]); err != nil { // (err has already been printed) if err == flag.ErrHelp { - fmt.Println(helpMessage) - fmt.Println("Flags:") - flag.PrintDefaults() + printHelp() } 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 @@ case "json", "plain", "xml": // ok default: - 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) + fmt.Fprintf(os.Stderr, "Error: illegal -format value: %q.\n"+useHelp, *formatFlag) 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 @@ 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 @@ 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)
diff --git a/cmd/oracle/oracle.el b/cmd/oracle/oracle.el index 57fc88f..03b267b 100644 --- a/cmd/oracle/oracle.el +++ b/cmd/oracle/oracle.el
@@ -111,9 +111,7 @@ (setq buffer-read-only nil) (erase-buffer) (insert "Go Oracle\n") - (let ((args (append (list go-oracle-command nil t nil - posflag - (format "-mode=%s" mode)) + (let ((args (append (list go-oracle-command nil t nil posflag mode) (split-string go-oracle-scope " " t)))) ;; Log the command to *Messages*, for debugging. (message "Command: %s:" args)
diff --git a/cmd/oracle/oracle.vim b/cmd/oracle/oracle.vim index c156219..cadda78 100644 --- a/cmd/oracle/oracle.vim +++ b/cmd/oracle/oracle.vim
@@ -68,14 +68,14 @@ 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', - \ s:go_oracle, a:mode, - \ shellescape(fname), pos1, pos2, shellescape(sname)) + let cmd = printf('%s -pos=%s:#%d,#%d %s %s', + \ s:go_oracle, + \ 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', - \ s:go_oracle, a:mode, - \ shellescape(fname), pos, shellescape(sname)) + let cmd = printf('%s -pos=%s:#%d %s %s', + \ s:go_oracle, + \ shellescape(fname), pos, a:mode, shellescape(sname)) endif call s:qflist(system(cmd)) endfun
diff --git a/oracle/callees.go b/oracle/callees.go index 66a10c3..3362bb5 100644 --- a/oracle/callees.go +++ b/oracle/callees.go
@@ -5,6 +5,7 @@ package oracle import ( + "fmt" "go/ast" "go/token" "sort" @@ -29,7 +30,7 @@ } } 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 @@ // 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 @@ 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.
diff --git a/oracle/callers.go b/oracle/callers.go index 1233bcc..bba19d2 100644 --- a/oracle/callers.go +++ b/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 @@ 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
diff --git a/oracle/callstack.go b/oracle/callstack.go index afec755..2c3aeba 100644 --- a/oracle/callstack.go +++ b/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 @@ 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], - "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 and build the complete call graph.
diff --git a/oracle/describe.go b/oracle/describe.go index 688beeb..2094191 100644 --- a/oracle/describe.go +++ b/oracle/describe.go
@@ -351,7 +351,7 @@ 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 @@ 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 @@ 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 @@ 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
diff --git a/oracle/freevars.go b/oracle/freevars.go index d3e1af9..0241bc2 100644 --- a/oracle/freevars.go +++ b/oracle/freevars.go
@@ -52,7 +52,7 @@ 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
diff --git a/oracle/oracle.go b/oracle/oracle.go index 2c9a33c..f3b6781 100644 --- a/oracle/oracle.go +++ b/oracle/oracle.go
@@ -18,7 +18,6 @@ import ( "bytes" - "errors" "fmt" "go/ast" "go/build" @@ -257,7 +256,7 @@ // 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 @@ // 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 @@ } 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 @@ 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
diff --git a/oracle/peers.go b/oracle/peers.go index 1039331..41010ae 100644 --- a/oracle/peers.go +++ b/oracle/peers.go
@@ -5,6 +5,7 @@ package oracle import ( + "fmt" "go/ast" "go/token" "sort" @@ -25,7 +26,7 @@ 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 @@ } } 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.
diff --git a/oracle/referrers.go b/oracle/referrers.go index 8ba9ffb..0388930 100644 --- a/oracle/referrers.go +++ b/oracle/referrers.go
@@ -5,6 +5,7 @@ package oracle import ( + "fmt" "go/ast" "go/token" "sort" @@ -19,13 +20,13 @@ 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.
diff --git a/oracle/testdata/src/main/calls.golden b/oracle/testdata/src/main/calls.golden index 225570a..259a798 100644 --- a/oracle/testdata/src/main/calls.golden +++ b/oracle/testdata/src/main/calls.golden
@@ -68,22 +68,18 @@ -------- @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 @@ -------- @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