typed_kwargs: Fix when ContainerTypeInfo is used in a tuple

info.types could be a tuple like (str, ContainerTypeInfo()). That means
we have to check types one by one and only print error if none of them
matched.

Also fix the case when default value is None for a container type, it
should leave the value to None to be able to distinguish between unset
and empty list.

2 files changed, 89 insertions, 39 deletions

diff --git a/mesonbuild/interpreterbase/decorators.py b/mesonbuild/interpreterbase/decorators.py
index 4c8d824..54a4960 100644
--- a/mesonbuild/interpreterbase/decorators.py
+++ b/mesonbuild/interpreterbase/decorators.py
@@ -22,6 +22,7 @@ from ._unholder import _unholder
 from functools import wraps
 import abc
 import itertools
+import copy
 import typing as T
 if T.TYPE_CHECKING:
     from .. import mparser
@@ -302,28 +303,40 @@ class ContainerTypeInfo:
         self.pairs = pairs
         self.allow_empty = allow_empty
 
-    def check(self, value: T.Any) -> T.Optional[str]:
+    def check(self, value: T.Any) -> bool:
         """Check that a value is valid.
 
         :param value: A value to check
-        :return: If there is an error then a string message, otherwise None
+        :return: True if it is valid, False otherwise
         """
         if not isinstance(value, self.container):
-            return f'container type was "{type(value).__name__}", but should have been "{self.container.__name__}"'
+            return False
         iter_ = iter(value.values()) if isinstance(value, dict) else iter(value)
         for each in iter_:
             if not isinstance(each, self.contains):
-                if isinstance(self.contains, tuple):
-                    shouldbe = 'one of: {}'.format(", ".join(f'"{t.__name__}"' for t in self.contains))
-                else:
-                    shouldbe = f'"{self.contains.__name__}"'
-                return f'contained a value of type "{type(each).__name__}" but should have been {shouldbe}'
+                return False
         if self.pairs and len(value) % 2 != 0:
-            return 'container should be of even length, but is not'
+            return False
         if not value and not self.allow_empty:
-            return 'container is empty, but not allowed to be'
-        return None
+            return False
+        return True
+
+    def description(self) -> str:
+        """Human readable description of this container type.
 
+        :return: string to be printed
+        """
+        container = 'dict' if self.container is dict else 'list'
+        if isinstance(self.contains, tuple):
+            contains = ','.join([t.__name__ for t in self.contains])
+        else:
+            contains = self.contains.__name__
+        s = f'{container}[{contains}]'
+        if self.pairs:
+            s += ' that has even size'
+        if not self.allow_empty:
+            s += ' that cannot be empty'
+        return s
 
 _T = T.TypeVar('_T')
 
@@ -365,8 +378,8 @@ class KwargInfo(T.Generic[_T]):
     :param not_set_warning: A warning messsage that is logged if the kwarg is not
         set by the user.
     """
-
-    def __init__(self, name: str, types: T.Union[T.Type[_T], T.Tuple[T.Type[_T], ...], ContainerTypeInfo],
+    def __init__(self, name: str,
+                 types: T.Union[T.Type[_T], T.Tuple[T.Union[T.Type[_T], ContainerTypeInfo], ...], ContainerTypeInfo],
                  *, required: bool = False, listify: bool = False,
                  default: T.Optional[_T] = None,
                  since: T.Optional[str] = None,
@@ -456,6 +469,25 @@ def typed_kwargs(name: str, *types: KwargInfo) -> T.Callable[..., T.Any]:
                 raise InvalidArguments(f'{name} got unknown keyword arguments {ustr}')
 
             for info in types:
+                types_tuple = info.types if isinstance(info.types, tuple) else (info.types,)
+                def check_value_type(value: T.Any) -> bool:
+                    for t in types_tuple:
+                        if isinstance(t, ContainerTypeInfo):
+                            if t.check(value):
+                                return True
+                        elif isinstance(value, t):
+                            return True
+                    return False
+                def types_description() -> str:
+                    candidates = []
+                    for t in types_tuple:
+                        if isinstance(t, ContainerTypeInfo):
+                            candidates.append(t.description())
+                        else:
+                            candidates.append(t.__name__)
+                    shouldbe = 'one of: ' if len(candidates) > 1 else ''
+                    shouldbe += ', '.join(candidates)
+                    return shouldbe
                 value = kwargs.get(info.name)
                 if value is not None:
                     if info.since:
@@ -466,17 +498,9 @@ def typed_kwargs(name: str, *types: KwargInfo) -> T.Callable[..., T.Any]:
                         FeatureDeprecated.single_use(feature_name, info.deprecated, subproject)
                     if info.listify:
                         kwargs[info.name] = value = mesonlib.listify(value)
-                    if isinstance(info.types, ContainerTypeInfo):
-                        msg = info.types.check(value)
-                        if msg is not None:
-                            raise InvalidArguments(f'{name} keyword argument "{info.name}" {msg}')
-                    else:
-                        if not isinstance(value, info.types):
-                            if isinstance(info.types, tuple):
-                                shouldbe = 'one of: {}'.format(", ".join(f'"{t.__name__}"' for t in info.types))
-                            else:
-                                shouldbe = f'"{info.types.__name__}"'
-                            raise InvalidArguments(f'{name} keyword argument "{info.name}"" was of type "{type(value).__name__}" but should have been {shouldbe}')
+                    if not check_value_type(value):
+                        shouldbe = types_description()
+                        raise InvalidArguments(f'{name} keyword argument {info.name!r} was of type {type(value).__name__!r} but should have been {shouldbe}')
 
                     if info.validator is not None:
                         msg = info.validator(value)
@@ -509,17 +533,10 @@ def typed_kwargs(name: str, *types: KwargInfo) -> T.Callable[..., T.Any]:
                 else:
                     # set the value to the default, this ensuring all kwargs are present
                     # This both simplifies the typing checking and the usage
-                    # Create a shallow copy of the container (and do a type
-                    # conversion if necessary). This allows mutable types to
-                    # be used safely as default values
-                    if isinstance(info.types, ContainerTypeInfo):
-                        assert isinstance(info.default, info.types.container), f'In function {name} default value of {info.name} is not a valid type, got {type(info.default)}, expected {info.types.container}[{info.types.contains}]'
-                        for item in info.default:
-                            assert isinstance(item, info.types.contains), f'In function {name} default value of {info.name}, container has invalid value of {item}, which is of type {type(item)}, but should be {info.types.contains}'
-                        kwargs[info.name] = info.types.container(info.default)
-                    else:
-                        assert isinstance(info.default, info.types), f'In funcion {name} default value of {info.name} is not a valid type, got {type(info.default)} expected {info.types}'
-                        kwargs[info.name] = info.default
+                    assert check_value_type(info.default), f'In funcion {name} default value of {info.name} is not a valid type, got {type(info.default)} expected {types_description()}'
+                    # Create a shallow copy of the container. This allows mutable
+                    # types to be used safely as default values
+                    kwargs[info.name] = copy.copy(info.default)
                     if info.not_set_warning:
                         mlog.warning(info.not_set_warning)
 
diff --git a/unittests/internaltests.py b/unittests/internaltests.py
index 957f180..cb50f37 100644
--- a/unittests/internaltests.py
+++ b/unittests/internaltests.py
@@ -42,7 +42,7 @@ from mesonbuild.mesonlib import (
     LibType, MachineChoice, PerMachine, Version, is_windows, is_osx,
     is_cygwin, is_openbsd, search_version, MesonException, OptionKey,
 )
-from mesonbuild.interpreter.type_checking import in_set_validator
+from mesonbuild.interpreter.type_checking import in_set_validator, NoneType
 from mesonbuild.dependencies import PkgConfigDependency
 from mesonbuild.programs import ExternalProgram
 import mesonbuild.modules.pkgconfig
@@ -1266,7 +1266,7 @@ class InternalTests(unittest.TestCase):
 
         with self.assertRaises(InvalidArguments) as cm:
             _(None, mock.Mock(), [], {'input': {}})
-        self.assertEqual(str(cm.exception), 'testfunc keyword argument "input" container type was "dict", but should have been "list"')
+        self.assertEqual(str(cm.exception), "testfunc keyword argument 'input' was of type 'dict' but should have been list[str]")
 
     def test_typed_kwarg_contained_invalid(self) -> None:
         @typed_kwargs(
@@ -1278,7 +1278,7 @@ class InternalTests(unittest.TestCase):
 
         with self.assertRaises(InvalidArguments) as cm:
             _(None, mock.Mock(), [], {'input': {'key': 1}})
-        self.assertEqual(str(cm.exception), 'testfunc keyword argument "input" contained a value of type "int" but should have been "str"')
+        self.assertEqual(str(cm.exception), "testfunc keyword argument 'input' was of type 'dict' but should have been dict[str]")
 
     def test_typed_kwarg_container_listify(self) -> None:
         @typed_kwargs(
@@ -1313,7 +1313,7 @@ class InternalTests(unittest.TestCase):
 
         with self.assertRaises(MesonException) as cm:
             _(None, mock.Mock(), [], {'input': ['a']})
-        self.assertEqual(str(cm.exception), "testfunc keyword argument \"input\" container should be of even length, but is not")
+        self.assertEqual(str(cm.exception), "testfunc keyword argument 'input' was of type 'list' but should have been list[str] that has even size")
 
     @mock.patch.dict(mesonbuild.mesonlib.project_meson_versions, {})
     def test_typed_kwarg_since(self) -> None:
@@ -1425,6 +1425,39 @@ class InternalTests(unittest.TestCase):
         self.assertEqual(k.default, 'foo')
         self.assertEqual(v.default, 'bar')
 
+    def test_typed_kwarg_default_type(self) -> None:
+        @typed_kwargs(
+            'testfunc',
+            KwargInfo('no_default', (str, ContainerTypeInfo(list, str), NoneType)),
+            KwargInfo('str_default', (str, ContainerTypeInfo(list, str)), default=''),
+            KwargInfo('list_default', (str, ContainerTypeInfo(list, str)), default=['']),
+        )
+        def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, str]) -> None:
+            self.assertEqual(kwargs['no_default'], None)
+            self.assertEqual(kwargs['str_default'], '')
+            self.assertEqual(kwargs['list_default'], [''])
+        _(None, mock.Mock(), [], {})
+
+    def test_typed_kwarg_invalid_default_type(self) -> None:
+        @typed_kwargs(
+            'testfunc',
+            KwargInfo('invalid_default', (str, ContainerTypeInfo(list, str), NoneType), default=42),
+        )
+        def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, str]) -> None:
+            pass
+        self.assertRaises(AssertionError, _, None, mock.Mock(), [], {})
+
+    def test_typed_kwarg_container_in_tuple(self) -> None:
+        @typed_kwargs(
+            'testfunc',
+            KwargInfo('input', (str, ContainerTypeInfo(list, str))),
+        )
+        def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, str]) -> None:
+            self.assertEqual(kwargs['input'], args[0])
+        _(None, mock.Mock(), [''], {'input': ''})
+        _(None, mock.Mock(), [['']], {'input': ['']})
+        self.assertRaises(InvalidArguments, _, None, mock.Mock(), [], {'input': 42})
+
     def test_detect_cpu_family(self) -> None:
         """Test the various cpu familes that we detect and normalize.