Fix instrumentation of types derived from bool

The previous attempt did not handle the boolean operators '&&', '||' and
'!' correctly. Converting back and forth between boolean types requires
type information for all expressions. Add this type information even
though it makes running gobco noticeably slower.

Fixes #35 properly.

instrumenter.go

@@ -5,9 +5,11 @@ import (
 	"fmt"
 	"go/ast"
 	"go/build"
+	"go/importer"
 	"go/parser"
 	"go/printer"
 	"go/token"
+	"go/types"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -38,7 +40,14 @@ type instrumenter struct {
 	coverTest   bool // also cover the test code
 	immediately bool // persist counts after each increment
 	listAll     bool // also list conditions that are covered
-	fset        *token.FileSet
+	debugTypes  bool
+
+	fset *token.FileSet
+	pkg  map[*ast.Package]*types.Package
+	typ  map[ast.Expr]types.Type
+
+	// While instrumenting of a file, the current package.
+	typePkg *types.Package
 
 	// Generates variable names that are unique per function.
 	varname int
@@ -83,6 +92,7 @@ func (i *instrumenter) instrument(srcDir, singleFile, dstDir string) bool {
 	// such as '//go:build 386' or '//go:embed'.
 	mode := parser.ParseComments
 	pkgsMap, err := parser.ParseDir(i.fset, srcDir, isRelevant, mode)
+	i.resolveTypes(pkgsMap)
 	ok(err)
 
 	pkgs := sortedPkgs(pkgsMap)
@@ -92,13 +102,52 @@ func (i *instrumenter) instrument(srcDir, singleFile, dstDir string) bool {
 
 	for _, pkg := range pkgs {
 		forEachFile(pkg, func(name string, file *ast.File) {
+			i.typePkg = i.pkg[pkg]
 			i.instrumentFile(name, file, dstDir)
 		})
 	}
 	i.writeGobcoFiles(dstDir, pkgs)
 	return true
 }
 
+func (i *instrumenter) resolveTypes(pkgsMap map[string]*ast.Package) {
+	imp := importer.ForCompiler(i.fset, "source", nil)
+	conf := types.Config{Importer: imp}
+	info := types.Info{
+		Types: make(map[ast.Expr]types.TypeAndValue),
+	}
+
+	rememberType := func(n ast.Node) bool {
+		expr, ok := n.(ast.Expr)
+		if !ok {
+			return true
+		}
+		tv, ok := info.Types[expr]
+		if !ok {
+			return true
+		}
+		i.typ[expr] = tv.Type
+		if i.debugTypes {
+			fmt.Printf("expression '%s' has type '%s'\n",
+				i.str(expr), tv.Type)
+		}
+		return true
+	}
+
+	for _, pkg := range pkgsMap {
+		var files []*ast.File
+		for _, file := range pkg.Files {
+			files = append(files, file)
+		}
+		typePkg, err := conf.Check(pkg.Name, i.fset, files, &info)
+		ok(err)
+		i.pkg[pkg] = typePkg
+		for _, f := range files {
+			ast.Inspect(f, rememberType)
+		}
+	}
+}
+
 func (i *instrumenter) instrumentFile(filename string, astFile *ast.File, dstDir string) {
 	isTest := strings.HasSuffix(filename, "_test.go")
 	if (i.coverTest || !isTest) && shouldBuild(filename) {
@@ -491,7 +540,7 @@ func (i *instrumenter) callCover(expr ast.Expr, pos token.Pos, code string) ast.
 	idx := len(i.conds) - 1
 
 	gen := codeGenerator{pos}
-	return gen.callGobcoCover(idx, expr)
+	return gen.callGobcoCover(idx, expr, i.typ[expr], i.typePkg)
 }
 
 // strEql returns the string representation of (lhs == rhs).
@@ -712,8 +761,12 @@ func (gen codeGenerator) callFinish(arg ast.Expr) ast.Expr {
 	}
 }
 
-func (gen codeGenerator) callGobcoCover(idx int, cond ast.Expr) ast.Expr {
-	return &ast.CallExpr{
+func (gen codeGenerator) callGobcoCover(idx int, cond ast.Expr, typ types.Type, typePkg *types.Package) ast.Expr {
+	convert := typ != nil && !types.Identical(typ, typ.Underlying())
+	if convert {
+		cond = gen.convert(cond, "bool")
+	}
+	var ret ast.Expr = &ast.CallExpr{
 		Fun:    gen.ident("GobcoCover"),
 		Lparen: gen.pos,
 		Args: []ast.Expr{
@@ -726,6 +779,18 @@ func (gen codeGenerator) callGobcoCover(idx int, cond ast.Expr) ast.Expr {
 		},
 		Rparen: gen.pos,
 	}
+	if convert {
+		typename := types.TypeString(typ, types.RelativeTo(typePkg))
+		ret = gen.convert(ret, typename)
+	}
+	return ret
+}
+
+func (gen codeGenerator) convert(x ast.Expr, t string) ast.Expr {
+	return &ast.CallExpr{
+		Fun:  gen.ident(t),
+		Args: []ast.Expr{x},
+	}
 }
 
 func (gen codeGenerator) define(lhs string, rhs ast.Expr) *ast.AssignStmt {

instrumenter_test.go

@@ -6,7 +6,10 @@ import (
 	"go/parser"
 	"go/printer"
 	"go/token"
+	"go/types"
+	"io/fs"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -56,13 +59,8 @@ func Test_instrumenter(t *testing.T) {
 	}
 
 	testInstrumenter := func(name string, branch bool, ext string) {
-		base := "testdata/instrumenter/" + name
-
-		goBytes, err := os.ReadFile(base + ".go")
-		if err != nil {
-			t.Fatal(err)
-		}
-		src := string(goBytes)
+		dir := "testdata/instrumenter"
+		base := dir + "/" + name
 
 		expectedBytes, err := os.ReadFile(base + ext)
 		if err != nil {
@@ -72,7 +70,12 @@ func Test_instrumenter(t *testing.T) {
 
 		fset := token.NewFileSet()
 		mode := parser.ParseComments
-		f, err := parser.ParseFile(fset, name+".go", src, mode)
+		relevant := func(info fs.FileInfo) bool {
+			n := info.Name()
+			return strings.HasPrefix(n, name) ||
+				strings.HasPrefix(n, "zzz")
+		}
+		pkgs, err := parser.ParseDir(fset, dir, relevant, mode)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -82,7 +85,11 @@ func Test_instrumenter(t *testing.T) {
 			false,
 			false,
 			false,
+			false,
 			fset,
+			map[*ast.Package]*types.Package{},
+			map[ast.Expr]types.Type{},
+			nil,
 			0,
 			map[ast.Expr]bool{},
 			map[ast.Expr]*exprSubst{},
@@ -91,6 +98,11 @@ func Test_instrumenter(t *testing.T) {
 			false,
 			nil,
 		}
+		fileName := filepath.Clean(base + ".go")
+		f := pkgs["instrumenter"].Files[fileName]
+		assert(f != nil, fileName)
+		i.resolveTypes(pkgs)
+		i.typePkg = i.pkg[pkgs["instrumenter"]]
 		i.instrumentFileNode(f)
 
 		var sb strings.Builder
@@ -102,7 +114,7 @@ func Test_instrumenter(t *testing.T) {
 			sb.WriteString("\n")
 		}
 		for _, cond := range i.conds {
-			location := strings.TrimPrefix(cond.pos, name+".go")
+			location := strings.TrimPrefix(cond.pos, fileName)
 			sb.WriteString(fmt.Sprintf("// %s: %q\n",
 				location, cond.text))
 		}
@@ -113,7 +125,7 @@ func Test_instrumenter(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			t.Errorf("expected:\n%s\nactual:\n%s\n", expected, actual)
+			t.Errorf("updated %s", base+ext)
 		}
 	}
 

main.go

@@ -6,6 +6,7 @@ import (
 	"flag"
 	"fmt"
 	"go/ast"
+	"go/types"
 	"io"
 	"os"
 	"os/exec"
@@ -254,6 +255,10 @@ func (g *gobco) instrument() bool {
 		g.coverTest,
 		g.immediately,
 		g.listAll,
+		false,
+		nil,
+		map[*ast.Package]*types.Package{},
+		map[ast.Expr]types.Type{},
 		nil,
 		0,
 		map[ast.Expr]bool{},

testdata/instrumenter/BinaryExpr.branch

@@ -77,7 +77,39 @@ func binaryExpr(i int, a bool, b bool, c bool) {
 	for GobcoCover(1, i == mi[i > 61]) {
 		_ = i == mi[i > 62]
 	}
+
+	type MyBool bool
+	var nativeTrue, nativeFalse = true, false
+	var myTrue, myFalse MyBool = true, false
+
+	if MyBool(GobcoCover(2, bool(myTrue && myFalse))) {
+	}
+	if MyBool(GobcoCover(3, bool(myFalse || myTrue))) {
+	}
+
+	{
+		gobco0 := myTrue && myFalse
+		switch {
+		case GobcoCover(4, gobco0 == myTrue):
+		case GobcoCover(5, gobco0 == myFalse):
+		}
+	}
+
+	{
+		gobco1 := nativeTrue && nativeFalse
+		switch {
+		case GobcoCover(6, gobco1 == nativeTrue):
+		case GobcoCover(7, gobco1 == nativeFalse):
+		}
+	}
+
 }
 
 // :74:5: "i == mi[i > 51]"
 // :77:6: "i == mi[i > 61]"
+// :85:5: "myTrue && myFalse"
+// :87:5: "myFalse || myTrue"
+// :91:7: "(myTrue && myFalse) == myTrue"
+// :92:7: "(myTrue && myFalse) == myFalse"
+// :96:7: "(nativeTrue && nativeFalse) == nativeTrue"
+// :97:7: "(nativeTrue && nativeFalse) == nativeFalse"

testdata/instrumenter/BinaryExpr.cond

@@ -77,6 +77,32 @@ func binaryExpr(i int, a bool, b bool, c bool) {
 	for GobcoCover(37, i == mi[GobcoCover(38, i > 61)]) {
 		_ = GobcoCover(39, i == mi[GobcoCover(40, i > 62)])
 	}
+
+	type MyBool bool
+	var nativeTrue, nativeFalse = true, false
+	var myTrue, myFalse MyBool = true, false
+
+	if MyBool(GobcoCover(41, bool(myTrue))) && MyBool(GobcoCover(42, bool(myFalse))) {
+	}
+	if MyBool(GobcoCover(43, bool(myFalse))) || MyBool(GobcoCover(44, bool(myTrue))) {
+	}
+
+	{
+		gobco0 := MyBool(GobcoCover(45, bool(myTrue))) && MyBool(GobcoCover(46, bool(myFalse)))
+		switch {
+		case GobcoCover(47, gobco0 == myTrue):
+		case GobcoCover(48, gobco0 == myFalse):
+		}
+	}
+
+	{
+		gobco1 := GobcoCover(49, nativeTrue) && GobcoCover(50, nativeFalse)
+		switch {
+		case GobcoCover(51, gobco1 == nativeTrue):
+		case GobcoCover(52, gobco1 == nativeFalse):
+		}
+	}
+
 }
 
 // :20:6: "i > 0"
@@ -120,3 +146,15 @@ func binaryExpr(i int, a bool, b bool, c bool) {
 // :77:14: "i > 61"
 // :78:7: "i == mi[i > 62]"
 // :78:15: "i > 62"
+// :85:5: "myTrue"
+// :85:15: "myFalse"
+// :87:5: "myFalse"
+// :87:16: "myTrue"
+// :90:9: "myTrue"
+// :90:19: "myFalse"
+// :91:7: "(myTrue && myFalse) == myTrue"
+// :92:7: "(myTrue && myFalse) == myFalse"
+// :95:9: "nativeTrue"
+// :95:23: "nativeFalse"
+// :96:7: "(nativeTrue && nativeFalse) == nativeTrue"
+// :97:7: "(nativeTrue && nativeFalse) == nativeFalse"

testdata/instrumenter/BinaryExpr.go

@@ -77,4 +77,23 @@ func binaryExpr(i int, a bool, b bool, c bool) {
 	for i == mi[i > 61] {
 		_ = i == mi[i > 62]
 	}
+
+	type MyBool bool
+	var nativeTrue, nativeFalse = true, false
+	var myTrue, myFalse MyBool = true, false
+
+	if myTrue && myFalse {
+	}
+	if myFalse || myTrue {
+	}
+
+	switch myTrue && myFalse {
+	case myTrue:
+	case myFalse:
+	}
+
+	switch nativeTrue && nativeFalse {
+	case nativeTrue:
+	case nativeFalse:
+	}
 }

testdata/instrumenter/CallExpr.branch

@@ -28,6 +28,14 @@ func callExpr(a bool, b string) bool {
 	// Type conversions end up as CallExpr as well.
 	type myBool bool
 	_ = myBool(3 > 0)
+	// The type of the expression '3 > 0' above is not an
+	// untyped bool but already myBool. No idea why.
+	//
+	// expression 'myBool(3 > 0)' has type 'instrumenter.myBool'
+	// expression 'myBool' has type 'instrumenter.myBool'
+	// expression '3 > 0' has type 'instrumenter.myBool'
+	// expression '3' has type 'untyped int'
+	// expression '0' has type 'untyped int'
 
 	return false
 }

testdata/instrumenter/CallExpr.cond

@@ -27,7 +27,15 @@ func callExpr(a bool, b string) bool {
 
 	// Type conversions end up as CallExpr as well.
 	type myBool bool
-	_ = myBool(GobcoCover(4, 3 > 0))
+	_ = myBool(myBool(GobcoCover(4, bool(3 > 0))))
+	// The type of the expression '3 > 0' above is not an
+	// untyped bool but already myBool. No idea why.
+	//
+	// expression 'myBool(3 > 0)' has type 'instrumenter.myBool'
+	// expression 'myBool' has type 'instrumenter.myBool'
+	// expression '3 > 0' has type 'instrumenter.myBool'
+	// expression '3' has type 'untyped int'
+	// expression '0' has type 'untyped int'
 
 	return false
 }

testdata/instrumenter/CallExpr.go

@@ -28,6 +28,14 @@ func callExpr(a bool, b string) bool {
 	// Type conversions end up as CallExpr as well.
 	type myBool bool
 	_ = myBool(3 > 0)
+	// The type of the expression '3 > 0' above is not an
+	// untyped bool but already myBool. No idea why.
+	//
+	// expression 'myBool(3 > 0)' has type 'instrumenter.myBool'
+	// expression 'myBool' has type 'instrumenter.myBool'
+	// expression '3 > 0' has type 'instrumenter.myBool'
+	// expression '3' has type 'untyped int'
+	// expression '0' has type 'untyped int'
 
 	return false
 }