go/internal/gcimporter: simplify IExportData API

In gcexportdata.Write, we ultimately write output to an io.Writer, so
we might as well do the same in IExportData. Also, omit the "i" prefix
here (to match IImportData's expectations), and add it back in
gcexportdata.Write instead.

Change-Id: I42dc577477587cd4ad6e943822dcea90e01bc236
Reviewed-on: https://go-review.googlesource.com/c/tools/+/292349
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
Trust: Matthew Dempsky <[email protected]>
Run-TryBot: Matthew Dempsky <[email protected]>

Author: mdempsky

go/gcexportdata/gcexportdata.go

@@ -100,10 +100,8 @@ func Read(in io.Reader, fset *token.FileSet, imports map[string]*types.Package,
 // Write writes encoded type information for the specified package to out.
 // The FileSet provides file position information for named objects.
 func Write(out io.Writer, fset *token.FileSet, pkg *types.Package) error {
-	b, err := gcimporter.IExportData(fset, pkg)
-	if err != nil {
+	if _, err := io.WriteString(out, "i"); err != nil {
 		return err
 	}
-	_, err = out.Write(b)
-	return err
+	return gcimporter.IExportData(out, fset, pkg)
 }

go/internal/gcimporter/iexport.go

@@ -25,12 +25,12 @@ import (
 // 0: Go1.11 encoding
 const iexportVersion = 0
 
-// IExportData returns the binary export data for pkg.
+// IExportData writes indexed export data for pkg to out.
 //
 // If no file set is provided, position info will be missing.
 // The package path of the top-level package will not be recorded,
 // so that calls to IImportData can override with a provided package path.
-func IExportData(fset *token.FileSet, pkg *types.Package) (b []byte, err error) {
+func IExportData(out io.Writer, fset *token.FileSet, pkg *types.Package) (err error) {
 	defer func() {
 		if e := recover(); e != nil {
 			if ierr, ok := e.(internalError); ok {
@@ -43,7 +43,6 @@ func IExportData(fset *token.FileSet, pkg *types.Package) (b []byte, err error)
 	}()
 
 	p := iexporter{
-		out:         bytes.NewBuffer(nil),
 		fset:        fset,
 		allPkgs:     map[*types.Package]bool{},
 		stringIndex: map[string]uint64{},
@@ -80,17 +79,16 @@ func IExportData(fset *token.FileSet, pkg *types.Package) (b []byte, err error)
 
 	// Assemble header.
 	var hdr intWriter
-	hdr.WriteByte('i')
 	hdr.uint64(iexportVersion)
 	hdr.uint64(uint64(p.strings.Len()))
 	hdr.uint64(dataLen)
 
 	// Flush output.
-	io.Copy(p.out, &hdr)
-	io.Copy(p.out, &p.strings)
-	io.Copy(p.out, &p.data0)
+	io.Copy(out, &hdr)
+	io.Copy(out, &p.strings)
+	io.Copy(out, &p.data0)
 
-	return p.out.Bytes(), nil
+	return nil
 }
 
 // writeIndex writes out an object index. mainIndex indicates whether

go/internal/gcimporter/iexport_test.go

@@ -9,6 +9,7 @@
 package gcimporter_test
 
 import (
+	"bytes"
 	"fmt"
 	"go/ast"
 	"go/build"
@@ -28,6 +29,14 @@ import (
 	"golang.org/x/tools/go/loader"
 )
 
+func iexport(fset *token.FileSet, pkg *types.Package) ([]byte, error) {
+	var buf bytes.Buffer
+	if err := gcimporter.IExportData(&buf, fset, pkg); err != nil {
+		return nil, err
+	}
+	return buf.Bytes(), nil
+}
+
 func TestIExportData_stdlib(t *testing.T) {
 	if runtime.Compiler == "gccgo" {
 		t.Skip("gccgo standard library is inaccessible")
@@ -84,15 +93,10 @@ type UnknownType undefined
 		if info.Files == nil {
 			continue // empty directory
 		}
-		exportdata, err := gcimporter.IExportData(conf.Fset, pkg)
+		exportdata, err := iexport(conf.Fset, pkg)
 		if err != nil {
 			t.Fatal(err)
 		}
-		if exportdata[0] == 'i' {
-			exportdata = exportdata[1:] // trim the 'i' in the header
-		} else {
-			t.Fatalf("unexpected first character of export data: %v", exportdata[0])
-		}
 
 		imports := make(map[string]*types.Package)
 		fset2 := token.NewFileSet()
@@ -151,15 +155,10 @@ func TestIExportData_long(t *testing.T) {
 	}
 
 	// export
-	exportdata, err := gcimporter.IExportData(fset1, pkg)
+	exportdata, err := iexport(fset1, pkg)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if exportdata[0] == 'i' {
-		exportdata = exportdata[1:] // trim the 'i' in the header
-	} else {
-		t.Fatalf("unexpected first character of export data: %v", exportdata[0])
-	}
 
 	// import
 	imports := make(map[string]*types.Package)
@@ -199,15 +198,10 @@ func TestIExportData_typealiases(t *testing.T) {
 
 	// export
 	// use a nil fileset here to confirm that it doesn't panic
-	exportdata, err := gcimporter.IExportData(nil, pkg1)
+	exportdata, err := iexport(nil, pkg1)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if exportdata[0] == 'i' {
-		exportdata = exportdata[1:] // trim the 'i' in the header
-	} else {
-		t.Fatalf("unexpected first character of export data: %v", exportdata[0])
-	}
 
 	// import
 	imports := make(map[string]*types.Package)