Consistent handling of unrecognized types in union conversion.

After this change `arg: int|Unrecognized` and `arg: Unrecognized|int` behave the same way. Earlier `int` conversions wasn't attempted in the latter case because the unrecognized type was encountered first. Fixes #4648. After this change `arg: T = None` and `arg: T|None = None` are handled the same way regardless is `T` known or not. That means that it was possible to remove the code needed to make `arg: T = None` behave consistently with different Python versions (#4626).
robotframework · Feb 7, 2023 · c76728e · c76728e
1 parent 1227fc8
commit c76728e
Show file tree

Hide file tree

Showing 11 changed files with 118 additions and 72 deletions.
diff --git a/atest/robot/keywords/type_conversion/unions.robot b/atest/robot/keywords/type_conversion/unions.robot
@@ -30,7 +30,10 @@ Union with item not liking isinstance
 Argument not matching union
     Check Test Case    ${TESTNAME}
 
-Union with custom type
+Union with unrecognized type
+    Check Test Case    ${TESTNAME}
+
+Union with only unrecognized types
     Check Test Case    ${TESTNAME}
 
 Multiple types using tuple
@@ -57,6 +60,12 @@ Avoid unnecessary conversion
 Avoid unnecessary conversion with ABC
     Check Test Case    ${TESTNAME}
 
+Default value type
+    Check Test Case    ${TESTNAME}
+
+Default value type with unrecognized type
+    Check Test Case    ${TESTNAME}
+
 Union with invalid types
     Check Test Case    ${TESTNAME}
 

diff --git a/atest/robot/keywords/type_conversion/unionsugar.robot b/atest/robot/keywords/type_conversion/unionsugar.robot
@@ -31,7 +31,10 @@ Union with item not liking isinstance
 Argument not matching union
     Check Test Case    ${TESTNAME}
 
-Union with custom type
+Union with unrecognized type
+    Check Test Case    ${TESTNAME}
+
+Union with only unrecognized types
     Check Test Case    ${TESTNAME}
 
 Avoid unnecessary conversion

diff --git a/atest/testdata/keywords/type_conversion/annotations.robot b/atest/testdata/keywords/type_conversion/annotations.robot
@@ -563,16 +563,14 @@ Return value annotation causes no error
 
 None as default with known type
     None as default
-    None as default                            []    []
+    None as default                            [1, 2]    [1, 2]
+    None as default                            None      None
 
 None as default with unknown type
-    [Documentation]    `a: T = None` was same as `a: T|None = None` prior to Python 3.11.
-    ...                With unions we don't look at the default if `T` isn't a known type
-    ...                and that behavior is preserved for backwards compatiblity.
     None as default with unknown type
     None as default with unknown type          hi!      'hi!'
     None as default with unknown type          ${42}    42
-    None as default with unknown type          None     'None'
+    None as default with unknown type          None     None
 
 Forward references
     Forward referenced concrete type           42    42

diff --git a/atest/testdata/keywords/type_conversion/conversion.resource b/atest/testdata/keywords/type_conversion/conversion.resource
@@ -1,3 +1,6 @@
+*** Variables ***
+${CUSTOM}         ${{type('Custom', (), {})()}}
+
 *** Keywords ***
 Conversion Should Fail
     [Arguments]    ${kw}    @{args}    ${error}=    ${type}=${kw.lower()}    ${arg_type}=    &{kwargs}

diff --git a/atest/testdata/keywords/type_conversion/unions.py b/atest/testdata/keywords/type_conversion/unions.py
@@ -10,7 +10,7 @@ class MyObject:
     pass
 
 
-class UnexpectedObject:
+class AnotherObject:
     pass
 
 
@@ -27,10 +27,6 @@ def create_my_object():
     return MyObject()
 
 
-def create_unexpected_object():
-    return UnexpectedObject()
-
-
 def union_of_int_float_and_string(argument: Union[int, float, str], expected):
     assert argument == expected
 
@@ -71,8 +67,12 @@ def union_with_item_not_liking_isinstance(argument: Union[BadRational, int], exp
     assert argument == expected, '%r != %r' % (argument, expected)
 
 
-def custom_type_in_union(argument: Union[MyObject, str], expected_type):
-    assert isinstance(argument, eval(expected_type))
+def unrecognized_type(argument: Union[MyObject, str], expected_type):
+    assert type(argument).__name__ == expected_type
+
+
+def only_unrecognized_types(argument: Union[MyObject, AnotherObject], expected_type):
+    assert type(argument).__name__ == expected_type
 
 
 def tuple_of_int_float_and_string(argument: (int, float, str), expected):
@@ -103,6 +103,15 @@ def union_with_string_first(argument: Union[str, None], expected):
     assert argument == expected
 
 
+def incompatible_default(argument: Union[None, int] = 1.1, expected=object()):
+    assert argument == expected
+
+
+def unrecognized_type_with_incompatible_default(argument: Union[MyObject, int] = 1.1,
+                                                expected=object()):
+    assert argument == expected
+
+
 def union_with_invalid_types(argument: Union['nonex', 'references'], expected):
     assert argument == expected
 

diff --git a/atest/testdata/keywords/type_conversion/unions.robot b/atest/testdata/keywords/type_conversion/unions.robot
@@ -24,6 +24,7 @@ Union with None and str
     1          1
     NONE       NONE
     ${2}       ${2}
+    ${2.0}     ${2}
     ${None}    ${None}
     three      three
 
@@ -60,17 +61,26 @@ Argument not matching union
     [Template]    Conversion Should Fail
     Union of int and float             not a number    type=integer or float
     Union of int and float             ${NONE}         type=integer or float    arg_type=None
-    Union of int and float             ${{type('Custom', (), {})()}}
-    ...                                                type=integer or float    arg_type=Custom
+    Union of int and float             ${CUSTOM}       type=integer or float    arg_type=Custom
     Union with int and None            invalid         type=integer or None
+    Union with int and None            ${1.1}          type=integer or None     arg_type=float
     Union with subscripted generics    invalid         type=list or integer
 
-Union with custom type
+Union with unrecognized type
     ${myobject}=    Create my object
-    ${object}=    Create unexpected object
-    Custom type in union    my string      str
-    Custom type in union    ${myobject}    MyObject
-    Custom type in union    ${object}      UnexpectedObject
+    Unrecognized type    my string      str
+    Unrecognized type    ${myobject}    MyObject
+    Unrecognized type    ${42}          str
+    Unrecognized type    ${CUSTOM}      str
+    Unrecognized type    ${{type('StrFails', (), {'__str__': lambda self: 1/0})()}}
+    ...                  StrFails
+
+Union with only unrecognized types
+    ${myobject}=    Create my object
+    Only unrecognized types    my string      str
+    Only unrecognized types    ${myobject}    MyObject
+    Only unrecognized types    ${42}          int
+    Only unrecognized types    ${CUSTOM}      Custom
 
 Multiple types using tuple
     [Template]    Tuple of int float and string
@@ -84,8 +94,7 @@ Argument not matching tuple types
     [Template]    Conversion Should Fail
     Tuple of int and float    not a number    type=integer or float
     Tuple of int and float    ${NONE}         type=integer or float    arg_type=None
-    Tuple of int and float    ${{type('Custom', (), {})()}}
-    ...                                       type=integer or float    arg_type=Custom
+    Tuple of int and float    ${CUSTOM}       type=integer or float    arg_type=Custom
 
 Optional argument
     [Template]    Optional argument
@@ -134,6 +143,16 @@ Avoid unnecessary conversion with ABC
     ${1}                             ${1}
     ${{fractions.Fraction(1, 3)}}    ${{fractions.Fraction(1, 3)}}
 
+Default value type
+    [Documentation]    Default value type is used if conversion fails.
+    Incompatible default    1      ${1}
+    Incompatible default    1.2    ${1.2}
+
+Default value type with unrecognized type
+    [Documentation]    Default value type is never used because conversion cannot fail.
+    Unrecognized type with incompatible default    1      ${1}
+    Unrecognized type with incompatible default    1.2    1.2
+
 Union with invalid types
     [Template]    Union with invalid types
     xxx      xxx

diff --git a/atest/testdata/keywords/type_conversion/unionsugar.py b/atest/testdata/keywords/type_conversion/unionsugar.py
@@ -6,7 +6,7 @@ class MyObject:
     pass
 
 
-class UnexpectedObject:
+class AnotherObject:
     pass
 
 
@@ -23,10 +23,6 @@ def create_my_object():
     return MyObject()
 
 
-def create_unexpected_object():
-    return UnexpectedObject()
-
-
 def union_of_int_float_and_string(argument: int | float | str, expected):
     assert argument == expected
 
@@ -68,7 +64,11 @@ def union_with_item_not_liking_isinstance(argument: BadRational | bool, expected
 
 
 def custom_type_in_union(argument: MyObject | str, expected_type):
-    assert isinstance(argument, eval(expected_type))
+    assert type(argument).__name__ == expected_type
+
+
+def only_custom_types_in_union(argument: MyObject | AnotherObject, expected_type):
+    assert type(argument).__name__ == expected_type
 
 
 def union_with_string_first(argument: str | None, expected):

diff --git a/atest/testdata/keywords/type_conversion/unionsugar.robot b/atest/testdata/keywords/type_conversion/unionsugar.robot
@@ -61,17 +61,23 @@ Argument not matching union
     [Template]    Conversion Should Fail
     Union of int and float             not a number    type=integer or float
     Union of int and float             ${NONE}         type=integer or float    arg_type=None
-    Union of int and float             ${{type('Custom', (), {})()}}
-    ...                                                type=integer or float    arg_type=Custom
+    Union of int and float             ${CUSTOM}       type=integer or float    arg_type=Custom
     Union with int and None            invalid         type=integer or None
     Union with subscripted generics    invalid         type=list or integer
 
-Union with custom type
+Union with unrecognized type
     ${myobject}=    Create my object
-    ${object}=    Create unexpected object
     Custom type in union    my string      str
     Custom type in union    ${myobject}    MyObject
-    Custom type in union    ${object}      UnexpectedObject
+    Custom type in union    ${42}          str
+    Custom type in union    ${CUSTOM}      str
+
+Union with only unrecognized types
+    ${myobject}=    Create my object
+    Only custom types in union    my string      str
+    Only custom types in union    ${myobject}    MyObject
+    Only custom types in union    ${42}          int
+    Only custom types in union    ${CUSTOM}      Custom
 
 Avoid unnecessary conversion
     [Template]    Union With String First

diff --git a/doc/userguide/src/ExtendingRobotFramework/CreatingTestLibraries.rst b/doc/userguide/src/ExtendingRobotFramework/CreatingTestLibraries.rst
@@ -1361,7 +1361,7 @@ has multiple possible types. In this situation argument conversion is attempted
 based on each type and the whole conversion fails if none of these conversions
 succeed.
 
-When using function annotations, the natural syntax to specify that argument
+When using function annotations, the natural syntax to specify that an argument
 has multiple possible types is using Union_:
 
 .. sourcecode:: python
@@ -1370,16 +1370,15 @@ has multiple possible types is using Union_:
 
 
   def example(length: Union[int, float], padding: Union[int, str, None] = None):
-      # ...
+      ...
 
-When using Python 3.10 or newer, it is possible to use the native `type1 | type2`
+When using Python 3.10 or newer, it is possible to use the native `type1 | type2`__
 syntax instead:
 
 .. sourcecode:: python
 
   def example(length: int | float, padding: int | str | None = None):
-      # ...
-
+      ...
 
 An alternative is specifying types as a tuple. It is not recommended with annotations,
 because that syntax is not supported by other tools, but it works well with
@@ -1392,7 +1391,7 @@ the `@keyword` decorator:
 
   @keyword(types={'length': (int, float), 'padding': (int, str, None)})
   def example(length, padding=None):
-      # ...
+      ...
 
 With the above examples the `length` argument would first be converted to an
 integer and if that fails then to a float. The `padding` would be first
@@ -1433,21 +1432,22 @@ attempted in the order types are specified. If any conversion succeeds, the
 resulting value is used without attempting remaining conversions. If no individual
 conversion succeeds, the whole conversion fails.
 
-If a specified type is not recognized by Robot Framework, then the original value
-is used as-is. For example, with this keyword conversion would first be attempted
-to an integer but if that fails the keyword would get the original given argument:
+If a specified type is not recognized by Robot Framework, then the original argument
+value is used as-is. For example, with this keyword conversion would first be attempted
+to an integer, but if that fails the keyword would get the original argument:
 
 .. sourcecode:: python
 
-  def example(argument: Union[int, MyCustomType]):
-      # ...
+  def example(argument: Union[int, Unrecognized]):
+      ...
 
-.. note:: In Robot Framework 4.0 argument conversion was done always, regardless
-          of the type of the given argument. It caused various__ problems__ and
-          was changed in Robot Framework 4.0.1.
+Starting from Robot Framework 6.1, the above logic works also if an unrecognized
+type is listed before a recognized type like `Union[Unrecognized, int]`.
+Also in this case `int` conversion is attempted, and the argument id passed as-is
+if it fails. With earlier Robot Framework versions, `int` conversion would not be
+attempted at all.
 
-__ https://github.com/robotframework/robotframework/issues/3897
-__ https://github.com/robotframework/robotframework/issues/3908
+__ https://peps.python.org/pep-0604/
 .. _Union: https://docs.python.org/3/library/typing.html#typing.Union
 
 Type conversion with generics

diff --git a/src/robot/running/arguments/argumentconverter.py b/src/robot/running/arguments/argumentconverter.py
@@ -65,7 +65,7 @@ def _convert(self, name, value):
                     return converter.convert(name, value)
                 except ValueError as err:
                     conversion_error = err
-        if self._convert_based_on_defaults(name, spec, bool(conversion_error)):
+        if name in spec.defaults:
             converter = TypeConverter.converter_for(type(spec.defaults[name]),
                                                     languages=self._languages)
             if converter:
@@ -77,15 +77,3 @@ def _convert(self, name, value):
         if conversion_error:
             raise conversion_error
         return value
-
-    def _convert_based_on_defaults(self, name, spec, has_known_type):
-        if name not in spec.defaults:
-            return False
-        # Handle `arg: T = None` consistently with different Python versions
-        # regardless is `T` a known type or not. Prior to 3.11 this syntax was
-        # considered same as `arg: Union[T, None] = None` and with unions we
-        # don't look at the possible default value if `T` is not known.
-        # https://github.com/robotframework/robotframework/issues/4626
-        return (name not in spec.types
-                or spec.defaults[name] is not None
-                or has_known_type)
diff --git a/src/robot/running/arguments/typeconverters.py b/src/robot/running/arguments/typeconverters.py
@@ -682,21 +682,32 @@ def _handles_value(self, value):
         return True
 
     def no_conversion_needed(self, value):
-        for converter in self.converters:
-            if converter and converter.no_conversion_needed(value):
-                return True
+        for converter, type_ in zip(self.converters, self.used_type):
+            if converter:
+                if converter.no_conversion_needed(value):
+                    return True
+            else:
+                try:
+                    if isinstance(value, type_):
+                        return True
+                except TypeError:
+                    pass
         return False
 
     def _convert(self, value, explicit_type=True):
         if not self.used_type:
             raise ValueError('Cannot have union without types.')
+        unrecognized_types = False
         for converter in self.converters:
-            if not converter:
-                return value
-            try:
-                return converter.convert('', value, explicit_type)
-            except ValueError:
-                pass
+            if converter:
+                try:
+                    return converter.convert('', value, explicit_type)
+                except ValueError:
+                    pass
+            else:
+                unrecognized_types = True
+        if unrecognized_types:
+            return value
         raise ValueError