Skip to content

Commit

Permalink
Consistent type conversion with arg: T = None.
Browse files Browse the repository at this point in the history
Prior to Python 3.11 that syntax was considered same as
`arg: T|None = None` and with unions we don't look at the default
value at all if `T` isn't known.

This commit makes behavior with Python 3.11 consistent with how
conversion works with older Python versions. Fixes #4626.

Might be better not to look at the default values in general if an
argument has type information. That would be a backwards incompatible
change, though, and needs to wait for a major version.
  • Loading branch information
pekkaklarck committed Feb 3, 2023
1 parent 63c82a3 commit d1febc5
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 5 deletions.
5 changes: 4 additions & 1 deletion atest/robot/keywords/type_conversion/annotations.robot
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,10 @@ Invalid kwonly
Return value annotation causes no error
Check Test Case ${TESTNAME}

None as default
None as default with known type
Check Test Case ${TESTNAME}

None as default with unknown type
Check Test Case ${TESTNAME}

Forward references
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ Invalid Set
None as default
Check Test Case ${TESTNAME}

None as default with Any
Check Test Case ${TESTNAME}

Forward references
Check Test Case ${TESTNAME}

Expand Down
4 changes: 4 additions & 0 deletions atest/testdata/keywords/type_conversion/Annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,10 @@ def none_as_default(argument: list = None, expected=None):
_validate_type(argument, expected)


def none_as_default_with_unknown_type(argument: Unknown = None, expected=None):
_validate_type(argument, expected)


def forward_referenced_concrete_type(argument: 'int', expected=None):
_validate_type(argument, expected)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import (Dict, List, Mapping, MutableMapping, MutableSet, MutableSequence,
from typing import (Any, Dict, List, Mapping, MutableMapping, MutableSet, MutableSequence,
Set, Sequence, Tuple, Union)
try:
from typing_extensions import TypedDict
Expand Down Expand Up @@ -117,6 +117,10 @@ def none_as_default(argument: List = None, expected=None):
_validate_type(argument, expected)


def none_as_default_with_any(argument: Any = None, expected=None):
_validate_type(argument, expected)


def forward_reference(argument: 'List', expected=None):
_validate_type(argument, expected)

Expand Down
12 changes: 10 additions & 2 deletions atest/testdata/keywords/type_conversion/annotations.robot
Original file line number Diff line number Diff line change
Expand Up @@ -561,12 +561,20 @@ Invalid kwonly
Return value annotation causes no error
Return value annotation 42 42

None as default
None as default with known type
None as default
None as default [] []

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'

Forward references
[Tags] require-py3.5
Forward referenced concrete type 42 42
Forward referenced ABC [] []

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,15 @@ None as default
None as default
None as default [1, 2, 3, 4] [1, 2, 3, 4]

None as default with Any
[Documentation] `a: Any = None` was same as `a: Any|None = None` prior to Python 3.11.
... With unions we don't look at the default in this case and that
... behavior is preserved for backwards compatiblity.
None as default with Any
None as default with Any hi! 'hi!'
None as default with Any ${42} 42
None as default with Any None 'None'

Forward references
Forward reference [1, 2, 3, 4] [1, 2, 3, 4]
Forward ref with types [1, '2', 3, 4.0] [1, 2, 3, 4]
Expand Down
14 changes: 13 additions & 1 deletion src/robot/running/arguments/argumentconverter.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def _convert(self, name, value):
return converter.convert(name, value)
except ValueError as err:
conversion_error = err
if name in spec.defaults:
if self._convert_based_on_defaults(name, spec, bool(conversion_error)):
converter = TypeConverter.converter_for(type(spec.defaults[name]),
languages=self._languages)
if converter:
Expand All @@ -77,3 +77,15 @@ 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)

0 comments on commit d1febc5

Please sign in to comment.