diff --git a/CHANGES.md b/CHANGES.md index c5115a3ff6a..0013611936f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -37,9 +37,11 @@ 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 `. +[#5055](https://github.com/cylc/cylc-flow/pull/5055) and +[#5086](https://github.com/cylc/cylc-flow/pull/5086) - Upgrades to `cylc lint` +- Allow users to ignore Cylc Lint issues using `--ignore `. +- Allow settings for `cylc lint` to be recorded in a pyproject.toml file. +- Allow files to be excluded from `cylc lint` checks. ------------------------------------------------------------------------------- ## __cylc-8.0.2 (Upcoming)__ diff --git a/conda-environment.yml b/conda-environment.yml index 00ccbd27a53..822e57dbf05 100644 --- a/conda-environment.yml +++ b/conda-environment.yml @@ -18,6 +18,8 @@ dependencies: - pyzmq >=22,<23 - setuptools >=49 - urwid >=2,<3 + # Add # [py<3.11] for tomli once Python 3.11 Released + - tomli >=2.0.0 # optional dependencies #- empy >=3.3,<3.4 diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index aeffadc94ab..d61cbe04145 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -28,24 +28,29 @@ By default, suggestions are written to stdout. -In-place mode ( "-i, --inplace") writes suggestions into the file as comments. +In-place mode ("-i, --inplace") writes suggestions into the file as comments. Commit to version control before using this, in case you want to back out. +Configurations for Cylc lint can also be set in a pyproject.toml file. + """ from colorama import Fore from optparse import Values from pathlib import Path import re -from typing import Generator +import tomli +from typing import Generator, Union from cylc.flow import LOG +from cylc.flow.exceptions import CylcError from cylc.flow.option_parsers import ( CylcOptionParser as COP, + WORKFLOW_ID_OR_PATH_ARG_DOC ) from cylc.flow.cfgspec.workflow import upg, SPEC +from cylc.flow.id_cli import parse_id from cylc.flow.parsec.config import ParsecConfig from cylc.flow.terminal import cli_function -from cylc.flow.workflow_files import check_flow_file STYLE_GUIDE = ( 'https://cylc.github.io/cylc-doc/stable/html/workflow-design-guide/' @@ -125,10 +130,10 @@ 'url': STYLE_GUIDE + 'task-naming-conventions', 'index': 7 }, - # Consider re-adding this as an option later: - # re.compile(r'^.{80,}'): { - # 'short': 'line > 79 characters.', + # re.compile(r'^.{{maxlen},}'): { + # 'short': 'line > {maxlen} characters.', # 'url': STYLE_GUIDE + 'line-length-and-continuation', + # 'index': 8 # }, } # Subset of deprecations which are tricky (impossible?) to scrape from the @@ -166,6 +171,143 @@ ) }, } +RULESETS = ['728', 'style', 'all'] +EXTRA_TOML_VALIDATION = { + 'ignore': { + lambda x: re.match(r'[A-Z]\d\d\d', x): + '{item} not valid: Ignore codes should be in the form X001', + lambda x: x in [ + f'{i["purpose"]}{i["index"]:03d}' + for i in parse_checks(['728', 'style']).values() + ]: + '{item} is a not a known linter code.' + }, + 'rulesets': { + lambda item: item in RULESETS: + '{item} not valid: Rulesets can be ' + '\'728\', \'style\' or \'all\'.' + }, + 'max-line-length': { + lambda x: isinstance(x, int): + 'max-line-length must be an integer.' + }, + # consider checking that item is file? + 'exclude': {} +} + + +def validate_toml_items(tomldata): + """Check that all tomldata items are lists of strings + + Explicitly checks and raises if tomldata not: + - str in EXTRA_TOML_VALIDATION.keys() + - item is not a list of strings. + + Plus additional validation for each set of values. + """ + for key, items in tomldata.items(): + # Key should only be one of the allowed keys: + if key not in EXTRA_TOML_VALIDATION.keys(): + raise CylcError( + f'Only {[*EXTRA_TOML_VALIDATION.keys()]} ' + f'allowed as toml sections but you used {key}' + ) + if key != 'max-line-length': + # Item should be a list... + if not isinstance(items, list): + raise CylcError( + f'{key} should be a list, but was: {items}') + # ... of strings + for item in items: + if not isinstance(item, str): + raise CylcError( + f'Config {item} should be a string but ' + f'is {str(type(item))}' + ) + for check, message in EXTRA_TOML_VALIDATION[key].items(): + if not check(item): + raise CylcError( + message.format(item=item) + ) + return True + + +def get_pyproject_toml(dir_): + """if a pyproject.toml file is present open it and return settings. + """ + keys = ['rulesets', 'ignore', 'exclude', 'max-line-length'] + tomlfile = Path(dir_ / 'pyproject.toml') + tomldata = {} + if tomlfile.is_file(): + try: + loadeddata = tomli.loads(tomlfile.read_text()) + except tomli.TOMLDecodeError as exc: + raise CylcError(f'pyproject.toml did not load: {exc}') + + if any( + [i in loadeddata for i in ['cylc-lint', 'cylclint', 'cylc_lint']] + ): + for key in keys: + tomldata[key] = loadeddata.get('cylc-lint').get(key, []) + validate_toml_items(tomldata) + if not tomldata: + tomldata = {key: [] for key in keys} + return tomldata + + +def merge_cli_with_tomldata( + clidata, tomldata, + override_cli_default_rules=False +): + """Merge options set by pyproject.toml with CLI options. + + rulesets: CLI should override toml. + ignore: CLI and toml should be combined with no duplicates. + exclude: No CLI equivalent, return toml if any. + + Args: + override_cli_default_rules: If user doesn't specifiy a ruleset use the + rules from the tomlfile - i.e: if we've set 'rulesets': 'style' + we probably don't want to get warnings about 728 upgrades by + default, but only if we ask for it on the CLI. + + Examples: + >>> result = merge_cli_with_tomldata( + ... {'rulesets': ['foo'], 'ignore': ['R101'], 'exclude': []}, + ... {'rulesets': ['bar'], 'ignore': ['R100'], 'exclude': ['*.bk']}) + >>> result['ignore'] + ['R100', 'R101'] + >>> result['rulesets'] + ['foo'] + >>> result['exclude'] + ['*.bk'] + """ + if isinstance(clidata['rulesets'][0], list): + clidata['rulesets'] = clidata['rulesets'][0] + + output = {} + + # Combine 'ignore' sections: + output['ignore'] = sorted(set(clidata['ignore'] + tomldata['ignore'])) + + # Replace 'rulesets from toml with those from CLI if they exist: + + if override_cli_default_rules: + output['rulesets'] = ( + tomldata['rulesets'] if tomldata['rulesets'] + else clidata['rulesets'] + ) + else: + output['rulesets'] = ( + clidata['rulesets'] if clidata['rulesets'] + else tomldata['rulesets'] + ) + + # Return 'exclude' and 'max-line-length' for the tomldata: + output['exclude'] = tomldata['exclude'] + output['max-line-length'] = tomldata.get('max-line-length', None) + + return output def list_to_config(path_, is_section=False): @@ -244,52 +386,64 @@ def get_upgrader_info(): return deprecations -def get_checkset_from_name(name): - """Take a ruleset name and return a ruleset code - - Examples: - >>> get_checkset_from_name('728') - ['U'] - >>> get_checkset_from_name('style') - ['S'] - >>> get_checkset_from_name('all') - ['U', 'S'] - """ - if name == '728': - purpose_filters = ['U'] - elif name == 'style': - purpose_filters = ['S'] - else: - purpose_filters = ['U', 'S'] - return purpose_filters +PURPOSE_FILTER_MAP = { + 'style': 'S', + '728': 'U', +} -def parse_checks(check_arg, ignores=None): - """Collapse metadata in checks dicts. +def parse_checks(check_args, ignores=None, max_line_len=None, reference=False): + """Prepare dictionary of checks. Args: - check_arg: type of checks to run, currently expecting '728', 'style' - or 'all'. + check_arg: list of types of checks to run, + currently expecting '728' and/or 'style' ignores: list of codes to ignore. + max_line_len: Adds a specific style warning for lines longer than + this. (If None, rule not enforced) + reference: Function is being used to get a reference. If true + max-line-length will have a generic message, rather than + using any specific value. """ ignores = ignores or [] parsedchecks = {} - purpose_filters = get_checkset_from_name(check_arg) + purpose_filters = [PURPOSE_FILTER_MAP[i] for i in check_args] checks = {'U': get_upgrader_info(), 'S': STYLE_CHECKS} for purpose, ruleset in checks.items(): if purpose in purpose_filters: + # Run through the rest of the config items. for index, (pattern, meta) in enumerate(ruleset.items(), start=1): meta.update({'purpose': purpose}) if 'index' not in meta: meta.update({'index': index}) if f'{purpose}{index:03d}' not in ignores: parsedchecks.update({pattern: meta}) + if 'S' in purpose and "S008" not in ignores: + if not max_line_len: + max_line_len = 130 + regex = r"^.{" + str(max_line_len) + r"}" + if reference: + msg = ( + 'line > {max_line_len} characters. Max line length ' + 'set in pyproject.toml (default 130)' + ) + else: + msg = f'line > {max_line_len} characters.' + parsedchecks[re.compile(regex)] = { + 'short': msg, + 'url': STYLE_GUIDE + 'line-length-and-continuation', + 'index': 8, + 'purpose': 'S' + } return parsedchecks -def check_cylc_file(dir_, file_, checks, modify=False): +def check_cylc_file( + dir_, file_, checks, + modify=False, +): """Check A Cylc File for Cylc 7 Config""" file_rel = file_.relative_to(dir_) # Set mode as read-write or read only. @@ -335,12 +489,20 @@ def check_cylc_file(dir_, file_, checks, modify=False): return count -def get_cylc_files(base: Path) -> Generator[Path, None, None]: +def get_cylc_files( + base: Path, exclusions: Union[list, None] = None +) -> Generator[Path, None, None]: """Given a directory yield paths to check.""" + exclusions = [] if exclusions is None else exclusions + except_these_files = [ + file for exclusion in exclusions for file in base.rglob(exclusion)] for rglob in FILEGLOBS: for path in base.rglob(rglob): # Exclude log directory: - if path.relative_to(base).parts[0] != 'log': + if ( + path.relative_to(base).parts[0] != 'log' + and path not in except_these_files + ): yield path @@ -420,8 +582,11 @@ def get_reference_text(checks): def get_option_parser() -> COP: parser = COP( COP_DOC, - argdoc=[COP.optional(('DIR ...', 'Directories to lint'))], + argdoc=[ + COP.optional(WORKFLOW_ID_OR_PATH_ARG_DOC) + ], ) + parser.add_option( '--inplace', '-i', help=( @@ -437,8 +602,8 @@ def get_option_parser() -> COP: 'Set of rules to use: ' '("728", "style", "all")' ), - default='all', - choices=('728', 'style', 'all'), + default='', + choices=["728", "style", "all", ''], dest='linter' ) parser.add_option( @@ -466,74 +631,98 @@ def get_option_parser() -> COP: @cli_function(get_option_parser) -def main(parser: COP, options: 'Values', *targets) -> None: +def main(parser: COP, options: 'Values', target=None) -> None: if options.ref_mode: - print(get_reference_text(parse_checks(options.linter))) + if options.linter in {'all', ''}: + rulesets = ['728', 'style'] + else: + rulesets = [options.linter] + print(get_reference_text(parse_checks(rulesets, reference=True))) exit(0) - # Expand paths so that message will return full path - # & ensure that CWD is used if no path is given: - if targets: - targets = tuple(Path(path).resolve() for path in targets) - else: - targets = (str(Path.cwd()),) + # If target not given assume we are looking at PWD + if target is None: + target = str(Path.cwd()) - # make sure the targets are all src/run directories - for target in targets: - check_flow_file(target) + # make sure the target is a src/run directories + _, _, target = parse_id( + target, + src=True, + constraint='workflows', + ) # Get a list of checks bas ed on the checking options: # Allow us to check any number of folders at once - for target in targets: - count = 0 - 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: ' - 'Lint after renaming ' - '"suite.rc" to "flow.cylc"' - ) - continue - elif not cylc8 and options.linter == 'all': - check_names = 'style' - else: - check_names = options.linter - - # Check each file: - checks = parse_checks(check_names, options.ignores) - for file_ in get_cylc_files(target): - LOG.debug(f'Checking {file_}') - count += check_cylc_file( - target, - file_, - checks, - options.inplace, - ) - - if count > 0: - msg = ( - f'\n{Fore.YELLOW}' - f'Checked {target} against {check_names} ' - f'rules and found {count} issue' - f'{"s" if count > 1 else ""}.' - ) - else: - msg = ( - f'{Fore.GREEN}' - f'Checked {target} against {check_names} rules and ' - 'found no issues.' - ) + count = 0 + target = target.parent + ruleset_default = False + if options.linter == 'all': + options.linter = ['728', 'style'] + elif options.linter == '': + options.linter = ['728', 'style'] + ruleset_default = True + else: + options.linter = [options.linter] + tomlopts = get_pyproject_toml(target) + mergedopts = merge_cli_with_tomldata( + { + 'exclude': [], + 'ignore': options.ignores, + 'rulesets': options.linter + }, + tomlopts, + ruleset_default + ) + + # 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 mergedopts['rulesets'] == ['728']: + LOG.error( + f'{target} not a Cylc 8 workflow: ' + 'Lint after renaming ' + '"suite.rc" to "flow.cylc"' + ) + exit(0) + elif not cylc8 and '728' in mergedopts['rulesets']: + check_names = mergedopts['rulesets'] + check_names.remove('728') + else: + check_names = mergedopts['rulesets'] + + # Check each file: + checks = parse_checks( + check_names, + ignores=mergedopts['ignore'], + max_line_len=mergedopts['max-line-length'] + ) + for file_ in get_cylc_files(target, mergedopts['exclude']): + LOG.debug(f'Checking {file_}') + count += check_cylc_file( + target, + file_, + checks, + options.inplace, + ) + + if count > 0: + msg = ( + f'\n{Fore.YELLOW}' + f'Checked {target} against {check_names} ' + f'rules and found {count} issue' + f'{"s" if count > 1 else ""}.' + ) + else: + msg = ( + f'{Fore.GREEN}' + f'Checked {target} against {check_names} rules and ' + 'found no issues.' + ) - print(msg) + print(msg) # NOTE: use += so that this works with __import__ # (docstring needed for `cylc help all` output) -__doc__ += get_reference_rst(parse_checks('all')) +__doc__ += get_reference_rst(parse_checks(['728', 'style'], reference=True)) diff --git a/setup.cfg b/setup.cfg index b07ab0673f6..06bf6dcea3b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -77,6 +77,8 @@ install_requires = # unpinned transient dependencies used for type checking rx promise + # Once Python 3.11 released -'; python_version<"3.11"' + tomli>=2.* [options.packages.find] include = cylc* diff --git a/tests/functional/cylc-lint/00.lint.t b/tests/functional/cylc-lint/00.lint.t index 86e84e29f21..cafaef32d27 100644 --- a/tests/functional/cylc-lint/00.lint.t +++ b/tests/functional/cylc-lint/00.lint.t @@ -18,7 +18,7 @@ #------------------------------------------------------------------------------ # Test workflow installation . "$(dirname "$0")/test_header" -set_test_number 11 +set_test_number 18 cat > flow.cylc <<__HERE__ # This is definitely not an OK flow.cylc file. @@ -29,7 +29,7 @@ __HERE__ rm etc/global.cylc TEST_NAME="${TEST_NAME_BASE}.vanilla" -run_ok "${TEST_NAME}" cylc lint +run_ok "${TEST_NAME}" cylc lint . named_grep_ok "check-for-error-code" "S004" "${TEST_NAME}.stdout" TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset" @@ -62,7 +62,22 @@ cat > flow.cylc <<__HERE__ __HERE__ TEST_NAME="${TEST_NAME_BASE}.zero-issues" -run_ok "${TEST_NAME}" cylc lint +run_ok "${TEST_NAME}" cylc lint . named_grep_ok "message on no errors" "found no issues" "${TEST_NAME}.stdout" +# It returns an error message if you attempt to lint a non-existant location +TEST_NAME="it-fails-if-not-target" +run_fail ${TEST_NAME} cylc lint "a-$(uuidgen)" +grep_ok "Workflow ID not found" "${TEST_NAME}.stderr" + +# It returns a reference in reference mode +TEST_NAME="it-returns-a-reference" +run_ok "${TEST_NAME}" cylc lint --list-codes +named_grep_ok "${TEST_NAME}-contains-style-codes" "^S001:" "${TEST_NAME}.stdout" +TEST_NAME="it-returns-a-reference-style" +run_ok "${TEST_NAME}" cylc lint --list-codes -r 'style' +named_grep_ok "${TEST_NAME}-contains-style-codes" "^S001:" "${TEST_NAME}.stdout" +grep_fail "^U" "${TEST_NAME}.stdout" + + rm flow.cylc diff --git a/tests/functional/cylc-lint/01.lint-toml.t b/tests/functional/cylc-lint/01.lint-toml.t new file mode 100644 index 00000000000..5f0d13ee17d --- /dev/null +++ b/tests/functional/cylc-lint/01.lint-toml.t @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +#------------------------------------------------------------------------------ +# Test linting with a toml file present. +. "$(dirname "$0")/test_header" +set_test_number 12 + +# Set Up: +rm etc/global.cylc + +cat > flow.cylc <<__HERE__ +# This is definitely not an OK flow.cylc file. +\t[scheduler] + + [cylc] + +[[dependencies]] + +[runtime] + [[foo]] + inherit = hello + [[[job]]] +something\t +__HERE__ + +mkdir sites + +cat > sites/niwa.cylc <<__HERE__ +blatantly = not valid +__HERE__ + + +# Control tests +TEST_NAME="it lints without toml file" +run_ok "${TEST_NAME}" cylc lint +TESTOUT="${TEST_NAME}.stdout" +named_grep_ok "it returns error code" "S004" "${TESTOUT}" +named_grep_ok "it returns error from subdirectory" "niwa.cylc" "${TESTOUT}" +named_grep_ok "it returns a 728 upgrade code" "^\[U" "${TESTOUT}" + + +# Add a pyproject.toml file +cat > pyproject.toml <<__HERE__ +[cylc-lint] + # Check against these rules + rulesets = [ + "style" + ] + # do not check for these errors + ignore = [ + "S004" + ] + # do not lint files matching + # these globs: + exclude = [ + "sites/*.cylc", + ] + +__HERE__ + +# Test that results are different: +TEST_NAME="it_lints_with_toml_file" +run_ok "${TEST_NAME}" cylc lint +TESTOUT="${TEST_NAME}.stdout" +grep_fail "S004" "${TESTOUT}" +grep_fail "niwa.cylc" "${TESTOUT}" +grep_fail "^\[U" "${TESTOUT}" + + +# Add a max line length to the pyproject.toml. +echo "" >> pyproject.toml +echo "max-line-length = 4" >> pyproject.toml + +cat > flow.cylc <<__HERE__ + script = """ + How long a line is too long a line + """ +__HERE__ + +TEST_NAME="it_fails_if_max-line-length_set" +run_ok "${TEST_NAME}" cylc lint +named_grep_ok "${TEST_NAME}-line-too-long-message" \ + "\[S008\] flow.cylc:2: line > 4 characters." \ + "${TEST_NAME}.stdout" + +TEST_NAME="it_does_not_fail_if_max-line-length_set_but_ignored" +cat > pyproject.toml <<__HERE__ +[cylc-lint] + # Check against these rules + rulesets = [ + "style" + ] + # do not check for these errors + ignore = [ + "S008" + ] + exclude = [ + "sites/*.cylc", + ] + max-line-length = 1 +__HERE__ +run_ok "${TEST_NAME}" cylc lint +grep_ok "rules and found no issues" "${TEST_NAME}.stdout" diff --git a/tests/unit/scripts/test_lint.py b/tests/unit/scripts/test_lint.py index 2ad27e98c88..82561a6cf6b 100644 --- a/tests/unit/scripts/test_lint.py +++ b/tests/unit/scripts/test_lint.py @@ -20,6 +20,7 @@ from itertools import combinations from pathlib import Path import pytest +from pytest import param import re from cylc.flow.scripts.lint import ( @@ -28,12 +29,16 @@ get_cylc_files, get_reference_rst, get_reference_text, + get_pyproject_toml, get_upgrader_info, - parse_checks + merge_cli_with_tomldata, + parse_checks, + validate_toml_items ) +from cylc.flow.exceptions import CylcError -UPG_CHECKS = parse_checks('728') +UPG_CHECKS = parse_checks(['728']) TEST_FILE = """ [visualization] @@ -149,7 +154,9 @@ @pytest.fixture() def create_testable_file(monkeypatch, capsys): - def _inner(test_file, checks, ignores=[]): + def _inner(test_file, checks, ignores=None): + if ignores is None: + ignores = [] monkeypatch.setattr(Path, 'read_text', lambda _: test_file) checks = parse_checks(checks, ignores) check_cylc_file(Path('x'), Path('x'), checks) @@ -162,7 +169,7 @@ def _inner(test_file, checks, ignores=[]): ) def test_check_cylc_file_7to8(create_testable_file, number, capsys): try: - result, checks = create_testable_file(TEST_FILE, '728') + result, checks = create_testable_file(TEST_FILE, ['728']) assert f'[U{number:03d}]' in result.out except AssertionError: raise AssertionError( @@ -173,14 +180,14 @@ def test_check_cylc_file_7to8(create_testable_file, number, capsys): def test_check_cylc_file_7to8_has_shebang(create_testable_file): """Jinja2 code comments will not be added if shebang present""" - result, _ = create_testable_file('#!jinja2\n{{FOO}}', '[scheduler]') + result, _ = create_testable_file('#!jinja2\n{{FOO}}', '', '[scheduler]') result = result.out assert result == '' def test_check_cylc_file_line_no(create_testable_file, capsys): """It prints the correct line numbers""" - result, _ = create_testable_file(TEST_FILE, '728') + result, _ = create_testable_file(TEST_FILE, ['728']) result = result.out assert result.split()[1] == '.:2:' @@ -191,7 +198,7 @@ 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, 'style') + LINT_TEST_FILE, ['style']) assert f'S{(number + 1):03d}' in result.out except AssertionError: raise AssertionError( @@ -212,7 +219,7 @@ def test_check_cylc_file_lint(create_testable_file, number): 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)) + LINT_TEST_FILE, ['style'], list(exclusion)) for item in exclusion: assert item not in result.out @@ -224,7 +231,7 @@ def create_testable_dir(tmp_path): check_cylc_file( test_file.parent, test_file, - parse_checks('all'), + parse_checks(['728', 'style']), modify=True, ) return '\n'.join([*difflib.Differ().compare( @@ -341,3 +348,150 @@ def test_get_upg_info(fixture_get_deprecations, findme): else: pattern = f'^\\s*{findme}\\s*=\\s*.*' assert pattern in [i.pattern for i in fixture_get_deprecations.keys()] + + +@pytest.mark.parametrize( + 'expect', + [ + param({ + 'rulesets': ['style'], + 'ignore': ['S004'], + 'exclude': ['sites/*.cylc']}, + id="it returns what we want" + ), + param({ + 'northgate': ['sites/*.cylc'], + 'mons-meg': 42}, + id="it only returns requested sections" + ), + param({ + 'max-line-length': 22}, + id='it sets max line length' + ) + ] +) +def test_get_pyproject_toml(tmp_path, expect): + """It returns only the lists we want from the toml file.""" + tomlcontent = "[cylc-lint]" + permitted_keys = ['rulesets', 'ignore', 'exclude', 'max-line-length'] + + for section, value in expect.items(): + tomlcontent += f'\n{section} = {value}' + (tmp_path / 'pyproject.toml').write_text(tomlcontent) + tomldata = get_pyproject_toml(tmp_path) + + control = {} + for key in permitted_keys: + control[key] = expect.get(key, []) + assert tomldata == control + + +@pytest.mark.parametrize('tomlfile', [None, '', '[cylc-lint]']) +def test_get_pyproject_toml_returns_blank(tomlfile, tmp_path): + if tomlfile is not None: + tfile = (tmp_path / 'pyproject.toml') + tfile.write_text(tomlfile) + expect = {k: [] for k in { + 'exclude', 'ignore', 'max-line-length', 'rulesets' + }} + assert get_pyproject_toml(tmp_path) == expect + + +@pytest.mark.parametrize( + 'input_, error', + [ + param( + {'exclude': ['hey', 'there', 'Delilah']}, + None, + id='it works' + ), + param( + {'foo': ['hey', 'there', 'Delilah', 42]}, + 'allowed', + id='it fails with illegal section name' + ), + param( + {'exclude': 'woo!'}, + 'should be a list, but', + id='it fails with illegal section type' + ), + param( + {'exclude': ['hey', 'there', 'Delilah', 42]}, + 'should be a string', + id='it fails with illegal value name' + ), + param( + {'rulesets': ['hey']}, + 'hey not valid: Rulesets can be', + id='it fails with illegal ruleset' + ), + param( + {'ignore': ['hey']}, + 'hey not valid: Ignore codes', + id='it fails with illegal ignores' + ), + param( + {'ignore': ['R999']}, + 'R999 is a not a known linter code.', + id='it fails with non-existant checks ignored' + ) + ] +) +def test_validate_toml_items(input_, error): + """It chucks out the wrong sort of items.""" + if error is not None: + with pytest.raises(CylcError, match=error): + validate_toml_items(input_) + else: + assert validate_toml_items(input_) is True + + +@pytest.mark.parametrize( + 'clidata, tomldata, expect', + [ + param( + { + 'rulesets': ['foo', 'bar'], + 'ignore': ['R101'], + }, + { + 'rulesets': ['baz'], + 'ignore': ['R100'], + 'exclude': ['not_me-*.cylc'] + }, + { + 'rulesets': ['foo', 'bar'], + 'ignore': ['R100', 'R101'], + 'exclude': ['not_me-*.cylc'], + 'max-line-length': None + }, + id='It works with good path' + ), + ] +) +def test_merge_cli_with_tomldata(clidata, tomldata, expect): + """It merges each of the three sections correctly: see function.__doc__""" + assert merge_cli_with_tomldata(clidata, tomldata) == expect + + +def test_invalid_tomlfile(tmp_path): + """It fails nicely if pyproject.toml is malformed""" + tomlfile = (tmp_path / 'pyproject.toml') + tomlfile.write_text('foo :{}') + expected_msg = 'pyproject.toml did not load:' + with pytest.raises(CylcError, match=expected_msg): + get_pyproject_toml(tmp_path) + + +@pytest.mark.parametrize( + 'ref, expect', + [ + [True, 'line > {max_line_len} characters'], + [False, 'line > 130 characters'] + ] +) +def test_parse_checks_reference_mode(ref, expect): + result = parse_checks(['style'], reference=ref) + key = [i for i in result.keys()][-1] + value = result[key] + assert expect in value['short']