Skip to content

Commit

Permalink
New command: qmk lint (qmk#10761)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
2 people authored and drashna committed Nov 24, 2020
1 parent 81b4caa commit f90458a
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 16 deletions.
1 change: 1 addition & 0 deletions lib/python/qmk/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
70 changes: 70 additions & 0 deletions lib/python/qmk/cli/lint.py
Original file line number Diff line number Diff line change
@@ -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
48 changes: 36 additions & 12 deletions lib/python/qmk/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand All @@ -119,15 +128,15 @@ 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))

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)
Expand All @@ -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.
"""
Expand Down Expand Up @@ -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`
Expand All @@ -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])
Expand Down
14 changes: 10 additions & 4 deletions lib/python/qmk/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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):
Expand Down

0 comments on commit f90458a

Please sign in to comment.