Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perform stricter lint checks #17094

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 85 additions & 38 deletions lib/python/qmk/cli/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,44 @@

from qmk.decorators import automagic_keyboard, automagic_keymap
from qmk.info import info_json
from qmk.keyboard import keyboard_completer, list_keyboards
from qmk.keymap import locate_keymap
from qmk.keyboard import keyboard_completer, keyboard_folder, list_keyboards
from qmk.keymap import locate_keymap, list_keymaps
from qmk.path import is_keyboard, keyboard
from qmk.git import git_get_ignored_files


def keymap_check(kb, km):
"""Perform the keymap level checks.
def _list_defaultish_keymaps(kb):
"""Return default like keymaps for a given keyboard
"""
ok = True
keymap_path = locate_keymap(kb, km)
defaultish = ['ansi', 'iso', 'via']

if not keymap_path:
keymaps = set()
for x in list_keymaps(kb):
if x in defaultish or x.startswith('default'):
keymaps.add(x)

return keymaps


def _handle_json_errors(kb, info):
"""Convert any json errors into lint errors
"""
ok = True
# Check for errors in the json
if info['parse_errors']:
ok = False
cli.log.error("%s: Can't find %s keymap.", kb, km)
cli.log.error(f'{kb}: Errors found when generating info.json.')

if cli.config.lint.strict and info['parse_warnings']:
ok = False
cli.log.error(f'{kb}: Warnings found when generating info.json (Strict mode enabled.)')
return ok


def rules_mk_assignment_only(keyboard_path):
def _rules_mk_assignment_only(kb):
"""Check the keyboard-level rules.mk to ensure it only has assignments.
"""
keyboard_path = keyboard(kb)
current_path = Path()
errors = []

Expand Down Expand Up @@ -58,8 +75,53 @@ def rules_mk_assignment_only(keyboard_path):
return errors


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

# Additional checks
invalid_files = git_get_ignored_files(keymap_path.parent)
for file in invalid_files:
cli.log.error(f'{kb}/{km}: The file "{file}" should not exist!')
ok = False

return ok


def keyboard_check(kb):
"""Perform the keyboard level checks.
"""
ok = True
kb_info = info_json(kb)

if not _handle_json_errors(kb, kb_info):
ok = False

# Additional checks
rules_mk_assignment_errors = _rules_mk_assignment_only(kb)
if rules_mk_assignment_errors:
ok = False
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)

invalid_files = git_get_ignored_files(f'keyboards/{kb}/')
for file in invalid_files:
cli.log.error(f'{kb}: The file "{file}" should not exist!')
ok = False

return ok


@cli.argument('--strict', action='store_true', help='Treat warnings as errors')
@cli.argument('-kb', '--keyboard', completer=keyboard_completer, help='Comma separated list of keyboards to check')
@cli.argument('-kb', '--keyboard', type=keyboard_folder, completer=keyboard_completer, help='Comma separated list of keyboards 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.')
Expand All @@ -73,7 +135,7 @@ def lint(cli):
# 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.')
cli.log.warning('Both --all-kb and --keyboard passed, --all-kb takes precedence.')

keyboard_list = list_keyboards()
elif not cli.config.lint.keyboard:
Expand All @@ -89,38 +151,23 @@ def lint(cli):
cli.log.error('No such keyboard: %s', kb)
continue

# Gather data about the keyboard.
ok = True
keyboard_path = keyboard(kb)
keyboard_info = info_json(kb)

# 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)
# Determine keymaps to also check
if cli.config.lint.keymap:
keymaps = {cli.config.lint.keymap}
else:
keymaps = _list_defaultish_keymaps(kb)
# Ensure that at least a 'default' keymap always exists
keymaps.add('default')

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)
ok = True

# Check the rules.mk file(s)
rules_mk_assignment_errors = rules_mk_assignment_only(keyboard_path)
if rules_mk_assignment_errors:
# keyboard level checks
if not keyboard_check(kb):
ok = False
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:
if not keymap_check(kb, cli.config.lint.keymap):
ok = False

# Check if all non-data driven macros exist in <keyboard.h>
for layout, data in keyboard_info['layouts'].items():
# Matrix data should be a list with exactly two integers: [0, 1]
if not data['c_macro'] and not all('matrix' in key_data.keys() or len(key_data) == 2 or all(isinstance(n, int) for n in key_data) for key_data in data['layout']):
cli.log.error(f'{kb}: "{layout}" has no "matrix" definition in either "info.json" or "<keyboard>.h"!')
for keymap in keymaps:
if not keymap_check(kb, keymap):
ok = False

# Report status
Expand Down
9 changes: 9 additions & 0 deletions lib/python/qmk/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,12 @@ def git_check_deviation(active_branch):
cli.run(['git', 'fetch', 'upstream', active_branch])
deviations = cli.run(['git', '--no-pager', 'log', f'upstream/{active_branch}...{active_branch}'])
return bool(deviations.returncode)


def git_get_ignored_files(check_dir='.'):
"""Return a list of files that would be captured by the current .gitingore
"""
invalid = cli.run(['git', 'ls-files', '-c', '-o', '-i', '--exclude-standard', check_dir])
if invalid.returncode != 0:
return []
return invalid.stdout.strip().splitlines()