Merge pull request #131608 from omerap12/migrate-ExtractCommentTags-conversion

Migrate ExtractCommentTags to ExtractFunctionStyleCommentTags

staging/src/k8s.io/code-generator/cmd/conversion-gen/generators/conversion.go

@@ -48,26 +48,48 @@ const (
 	externalTypesTagName = "k8s:conversion-gen-external-types"
 )
 
-func extractTag(comments []string) []string {
-	return gengo.ExtractCommentTags("+", comments)[tagName]
+func extractTagValues(tagName string, comments []string) ([]string, error) {
+	tags, err := gengo.ExtractFunctionStyleCommentTags("+", []string{tagName}, comments)
+	if err != nil {
+		return nil, err
+	}
+	tagList, exists := tags[tagName]
+	if !exists {
+		return nil, nil
+	}
+	values := make([]string, len(tagList))
+	for i, v := range tagList {
+		values[i] = v.Value
+	}
+	return values, nil
 }
 
-func extractExplicitFromTag(comments []string) []string {
-	return gengo.ExtractCommentTags("+", comments)[explicitFromTagName]
+func extractTag(comments []string) ([]string, error) {
+	return extractTagValues(tagName, comments)
 }
 
-func extractExternalTypesTag(comments []string) []string {
-	return gengo.ExtractCommentTags("+", comments)[externalTypesTagName]
+func extractExplicitFromTag(comments []string) ([]string, error) {
+	return extractTagValues(explicitFromTagName, comments)
 }
 
-func isCopyOnly(comments []string) bool {
-	values := gengo.ExtractCommentTags("+", comments)["k8s:conversion-fn"]
-	return len(values) == 1 && values[0] == "copy-only"
+func extractExternalTypesTag(comments []string) ([]string, error) {
+	return extractTagValues(externalTypesTagName, comments)
 }
 
-func isDrop(comments []string) bool {
-	values := gengo.ExtractCommentTags("+", comments)["k8s:conversion-fn"]
-	return len(values) == 1 && values[0] == "drop"
+func isCopyOnly(comments []string) (bool, error) {
+	values, err := extractTagValues("k8s:conversion-fn", comments)
+	if err != nil {
+		return false, err
+	}
+	return len(values) == 1 && values[0] == "copy-only", nil
+}
+
+func isDrop(comments []string) (bool, error) {
+	values, err := extractTagValues("k8s:conversion-fn", comments)
+	if err != nil {
+		return false, err
+	}
+	return len(values) == 1 && values[0] == "drop", nil
 }
 
 // TODO: This is created only to reduce number of changes in a single PR.
@@ -229,11 +251,15 @@ func GetTargets(context *generator.Context, args *args.Args) []generator.Target
 		// Only generate conversions for packages which explicitly request it
 		// by specifying one or more "+k8s:conversion-gen=<peer-pkg>"
 		// in their doc.go file.
-		peerPkgs := extractTag(pkg.Comments)
+		peerPkgs, err := extractTag(pkg.Comments)
 		if peerPkgs == nil {
 			klog.V(3).Infof("  no tag")
 			continue
 		}
+		if err != nil {
+			klog.Errorf("failed to extract tag %s", err)
+			continue
+		}
 		klog.V(3).Infof("  tags: %q", peerPkgs)
 		if len(peerPkgs) == 1 && peerPkgs[0] == "false" {
 			// If a single +k8s:conversion-gen=false tag is defined, we still want
@@ -250,7 +276,10 @@ func GetTargets(context *generator.Context, args *args.Args) []generator.Target
 
 		// if the external types are not in the same package where the
 		// conversion functions to be generated
-		externalTypesValues := extractExternalTypesTag(pkg.Comments)
+		externalTypesValues, err := extractExternalTypesTag(pkg.Comments)
+		if err != nil {
+			klog.Fatalf("Failed to extract external types tag for package %q: %v", i, err)
+		}
 		if externalTypesValues != nil {
 			if len(externalTypesValues) != 1 {
 				klog.Fatalf("  expect only one value for %q tag, got: %q", externalTypesTagName, externalTypesValues)
@@ -337,7 +366,10 @@ func GetTargets(context *generator.Context, args *args.Args) []generator.Target
 	// If there is a manual conversion defined between two types, exclude it
 	// from being a candidate for unsafe conversion
 	for k, v := range manualConversions {
-		if isCopyOnly(v.CommentLines) {
+		copyOnly, err := isCopyOnly(v.CommentLines)
+		if err != nil {
+			klog.Errorf("error extracting tags: %v", err)
+		} else if copyOnly {
 			klog.V(4).Infof("Conversion function %s will not block memory copy because it is copy-only", v.Name)
 			continue
 		}
@@ -520,7 +552,12 @@ func (g *genConversion) convertibleOnlyWithinPackage(inType, outType *types.Type
 		return false
 	}
 	// If the type has opted out, skip it.
-	tagvals := extractTag(t.CommentLines)
+	tagvals, err := extractTag(t.CommentLines)
+	if err != nil {
+		klog.Errorf("Type %v: error extracting tags: %v", t, err)
+		return false
+	}
+
 	if tagvals != nil {
 		if tagvals[0] != "false" {
 			klog.Fatalf("Type %v: unsupported %s value: %q", t, tagName, tagvals[0])
@@ -542,8 +579,12 @@ func (g *genConversion) convertibleOnlyWithinPackage(inType, outType *types.Type
 func getExplicitFromTypes(t *types.Type) []types.Name {
 	comments := t.SecondClosestCommentLines
 	comments = append(comments, t.CommentLines...)
-	paths := extractExplicitFromTag(comments)
 	result := []types.Name{}
+	paths, err := extractExplicitFromTag(comments)
+	if err != nil {
+		klog.Errorf("Error extracting explicit-from tag for %v: %v", t.Name, err)
+		return result
+	}
 	for _, path := range paths {
 		items := strings.Split(path, ".")
 		if len(items) != 2 {
@@ -869,7 +910,11 @@ func (g *genConversion) doSlice(inType, outType *types.Type, sw *generator.Snipp
 
 func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.SnippetWriter) {
 	for _, inMember := range inType.Members {
-		if tagvals := extractTag(inMember.CommentLines); tagvals != nil && tagvals[0] == "false" {
+		tagvals, err := extractTag(inMember.CommentLines)
+		if err != nil {
+			klog.Errorf("Member %v.%v: error extracting tags: %v", inType, inMember.Name, err)
+		}
+		if tagvals != nil && tagvals[0] == "false" {
 			// This field is excluded from conversion.
 			sw.Do("// INFO: in."+inMember.Name+" opted out of conversion generation\n", nil)
 			continue
@@ -918,7 +963,10 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 
 		// check based on the top level name, not the underlying names
 		if function, ok := g.preexists(inMember.Type, outMember.Type); ok {
-			if isDrop(function.CommentLines) {
+			dropFn, err := isDrop(function.CommentLines)
+			if err != nil {
+				klog.Errorf("Error extracting drop tag for function %s: %v", function.Name, err)
+			} else if dropFn {
 				continue
 			}
 			// copy-only functions that are directly assignable can be inlined instead of invoked.
@@ -926,7 +974,12 @@ func (g *genConversion) doStruct(inType, outType *types.Type, sw *generator.Snip
 			// correctly copied between types. These functions are equivalent to a memory assignment,
 			// and are necessary for the reflection path, but should not block memory conversion.
 			// Convert_unversioned_Time_to_unversioned_Time is an example of this logic.
-			if !isCopyOnly(function.CommentLines) || !g.isFastConversion(inMemberType, outMemberType) {
+			copyOnly, copyErr := isCopyOnly(function.CommentLines)
+			if copyErr != nil {
+				klog.Errorf("Error extracting copy-only tag for function %s: %v", function.Name, copyErr)
+				copyOnly = false
+			}
+			if !copyOnly || !g.isFastConversion(inMemberType, outMemberType) {
 				args["function"] = function
 				sw.Do("if err := $.function|raw$(&in.$.name$, &out.$.name$, s); err != nil {\n", args)
 				sw.Do("return err\n", nil)
@@ -1080,7 +1133,11 @@ func (g *genConversion) generateFromURLValues(inType, outType *types.Type, sw *g
 	}
 	sw.Do("func auto"+nameTmpl+"(in *$.inType|raw$, out *$.outType|raw$, s $.Scope|raw$) error {\n", args)
 	for _, outMember := range outType.Members {
-		if tagvals := extractTag(outMember.CommentLines); tagvals != nil && tagvals[0] == "false" {
+		tagvals, err := extractTag(outMember.CommentLines)
+		if err != nil {
+			klog.Errorf("Member %v.%v: error extracting tags: %v", outType, outMember.Name, err)
+		}
+		if tagvals != nil && tagvals[0] == "false" {
 			// This field is excluded from conversion.
 			sw.Do("// INFO: in."+outMember.Name+" opted out of conversion generation\n", nil)
 			continue