fix(api): raise an error if protocol's defined parameters have duplic…

…ate variable names (#14888)

api/src/opentrons/protocol_api/_parameter_context.py

@@ -52,6 +52,7 @@ def add_int(
             description: A description of the parameter as it will show up on the frontend.
             unit: An optional unit to be appended to the end of the integer as it shown on the frontend.
         """
+        validation.validate_variable_name_unique(variable_name, set(self._parameters))
         parameter = parameter_definition.create_int_parameter(
             display_name=display_name,
             variable_name=variable_name,
@@ -88,6 +89,7 @@ def add_float(
             description: A description of the parameter as it will show up on the frontend.
             unit: An optional unit to be appended to the end of the float as it shown on the frontend.
         """
+        validation.validate_variable_name_unique(variable_name, set(self._parameters))
         parameter = parameter_definition.create_float_parameter(
             display_name=display_name,
             variable_name=variable_name,
@@ -115,6 +117,7 @@ def add_bool(
             default: The default value the boolean parameter will be set to. This will be used in initial analysis.
             description: A description of the parameter as it will show up on the frontend.
         """
+        validation.validate_variable_name_unique(variable_name, set(self._parameters))
         parameter = parameter_definition.create_bool_parameter(
             display_name=display_name,
             variable_name=variable_name,
@@ -145,6 +148,7 @@ def add_str(
                 Mutually exclusive with minimum and maximum.
             description: A description of the parameter as it will show up on the frontend.
         """
+        validation.validate_variable_name_unique(variable_name, set(self._parameters))
         parameter = parameter_definition.create_str_parameter(
             display_name=display_name,
             variable_name=variable_name,

api/src/opentrons/protocols/parameters/validation.py

@@ -1,5 +1,5 @@
 import keyword
-from typing import List, Optional, Union, Literal
+from typing import List, Set, Optional, Union, Literal
 
 from .types import (
     AllowedTypes,
@@ -16,6 +16,17 @@
 DESCRIPTION_MAX_LEN = 100
 
 
+def validate_variable_name_unique(
+    variable_name: str, other_variable_names: Set[str]
+) -> None:
+    """Validate that the given variable name is unique."""
+    if variable_name in other_variable_names:
+        raise ParameterNameError(
+            f'"{variable_name}" is already defined as a variable name for another parameter.'
+            f" All variable names must be unique."
+        )
+
+
 def ensure_display_name(display_name: str) -> str:
     """Validate display name is within the character limit."""
     if len(display_name) > DISPLAY_NAME_MAX_LEN:

api/tests/opentrons/protocol_api/test_parameter_context.py

@@ -46,6 +46,9 @@ def subject(api_version: APIVersion) -> ParameterContext:
 
 def test_add_int(decoy: Decoy, subject: ParameterContext) -> None:
     """It should create and add an int parameter definition."""
+    subject._parameters["other_param"] = decoy.mock(
+        cls=mock_parameter_definition.ParameterDefinition
+    )
     param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition)
     decoy.when(param_def.variable_name).then_return("my cool variable")
     decoy.when(
@@ -60,6 +63,7 @@ def test_add_int(decoy: Decoy, subject: ParameterContext) -> None:
             unit="foot candles",
         )
     ).then_return(param_def)
+
     subject.add_int(
         display_name="abc",
         variable_name="xyz",
@@ -70,11 +74,16 @@ def test_add_int(decoy: Decoy, subject: ParameterContext) -> None:
         description="blah blah blah",
         unit="foot candles",
     )
+
     assert param_def is subject._parameters["my cool variable"]
+    decoy.verify(mock_validation.validate_variable_name_unique("xyz", {"other_param"}))
 
 
 def test_add_float(decoy: Decoy, subject: ParameterContext) -> None:
     """It should create and add a float parameter definition."""
+    subject._parameters["other_param"] = decoy.mock(
+        cls=mock_parameter_definition.ParameterDefinition
+    )
     param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition)
     decoy.when(param_def.variable_name).then_return("my cooler variable")
     decoy.when(mock_validation.ensure_float_value(12.3)).then_return(3.21)
@@ -83,7 +92,6 @@ def test_add_float(decoy: Decoy, subject: ParameterContext) -> None:
     decoy.when(
         mock_validation.ensure_float_choices([{"display_name": "foo", "value": 4.2}])
     ).then_return([{"display_name": "bar", "value": 2.4}])
-
     decoy.when(
         mock_parameter_definition.create_float_parameter(
             display_name="abc",
@@ -96,6 +104,7 @@ def test_add_float(decoy: Decoy, subject: ParameterContext) -> None:
             unit="lux",
         )
     ).then_return(param_def)
+
     subject.add_float(
         display_name="abc",
         variable_name="xyz",
@@ -106,11 +115,16 @@ def test_add_float(decoy: Decoy, subject: ParameterContext) -> None:
         description="blah blah blah",
         unit="lux",
     )
+
     assert param_def is subject._parameters["my cooler variable"]
+    decoy.verify(mock_validation.validate_variable_name_unique("xyz", {"other_param"}))
 
 
 def test_add_bool(decoy: Decoy, subject: ParameterContext) -> None:
     """It should create and add a boolean parameter definition."""
+    subject._parameters["other_param"] = decoy.mock(
+        cls=mock_parameter_definition.ParameterDefinition
+    )
     param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition)
     decoy.when(param_def.variable_name).then_return("my coolest variable")
     decoy.when(
@@ -125,17 +139,23 @@ def test_add_bool(decoy: Decoy, subject: ParameterContext) -> None:
             description="lorem ipsum",
         )
     ).then_return(param_def)
+
     subject.add_bool(
         display_name="cba",
         variable_name="zxy",
         default=False,
         description="lorem ipsum",
     )
+
     assert param_def is subject._parameters["my coolest variable"]
+    decoy.verify(mock_validation.validate_variable_name_unique("zxy", {"other_param"}))
 
 
 def test_add_string(decoy: Decoy, subject: ParameterContext) -> None:
     """It should create and add a string parameter definition."""
+    subject._parameters["other_param"] = decoy.mock(
+        cls=mock_parameter_definition.ParameterDefinition
+    )
     param_def = decoy.mock(cls=mock_parameter_definition.ParameterDefinition)
     decoy.when(param_def.variable_name).then_return("my slightly less cool variable")
     decoy.when(
@@ -147,14 +167,19 @@ def test_add_string(decoy: Decoy, subject: ParameterContext) -> None:
             description="fee foo fum",
         )
     ).then_return(param_def)
+
     subject.add_str(
         display_name="jkl",
         variable_name="qwerty",
         default="asdf",
         choices=[{"display_name": "bar", "value": "aaa"}],
         description="fee foo fum",
     )
+
     assert param_def is subject._parameters["my slightly less cool variable"]
+    decoy.verify(
+        mock_validation.validate_variable_name_unique("qwerty", {"other_param"})
+    )
 
 
 def test_set_parameters(decoy: Decoy, subject: ParameterContext) -> None:
@@ -200,5 +225,5 @@ def test_export_parameters_for_protocol(
     subject._parameters = {"foo": param_def_1, "bar": param_def_2}
 
     result = subject.export_parameters_for_protocol()
-    assert result.x == "a"  # type: ignore [attr-defined]
-    assert result.y == 1.23  # type: ignore [attr-defined]
+    assert result.x == "a"  # type: ignore[attr-defined]
+    assert result.y == 1.23  # type: ignore[attr-defined]

api/tests/opentrons/protocols/parameters/test_validation.py

@@ -12,6 +12,13 @@
 from opentrons.protocols.parameters import validation as subject
 
 
+def test_validate_variable_name_unique() -> None:
+    """It should no-op if the name is unique and raise if it is not."""
+    subject.validate_variable_name_unique("one of a kind", {"fee", "foo", "fum"})
+    with pytest.raises(ParameterNameError):
+        subject.validate_variable_name_unique("copy", {"paste", "copy", "cut"})
+
+
 def test_ensure_display_name() -> None:
     """It should ensure the display name is within the character limit."""
     result = subject.ensure_display_name("abc")