Merge pull request #9599 from dcbaker/submit/typed_kwargs-message-improvements

Improve error messages for typed_kwarg type mismatches in containers

mesonbuild/interpreterbase/decorators.py

@@ -326,9 +326,9 @@ class ContainerTypeInfo:
 
         :return: string to be printed
         """
-        container = 'dict' if self.container is dict else 'list'
+        container = 'dict' if self.container is dict else 'array'
         if isinstance(self.contains, tuple):
-            contains = ','.join([t.__name__ for t in self.contains])
+            contains = ' | '.join([t.__name__ for t in self.contains])
         else:
             contains = self.contains.__name__
         s = f'{container}[{contains}]'
@@ -456,6 +456,39 @@ def typed_kwargs(name: str, *types: KwargInfo) -> T.Callable[..., T.Any]:
     """
     def inner(f: TV_func) -> TV_func:
 
+        def types_description(types_tuple: T.Tuple[T.Union[T.Type, ContainerTypeInfo], ...]) -> 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
+
+        def raw_description(t: object) -> str:
+            """describe a raw type (ie, one that is not a ContainerTypeInfo)."""
+            if isinstance(t, list):
+                if t:
+                    return f"array[{' | '.join(sorted(mesonlib.OrderedSet(type(v).__name__ for v in t)))}]"
+                return 'array[]'
+            elif isinstance(t, dict):
+                if t:
+                    return f"dict[{' | '.join(sorted(mesonlib.OrderedSet(type(v).__name__ for v in t.values())))}]"
+                return 'dict[]'
+            return type(t).__name__
+
+        def check_value_type(types_tuple: T.Tuple[T.Union[T.Type, ContainerTypeInfo], ...],
+                             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
+
         @wraps(f)
         def wrapper(*wrapped_args: T.Any, **wrapped_kwargs: T.Any) -> T.Any:
             node, _, _kwargs, subproject = get_callee_args(wrapped_args)
@@ -470,24 +503,6 @@ def typed_kwargs(name: str, *types: KwargInfo) -> T.Callable[..., T.Any]:
 
             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:
@@ -498,9 +513,9 @@ def typed_kwargs(name: str, *types: KwargInfo) -> T.Callable[..., T.Any]:
                         FeatureDeprecated.single_use(feature_name, info.deprecated, subproject, location=node)
                     if info.listify:
                         kwargs[info.name] = value = mesonlib.listify(value)
-                    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 not check_value_type(types_tuple, value):
+                        shouldbe = types_description(types_tuple)
+                        raise InvalidArguments(f'{name} keyword argument {info.name!r} was of type {raw_description(value)} but should have been {shouldbe}')
 
                     if info.validator is not None:
                         msg = info.validator(value)
@@ -533,7 +548,7 @@ 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
-                    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()}'
+                    assert check_value_type(types_tuple, info.default), f'In funcion {name} default value of {info.name} is not a valid type, got {type(info.default)} expected {types_description(types_tuple)}'
                     # 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)

run_single_test.py

@@ -47,7 +47,16 @@ def main() -> None:
     if args.subtests:
         tests = [t for i, t in enumerate(tests) if i in args.subtests]
 
-    results = [run_test(t, t.args, '', True) for t in tests]
+    def should_fail(path: pathlib.Path) -> str:
+        dir_ = path.parent.stem
+        # FIXME: warning tets might not be handled correctly still…
+        if dir_.startswith(('failing', 'warning')):
+            if ' ' in dir_:
+                return dir_.split(' ')[1]
+            return 'meson'
+        return ''
+
+    results = [run_test(t, t.args, should_fail(t.path), args.use_tmpdir) for t in tests]
     failed = False
     for test, result in zip(tests, results):
         if (result is None) or ('MESON_SKIP_TEST' in result.stdo):

test cases/failing/116 run_target in test/test.json

@@ -1,7 +1,7 @@
 {
     "stdout": [
         {
-            "line": "test cases/failing/116 run_target in test/meson.build:4:0: ERROR: test keyword argument 'args' was of type 'list' but should have been list[str,File,BuildTarget,CustomTarget]"
+            "line": "test cases/failing/116 run_target in test/meson.build:4:0: ERROR: test keyword argument 'args' was of type array[RunTarget] but should have been array[str | File | BuildTarget | CustomTarget]"
         }
     ]
 }

unittests/internaltests.py

@@ -1267,7 +1267,7 @@ class InternalTests(unittest.TestCase):
 
         with self.assertRaises(InvalidArguments) as cm:
             _(None, mock.Mock(), [], {'input': {}})
-        self.assertEqual(str(cm.exception), "testfunc keyword argument 'input' was of type 'dict' but should have been list[str]")
+        self.assertEqual(str(cm.exception), "testfunc keyword argument 'input' was of type dict[] but should have been array[str]")
 
     def test_typed_kwarg_contained_invalid(self) -> None:
         @typed_kwargs(
@@ -1278,8 +1278,8 @@ class InternalTests(unittest.TestCase):
             self.assertTrue(False)  # should be unreachable
 
         with self.assertRaises(InvalidArguments) as cm:
-            _(None, mock.Mock(), [], {'input': {'key': 1}})
-        self.assertEqual(str(cm.exception), "testfunc keyword argument 'input' was of type 'dict' but should have been dict[str]")
+            _(None, mock.Mock(), [], {'input': {'key': 1, 'bar': 2}})
+        self.assertEqual(str(cm.exception), "testfunc keyword argument 'input' was of type dict[int] but should have been dict[str]")
 
     def test_typed_kwarg_container_listify(self) -> None:
         @typed_kwargs(
@@ -1314,7 +1314,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' was of type 'list' but should have been list[str] that has even size")
+        self.assertEqual(str(cm.exception), "testfunc keyword argument 'input' was of type array[str] but should have been array[str] that has even size")
 
     @mock.patch.dict(mesonbuild.mesonlib.project_meson_versions, {})
     def test_typed_kwarg_since(self) -> None: