From f90458aff6c0fd08d8f6fa2798e2beb965fb9365 Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 7 Nov 2020 09:56:08 -0800 Subject: [PATCH] New command: qmk lint (#10761) * Basic qmk lint command * check for keymap readme * change the workflow from qmk info to qmk lint * add a strict mode * parsing -> parse * document qmk lint * small info logging cleanup * Apply suggestions from code review Co-authored-by: Ryan * honor --strict in more places * change the job name to lint Co-authored-by: Ryan --- lib/python/qmk/cli/__init__.py | 1 + lib/python/qmk/cli/lint.py | 70 ++++++++++++++++++++++++++++++++++ lib/python/qmk/info.py | 48 +++++++++++++++++------ lib/python/qmk/path.py | 14 +++++-- 4 files changed, 117 insertions(+), 16 deletions(-) create mode 100644 lib/python/qmk/cli/lint.py diff --git a/lib/python/qmk/cli/__init__.py b/lib/python/qmk/cli/__init__.py index 3702b51db199..1757792b310f 100644 --- a/lib/python/qmk/cli/__init__.py +++ b/lib/python/qmk/cli/__init__.py @@ -18,6 +18,7 @@ from . import info from . import json from . import json2c +from . import lint from . import list from . import kle2json from . import new diff --git a/lib/python/qmk/cli/lint.py b/lib/python/qmk/cli/lint.py new file mode 100644 index 000000000000..74467021e075 --- /dev/null +++ b/lib/python/qmk/cli/lint.py @@ -0,0 +1,70 @@ +"""Command to look over a keyboard/keymap and check for common mistakes. +""" +from milc import cli + +from qmk.decorators import automagic_keyboard, automagic_keymap +from qmk.info import info_json +from qmk.keymap import locate_keymap +from qmk.path import is_keyboard, keyboard + + +@cli.argument('--strict', action='store_true', help='Treat warnings as errors.') +@cli.argument('-kb', '--keyboard', help='The keyboard to check.') +@cli.argument('-km', '--keymap', help='The keymap to check.') +@cli.subcommand('Check keyboard and keymap for common mistakes.') +@automagic_keyboard +@automagic_keymap +def lint(cli): + """Check keyboard and keymap for common mistakes. + """ + if not cli.config.lint.keyboard: + cli.log.error('Missing required argument: --keyboard') + cli.print_help() + return False + + if not is_keyboard(cli.config.lint.keyboard): + cli.log.error('No such keyboard: %s', cli.config.lint.keyboard) + return False + + # Gather data about the keyboard. + ok = True + keyboard_path = keyboard(cli.config.lint.keyboard) + keyboard_info = info_json(cli.config.lint.keyboard) + readme_path = keyboard_path / 'readme.md' + + # Check for errors in the info.json + if keyboard_info['parse_errors']: + ok = False + cli.log.error('Errors found when generating info.json.') + + if cli.config.lint.strict and keyboard_info['parse_warnings']: + ok = False + cli.log.error('Warnings found when generating info.json (Strict mode enabled.)') + + # Check for a readme.md and warn if it doesn't exist + if not readme_path.exists(): + ok = False + cli.log.error('Missing %s', readme_path) + + # Keymap specific checks + if cli.config.lint.keymap: + keymap_path = locate_keymap(cli.config.lint.keyboard, cli.config.lint.keymap) + + if not keymap_path: + ok = False + cli.log.error("Can't find %s keymap for %s keyboard.", cli.config.lint.keymap, cli.config.lint.keyboard) + else: + keymap_readme = keymap_path.parent / 'readme.md' + if not keymap_readme.exists(): + cli.log.warning('Missing %s', keymap_readme) + + if cli.config.lint.strict: + ok = False + + # Check and report the overall status + if ok: + cli.log.info('Lint check passed!') + return True + + cli.log.error('Lint check failed!') + return False diff --git a/lib/python/qmk/info.py b/lib/python/qmk/info.py index 0e540c00a827..e8a44a33f904 100644 --- a/lib/python/qmk/info.py +++ b/lib/python/qmk/info.py @@ -26,10 +26,17 @@ def info_json(keyboard): 'keyboard_name': str(keyboard), 'keyboard_folder': str(keyboard), 'layouts': {}, + 'parse_errors': [], + 'parse_warnings': [], 'maintainer': 'qmk', } - for layout_name, layout_json in _find_all_layouts(keyboard, rules).items(): + # Populate the list of JSON keymaps + for keymap in list_keymaps(keyboard, c=False, fullpath=True): + info_data['keymaps'][keymap.name] = {'url': f'https://raw.githubusercontent.com/qmk/qmk_firmware/master/{keymap}/keymap.json'} + + # Populate layout data + for layout_name, layout_json in _find_all_layouts(info_data, keyboard, rules).items(): if not layout_name.startswith('LAYOUT_kc'): info_data['layouts'][layout_name] = layout_json @@ -96,14 +103,16 @@ def _extract_rules_mk(info_data): mcu = rules.get('MCU') if mcu in CHIBIOS_PROCESSORS: - arm_processor_rules(info_data, rules) + return arm_processor_rules(info_data, rules) + elif mcu in LUFA_PROCESSORS + VUSB_PROCESSORS: - avr_processor_rules(info_data, rules) - else: - cli.log.warning("%s: Unknown MCU: %s" % (info_data['keyboard_folder'], mcu)) - unknown_processor_rules(info_data, rules) + return avr_processor_rules(info_data, rules) - return info_data + msg = "Unknown MCU: " + str(mcu) + + _log_warning(info_data, msg) + + return unknown_processor_rules(info_data, rules) def _search_keyboard_h(path): @@ -119,7 +128,7 @@ def _search_keyboard_h(path): return layouts -def _find_all_layouts(keyboard, rules): +def _find_all_layouts(info_data, keyboard, rules): """Looks for layout macros associated with this keyboard. """ layouts = _search_keyboard_h(Path(keyboard)) @@ -127,7 +136,7 @@ def _find_all_layouts(keyboard, rules): if not layouts: # If we didn't find any layouts above we widen our search. This is error # prone which is why we want to encourage people to follow the standard above. - cli.log.warning('%s: Falling back to searching for KEYMAP/LAYOUT macros.' % (keyboard)) + _log_warning(info_data, 'Falling back to searching for KEYMAP/LAYOUT macros.') for file in glob('keyboards/%s/*.h' % keyboard): if file.endswith('.h'): these_layouts = find_layouts(file) @@ -145,11 +154,25 @@ def _find_all_layouts(keyboard, rules): supported_layouts.remove(layout_name) if supported_layouts: - cli.log.error('%s: Missing LAYOUT() macro for %s' % (keyboard, ', '.join(supported_layouts))) + _log_error(info_data, 'Missing LAYOUT() macro for %s' % (', '.join(supported_layouts))) return layouts +def _log_error(info_data, message): + """Send an error message to both JSON and the log. + """ + info_data['parse_errors'].append(message) + cli.log.error('%s: %s', info_data.get('keyboard_folder', 'Unknown Keyboard!'), message) + + +def _log_warning(info_data, message): + """Send a warning message to both JSON and the log. + """ + info_data['parse_warnings'].append(message) + cli.log.warning('%s: %s', info_data.get('keyboard_folder', 'Unknown Keyboard!'), message) + + def arm_processor_rules(info_data, rules): """Setup the default info for an ARM board. """ @@ -208,7 +231,7 @@ def merge_info_jsons(keyboard, info_data): new_info_data = json.load(info_fd) if not isinstance(new_info_data, dict): - cli.log.error("Invalid file %s, root object should be a dictionary.", str(info_file)) + _log_error(info_data, "Invalid file %s, root object should be a dictionary.", str(info_file)) continue # Copy whitelisted keys into `info_data` @@ -222,7 +245,8 @@ def merge_info_jsons(keyboard, info_data): # Only pull in layouts we have a macro for if layout_name in info_data['layouts']: if info_data['layouts'][layout_name]['key_count'] != len(json_layout['layout']): - cli.log.error('%s: %s: Number of elements in info.json does not match! info.json:%s != %s:%s', info_data['keyboard_folder'], layout_name, len(json_layout['layout']), layout_name, len(info_data['layouts'][layout_name]['layout'])) + msg = '%s: Number of elements in info.json does not match! info.json:%s != %s:%s' + _log_error(info_data, msg % (layout_name, len(json_layout['layout']), layout_name, len(info_data['layouts'][layout_name]['layout']))) else: for i, key in enumerate(info_data['layouts'][layout_name]['layout']): key.update(json_layout['layout'][i]) diff --git a/lib/python/qmk/path.py b/lib/python/qmk/path.py index 591fad034b21..54def1d5d6cc 100644 --- a/lib/python/qmk/path.py +++ b/lib/python/qmk/path.py @@ -28,15 +28,21 @@ def under_qmk_firmware(): return None -def keymap(keyboard): +def keyboard(keyboard_name): + """Returns the path to a keyboard's directory relative to the qmk root. + """ + return Path('keyboards') / keyboard_name + + +def keymap(keyboard_name): """Locate the correct directory for storing a keymap. Args: - keyboard + keyboard_name The name of the keyboard. Example: clueboard/66/rev3 """ - keyboard_folder = Path('keyboards') / keyboard + keyboard_folder = keyboard(keyboard_name) for i in range(MAX_KEYBOARD_SUBFOLDERS): if (keyboard_folder / 'keymaps').exists(): @@ -45,7 +51,7 @@ def keymap(keyboard): keyboard_folder = keyboard_folder.parent logging.error('Could not find the keymaps directory!') - raise NoSuchKeyboardError('Could not find keymaps directory for: %s' % keyboard) + raise NoSuchKeyboardError('Could not find keymaps directory for: %s' % keyboard_name) def normpath(path):