Skip to content

Commit

Permalink
go/analysis/internal/checker: implement three-way merge
Browse files Browse the repository at this point in the history
This CL adds support for three-way merging to the checker's
-fix operation. It consists of three parts:

1. a rewritten applyFix function that applies as many
   changes as can be cleanly merged;
2. a script-based test framework that allows all existing
   and new tests to be written as txtar files in testdata
   instead of ad-hoc Go logic; and
3. a data-driven "marker" analyzer that reports diagnostics
   containing fixes according to //@f comments in the
   target Go source files.

Also, it adds a -diff flag to the checker tools that causes
them to print the computed file changes instead of directly
applying them.

The new applyFix treats each SuggestedFix as an
independent change, analogous to a git commit.
Fixes are combined by invoking a three-way merge algorithm,
diff.Merge, analogous to git merge, except simpler since it
works on the list of []diff.Edit instead of text.

If any fix does not apply cleanly, we discard it, and
report that we did so, with a hint to run the tool again
until a fixed point is reached. (This is just a starting
point; a better UX would be for the tool to do this itself.)

If a diagnostic has multiple suggested fixes, we select
the first one. The old behavior of attempting to apply
them all makes no sense.

The support for filesystem-level aliases (e.g. symbolic and
hard links) previously implemented using FileID has been
removed, as its interactions with the new logic were tricky.
I ran gopls' modernize singlechecker on k8s/... and it
was able to cleanly resolved 142 edits across 53 files;
the result builds, and symbolic links were not evidently
a problem.

Update golang/go#68765
Update golang/go#67049

Change-Id: Id3fb55118b3d0612cafe7e86f52589812bd74a96
Reviewed-on: https://go-review.googlesource.com/c/tools/+/644835
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
  • Loading branch information
adonovan authored and gopherbot committed Feb 7, 2025
1 parent a9bf6fd commit e65ea15
Show file tree
Hide file tree
Showing 12 changed files with 973 additions and 497 deletions.
6 changes: 6 additions & 0 deletions go/analysis/analysistest/analysistest.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ import (
// and populates it with a GOPATH-style project using filemap (which
// maps file names to contents). On success it returns the name of the
// directory and a cleanup function to delete it.
//
// TODO(adonovan): provide a newer version that accepts a testing.T,
// calls T.TempDir, and calls T.Fatal on any error, avoiding the need
// to return cleanup or err:
//
// func WriteFilesToTmp(t *testing.T filemap map[string]string) string
func WriteFiles(filemap map[string]string) (dir string, cleanup func(), err error) {
gopath, err := os.MkdirTemp("", "analysistest")
if err != nil {
Expand Down
Loading

0 comments on commit e65ea15

Please sign in to comment.