Skip to content

Commit

Permalink
Update cylc/flow/scripts/lint.py
Browse files Browse the repository at this point in the history 
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

responded to review; fixed line numbering bug; updated tests

added some more regexes

make 728 linter checks only work if flow.cylc presnet
  • Loading branch information
@wxtim
wxtim committed Jun 24, 2022
1 parent 24a311d commit 6623ebb
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 49 deletions.
123 changes: 76 additions & 47 deletions cylc/flow/scripts/lint.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@
CHECKS_DESC = {'U': '7 to 8 upgrades', 'S': 'Style'}
CHECKS = {
'U': {
re.compile(SECTION1.format('vizualization')): {
'short': 'section ``[vizualization]`` has been removed.',
re.compile(SECTION1.format('visualization')): {
'short': 'section ``[visualization]`` has been removed.',
'url': 'summary.html#new-web-and-terminal-uis'
},
re.compile(SECTION1.format('cylc')): {
Expand Down Expand Up @@ -174,18 +174,18 @@
},
re.compile(r'mail smtp\s*='): {
'short': (
'``[events]mail smtp`` => ``[mail]smtp``'
'``[events]mail smtp`` => ``global.cylc[scheduler][mail]smtp``'
),
'url': ''
},
re.compile(r'timeout\s*='): {
re.compile(r'^\s*timeout\s*='): {
'short': (
'``[cylc][events]timeout`` => '
'``[scheduler][events]stall timeout``'
),
'url': ''
},
re.compile(r'inactivity\s*='): {
re.compile(r'^\s*inactivity\s*='): {
'short': (
'``[cylc][events]inactivity`` => '
'``[scheduler][events]inactivity timeout``'
Expand All @@ -194,22 +194,20 @@
},
re.compile(r'abort on inactivity\s*='): {
'short': (
'``[cylc][events]timeout`` => '
'``[cylc][events]abort on inactivity`` => '
'``[scheduler][events]abort on inactivity timeout``'
),
'url': ''
},
re.compile(SECTION2.format('force run mode')): {
re.compile(r'force run mode\s*='): {
'short': (
'``[cylc][force run mode]abort if ___ handler fails`` '
'commands are obsolete.'
'``[cylc]force run mode`` is obsolete.'
),
'url': ''
},
re.compile(SECTION2.format('environment')): {
'short': (
'``[cylc][force run mode]abort if ___ handler fails`` '
'commands are obsolete.'
'``[cylc][environment]`` is obsolete.'
),
'url': ''
},
Expand Down Expand Up @@ -266,6 +264,13 @@
),
'url': 'major-changes/platforms.html#platforms'
},
re.compile(r'suite state polling\s*='): {
'short': (
'``[runtime][<namespace>]suite state polling`` is deprecated, '
'use ``[runtime][<namespace>]workflow state polling``'
),
'url': 'major-changes/platforms.html#platforms'
},
re.compile(SECTION3.format('job')): {
'short': (
'``[runtime][<namespace>][job]`` is deprecated, '
Expand Down Expand Up @@ -315,6 +320,20 @@
),
'url': ''
},
re.compile(r'max active cycle points\s*='): {
'short': (
'``[scheduling]max active cycle points`` is deprecated'
'use [scheduling]runahead limit'
),
'url': ''
},
re.compile(r'hold after point\s*='): {
'short': (
'``[scheduling]hold after point`` is deprecated'
'use [scheduling]hold after cycle point'
),
'url': ''
},
},
'S': {
re.compile(r'^\t'): {
Expand Down Expand Up @@ -363,7 +382,7 @@ def parse_checks(check_arg):
parsedchecks = {}
if check_arg == '728':
purpose_filters = ['U']
elif check_arg == 'lint':
elif check_arg == 'style':
purpose_filters = ['S']
else:
purpose_filters = ['U', 'S']
Expand All @@ -386,7 +405,7 @@ def check_cylc_file(file_, checks, modify=False):
lines = file_.read_text().split('\n')
jinja_shebang = lines[0].strip().lower() == JINJA2_SHEBANG
count = 0
for line_no, line in enumerate(lines):
for line_no, line in enumerate(lines, start=1):
for check, message in checks.items():
# Tests with for presence of Jinja2 if no shebang line is
# present.
Expand Down Expand Up @@ -484,23 +503,14 @@ def get_option_parser() -> COP:
action='store_true',
default=False,
)
parser.add_option(
'--reference', '--ref', '-R',
help=(
'generate a reference of errors'
),
action='store_true',
default=False,
dest="ref"
)
parser.add_option(
'--ruleset', '-r',
help=(
'Set of rules to use: '
'("728", "lint", "all")'
'("728", "style", "all")'
),
default='728',
choices=('728', 'lint', 'all'),
choices=('728', 'style', 'all'),
dest='linter'
)

Expand All @@ -515,32 +525,51 @@ def main(parser: COP, options: 'Values', *targets) -> None:
if targets:
targets = tuple(Path(path).resolve() for path in targets)
else:
targets = tuple(str(Path.cwd()))

# Get a list of checks based on the checking options:
checks = parse_checks(options.linter)
targets = (str(Path.cwd()),)

if options.ref:
print(get_reference(checks))

else:
count = 0
for target in targets:
target = Path(target)
if not target.exists():
LOG.warn(f'Path {target} does not exist.')
else:
for file_ in get_cylc_files(target):
LOG.debug(f'Checking {file_}')
count += check_cylc_file(file_, checks, options.inplace)
if count > 0:
print(
Fore.YELLOW + f'Checked {target} and found {count} issues.'
# Get a list of checks bas ed on the checking options:
count = 0
# Allow us to check any number of folders at once
for target in targets:
target = Path(target)
if not target.exists():
LOG.warn(f'Path {target} does not exist.')
else:
# Check whether target is an upgraded Cylc 8 workflow.
# If it isn't then we shouldn't run the 7-to-8 checks upon
# it:
cylc8 = (target / 'flow.cylc').exists()
if not cylc8 and options.linter == '728':
LOG.error(
f'{target} not a Cylc 8 workflow: '
'No checks will be made.'
)
else:
print(
Fore.GREEN + f'Checked {target} and found {count} issues.'
continue
elif not cylc8 and options.linter == 'all':
LOG.error(
f'{target} not a Cylc 8 workflow: '
'Checking only for style.'
)
check_names = parse_checks('style')
else:
check_names = options.linter

# Check each file:
checks = parse_checks(check_names)
for file_ in get_cylc_files(target):
LOG.debug(f'Checking {file_}')
count += check_cylc_file(file_, checks, options.inplace)

# Summing up:
if count > 0:
color = Fore.YELLOW
else:
color = Fore.GREEN
msg = (
f'Checked {target} against {check_names} '
f'rules and found {count} issues.'
)
print(f'{color}{"-" * len(msg)}\n{msg}')


__doc__ += get_reference(parse_checks('all'))
13 changes: 11 additions & 2 deletions tests/unit/scripts/test_lint.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@


TEST_FILE = """
[vizualization]
[visualization]
[cylc]
include at start-up = foo
Expand All @@ -48,7 +48,7 @@
disable suite event handlers = true
[[authentication]]
[[environment]]
[[force run mode]]
force run mode = dummy
[[events]]
abort on stalled = True
abort if startup handler fails= True # deliberately not added a space.
Expand All @@ -69,6 +69,8 @@
task event mail interval = PT4M # deliberately added lots of spaces.
[scheduling]
max active cycle points = 5
hold after point = 20220101T0000Z
[[dependencies]]
[[[R1]]]
graph = foo
Expand All @@ -80,6 +82,7 @@
extra log files = True
{% from 'cylc.flow' import LOG %}
script = {{HELLOWORLD}}
suite state polling = PT1H
[[[remote]]]
host = parasite
suite definition directory = '/home/bar'
Expand Down Expand Up @@ -134,6 +137,12 @@ def test_check_cylc_file_7to8(create_testable_file, number, capsys):
)


def test_check_cylc_file_line_no(create_testable_file, capsys):
"""It prints the correct line numbers"""
result = create_testable_file(TEST_FILE, '728').out
assert result.split()[1] == '2:'


@pytest.mark.parametrize(
'number', range(len(CHECKS['S']))
)
Expand Down

0 comments on commit 6623ebb

Please sign in to comment.