From a6f71d78001063955e64a6658b2b5db33d5e22b4 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 11 Aug 2022 16:33:39 +0100 Subject: [PATCH] Hardcoded error numbers for the style checks to prevent error number drift in future Added an --ignore/-n command to exclude style rules. added testing for ignore --- cylc/flow/scripts/lint.py | 55 ++++++++++++++++++++++++--------- tests/unit/scripts/test_lint.py | 30 ++++++++++++++---- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index a1d917f7f43..c9d086976df 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -83,17 +83,20 @@ STYLE_CHECKS = { re.compile(r'^\t'): { 'short': 'Use multiple spaces, not tabs', - 'url': STYLE_GUIDE + 'tab-characters' + 'url': STYLE_GUIDE + 'tab-characters', + 'index': 1 }, # Not a full test, but if a non section is not indented... re.compile(r'^[^\{\[|\s]'): { 'short': 'Item not indented.', - 'url': STYLE_GUIDE + 'indentation' + 'url': STYLE_GUIDE + 'indentation', + 'index': 2 }, # [section] re.compile(r'^\s+\[[^\[.]*\]'): { 'short': 'Top level sections should not be indented.', - 'url': STYLE_GUIDE + 'indentation' + 'url': STYLE_GUIDE + 'indentation', + 'index': 3 }, # 2 or 4 space indentation both seem reasonable: re.compile(r'^(|\s|\s{2,3}|\s{5,})\[\[[^\[.]*\]\]'): { @@ -101,28 +104,32 @@ 'Second level sections should be indented exactly ' '4 spaces.' ), - 'url': STYLE_GUIDE + 'indentation' + 'url': STYLE_GUIDE + 'indentation', + 'index': 4 }, re.compile(r'^(|\s{1,7}|\s{9,})\[\[\[[^\[.]*\]\]\]'): { 'short': ( 'Third level sections should be indented exactly ' '8 spaces.' ), - 'url': STYLE_GUIDE + 'indentation' + 'url': STYLE_GUIDE + 'indentation', + 'index': 5 }, re.compile(r'\s$'): { 'short': 'trailing whitespace.', - 'url': STYLE_GUIDE + 'trailing-whitespace' + 'url': STYLE_GUIDE + 'trailing-whitespace', + 'index': 6 + }, + re.compile(r'inherit\s*=\s*[a-z].*$'): { + 'short': 'Family name contains lowercase characters.', + 'url': STYLE_GUIDE + 'task-naming-conventions', + 'index': 7 }, # Consider re-adding this as an option later: # re.compile(r'^.{80,}'): { # 'short': 'line > 79 characters.', - # 'url': STYLE_GUIDE + 'line-length-and-continuation' + # 'url': STYLE_GUIDE + 'line-length-and-continuation', # }, - re.compile(r'inherit\s*=\s*[a-z].*$'): { - 'short': 'Family name contains lowercase characters.', - 'url': STYLE_GUIDE + 'task-naming-conventions' - }, } # Subset of deprecations which are tricky (impossible?) to scrape from the # upgrader. @@ -257,9 +264,15 @@ def get_checkset_from_name(name): return purpose_filters -def parse_checks(check_arg): +def parse_checks(check_arg, ignores=None): """Collapse metadata in checks dicts. + + Args: + check_arg: type of checks to run, currently expecting '728', 'style' + or 'all'. + ignores: list of codes to ignore. """ + ignores = ignores or [] parsedchecks = {} purpose_filters = get_checkset_from_name(check_arg) @@ -269,8 +282,10 @@ def parse_checks(check_arg): if purpose in purpose_filters: for index, (pattern, meta) in enumerate(ruleset.items(), start=1): meta.update({'purpose': purpose}) - meta.update({'index': index}) - parsedchecks.update({pattern: meta}) + if 'index' not in meta: + meta.update({'index': index}) + if f'{purpose}{index:03d}' not in ignores: + parsedchecks.update({pattern: meta}) return parsedchecks @@ -435,6 +450,16 @@ def get_option_parser() -> COP: default=False, dest='ref_mode' ) + parser.add_option( + '--ignore', '-n', + help=( + 'Ignore this check number.' + ), + action='append', + default=[], + dest='ignores', + choices=tuple([f'S{i["index"]:03d}' for i in STYLE_CHECKS.values()]) + ) return parser @@ -481,7 +506,7 @@ def main(parser: COP, options: 'Values', *targets) -> None: check_names = options.linter # Check each file: - checks = parse_checks(check_names) + checks = parse_checks(check_names, options.ignores) for file_ in get_cylc_files(target): LOG.debug(f'Checking {file_}') count += check_cylc_file( diff --git a/tests/unit/scripts/test_lint.py b/tests/unit/scripts/test_lint.py index 560c716024f..2ad27e98c88 100644 --- a/tests/unit/scripts/test_lint.py +++ b/tests/unit/scripts/test_lint.py @@ -17,6 +17,7 @@ """Tests `cylc lint` CLI Utility. """ import difflib +from itertools import combinations from pathlib import Path import pytest import re @@ -148,9 +149,9 @@ @pytest.fixture() def create_testable_file(monkeypatch, capsys): - def _inner(test_file, checks): + def _inner(test_file, checks, ignores=[]): monkeypatch.setattr(Path, 'read_text', lambda _: test_file) - checks = parse_checks(checks) + checks = parse_checks(checks, ignores) check_cylc_file(Path('x'), Path('x'), checks) return capsys.readouterr(), checks return _inner @@ -190,15 +191,32 @@ def test_check_cylc_file_line_no(create_testable_file, capsys): def test_check_cylc_file_lint(create_testable_file, number): try: result, _ = create_testable_file( - LINT_TEST_FILE, 'lint') - assert f'[S{number + 1:03d}]' in result.out + LINT_TEST_FILE, 'style') + assert f'S{(number + 1):03d}' in result.out except AssertionError: raise AssertionError( - f'missing error number S:{number:03d}' - f'{[*STYLE_CHECKS.keys()][number]}' + f'missing error number S{number:03d}:' + f'{[*STYLE_CHECKS.keys()][number].pattern}' ) +@pytest.mark.parametrize( + 'exclusion', + [ + comb for i in range(len(STYLE_CHECKS.values())) + for comb in combinations( + [f'S{i["index"]:03d}' for i in STYLE_CHECKS.values()], i + 1 + ) + ] +) +def test_check_exclusions(create_testable_file, exclusion): + """It does not report any items excluded.""" + result, _ = create_testable_file( + LINT_TEST_FILE, 'style', list(exclusion)) + for item in exclusion: + assert item not in result.out + + @pytest.fixture def create_testable_dir(tmp_path): test_file = (tmp_path / 'suite.rc')