From 35c670923e21e5c3722e299a75d7cf2fb7daefac Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Fri, 26 Apr 2019 12:45:14 -0400 Subject: [PATCH] internal/lsp: clean up formatting header handling This is mostly to set things up to use golden files the same way for other commands. Change-Id: I7fcc7165706763e655b0e46f0790b367fe5d3d59 Reviewed-on: https://go-review.googlesource.com/c/tools/+/174018 Run-TryBot: Ian Cottrell TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/cmd/cmd_test.go | 66 +++++++++++++++++++ internal/lsp/cmd/format.go | 2 +- internal/lsp/cmd/format_test.go | 55 ++++------------ .../format/bad_format.gofmt-d.golden.go | 5 +- .../format/newline_format.gofmt-d.golden.go | 4 +- 5 files changed, 85 insertions(+), 47 deletions(-) diff --git a/internal/lsp/cmd/cmd_test.go b/internal/lsp/cmd/cmd_test.go index 3a5b45bf..97eb94f1 100644 --- a/internal/lsp/cmd/cmd_test.go +++ b/internal/lsp/cmd/cmd_test.go @@ -5,9 +5,13 @@ package cmd_test import ( + "bytes" "io/ioutil" "os" + "strings" "testing" + "unicode" + "unicode/utf8" "golang.org/x/tools/go/packages/packagestest" "golang.org/x/tools/internal/lsp/cmd" @@ -73,3 +77,65 @@ func captureStdOut(t testing.TB, f func()) string { } return string(data) } + +// normalizePaths replaces all paths present in s with just the fragment portion +// this is used to make golden files not depend on the temporary paths of the files +func (r *runner) normalizePaths(s string) string { + type entry struct { + path string + index int + } + match := make([]entry, len(r.data.Exported.Modules)) + // collect the initial state of all the matchers + for i, m := range r.data.Exported.Modules { + // any random file will do, we collect the first one only + for f := range m.Files { + path := strings.TrimSuffix(r.data.Exported.File(m.Name, f), f) + index := strings.Index(s, path) + match[i] = entry{path, index} + break + } + } + // result should be the same or shorter than the input + buf := bytes.NewBuffer(make([]byte, 0, len(s))) + last := 0 + for { + // find the nearest path match to the start of the buffer + next := -1 + nearest := len(s) + for i, c := range match { + if c.index >= 0 && nearest > c.index { + nearest = c.index + next = i + } + } + // if there are no matches, we copy the rest of the string and are done + if next < 0 { + buf.WriteString(s[last:]) + return buf.String() + } + // we have a match + n := &match[next] + // copy up to the start of the match + buf.WriteString(s[last:n.index]) + // skip over the non fragment prefix + last = n.index + len(n.path) + // now try to convert the fragment part + for last < len(s) { + r, size := utf8.DecodeRuneInString(s[last:]) + if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '/' { + buf.WriteRune(r) + } else if r == '\\' { + buf.WriteRune('/') + } else { + break + } + last += size + } + // see what the next match for this path is + n.index = strings.Index(s[last:], n.path) + if n.index >= 0 { + n.index += last + } + } +} diff --git a/internal/lsp/cmd/format.go b/internal/lsp/cmd/format.go index 02f9bc3a..79403b08 100644 --- a/internal/lsp/cmd/format.go +++ b/internal/lsp/cmd/format.go @@ -97,7 +97,7 @@ func (f *format) Run(ctx context.Context, args ...string) error { } if f.Diff { printIt = false - u := diff.ToUnified(filename, filename, lines, ops) + u := diff.ToUnified(filename+".orig", filename, lines, ops) fmt.Print(u) } if printIt { diff --git a/internal/lsp/cmd/format_test.go b/internal/lsp/cmd/format_test.go index 380c6c53..f95c24a8 100644 --- a/internal/lsp/cmd/format_test.go +++ b/internal/lsp/cmd/format_test.go @@ -7,9 +7,9 @@ package cmd_test import ( "bytes" "context" - "fmt" "io/ioutil" "os/exec" + "regexp" "strings" "testing" @@ -26,14 +26,7 @@ var formatModes = [][]string{ func (r *runner) Format(t *testing.T, data tests.Formats) { for _, spn := range data { for _, mode := range formatModes { - isDiff := false - tag := "gofmt" - for _, arg := range mode { - tag += arg - if arg == "-d" { - isDiff = true - } - } + tag := "gofmt" + strings.Join(mode, "") uri := spn.URI() filename, err := uri.Filename() if err != nil { @@ -45,16 +38,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { buf := &bytes.Buffer{} cmd.Stdout = buf cmd.Run() // ignore error, sometimes we have intentionally ungofmt-able files - contents := buf.String() - // strip the unwanted diff line - if isDiff { - if strings.HasPrefix(contents, "diff -u") { - if i := strings.IndexRune(contents, '\n'); i >= 0 && i < len(contents)-1 { - contents = contents[i+1:] - } - } - contents, _ = stripFileHeader(contents) - } + contents := r.normalizePaths(fixFileHeader(buf.String())) return ioutil.WriteFile(golden, []byte(contents), 0666) })) if expect == "" { @@ -66,13 +50,7 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { got := captureStdOut(t, func() { tool.Main(context.Background(), app, append([]string{"format"}, args...)) }) - if isDiff { - got, err = stripFileHeader(got) - if err != nil { - t.Errorf("%v: got: %v\n%v", filename, err, got) - continue - } - } + got = r.normalizePaths(got) // check the first two lines are the expected file header if expect != got { t.Errorf("format failed with %#v expected:\n%s\ngot:\n%s", args, expect, got) @@ -81,23 +59,12 @@ func (r *runner) Format(t *testing.T, data tests.Formats) { } } -func stripFileHeader(s string) (string, error) { - s = strings.TrimSpace(s) - if !strings.HasPrefix(s, "---") { - return s, fmt.Errorf("missing original") +var unifiedHeader = regexp.MustCompile(`^diff -u.*\n(---\s+\S+\.go\.orig)\s+[\d-:. ]+(\n\+\+\+\s+\S+\.go)\s+[\d-:. ]+(\n@@)`) + +func fixFileHeader(s string) string { + match := unifiedHeader.FindStringSubmatch(s) + if match == nil { + return s } - if i := strings.IndexRune(s, '\n'); i >= 0 && i < len(s)-1 { - s = s[i+1:] - } else { - return s, fmt.Errorf("no EOL for original") - } - if !strings.HasPrefix(s, "+++") { - return s, fmt.Errorf("missing output") - } - if i := strings.IndexRune(s, '\n'); i >= 0 && i < len(s)-1 { - s = s[i+1:] - } else { - return s, fmt.Errorf("no EOL for output") - } - return s, nil + return strings.Join(append(match[1:], s[len(match[0]):]), "") } diff --git a/internal/lsp/testdata/format/bad_format.gofmt-d.golden.go b/internal/lsp/testdata/format/bad_format.gofmt-d.golden.go index a01634c7..1e101d74 100644 --- a/internal/lsp/testdata/format/bad_format.gofmt-d.golden.go +++ b/internal/lsp/testdata/format/bad_format.gofmt-d.golden.go @@ -1,3 +1,5 @@ +--- format/bad_format.go.orig ++++ format/bad_format.go @@ -1,16 +1,13 @@ package format //@format("package") @@ -14,4 +16,5 @@ - - var x int //@diag("x", "LSP", "x declared but not used") - } \ No newline at end of file + } + diff --git a/internal/lsp/testdata/format/newline_format.gofmt-d.golden.go b/internal/lsp/testdata/format/newline_format.gofmt-d.golden.go index 1f356fb5..4470d1ef 100644 --- a/internal/lsp/testdata/format/newline_format.gofmt-d.golden.go +++ b/internal/lsp/testdata/format/newline_format.gofmt-d.golden.go @@ -1,5 +1,7 @@ +--- format/newline_format.go.orig ++++ format/newline_format.go @@ -1,2 +1,2 @@ package format //@format("package") -func _() {} \ No newline at end of file -+func _() {} \ No newline at end of file ++func _() {}