diff --git a/go/analysis/internal/analysisflags/flags.go b/go/analysis/internal/analysisflags/flags.go index a3c2f096300..14274eaacf8 100644 --- a/go/analysis/internal/analysisflags/flags.go +++ b/go/analysis/internal/analysisflags/flags.go @@ -168,10 +168,10 @@ func printFlags() { var flags []jsonFlag = nil flag.VisitAll(func(f *flag.Flag) { // Don't report {single,multi}checker debugging - // flags as these have no effect on unitchecker + // flags or fix as these have no effect on unitchecker // (as invoked by 'go vet'). switch f.Name { - case "debug", "cpuprofile", "memprofile", "trace": + case "debug", "cpuprofile", "memprofile", "trace", "fix": return } diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index e3b05066ea7..4a29a968f45 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -15,6 +15,7 @@ import ( "fmt" "go/token" "go/types" + "io/ioutil" "log" "os" "reflect" @@ -44,6 +45,9 @@ var ( // Log files for optional performance tracing. CPUProfile, MemProfile, Trace string + + // Fix determines whether to apply all suggested fixes. + Fix bool ) // RegisterFlags registers command-line flags used by the analysis driver. @@ -56,6 +60,8 @@ func RegisterFlags() { flag.StringVar(&CPUProfile, "cpuprofile", "", "write CPU profile to this file") flag.StringVar(&MemProfile, "memprofile", "", "write memory profile to this file") flag.StringVar(&Trace, "trace", "", "write trace log to this file") + + flag.BoolVar(&Fix, "fix", false, "apply all suggested fixes") } // Run loads the packages specified by args using go/packages, @@ -126,6 +132,10 @@ func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) { // Print the results. roots := analyze(initial, analyzers) + if Fix { + applyFixes(roots) + } + return printDiagnostics(roots) } @@ -250,6 +260,125 @@ func analyze(pkgs []*packages.Package, analyzers []*analysis.Analyzer) []*action return roots } +func applyFixes(roots []*action) { + visited := make(map[*action]bool) + var apply func(*action) error + var visitAll func(actions []*action) error + visitAll = func(actions []*action) error { + for _, act := range actions { + if !visited[act] { + visited[act] = true + visitAll(act.deps) + if err := apply(act); err != nil { + return err + } + } + } + return nil + } + + // TODO(matloob): Is this tree business too complicated? (After all this is Go!) + // Just create a set (map) of edits, sort by pos and call it a day? + type offsetedit struct { + start, end int + newText []byte + } // TextEdit using byteOffsets instead of pos + type node struct { + edit offsetedit + left, right *node + } + + var insert func(tree **node, edit offsetedit) error + insert = func(treeptr **node, edit offsetedit) error { + if *treeptr == nil { + *treeptr = &node{edit, nil, nil} + return nil + } + tree := *treeptr + if edit.end <= tree.edit.start { + return insert(&tree.left, edit) + } else if edit.start >= tree.edit.end { + return insert(&tree.right, edit) + } + + // Overlapping text edit. + return fmt.Errorf("analyses applying overlapping text edits affecting pos range (%v, %v) and (%v, %v)", + edit.start, edit.end, tree.edit.start, tree.edit.end) + + } + + editsForFile := make(map[*token.File]*node) + + apply = func(act *action) error { + for _, diag := range act.diagnostics { + for _, sf := range diag.SuggestedFixes { + for _, edit := range sf.TextEdits { + // Validate the edit. + if edit.Pos > edit.End { + return fmt.Errorf( + "diagnostic for analysis %v contains Suggested Fix with malformed edit: pos (%v) > end (%v)", + act.a.Name, edit.Pos, edit.End) + } + file, endfile := act.pkg.Fset.File(edit.Pos), act.pkg.Fset.File(edit.End) + if file == nil || endfile == nil || file != endfile { + return (fmt.Errorf( + "diagnostic for analysis %v contains Suggested Fix with malformed spanning files %v and %v", + act.a.Name, file.Name(), endfile.Name())) + } + start, end := file.Offset(edit.Pos), file.Offset(edit.End) + + // TODO(matloob): Validate that edits do not affect other packages. + root := editsForFile[file] + if err := insert(&root, offsetedit{start, end, edit.NewText}); err != nil { + return err + } + editsForFile[file] = root // In case the root changed + } + } + } + return nil + } + + visitAll(roots) + + // Now we've got a set of valid edits for each file. Get the new file contents. + for f, tree := range editsForFile { + contents, err := ioutil.ReadFile(f.Name()) + if err != nil { + log.Fatal(err) + } + + cur := 0 // current position in the file + + var out bytes.Buffer + + var recurse func(*node) + recurse = func(node *node) { + if node.left != nil { + recurse(node.left) + } + + edit := node.edit + if edit.start > cur { + out.Write(contents[cur:edit.start]) + out.Write(edit.newText) + } + cur = edit.end + + if node.right != nil { + recurse(node.right) + } + } + recurse(tree) + // Write out the rest of the file. + if cur < len(contents) { + out.Write(contents[cur:]) + } + + ioutil.WriteFile(f.Name(), out.Bytes(), 0644) + } +} + // printDiagnostics prints the diagnostics for the root packages in either // plain text or JSON format. JSON format also includes errors for any // dependencies. diff --git a/go/analysis/internal/checker/checker_test.go b/go/analysis/internal/checker/checker_test.go new file mode 100644 index 00000000000..8a3ec4b2f64 --- /dev/null +++ b/go/analysis/internal/checker/checker_test.go @@ -0,0 +1,88 @@ +package checker_test + +import ( + "fmt" + "go/ast" + "io/ioutil" + "path/filepath" + "testing" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/internal/checker" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +var from, to string + +func TestApplyFixes(t *testing.T) { + from = "bar" + to = "baz" + + files := map[string]string{ + "rename/test.go": `package rename + +func Foo() { + bar := 12 + _ = bar +} + +// the end +`} + want := `package rename + +func Foo() { + baz := 12 + _ = baz +} + +// the end +` + + testdata, cleanup, err := analysistest.WriteFiles(files) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(testdata, "src/rename/test.go") + checker.Fix = true + checker.Run([]string{"file=" + path}, []*analysis.Analyzer{analyzer}) + + contents, err := ioutil.ReadFile(path) + if err != nil { + t.Fatal(err) + } + + got := string(contents) + if got != want { + t.Errorf("contents of rewrtitten file\ngot: %s\nwant: %s", got, want) + } + + defer cleanup() +} + +var analyzer = &analysis.Analyzer{ + Name: "rename", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{(*ast.Ident)(nil)} + inspect.Preorder(nodeFilter, func(n ast.Node) { + ident := n.(*ast.Ident) + if ident.Name == from { + msg := fmt.Sprintf("renaming %q to %q", from, to) + pass.Report(analysis.Diagnostic{ + ident.Pos(), ident.End(), "", msg, + []analysis.SuggestedFix{ + {msg, []analysis.TextEdit{ + {ident.Pos(), ident.End(), []byte(to)}}, + }}, + }) + } + }) + + return nil, nil +}