go2idl: Allow multiple values for a comment-tag

cmd/libs/go2idl/client-gen/generators/client_generator.go

@@ -120,7 +120,7 @@ func packageForGroup(gv unversioned.GroupVersion, typeList []*types.Type, packag
 			return generators
 		},
 		FilterFunc: func(c *generator.Context, t *types.Type) bool {
-			return types.ExtractCommentTags("+", t.SecondClosestCommentLines)["genclient"] == "true"
+			return extractBoolTagOrDie("genclient", t.SecondClosestCommentLines) == true
 		},
 	}
 }
@@ -190,7 +190,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 			} else {
 				// User has not specified any override for this group version.
 				// filter out types which dont have genclient=true.
-				if types.ExtractCommentTags("+", t.SecondClosestCommentLines)["genclient"] != "true" {
+				if extractBoolTagOrDie("genclient", t.SecondClosestCommentLines) == false {
 					continue
 				}
 			}

cmd/libs/go2idl/client-gen/generators/fake/fake_client_generator.go

@@ -20,6 +20,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/golang/glog"
+
 	clientgenargs "k8s.io/kubernetes/cmd/libs/go2idl/client-gen/args"
 	"k8s.io/kubernetes/cmd/libs/go2idl/client-gen/generators/normalization"
 	"k8s.io/kubernetes/cmd/libs/go2idl/generator"
@@ -75,11 +77,19 @@ func PackageForGroup(gv unversioned.GroupVersion, typeList []*types.Type, packag
 			return generators
 		},
 		FilterFunc: func(c *generator.Context, t *types.Type) bool {
-			return types.ExtractCommentTags("+", t.SecondClosestCommentLines)["genclient"] == "true"
+			return extractBoolTagOrDie("genclient", t.SecondClosestCommentLines) == true
 		},
 	}
 }
 
+func extractBoolTagOrDie(key string, lines []string) bool {
+	val, err := types.ExtractSingleBoolCommentTag("+", key, false, lines)
+	if err != nil {
+		glog.Fatalf(err.Error())
+	}
+	return val
+}
+
 func PackageForClientset(customArgs clientgenargs.Args, typedClientBasePath string, boilerplate []byte, generatedBy string) generator.Package {
 	return &generator.DefaultPackage{
 		// TODO: we'll generate fake clientset for different release in the future.

cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_group.go

@@ -72,7 +72,7 @@ func (g *genFakeForGroup) GenerateType(c *generator.Context, t *types.Type, w io
 			"Group":             namer.IC(g.group),
 			"realClientPackage": filepath.Base(g.realClientPath),
 		}
-		namespaced := !(types.ExtractCommentTags("+", t.SecondClosestCommentLines)["nonNamespaced"] == "true")
+		namespaced := !extractBoolTagOrDie("nonNamespaced", t.SecondClosestCommentLines)
 		if namespaced {
 			sw.Do(getterImplNamespaced, wrapper)
 		} else {

cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_type.go

@@ -79,7 +79,7 @@ func (g *genFakeForType) GenerateType(c *generator.Context, t *types.Type, w io.
 	sw := generator.NewSnippetWriter(w, c, "$", "$")
 	pkg := filepath.Base(t.Name.Package)
 	const pkgTestingCore = "k8s.io/kubernetes/pkg/client/testing/core"
-	namespaced := !(types.ExtractCommentTags("+", t.SecondClosestCommentLines)["nonNamespaced"] == "true")
+	namespaced := !extractBoolTagOrDie("nonNamespaced", t.SecondClosestCommentLines)
 	canonicalGroup := g.group
 	if canonicalGroup == "core" {
 		canonicalGroup = ""
@@ -96,8 +96,8 @@ func (g *genFakeForType) GenerateType(c *generator.Context, t *types.Type, w io.
 
 	// allow user to define a group name that's different from the one parsed from the directory.
 	p := c.Universe.Package(g.inputPackage)
-	if override, ok := types.ExtractCommentTags("+", p.DocComments)["groupName"]; ok {
-		groupName = override
+	if override := types.ExtractCommentTags("+", p.DocComments)["groupName"]; override != nil {
+		groupName = override[0]
 	}
 
 	m := map[string]interface{}{
@@ -136,7 +136,7 @@ func (g *genFakeForType) GenerateType(c *generator.Context, t *types.Type, w io.
 		"NewPatchAction":                 c.Universe.Function(types.Name{Package: pkgTestingCore, Name: "NewPatchAction"}),
 	}
 
-	noMethods := types.ExtractCommentTags("+", t.SecondClosestCommentLines)["noMethods"] == "true"
+	noMethods := extractBoolTagOrDie("noMethods", t.SecondClosestCommentLines) == true
 
 	if namespaced {
 		sw.Do(structNamespaced, m)

cmd/libs/go2idl/client-gen/generators/generator_for_group.go

@@ -74,8 +74,8 @@ func (g *genGroup) GenerateType(c *generator.Context, t *types.Type, w io.Writer
 	}
 	// allow user to define a group name that's different from the one parsed from the directory.
 	p := c.Universe.Package(g.inputPacakge)
-	if override, ok := types.ExtractCommentTags("+", p.DocComments)["groupName"]; ok && override != "" {
-		groupName = override
+	if override := types.ExtractCommentTags("+", p.DocComments)["groupName"]; override != nil {
+		groupName = override[0]
 	}
 
 	m := map[string]interface{}{
@@ -101,7 +101,7 @@ func (g *genGroup) GenerateType(c *generator.Context, t *types.Type, w io.Writer
 			"type":  t,
 			"Group": namer.IC(normalization.BeforeFirstDot(g.group)),
 		}
-		namespaced := !(types.ExtractCommentTags("+", t.SecondClosestCommentLines)["nonNamespaced"] == "true")
+		namespaced := !extractBoolTagOrDie("nonNamespaced", t.SecondClosestCommentLines)
 		if namespaced {
 			sw.Do(getterImplNamespaced, wrapper)
 		} else {

cmd/libs/go2idl/client-gen/generators/generator_for_type.go

@@ -66,7 +66,7 @@ func hasStatus(t *types.Type) bool {
 func (g *genClientForType) GenerateType(c *generator.Context, t *types.Type, w io.Writer) error {
 	sw := generator.NewSnippetWriter(w, c, "$", "$")
 	pkg := filepath.Base(t.Name.Package)
-	namespaced := !(types.ExtractCommentTags("+", t.SecondClosestCommentLines)["nonNamespaced"] == "true")
+	namespaced := !extractBoolTagOrDie("nonNamespaced", t.SecondClosestCommentLines)
 	m := map[string]interface{}{
 		"type":              t,
 		"package":           pkg,
@@ -86,7 +86,7 @@ func (g *genClientForType) GenerateType(c *generator.Context, t *types.Type, w i
 	} else {
 		sw.Do(getterNonNamesapced, m)
 	}
-	noMethods := types.ExtractCommentTags("+", t.SecondClosestCommentLines)["noMethods"] == "true"
+	noMethods := extractBoolTagOrDie("noMethods", t.SecondClosestCommentLines) == true
 
 	sw.Do(interfaceTemplate1, m)
 	if !noMethods {

cmd/libs/go2idl/client-gen/generators/tags.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package generators
+
+import (
+	"github.com/golang/glog"
+	"k8s.io/kubernetes/cmd/libs/go2idl/types"
+)
+
+// extractBoolTagOrDie gets the comment-tags for the key and asserts that, if
+// it exists, the value is boolean.  If the tag did not exist, it returns
+// false.
+func extractBoolTagOrDie(key string, lines []string) bool {
+	val, err := types.ExtractSingleBoolCommentTag("+", key, false, lines)
+	if err != nil {
+		glog.Fatalf(err.Error())
+	}
+	return val
+}

cmd/libs/go2idl/conversion-gen/generators/conversion.go

@@ -232,7 +232,7 @@ func Packages(context *generator.Context, arguments *args.GeneratorArgs) generat
 		// Only generate conversions for package which explicitly requested it
 		// byt setting "+genversion=true" in their doc.go file.
 		filtered := false
-		if types.ExtractCommentTags("+", p.DocComments)["genconversion"] == "true" {
+		if extractBoolTagOrDie("genconversion", false, p.DocComments) == true {
 			filtered = true
 		}
 		if !filtered {
@@ -342,7 +342,7 @@ func isDirectlyConvertible(in, out *types.Type, preexisting conversions) bool {
 				// "+ genconversion=false"
 				// comment to ignore this field for conversion.
 				// TODO: Switch to SecondClosestCommentLines.
-				if types.ExtractCommentTags("+", inMember.CommentLines)["genconversion"] == "false" {
+				if extractBoolTagOrDie("genconversion", true, inMember.CommentLines) == false {
 					continue
 				}
 				return false
@@ -428,7 +428,7 @@ func (g *genConversion) convertibleOnlyWithinPackage(inType, outType *types.Type
 	if t.Name.Package != g.targetPackage {
 		return false
 	}
-	if types.ExtractCommentTags("+", t.CommentLines)["genconversion"] == "false" {
+	if extractBoolTagOrDie("genconversion", true, t.CommentLines) == false {
 		return false
 	}
 	// TODO: Consider generating functions for other kinds too.

cmd/libs/go2idl/conversion-gen/generators/tags.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package generators
+
+import (
+	"github.com/golang/glog"
+	"k8s.io/kubernetes/cmd/libs/go2idl/types"
+)
+
+// extractBoolTagOrDie gets the comment-tags for the key and asserts that, if
+// it exists, the value is boolean.  If the tag did not exist, it returns
+// defaultVal.
+func extractBoolTagOrDie(key string, defaultVal bool, lines []string) bool {
+	val, err := types.ExtractSingleBoolCommentTag("+", key, defaultVal, lines)
+	if err != nil {
+		glog.Fatalf(err.Error())
+	}
+	return val
+}

cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go

@@ -200,7 +200,7 @@ func isRootedUnder(pkg string, roots []string) bool {
 
 func copyableWithinPackage(t *types.Type, boundingDirs []string) bool {
 	// If the type opts out of copy-generation, stop.
-	if types.ExtractCommentTags("+", t.CommentLines)["gencopy"] == "false" {
+	if extractBoolTagOrDie("gencopy", true, t.CommentLines) == false {
 		return false
 	}
 	// Only packages within the restricted range can be processed.

cmd/libs/go2idl/deepcopy-gen/generators/tags.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package generators
+
+import (
+	"github.com/golang/glog"
+	"k8s.io/kubernetes/cmd/libs/go2idl/types"
+)
+
+// extractBoolTagOrDie gets the comment-tags for the key and asserts that, if
+// it exists, the value is boolean.  If the tag did not exist, it returns
+// defaultVal.
+func extractBoolTagOrDie(key string, defaultVal bool, lines []string) bool {
+	val, err := types.ExtractSingleBoolCommentTag("+", key, defaultVal, lines)
+	if err != nil {
+		glog.Fatalf(err.Error())
+	}
+	return val
+}

cmd/libs/go2idl/go-to-protobuf/protobuf/generator.go

@@ -25,6 +25,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/golang/glog"
+
 	"k8s.io/kubernetes/cmd/libs/go2idl/generator"
 	"k8s.io/kubernetes/cmd/libs/go2idl/namer"
 	"k8s.io/kubernetes/cmd/libs/go2idl/types"
@@ -70,13 +72,20 @@ func (g *genProtoIDL) Namers(c *generator.Context) namer.NameSystems {
 
 // Filter ignores types that are identified as not exportable.
 func (g *genProtoIDL) Filter(c *generator.Context, t *types.Type) bool {
-	flags := types.ExtractCommentTags("+", t.CommentLines)
-	switch {
-	case flags["protobuf"] == "false":
-		return false
-	case flags["protobuf"] == "true":
-		return true
-	case !g.generateAll:
+	tagVals := types.ExtractCommentTags("+", t.CommentLines)["protobuf"]
+	if tagVals != nil {
+		if tagVals[0] == "false" {
+			// Type specified "false".
+			return false
+		}
+		if tagVals[0] == "true" {
+			// Type specified "true".
+			return true
+		}
+		glog.Fatalf(`Comment tag "protobuf" must be true or false, found: %q`, tagVals[0])
+	}
+	if !g.generateAll {
+		// We're not generating everything.
 		return false
 	}
 	seen := map[*types.Type]bool{}
@@ -125,7 +134,7 @@ func isOptionalAlias(t *types.Type) bool {
 	if t.Underlying == nil || (t.Underlying.Kind != types.Map && t.Underlying.Kind != types.Slice) {
 		return false
 	}
-	if types.ExtractCommentTags("+", t.CommentLines)["protobuf.nullable"] != "true" {
+	if extractBoolTagOrDie("protobuf.nullable", t.CommentLines) == false {
 		return false
 	}
 	return true
@@ -296,7 +305,7 @@ func (b bodyGen) doStruct(sw *generator.SnippetWriter) error {
 			key := strings.TrimPrefix(k, "protobuf.options.")
 			switch key {
 			case "marshal":
-				if v == "false" {
+				if v[0] == "false" {
 					if !b.omitGogo {
 						options = append(options,
 							"(gogoproto.marshaler) = false",
@@ -307,14 +316,14 @@ func (b bodyGen) doStruct(sw *generator.SnippetWriter) error {
 				}
 			default:
 				if !b.omitGogo || !strings.HasPrefix(key, "(gogoproto.") {
-					options = append(options, fmt.Sprintf("%s = %s", key, v))
+					options = append(options, fmt.Sprintf("%s = %s", key, v[0]))
 				}
 			}
 		// protobuf.as allows a type to have the same message contents as another Go type
 		case k == "protobuf.as":
 			fields = nil
-			if alias = b.locator.GoTypeForName(types.Name{Name: v}); alias == nil {
-				return fmt.Errorf("type %v references alias %q which does not exist", b.t, v)
+			if alias = b.locator.GoTypeForName(types.Name{Name: v[0]}); alias == nil {
+				return fmt.Errorf("type %v references alias %q which does not exist", b.t, v[0])
 			}
 		// protobuf.embed instructs the generator to use the named type in this package
 		// as an embedded message.
@@ -322,10 +331,10 @@ func (b bodyGen) doStruct(sw *generator.SnippetWriter) error {
 			fields = []protoField{
 				{
 					Tag:  1,
-					Name: v,
+					Name: v[0],
 					Type: &types.Type{
 						Name: types.Name{
-							Name:    v,
+							Name:    v[0],
 							Package: b.localPackage.Package,
 							Path:    b.localPackage.Path,
 						},

cmd/libs/go2idl/go-to-protobuf/protobuf/tags.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package protobuf
+
+import (
+	"github.com/golang/glog"
+	"k8s.io/kubernetes/cmd/libs/go2idl/types"
+)
+
+// extractBoolTagOrDie gets the comment-tags for the key and asserts that, if
+// it exists, the value is boolean.  If the tag did not exist, it returns
+// false.
+func extractBoolTagOrDie(key string, lines []string) bool {
+	val, err := types.ExtractSingleBoolCommentTag("+", key, false, lines)
+	if err != nil {
+		glog.Fatalf(err.Error())
+	}
+	return val
+}

cmd/libs/go2idl/set-gen/generators/sets.go

@@ -108,7 +108,7 @@ func Packages(_ *generator.Context, arguments *args.GeneratorArgs) generator.Pac
 				// // +genset
 				// or
 				// // +genset=true
-				return types.ExtractCommentTags("+", t.CommentLines)["genset"] == "true"
+				return extractBoolTagOrDie("genset", t.CommentLines) == true
 			}
 			return false
 		},

cmd/libs/go2idl/set-gen/generators/tags.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package generators
+
+import (
+	"github.com/golang/glog"
+	"k8s.io/kubernetes/cmd/libs/go2idl/types"
+)
+
+// extractBoolTagOrDie gets the comment-tags for the key and asserts that, if
+// it exists, the value is boolean.  If the tag did not exist, it returns
+// false.
+func extractBoolTagOrDie(key string, lines []string) bool {
+	val, err := types.ExtractSingleBoolCommentTag("+", key, false, lines)
+	if err != nil {
+		glog.Fatalf(err.Error())
+	}
+	return val
+}

cmd/libs/go2idl/types/comments.go

@@ -19,28 +19,26 @@ limitations under the License.
 package types
 
 import (
+	"fmt"
 	"strings"
 )
 
 // ExtractCommentTags parses comments for lines of the form:
 //
-//   'marker'+"key1=value1,key2=value2".
+//   'marker' + "key1=value1,key2=value2".
 //
-// Values are optional; 'true' is the default. If a key is set multiple times,
-// the last one wins.
+// Values are optional; "" is the default.  A tag can be specified more than
+// one time and all values are returned.  If the resulting map has an entry for
+// a key, the value (a slice) is guaranteed to have at least 1 element.
 //
 // Example: if you pass "+" for 'marker', and the following two lines are in
 // the comments:
 //   +foo=value1,bar
-//   +foo=value2,baz="frobber"
+//   +foo=value2,baz="qux"
 // Then this function will return:
-//   map[string]string{"foo":"value2", "bar": "true", "baz": "frobber"}
-//
-// TODO: Basically we need to define a standard way of giving instructions to
-// autogenerators in the comments of a type. This is a first iteration of that.
-// TODO: allow multiple values per key?
-func ExtractCommentTags(marker string, lines []string) map[string]string {
-	out := map[string]string{}
+//   map[string][]string{"foo":{"value1, "value2"}, "bar": {""}, "baz": {"qux"}}
+func ExtractCommentTags(marker string, lines []string) map[string][]string {
+	out := map[string][]string{}
 	for _, line := range lines {
 		line = strings.Trim(line, " ")
 		if len(line) == 0 {
@@ -53,11 +51,32 @@ func ExtractCommentTags(marker string, lines []string) map[string]string {
 		for _, p := range pairs {
 			kv := strings.Split(p, "=")
 			if len(kv) == 2 {
-				out[kv[0]] = kv[1]
+				out[kv[0]] = append(out[kv[0]], kv[1])
 			} else if len(kv) == 1 {
-				out[kv[0]] = "true"
+				out[kv[0]] = append(out[kv[0]], "")
 			}
 		}
 	}
 	return out
 }
+
+// ExtractSingleBoolCommentTag parses comments for lines of the form:
+//
+//   'marker' + "key=value1"
+//
+// If the tag is not found, the default value is returned.  Values are asserted
+// to be boolean ("true" or "false"), and any other value will cause an error
+// to be returned.  If the key has multiple values, the first one will be used.
+func ExtractSingleBoolCommentTag(marker string, key string, defaultVal bool, lines []string) (bool, error) {
+	values := ExtractCommentTags(marker, lines)[key]
+	if values == nil {
+		return defaultVal, nil
+	}
+	if values[0] == "true" {
+		return true, nil
+	}
+	if values[0] == "false" {
+		return false, nil
+	}
+	return false, fmt.Errorf("tag value for %q is not boolean: %q", key, values[0])
+}

cmd/libs/go2idl/types/comments_test.go

@@ -18,18 +18,63 @@ package types
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 )
 
 func TestExtractCommentTags(t *testing.T) {
-	commentLines := `
-Human comment that is ignored.
-+foo=value1,bar
-+foo=value2,baz=frobber
-`
+	commentLines := []string{
+		"Human comment that is ignored.",
+		"+foo=value1,bar",
+		"+foo=value2,baz=qux",
+	}
+
 	a := ExtractCommentTags("+", commentLines)
-	e := map[string]string{"foo": "value2", "bar": "true", "baz": "frobber"}
+	e := map[string][]string{"foo": {"value1", "value2"}, "bar": {""}, "baz": {"qux"}}
 	if !reflect.DeepEqual(e, a) {
-		t.Errorf("Wanted %#v, got %#v", e, a)
+		t.Errorf("Wanted %q, got %q", e, a)
+	}
+}
+
+func TestExtractSingleBoolCommentTag(t *testing.T) {
+	commentLines := []string{
+		"Human comment that is ignored.",
+		"+TRUE=true",
+		"+FALSE=false",
+		"+MULTI=true",
+		"+MULTI=false",
+		"+MULTI=multi",
+		"+NOTBOOL=blue",
+		"+EMPTY",
+	}
+
+	testCases := []struct {
+		key string
+		def bool
+		exp bool
+		err string // if set, ignore exp.
+	}{
+		{"TRUE", false, true, ""},
+		{"FALSE", true, false, ""},
+		{"MULTI", false, true, ""},
+		{"NOTBOOL", false, true, "is not boolean"},
+		{"EMPTY", false, true, "is not boolean"},
+		{"ABSENT", true, true, ""},
+		{"ABSENT", false, false, ""},
+	}
+
+	for i, tc := range testCases {
+		v, err := ExtractSingleBoolCommentTag("+", tc.key, tc.def, commentLines)
+		if err != nil && tc.err == "" {
+			t.Errorf("[%d]: unexpected failure: %v", i, err)
+		} else if err == nil && tc.err != "" {
+			t.Errorf("[%d]: expected failure: %v", i, tc.err)
+		} else if err != nil {
+			if !strings.Contains(err.Error(), tc.err) {
+				t.Errorf("[%d]: unexpected error: expected %q, got %q", i, tc.err, err)
+			}
+		} else if v != tc.exp {
+			t.Errorf("[%d]: unexpected value: expected %t, got %t", i, tc.exp, v)
+		}
 	}
 }