From f36c2229d552942a017e577ae93cd4b54ec1fd0e Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 28 Aug 2021 19:37:55 -0700 Subject: [PATCH] Add check for non-assignment code in rules.mk (#12108) * Add check for non-assignment code in rules.mk * fix lint check * fix lint * fixup to reflect the final state of #8422 * fix lint --- lib/python/qmk/cli/lint.py | 139 ++++++++++++++++++++++++++----------- 1 file changed, 98 insertions(+), 41 deletions(-) diff --git a/lib/python/qmk/cli/lint.py b/lib/python/qmk/cli/lint.py index 02b31fbc41fe..008ec1393d39 100644 --- a/lib/python/qmk/cli/lint.py +++ b/lib/python/qmk/cli/lint.py @@ -1,72 +1,129 @@ """Command to look over a keyboard/keymap and check for common mistakes. """ +from pathlib import Path + from milc import cli from qmk.decorators import automagic_keyboard, automagic_keymap from qmk.info import info_json -from qmk.keyboard import find_readme, keyboard_completer +from qmk.keyboard import keyboard_completer, list_keyboards from qmk.keymap import locate_keymap from qmk.path import is_keyboard, keyboard +def keymap_check(kb, km): + """Perform the keymap level checks. + """ + ok = True + keymap_path = locate_keymap(kb, km) + + if not keymap_path: + ok = False + cli.log.error("%s: Can't find %s keymap.", kb, km) + + return ok + + +def rules_mk_assignment_only(keyboard_path): + """Check the keyboard-level rules.mk to ensure it only has assignments. + """ + current_path = Path() + errors = [] + + for path_part in keyboard_path.parts: + current_path = current_path / path_part + rules_mk = current_path / 'rules.mk' + + if rules_mk.exists(): + continuation = None + + for i, line in enumerate(rules_mk.open()): + line = line.strip() + + if '#' in line: + line = line[:line.index('#')] + + if continuation: + line = continuation + line + continuation = None + + if line: + if line[-1] == '\\': + continuation = line[:-1] + continue + + if line and '=' not in line: + errors.append(f'Non-assignment code on line +{i} {rules_mk}: {line}') + + return errors + + @cli.argument('--strict', action='store_true', help='Treat warnings as errors.') @cli.argument('-kb', '--keyboard', completer=keyboard_completer, help='The keyboard to check.') @cli.argument('-km', '--keymap', help='The keymap to check.') +@cli.argument('--all-kb', action='store_true', arg_only=True, help='Check all keyboards.') @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 + failed = [] - if not is_keyboard(cli.config.lint.keyboard): - cli.log.error('No such keyboard: %s', cli.config.lint.keyboard) - return False + # Determine our keyboard list + if cli.args.all_kb: + if cli.args.keyboard: + cli.log.warning('Both --all-kb and --keyboard passed, --all-kb takes presidence.') - # Gather data about the keyboard. - ok = True - keyboard_path = keyboard(cli.config.lint.keyboard) - keyboard_info = info_json(cli.config.lint.keyboard) - readme_path = find_readme(cli.config.lint.keyboard) - missing_readme_path = keyboard_path / 'readme.md' + keyboard_list = list_keyboards() + elif not cli.config.lint.keyboard: + cli.log.error('Missing required arguments: --keyboard or --all-kb') + cli.print_help() + return False + else: + keyboard_list = cli.args.keyboard.split(',') - # Check for errors in the info.json - if keyboard_info['parse_errors']: - ok = False - cli.log.error('Errors found when generating info.json.') + # Lint each keyboard + for kb in keyboard_list: + if not is_keyboard(kb): + cli.log.error('No such keyboard: %s', kb) + continue - if cli.config.lint.strict and keyboard_info['parse_warnings']: - ok = False - cli.log.error('Warnings found when generating info.json (Strict mode enabled.)') + # Gather data about the keyboard. + ok = True + keyboard_path = keyboard(kb) + keyboard_info = info_json(kb) - # Check for a readme.md and warn if it doesn't exist - if not readme_path: - ok = False - cli.log.error('Missing %s', missing_readme_path) + # Check for errors in the info.json + if keyboard_info['parse_errors']: + ok = False + cli.log.error('%s: Errors found when generating info.json.', kb) - # Keymap specific checks - if cli.config.lint.keymap: - keymap_path = locate_keymap(cli.config.lint.keyboard, cli.config.lint.keymap) + if cli.config.lint.strict and keyboard_info['parse_warnings']: + ok = False + cli.log.error('%s: Warnings found when generating info.json (Strict mode enabled.)', kb) - if not keymap_path: + # Check the rules.mk file(s) + rules_mk_assignment_errors = rules_mk_assignment_only(keyboard_path) + if rules_mk_assignment_errors: 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) + cli.log.error('%s: Non-assignment code found in rules.mk. Move it to post_rules.mk instead.', kb) + for assignment_error in rules_mk_assignment_errors: + cli.log.error(assignment_error) - if cli.config.lint.strict: - ok = False + # Keymap specific checks + if cli.config.lint.keymap: + if not keymap_check(kb, cli.config.lint.keymap): + ok = False + + # Report status + if not ok: + failed.append(kb) # Check and report the overall status - if ok: - cli.log.info('Lint check passed!') - return True + if failed: + cli.log.error('Lint check failed for: %s', ', '.join(failed)) + return False - cli.log.error('Lint check failed!') - return False + cli.log.info('Lint check passed!') + return True