Update address parameters on overrides (#20934)

This fix a bug where default parametrize resolve could not get
overriden.

The fix ensure that an address parameters are updated on any override,
not just parametrized one.

Fixes #20933

docs/notes/2.22.x.md

@@ -25,6 +25,10 @@ If you encounter such discrepancies, and you can't resolve them easily, please [
 
 ### Backends
 
+#### BUILD
+
+Non-parametrized values in `overrides` will now be removed from the target's address parameters. This fixes [a bug](https://github.com/pantsbuild/pants/issues/20933) where a target with parametrized default values would have inconsistent address parameters with its field values.
+
 #### NEW: SQL
 
 A new experimental `SQL` backend was added along with the [sqlfluff

src/python/pants/engine/internals/native_engine.pyi

@@ -224,8 +224,9 @@ class Address:
         ...
     @property
     def path_safe_spec(self) -> str: ...
-    def parametrize(self, parameters: Mapping[str, str]) -> Address:
-        """Creates a new Address with the given `parameters` merged over self.parameters."""
+    def parametrize(self, parameters: Mapping[str, str], replace: bool = False) -> Address:
+        """Creates a new Address with the given `parameters` merged or replaced over
+        self.parameters."""
         ...
     def maybe_convert_to_target_generator(self) -> Address:
         """If this address is generated or parametrized, convert it to its generator target.

src/python/pants/engine/internals/parametrize.py

@@ -166,42 +166,55 @@ def expand(
         except Exception as e:
             raise Exception(f"Failed to parametrize `{address}`:\n  {e}") from e
 
+        parameters = address.parameters
+        non_parametrized = tuple(
+            (field_name, field_value)
+            for field_name, field_value in fields.items()
+            if not isinstance(field_value, Parametrize)
+        )
         if parametrized_groups:
             # Add the groups as one vector for the cross-product.
             parametrized.append(parametrized_groups)
 
+        unparametrize_keys = {k for k, _ in non_parametrized if k in parameters}
+
+        # Remove non-parametrized fields from the address parameters.
+        for k in unparametrize_keys:
+            parameters.pop(k, None)
+
         if not parametrized:
+            if unparametrize_keys:
+                address = address.parametrize(parameters, replace=True)
             yield (address, fields)
             return
 
-        non_parametrized = tuple(
-            (field_name, field_value)
-            for field_name, field_value in fields.items()
-            if not isinstance(field_value, Parametrize)
-        )
-
         for parametrized_args in itertools.product(*parametrized):
-            expanded_address = address.parametrize(
-                {field_name: alias for field_name, alias, _ in parametrized_args}
+            expanded_parameters = parameters | {
+                field_name: alias for field_name, alias, _ in parametrized_args
+            }
+            # There will be at most one group per cross product.
+            group_kwargs: Mapping[str, Any] = next(
+                (
+                    field_value.kwargs
+                    for _, _, field_value in parametrized_args
+                    if isinstance(field_value, Parametrize) and field_value.is_group
+                ),
+                {},
             )
+            # Exclude fields from parametrize group from address parameters.
+            for k in group_kwargs.keys() & parameters.keys():
+                expanded_parameters.pop(k, None)
+
             parametrized_args_fields = tuple(
                 (field_name, field_value)
                 for field_name, _, field_value in parametrized_args
                 # Exclude any parametrize group
                 if not (isinstance(field_value, Parametrize) and field_value.is_group)
             )
             expanded_fields: dict[str, Any] = dict(non_parametrized + parametrized_args_fields)
-            expanded_fields.update(
-                # There will be at most one group per cross product.
-                next(
-                    (
-                        field_value.kwargs
-                        for _, _, field_value in parametrized_args
-                        if isinstance(field_value, Parametrize) and field_value.is_group
-                    ),
-                    {},
-                )
-            )
+            expanded_fields.update(group_kwargs)
+
+            expanded_address = address.parametrize(expanded_parameters, replace=True)
             yield expanded_address, expanded_fields
 
     @staticmethod

src/python/pants/engine/internals/parametrize_test.py

@@ -127,6 +127,59 @@ def test_expand(
     )
 
 
+@pytest.mark.parametrize(
+    "expected,parameters,fields",
+    [
+        # Single parameters remove any existing parameters from the address.
+        ([("a:a", {"f": "1"})], {"f": "0"}, {"f": "1"}),
+        # But keeps other parameters.
+        ([("a@g=0", {"f": "1"})], {"g": "0"}, {"f": "1"}),
+        # Test both case at the same time.
+        ([("a@g=0", {"f": "1"})], {"f": "0", "g": "0"}, {"f": "1"}),
+        # Group parameters remove existing covered parameters.
+        (
+            [
+                ("a@h=0,parametrize=A", {"f": "1", "g": "2", "i": "1"}),
+                ("a@f=0,h=0,parametrize=B", {"g": "1", "i": "1"}),
+            ],
+            {"f": "0", "g": "0", "h": "0", "i": "0"},
+            dict(
+                g="1",
+                i="1",
+                **Parametrize("A", f="1", g="2"),  # type: ignore[arg-type]
+                **Parametrize("B"),
+            ),
+        ),
+        # Re-Parametrize existing parameters
+        (
+            [
+                ("a@f=1,g=1,h=0", {"f": "1", "g": "1"}),
+                ("a@f=1,g=2,h=0", {"f": "1", "g": "2"}),
+            ],
+            {"f": "0", "g": "0", "h": "0"},
+            {
+                "f": Parametrize("1"),
+                "g": Parametrize("1", "2"),
+            },
+        ),
+    ],
+)
+def test_expand_existing_parameters(
+    expected: list[tuple[str, dict[str, Any]]],
+    parameters: dict[str, Any],
+    fields: dict[str, Any | Parametrize],
+) -> None:
+    assert sorted(expected) == sorted(
+        (
+            (address.spec, result_fields)
+            for address, result_fields in Parametrize.expand(
+                Address("a", parameters=parameters), fields
+            )
+        ),
+        key=lambda value: value[0],
+    )
+
+
 @pytest.mark.parametrize(
     "fields, expected_error",
     [

src/rust/engine/src/externs/address.rs

@@ -680,9 +680,15 @@ impl Address {
         Ok(format!("{prefix}{path}{target}{generated}{params}"))
     }
 
-    fn parametrize(&self, parameters: BTreeMap<String, String>) -> Self {
-        let mut merged_parameters = self.parameters.clone();
-        merged_parameters.extend(parameters);
+    #[pyo3(signature = (parameters, replace=false))]
+    fn parametrize(&self, parameters: BTreeMap<String, String>, replace: bool) -> Self {
+        let merged_parameters = if replace {
+            parameters
+        } else {
+            let mut merged_parameters = parameters.clone();
+            merged_parameters.extend(parameters);
+            merged_parameters
+        };
 
         Self {
             spec_path: self.spec_path.clone(),