From 851fd80df16a0a5cf2c3653b6b059bd8b8f71d0d Mon Sep 17 00:00:00 2001 From: Zach White Date: Wed, 3 Mar 2021 22:31:25 -0800 Subject: [PATCH 1/5] Add check for non-assignment code in rules.mk --- lib/python/qmk/cli/lint.py | 141 +++++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 39 deletions(-) diff --git a/lib/python/qmk/cli/lint.py b/lib/python/qmk/cli/lint.py index 02b31fbc41fe..0bd32e189254 100644 --- a/lib/python/qmk/cli/lint.py +++ b/lib/python/qmk/cli/lint.py @@ -1,72 +1,135 @@ """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 find_readme, keyboard_completer, list_keyboards from qmk.keymap import locate_keymap from qmk.path import is_keyboard, keyboard +def rules_mk_assignment_only(keyboard_path): + """Check the keyboard-level rules.mk to ensure it only has assignments. + """ + current_path = Path() + found_invalid = False + + 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: + cli.log.error('Non-assignment code: +%s %s: %s', i, rules_mk, line) + found_invalid = True + + return not found_invalid + + @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') + failed = [] + + # 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.') + + 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(',') - if not is_keyboard(cli.config.lint.keyboard): - cli.log.error('No such keyboard: %s', cli.config.lint.keyboard) - return False + # Lint each keyboard + for kb in keyboard_list: + if not is_keyboard(kb): + cli.log.error('No such keyboard: %s', kb) + continue - # 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' + # Gather data about the keyboard. + ok = True + keyboard_path = keyboard(kb) + keyboard_info = info_json(kb) + info_path = keyboard_path / 'info.json' + 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.') + # 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) - if cli.config.lint.strict and keyboard_info['parse_warnings']: - ok = False - cli.log.error('Warnings found when generating info.json (Strict mode enabled.)') + 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) - # 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 info.json and warn if it doesn't exist + if not info_path.exists(): + cli.log.warning('%s: Missing %s', kb, info_path) - # Keymap specific checks - if cli.config.lint.keymap: - keymap_path = locate_keymap(cli.config.lint.keyboard, cli.config.lint.keymap) + # Check for a readme.md and throw an error if it doesn't exist + if not readme_path.exists(): + ok = False + cli.log.error('%s: Missing %s', kb, readme_path) - if not keymap_path: + # Check the rules.mk file(s) + if not rules_mk_assignment_only(keyboard_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) + cli.log.error('%s: Non-assignment code found in rules.mk. Move it to processing.mk instead.', kb) - if cli.config.lint.strict: - ok = False + # Keymap specific checks + if cli.config.lint.keymap: + keymap_path = locate_keymap(kb, cli.config.lint.keymap) + + if not keymap_path: + ok = False + cli.log.error("%s: Can't find %s keymap.", kb, cli.config.lint.keymap) + else: + keymap_readme = keymap_path.parent / 'readme.md' + if not keymap_readme.exists(): + cli.log.warning('%s: %s: Missing %s', kb, cli.config.lint.keymap, keymap_readme) + + if cli.config.lint.strict: + ok = False + + 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.info('Lint check passed!') + return True - cli.log.error('Lint check failed!') - return False From a03a66caed0507a3694f3248d90efb85d3e30a1c Mon Sep 17 00:00:00 2001 From: Zach White Date: Wed, 3 Mar 2021 23:30:19 -0800 Subject: [PATCH 2/5] fix lint check --- lib/python/qmk/cli/lint.py | 42 ++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/lib/python/qmk/cli/lint.py b/lib/python/qmk/cli/lint.py index 0bd32e189254..b69ab633054f 100644 --- a/lib/python/qmk/cli/lint.py +++ b/lib/python/qmk/cli/lint.py @@ -11,11 +11,33 @@ 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) + + else: + keymap_readme = keymap_path.parent / 'readme.md' + + if not keymap_readme.exists(): + cli.log.warning('%s: %s: Missing %s', kb, km, keymap_readme) + + if cli.config.lint.strict: + ok = False + + return ok + + def rules_mk_assignment_only(keyboard_path): """Check the keyboard-level rules.mk to ensure it only has assignments. """ current_path = Path() - found_invalid = False + ok = True for path_part in keyboard_path.parts: current_path = current_path / path_part @@ -41,9 +63,9 @@ def rules_mk_assignment_only(keyboard_path): if line and '=' not in line: cli.log.error('Non-assignment code: +%s %s: %s', i, rules_mk, line) - found_invalid = True + ok = False - return not found_invalid + return ok @cli.argument('--strict', action='store_true', help='Treat warnings as errors.') @@ -109,19 +131,10 @@ def lint(cli): # Keymap specific checks if cli.config.lint.keymap: - keymap_path = locate_keymap(kb, cli.config.lint.keymap) - - if not keymap_path: + if not keymap_check(kb, cli.config.lint.keymap): ok = False - cli.log.error("%s: Can't find %s keymap.", kb, cli.config.lint.keymap) - else: - keymap_readme = keymap_path.parent / 'readme.md' - if not keymap_readme.exists(): - cli.log.warning('%s: %s: Missing %s', kb, cli.config.lint.keymap, keymap_readme) - - if cli.config.lint.strict: - ok = False + # Report status if not ok: failed.append(kb) @@ -132,4 +145,3 @@ def lint(cli): cli.log.info('Lint check passed!') return True - From ec90831bcaecb9fadeb24ac2d2926dbe54cdd838 Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 28 Aug 2021 17:56:29 -0700 Subject: [PATCH 3/5] fix lint --- lib/python/qmk/cli/lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/python/qmk/cli/lint.py b/lib/python/qmk/cli/lint.py index b69ab633054f..1e48a4af2403 100644 --- a/lib/python/qmk/cli/lint.py +++ b/lib/python/qmk/cli/lint.py @@ -6,7 +6,7 @@ from qmk.decorators import automagic_keyboard, automagic_keymap from qmk.info import info_json -from qmk.keyboard import find_readme, keyboard_completer, list_keyboards +from qmk.keyboard import keyboard_completer, list_keyboards from qmk.keymap import locate_keymap from qmk.path import is_keyboard, keyboard From d5e38f183eda0e091ef2cf81fcb70726b1b0cc53 Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 28 Aug 2021 18:09:36 -0700 Subject: [PATCH 4/5] fixup to reflect the final state of #8422 --- lib/python/qmk/cli/lint.py | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/lib/python/qmk/cli/lint.py b/lib/python/qmk/cli/lint.py index 1e48a4af2403..1405258c239f 100644 --- a/lib/python/qmk/cli/lint.py +++ b/lib/python/qmk/cli/lint.py @@ -21,15 +21,6 @@ def keymap_check(kb, km): ok = False cli.log.error("%s: Can't find %s keymap.", kb, km) - else: - keymap_readme = keymap_path.parent / 'readme.md' - - if not keymap_readme.exists(): - cli.log.warning('%s: %s: Missing %s', kb, km, keymap_readme) - - if cli.config.lint.strict: - ok = False - return ok @@ -37,7 +28,7 @@ def rules_mk_assignment_only(keyboard_path): """Check the keyboard-level rules.mk to ensure it only has assignments. """ current_path = Path() - ok = True + errors = [] for path_part in keyboard_path.parts: current_path = current_path / path_part @@ -62,10 +53,9 @@ def rules_mk_assignment_only(keyboard_path): continue if line and '=' not in line: - cli.log.error('Non-assignment code: +%s %s: %s', i, rules_mk, line) - ok = False + errors.append(f'Non-assignment code on line +{i} {rules_mk}: {line}') - return ok + return errors @cli.argument('--strict', action='store_true', help='Treat warnings as errors.') @@ -104,7 +94,6 @@ def lint(cli): keyboard_path = keyboard(kb) keyboard_info = info_json(kb) info_path = keyboard_path / 'info.json' - readme_path = keyboard_path / 'readme.md' # Check for errors in the info.json if keyboard_info['parse_errors']: @@ -115,19 +104,13 @@ def lint(cli): ok = False cli.log.error('%s: Warnings found when generating info.json (Strict mode enabled.)', kb) - # Check for info.json and warn if it doesn't exist - if not info_path.exists(): - cli.log.warning('%s: Missing %s', kb, info_path) - - # Check for a readme.md and throw an error if it doesn't exist - if not readme_path.exists(): - ok = False - cli.log.error('%s: Missing %s', kb, readme_path) - # Check the rules.mk file(s) - if not rules_mk_assignment_only(keyboard_path): + rules_mk_assignment_errors = rules_mk_assignment_only(keyboard_path) + if rules_mk_assignment_errors: ok = False - cli.log.error('%s: Non-assignment code found in rules.mk. Move it to processing.mk instead.', kb) + 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) # Keymap specific checks if cli.config.lint.keymap: From d51eb0d0e0ca993327c920cfd6d749fe00cd2d44 Mon Sep 17 00:00:00 2001 From: Zach White Date: Sat, 28 Aug 2021 18:51:29 -0700 Subject: [PATCH 5/5] fix lint --- lib/python/qmk/cli/lint.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/python/qmk/cli/lint.py b/lib/python/qmk/cli/lint.py index 1405258c239f..008ec1393d39 100644 --- a/lib/python/qmk/cli/lint.py +++ b/lib/python/qmk/cli/lint.py @@ -93,7 +93,6 @@ def lint(cli): ok = True keyboard_path = keyboard(kb) keyboard_info = info_json(kb) - info_path = keyboard_path / 'info.json' # Check for errors in the info.json if keyboard_info['parse_errors']: