diff --git a/go/tools/builders/BUILD.bazel b/go/tools/builders/BUILD.bazel index 520de2e3f7..d4924ff84a 100644 --- a/go/tools/builders/BUILD.bazel +++ b/go/tools/builders/BUILD.bazel @@ -11,6 +11,18 @@ go_test( ], ) +go_test( + name = "cover_test", + size = "small", + srcs = [ + "cover.go", + "cover_test.go", + "edit.go", + "env.go", + "flags.go", + ], +) + filegroup( name = "builder_srcs", srcs = [ @@ -21,6 +33,7 @@ filegroup( "compile.go", "compilepkg.go", "cover.go", + "edit.go", "embedcfg.go", "env.go", "filter.go", @@ -28,7 +41,6 @@ filegroup( "flags.go", "generate_nogo_main.go", "generate_test_main.go", - "imports.go", "importcfg.go", "link.go", "pack.go", diff --git a/go/tools/builders/cover.go b/go/tools/builders/cover.go index 860705003d..efaab8b61a 100644 --- a/go/tools/builders/cover.go +++ b/go/tools/builders/cover.go @@ -18,10 +18,10 @@ import ( "bytes" "flag" "fmt" - "go/format" "go/parser" "go/token" "io/ioutil" + "os" "strconv" ) @@ -74,19 +74,29 @@ func instrumentForCoverage(goenv *env, srcPath, srcName, coverVar, mode, outPath return registerCoverage(outPath, coverVar, srcName) } -// registerCoverage modifies coverSrc, the output file from go tool cover. It -// adds a call to coverdata.RegisterCoverage, which ensures the coverage +// registerCoverage modifies coverSrcFilename, the output file from go tool cover. +// It adds a call to coverdata.RegisterCoverage, which ensures the coverage // data from each file is reported. The name by which the file is registered // need not match its original name (it may use the importpath). -func registerCoverage(coverSrc, varName, srcName string) error { +func registerCoverage(coverSrcFilename, varName, srcName string) error { + coverSrc, err := os.ReadFile(coverSrcFilename) + if err != nil { + return fmt.Errorf("instrumentForCoverage: reading instrumented source: %w", err) + } + // Parse the file. fset := token.NewFileSet() - f, err := parser.ParseFile(fset, coverSrc, nil, parser.ParseComments) + f, err := parser.ParseFile(fset, coverSrcFilename, coverSrc, parser.ParseComments) if err != nil { return nil // parse error: proceed and let the compiler fail } - // Ensure coverdata is imported in the AST. Use an existing import if present + // Perform edits using a byte buffer instead of the AST, because + // we can not use go/format to write the AST back out without + // changing line numbers. + editor := NewBuffer(coverSrc) + + // Ensure coverdata is imported. Use an existing import if present // or add a new one. const coverdataPath = "github.com/bazelbuild/rules_go/go/tools/coverdata" var coverdataName string @@ -100,9 +110,14 @@ func registerCoverage(coverSrc, varName, srcName string) error { // renaming import if imp.Name.Name == "_" { // Change blank import to named import - imp.Name.Name = "coverdata" + editor.Replace( + fset.Position(imp.Name.Pos()).Offset, + fset.Position(imp.Name.End()).Offset, + "coverdata") + coverdataName = "coverdata" + } else { + coverdataName = imp.Name.Name } - coverdataName = imp.Name.Name } else { // default import coverdataName = "coverdata" @@ -113,15 +128,12 @@ func registerCoverage(coverSrc, varName, srcName string) error { if coverdataName == "" { // No existing import. Add a new one. coverdataName = "coverdata" - addNamedImport(fset, f, coverdataName, coverdataPath) - } - var buf bytes.Buffer - if err := format.Node(&buf, fset, f); err != nil { - return fmt.Errorf("registerCoverage: could not reformat coverage source %s: %v", coverSrc, err) + editor.Insert(fset.Position(f.Name.End()).Offset, fmt.Sprintf("; import %q", coverdataPath)) } // Append an init function. - fmt.Fprintf(&buf, ` + var buf = bytes.NewBuffer(editor.Bytes()) + fmt.Fprintf(buf, ` func init() { %s.RegisterFile(%q, %[3]s.Count[:], @@ -129,9 +141,8 @@ func init() { %[3]s.NumStmt[:]) } `, coverdataName, srcName, varName) - if err := ioutil.WriteFile(coverSrc, buf.Bytes(), 0666); err != nil { + if err := ioutil.WriteFile(coverSrcFilename, buf.Bytes(), 0666); err != nil { return fmt.Errorf("registerCoverage: %v", err) } - return nil } diff --git a/go/tools/builders/cover_test.go b/go/tools/builders/cover_test.go new file mode 100644 index 0000000000..fc1ba8180e --- /dev/null +++ b/go/tools/builders/cover_test.go @@ -0,0 +1,130 @@ +package main + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" +) + +type test struct { + name string + in string + out string +} + +var tests = []test{ + { + name: "no imports", + in: `package main +`, + out: `package main; import "github.com/bazelbuild/rules_go/go/tools/coverdata" + +func init() { + coverdata.RegisterFile("srcName", + varName.Count[:], + varName.Pos[:], + varName.NumStmt[:]) +} +`, + }, + { + name: "other imports", + in: `package main + +import ( + "os" +) +`, + out: `package main; import "github.com/bazelbuild/rules_go/go/tools/coverdata" + +import ( + "os" +) + +func init() { + coverdata.RegisterFile("srcName", + varName.Count[:], + varName.Pos[:], + varName.NumStmt[:]) +} +`, + }, + { + name: "existing import", + in: `package main + +import "github.com/bazelbuild/rules_go/go/tools/coverdata" +`, + out: `package main + +import "github.com/bazelbuild/rules_go/go/tools/coverdata" + +func init() { + coverdata.RegisterFile("srcName", + varName.Count[:], + varName.Pos[:], + varName.NumStmt[:]) +} +`, + }, + { + name: "existing _ import", + in: `package main + +import _ "github.com/bazelbuild/rules_go/go/tools/coverdata" +`, + out: `package main + +import coverdata "github.com/bazelbuild/rules_go/go/tools/coverdata" + +func init() { + coverdata.RegisterFile("srcName", + varName.Count[:], + varName.Pos[:], + varName.NumStmt[:]) +} +`, + }, + { + name: "existing renamed import", + in: `package main + +import cover0 "github.com/bazelbuild/rules_go/go/tools/coverdata" +`, + out: `package main + +import cover0 "github.com/bazelbuild/rules_go/go/tools/coverdata" + +func init() { + cover0.RegisterFile("srcName", + varName.Count[:], + varName.Pos[:], + varName.NumStmt[:]) +} +`, + }, +} + +func TestRegisterCoverage(t *testing.T) { + var filename = filepath.Join(t.TempDir(), "test_input.go") + for _, test := range tests { + if err := ioutil.WriteFile(filename, []byte(test.in), 0666); err != nil { + t.Errorf("writing input file: %v", err) + return + } + err := registerCoverage(filename, "varName", "srcName") + if err != nil { + t.Errorf("%q: %+v", test.name, err) + continue + } + coverSrc, err := os.ReadFile(filename) + if err != nil { + t.Errorf("%q: %+v", test.name, err) + continue + } + if got, want := string(coverSrc), test.out; got != want { + t.Errorf("%q: got %v, want %v", test.name, got, want) + } + } +} diff --git a/go/tools/builders/edit.go b/go/tools/builders/edit.go new file mode 100644 index 0000000000..f8ccd52b52 --- /dev/null +++ b/go/tools/builders/edit.go @@ -0,0 +1,95 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Copied from go1.17 tree: //src/cmd/internal/edit/edit.go + +// Package edit implements buffered position-based editing of byte slices. +package main + +import ( + "fmt" + "sort" +) + +// A Buffer is a queue of edits to apply to a given byte slice. +type Buffer struct { + old []byte + q edits +} + +// An edit records a single text modification: change the bytes in [start,end) to new. +type edit struct { + start int + end int + new string +} + +// An edits is a list of edits that is sortable by start offset, breaking ties by end offset. +type edits []edit + +func (x edits) Len() int { return len(x) } +func (x edits) Swap(i, j int) { x[i], x[j] = x[j], x[i] } +func (x edits) Less(i, j int) bool { + if x[i].start != x[j].start { + return x[i].start < x[j].start + } + return x[i].end < x[j].end +} + +// NewBuffer returns a new buffer to accumulate changes to an initial data slice. +// The returned buffer maintains a reference to the data, so the caller must ensure +// the data is not modified until after the Buffer is done being used. +func NewBuffer(data []byte) *Buffer { + return &Buffer{old: data} +} + +func (b *Buffer) Insert(pos int, new string) { + if pos < 0 || pos > len(b.old) { + panic("invalid edit position") + } + b.q = append(b.q, edit{pos, pos, new}) +} + +func (b *Buffer) Delete(start, end int) { + if end < start || start < 0 || end > len(b.old) { + panic("invalid edit position") + } + b.q = append(b.q, edit{start, end, ""}) +} + +func (b *Buffer) Replace(start, end int, new string) { + if end < start || start < 0 || end > len(b.old) { + panic("invalid edit position") + } + b.q = append(b.q, edit{start, end, new}) +} + +// Bytes returns a new byte slice containing the original data +// with the queued edits applied. +func (b *Buffer) Bytes() []byte { + // Sort edits by starting position and then by ending position. + // Breaking ties by ending position allows insertions at point x + // to be applied before a replacement of the text at [x, y). + sort.Stable(b.q) + + var new []byte + offset := 0 + for i, e := range b.q { + if e.start < offset { + e0 := b.q[i-1] + panic(fmt.Sprintf("overlapping edits: [%d,%d)->%q, [%d,%d)->%q", e0.start, e0.end, e0.new, e.start, e.end, e.new)) + } + new = append(new, b.old[offset:e.start]...) + offset = e.end + new = append(new, e.new...) + } + new = append(new, b.old[offset:]...) + return new +} + +// String returns a string containing the original data +// with the queued edits applied. +func (b *Buffer) String() string { + return string(b.Bytes()) +} diff --git a/go/tools/builders/imports.go b/go/tools/builders/imports.go deleted file mode 100644 index 7272f61517..0000000000 --- a/go/tools/builders/imports.go +++ /dev/null @@ -1,221 +0,0 @@ -// Copyright 2013 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found at -// -// https://github.com/golang/tools/blob/master/LICENSE - -package main - -import ( - "go/ast" - "go/token" - "strconv" - "strings" -) - -// Adapted from golang.org/x/tools/go/ast/astutil.AddNamedImport -func addNamedImport(fset *token.FileSet, f *ast.File, name, path string) bool { - newImport := &ast.ImportSpec{ - Path: &ast.BasicLit{ - Kind: token.STRING, - Value: strconv.Quote(path), - }, - } - if name != "" { - newImport.Name = &ast.Ident{Name: name} - } - - // Find an import decl to add to. - // The goal is to find an existing import - // whose import path has the longest shared - // prefix with path. - var ( - bestMatch = -1 // length of longest shared prefix - lastImport = -1 // index in f.Decls of the file's final import decl - impDecl *ast.GenDecl // import decl containing the best match - impIndex = -1 // spec index in impDecl containing the best match - - isThirdPartyPath = isThirdParty(path) - ) - for i, decl := range f.Decls { - gen, ok := decl.(*ast.GenDecl) - if ok && gen.Tok == token.IMPORT { - lastImport = i - // Do not add to import "C", to avoid disrupting the - // association with its doc comment, breaking cgo. - if declImports(gen, "C") { - continue - } - - // Match an empty import decl if that's all that is available. - if len(gen.Specs) == 0 && bestMatch == -1 { - impDecl = gen - } - - // Compute longest shared prefix with imports in this group and find best - // matched import spec. - // 1. Always prefer import spec with longest shared prefix. - // 2. While match length is 0, - // - for stdlib package: prefer first import spec. - // - for third party package: prefer first third party import spec. - // We cannot use last import spec as best match for third party package - // because grouped imports are usually placed last by goimports -local - // flag. - // See issue #19190. - seenAnyThirdParty := false - for j, spec := range gen.Specs { - impspec := spec.(*ast.ImportSpec) - p := importPath(impspec) - n := matchLen(p, path) - if n > bestMatch || (bestMatch == 0 && !seenAnyThirdParty && isThirdPartyPath) { - bestMatch = n - impDecl = gen - impIndex = j - } - seenAnyThirdParty = seenAnyThirdParty || isThirdParty(p) - } - } - } - - // If no import decl found, add one after the last import. - if impDecl == nil { - impDecl = &ast.GenDecl{ - Tok: token.IMPORT, - } - if lastImport >= 0 { - impDecl.TokPos = f.Decls[lastImport].End() - } else { - // There are no existing imports. - // Our new import, preceded by a blank line, goes after the package declaration - // and after the comment, if any, that starts on the same line as the - // package declaration. - impDecl.TokPos = f.Package - - file := fset.File(f.Package) - pkgLine := file.Line(f.Package) - for _, c := range f.Comments { - if file.Line(c.Pos()) > pkgLine { - break - } - // +2 for a blank line - impDecl.TokPos = c.End() + 2 - } - } - f.Decls = append(f.Decls, nil) - copy(f.Decls[lastImport+2:], f.Decls[lastImport+1:]) - f.Decls[lastImport+1] = impDecl - } - - // Insert new import at insertAt. - insertAt := 0 - if impIndex >= 0 { - // insert after the found import - insertAt = impIndex + 1 - } - impDecl.Specs = append(impDecl.Specs, nil) - copy(impDecl.Specs[insertAt+1:], impDecl.Specs[insertAt:]) - impDecl.Specs[insertAt] = newImport - pos := impDecl.Pos() - if insertAt > 0 { - // If there is a comment after an existing import, preserve the comment - // position by adding the new import after the comment. - if spec, ok := impDecl.Specs[insertAt-1].(*ast.ImportSpec); ok && spec.Comment != nil { - pos = spec.Comment.End() - } else { - // Assign same position as the previous import, - // so that the sorter sees it as being in the same block. - pos = impDecl.Specs[insertAt-1].Pos() - } - } - if newImport.Name != nil { - newImport.Name.NamePos = pos - } - newImport.Path.ValuePos = pos - newImport.EndPos = pos - - // Clean up parens. impDecl contains at least one spec. - if len(impDecl.Specs) == 1 { - // Remove unneeded parens. - impDecl.Lparen = token.NoPos - } else if !impDecl.Lparen.IsValid() { - // impDecl needs parens added. - impDecl.Lparen = impDecl.Specs[0].Pos() - } - - f.Imports = append(f.Imports, newImport) - - if len(f.Decls) <= 1 { - return true - } - - // Merge all the import declarations into the first one. - var first *ast.GenDecl - for i := 0; i < len(f.Decls); i++ { - decl := f.Decls[i] - gen, ok := decl.(*ast.GenDecl) - if !ok || gen.Tok != token.IMPORT || declImports(gen, "C") { - continue - } - if first == nil { - first = gen - continue // Don't touch the first one. - } - // We now know there is more than one package in this import - // declaration. Ensure that it ends up parenthesized. - first.Lparen = first.Pos() - // Move the imports of the other import declaration to the first one. - for _, spec := range gen.Specs { - spec.(*ast.ImportSpec).Path.ValuePos = first.Pos() - first.Specs = append(first.Specs, spec) - } - f.Decls = append(f.Decls[:i], f.Decls[i+1:]...) - i-- - } - - return true -} - -// This function is copied from golang.org/x/tools/go/ast/astutil.isThirdParty -func isThirdParty(importPath string) bool { - // Third party package import path usually contains "." (".com", ".org", ...) - // This logic is taken from golang.org/x/tools/imports package. - return strings.Contains(importPath, ".") -} - -// importPath returns the unquoted import path of s, -// or "" if the path is not properly quoted. -// This function is copied from golang.org/x/tools/go/ast/astutil.importPath -func importPath(s *ast.ImportSpec) string { - t, err := strconv.Unquote(s.Path.Value) - if err != nil { - return "" - } - return t -} - -// declImports reports whether gen contains an import of path. -// This function is copied from golang.org/x/tools/go/ast/astutil.declImports -func declImports(gen *ast.GenDecl, path string) bool { - if gen.Tok != token.IMPORT { - return false - } - for _, spec := range gen.Specs { - impspec := spec.(*ast.ImportSpec) - if importPath(impspec) == path { - return true - } - } - return false -} - -// matchLen returns the length of the longest path segment prefix shared by x and y. -// This function is copied from golang.org/x/tools/go/ast/astutil.matchLen -func matchLen(x, y string) int { - n := 0 - for i := 0; i < len(x) && i < len(y) && x[i] == y[i]; i++ { - if x[i] == '/' { - n++ - } - } - return n -} diff --git a/tests/core/coverage/coverage_test.go b/tests/core/coverage/coverage_test.go index cd9b4ab455..12b5302696 100644 --- a/tests/core/coverage/coverage_test.go +++ b/tests/core/coverage/coverage_test.go @@ -75,6 +75,18 @@ go_test( name = "d_test", embed = [":d"], ) + +go_library( + name = "panicking", + srcs = ["panicking.go"], + importpath = "example.com/coverage/panicking", +) + +go_test( + name = "panicking_test", + srcs = ["panicking_test.go"], + embed = [":panicking"], +) -- a_test.go -- package a @@ -83,7 +95,6 @@ import "testing" func TestA(t *testing.T) { ALive() } - -- a.go -- package a @@ -128,6 +139,33 @@ package lzma // ntz32Const is used by the functions NTZ and NLZ. const ntz32Const = 0x04d7651f +-- panicking.go -- +package panicking + +func Panic() { + panic("from line 4") +} +-- panicking_test.go -- +package panicking + +import ( + "regexp" + "runtime/debug" + "testing" +) + +func TestPanic(t *testing.T) { + defer func() { + if err := recover(); err != nil { + got := regexp.MustCompile("panicking.go:[0-9]+"). + FindString(string(debug.Stack())) + if want := "panicking.go:4"; want != got { + t.Errorf("want %q; got %q", want, got) + } + } + }() + Panic() +} `, }) } @@ -170,3 +208,9 @@ func TestCoverageWithComments(t *testing.T) { t.Fatal(err) } } + +func TestCoverageWithCorrectLineNumbers(t *testing.T) { + if err := bazel_testing.RunBazel("coverage", ":panicking_test"); err != nil { + t.Fatal(err) + } +}