diff --git a/siliconcompiler/core.py b/siliconcompiler/core.py index 1479490df..c234323d2 100644 --- a/siliconcompiler/core.py +++ b/siliconcompiler/core.py @@ -793,9 +793,9 @@ def get(self, *keypath, field='value', job=None, cfg=None): else: cfg = self.cfg - keypathstr = ','.join(keypath) + keypathstr = f'{keypath}' - self.logger.debug(f"Reading from [{keypathstr}]. Field = '{field}'") + self.logger.debug(f"Reading from {keypathstr}. Field = '{field}'") return self._search(cfg, keypathstr, *keypath, field=field, mode='get') ########################################################################### @@ -831,7 +831,7 @@ def getkeys(self, *keypath, cfg=None, job=None): cfg = self.cfg['history'][job] if len(list(keypath)) > 0: - keypathstr = ','.join(keypath) + keypathstr = f'{keypath}' self.logger.debug('Getting schema parameter keys for: %s', keypathstr) keys = list(self._search(cfg, keypathstr, *keypath, mode='getkeys')) if 'default' in keys: @@ -904,19 +904,14 @@ def set(self, *args, field='value', clobber=True, cfg=None): if cfg is None: cfg = self.cfg - # Verify that all keys are strings - for key in args[:-1]: - if not isinstance(key,str): - self.logger.error(f"Key [{key}] is not a string [{args}]") - - keypathstr = ','.join(args[:-1]) + keypathstr = f'{args[:-1]}' all_args = list(args) # Special case to ensure loglevel is updated ASAP if len(args) == 3 and args[1] == 'loglevel' and field == 'value': self.logger.setLevel(args[2]) - self.logger.debug(f"Setting [{keypathstr}] to {args[-1]}") + self.logger.debug(f"Setting {keypathstr} to {args[-1]}") return self._search(cfg, keypathstr, *all_args, field=field, mode='set', clobber=clobber) ########################################################################### @@ -948,15 +943,10 @@ def add(self, *args, cfg=None, field='value'): if cfg is None: cfg = self.cfg - # Verify that all keys are strings - for key in args[:-1]: - if not isinstance(key,str): - self.logger.error(f"Key [{key}] is not a string [{args}]") - - keypathstr = ','.join(args[:-1]) + keypathstr = f'{args[:-1]}' all_args = list(args) - self.logger.debug(f'Appending value {args[-1]} to [{keypathstr}]') + self.logger.debug(f'Appending value {args[-1]} to {keypathstr}') return self._search(cfg, keypathstr, *all_args, field=field, mode='add') @@ -1000,11 +990,24 @@ def _search(self, cfg, keypath, *args, field='value', mode='get', clobber=True): val = all_args[-1] empty = [None, 'null', [], 'false'] + # Ensure that all keypath values are strings. + # Scripts may accidentally pass in [None] if a prior schema entry was unexpectedly empty. + keys_to_check = args + if mode in ['set', 'add']: + # Ignore the value parameter for 'set' and 'add' operations. + keys_to_check = args[:-1] + for key in keys_to_check: + if not isinstance(key, str): + self.error( + f'Invalid keypath: {keypath}\n' + 'Your Chip configuration may be missing a parameter which is expected by your build script.') + return None + #set/add leaf cell (all_args=(param,val)) if (mode in ('set', 'add')) & (len(all_args) == 2): # clean error if key not found if (not param in cfg) & (not 'default' in cfg): - self.error(f"Set/Add keypath [{keypath}] does not exist.") + self.error(f"Set/Add keypath {keypath} does not exist.") else: # making an 'instance' of default if not found if (not param in cfg) & ('default' in cfg): @@ -1012,7 +1015,7 @@ def _search(self, cfg, keypath, *args, field='value', mode='get', clobber=True): list_type =bool(re.match(r'\[', cfg[param]['type'])) # checking for illegal fields if not field in cfg[param] and (field != 'value'): - self.error(f"Field '{field}' for keypath [{keypath}]' is not a valid field.") + self.error(f"Field '{field}' for keypath {keypath}' is not a valid field.") # check legality of value if field == 'value': (type_ok,type_error) = self._typecheck(cfg[param], param, val) @@ -1029,7 +1032,7 @@ def _search(self, cfg, keypath, *args, field='value', mode='get', clobber=True): selval = cfg[param]['value'] # updating values if cfg[param]['lock'] == "true": - self.logger.debug("Ignoring {mode}{} to [{keypath}]. Lock bit is set.") + self.logger.debug("Ignoring {mode}{} to {keypath}. Lock bit is set.") elif (mode == 'set'): if (field != 'value') or (selval in empty) or clobber: if field in ('copy', 'lock'): @@ -1069,9 +1072,9 @@ def _search(self, cfg, keypath, *args, field='value', mode='get', clobber=True): else: cfg[param][field] = val else: - self.error(f"Assigning list to scalar for [{keypath}]") + self.error(f"Assigning list to scalar for {keypath}") else: - self.logger.debug(f"Ignoring set() to [{keypath}], value already set. Use clobber=true to override.") + self.logger.debug(f"Ignoring set() to {keypath}, value already set. Use clobber=true to override.") elif (mode == 'add'): if field in ('filehash', 'date', 'author', 'signature'): cfg[param][field].append(str(val)) @@ -1082,19 +1085,19 @@ def _search(self, cfg, keypath, *args, field='value', mode='get', clobber=True): elif list_type & isinstance(val, list): cfg[param][field].extend(val) else: - self.error(f"Illegal use of add() for scalar parameter [{keypath}].") + self.error(f"Illegal use of add() for scalar parameter {keypath}.") return cfg[param][field] #get leaf cell (all_args=param) elif len(all_args) == 1: if not param in cfg: - self.error(f"Get keypath [{keypath}] does not exist.") + self.error(f"Get keypath {keypath} does not exist.") elif mode == 'getcfg': return cfg[param] elif mode == 'getkeys': return cfg[param].keys() else: if not (field in cfg[param]) and (field!='value'): - self.error(f"Field '{field}' not found for keypath [{keypath}]") + self.error(f"Field '{field}' not found for keypath {keypath}") elif field == 'value': #Select default if no value has been set if field not in cfg[param]: @@ -1158,7 +1161,7 @@ def _search(self, cfg, keypath, *args, field='value', mode='get', clobber=True): if not param in cfg and 'default' in cfg: cfg[param] = copy.deepcopy(cfg['default']) elif not param in cfg: - self.error(f"Get keypath [{keypath}] does not exist.") + self.error(f"Get keypath {keypath} does not exist.") return None all_args.pop(0) return self._search(cfg[param], keypath, *all_args, field=field, mode=mode, clobber=clobber) diff --git a/tests/core/test_setget.py b/tests/core/test_setget.py index 423ee0fdf..0a9635fdd 100644 --- a/tests/core/test_setget.py +++ b/tests/core/test_setget.py @@ -1,6 +1,7 @@ # Copyright 2020 Silicon Compiler Authors. All Rights Reserved. -import siliconcompiler +import pytest import re +import siliconcompiler def _cast(val, sctype): if sctype.startswith('['): @@ -78,6 +79,40 @@ def test_set_field_bool(): chip.set('input', 'txt', False, field='copy') assert chip.get('input', 'txt', field='copy') is False +def test_getkeys_invalid_field(): + chip = siliconcompiler.Chip('test') + with pytest.raises(siliconcompiler.core.SiliconCompilerError): + chip.getkeys('option', None) + +def test_add_invalid_field(): + chip = siliconcompiler.Chip('test') + with pytest.raises(siliconcompiler.core.SiliconCompilerError): + chip.add('option', None, 'test_val') + +def test_set_invalid_field(): + chip = siliconcompiler.Chip('test') + with pytest.raises(siliconcompiler.core.SiliconCompilerError): + chip.set('option', None, 'test_val') + +def test_get_invalid_field(): + chip = siliconcompiler.Chip('test') + with pytest.raises(siliconcompiler.core.SiliconCompilerError): + chip.get('option', None) + +def test_get_invalid_field_continue(): + chip = siliconcompiler.Chip('test') + chip.set('option', 'continue', True) + ret_val = chip.get('option', None) + assert ret_val == None + +def test_set_valid_field_to_none(): + chip = siliconcompiler.Chip('test') + chip.set('option', 'jobscheduler', 'slurm') + chip.set('option', 'jobscheduler', None) + jobscheduler = chip.get('option', 'jobscheduler') + assert jobscheduler == None + assert chip._error == False + def test_set_field_error(): chip = siliconcompiler.Chip('test') chip.set('option', 'continue', True)