bind,cmd/gomobile: fix follow ups to doc generator CL (52330)

Also, add tests for blank, anonymous and multiple struct fields.

Change-Id: I15e6fff8d1684f2a31e99e1adf023b92f1f3cb48
Reviewed-on: https://go-review.googlesource.com/59550
Reviewed-by: Alan Donovan <adonovan@google.com>

bind/bind_test.go

@@ -74,7 +74,7 @@ func fileRefs(t *testing.T, filename string, pkgPrefix string) *importers.Refere
 	return refs
 }
 
-func typeCheck(t *testing.T, filename string, gopath string) (*types.Package, *ast.Package) {
+func typeCheck(t *testing.T, filename string, gopath string) (*types.Package, *ast.File) {
 	f, err := parser.ParseFile(fset, filename, nil, parser.AllErrors|parser.ParseComments)
 	if err != nil {
 		t.Fatalf("%s: %v", filename, err)
@@ -98,9 +98,7 @@ func typeCheck(t *testing.T, filename string, gopath string) (*types.Package, *a
 	if err != nil {
 		t.Fatal(err)
 	}
-	// Ignore errors (probably from unknown imports): we only need the package documentation.
-	astPkg, _ := ast.NewPackage(fset, map[string]*ast.File{filename: f}, nil, nil)
-	return pkg, astPkg
+	return pkg, f
 }
 
 // diff runs the command "diff a b" and returns its output
@@ -136,9 +134,9 @@ func writeTempFile(t *testing.T, name string, contents []byte) string {
 func TestGenObjc(t *testing.T) {
 	for _, filename := range tests {
 		var pkg *types.Package
-		var astPkg *ast.Package
+		var file *ast.File
 		if filename != "" {
-			pkg, astPkg = typeCheck(t, filename, "")
+			pkg, file = typeCheck(t, filename, "")
 		}
 
 		var buf bytes.Buffer
@@ -146,7 +144,7 @@ func TestGenObjc(t *testing.T) {
 			Generator: &Generator{
 				Printer: &Printer{Buf: &buf, IndentEach: []byte("\t")},
 				Fset:    fset,
-				AST:     astPkg,
+				Files:   []*ast.File{file},
 				Pkg:     pkg,
 			},
 		}
@@ -285,7 +283,7 @@ func TestGenJava(t *testing.T) {
 	}
 	for _, filename := range allTests {
 		var pkg *types.Package
-		var astPkg *ast.Package
+		var file *ast.File
 		var buf bytes.Buffer
 		var cg *ClassGen
 		var classes []*java.Class
@@ -314,13 +312,13 @@ func TestGenJava(t *testing.T) {
 				genJavaPackages(t, tmpGopath, cg)
 				cg.Buf = &buf
 			}
-			pkg, astPkg = typeCheck(t, filename, tmpGopath)
+			pkg, file = typeCheck(t, filename, tmpGopath)
 		}
 		g := &JavaGen{
 			Generator: &Generator{
 				Printer: &Printer{Buf: &buf, IndentEach: []byte("    ")},
 				Fset:    fset,
-				AST:     astPkg,
+				Files:   []*ast.File{file},
 				Pkg:     pkg,
 			},
 		}
@@ -503,7 +501,7 @@ func testGenGo(t *testing.T, filename string, buf *bytes.Buffer, pkg *types.Pack
 
 func TestCustomPrefix(t *testing.T) {
 	const datafile = "testdata/customprefix.go"
-	pkg, astPkg := typeCheck(t, datafile, "")
+	pkg, file := typeCheck(t, datafile, "")
 
 	type testCase struct {
 		golden string
@@ -516,7 +514,7 @@ func TestCustomPrefix(t *testing.T) {
 			Printer: &Printer{Buf: &buf, IndentEach: []byte("    ")},
 			Fset:    fset,
 			AllPkg:  []*types.Package{pkg},
-			AST:     astPkg,
+			Files:   []*ast.File{file},
 			Pkg:     pkg,
 		},
 	}
@@ -649,3 +647,15 @@ func TestLowerFirst(t *testing.T) {
 		}
 	}
 }
+
+// Test that typeName work for anonymous qualified fields.
+func TestSelectorExprTypeName(t *testing.T) {
+	e, err := parser.ParseExprFrom(fset, "", "struct { bytes.Buffer }", 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ft := e.(*ast.StructType).Fields.List[0].Type
+	if got, want := typeName(ft), "Buffer"; got != want {
+		t.Errorf("got: %q; want %q", got, want)
+	}
+}

bind/gen.go

@@ -81,7 +81,7 @@ type Generator struct {
 	*Printer
 	Fset   *token.FileSet
 	AllPkg []*types.Package
-	AST    *ast.Package
+	Files  []*ast.File
 	Pkg    *types.Package
 	err    ErrorList
 
@@ -101,7 +101,7 @@ type Generator struct {
 	docs pkgDocs
 }
 
-// A pkgDocs maps identifier names to its extracted documentation for a package.
+// A pkgDocs maps the name of each exported package-level declaration to its extracted documentation.
 type pkgDocs map[string]*pkgDoc
 
 type pkgDoc struct {
@@ -189,29 +189,34 @@ func (g *Generator) Init() {
 	}
 }
 
-func (d pkgDocs) Visit(n ast.Node) ast.Visitor {
-	switch n := n.(type) {
-	case *ast.GenDecl:
-		outerDoc := n.Doc
-		for _, spec := range n.Specs {
-			switch t := spec.(type) {
-			case *ast.TypeSpec:
-				d.addType(t, outerDoc)
-			case *ast.ValueSpec:
-				d.addValue(t, outerDoc)
+// parseDocs extracts documentation from a package in a form useful for lookups.
+func (g *Generator) parseDocs() {
+	d := make(pkgDocs)
+	for _, f := range g.Files {
+		for _, decl := range f.Decls {
+			switch decl := decl.(type) {
+			case *ast.GenDecl:
+				for _, spec := range decl.Specs {
+					switch spec := spec.(type) {
+					case *ast.TypeSpec:
+						d.addType(spec, decl.Doc)
+					case *ast.ValueSpec:
+						d.addValue(spec, decl.Doc)
+					}
+				}
+			case *ast.FuncDecl:
+				d.addFunc(decl)
 			}
 		}
-		return nil
-	case *ast.FuncDecl:
-		d.addFunc(n)
-		return nil
-	default:
-		return d
 	}
+	g.docs = d
 }
 
 func (d pkgDocs) addValue(t *ast.ValueSpec, outerDoc *ast.CommentGroup) {
 	for _, n := range t.Names {
+		if !ast.IsExported(n.Name) {
+			continue
+		}
 		doc := t.Doc
 		if doc == nil {
 			doc = outerDoc
@@ -228,15 +233,18 @@ func (d pkgDocs) addFunc(f *ast.FuncDecl) {
 		return
 	}
 	fn := f.Name.Name
+	if !ast.IsExported(fn) {
+		return
+	}
 	if r := f.Recv; r != nil {
 		// f is a method.
-		switch t := r.List[0].Type.(type) {
-		case *ast.Ident:
-			d.addMethod(t.Name, fn, doc.Text())
-		case *ast.StarExpr:
-			sname := t.X.(*ast.Ident).Name
-			d.addMethod(sname, fn, doc.Text())
+		n := typeName(r.List[0].Type)
+		pd, exists := d[n]
+		if !exists {
+			pd = &pkgDoc{members: make(map[string]string)}
+			d[n] = pd
 		}
+		pd.members[fn] = doc.Text()
 	} else {
 		// f is a function.
 		d[fn] = &pkgDoc{doc: doc.Text()}
@@ -244,15 +252,16 @@ func (d pkgDocs) addFunc(f *ast.FuncDecl) {
 }
 
 func (d pkgDocs) addType(t *ast.TypeSpec, outerDoc *ast.CommentGroup) {
+	if !ast.IsExported(t.Name.Name) {
+		return
+	}
 	doc := t.Doc
 	if doc == nil {
 		doc = outerDoc
 	}
-	pd, exists := d[t.Name.Name]
-	if !exists {
-		pd = &pkgDoc{members: make(map[string]string)}
-		d[t.Name.Name] = pd
-	}
+	pd := d[t.Name.Name]
+	pd = &pkgDoc{members: make(map[string]string)}
+	d[t.Name.Name] = pd
 	if doc != nil {
 		pd.doc = doc.Text()
 	}
@@ -266,35 +275,35 @@ func (d pkgDocs) addType(t *ast.TypeSpec, outerDoc *ast.CommentGroup) {
 	if fields != nil {
 		for _, field := range fields.List {
 			if field.Doc != nil {
-				pd.members[field.Names[0].Name] = field.Doc.Text()
+				if field.Names == nil {
+					// Anonymous field. Extract name from its type.
+					if n := typeName(field.Type); ast.IsExported(n) {
+						pd.members[n] = field.Doc.Text()
+					}
+				}
+				for _, n := range field.Names {
+					if ast.IsExported(n.Name) {
+						pd.members[n.Name] = field.Doc.Text()
+					}
+				}
 			}
 		}
 	}
 }
 
-func (d pkgDocs) addMethod(sname, fname, doc string) {
-	pd, exists := d[sname]
-	if !exists {
-		pd = &pkgDoc{members: make(map[string]string)}
-		d[sname] = pd
+// typeName returns the type name T for expressions on the
+// T, *T, **T (etc.) form.
+func typeName(t ast.Expr) string {
+	switch t := t.(type) {
+	case *ast.StarExpr:
+		return typeName(t.X)
+	case *ast.Ident:
+		return t.Name
+	case *ast.SelectorExpr:
+		return t.Sel.Name
+	default:
+		return ""
 	}
-	pd.members[fname] = doc
-}
-
-// parseDocs extracts documentation from a package in a form useful for lookups.
-func (g *Generator) parseDocs() {
-	g.docs = make(pkgDocs)
-	if g.AST != nil {
-		ast.Walk(g.docs, g.AST)
-	}
-}
-
-func (d *pkgDoc) Visit(n ast.Node) ast.Visitor {
-	switch n := n.(type) {
-	case *ast.Field:
-		d.members[n.Names[0].Name] = n.Doc.Text()
-	}
-	return d
 }
 
 func (d *pkgDoc) Doc() string {

bind/testdata/doc.go

@@ -29,6 +29,12 @@ func (_ *S) Before() {}
 type S struct {
 	// SF is a field.
 	SF string
+	// blank (unexported) field.
+	_ string
+	// Anonymous field.
+	*S2
+	// Multiple fields.
+	F1, F2 string
 }
 
 // After is another method.

bind/testdata/doc.go.golden

@@ -41,6 +41,58 @@ func proxydoc_S_SF_Get(refnum C.int32_t) C.nstring {
 	return _v
 }
 
+//export proxydoc_S_S2_Set
+func proxydoc_S_S2_Set(refnum C.int32_t, v C.int32_t) {
+	ref := _seq.FromRefNum(int32(refnum))
+	// Must be a Go object
+	var _v *doc.S2
+	if _v_ref := _seq.FromRefNum(int32(v)); _v_ref != nil {
+		_v = _v_ref.Get().(*doc.S2)
+	}
+	ref.Get().(*doc.S).S2 = _v
+}
+
+//export proxydoc_S_S2_Get
+func proxydoc_S_S2_Get(refnum C.int32_t) C.int32_t {
+	ref := _seq.FromRefNum(int32(refnum))
+	v := ref.Get().(*doc.S).S2
+	var _v C.int32_t = _seq.NullRefNum
+	if v != nil {
+		_v = C.int32_t(_seq.ToRefNum(v))
+	}
+	return _v
+}
+
+//export proxydoc_S_F1_Set
+func proxydoc_S_F1_Set(refnum C.int32_t, v C.nstring) {
+	ref := _seq.FromRefNum(int32(refnum))
+	_v := decodeString(v)
+	ref.Get().(*doc.S).F1 = _v
+}
+
+//export proxydoc_S_F1_Get
+func proxydoc_S_F1_Get(refnum C.int32_t) C.nstring {
+	ref := _seq.FromRefNum(int32(refnum))
+	v := ref.Get().(*doc.S).F1
+	_v := encodeString(v)
+	return _v
+}
+
+//export proxydoc_S_F2_Set
+func proxydoc_S_F2_Set(refnum C.int32_t, v C.nstring) {
+	ref := _seq.FromRefNum(int32(refnum))
+	_v := decodeString(v)
+	ref.Get().(*doc.S).F2 = _v
+}
+
+//export proxydoc_S_F2_Get
+func proxydoc_S_F2_Get(refnum C.int32_t) C.nstring {
+	ref := _seq.FromRefNum(int32(refnum))
+	v := ref.Get().(*doc.S).F2
+	_v := encodeString(v)
+	return _v
+}
+
 //export proxydoc_S_After
 func proxydoc_S_After(refnum C.int32_t) {
 	ref := _seq.FromRefNum(int32(refnum))

bind/testdata/doc.java.c.golden

@@ -90,6 +90,51 @@ Java_doc_S_getSF(JNIEnv *env, jobject this) {
     return _r0;
 }
 
+JNIEXPORT void JNICALL
+Java_doc_S_setS2(JNIEnv *env, jobject this, jobject v) {
+    int32_t o = go_seq_to_refnum_go(env, this);
+    int32_t _v = go_seq_to_refnum(env, v);
+    proxydoc_S_S2_Set(o, _v);
+}
+
+JNIEXPORT jobject JNICALL
+Java_doc_S_getS2(JNIEnv *env, jobject this) {
+    int32_t o = go_seq_to_refnum_go(env, this);
+    int32_t r0 = proxydoc_S_S2_Get(o);
+    jobject _r0 = go_seq_from_refnum(env, r0, proxy_class_doc_S2, proxy_class_doc_S2_cons);
+    return _r0;
+}
+
+JNIEXPORT void JNICALL
+Java_doc_S_setF1(JNIEnv *env, jobject this, jstring v) {
+    int32_t o = go_seq_to_refnum_go(env, this);
+    nstring _v = go_seq_from_java_string(env, v);
+    proxydoc_S_F1_Set(o, _v);
+}
+
+JNIEXPORT jstring JNICALL
+Java_doc_S_getF1(JNIEnv *env, jobject this) {
+    int32_t o = go_seq_to_refnum_go(env, this);
+    nstring r0 = proxydoc_S_F1_Get(o);
+    jstring _r0 = go_seq_to_java_string(env, r0);
+    return _r0;
+}
+
+JNIEXPORT void JNICALL
+Java_doc_S_setF2(JNIEnv *env, jobject this, jstring v) {
+    int32_t o = go_seq_to_refnum_go(env, this);
+    nstring _v = go_seq_from_java_string(env, v);
+    proxydoc_S_F2_Set(o, _v);
+}
+
+JNIEXPORT jstring JNICALL
+Java_doc_S_getF2(JNIEnv *env, jobject this) {
+    int32_t o = go_seq_to_refnum_go(env, this);
+    nstring r0 = proxydoc_S_F2_Get(o);
+    jstring _r0 = go_seq_to_java_string(env, r0);
+    return _r0;
+}
+
 JNIEXPORT jobject JNICALL
 Java_doc_S2__1_1New(JNIEnv *env, jclass clazz) {
     int32_t refnum = new_doc_S2();

bind/testdata/doc.java.golden

@@ -87,13 +87,37 @@ public final class S implements Seq.Proxy {
      */
     public final native void setSF(String v);
     
+    /**
+     * Anonymous field.
+     */
+    public final native S2 getS2();
+    /**
+     * Anonymous field.
+     */
+    public final native void setS2(S2 v);
+    
+    /**
+     * Multiple fields.
+     */
+    public final native String getF1();
+    /**
+     * Multiple fields.
+     */
+    public final native void setF1(String v);
+    
+    /**
+     * Multiple fields.
+     */
+    public final native String getF2();
+    /**
+     * Multiple fields.
+     */
+    public final native void setF2(String v);
+    
     /**
      * After is another method.
      */
     public native void after();
-    /**
-     * Before is a method.
-     */
     public native void before();
     @Override public boolean equals(Object o) {
         if (o == null || !(o instanceof S)) {
@@ -109,17 +133,47 @@ public final class S implements Seq.Proxy {
         } else if (!thisSF.equals(thatSF)) {
             return false;
         }
+        S2 thisS2 = getS2();
+        S2 thatS2 = that.getS2();
+        if (thisS2 == null) {
+            if (thatS2 != null) {
+                return false;
+            }
+        } else if (!thisS2.equals(thatS2)) {
+            return false;
+        }
+        String thisF1 = getF1();
+        String thatF1 = that.getF1();
+        if (thisF1 == null) {
+            if (thatF1 != null) {
+                return false;
+            }
+        } else if (!thisF1.equals(thatF1)) {
+            return false;
+        }
+        String thisF2 = getF2();
+        String thatF2 = that.getF2();
+        if (thisF2 == null) {
+            if (thatF2 != null) {
+                return false;
+            }
+        } else if (!thisF2.equals(thatF2)) {
+            return false;
+        }
         return true;
     }
     
     @Override public int hashCode() {
-        return java.util.Arrays.hashCode(new Object[] {getSF()});
+        return java.util.Arrays.hashCode(new Object[] {getSF(), getS2(), getF1(), getF2()});
     }
     
     @Override public String toString() {
         StringBuilder b = new StringBuilder();
         b.append("S").append("{");
         b.append("SF:").append(getSF()).append(",");
+        b.append("S2:").append(getS2()).append(",");
+        b.append("F1:").append(getF1()).append(",");
+        b.append("F2:").append(getF2()).append(",");
         return b.append("}").toString();
     }
 }

bind/testdata/doc.objc.h.golden

@@ -48,13 +48,25 @@
  */
 - (NSString*)sf;
 - (void)setSF:(NSString*)v;
+/**
+ * Anonymous field.
+ */
+- (DocS2*)s2;
+- (void)setS2:(DocS2*)v;
+/**
+ * Multiple fields.
+ */
+- (NSString*)f1;
+- (void)setF1:(NSString*)v;
+/**
+ * Multiple fields.
+ */
+- (NSString*)f2;
+- (void)setF2:(NSString*)v;
 /**
  * After is another method.
  */
 - (void)after;
-/**
- * Before is a method.
- */
 - (void)before;
 @end
 

bind/testdata/doc.objc.m.golden

@@ -60,6 +60,58 @@
 	proxydoc_S_SF_Set(refnum, _v);
 }
 
+- (DocS2*)s2 {
+	int32_t refnum = go_seq_go_to_refnum(self._ref);
+	int32_t r0 = proxydoc_S_S2_Get(refnum);
+	DocS2* _r0 = nil;
+	GoSeqRef* _r0_ref = go_seq_from_refnum(r0);
+	if (_r0_ref != NULL) {
+		_r0 = _r0_ref.obj;
+		if (_r0 == nil) {
+			_r0 = [[DocS2 alloc] initWithRef:_r0_ref];
+		}
+	}
+	return _r0;
+}
+
+- (void)setS2:(DocS2*)v {
+	int32_t refnum = go_seq_go_to_refnum(self._ref);
+	int32_t _v;
+	if ([v conformsToProtocol:@protocol(goSeqRefInterface)]) {
+		id<goSeqRefInterface> v_proxy = (id<goSeqRefInterface>)(v);
+		_v = go_seq_go_to_refnum(v_proxy._ref);
+	} else {
+		_v = go_seq_to_refnum(v);
+	}
+	proxydoc_S_S2_Set(refnum, _v);
+}
+
+- (NSString*)f1 {
+	int32_t refnum = go_seq_go_to_refnum(self._ref);
+	nstring r0 = proxydoc_S_F1_Get(refnum);
+	NSString *_r0 = go_seq_to_objc_string(r0);
+	return _r0;
+}
+
+- (void)setF1:(NSString*)v {
+	int32_t refnum = go_seq_go_to_refnum(self._ref);
+	nstring _v = go_seq_from_objc_string(v);
+	proxydoc_S_F1_Set(refnum, _v);
+}
+
+- (NSString*)f2 {
+	int32_t refnum = go_seq_go_to_refnum(self._ref);
+	nstring r0 = proxydoc_S_F2_Get(refnum);
+	NSString *_r0 = go_seq_to_objc_string(r0);
+	return _r0;
+}
+
+- (void)setF2:(NSString*)v {
+	int32_t refnum = go_seq_go_to_refnum(self._ref);
+	nstring _v = go_seq_from_objc_string(v);
+	proxydoc_S_F2_Set(refnum, _v);
+}
+
 - (void)after {
 	int32_t refnum = go_seq_go_to_refnum(self._ref);
 	proxydoc_S_After(refnum);

cmd/gomobile/bind.go

@@ -198,7 +198,7 @@ func (b *binder) GenObjcSupport(outdir string) error {
 	return copyFile(filepath.Join(outdir, "seq.h"), filepath.Join(objcPkg.Dir, "seq.h"))
 }
 
-func (b *binder) GenObjc(pkg *types.Package, astPkg *ast.Package, allPkg []*types.Package, outdir string, wrappers []*objc.Named) (string, error) {
+func (b *binder) GenObjc(pkg *types.Package, files []*ast.File, allPkg []*types.Package, outdir string, wrappers []*objc.Named) (string, error) {
 	if pkg == nil {
 		bindPrefix = ""
 	}
@@ -227,7 +227,7 @@ func (b *binder) GenObjc(pkg *types.Package, astPkg *ast.Package, allPkg []*type
 			Fset:    b.fset,
 			AllPkg:  allPkg,
 			Pkg:     pkg,
-			AST:     astPkg,
+			Files:   files,
 		},
 		Prefix: bindPrefix,
 	}
@@ -495,7 +495,7 @@ func GenClasses(pkgs []*build.Package, srcDir, jpkgSrc string) ([]*java.Class, e
 	return classes, nil
 }
 
-func (b *binder) GenJava(pkg *types.Package, astPkg *ast.Package, allPkg []*types.Package, classes []*java.Class, outdir, androidDir string) error {
+func (b *binder) GenJava(pkg *types.Package, files []*ast.File, allPkg []*types.Package, classes []*java.Class, outdir, androidDir string) error {
 	jpkgname := bind.JavaPkgName(bindJavaPkg, pkg)
 	javadir := filepath.Join(androidDir, strings.Replace(jpkgname, ".", "/", -1))
 	var className string
@@ -525,7 +525,7 @@ func (b *binder) GenJava(pkg *types.Package, astPkg *ast.Package, allPkg []*type
 			Fset:    b.fset,
 			AllPkg:  allPkg,
 			Pkg:     pkg,
-			AST:     astPkg,
+			Files:   files,
 		},
 	}
 	g.Init(classes)
@@ -723,22 +723,20 @@ func loadExportData(pkgs []*build.Package, env []string, args ...string) ([]*typ
 	return typePkgs, nil
 }
 
-func parseAST(pkgs []*build.Package) ([]*ast.Package, error) {
+func parse(pkgs []*build.Package) ([][]*ast.File, error) {
 	fset := token.NewFileSet()
-	var astPkgs []*ast.Package
+	var astPkgs [][]*ast.File
 	for _, pkg := range pkgs {
 		fileNames := append(append([]string{}, pkg.GoFiles...), pkg.CgoFiles...)
-		files := make(map[string]*ast.File)
+		var files []*ast.File
 		for _, name := range fileNames {
 			f, err := parser.ParseFile(fset, filepath.Join(pkg.Dir, name), nil, parser.ParseComments)
 			if err != nil {
 				return nil, err
 			}
-			files[name] = f
+			files = append(files, f)
 		}
-		// Ignore errors (from unknown packages).
-		astPkg, _ := ast.NewPackage(fset, files, nil, nil)
-		astPkgs = append(astPkgs, astPkg)
+		astPkgs = append(astPkgs, files)
 	}
 	return astPkgs, nil
 }

cmd/gomobile/bind_androidapp.go

@@ -76,7 +76,7 @@ func goAndroidBind(pkgs []*build.Package, androidArchs []string) error {
 			return fmt.Errorf("loadExportData failed: %v", err)
 		}
 
-		astPkgs, err := parseAST(pkgs)
+		astPkgs, err := parse(pkgs)
 		if err != nil {
 			return fmt.Errorf("parseAST failed: %v", err)
 		}

cmd/gomobile/bind_iosapp.go

@@ -31,7 +31,7 @@ func goIOSBind(pkgs []*build.Package) error {
 		return err
 	}
 
-	astPkgs, err := parseAST(pkgs)
+	astPkgs, err := parse(pkgs)
 	if err != nil {
 		return err
 	}