From d46b792624199e09d16b5e381553f22ee1c5f1fb Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josharian@gmail.com>
Date: Fri, 13 Jun 2014 18:44:31 -0700
Subject: [PATCH] cmd/vet: check for use of json/xml struct tags with
 unexported fields

This is a common source of bugs, particularly for those new to Go. Running this on a corpus of public code flagged 114 instances.

This check may need to be updated once issue 7363 is resolved.

LGTM=r
R=golang-codereviews, r
CC=bradfitz, golang-codereviews
https://golang.org/cl/91010047
---
 cmd/vet/doc.go                |  1 +
 cmd/vet/structtag.go          | 24 ++++++++++++++++++++++--
 cmd/vet/testdata/structtag.go | 14 ++++++++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/cmd/vet/doc.go b/cmd/vet/doc.go
index b82da7ac0d..3b3e5ae7f8 100644
--- a/cmd/vet/doc.go
+++ b/cmd/vet/doc.go
@@ -72,6 +72,7 @@ Struct tags
 Flag: -structtags
 
 Struct tags that do not follow the format understood by reflect.StructTag.Get.
+Well-known encoding struct tags (json, xml) used with unexported fields.
 
 Unkeyed composite literals
 
diff --git a/cmd/vet/structtag.go b/cmd/vet/structtag.go
index 6735193aa7..5da390462c 100644
--- a/cmd/vet/structtag.go
+++ b/cmd/vet/structtag.go
@@ -14,7 +14,7 @@ import (
 
 func init() {
 	register("structtags",
-		"check that struct field tags have canonical format",
+		"check that struct field tags have canonical format and apply to exported fields as needed",
 		checkCanonicalFieldTag,
 		field)
 }
@@ -35,8 +35,28 @@ func checkCanonicalFieldTag(f *File, node ast.Node) {
 	// Check tag for validity by appending
 	// new key:value to end and checking that
 	// the tag parsing code can find it.
-	if reflect.StructTag(tag+` _gofix:"_magic"`).Get("_gofix") != "_magic" {
+	st := reflect.StructTag(tag + ` _gofix:"_magic"`)
+	if st.Get("_gofix") != "_magic" {
 		f.Badf(field.Pos(), "struct field tag %s not compatible with reflect.StructTag.Get", field.Tag.Value)
 		return
 	}
+
+	// Check for use of json or xml tags with unexported fields.
+
+	// Embedded struct. Nothing to do for now, but that
+	// may change, depending on what happens with issue 7363.
+	if len(field.Names) == 0 {
+		return
+	}
+
+	if field.Names[0].IsExported() {
+		return
+	}
+
+	for _, enc := range [...]string{"json", "xml"} {
+		if st.Get(enc) != "" {
+			f.Badf(field.Pos(), "struct field %s has %s tag but is not exported", field.Names[0].Name, enc)
+			return
+		}
+	}
 }
diff --git a/cmd/vet/testdata/structtag.go b/cmd/vet/testdata/structtag.go
index 502c58b08b..55462e5a45 100644
--- a/cmd/vet/testdata/structtag.go
+++ b/cmd/vet/testdata/structtag.go
@@ -11,3 +11,17 @@ package testdata
 type StructTagTest struct {
 	X int "hello" // ERROR "not compatible with reflect.StructTag.Get"
 }
+
+type UnexportedEncodingTagTest struct {
+	x int `json:"xx"` // ERROR "struct field x has json tag but is not exported"
+	y int `xml:"yy"`  // ERROR "struct field y has xml tag but is not exported"
+	z int
+	A int `json:"aa" xml:"bb"`
+}
+
+type unexp struct{}
+
+type JSONEmbeddedField struct {
+	UnexportedEncodingTagTest `is:"embedded"`
+	unexp                     `is:"embedded,notexported" json:"unexp"` // OK for now, see issue 7363
+}