Implement mapping replacement logic for use_repo_add

When using use_repo_add, the behavior now depends on what is being added:

- Adding a positional argument (e.g., "foo") that already exists as a
  mapping (e.g., my_foo = "foo") will preserve the existing mapping
  instead of adding a duplicate or converting it to positional.

- Adding a mapping (e.g., bar=baz) that conflicts with an existing
  mapping will replace it:
  - Same key, different value: foo=bar + foo=baz -> foo=baz
  - Different key, same value: foo=bar + qux=bar -> qux=bar

This allows users to explicitly rename or remap repositories while
preserving user-defined mappings when they're not explicitly changed.

Author: malt3

edit/bzlmod/bzlmod.go

@@ -125,6 +125,7 @@ func NewUseRepo(f *build.File, proxies []string) (*build.File, *build.CallExpr)
 // useRepos must not be empty.
 // Keyword arguments are preserved and can be added using the syntax "key=value".
 // For invalid identifiers (e.g., containing dots), dict unpacking syntax is used.
+// If a mapping conflicts with an existing one (same key or same value), the existing one is replaced.
 func AddRepoUsages(useRepos []*build.CallExpr, repos ...string) {
 	if len(repos) == 0 {
 		return
@@ -133,43 +134,153 @@ func AddRepoUsages(useRepos []*build.CallExpr, repos ...string) {
 		panic("useRepos must not be empty")
 	}
 
-	seen := make(map[string]struct{})
+	// Parse all repos to add and track which keys/values are being added
+	type repoToAdd struct {
+		key             string
+		value           string
+		isMapping       bool
+		isValidIdent    bool
+		isDictUnpacking bool
+	}
+
+	var reposToAdd []repoToAdd
+	mappingValuesToAdd := make(map[string]struct{})  // Values being added as mappings
+	positionalValuesToAdd := make(map[string]struct{}) // Values being added as positional
+	keysToAdd := make(map[string]struct{})
+
+	for _, repo := range repos {
+		key, value, isMapping := parseRepoMapping(repo)
+		validIdent := isValidIdentifier(key)
+
+		reposToAdd = append(reposToAdd, repoToAdd{
+			key:             key,
+			value:           value,
+			isMapping:       isMapping,
+			isValidIdent:    validIdent,
+			isDictUnpacking: isMapping && !validIdent,
+		})
+
+		if isMapping {
+			mappingValuesToAdd[value] = struct{}{}
+			keysToAdd[key] = struct{}{}
+		} else {
+			positionalValuesToAdd[value] = struct{}{}
+		}
+	}
+
+	// Track existing values to detect duplicates when adding positional args
+	existingValues := make(map[string]struct{})
 	for _, useRepo := range useRepos {
 		if len(useRepo.List) == 0 {
-			// Invalid use_repo call, skip.
 			continue
 		}
 		for _, arg := range useRepo.List[1:] {
-			seen[repoFromUseRepoArg(arg)] = struct{}{}
+			if val := repoFromUseRepoArg(arg); val != "" {
+				existingValues[val] = struct{}{}
+			}
 		}
 	}
 
-	lastUseRepo := getLastUseRepo(useRepos)
-	var dictEntries []*build.KeyValueExpr
+	// Remove conflicting arguments from all use_repo calls
+	for _, useRepo := range useRepos {
+		if len(useRepo.List) == 0 {
+			continue
+		}
 
-	for _, repo := range repos {
-		key, value, isMapping := parseRepoMapping(repo)
+		var newArgs []build.Expr
+		var dictToUpdate *build.DictExpr
 
-		// Check if already exists
-		if _, ok := seen[value]; ok {
-			continue
+		for _, arg := range useRepo.List[1:] {
+			shouldKeep := true
+
+			switch arg := arg.(type) {
+			case *build.StringExpr:
+				// Positional argument: only remove if a mapping is being added with this value
+				if _, conflicts := mappingValuesToAdd[arg.Value]; conflicts {
+					shouldKeep = false
+				}
+			case *build.AssignExpr:
+				// Keyword argument: remove if same key is being added
+				if ident, ok := arg.LHS.(*build.Ident); ok {
+					if _, conflicts := keysToAdd[ident.Name]; conflicts {
+						shouldKeep = false
+					}
+				}
+				// Or if same value is being added as a mapping (replacement)
+				if str, ok := arg.RHS.(*build.StringExpr); ok {
+					if _, conflicts := mappingValuesToAdd[str.Value]; conflicts {
+						shouldKeep = false
+					}
+				}
+			case *build.UnaryExpr:
+				// Dict unpacking: we'll handle this separately
+				if arg.Op == "**" {
+					if dict, ok := arg.X.(*build.DictExpr); ok {
+						dictToUpdate = dict
+						shouldKeep = false // Remove from list, we'll re-add after filtering
+					}
+				}
+			}
+
+			if shouldKeep {
+				newArgs = append(newArgs, arg)
+			}
 		}
 
-		if !isMapping {
+		// Filter dict entries if we have a dict to update
+		if dictToUpdate != nil {
+			var filteredDictEntries []*build.KeyValueExpr
+			for _, kv := range dictToUpdate.List {
+				shouldKeep := true
+				// Check if key conflicts
+				if keyStr, ok := kv.Key.(*build.StringExpr); ok {
+					if _, conflicts := keysToAdd[keyStr.Value]; conflicts {
+						shouldKeep = false
+					}
+				}
+				// Check if value conflicts with a mapping being added
+				if valStr, ok := kv.Value.(*build.StringExpr); ok {
+					if _, conflicts := mappingValuesToAdd[valStr.Value]; conflicts {
+						shouldKeep = false
+					}
+				}
+				if shouldKeep {
+					filteredDictEntries = append(filteredDictEntries, kv)
+				}
+			}
+			// Only keep dict if it still has entries after filtering
+			if len(filteredDictEntries) > 0 {
+				dictToUpdate.List = filteredDictEntries
+				newArgs = append(newArgs, &build.UnaryExpr{Op: "**", X: dictToUpdate})
+			}
+		}
+
+		useRepo.List = append(useRepo.List[:1], newArgs...)
+	}
+
+	// Add new repos to the last use_repo call
+	lastUseRepo := getLastUseRepo(useRepos)
+	var dictEntries []*build.KeyValueExpr
+
+	for _, repo := range reposToAdd {
+		if !repo.isMapping {
 			// Simple positional argument: use_repo(ext, "foo")
-			lastUseRepo.List = append(lastUseRepo.List, &build.StringExpr{Value: repo})
-		} else if isValidIdentifier(key) {
+			// Skip if already exists (to preserve existing mappings)
+			if _, exists := existingValues[repo.value]; !exists {
+				lastUseRepo.List = append(lastUseRepo.List, &build.StringExpr{Value: repo.value})
+			}
+		} else if repo.isValidIdent {
 			// Valid identifier as keyword argument: use_repo(ext, bar = "baz")
 			lastUseRepo.List = append(lastUseRepo.List, &build.AssignExpr{
-				LHS: &build.Ident{Name: key},
+				LHS: &build.Ident{Name: repo.key},
 				Op:  "=",
-				RHS: &build.StringExpr{Value: value},
+				RHS: &build.StringExpr{Value: repo.value},
 			})
 		} else {
 			// Invalid identifier, need dict unpacking: use_repo(ext, **{"foo.2": "foo"})
 			dictEntries = append(dictEntries, &build.KeyValueExpr{
-				Key:   &build.StringExpr{Value: key},
-				Value: &build.StringExpr{Value: value},
+				Key:   &build.StringExpr{Value: repo.key},
+				Value: &build.StringExpr{Value: repo.value},
 			})
 		}
 	}

edit/bzlmod/bzlmod_test.go

@@ -502,7 +502,7 @@ use_repo(prox, "repo3", "repo4")
 		{
 			`use_repo(prox, my_repo = "actual")`,
 			[]string{"my_other=actual", "new=other"},
-			`use_repo(prox, my_repo = "actual", new = "other")
+			`use_repo(prox, my_other = "actual", new = "other")
 `,
 		},
 		// Test that existing mappings are preserved when adding the same underlying repo
@@ -522,6 +522,31 @@ use_repo(prox, "repo3", "repo4")
 			`use_repo(ext, my_mapping = "actual_repo", "existing")`,
 			[]string{"actual_repo", "new_repo"},
 			`use_repo(ext, "existing", "new_repo", my_mapping = "actual_repo")
+`,
+		},
+		// Test that mappings can be replaced/renamed
+		{
+			`use_repo(proxy, foo = "bar")`,
+			[]string{"foo=baz"},
+			`use_repo(proxy, foo = "baz")
+`,
+		},
+		{
+			`use_repo(proxy, foo = "bar")`,
+			[]string{"qux=bar"},
+			`use_repo(proxy, qux = "bar")
+`,
+		},
+		{
+			`use_repo(proxy, foo = "bar", "other")`,
+			[]string{"foo=baz"},
+			`use_repo(proxy, "other", foo = "baz")
+`,
+		},
+		{
+			`use_repo(proxy, foo = "bar", "other")`,
+			[]string{"qux=bar", "new"},
+			`use_repo(proxy, "other", "new", qux = "bar")
 `,
 		},
 	} {