Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error messages for typed_kwarg type mismatches in containers #9599

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 39 additions & 24 deletions mesonbuild/interpreterbase/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,9 @@ def description(self) -> str:

: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}]'
Expand Down Expand Up @@ -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)
Expand All @@ -470,24 +503,6 @@ def wrapper(*wrapped_args: T.Any, **wrapped_kwargs: T.Any) -> 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:
Expand All @@ -498,9 +513,9 @@ def types_description() -> str:
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)
Expand Down Expand Up @@ -533,7 +548,7 @@ def types_description() -> str:
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)
Expand Down
11 changes: 10 additions & 1 deletion run_single_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion test cases/failing/116 run_target in test/test.json
Original file line number Diff line number Diff line change
@@ -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]"
}
]
}
Expand Down
8 changes: 4 additions & 4 deletions unittests/internaltests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,7 @@ def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, T.List[str]]) -> None:

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(
Expand All @@ -1277,8 +1277,8 @@ def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, T.Dict[str, str]]) -> None:
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(
Expand Down Expand Up @@ -1313,7 +1313,7 @@ def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, T.List[str]]) -> None:

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:
Expand Down