Add wildcard support for key in dict_replace_if_equal

[mering]

No description provided.

[oreflow]

It feels like iterating over key/value pairs here, identifying the key, and then doing DictionarySet, which uses the key to identify the key/value pair, gets a bit redundant.

Additionally, the implementation here introduces some edge cases, like if called on an empty dict, or a dict where the keys are not StringExpr, it would currently return error, where as I believe the assumption would be that when using key = "*", this would just match any types or value as key?

it might be simpler to just put this entire implementation in a single loop like this. Haven't fully tested it but I believe this should be consistent

	// If not using wildcard, missing key is considered an error.
	if prev := DictionaryGet(dictAttr, key); key != "*" && prev == nil {
		return nil, fmt.Errorf("key '%s' not found in dict", key)
	}

	for _, kv := range dictAttr.List {
		if k, ok := kv.Key.(*build.StringExpr); key == "*" || (ok && k.Value == key) {
			// Key-Value Pair matches key.
			if val, ok := kv.Value.(*build.StringExpr); ok {
				if labels.Equal(val.Value, oldV, env.Pkg) {
					kv.Value = getStringExpr(newV, env.Pkg)
				}
			} else if val, ok := kv.Value.(*build.Ident); ok {
				if val.Name == oldV {
					kv.Value = getStringExpr(newV, env.Pkg)
				}
			}
		}
	}

[oreflow]

Can we add a test case which verifies the new behaviour?

[oreflow]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds wildcard support for the key in dict_replace_if_equal. The implementation is mostly correct, and the accompanying documentation update is clear. I've identified a couple of areas for improvement: one is a correctness issue regarding the handling of wildcards when no keys match, and the other is a small refactoring to remove redundant code. Overall, a good addition.

[gemini-code-assist]

When using a wildcard * for the key, it shouldn't be considered an error if no keys are found in the dictionary (e.g., if the dictionary is empty or contains no string keys). The command should simply be a no-op in this case. This is consistent with how wildcards are handled in other commands like remove. The current implementation returns an error.

if len(keyValues) == 0 {
	if key != "*" {
		return nil, fmt.Errorf("key '%s' not found in dict", key)
	}
	return env.File, nil
}

[gemini-code-assist]

The check for kv.Key being a *build.StringExpr inside this loop is redundant, as the loop at lines 771-782 already ensures this. You can simplify this loop by removing the check and using a direct type assertion.

for _, kv := range keyValues {
	k := kv.Key.(*build.StringExpr)
	if e, ok := kv.Value.(*build.StringExpr); ok {
		if labels.Equal(e.Value, oldV, env.Pkg) {
			DictionarySet(dictAttr, k.Value, getStringExpr(newV, env.Pkg))
		}
	} else if e, ok := kv.Value.(*build.Ident); ok {
		if e.Name == oldV {
			DictionarySet(dictAttr, k.Value, getStringExpr(newV, env.Pkg))
		}
	}
}