From 86f0516f376983b703cb96e1a7cd2f9d39e8fd44 Mon Sep 17 00:00:00 2001 From: Gereon Kremer Date: Fri, 3 Sep 2021 12:12:13 -0700 Subject: Refactor option sanitizations (#7129) This PR refactors the code that performs sanity checks on the parsed option data. --- src/options/mkoptions.py | 260 ++++++++++++++++++++++------------------------- 1 file changed, 122 insertions(+), 138 deletions(-) diff --git a/src/options/mkoptions.py b/src/options/mkoptions.py index 6265ca7b1..55e047047 100644 --- a/src/options/mkoptions.py +++ b/src/options/mkoptions.py @@ -96,8 +96,6 @@ def all_options(modules, sorted=False): ### Other globals -g_long_cache = dict() # maps long options to filename/fileno - g_getopt_long_start = 256 ### Source code templates @@ -331,6 +329,15 @@ def generate_get_impl(modules): res.append('if ({}) {}'.format(cond, ret)) return '\n '.join(res) + def __lt__(self, other): + if self.long_name and other.long_name: + return self.long_name < other.long_name + if self.long_name: return True + return False + + def __str__(self): + return self.long_name if self.long_name else self.name + class SphinxGenerator: def __init__(self): @@ -435,16 +442,6 @@ def die(msg): sys.exit('[error] {}'.format(msg)) -def perr(filename, msg, option=None): - msg_suffix = '' - if option: - if option.name: - msg_suffix = "option '{}' ".format(option.name) - else: - msg_suffix = "option '{}' ".format(option.long) - die('parse error in {}: {}{}'.format(filename, msg, msg_suffix)) - - def write_file(directory, name, content): """ Write string 'content' to file directory/name. If the file already exists, @@ -951,120 +948,112 @@ def codegen_all_modules(modules, build_dir, dst_dir, tpls): sphinxgen.render('{}/docs/'.format(build_dir), 'options_generated.rst') -def check_attribs(filename, req_attribs, valid_attribs, attribs, ctype): - """ - Check if for a given module/option the defined attributes are valid and - if all required attributes are defined. - """ - msg_for = "" - if 'name' in attribs: - msg_for = " for '{}'".format(attribs['name']) - elif 'long' in attribs: - msg_for = " for '{}'".format(attribs['long']) - for k in req_attribs: - if k not in attribs: - perr(filename, - "required {} attribute '{}' not specified{}".format( - ctype, k, msg_for)) - for k in attribs: - if k not in valid_attribs: - perr(filename, - "invalid {} attribute '{}' specified{}".format( - ctype, k, msg_for)) - - -def check_unique(filename, value, cache): - """ - Check if given name is unique in cache. - """ - if value in cache: - perr(filename, - "'{}' already defined in '{}'".format(value, cache[value])) - else: - cache[value] = filename - - -def check_long(filename, option, long_name, ctype=None): - """ - Check if given long option name is valid. - """ - global g_long_cache - if long_name is None: - return - if long_name.startswith('--'): - perr(filename, 'remove -- prefix from long', option) - r = r'^[0-9a-zA-Z\-=]+$' - if not re.match(r, long_name): - perr(filename, - "long '{}' does not match regex criteria '{}'".format( - long_name, r), option) - name = long_get_option(long_name) - check_unique(filename, name, g_long_cache) - - if ctype == 'bool': - check_unique(filename, 'no-{}'.format(name), g_long_cache) - - -def parse_module(filename, module): - """ - Parse options module file. - - Note: We could use an existing toml parser to parse the configuration - files. However, since we only use a very restricted feature set of the - toml format, we chose to implement our own parser to get better error - messages. - """ - # Check if parsed module attributes are valid and if all required - # attributes are defined. - check_attribs(filename, - MODULE_ATTR_REQ, MODULE_ATTR_ALL, module, 'module') - res = Module(module, filename) - - if 'option' in module: - for attribs in module['option']: - check_attribs(filename, - OPTION_ATTR_REQ, OPTION_ATTR_ALL, attribs, 'option') - option = Option(attribs) - if option.mode and not option.help_mode: - perr(filename, 'defines modes but no help_mode', option) - if option.mode and not option.default: - perr(filename, "mode option has no default", option) - if option.mode and option.default and \ - option.default not in option.mode.keys(): - perr(filename, - "invalid default value '{}'".format(option.default), - option) - if option.alternate and option.type != 'bool': - perr(filename, 'is alternate but not bool', option) - if option.short and not option.long: - perr(filename, - "short option '{}' specified but no long option".format( - option.short), - option) - if option.type == 'bool' and option.handler: - perr(filename, - 'defining handlers for bool options is not allowed', - option) - if option.category not in CATEGORY_VALUES: - perr(filename, - "has invalid category '{}'".format(option.category), - option) - if option.category != 'undocumented' and not option.help: - perr(filename, - 'help text required for {} options'.format(option.category), - option) - res.options.append(option) - - return res +class Checker: + """Performs a variety of sanity checks on options and option modules, and + constructs `Module` and `Option` from dictionaries.""" + def __init__(self): + self.__filename = None + self.__long_cache = {} + + def perr(self, msg, *args, **kwargs): + """Print an error and die.""" + if 'option' in kwargs: + msg = "option '{}' {}".format(kwargs['option'], msg) + msg = 'parse error in {}: {}'.format(self.__filename, msg) + die(msg.format(*args, **kwargs)) + + def __check_module_attribs(self, req, valid, module): + """Check the attributes of an option module.""" + for k in req: + if k not in module: + self.perr("required module attribute '{}' not specified", k) + for k in module: + if k not in valid: + self.perr("invalid module attribute '{}' specified", k) + + def __check_option_attribs(self, req, valid, option): + """Check the attributes of an option.""" + if 'name' in option: + name = option['name'] + else: + name = option.get('long', '--') + for k in req: + if k not in option: + self.perr( + "required option attribute '{}' not specified for '{}'", k, + name) + for k in option: + if k not in valid: + self.perr("invalid option attribute '{}' specified for '{}'", + k, name) + + def __check_option_long(self, option, long): + """Check a long argument of an option (name and uniqueness).""" + if long.startswith('--'): + self.perr("remove '--' prefix from '{}'", long, option=option) + r = r'^[0-9a-zA-Z\-]+$' + if not re.match(r, long): + self.perr("long '{}' does not match '{}'", long, r, option=option) + if long in self.__long_cache: + file = self.__long_cache[long] + self.perr("long '{}' was already defined in '{}'", + long, + file, + option=option) + self.__long_cache[long] = self.__filename + + def check_module(self, module, filename): + """Check the given module and return a `Module` object.""" + self.__filename = os.path.basename(filename) + self.__check_module_attribs(MODULE_ATTR_REQ, MODULE_ATTR_ALL, module) + return Module(module, filename) + + def check_option(self, option): + """Check the option module and return an `Option` object.""" + self.__check_option_attribs(OPTION_ATTR_REQ, OPTION_ATTR_ALL, option) + o = Option(option) + if o.category not in CATEGORY_VALUES: + self.perr("has invalid category '{}'", o.category, option=o) + if o.mode and not o.help_mode: + self.perr('defines modes but no help_mode', option=o) + if o.mode and not o.default: + self.perr('mode option has no default', option=o) + if o.mode and o.default and o.default not in o.mode.keys(): + self.perr("invalid default value '{}'", o.default, option=o) + if o.short and not o.long: + self.perr("has short '{}' but no long", o.short, option=o) + if o.category != 'undocumented' and not o.help: + self.perr("of type '{}' has no help text", o.category, option=o) + if o.alias and not o.long: + self.perr('has aliases but no long', option=o) + if o.alternate and o.type != 'bool': + self.perr('is alternate but not bool', option=o) + if o.long: + self.__check_option_long(o, o.long_name) + if o.alternate: + self.__check_option_long(o, 'no-' + o.long_name) + if o.type in ['bool', 'void'] and '=' in o.long: + self.perr('must not have an argument description', option=o) + if o.type not in ['bool', 'void'] and not '=' in o.long: + self.perr("needs argument description ('{}=...')", + o.long, + option=o) + if o.alias: + for alias in o.alias: + self.__check_option_long(o, alias) + if o.alternate: + self.__check_option_long(o, 'no-' + alias) + return o def usage(): - print('mkoptions.py +') + """Print the command-line usage""" + print('mkoptions.py +') print('') - print(' location of all *_template.{cpp,h} files') - print(' build directory') - print(' destination directory for the generated files') - print(' + one or more *_optios.toml files') + print(' base source directory of all toml files') + print(' build directory to write the generated sphinx docs') + print(' base destination directory for all generated files') + print(' + one or more *_options.toml files') print('') @@ -1073,10 +1062,8 @@ def mkoptions_main(): usage() die('missing arguments') - src_dir = sys.argv[1] - build_dir = sys.argv[2] - dst_dir = sys.argv[3] - filenames = sys.argv[4:] + # Load command line arguments + _, src_dir, build_dir, dst_dir, *filenames = sys.argv # Check if given directories exist. for d in [src_dir, dst_dir]: @@ -1100,28 +1087,25 @@ def mkoptions_main(): {'input': 'main/options_template.cpp'}, ] + # Load all template files for tpl in module_tpls + global_tpls: tpl['output'] = tpl['input'].replace('_template', '') tpl['content'] = read_tpl(src_dir, tpl['input']) - - # Parse files, check attributes and create module/option objects + # Parse and check toml files + checker = Checker() modules = [] for filename in filenames: - module = parse_module(filename, toml.load(filename)) - - # Check if long options are valid and unique. First populate - # g_long_cache with option.long and --no- alternatives if - # applicable. - for option in module.options: - check_long(filename, option, option.long, option.type) + data = toml.load(filename) + module = checker.check_module(data, filename) + if 'option' in data: + module.options = sorted( + [checker.check_option(a) for a in data['option']]) modules.append(module) - # Create *_options.{h,cpp} in destination directory + # Generate code for module in modules: codegen_module(module, dst_dir, module_tpls) - - # Create options.cpp in destination directory codegen_all_modules(modules, build_dir, dst_dir, global_tpls) -- cgit v1.2.3