From e08a7ae6bc768150ab5a607623d45dff165065f8 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Sun, 14 Feb 2016 22:14:31 -0500 Subject: [PATCH] cmd/guru: add support for loading modified files The -modified flag causes guru to read a simple archive file from stdin. This archive specifies alternative contents for one or more file names. The build.Context checks this table before delegating to the usual behavior. This will not work for files that import "C" since cgo accesses the file system directly. Added end-to-end test via Emacs. Simplify findQueryPos (now: fileOffsetToPos) Credit: Daniel Morsing, for the prototype of this feature. Change-Id: I5ae818ed5e8bb81001781893dded2d085e9cf8d6 Reviewed-on: https://go-review.googlesource.com/19498 Reviewed-by: Daniel Morsing --- cmd/guru/emacs-test.bash | 1 + cmd/guru/guru.el | 58 ++++++++++++++++------- cmd/guru/guru.go | 16 ++++++- cmd/guru/main.go | 100 ++++++++++++++++++++++++++++++++++++++- cmd/guru/pos.go | 29 +++--------- cmd/guru/referrers.go | 23 ++++++++- 6 files changed, 183 insertions(+), 44 deletions(-) diff --git a/cmd/guru/emacs-test.bash b/cmd/guru/emacs-test.bash index 8dcc8b75..5036d3d0 100755 --- a/cmd/guru/emacs-test.bash +++ b/cmd/guru/emacs-test.bash @@ -35,6 +35,7 @@ emacs --batch --no-splash --no-window-system --no-init \ (progn (princ (emacs-version)) ; requires Emacs v23 (find-file "'$thisdir'/main.go") + (insert "// modify but do not save the editor buffer\n") (search-forward "\"fmt\"") (backward-char) (go-guru-describe) diff --git a/cmd/guru/guru.el b/cmd/guru/guru.el index d00f44c6..e9fed471 100644 --- a/cmd/guru/guru.el +++ b/cmd/guru/guru.el @@ -86,11 +86,6 @@ 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 guru on a buffer without a file name")) - ;; It's not sufficient to save a modified buffer since if - ;; gofmt-before-save is on the before-save-hook, saving will - ;; disturb the selected region. - (if (buffer-modified-p) - (error "Please save the buffer before invoking go-guru")) (and need-scope (string-equal "" go-guru-scope) (go-guru-set-scope)) @@ -105,21 +100,34 @@ file name with a small hyperlink. Display the result." (1- (position-bytes (point)))))) (env-vars (go-root-and-paths)) (goroot-env (concat "GOROOT=" (car env-vars))) - (gopath-env (concat "GOPATH=" (mapconcat #'identity (cdr env-vars) ":")))) - (with-current-buffer (get-buffer-create "*go-guru*") + (gopath-env (concat "GOPATH=" (mapconcat #'identity (cdr env-vars) ":"))) + (output-buffer (get-buffer-create "*go-guru*"))) + (with-current-buffer output-buffer (setq buffer-read-only nil) (erase-buffer) - (insert "Go Guru\n") - (let ((args (list go-guru-command nil t nil - "-scope" go-guru-scope mode posn))) - ;; Log the command to *Messages*, for debugging. - (message "Command: %s:" args) - (message nil) ; clears/shrinks minibuffer - - (message "Running guru...") - ;; Use dynamic binding to modify/restore the environment - (let ((process-environment (list* goroot-env gopath-env process-environment))) - (apply #'call-process args))) + (insert "Go Guru\n")) + (with-current-buffer (get-buffer-create "*go-guru-input*") + (setq buffer-read-only nil) + (erase-buffer) + (go-guru--insert-modified-files) + (let* ((args (list "-modified" + "-scope" go-guru-scope + mode + posn))) + ;; Log the command to *Messages*, for debugging. + (message "Command: %s:" args) + (message nil) ; clears/shrinks minibuffer + (message "Running guru...") + ;; Use dynamic binding to modify/restore the environment + (let ((process-environment (list* goroot-env gopath-env process-environment))) + (apply #'call-process-region (append (list (point-min) + (point-max) + go-guru-command + nil ; delete + output-buffer + t) + args))))) + (with-current-buffer output-buffer (insert "\n") (compilation-mode) (setq compilation-error-screen-columns nil) @@ -158,6 +166,20 @@ file name with a small hyperlink. Display the result." (shrink-window-if-larger-than-buffer w) (set-window-point w (point-min)))))) +(defun go-guru--insert-modified-files () + "Insert the contents of each modified Go buffer into the +current buffer in the format specified by guru's -modified flag." + (mapc #'(lambda (b) + (and (buffer-modified-p b) + (buffer-file-name b) + (string= (file-name-extension (buffer-file-name b)) "go") + (progn + (insert (format "%s\n%d\n" + (buffer-file-name b) + (buffer-size b))) + (insert-buffer-substring b)))) + (buffer-list))) + (defun go-guru-callees () "Show possible callees of the function call at the current point." (interactive) diff --git a/cmd/guru/guru.go b/cmd/guru/guru.go index 8d2da63d..d48f3088 100644 --- a/cmd/guru/guru.go +++ b/cmd/guru/guru.go @@ -252,7 +252,21 @@ func parseQueryPos(lprog *loader.Program, pos string, needExact bool) (*queryPos if err != nil { return nil, err } - start, end, err := findQueryPos(lprog.Fset, filename, startOffset, endOffset) + + // Find the named file among those in the loaded program. + var file *token.File + lprog.Fset.Iterate(func(f *token.File) bool { + if sameFile(filename, f.Name()) { + file = f + return false // done + } + return true // continue + }) + if file == nil { + return nil, fmt.Errorf("file %s not found in loaded program", filename) + } + + start, end, err := fileOffsetToPos(file, startOffset, endOffset) if err != nil { return nil, err } diff --git a/cmd/guru/main.go b/cmd/guru/main.go index 0b85f03e..538bcd5b 100644 --- a/cmd/guru/main.go +++ b/cmd/guru/main.go @@ -13,15 +13,19 @@ package main // import "golang.org/x/tools/cmd/guru" import ( "bufio" + "bytes" "encoding/json" "encoding/xml" "flag" "fmt" "go/build" "io" + "io/ioutil" "log" "os" + "path/filepath" "runtime/pprof" + "strconv" "strings" "golang.org/x/tools/go/buildutil" @@ -29,6 +33,7 @@ import ( // flags var ( + modifiedFlag = flag.Bool("modified", false, "read archive of modified files from standard input") scopeFlag = flag.String("scope", "", "comma-separated list of `packages` the analysis should be limited to (default=all)") ptalogFlag = flag.String("ptalog", "", "write points-to analysis log to `file`") formatFlag = flag.String("format", "plain", "output `format`; one of {plain,json,xml}") @@ -72,6 +77,14 @@ The -format flag controls the output format: json structured data in JSON syntax. xml structured data in XML syntax. +The -modified flag causes guru to read an archive from standard input. + + Files in this archive will be used in preference to those in + the file system. In this way, a text editor may supply guru + with the contents of its unsaved buffers. Each archive entry + consists of the file name, a newline, the decimal file size, + another newline, and the contents of the file. + User manual: http://golang.org/s/oracle-user-manual Example: describe syntax at offset 530 in this file (an import spec): @@ -150,11 +163,31 @@ func main() { log.Fatalf("illegal -format value: %q.\n"+useHelp, *formatFlag) } + ctxt := &build.Default + + // If there were modified files, + // read them from the standard input and + // overlay them on the build context. + if *modifiedFlag { + modified, err := parseArchive(os.Stdin) + if err != nil { + log.Fatal(err) + } + + // All I/O done by guru needs to consult the modified map. + // The ReadFile done by referrers does, + // but the loader's cgo preprocessing currently does not. + + if len(modified) > 0 { + ctxt = useModifiedFiles(ctxt, modified) + } + } + // Ask the guru. query := Query{ Mode: mode, Pos: posn, - Build: &build.Default, + Build: ctxt, Scope: strings.Split(*scopeFlag, ","), PTALog: ptalog, Reflection: *reflectFlag, @@ -184,3 +217,68 @@ func main() { query.WriteTo(os.Stdout) } } + +func parseArchive(archive io.Reader) (map[string][]byte, error) { + modified := make(map[string][]byte) + r := bufio.NewReader(archive) + for { + // Read file name. + filename, err := r.ReadString('\n') + if err != nil { + if err == io.EOF { + break // OK + } + return nil, fmt.Errorf("reading modified file name: %v", err) + } + filename = filepath.Clean(strings.TrimSpace(filename)) + + // Read file size. + sz, err := r.ReadString('\n') + if err != nil { + return nil, fmt.Errorf("reading size of modified file %s: %v", filename, err) + } + sz = strings.TrimSpace(sz) + size, err := strconv.ParseInt(sz, 10, 32) + if err != nil { + return nil, fmt.Errorf("parsing size of modified file %s: %v", filename, err) + } + + // Read file content. + var content bytes.Buffer + content.Grow(int(size)) + if _, err := io.CopyN(&content, r, size); err != nil { + return nil, fmt.Errorf("reading modified file %s: %v", filename, err) + } + modified[filename] = content.Bytes() + } + + return modified, nil +} + +// useModifiedFiles augments the provided build.Context by the +// mapping from file names to alternative contents. +func useModifiedFiles(orig *build.Context, modified map[string][]byte) *build.Context { + rc := func(data []byte) (io.ReadCloser, error) { + return ioutil.NopCloser(bytes.NewBuffer(data)), nil + } + + copy := *orig // make a copy + ctxt := © + ctxt.OpenFile = func(path string) (io.ReadCloser, error) { + // Fast path: names match exactly. + if content, ok := modified[path]; ok { + return rc(content) + } + + // Slow path: check for same file under a different + // alias, perhaps due to a symbolic link. + for filename, content := range modified { + if sameFile(path, filename) { + return rc(content) + } + } + + return buildutil.OpenFile(orig, path) + } + return ctxt +} diff --git a/cmd/guru/pos.go b/cmd/guru/pos.go index d9235b46..4566c06b 100644 --- a/cmd/guru/pos.go +++ b/cmd/guru/pos.go @@ -66,26 +66,11 @@ func parsePos(pos string) (filename string, startOffset, endOffset int, err erro return } -// findQueryPos searches fset for filename and translates the -// specified file-relative byte offsets into token.Pos form. It -// returns an error if the file was not found or the offsets were out -// of bounds. +// fileOffsetToPos translates the specified file-relative byte offsets +// into token.Pos form. It returns an error if the file was not found +// or the offsets were out of bounds. // -func findQueryPos(fset *token.FileSet, filename string, startOffset, endOffset int) (start, end token.Pos, err error) { - var file *token.File - fset.Iterate(func(f *token.File) bool { - if sameFile(filename, f.Name()) { - // (f.Name() is absolute) - file = f - return false // done - } - return true // continue - }) - if file == nil { - err = fmt.Errorf("couldn't find file containing position") - return - } - +func fileOffsetToPos(file *token.File, startOffset, endOffset int) (start, end token.Pos, err error) { // Range check [start..end], inclusive of both end-points. if 0 <= startOffset && startOffset <= file.Size() { @@ -119,8 +104,8 @@ func sameFile(x, y string) bool { return false } -// fastQueryPos parses the position string and returns a QueryPos. -// It parses only a single file, and does not run the type checker. +// fastQueryPos parses the position string and returns a queryPos. +// It parses only a single file and does not run the type checker. func fastQueryPos(pos string) (*queryPos, error) { filename, startOffset, endOffset, err := parsePos(pos) if err != nil { @@ -133,7 +118,7 @@ func fastQueryPos(pos string) (*queryPos, error) { return nil, err } - start, end, err := findQueryPos(fset, filename, startOffset, endOffset) + start, end, err := fileOffsetToPos(fset.File(f.Pos()), startOffset, endOffset) if err != nil { return nil, err } diff --git a/cmd/guru/referrers.go b/cmd/guru/referrers.go index 53d60273..aae82dbe 100644 --- a/cmd/guru/referrers.go +++ b/cmd/guru/referrers.go @@ -8,12 +8,14 @@ import ( "bytes" "fmt" "go/ast" + "go/build" "go/token" "go/types" - "io/ioutil" + "io" "sort" "golang.org/x/tools/cmd/guru/serial" + "golang.org/x/tools/go/buildutil" "golang.org/x/tools/go/loader" "golang.org/x/tools/refactor/importgraph" ) @@ -108,6 +110,7 @@ func referrers(q *Query) error { sort.Sort(byNamePos{q.Fset, refs}) q.result = &referrersResult{ + build: q.Build, qpos: qpos, query: id, obj: obj, @@ -156,6 +159,7 @@ func (p byNamePos) Less(i, j int) bool { } type referrersResult struct { + build *build.Context qpos *queryPos query *ast.Ident // identifier of query obj types.Object // object it denotes @@ -188,7 +192,7 @@ func (r *referrersResult) display(printf printfFunc) { // start asynchronous read. go func() { sema <- struct{}{} // acquire token - content, err := ioutil.ReadFile(posn.Filename) + content, err := readFile(r.build, posn.Filename) <-sema // release token if err != nil { fi.data <- err @@ -224,6 +228,21 @@ func (r *referrersResult) display(printf printfFunc) { } } +// readFile is like ioutil.ReadFile, but +// it goes through the virtualized build.Context. +func readFile(ctxt *build.Context, filename string) ([]byte, error) { + rc, err := buildutil.OpenFile(ctxt, filename) + if err != nil { + return nil, err + } + defer rc.Close() + var buf bytes.Buffer + if _, err := io.Copy(&buf, rc); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + // TODO(adonovan): encode extent, not just Pos info, in Serial form. func (r *referrersResult) toSerial(res *serial.Result, fset *token.FileSet) {