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 <fauxpark@gmail.com>

* honor --strict in more places

* change the job name to lint

Co-authored-by: Ryan <fauxpark@gmail.com>

lib/python/qmk/cli/__init__.py

@@ -19,6 +19,7 @@ from . import hello
 from . import info
 from . import json
 from . import json2c
+from . import lint
 from . import list
 from . import kle2json
 from . import new

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

lib/python/qmk/info.py

@@ -28,6 +28,8 @@ def info_json(keyboard):
         'keyboard_folder': str(keyboard),
         'keymaps': {},
         'layouts': {},
+        'parse_errors': [],
+        'parse_warnings': [],
         'maintainer': 'qmk',
     }
 
@@ -36,7 +38,7 @@ def info_json(keyboard):
         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(keyboard, rules).items():
+    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
 
@@ -104,14 +106,16 @@ def _extract_rules_mk(info_data):
     mcu = rules.get('MCU')
 
     if mcu in CHIBIOS_PROCESSORS:
-        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 arm_processor_rules(info_data, rules)
 
-    return info_data
+    elif mcu in LUFA_PROCESSORS + VUSB_PROCESSORS:
+        return avr_processor_rules(info_data, rules)
+
+    msg = "Unknown MCU: " + str(mcu)
+
+    _log_warning(info_data, msg)
+
+    return unknown_processor_rules(info_data, rules)
 
 
 def _search_keyboard_h(path):
@@ -127,7 +131,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))
@@ -135,7 +139,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)
@@ -153,11 +157,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.
     """
@@ -216,7 +234,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`
@@ -230,7 +248,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])

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):