cmd/oracle: usability improvements to "describe", "referrers"
Emacs integration:
- eliminate oracle minor mode
- in go-mode, bind F5, F6 to "describe", "referrers".
This reverts a previous policy decision but convenience matters too.
- don't insist on an analysis scope for modes that don't do PTA.
- don't hide the filename as "▶"; show the last 20 chars.
(Especially useful for "referrers" mode.)
- output postprocessing: don't get stuck in a loop if the output
is not as expected (e.g. when it includes a panic log).
referrers:
- show the matching lines (like grep does).
We do the I/O in parallel.
Change-Id: I86b18c1d3a4d9fa4242984cba62b314796669d8e
Reviewed-on: https://go-review.googlesource.com/8120
Reviewed-by: David Crawshaw <crawshaw@golang.org>
diff --git a/cmd/oracle/oracle.el b/cmd/oracle/oracle.el
index f90a5fd..825eee1 100644
--- a/cmd/oracle/oracle.el
+++ b/cmd/oracle/oracle.el
@@ -36,22 +36,21 @@
nil
"History of values supplied to `go-oracle-set-scope'.")
-;; TODO(adonovan): I'd like to get rid of this separate mode since it
-;; makes it harder to use the oracle.
-(defvar go-oracle-mode-map
- (let ((m (make-sparse-keymap)))
- (define-key m (kbd "C-c C-o t") #'go-oracle-describe) ; t for type
- (define-key m (kbd "C-c C-o f") #'go-oracle-freevars)
- (define-key m (kbd "C-c C-o g") #'go-oracle-callgraph)
- (define-key m (kbd "C-c C-o i") #'go-oracle-implements)
- (define-key m (kbd "C-c C-o c") #'go-oracle-peers) ; c for channel
- (define-key m (kbd "C-c C-o r") #'go-oracle-referrers)
- (define-key m (kbd "C-c C-o d") #'go-oracle-definition)
- (define-key m (kbd "C-c C-o p") #'go-oracle-pointsto)
- (define-key m (kbd "C-c C-o s") #'go-oracle-callstack)
- (define-key m (kbd "C-c C-o <") #'go-oracle-callers)
- (define-key m (kbd "C-c C-o >") #'go-oracle-callees)
- m))
+;; Extend go-mode-map.
+(let ((m go-mode-map))
+ (define-key m (kbd "C-c C-o t") #'go-oracle-describe) ; t for type
+ (define-key m (kbd "C-c C-o f") #'go-oracle-freevars)
+ (define-key m (kbd "C-c C-o g") #'go-oracle-callgraph)
+ (define-key m (kbd "C-c C-o i") #'go-oracle-implements)
+ (define-key m (kbd "C-c C-o c") #'go-oracle-peers) ; c for channel
+ (define-key m (kbd "C-c C-o r") #'go-oracle-referrers)
+ (define-key m (kbd "C-c C-o d") #'go-oracle-definition)
+ (define-key m (kbd "C-c C-o p") #'go-oracle-pointsto)
+ (define-key m (kbd "C-c C-o s") #'go-oracle-callstack)
+ (define-key m (kbd "C-c C-o <") #'go-oracle-callers)
+ (define-key m (kbd "C-c C-o >") #'go-oracle-callees)
+ (define-key m (kbd "<f5>") #'go-oracle-describe)
+ (define-key m (kbd "<f6>") #'go-oracle-referrers))
;; TODO(dominikh): Rethink set-scope some. Setting it to a file is
;; painful because it doesn't use find-file, and variables/~ aren't
@@ -84,11 +83,11 @@
(error "You must specify a non-empty scope for the Go oracle"))
(setq go-oracle-scope scope)))
-(defun go-oracle--run (mode)
+(defun go-oracle--run (mode &optional need-scope)
"Run the Go oracle in the specified MODE, passing it the
-selected region of the current buffer. Process the output to
-replace each file name with a small hyperlink. Display the
-result."
+selected region of the current buffer. If NEED-SCOPE, prompt for
+a scope if not already set. Process the output to replace each
+file name with a small hyperlink. Display the result."
(if (not buffer-file-name)
(error "Cannot use oracle on a buffer without a file name"))
;; It's not sufficient to save a modified buffer since if
@@ -96,8 +95,9 @@
;; disturb the selected region.
(if (buffer-modified-p)
(error "Please save the buffer before invoking go-oracle"))
- (if (string-equal "" go-oracle-scope)
- (go-oracle-set-scope))
+ (and need-scope
+ (string-equal "" go-oracle-scope)
+ (go-oracle-set-scope))
(let* ((filename (file-truename buffer-file-name))
(posflag (if (use-region-p)
(format "-pos=%s:#%d,#%d"
@@ -107,7 +107,6 @@
(format "-pos=%s:#%d"
filename
(1- (position-bytes (point))))))
- ;; This would be simpler if we could just run 'go tool oracle'.
(env-vars (go-root-and-paths))
(goroot-env (concat "GOROOT=" (car env-vars)))
(gopath-env (concat "GOPATH=" (mapconcat #'identity (cdr env-vars) ":"))))
@@ -138,14 +137,23 @@
(p 1))
(while (not (null p))
(let ((np (compilation-next-single-property-change p 'compilation-message)))
- ;; TODO(adonovan): this can be verbose in the *Messages* buffer.
- ;; (message "Post-processing link (%d%%)" (/ (* p 100) (point-max)))
(if np
(when (equal (line-number-at-pos p) (line-number-at-pos np))
- ;; np is (typically) the space following ":"; consume it too.
- (put-text-property p np 'display "▶")
+ ;; Using a fixed width greatly improves readability, so
+ ;; if the filename is longer than 20, show ".../last/17chars.go".
+ ;; This usually includes the last segment of the package name.
+ ;; Don't show the line or column number.
+ (let* ((loc (buffer-substring p np)) ; "/home/foo/go/pkg/file.go:1:2-3:4"
+ (i (search ":" loc)))
+ (setq loc (cond
+ ((null i) "...")
+ ((>= i 17) (concat "..." (substring loc (- i 17) i)))
+ (t (substring loc 0 i))))
+ ;; np is (typically) the space following ":"; consume it too.
+ (put-text-property p np 'display (concat loc ":")))
(goto-char np)
- (insert " ")))
+ (insert " ")
+ (incf np))) ; so we don't get stuck (e.g. on a panic stack dump)
(setq p np)))
(message nil))
@@ -157,23 +165,23 @@
(defun go-oracle-callees ()
"Show possible callees of the function call at the current point."
(interactive)
- (go-oracle--run "callees"))
+ (go-oracle--run "callees" t))
(defun go-oracle-callers ()
"Show the set of callers of the function containing the current point."
(interactive)
- (go-oracle--run "callers"))
+ (go-oracle--run "callers" t))
(defun go-oracle-callgraph ()
"Show the callgraph of the current program."
(interactive)
- (go-oracle--run "callgraph"))
+ (go-oracle--run "callgraph" t))
(defun go-oracle-callstack ()
"Show an arbitrary path from a root of the call graph to the
function containing the current point."
(interactive)
- (go-oracle--run "callstack"))
+ (go-oracle--run "callstack" t))
(defun go-oracle-definition ()
"Show the definition of the selected identifier."
@@ -188,7 +196,7 @@
(defun go-oracle-pointsto ()
"Show what the selected expression points to."
(interactive)
- (go-oracle--run "pointsto"))
+ (go-oracle--run "pointsto" t))
(defun go-oracle-implements ()
"Describe the 'implements' relation for types in the package
@@ -205,25 +213,18 @@
"Enumerate the set of possible corresponding sends/receives for
this channel receive/send operation."
(interactive)
- (go-oracle--run "peers"))
+ (go-oracle--run "peers" t))
(defun go-oracle-referrers ()
"Enumerate all references to the object denoted by the selected
identifier."
(interactive)
- (go-oracle--run "referrers"))
+ (go-oracle--run "referrers" t))
(defun go-oracle-whicherrs ()
"Show globals, constants and types to which the selected
expression (of type 'error') may refer."
(interactive)
- (go-oracle--run "whicherrs"))
-
-;; TODO(dominikh): better docstring
-(define-minor-mode go-oracle-mode "Oracle minor mode for go-mode
-
-Keys specific to go-oracle-mode:
-\\{go-oracle-mode-map}"
- nil " oracle" go-oracle-mode-map)
+ (go-oracle--run "whicherrs" t))
(provide 'go-oracle)
diff --git a/oracle/oracle.go b/oracle/oracle.go
index 3cff219..bf65c14 100644
--- a/oracle/oracle.go
+++ b/oracle/oracle.go
@@ -225,6 +225,13 @@
conf := loader.Config{Build: buildContext}
+ // TODO(adonovan): tolerate type errors if we don't need SSA form.
+ // First we'll need ot audit the non-SSA modes for robustness
+ // in the face of type errors.
+ // if minfo.needs&needSSA == 0 {
+ // conf.AllowErrors = true
+ // }
+
// Determine initial packages.
args, err := conf.FromArgs(args, true)
if err != nil {
diff --git a/oracle/referrers.go b/oracle/referrers.go
index 037b6db..d54ffd4 100644
--- a/oracle/referrers.go
+++ b/oracle/referrers.go
@@ -5,15 +5,20 @@
package oracle
import (
+ "bytes"
"fmt"
"go/ast"
"go/token"
+ "io/ioutil"
"sort"
"golang.org/x/tools/go/types"
"golang.org/x/tools/oracle/serial"
)
+// TODO(adonovan): use golang.org/x/tools/refactor/importgraph to choose
+// the scope automatically.
+
// Referrers reports all identifiers that resolve to the same object
// as the queried identifier, within any package in the analysis scope.
//
@@ -41,6 +46,7 @@
sort.Sort(byNamePos(refs))
return &referrersResult{
+ qpos: qpos,
query: id,
obj: obj,
refs: refs,
@@ -71,21 +77,54 @@
func (p byNamePos) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
type referrersResult struct {
+ qpos *QueryPos
query *ast.Ident // identifier of query
obj types.Object // object it denotes
refs []*ast.Ident // set of all other references to it
}
func (r *referrersResult) display(printf printfFunc) {
- if r.query.Pos() != r.obj.Pos() {
- printf(r.query, "reference to %s", r.obj.Name())
+ printf(r.obj, "%d references to %s", len(r.refs), r.obj)
+
+ // Show referring lines, like grep.
+ type fileinfo struct {
+ refs []*ast.Ident
+ linenums []int // line number of refs[i]
+ data chan []byte // file contents
}
- // TODO(adonovan): pretty-print object using same logic as
- // (*describeValueResult).display.
- printf(r.obj, "defined here as %s", r.obj)
+ var fileinfos []*fileinfo
+ fileinfosByName := make(map[string]*fileinfo)
+
+ // First pass: start the file reads concurrently.
for _, ref := range r.refs {
- if r.query != ref {
- printf(ref, "referenced here")
+ posn := r.qpos.fset.Position(ref.Pos())
+ fi := fileinfosByName[posn.Filename]
+ if fi == nil {
+ fi = &fileinfo{data: make(chan []byte)}
+ fileinfosByName[posn.Filename] = fi
+ fileinfos = append(fileinfos, fi)
+
+ // First request for this file:
+ // start asynchronous read.
+ go func() {
+ content, err := ioutil.ReadFile(posn.Filename)
+ if err != nil {
+ content = []byte(fmt.Sprintf("error: %v", err))
+ }
+ fi.data <- content
+ }()
+ }
+ fi.refs = append(fi.refs, ref)
+ fi.linenums = append(fi.linenums, posn.Line)
+ }
+
+ // Second pass: print refs in original order.
+ // One line may have several refs at different columns.
+ for _, fi := range fileinfos {
+ content := <-fi.data // wait for I/O completion
+ lines := bytes.Split(content, []byte("\n"))
+ for i, ref := range fi.refs {
+ printf(ref, "%s", lines[fi.linenums[i]-1])
}
}
}
