From 3f5d80b8bbd40d166657c1c7f856fe7777623df1 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Fri, 27 Aug 2021 11:09:57 -0700 Subject: [PATCH 1/4] interpreter: fix name of typed_kwargs for `test()` There was a copy-n-paste error here, and it was benchmark instead. --- mesonbuild/interpreter/interpreter.py | 2 +- test cases/failing/23 rel testdir/test.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mesonbuild/interpreter/interpreter.py b/mesonbuild/interpreter/interpreter.py index 5b03e7eb0..3efe6780a 100644 --- a/mesonbuild/interpreter/interpreter.py +++ b/mesonbuild/interpreter/interpreter.py @@ -1715,7 +1715,7 @@ This will become a hard error in the future.''' % kwargs['input'], location=self self.add_test(node, args, kwargs, False) @typed_pos_args('test', str, (build.Executable, build.Jar, ExternalProgram, mesonlib.File)) - @typed_kwargs('benchmark', *TEST_KWARGS, KwargInfo('is_parallel', bool, default=True)) + @typed_kwargs('test', *TEST_KWARGS, KwargInfo('is_parallel', bool, default=True)) def func_test(self, node: mparser.BaseNode, args: T.Tuple[str, T.Union[build.Executable, build.Jar, ExternalProgram, mesonlib.File]], kwargs: 'kwargs.FuncTest') -> None: diff --git a/test cases/failing/23 rel testdir/test.json b/test cases/failing/23 rel testdir/test.json index 79ab48a43..bc80bc634 100644 --- a/test cases/failing/23 rel testdir/test.json +++ b/test cases/failing/23 rel testdir/test.json @@ -1,7 +1,7 @@ { "stdout": [ { - "line": "test cases/failing/23 rel testdir/meson.build:4:0: ERROR: benchmark keyword argument \"workdir\" must be an absolute path" + "line": "test cases/failing/23 rel testdir/meson.build:4:0: ERROR: test keyword argument \"workdir\" must be an absolute path" } ] } From 11fbaf29d8444ca35269a938e46327dfbe7820bd Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Fri, 27 Aug 2021 11:47:12 -0700 Subject: [PATCH 2/4] interpreter: fix cases of `KwargInfo(..., T, default=None)` The correct way to mark these is `KwargInfo(..., (T, type(None)))`. There's also a few cases of `(T, None)` which is invalid, as `None` isn't a type --- mesonbuild/interpreter/compiler.py | 10 ++++----- mesonbuild/interpreter/interpreter.py | 23 ++++++++++---------- mesonbuild/interpreter/interpreterobjects.py | 3 ++- mesonbuild/interpreter/mesonmain.py | 17 ++++++++++----- mesonbuild/interpreter/type_checking.py | 3 +++ mesonbuild/modules/python.py | 3 ++- mesonbuild/modules/qt.py | 5 +++-- unittests/internaltests.py | 15 +++++++------ 8 files changed, 46 insertions(+), 33 deletions(-) diff --git a/mesonbuild/interpreter/compiler.py b/mesonbuild/interpreter/compiler.py index 53c3ce0a2..54f4beef2 100644 --- a/mesonbuild/interpreter/compiler.py +++ b/mesonbuild/interpreter/compiler.py @@ -17,7 +17,7 @@ from ..interpreterbase import (ObjectHolder, noPosargs, noKwargs, InterpreterException) from ..interpreterbase.decorators import ContainerTypeInfo, typed_kwargs, KwargInfo, typed_pos_args from .interpreterobjects import (extract_required_kwarg, extract_search_dirs) -from .type_checking import REQUIRED_KW, in_set_validator +from .type_checking import REQUIRED_KW, in_set_validator, NoneType if T.TYPE_CHECKING: from ..interpreter import Interpreter @@ -387,9 +387,9 @@ class CompilerHolder(ObjectHolder['Compiler']): @typed_pos_args('compiler.compute_int', str) @typed_kwargs( 'compiler.compute_int', - KwargInfo('low', (int, None)), - KwargInfo('high', (int, None)), - KwargInfo('guess', (int, None)), + KwargInfo('low', (int, NoneType)), + KwargInfo('high', (int, NoneType)), + KwargInfo('guess', (int, NoneType)), *_COMMON_KWS, ) def compute_int_method(self, args: T.Tuple[str], kwargs: 'CompupteIntKW') -> int: @@ -556,7 +556,7 @@ class CompilerHolder(ObjectHolder['Compiler']): 'compiler.find_library', KwargInfo('required', (bool, coredata.UserFeatureOption), default=True), KwargInfo('has_headers', ContainerTypeInfo(list, str), listify=True, default=[], since='0.50.0'), - KwargInfo('static', (bool, None), since='0.51.0'), + KwargInfo('static', (bool, NoneType), since='0.51.0'), KwargInfo('disabler', bool, default=False, since='0.49.0'), KwargInfo('dirs', ContainerTypeInfo(list, str), listify=True, default=[]), *[k.evolve(name=f'header_{k.name}') for k in _HEADER_KWS] diff --git a/mesonbuild/interpreter/interpreter.py b/mesonbuild/interpreter/interpreter.py index 3efe6780a..d104f7411 100644 --- a/mesonbuild/interpreter/interpreter.py +++ b/mesonbuild/interpreter/interpreter.py @@ -55,6 +55,7 @@ from .type_checking import ( LANGUAGE_KW, NATIVE_KW, REQUIRED_KW, + NoneType, in_set_validator, ) @@ -178,7 +179,7 @@ TEST_KWARGS: T.List[KwargInfo] = [ listify=True, default=[]), KwargInfo('should_fail', bool, default=False), KwargInfo('timeout', int, default=30), - KwargInfo('workdir', str, default=None, + KwargInfo('workdir', (str, NoneType), default=None, validator=lambda x: 'must be an absolute path' if not os.path.isabs(x) else None), KwargInfo('protocol', str, default='exitcode', @@ -188,7 +189,7 @@ TEST_KWARGS: T.List[KwargInfo] = [ listify=True, default=[], since='0.46.0'), KwargInfo('priority', int, default=0, since='0.52.0'), # TODO: env needs reworks of the way the environment variable holder itself works probably - KwargInfo('env', (EnvironmentVariablesObject, list, dict, str)), + KwargInfo('env', (EnvironmentVariablesObject, list, dict, str, NoneType)), KwargInfo('suite', ContainerTypeInfo(list, str), listify=True, default=['']), # yes, a list of empty string ] @@ -1100,7 +1101,7 @@ external dependencies (including libraries) must go to "dependencies".''') if not self.is_subproject(): self.check_stdlibs() - @typed_kwargs('add_languages', KwargInfo('native', (bool, type(None)), since='0.54.0'), REQUIRED_KW) + @typed_kwargs('add_languages', KwargInfo('native', (bool, NoneType), since='0.54.0'), REQUIRED_KW) @typed_pos_args('add_languages', varargs=str) def func_add_languages(self, node: mparser.FunctionNode, args: T.Tuple[T.List[str]], kwargs: 'kwargs.FuncAddLanguages') -> bool: langs = args[0] @@ -1686,7 +1687,7 @@ This will become a hard error in the future.''' % kwargs['input'], location=self 'generator', KwargInfo('arguments', ContainerTypeInfo(list, str, allow_empty=False), required=True, listify=True), KwargInfo('output', ContainerTypeInfo(list, str, allow_empty=False), required=True, listify=True), - KwargInfo('depfile', str, validator=lambda x: 'Depfile must be a plain filename with a subdirectory' if has_path_sep(x) else None), + KwargInfo('depfile', (str, NoneType), validator=lambda x: 'Depfile must be a plain filename with a subdirectory' if has_path_sep(x) else None), KwargInfo('capture', bool, default=False, since='0.43.0'), KwargInfo('depends', ContainerTypeInfo(list, (build.BuildTarget, build.CustomTarget)), default=[], listify=True), ) @@ -1786,8 +1787,8 @@ This will become a hard error in the future.''' % kwargs['input'], location=self @typed_pos_args('install_headers', varargs=(str, mesonlib.File)) @typed_kwargs( 'install_headers', - KwargInfo('install_dir', (str, None)), - KwargInfo('subdir', (str, None)), + KwargInfo('install_dir', (str, NoneType)), + KwargInfo('subdir', (str, NoneType)), INSTALL_MODE_KW.evolve(since='0.47.0'), ) def func_install_headers(self, node: mparser.BaseNode, @@ -1807,8 +1808,8 @@ This will become a hard error in the future.''' % kwargs['input'], location=self @typed_pos_args('install_man', varargs=(str, mesonlib.File)) @typed_kwargs( 'install_man', - KwargInfo('install_dir', (str, None)), - KwargInfo('locale', (str, None), since='0.58.0'), + KwargInfo('install_dir', (str, NoneType)), + KwargInfo('locale', (str, NoneType), since='0.58.0'), INSTALL_MODE_KW.evolve(since='0.47.0') ) def func_install_man(self, node: mparser.BaseNode, @@ -1902,11 +1903,11 @@ This will become a hard error in the future.''' % kwargs['input'], location=self @typed_pos_args('install_data', varargs=(str, mesonlib.File)) @typed_kwargs( 'install_data', - KwargInfo('install_dir', str), + KwargInfo('install_dir', (str, NoneType)), KwargInfo('sources', ContainerTypeInfo(list, (str, mesonlib.File)), listify=True, default=[]), KwargInfo('rename', ContainerTypeInfo(list, str), default=[], listify=True, since='0.46.0'), INSTALL_MODE_KW.evolve(since='0.38.0'), - KwargInfo('install_tag', str, since='0.60.0'), + KwargInfo('install_tag', (str, NoneType), since='0.60.0'), ) def func_install_data(self, node: mparser.BaseNode, args: T.Tuple[T.List['mesonlib.FileOrString']], @@ -1934,7 +1935,7 @@ This will become a hard error in the future.''' % kwargs['input'], location=self @typed_kwargs( 'install_subdir', KwargInfo('install_dir', str, required=True), - KwargInfo('install_tag', str, since='0.60.0'), + KwargInfo('install_tag', (str, NoneType), since='0.60.0'), KwargInfo('strip_directory', bool, default=False), KwargInfo('exclude_files', ContainerTypeInfo(list, str), default=[], listify=True, since='0.42.0', diff --git a/mesonbuild/interpreter/interpreterobjects.py b/mesonbuild/interpreter/interpreterobjects.py index 2bff1bb09..d524059e7 100644 --- a/mesonbuild/interpreter/interpreterobjects.py +++ b/mesonbuild/interpreter/interpreterobjects.py @@ -20,6 +20,7 @@ from ..interpreterbase import ( typed_pos_args, typed_kwargs, stringArgs, permittedKwargs, noArgsFlattening, noPosargs, noKwargs, permissive_unholder_return, TYPE_var, TYPE_kwargs, TYPE_nvar, TYPE_nkwargs, flatten, resolve_second_level_holders, InterpreterException, InvalidArguments, InvalidCode) +from ..interpreter.type_checking import NoneType from ..dependencies import Dependency, ExternalLibrary, InternalDependency from ..programs import ExternalProgram from ..mesonlib import HoldableObject, MesonException, OptionKey, listify, Popen_safe @@ -975,7 +976,7 @@ class GeneratorHolder(ObjectHolder[build.Generator]): @typed_pos_args('generator.process', min_varargs=1, varargs=(str, mesonlib.File, build.CustomTarget, build.CustomTargetIndex, build.GeneratedList)) @typed_kwargs( 'generator.process', - KwargInfo('preserve_path_from', str, since='0.45.0'), + KwargInfo('preserve_path_from', (str, NoneType), since='0.45.0'), KwargInfo('extra_args', ContainerTypeInfo(list, str), listify=True, default=[]), ) def process_method(self, diff --git a/mesonbuild/interpreter/mesonmain.py b/mesonbuild/interpreter/mesonmain.py index 1c850b233..a1ba4456e 100644 --- a/mesonbuild/interpreter/mesonmain.py +++ b/mesonbuild/interpreter/mesonmain.py @@ -14,7 +14,7 @@ from ..interpreterbase import (MesonInterpreterObject, FeatureNew, FeatureDeprec from .interpreterobjects import (ExecutableHolder, ExternalProgramHolder, CustomTargetHolder, CustomTargetIndexHolder, EnvironmentVariablesObject) -from .type_checking import NATIVE_KW +from .type_checking import NATIVE_KW, NoneType import typing as T @@ -112,9 +112,11 @@ class MesonMain(MesonInterpreterObject): '0.55.0', self.interpreter.subproject) return script_args - @typed_kwargs('add_install_script', - KwargInfo('skip_if_destdir', bool, default=False, since='0.57.0'), - KwargInfo('install_tag', str, since='0.60.0')) + @typed_kwargs( + 'add_install_script', + KwargInfo('skip_if_destdir', bool, default=False, since='0.57.0'), + KwargInfo('install_tag', (str, NoneType), since='0.60.0'), + ) def add_install_script_method(self, args: 'T.Tuple[T.Union[str, mesonlib.File, ExecutableHolder], T.Union[str, mesonlib.File, CustomTargetHolder, CustomTargetIndexHolder], ...]', kwargs): if len(args) < 1: raise InterpreterException('add_install_script takes one or more arguments') @@ -299,8 +301,11 @@ class MesonMain(MesonInterpreterObject): raise InterpreterException('Second argument must be an external program or executable.') self.interpreter.add_find_program_override(name, exe) - @typed_kwargs('meson.override_dependency', NATIVE_KW, - KwargInfo('static', bool, since='0.60.0')) + @typed_kwargs( + 'meson.override_dependency', + NATIVE_KW, + KwargInfo('static', (bool, NoneType), since='0.60.0'), + ) @typed_pos_args('meson.override_dependency', str, dependencies.Dependency) @FeatureNew('meson.override_dependency', '0.54.0') def override_dependency_method(self, args: T.Tuple[str, dependencies.Dependency], kwargs: 'FuncOverrideDependency') -> None: diff --git a/mesonbuild/interpreter/type_checking.py b/mesonbuild/interpreter/type_checking.py index dd0411c5a..7d5665c46 100644 --- a/mesonbuild/interpreter/type_checking.py +++ b/mesonbuild/interpreter/type_checking.py @@ -10,6 +10,9 @@ from ..coredata import UserFeatureOption from ..interpreterbase.decorators import KwargInfo, ContainerTypeInfo from ..mesonlib import FileMode, MachineChoice +# Helper definition for type checks that are `Optional[T]` +NoneType = type(None) + def in_set_validator(choices: T.Set[str]) -> T.Callable[[str], T.Optional[str]]: """Check that the choice given was one of the given set.""" diff --git a/mesonbuild/modules/python.py b/mesonbuild/modules/python.py index bb6b9abda..d03f951cd 100644 --- a/mesonbuild/modules/python.py +++ b/mesonbuild/modules/python.py @@ -28,6 +28,7 @@ from ..dependencies import DependencyMethods, PkgConfigDependency, NotFoundDepen from ..dependencies.base import process_method_kw from ..environment import detect_cpu_family from ..interpreter import ExternalProgramHolder, extract_required_kwarg, permitted_dependency_kwargs +from ..interpreter.type_checking import NoneType from ..interpreterbase import ( noPosargs, noKwargs, permittedKwargs, ContainerTypeInfo, InvalidArguments, typed_pos_args, typed_kwargs, KwargInfo, @@ -484,7 +485,7 @@ class PythonInstallation(ExternalProgramHolder): @typed_pos_args('install_data', varargs=(str, mesonlib.File)) @typed_kwargs('python_installation.install_sources', _PURE_KW, _SUBDIR_KW, - KwargInfo('install_tag', str, since='0.60.0')) + KwargInfo('install_tag', (str, NoneType), since='0.60.0')) def install_sources_method(self, args: T.Tuple[T.List[T.Union[str, mesonlib.File]]], kwargs: 'PyInstallKw') -> 'Data': tag = kwargs['install_tag'] or 'runtime' diff --git a/mesonbuild/modules/qt.py b/mesonbuild/modules/qt.py index 9a1220f8d..3ee5bb2e4 100644 --- a/mesonbuild/modules/qt.py +++ b/mesonbuild/modules/qt.py @@ -26,6 +26,7 @@ from ..dependencies import find_external_dependency, Dependency, ExternalLibrary from ..mesonlib import MesonException, File, FileOrString, version_compare, Popen_safe from . import ModuleReturnValue, ExtensionModule from ..interpreter import extract_required_kwarg +from ..interpreter.type_checking import NoneType from ..interpreterbase import ContainerTypeInfo, FeatureDeprecated, KwargInfo, noPosargs, FeatureNew, typed_kwargs from ..programs import ExternalProgram, NonExistingExternalProgram @@ -510,9 +511,9 @@ class QtBaseModule(ExtensionModule): 'qt.compile_translations', KwargInfo('build_by_default', bool, default=False), KwargInfo('install', bool, default=False), - KwargInfo('install_dir', str), + KwargInfo('install_dir', (str, NoneType)), KwargInfo('method', str, default='auto'), - KwargInfo('qresource', str, since='0.56.0'), + KwargInfo('qresource', (str, NoneType), since='0.56.0'), KwargInfo('rcc_extra_arguments', ContainerTypeInfo(list, str), listify=True, default=[], since='0.56.0'), KwargInfo('ts_files', ContainerTypeInfo(list, (str, File, build.CustomTarget, build.CustomTargetIndex, build.GeneratedList)), listify=True, default=[]), ) diff --git a/unittests/internaltests.py b/unittests/internaltests.py index daf3efd33..911580367 100644 --- a/unittests/internaltests.py +++ b/unittests/internaltests.py @@ -1204,7 +1204,7 @@ class InternalTests(unittest.TestCase): def test_typed_kwarg_basic(self) -> None: @typed_kwargs( 'testfunc', - KwargInfo('input', str) + KwargInfo('input', str, default='') ) def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, str]) -> None: self.assertIsInstance(kwargs['input'], str) @@ -1227,7 +1227,7 @@ class InternalTests(unittest.TestCase): def test_typed_kwarg_missing_optional(self) -> None: @typed_kwargs( 'testfunc', - KwargInfo('input', str), + KwargInfo('input', (str, type(None))), ) def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, T.Optional[str]]) -> None: self.assertIsNone(kwargs['input']) @@ -1247,7 +1247,7 @@ class InternalTests(unittest.TestCase): def test_typed_kwarg_container_valid(self) -> None: @typed_kwargs( 'testfunc', - KwargInfo('input', ContainerTypeInfo(list, str), required=True), + KwargInfo('input', ContainerTypeInfo(list, str), default=[], required=True), ) def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, T.List[str]]) -> None: self.assertEqual(kwargs['input'], ['str']) @@ -1281,7 +1281,7 @@ class InternalTests(unittest.TestCase): def test_typed_kwarg_container_listify(self) -> None: @typed_kwargs( 'testfunc', - KwargInfo('input', ContainerTypeInfo(list, str), listify=True), + KwargInfo('input', ContainerTypeInfo(list, str), default=[], listify=True), ) def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, T.List[str]]) -> None: self.assertEqual(kwargs['input'], ['str']) @@ -1347,7 +1347,7 @@ class InternalTests(unittest.TestCase): def test_typed_kwarg_validator(self) -> None: @typed_kwargs( 'testfunc', - KwargInfo('input', str, validator=lambda x: 'invalid!' if x != 'foo' else None) + KwargInfo('input', str, default='', validator=lambda x: 'invalid!' if x != 'foo' else None) ) def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, str]) -> None: pass @@ -1362,7 +1362,7 @@ class InternalTests(unittest.TestCase): def test_typed_kwarg_convertor(self) -> None: @typed_kwargs( 'testfunc', - KwargInfo('native', bool, convertor=lambda n: MachineChoice.BUILD if n else MachineChoice.HOST) + KwargInfo('native', bool, default=False, convertor=lambda n: MachineChoice.BUILD if n else MachineChoice.HOST) ) def _(obj, node, args: T.Tuple, kwargs: T.Dict[str, MachineChoice]) -> None: assert isinstance(kwargs['native'], MachineChoice) @@ -1376,7 +1376,8 @@ class InternalTests(unittest.TestCase): KwargInfo('input', ContainerTypeInfo(list, str), listify=True, default=[], deprecated_values={'foo': '0.9'}, since_values={'bar': '1.1'}), KwargInfo('output', ContainerTypeInfo(dict, str), default={}, deprecated_values={'foo': '0.9'}, since_values={'bar': '1.1'}), KwargInfo( - 'mode', str, + 'mode', + (str, type(None)), validator=in_set_validator({'clean', 'build', 'rebuild', 'deprecated', 'since'}), deprecated_values={'deprecated': '1.0'}, since_values={'since': '1.1'}), From ce392acad4ee0c2eb8c15742d8194f82f88eb131 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Fri, 27 Aug 2021 11:03:42 -0700 Subject: [PATCH 3/4] interpreterbase: ensure that the default vaule to KwargInfo is a valid type Because currently you can write something like: ```python KwargInfo('capture', bool) ``` Which proclaims "this must be bool", but the default is then not valid. --- mesonbuild/interpreterbase/decorators.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mesonbuild/interpreterbase/decorators.py b/mesonbuild/interpreterbase/decorators.py index 2f59dd7fd..6aa4fcae7 100644 --- a/mesonbuild/interpreterbase/decorators.py +++ b/mesonbuild/interpreterbase/decorators.py @@ -467,8 +467,12 @@ def typed_kwargs(name: str, *types: KwargInfo) -> T.Callable[..., T.Any]: # 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 if info.not_set_warning: mlog.warning(info.not_set_warning) From b4bc8464e69c353fc0792054ab4209d4e8b5a096 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Sat, 28 Aug 2021 10:38:39 -0700 Subject: [PATCH 4/4] Try to fix NoneType Because mypy doesn't like the type alias. --- mesonbuild/interpreter/type_checking.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mesonbuild/interpreter/type_checking.py b/mesonbuild/interpreter/type_checking.py index 7d5665c46..d4e178d12 100644 --- a/mesonbuild/interpreter/type_checking.py +++ b/mesonbuild/interpreter/type_checking.py @@ -11,7 +11,7 @@ from ..interpreterbase.decorators import KwargInfo, ContainerTypeInfo from ..mesonlib import FileMode, MachineChoice # Helper definition for type checks that are `Optional[T]` -NoneType = type(None) +NoneType: T.Type[None] = type(None) def in_set_validator(choices: T.Set[str]) -> T.Callable[[str], T.Optional[str]]: