From 62de3303aa82b4900bf61592e3d19de84f50de07 Mon Sep 17 00:00:00 2001 From: Bob Glickstein Date: Thu, 12 Sep 2024 20:24:19 -0700 Subject: [PATCH 1/3] Consider it a minor change, not major, when a method is added to an interface that includes any unexported methods. --- testdata/minor/addmethod.tmpl | 20 ++++++++++++++++++++ types.go | 15 ++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 testdata/minor/addmethod.tmpl diff --git a/testdata/minor/addmethod.tmpl b/testdata/minor/addmethod.tmpl new file mode 100644 index 0000000..1100508 --- /dev/null +++ b/testdata/minor/addmethod.tmpl @@ -0,0 +1,20 @@ +// -*- mode: go -*- + +// {{ define "older" }} +package addmethod + +type X interface { + A() int + unexported() +} +// {{ end }} + +// {{ define "newer" }} +package addmethod + +type X interface { + A() int + B() string + unexported() +} +// {{ end }} diff --git a/types.go b/types.go index 5ad29a9..81578b7 100644 --- a/types.go +++ b/types.go @@ -237,7 +237,11 @@ func (c *comparer) compareInterfaces(older, newer *types.Interface) Result { if c.implements(newer, older) { if !c.implements(older, newer) { - res = rwrapf(Major, "new interface %s is a superset of older", newer) + if anyUnexportedMethods(older) { + res = rwrapf(Minor, "new interface %s is a superset of older, with unexported methods", newer) + } else { + res = rwrapf(Major, "new interface %s is a superset of older", newer) + } } } else { return rwrapf(Major, "new interface %s does not implement old", newer) @@ -302,6 +306,15 @@ func (c *comparer) compareInterfaces(older, newer *types.Interface) Result { return rwrapf(Major, "constraint type unions differ") } +func anyUnexportedMethods(intf *types.Interface) bool { + for i := 0; i < intf.NumMethods(); i++ { + if !ast.IsExported(intf.Method(i).Name()) { + return true + } + } + return false +} + // This takes an interface and flattens its typelists by traversing embeds. func termsOf(typ types.Type) []*types.Term { var res []*types.Term From c85f9c463b2fb76c703e48882c0f00672afa6db5 Mon Sep 17 00:00:00 2001 From: Bob Glickstein Date: Thu, 12 Sep 2024 21:11:19 -0700 Subject: [PATCH 2/3] Adding methods to an interface that uses internal types is also minor. --- .../minor/{addmethod.tmpl => addmethod1.tmpl} | 4 +- types.go | 52 ++++++++++++++++++- 2 files changed, 52 insertions(+), 4 deletions(-) rename testdata/minor/{addmethod.tmpl => addmethod1.tmpl} (83%) diff --git a/testdata/minor/addmethod.tmpl b/testdata/minor/addmethod1.tmpl similarity index 83% rename from testdata/minor/addmethod.tmpl rename to testdata/minor/addmethod1.tmpl index 1100508..bf6607d 100644 --- a/testdata/minor/addmethod.tmpl +++ b/testdata/minor/addmethod1.tmpl @@ -1,7 +1,7 @@ // -*- mode: go -*- // {{ define "older" }} -package addmethod +package addmethod1 type X interface { A() int @@ -10,7 +10,7 @@ type X interface { // {{ end }} // {{ define "newer" }} -package addmethod +package addmethod1 type X interface { A() int diff --git a/types.go b/types.go index 81578b7..a6bc8ba 100644 --- a/types.go +++ b/types.go @@ -6,6 +6,7 @@ import ( "go/types" "reflect" "regexp" + "strings" "golang.org/x/tools/go/packages" ) @@ -237,9 +238,12 @@ func (c *comparer) compareInterfaces(older, newer *types.Interface) Result { if c.implements(newer, older) { if !c.implements(older, newer) { - if anyUnexportedMethods(older) { + switch { + case anyUnexportedMethods(older): res = rwrapf(Minor, "new interface %s is a superset of older, with unexported methods", newer) - } else { + case anyInternalTypes(older): + res = rwrapf(Minor, "new interface %s is a superset of older, using internal types", newer) + default: res = rwrapf(Major, "new interface %s is a superset of older", newer) } } @@ -315,6 +319,50 @@ func anyUnexportedMethods(intf *types.Interface) bool { return false } +// Do any of the types in the method args or results have "internal" in their pkgpaths? +func anyInternalTypes(intf *types.Interface) bool { + for i := 0; i < intf.NumMethods(); i++ { + sig, ok := intf.Method(i).Type().(*types.Signature) + if !ok { + // Should be impossible. + continue + } + if anyInternalTypesInTuple(sig.Params()) || anyInternalTypesInTuple(sig.Results()) { + return true + } + if recv := sig.Recv(); recv != nil && isInternalType(recv.Type()) { + return true + } + } + return false +} + +func anyInternalTypesInTuple(tup *types.Tuple) bool { + for i := 0; i < tup.Len(); i++ { + if isInternalType(tup.At(i).Type()) { + return true + } + } + return false +} + +func isInternalType(typ types.Type) bool { + s := types.TypeString(typ, nil) + if strings.HasPrefix(s, "internal.") { + return true + } + if strings.Contains(s, "/internal.") { + return true + } + if strings.Contains(s, "/internal/") { + return true + } + if strings.HasPrefix(s, "main.") { + return true + } + return strings.Contains(s, "/main.") +} + // This takes an interface and flattens its typelists by traversing embeds. func termsOf(typ types.Type) []*types.Term { var res []*types.Term From 22f92a066079482d766c018ebfb49b708a16af90 Mon Sep 17 00:00:00 2001 From: Bob Glickstein Date: Thu, 12 Sep 2024 21:37:00 -0700 Subject: [PATCH 3/3] Oops, add missing testdata file. --- testdata/minor/addmethod2.tmpl | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 testdata/minor/addmethod2.tmpl diff --git a/testdata/minor/addmethod2.tmpl b/testdata/minor/addmethod2.tmpl new file mode 100644 index 0000000..9458e4e --- /dev/null +++ b/testdata/minor/addmethod2.tmpl @@ -0,0 +1,34 @@ +// -*- mode: go -*- + +// {{ define "older/internal/q.go" }} +package internal + +type Q int +// {{ end }} + +// {{ define "newer/internal/q.go" }} +package internal + +type Q int +// {{ end }} + +// {{ define "older" }} +package addmethod2 + +import "addmethod2/internal" + +type X interface { + A() internal.Q +} +// {{ end }} + +// {{ define "newer" }} +package addmethod2 + +import "addmethod2/internal" + +type X interface { + A() internal.Q + B() string +} +// {{ end }}