From a02ecff057daddfe66ae4f9e8764e2f315718ef3 Mon Sep 17 00:00:00 2001
From: Paul Jolly <paul@myitcv.io>
Date: Thu, 14 Feb 2019 15:49:37 +0000
Subject: [PATCH] immutable/cmd/immutableGen: fix go/packages res and
 type-based res

We need to fight go/packages a bit to find the right package to use for
type information (see https://github.com/golang/go/issues/27910 for more
details).

Also, our logic for surfacing embedded fields where the embedded type
had a valid go/types.Type was incorrect. It assumed that all such types
must be external to the package being generated, and hence required that
all fields be exported. However, it is possible for a package-local type
to fully type check and therefore be valid. In this case, non-exported
fields _are_ valid for surfacing.

Also a small fix in cmd/gg whereby xtest packages were incorrectly being
considered as not Done() because they were missing a hash (which they
will never have). Added a failsafe that errors in case any work was left
un-Done
---
 immutable/cmd/immutableGen/generator.go       |  51 +++-
 .../immutableGen/internal/coretest/core.go    |   5 +-
 .../internal/coretest/core_test.go            |  17 ++
 .../coretest/gen_core_immutableGen.go         |  29 ++-
 .../coretest/gen_core_immutableGen_test.go    | 239 ++++++++++++++++++
 .../internal/coretest/general_test.go         |   8 +-
 immutable/cmd/immutableGen/method_sets.go     |  26 +-
 7 files changed, 364 insertions(+), 11 deletions(-)
 create mode 100644 immutable/cmd/immutableGen/internal/coretest/core_test.go
 create mode 100644 immutable/cmd/immutableGen/internal/coretest/gen_core_immutableGen_test.go

diff --git a/immutable/cmd/immutableGen/generator.go b/immutable/cmd/immutableGen/generator.go
index eaa49d55..cfb91384 100644
--- a/immutable/cmd/immutableGen/generator.go
+++ b/immutable/cmd/immutableGen/generator.go
@@ -9,6 +9,9 @@ import (
 	"go/token"
 	"go/types"
 	"path/filepath"
+	"regexp"
+	"sort"
+	"strings"
 
 	"golang.org/x/tools/go/packages"
 	"myitcv.io/gogenerate"
@@ -29,10 +32,9 @@ func execute(dir string, envPkg string, licenseHeader string, cmds gogenCmds) {
 	}
 
 	config := &packages.Config{
-		Dir:   absDir,
 		Mode:  packages.LoadSyntax,
 		Fset:  token.NewFileSet(),
-		Tests: false,
+		Tests: true,
 	}
 
 	pkgs, err := packages.Load(config, absDir)
@@ -40,7 +42,49 @@ func execute(dir string, envPkg string, licenseHeader string, cmds gogenCmds) {
 		fatalf("could not load packages from dir %v: %v", dir, err)
 	}
 
-	pkg := pkgs[0]
+	forTest := regexp.MustCompile(` \[[^\]]+\]$`)
+
+	testPkgs := make(map[string]*packages.Package)
+	var nonTestPkg *packages.Package
+
+	// Becase of https://github.com/golang/go/issues/27910 we have to
+	// apply some janky logic to find the "right" package
+	for _, p := range pkgs {
+		switch {
+		case strings.HasSuffix(p.PkgPath, ".test"):
+			// we don't ever want this package
+			continue
+		case forTest.MatchString(p.ID):
+			testPkgs[p.Name] = p
+		default:
+			nonTestPkg = p
+		}
+	}
+
+	ids := func() []string {
+		var ids []string
+		for _, p := range pkgs {
+			ids = append(ids, p.ID)
+		}
+		sort.Strings(ids)
+		return ids
+	}
+
+	if nonTestPkg == nil {
+		fatalf("always expect to have the actual package. Got %v", ids())
+	}
+
+	var pkg *packages.Package
+
+	if strings.HasSuffix(envPkg, "_test") {
+		if pkg = testPkgs[envPkg]; pkg == nil {
+			fatalf("called with package name %v, but go/packages did not give us such a package. Got %v", envPkg, ids())
+		}
+	} else {
+		if pkg = testPkgs[envPkg]; pkg == nil {
+			pkg = nonTestPkg
+		}
+	}
 
 	if pkg.Name != envPkg {
 		pps := make([]string, 0, len(pkgs))
@@ -135,6 +179,7 @@ type embedded struct {
 	typ  types.Type
 	es   string
 	path []string
+	info *types.Info
 }
 
 type field struct {
diff --git a/immutable/cmd/immutableGen/internal/coretest/core.go b/immutable/cmd/immutableGen/internal/coretest/core.go
index bb54e533..6d45fdf5 100644
--- a/immutable/cmd/immutableGen/internal/coretest/core.go
+++ b/immutable/cmd/immutableGen/internal/coretest/core.go
@@ -7,7 +7,7 @@ import (
 	"myitcv.io/immutable/cmd/immutableGen/internal/coretest/pkga"
 )
 
-//go:generate immutableGen -licenseFile license.txt -G "echo \"hello world\""
+//go:generate gobin -m -run myitcv.io/immutable/cmd/immutableGen -licenseFile license.txt -G "echo \"hello world\""
 
 // a comment about MyMap
 type _Imm_MyMap map[string]int
@@ -101,7 +101,8 @@ type _Imm_Embed1 struct {
 }
 
 type _Imm_Embed2 struct {
-	Age int
+	Age          int
+	otherdetails string
 }
 
 type NonImmStruct struct {
diff --git a/immutable/cmd/immutableGen/internal/coretest/core_test.go b/immutable/cmd/immutableGen/internal/coretest/core_test.go
new file mode 100644
index 00000000..909b155a
--- /dev/null
+++ b/immutable/cmd/immutableGen/internal/coretest/core_test.go
@@ -0,0 +1,17 @@
+package coretest_test
+
+import (
+	_ "myitcv.io/immutable/cmd/immutableGen/internal/coretest"
+)
+
+//go:generate gobin -m -run myitcv.io/immutable/cmd/immutableGen -licenseFile license.txt -G "echo \"hello world\""
+
+type _Imm_xtestA struct {
+	*XTestB
+
+	age int
+}
+
+type _Imm_XTestB struct {
+	Name string
+}
diff --git a/immutable/cmd/immutableGen/internal/coretest/gen_core_immutableGen.go b/immutable/cmd/immutableGen/internal/coretest/gen_core_immutableGen.go
index f99f908f..a35d5530 100644
--- a/immutable/cmd/immutableGen/internal/coretest/gen_core_immutableGen.go
+++ b/immutable/cmd/immutableGen/internal/coretest/gen_core_immutableGen.go
@@ -1568,16 +1568,26 @@ func (s *Embed1) SetPostcode(n string) *Embed1 {
 	v0 := s.SetPkgA(v1)
 	return v0
 }
+func (s *Embed1) otherdetails() string {
+	return s.Embed2().otherdetails()
+}
+func (s *Embed1) setOtherdetails(n string) *Embed1 {
+	v1 := s.Embed2().setOtherdetails(n)
+	v0 := s.SetEmbed2(v1)
+	return v0
+}
 
 //
 // Embed2 is an immutable type and has the following template:
 //
 // 	struct {
-// 		Age int
+// 		Age		int
+// 		otherdetails	string
 // 	}
 //
 type Embed2 struct {
-	field_Age int
+	field_Age          int
+	field_otherdetails string
 
 	mutable bool
 	__tmpl  *_Imm_Embed2
@@ -1665,6 +1675,21 @@ func (s *Embed2) SetAge(n int) *Embed2 {
 	res.field_Age = n
 	return &res
 }
+func (s *Embed2) otherdetails() string {
+	return s.field_otherdetails
+}
+
+// setOtherdetails is the setter for Otherdetails()
+func (s *Embed2) setOtherdetails(n string) *Embed2 {
+	if s.mutable {
+		s.field_otherdetails = n
+		return s
+	}
+
+	res := *s
+	res.field_otherdetails = n
+	return &res
+}
 
 //
 // Other is an immutable type and has the following template:
diff --git a/immutable/cmd/immutableGen/internal/coretest/gen_core_immutableGen_test.go b/immutable/cmd/immutableGen/internal/coretest/gen_core_immutableGen_test.go
new file mode 100644
index 00000000..4e45de91
--- /dev/null
+++ b/immutable/cmd/immutableGen/internal/coretest/gen_core_immutableGen_test.go
@@ -0,0 +1,239 @@
+// Code generated by immutableGen. DO NOT EDIT.
+
+// My favourite license
+
+package coretest_test
+
+//go:generate echo "hello world"
+//immutableVet:skipFile
+
+import (
+	"myitcv.io/immutable"
+)
+
+//
+// xtestA is an immutable type and has the following template:
+//
+// 	struct {
+// 		*XTestB
+//
+// 		age	int
+// 	}
+//
+type xtestA struct {
+	anonfield_XTestB *XTestB
+	field_age        int
+
+	mutable bool
+	__tmpl  *_Imm_xtestA
+}
+
+var _ immutable.Immutable = new(xtestA)
+var _ = new(xtestA).__tmpl
+
+func (s *xtestA) AsMutable() *xtestA {
+	if s.Mutable() {
+		return s
+	}
+
+	res := *s
+	res.mutable = true
+	return &res
+}
+
+func (s *xtestA) AsImmutable(v *xtestA) *xtestA {
+	if s == nil {
+		return nil
+	}
+
+	if s == v {
+		return s
+	}
+
+	s.mutable = false
+	return s
+}
+
+func (s *xtestA) Mutable() bool {
+	return s.mutable
+}
+
+func (s *xtestA) WithMutable(f func(si *xtestA)) *xtestA {
+	res := s.AsMutable()
+	f(res)
+	res = res.AsImmutable(s)
+
+	return res
+}
+
+func (s *xtestA) WithImmutable(f func(si *xtestA)) *xtestA {
+	prev := s.mutable
+	s.mutable = false
+	f(s)
+	s.mutable = prev
+
+	return s
+}
+
+func (s *xtestA) IsDeeplyNonMutable(seen map[interface{}]bool) bool {
+	if s == nil {
+		return true
+	}
+
+	if s.Mutable() {
+		return false
+	}
+
+	if seen == nil {
+		return s.IsDeeplyNonMutable(make(map[interface{}]bool))
+	}
+
+	if seen[s] {
+		return true
+	}
+
+	seen[s] = true
+	{
+		v := s.anonfield_XTestB
+
+		if v != nil && !v.IsDeeplyNonMutable(seen) {
+			return false
+		}
+	}
+	return true
+}
+func (s *xtestA) Name() string {
+	return s.XTestB().Name()
+}
+func (s *xtestA) SetName(n string) *xtestA {
+	v1 := s.XTestB().SetName(n)
+	v0 := s.SetXTestB(v1)
+	return v0
+}
+func (s *xtestA) XTestB() *XTestB {
+	return s.anonfield_XTestB
+}
+
+// SetXTestB is the setter for XTestB()
+func (s *xtestA) SetXTestB(n *XTestB) *xtestA {
+	if s.mutable {
+		s.anonfield_XTestB = n
+		return s
+	}
+
+	res := *s
+	res.anonfield_XTestB = n
+	return &res
+}
+func (s *xtestA) age() int {
+	return s.field_age
+}
+
+// setAge is the setter for Age()
+func (s *xtestA) setAge(n int) *xtestA {
+	if s.mutable {
+		s.field_age = n
+		return s
+	}
+
+	res := *s
+	res.field_age = n
+	return &res
+}
+
+//
+// XTestB is an immutable type and has the following template:
+//
+// 	struct {
+// 		Name string
+// 	}
+//
+type XTestB struct {
+	field_Name string
+
+	mutable bool
+	__tmpl  *_Imm_XTestB
+}
+
+var _ immutable.Immutable = new(XTestB)
+var _ = new(XTestB).__tmpl
+
+func (s *XTestB) AsMutable() *XTestB {
+	if s.Mutable() {
+		return s
+	}
+
+	res := *s
+	res.mutable = true
+	return &res
+}
+
+func (s *XTestB) AsImmutable(v *XTestB) *XTestB {
+	if s == nil {
+		return nil
+	}
+
+	if s == v {
+		return s
+	}
+
+	s.mutable = false
+	return s
+}
+
+func (s *XTestB) Mutable() bool {
+	return s.mutable
+}
+
+func (s *XTestB) WithMutable(f func(si *XTestB)) *XTestB {
+	res := s.AsMutable()
+	f(res)
+	res = res.AsImmutable(s)
+
+	return res
+}
+
+func (s *XTestB) WithImmutable(f func(si *XTestB)) *XTestB {
+	prev := s.mutable
+	s.mutable = false
+	f(s)
+	s.mutable = prev
+
+	return s
+}
+
+func (s *XTestB) IsDeeplyNonMutable(seen map[interface{}]bool) bool {
+	if s == nil {
+		return true
+	}
+
+	if s.Mutable() {
+		return false
+	}
+
+	if seen == nil {
+		return s.IsDeeplyNonMutable(make(map[interface{}]bool))
+	}
+
+	if seen[s] {
+		return true
+	}
+
+	seen[s] = true
+	return true
+}
+func (s *XTestB) Name() string {
+	return s.field_Name
+}
+
+// SetName is the setter for Name()
+func (s *XTestB) SetName(n string) *XTestB {
+	if s.mutable {
+		s.field_Name = n
+		return s
+	}
+
+	res := *s
+	res.field_Name = n
+	return &res
+}
diff --git a/immutable/cmd/immutableGen/internal/coretest/general_test.go b/immutable/cmd/immutableGen/internal/coretest/general_test.go
index 25259277..ab8849a7 100644
--- a/immutable/cmd/immutableGen/internal/coretest/general_test.go
+++ b/immutable/cmd/immutableGen/internal/coretest/general_test.go
@@ -32,7 +32,7 @@ func TestEmbedAccess(t *testing.T) {
 	c1 := new(Clash1).SetNoClash1("NoClash1")
 	c2 := new(pkga.Clash2).SetNoClash2("NoClash2")
 	a := new(pkga.PkgA).SetAddress("home").SetPkgB(b)
-	e2 := new(Embed2).SetAge(42)
+	e2 := new(Embed2).SetAge(42).setOtherdetails("otherdetails")
 	e1 := new(Embed1).WithMutable(func(e1 *Embed1) {
 		e1.SetName("Paul")
 		e1.SetEmbed2(e2)
@@ -54,6 +54,12 @@ func TestEmbedAccess(t *testing.T) {
 			t.Fatalf("e1.Age(): want %v, got %v", want, got)
 		}
 	}
+	{
+		want := "otherdetails"
+		if got := e1.otherdetails(); want != got {
+			t.Fatalf("e1.otherdetails(): want %v, got %v", want, got)
+		}
+	}
 	{
 		want := "home"
 		if got := e1.Address(); want != got {
diff --git a/immutable/cmd/immutableGen/method_sets.go b/immutable/cmd/immutableGen/method_sets.go
index 1b10cc53..1158552e 100644
--- a/immutable/cmd/immutableGen/method_sets.go
+++ b/immutable/cmd/immutableGen/method_sets.go
@@ -148,6 +148,7 @@ func (o *output) calcMethodSets() {
 					}
 					seen[kt] = true
 					debugf("using type check on %T %v\n", h.typ, h.typ)
+					isPkgLocal := o.isPkgLocal(h.typ)
 					if v, ok := util.IsImmType(h.typ).(util.ImmTypeStruct); ok {
 						is := v.Struct
 						for i := 0; i < is.NumFields(); i++ {
@@ -162,8 +163,9 @@ func (o *output) calcMethodSets() {
 								continue
 							}
 							name = strings.TrimPrefix(name, "field_")
-							// we can only consider exported fields
-							if r, _ := utf8.DecodeRuneInString(name); unicode.IsLower(r) {
+							// we can only consider exported fields where the
+							// named type (if it is such) belongs to another package
+							if r, _ := utf8.DecodeRuneInString(name); !isPkgLocal && unicode.IsLower(r) {
 								continue
 							}
 							typStr := varTypeString(f)
@@ -183,7 +185,7 @@ func (o *output) calcMethodSets() {
 					} else if v, ok := h.typ.Underlying().(*types.Struct); ok {
 						for i := 0; i < v.NumFields(); i++ {
 							f := v.Field(i)
-							if !f.Exported() {
+							if !isPkgLocal && !f.Exported() {
 								continue
 							}
 							typStr := varTypeString(f)
@@ -231,6 +233,24 @@ func (o *output) calcMethodSets() {
 	}
 }
 
+func (o *output) isPkgLocal(t types.Type) bool {
+	// TODO this does not support **T types
+	ct := t
+	for {
+		if pt, ok := ct.(*types.Pointer); !ok {
+			break
+		} else {
+			ct = pt.Elem()
+		}
+	}
+	if nt, ok := ct.(*types.Named); ok {
+		return o.pkgPath == nt.Obj().Pkg().Path()
+	}
+
+	// TODO in what circumstances is it wrong to return true?
+	return true
+}
+
 func (o *output) findFieldFromVar(v *types.Var) (field *ast.Field, err error) {
 	defer func() {
 		if v := recover(); v != nil {