浏览代码

codec: only generate Selfer methods if AST doesn't have those methods already

This is a cleaner solution, that checking using reflection if
codec.Selfer is implemented.

It solves the issue where a type implements codec.Selfer implicitly (by
embedding).  In this situation, we still want to generate Selfer
implementations for the enclosing type.

We can't do this with reflection effectively, because golang reflection
has the limitation that you cannot (at runtime) determine whether a Type
explicitly/directly (itself) implements an interface or whether it
implements that interface implicitly (via embedding).

By design (of codecgen)
- first (initial) phase of codecgen is the only place where we
  inspect the AST to infer the types to codecgen on.
- an execution of codecgen only works on files within a
  specific directory (within a single package).
  It makes the problem easier to handle and solve.

However, this implementation has some caveats:
- only generate types if the type doesn't explicitly implement Selfer methods
  in one of the files passed to that execution of codecgen

we can check this while doing the AST analysis for type names to generate
in this mode, we don't check anymore at the top of Gen(...) to see if the reflect.Type implements codec.Selfer

This will handle the following cases:

- file1.go: defines type T, and CodecEncodeSelfer and CodecDecodeSelfer
  run codecgen on file1.go
- file1.go: defines type T
  file2.go: defines CodecEncodeSelfer and CodecDecodeSelfer for type T
  run codecgen on file1.go AND file2.go (in one execution)

However, it will NOT handle the following cases:
- file1.go: defines type T
  file2.go: defines CodecEncodeSelfer and CodecDecodeSelfer for type T
  run codecgen on file1.go (without passing in file2.go also)

In practice, folks can easily work around this limitation.

Fixes #185 #186
Ugorji Nwoke 9 年之前
父节点
当前提交
ded73eae5d
共有 2 个文件被更改,包括 36 次插入10 次删除
  1. 33 1
      codec/codecgen/gen.go
  2. 3 9
      codec/gen.go

+ 33 - 1
codec/codecgen/gen.go

@@ -171,6 +171,31 @@ func Generate(outfile, buildTag, codecPkgPath string, uid int64, useUnsafe bool,
 		}
 	}
 
+	// keep track of types with selfer methods
+	// selferMethods := []string{"CodecEncodeSelf", "CodecDecodeSelf"}
+	selferEncTyps := make(map[string]bool)
+	selferDecTyps := make(map[string]bool)
+	for _, f := range astfiles {
+		for _, d := range f.Decls {
+			// if fd, ok := d.(*ast.FuncDecl); ok && fd.Recv != nil && fd.Recv.NumFields() == 1 {
+			if fd, ok := d.(*ast.FuncDecl); ok && fd.Recv != nil && len(fd.Recv.List) == 1 {
+				recvType := fd.Recv.List[0].Type
+				if ptr, ok := recvType.(*ast.StarExpr); ok {
+					recvType = ptr.X
+				}
+				if id, ok := recvType.(*ast.Ident); ok {
+					switch fd.Name.Name {
+					case "CodecEncodeSelf":
+						selferEncTyps[id.Name] = true
+					case "CodecDecodeSelf":
+						selferDecTyps[id.Name] = true
+					}
+				}
+			}
+		}
+	}
+
+	// now find types
 	for _, f := range astfiles {
 		for _, d := range f.Decls {
 			if gd, ok := d.(*ast.GenDecl); ok {
@@ -191,7 +216,14 @@ func Generate(outfile, buildTag, codecPkgPath string, uid int64, useUnsafe bool,
 						//   FuncType, InterfaceType, StarExpr (ptr), etc
 						switch td.Type.(type) {
 						case *ast.StructType, *ast.Ident, *ast.MapType, *ast.ArrayType, *ast.ChanType:
-							if regexName.FindStringIndex(td.Name.Name) != nil && notRegexName.FindStringIndex(td.Name.Name) == nil {
+							// only add to tv.Types iff
+							//   - it matches per the -r parameter
+							//   - it doesn't match per the -nr parameter
+							//   - it doesn't have any of the Selfer methods in the file
+							if regexName.FindStringIndex(td.Name.Name) != nil &&
+								notRegexName.FindStringIndex(td.Name.Name) == nil &&
+								!selferEncTyps[td.Name.Name] &&
+								!selferDecTyps[td.Name.Name] {
 								tv.Types = append(tv.Types, td.Name.Name)
 							}
 						}

+ 3 - 9
codec/gen.go

@@ -165,15 +165,9 @@ type genRunner struct {
 //
 // Library users: *DO NOT USE IT DIRECTLY. IT WILL CHANGE CONTINOUSLY WITHOUT NOTICE.*
 func Gen(w io.Writer, buildTags, pkgName, uid string, useUnsafe bool, ti *TypeInfos, typ ...reflect.Type) {
-	// trim out all types which already implement Selfer
-	typ2 := make([]reflect.Type, 0, len(typ))
-	for _, t := range typ {
-		if reflect.PtrTo(t).Implements(selferTyp) || t.Implements(selferTyp) {
-			continue
-		}
-		typ2 = append(typ2, t)
-	}
-	typ = typ2
+	// All types passed to this method do not have a codec.Selfer method implemented directly.
+	// codecgen already checks the AST and skips any types that define the codec.Selfer methods.
+	// Consequently, there's no need to check and trim them if they implement codec.Selfer
 
 	if len(typ) == 0 {
 		return