Refine changelog tool to combine similar item and `Fix auto release…

… set tag` (#20614)

* Refine changelog tool to combine similar item

* fix

* add test

* removed enum print

* update test

* fix test

* fix

eng/tools/generator/autorest/model/changelog.go

@@ -198,12 +198,22 @@ func getNewContents(c *delta.Content) []string {
 		modified := c.GetModifiedStructs()
 		for _, s := range sortChangeItem(modified) {
 			f := modified[s]
+			afs := ""
+			sort.Strings(f.AnonymousFields)
 			for _, af := range f.AnonymousFields {
-				line := fmt.Sprintf("New anonymous field `%s` in struct `%s`", af, s)
+				afs = fmt.Sprintf("%s`%s`, ", afs, af)
+			}
+			if afs != "" {
+				line := fmt.Sprintf("New anonymous field %s in struct `%s`", strings.TrimSuffix(strings.TrimSpace(afs), ","), s)
 				items = append(items, line)
 			}
+
+			newFields := ""
 			for _, field := range sortChangeItem(f.Fields) {
-				line := fmt.Sprintf("New field `%s` in struct `%s`", field, s)
+				newFields = fmt.Sprintf("%s`%s`, ", newFields, field)
+			}
+			if newFields != "" {
+				line := fmt.Sprintf("New field %s in struct `%s`", strings.TrimSuffix(strings.TrimSpace(newFields), ","), s)
 				items = append(items, line)
 			}
 		}
@@ -299,14 +309,14 @@ func getRemovedContent(removed *delta.Content) []string {
 			for _, cs := range removedConst[k] {
 				consts = fmt.Sprintf("%s`%s`, ", consts, cs)
 			}
-			line := fmt.Sprintf("Const %s from type alias `%s` has been removed", strings.TrimRight(strings.TrimSpace(consts), ","), k)
+			line := fmt.Sprintf("%s from enum `%s` has been removed", strings.TrimRight(strings.TrimSpace(consts), ","), k)
 			items = append(items, line)
 		}
 	}
 	// write type alias
 	if len(removed.TypeAliases) > 0 {
 		for _, k := range sortChangeItem(removed.TypeAliases) {
-			line := fmt.Sprintf("Type alias `%s` has been removed", k)
+			line := fmt.Sprintf("Enum `%s` has been removed", k)
 			items = append(items, line)
 		}
 	}
@@ -353,12 +363,22 @@ func getRemovedContent(removed *delta.Content) []string {
 	if len(modified) > 0 {
 		for _, s := range sortChangeItem(modified) {
 			f := modified[s]
+			afs := ""
+			sort.Strings(f.AnonymousFields)
 			for _, af := range f.AnonymousFields {
-				line := fmt.Sprintf("Field `%s` of struct `%s` has been removed", af, s)
+				afs = fmt.Sprintf("%s`%s`, ", afs, af)
+			}
+			if afs != "" {
+				line := fmt.Sprintf("Field %s of struct `%s` has been removed", strings.TrimSuffix(strings.TrimSpace(afs), ","), s)
 				items = append(items, line)
 			}
+
+			newFields := ""
 			for _, field := range sortChangeItem(f.Fields) {
-				line := fmt.Sprintf("Field `%s` of struct `%s` has been removed", field, s)
+				newFields = fmt.Sprintf("%s`%s`, ", newFields, field)
+			}
+			if newFields != "" {
+				line := fmt.Sprintf("Field %s of struct `%s` has been removed", strings.TrimSuffix(strings.TrimSpace(newFields), ","), s)
 				items = append(items, line)
 			}
 		}

eng/tools/generator/autorest/model/changelog_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/eng/tools/internal/delta"
 	"github.com/Azure/azure-sdk-for-go/eng/tools/internal/exports"
+	"github.com/Azure/azure-sdk-for-go/eng/tools/internal/report"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -77,8 +78,44 @@ func TestRemovedConstAndTypeAlias(t *testing.T) {
 
 	actual := getRemovedContent(&removedConst)
 	expected := []string{
-		"Const `ConstA`, `ConstB`, `ConstC` from type alias `Const` has been removed",
-		"Type alias `RemovedTypeAlias` has been removed",
+		"`ConstA`, `ConstB`, `ConstC` from enum `Const` has been removed",
+		"Enum `RemovedTypeAlias` has been removed",
 	}
 	assert.Equal(t, expected, actual)
 }
+
+func TestCombineSimilarItem(t *testing.T) {
+	r := &report.Package{
+		AdditiveChanges: &delta.Content{
+			Content: exports.Content{
+				Structs: map[string]exports.Struct{
+					"Struct": {
+						AnonymousFields: []string{"AnonymousA", "AnonymousB"},
+						Fields: map[string]string{
+							"FieldB": "",
+							"FieldA": "",
+						},
+					},
+				},
+			},
+		},
+		BreakingChanges: &report.BreakingChanges{
+			Removed: &delta.Content{
+				Content: exports.Content{
+					Structs: map[string]exports.Struct{
+						"RemovedStruct": {
+							AnonymousFields: []string{"RemovedAnonymousA", "RemovedAnonymousB"},
+							Fields: map[string]string{
+								"RemovedFieldB": "",
+								"RemovedFieldA": "",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	actual := writeChangelogForPackage(r)
+	expected := "### Breaking Changes\n\n- Field `RemovedAnonymousA`, `RemovedAnonymousB` of struct `RemovedStruct` has been removed\n- Field `RemovedFieldA`, `RemovedFieldB` of struct `RemovedStruct` has been removed\n\n### Features Added\n\n- New anonymous field `AnonymousA`, `AnonymousB` in struct `Struct`\n- New field `FieldA`, `FieldB` in struct `Struct`\n"
+	assert.Equal(t, expected, actual)
+}

eng/tools/generator/cmd/v2/common/changelogProcessor_test.go

@@ -31,7 +31,7 @@ func TestEnumFilter(t *testing.T) {
 
 	common.FilterChangelog(changelog, common.EnumFilter)
 
-	excepted := fmt.Sprint("### Breaking Changes\n\n- Type alias `EnumRemove` has been removed\n\n### Features Added\n\n- New value `EnumExistB` added to enum type `EnumExist`\n- New enum type `EnumAdd` with values `EnumAddA`, `EnumAddB`\n")
+	excepted := fmt.Sprint("### Breaking Changes\n\n- Enum `EnumRemove` has been removed\n\n### Features Added\n\n- New value `EnumExistB` added to enum type `EnumExist`\n- New enum type `EnumAdd` with values `EnumAddA`, `EnumAddB`\n")
 	assert.Equal(t, excepted, changelog.ToCompactMarkdown())
 }
 

eng/tools/generator/cmd/v2/common/fileProcessor.go

@@ -55,6 +55,7 @@ type PackageInfo struct {
 	Config      string
 	SpecName    string
 	RequestLink string
+	Tag         string
 	ReleaseDate *time.Time
 }
 
@@ -180,8 +181,13 @@ func ChangeConfigWithCommitID(path, repoURL, commitID, specRPName string) error
 	lines := strings.Split(string(b), "\n")
 	for i, line := range lines {
 		if strings.Contains(line, autorest_md_file_suffix) {
-			lines[i] = fmt.Sprintf("- %s/blob/%s/specification/%s/resource-manager/readme.md", repoURL, commitID, specRPName)
-			lines[i+1] = fmt.Sprintf("- %s/blob/%s/specification/%s/resource-manager/readme.go.md", repoURL, commitID, specRPName)
+			indexResourceManager := strings.Index(line, "resource-manager")
+			indexReadme := strings.Index(line, autorest_md_file_suffix)
+			resourceManagerPath := []byte(line)
+			resourceManagerPath = resourceManagerPath[indexResourceManager : indexReadme-1]
+
+			lines[i] = fmt.Sprintf("- %s/blob/%s/specification/%s/%s/readme.md", repoURL, commitID, specRPName, resourceManagerPath)
+			lines[i+1] = fmt.Sprintf("- %s/blob/%s/specification/%s/%s/readme.go.md", repoURL, commitID, specRPName, resourceManagerPath)
 			break
 		}
 	}
@@ -449,8 +455,8 @@ func GetAlwaysSetBodyParamRequiredFlag(path string) (string, error) {
 	return "", nil
 }
 
-// AddPackageConfig add config in file
-func AddPackageConfig(path, config string) error {
+// AddTagSet add tag in file
+func AddTagSet(path, tag string) error {
 	b, err := os.ReadFile(path)
 	if err != nil {
 		return err
@@ -459,7 +465,7 @@ func AddPackageConfig(path, config string) error {
 	lines := strings.Split(string(b), "\n")
 	for i, line := range lines {
 		if strings.Contains(line, "tag:") {
-			lines[i] = config
+			lines[i] = tag
 			break
 		}
 
@@ -468,13 +474,13 @@ func AddPackageConfig(path, config string) error {
 			for j := len(lines) - 1; j > 0; j-- {
 				if strings.Contains(lines[j], "```") {
 					if lines[j-1] == "" {
-						lines[j-1] = config
+						lines[j-1] = tag
 						break
 					} else {
 						newLines := make([]string, len(lines))
 						copy(newLines, lines)
 
-						newLines = append(newLines[:j], config)
+						newLines = append(newLines[:j], tag)
 						tailLines := lines[j:]
 						lines = append(newLines, tailLines...)
 						break

eng/tools/generator/cmd/v2/common/generation.go

@@ -172,11 +172,11 @@ func (ctx *GenerateContext) GenerateForSingleRPNamespace(generateParam *Generate
 		}
 	}
 
-	// add package config
+	// add tag set
 	if !generateParam.RemoveTagSet && generateParam.NamespaceConfig != "" {
-		log.Printf("Add package config in `autorest.md`...")
+		log.Printf("Add tag in `autorest.md`...")
 		autorestMdPath := filepath.Join(packagePath, "autorest.md")
-		if err := AddPackageConfig(autorestMdPath, generateParam.NamespaceConfig); err != nil {
+		if err := AddTagSet(autorestMdPath, generateParam.NamespaceConfig); err != nil {
 			return nil, err
 		}
 	}

eng/tools/generator/cmd/v2/release/releaseCmd.go

@@ -214,7 +214,7 @@ func (c *commandContext) generateFromRequest(sdkRepo repo.SDKRepository, specRep
 			c.rpName = arm
 			c.namespaceName = info.Name
 			c.flags.SpecRPName = info.SpecName
-			c.flags.PackageConfig = info.Config
+			c.flags.PackageConfig = info.Tag
 			if info.ReleaseDate != nil {
 				c.flags.ReleaseDate = info.ReleaseDate.Format("2006-01-02")
 			}

eng/tools/generator/config/validate/validator.go

@@ -49,22 +49,21 @@ func ParseTrack2(config *config.Config, specRoot string) (armServices map[string
 				subService = s[0]
 			}
 
-			for _, packageInfos := range service {
-				for i := range packageInfos {
-					packageInfos[i].RequestLink = request.RequestLink
-					packageInfos[i].ReleaseDate = request.TargetDate
-					packageInfos[i].Config = fmt.Sprintf("tag: %s", request.PackageFlag)
+			for arm, packageInfos := range service {
+				if subService == "" && len(packageInfos) > 0 {
+					packageInfos[0].RequestLink = request.RequestLink
+					packageInfos[0].ReleaseDate = request.TargetDate
+					packageInfos[0].Tag = fmt.Sprintf("tag: %s", request.PackageFlag)
+					armServices[arm] = append(armServices[arm], packageInfos[0])
 				}
-			}
 
-			for arm, packageInfos := range service {
-				armServices[arm] = make([]common.PackageInfo, 0)
-				if subService == "" || len(packageInfos) == 1 {
-					armServices[arm] = packageInfos
-				} else {
-					for _, info := range packageInfos {
-						if strings.Contains(info.Config, subService) {
-							armServices[arm] = append(armServices[arm], info)
+				if subService != "" && len(packageInfos) > 0 {
+					for _, packageInfo := range packageInfos {
+						if strings.Contains(packageInfo.Config, subService) {
+							packageInfo.RequestLink = request.RequestLink
+							packageInfo.ReleaseDate = request.TargetDate
+							packageInfo.Tag = fmt.Sprintf("tag: %s", request.PackageFlag)
+							armServices[arm] = append(armServices[arm], packageInfo)
 							break
 						}
 					}