Skip to content

Commit

Permalink
Hardcoded error numbers for the style checks to prevent error number …
Browse files Browse the repository at this point in the history
…drift in future

Added an --ignore/-n command to exclude style rules.

added testing for ignore
  • Loading branch information
wxtim committed Aug 14, 2022
1 parent 4ddb3ee commit a6f71d7
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 21 deletions.
55 changes: 40 additions & 15 deletions cylc/flow/scripts/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,46 +83,53 @@
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,})\[\[[^\[.]*\]\]'): {
'short': (
'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.
Expand Down Expand Up @@ -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)

Expand All @@ -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


Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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(
Expand Down
30 changes: 24 additions & 6 deletions tests/unit/scripts/test_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"""Tests `cylc lint` CLI Utility.
"""
import difflib
from itertools import combinations
from pathlib import Path
import pytest
import re
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit a6f71d7

Please sign in to comment.