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

Lint.hardcode style index numbers #5055

Merged
merged 2 commits into from
Aug 16, 2022
Merged
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
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ ones in. -->
[#5032](https://github.com/cylc/cylc-flow/pull/5032) - set a default limit of
100 for the "default" queue.

[#5055](https://github.com/cylc/cylc-flow/pull/5055) - Hard-code the serial
numbers of Cylc Lint's style issues and allow users to ignore Cylc Lint issues
using `--ignore <Issue Code>`.


-------------------------------------------------------------------------------
## __cylc-8.0.1 (<span actions:bind='release-date'>Upcoming</span>)__
Expand Down
56 changes: 41 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,17 @@ 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',
metavar="CODE",
choices=tuple([f'S{i["index"]:03d}' for i in STYLE_CHECKS.values()])
)

return parser

Expand Down Expand Up @@ -481,7 +507,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