From a599550adbd6b1291509d7cdc7ea61c92550a60c Mon Sep 17 00:00:00 2001 From: Joel Challis Date: Thu, 9 Jun 2022 21:02:16 +0100 Subject: [PATCH] Add support for linting deprecated and removed functionality (#17063) * Add support for more lint warnings/errors * Develop currently needs extra deps installed * Lint a few more scenarios * fix tests --- .github/workflows/lint.yml | 3 + data/mappings/info_config.json | 10 +- data/mappings/info_rules.json | 9 +- lib/python/qmk/info.py | 120 ++++++++++------------ lib/python/qmk/tests/test_cli_commands.py | 1 - 5 files changed, 73 insertions(+), 70 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c7a8624ee1..ab694ee668 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,6 +16,9 @@ jobs: with: fetch-depth: 0 + - name: Install dependencies + run: pip3 install -r requirements-dev.txt + - uses: trilom/file-changes-action@v1.2.4 id: file_changes with: diff --git a/data/mappings/info_config.json b/data/mappings/info_config.json index d9f96b5892..5f0d903bd7 100644 --- a/data/mappings/info_config.json +++ b/data/mappings/info_config.json @@ -7,6 +7,8 @@ # to_json: Default `true`. Set to `false` to exclude this mapping from info.json # to_c: Default `true`. Set to `false` to exclude this mapping from config.h # warn_duplicate: Default `true`. Set to `false` to turn off warning when a value exists in both places + # deprecated: Default `false`. Set to `true` to turn on warning when a value exists + # invalid: Default `false`. Set to `true` to generate errors when a value exists "AUDIO_VOICES": {"info_key": "audio.voices", "value_type": "bool"}, "BACKLIGHT_BREATHING": {"info_key": "backlight.breathing", "value_type": "bool"}, "BREATHING_PERIOD": {"info_key": "backlight.breathing_period", "value_type": "int"}, @@ -19,7 +21,6 @@ "DEVICE_VER": {"info_key": "usb.device_ver", "value_type": "hex"}, # TODO: Replace ^^^ with vvv #"DEVICE_VER": {"info_key": "usb.device_version", "value_type": "bcd_version"}, - "DESCRIPTION": {"info_key": "keyboard_folder", "value_type": "str", "to_json": false}, "DIODE_DIRECTION": {"info_key": "diode_direction"}, "DOUBLE_TAP_SHIFT_TURNS_ON_CAPS_WORD": {"info_key": "caps_word.double_tap_shift_turns_on", "value_type": "bool"}, "FORCE_NKRO": {"info_key": "usb.force_nkro", "value_type": "bool"}, @@ -102,4 +103,11 @@ "USB_MAX_POWER_CONSUMPTION": {"info_key": "usb.max_power", "value_type": "int"}, "USB_POLLING_INTERVAL_MS": {"info_key": "usb.polling_interval", "value_type": "int"}, "USB_SUSPEND_WAKEUP_DELAY": {"info_key": "usb.suspend_wakeup_delay", "value_type": "int"}, + + # Items we want flagged in lint + "NO_ACTION_MACRO": {"info_key": "_invalid.no_action_macro", "invalid": true}, + "NO_ACTION_FUNCTION": {"info_key": "_invalid.no_action_function", "invalid": true}, + "DESCRIPTION": {"info_key": "_invalid.usb_description", "invalid": true}, + "DEBOUNCING_DELAY": {"info_key": "_invalid.debouncing_delay", "invalid": true}, + "PREVENT_STUCK_MODIFIERS": {"info_key": "_invalid.prevent_stuck_mods", "invalid": true}, } diff --git a/data/mappings/info_rules.json b/data/mappings/info_rules.json index a8b39afbd1..d4eec37ba0 100644 --- a/data/mappings/info_rules.json +++ b/data/mappings/info_rules.json @@ -7,6 +7,8 @@ # to_json: Default `true`. Set to `false` to exclude this mapping from info.json # to_c: Default `true`. Set to `false` to exclude this mapping from rules.mk # warn_duplicate: Default `true`. Set to `false` to turn off warning when a value exists in both places + # deprecated: Default `false`. Set to `true` to turn on warning when a value exists + # invalid: Default `false`. Set to `true` to generate errors when a value exists "BOARD": {"info_key": "board"}, "BOOTLOADER": {"info_key": "bootloader", "warn_duplicate": false}, "BLUETOOTH": {"info_key": "bluetooth.driver"}, @@ -24,5 +26,10 @@ "SECURE_ENABLE": {"info_key": "secure.enabled", "value_type": "bool"}, "SPLIT_KEYBOARD": {"info_key": "split.enabled", "value_type": "bool"}, "SPLIT_TRANSPORT": {"info_key": "split.transport.protocol", "to_c": false}, - "WAIT_FOR_USB": {"info_key": "usb.wait_for", "value_type": "bool"} + "WAIT_FOR_USB": {"info_key": "usb.wait_for", "value_type": "bool"}, + + # Items we want flagged in lint + "CTPC": {"info_key": "_deprecated.ctpc", "deprecated": true}, + "CONVERT_TO_PROTON_C": {"info_key": "_deprecated.ctpc", "deprecated": true}, + "VIAL_ENABLE": {"info_key": "_invalid.vial", "invalid": true}, } diff --git a/lib/python/qmk/info.py b/lib/python/qmk/info.py index 0763433b3d..6ff9cba45b 100644 --- a/lib/python/qmk/info.py +++ b/lib/python/qmk/info.py @@ -440,6 +440,47 @@ def _extract_device_version(info_data): info_data['usb']['device_version'] = f'{major}.{minor}.{revision}' +def _config_to_json(key_type, config_value): + """Convert config value using spec + """ + if key_type.startswith('array'): + if '.' in key_type: + key_type, array_type = key_type.split('.', 1) + else: + array_type = None + + config_value = config_value.replace('{', '').replace('}', '').strip() + + if array_type == 'int': + return list(map(int, config_value.split(','))) + else: + return config_value.split(',') + + elif key_type == 'bool': + return config_value in true_values + + elif key_type == 'hex': + return '0x' + config_value[2:].upper() + + elif key_type == 'list': + return config_value.split() + + elif key_type == 'int': + return int(config_value) + + elif key_type == 'str': + return config_value.strip('"') + + elif key_type == 'bcd_version': + major = int(config_value[2:4]) + minor = int(config_value[4]) + revision = int(config_value[5]) + + return f'{major}.{minor}.{revision}' + + return config_value + + def _extract_config_h(info_data, config_c): """Pull some keyboard information from existing config.h files """ @@ -452,47 +493,16 @@ def _extract_config_h(info_data, config_c): key_type = info_dict.get('value_type', 'raw') try: + if config_key in config_c and info_dict.get('invalid', False): + _log_error(info_data, '%s in config.h is no longer a valid option' % config_key) + elif config_key in config_c and info_dict.get('deprecated', False): + _log_warning(info_data, '%s in config.h is deprecated and will be removed at a later date' % config_key) + if config_key in config_c and info_dict.get('to_json', True): if dotty_info.get(info_key) and info_dict.get('warn_duplicate', True): _log_warning(info_data, '%s in config.h is overwriting %s in info.json' % (config_key, info_key)) - if key_type.startswith('array'): - if '.' in key_type: - key_type, array_type = key_type.split('.', 1) - else: - array_type = None - - config_value = config_c[config_key].replace('{', '').replace('}', '').strip() - - if array_type == 'int': - dotty_info[info_key] = list(map(int, config_value.split(','))) - else: - dotty_info[info_key] = config_value.split(',') - - elif key_type == 'bool': - dotty_info[info_key] = config_c[config_key] in true_values - - elif key_type == 'hex': - dotty_info[info_key] = '0x' + config_c[config_key][2:].upper() - - elif key_type == 'list': - dotty_info[info_key] = config_c[config_key].split() - - elif key_type == 'int': - dotty_info[info_key] = int(config_c[config_key]) - - elif key_type == 'str': - dotty_info[info_key] = config_c[config_key].strip('"') - - elif key_type == 'bcd_version': - major = int(config_c[config_key][2:4]) - minor = int(config_c[config_key][4]) - revision = int(config_c[config_key][5]) - - dotty_info[info_key] = f'{major}.{minor}.{revision}' - - else: - dotty_info[info_key] = config_c[config_key] + dotty_info[info_key] = _config_to_json(key_type, config_c[config_key]) except Exception as e: _log_warning(info_data, f'{config_key}->{info_key}: {e}') @@ -547,40 +557,16 @@ def _extract_rules_mk(info_data, rules): key_type = info_dict.get('value_type', 'raw') try: + if rules_key in rules and info_dict.get('invalid', False): + _log_error(info_data, '%s in rules.mk is no longer a valid option' % rules_key) + elif rules_key in rules and info_dict.get('deprecated', False): + _log_warning(info_data, '%s in rules.mk is deprecated and will be removed at a later date' % rules_key) + if rules_key in rules and info_dict.get('to_json', True): if dotty_info.get(info_key) and info_dict.get('warn_duplicate', True): _log_warning(info_data, '%s in rules.mk is overwriting %s in info.json' % (rules_key, info_key)) - if key_type.startswith('array'): - if '.' in key_type: - key_type, array_type = key_type.split('.', 1) - else: - array_type = None - - rules_value = rules[rules_key].replace('{', '').replace('}', '').strip() - - if array_type == 'int': - dotty_info[info_key] = list(map(int, rules_value.split(','))) - else: - dotty_info[info_key] = rules_value.split(',') - - elif key_type == 'list': - dotty_info[info_key] = rules[rules_key].split() - - elif key_type == 'bool': - dotty_info[info_key] = rules[rules_key] in true_values - - elif key_type == 'hex': - dotty_info[info_key] = '0x' + rules[rules_key][2:].upper() - - elif key_type == 'int': - dotty_info[info_key] = int(rules[rules_key]) - - elif key_type == 'str': - dotty_info[info_key] = rules[rules_key].strip('"') - - else: - dotty_info[info_key] = rules[rules_key] + dotty_info[info_key] = _config_to_json(key_type, rules[rules_key]) except Exception as e: _log_warning(info_data, f'{rules_key}->{info_key}: {e}') diff --git a/lib/python/qmk/tests/test_cli_commands.py b/lib/python/qmk/tests/test_cli_commands.py index d40d4bf573..2463543ef1 100644 --- a/lib/python/qmk/tests/test_cli_commands.py +++ b/lib/python/qmk/tests/test_cli_commands.py @@ -259,7 +259,6 @@ def test_generate_config_h(): result = check_subcommand('generate-config-h', '-kb', 'handwired/pytest/basic') check_returncode(result) assert '# define DEVICE_VER 0x0001' in result.stdout - assert '# define DESCRIPTION "handwired/pytest/basic"' in result.stdout assert '# define DIODE_DIRECTION COL2ROW' in result.stdout assert '# define MANUFACTURER none' in result.stdout assert '# define PRODUCT pytest' in result.stdout