Remove replace directive labels when not needed

edit/edit.go

@@ -179,3 +179,37 @@ func AddLabel(rule *build.Rule, label string) error {
 	rule.SetAttr("labels", ruleLabelsList)
 	return nil
 }
+
+func RemoveLabel(rule *build.Rule, label string) error {
+	// Fetch the labels attribute, if it exists
+	ruleLabels := rule.Attr("labels")
+	if ruleLabels == nil {
+		return nil
+	}
+	// Check it's a list of expressions
+	ruleLabelsList, ok := ruleLabels.(*build.ListExpr)
+	if !ok {
+		return errors.New("rule already has a `labels` attribute, and it is not a list")
+	}
+	// Build a new list of labels which doesn't include the specified one.
+	newLabels := &build.ListExpr{}
+	for _, labelExpr := range ruleLabelsList.List {
+		// Copy any non-string labels
+		labelStringExpr, ok := labelExpr.(*build.StringExpr)
+		if !ok {
+			newLabels.List = append(newLabels.List, labelExpr)
+			continue
+		}
+		// Copy any string labels which do not match
+		if labelStringExpr.Value != label {
+			newLabels.List = append(newLabels.List, labelExpr)
+		}
+	}
+	// If labels is empty, remove the attribute
+	if len(newLabels.List) == 0 {
+		rule.DelAttr("labels")
+	} else {
+		rule.SetAttr("labels", newLabels)
+	}
+	return nil
+}

sync/integration/syncmod/sync_mod_test.go

@@ -42,7 +42,7 @@ func TestModSync(t *testing.T) {
 	// Read version info from the go.mod file
 	expectedVers := readModFileVersions()
 
-	// We expect to generate the following for the replace in the go.mod:
+	// We expect to generate the following for the replace directives in the go.mod:
 	// go_mod_download(
 	//   name = "github.com_peterebden_buildtools_dl",
 	//   module = "github.com/peterebden/buildtools",
@@ -52,6 +52,13 @@ func TestModSync(t *testing.T) {
 	// go_repo(
 	//   download = ":github.com_peterebden_buildtools_dl",
 	//   module = "github.com/bazelbuild/buildtools",
+	//   labels = ["go_replace_directive"],
+	// )
+	//
+	// go_repo(
+	//   module = "github.com/stretchr/testify",
+	//   version = "v1.3.0",
+	//   labels = ["go_replace_directive"],
 	// )
 
 	for _, repoRule := range thirdPartyBuildFile.Rules("go_repo") {
@@ -68,10 +75,16 @@ func TestModSync(t *testing.T) {
 				return
 			}
 
-			// Check that testify is labelled for a replace directive
+			// Check that testify is the only other one labelled for a replace directive
+			labels := listLabels(repoRule)
 			if repoRule.AttrString("module") == "github.com/stretchr/testify" {
-				labels := listLabels(repoRule)
 				assert.Contains(t, labels, "go_replace_directive")
+			} else {
+				assert.NotContains(t, labels, "go_replace_directive")
+				// Ensure there aren't empty labels list attributes
+				if len(labels) == 0 {
+					assert.Nil(t, repoRule.Attr("labels"))
+				}
 			}
 
 			// All rules start off at v0.0.1 and should be updated to their version listed in the go.mod
@@ -90,6 +103,9 @@ func TestModSync(t *testing.T) {
 
 func listLabels(rule *build.Rule) []string {
 	labelsExpr := rule.Attr("labels")
+	if labelsExpr == nil {
+		return []string{}
+	}
 	labels := make([]string, 0, len(labelsExpr.(*build.ListExpr).List))
 	for _, labelExpr := range labelsExpr.(*build.ListExpr).List {
 		labels = append(labels, labelExpr.(*build.StringExpr).Value)

sync/integration/syncmod/test_repo/third_party/go/BUILD_FILE

@@ -6,16 +6,22 @@ go_repo(
 )
 
 go_repo(
+    labels = [
+      "example_label",
+      "go_replace_directive",
+    ],
     module = "github.com/davecgh/go-spew",
     version = "v0.0.1",
 )
 
 go_repo(
+    labels = ["example_label"],
     module = "github.com/pmezard/go-difflib",
     version = "v0.0.1",
 )
 
 go_repo(
+    labels = ["go_replace_directive"],
     module = "github.com/stretchr/objx",
     version = "v0.0.1",
 )

sync/sync.go

@@ -103,13 +103,32 @@ func (s *syncer) syncModFile(conf *config.Config, file *build.File, existingRule
 		return err
 	}
 
+	// Remove "go_replace_directive" label from any rules which lack a replace directive
+	for modPath, rule := range existingRules {
+		// Find any matching replace directive
+		var matchingReplace *modfile.Replace
+		for _, replace := range f.Replace {
+			if replace.Old.Path == modPath {
+				matchingReplace = replace
+			}
+		}
+
+		// Remove the replace label if not needed
+		if matchingReplace == nil {
+			err := edit.RemoveLabel(rule, ReplaceLabel)
+			if err != nil {
+				log.Warningf("Failed to remove replace label from %v: %v", modPath, err)
+			}
+		}
+	}
+
+	// Check all modules listed in go.mod
 	for _, req := range f.Require {
-		reqVersion := req.Mod.Version
+		// Find any matching replace directive
 		var matchingReplace *modfile.Replace
-		for _, r := range f.Replace {
-			if r.Old.Path == req.Mod.Path {
-				matchingReplace = r
-				reqVersion = r.New.Version
+		for _, replace := range f.Replace {
+			if replace.Old.Path == req.Mod.Path {
+				matchingReplace = replace
 			}
 		}
 
@@ -121,36 +140,58 @@ func (s *syncer) syncModFile(conf *config.Config, file *build.File, existingRule
 				// delete the old go_repo rule and regenerate it with a go_mod_download and a go_repo.
 				edit.RemoveTarget(file, rule)
 			} else {
-				// Make sure the version is up-to-date
-				rule.SetAttr("version", edit.NewStringExpr(reqVersion))
-				// Add label for the replace directive
-				if matchingReplace != nil {
-					err := edit.AddLabel(rule, ReplaceLabel)
-					if err != nil {
-						log.Warningf("Failed to add replace label to %v: %v", req.Mod.Path, err)
-					}
-				}
+				s.syncExistingRule(rule, req, matchingReplace)
 				// No other changes needed
 				continue
 			}
 		}
 
-		ls, err := s.licences.Get(req.Mod.Path, req.Mod.Version)
-		if err != nil {
-			return fmt.Errorf("failed to get licences for %v: %v", req.Mod.Path, err)
+		// Add a new rule to the build file if one does not exist
+		if err = s.addNewRule(file, req, matchingReplace); err != nil {
+			return fmt.Errorf("failed to add new rule %v: %v", req.Mod.Path, err)
 		}
+	}
+
+	return nil
+}
 
-		// If no replace directive, or replace directive is just replacing the version, add a simple rule
-		if matchingReplace == nil || matchingReplace.New.Path == req.Mod.Path {
-			file.Stmt = append(file.Stmt, edit.NewGoRepoRule(req.Mod.Path, reqVersion, "", ls, []string{ReplaceLabel}))
-			continue
+func (s *syncer) syncExistingRule(rule *build.Rule, requireDirective *modfile.Require, replaceDirective *modfile.Replace) {
+	reqVersion := requireDirective.Mod.Version
+	// Add label for the replace directive
+	if replaceDirective != nil {
+		err := edit.AddLabel(rule, ReplaceLabel)
+		if err != nil {
+			log.Warningf("Failed to add replace label to %v: %v", requireDirective.Mod.Path, err)
 		}
+		// Update the requested version
+		reqVersion = replaceDirective.New.Version
+	}
+	// Make sure the version is up-to-date
+	rule.SetAttr("version", edit.NewStringExpr(reqVersion))
+}
+
+func (s *syncer) addNewRule(file *build.File, requireDirective *modfile.Require, replaceDirective *modfile.Replace) error {
+	// List licences
+	ls, err := s.licences.Get(requireDirective.Mod.Path, requireDirective.Mod.Version)
+	if err != nil {
+		return fmt.Errorf("failed to get licences for %v: %v", requireDirective.Mod.Path, err)
+	}
+
+	// If no replace directive, add a simple rule
+	if replaceDirective == nil {
+		file.Stmt = append(file.Stmt, edit.NewGoRepoRule(requireDirective.Mod.Path, requireDirective.Mod.Version, "", ls, []string{}))
+		return nil
+	}
 
-		dl, dlName := edit.NewModDownloadRule(matchingReplace.New.Path, matchingReplace.New.Version, ls)
-		file.Stmt = append(file.Stmt, dl)
-		file.Stmt = append(file.Stmt, edit.NewGoRepoRule(req.Mod.Path, "", dlName, nil, []string{ReplaceLabel}))
+	// If replace directive is just replacing the version, add a simple rule
+	if replaceDirective.New.Path == requireDirective.Mod.Path {
+		file.Stmt = append(file.Stmt, edit.NewGoRepoRule(requireDirective.Mod.Path, replaceDirective.New.Version, "", ls, []string{ReplaceLabel}))
+		return nil
 	}
 
+	dl, dlName := edit.NewModDownloadRule(replaceDirective.New.Path, replaceDirective.New.Version, ls)
+	file.Stmt = append(file.Stmt, dl)
+	file.Stmt = append(file.Stmt, edit.NewGoRepoRule(requireDirective.Mod.Path, "", dlName, nil, []string{ReplaceLabel}))
 	return nil
 }