Allow CLI selection of a single rule to check.

Fix a bug where empty flow.cylc files caused traceback.

changes.d/6120.feat.md

@@ -0,0 +1 @@
+Allow checking of individual ``cylc lint`` checks using ``--rule`` (or ``-R``)

cylc/flow/loggingutil.py

@@ -31,7 +31,7 @@
 import re
 import sys
 import textwrap
-from typing import List, Optional, Union
+from typing import Iterable, List, Optional, Union
 
 from ansimarkup import parse as cparse, strip as cstrip
 
@@ -434,3 +434,25 @@ def patch_log_level(logger: logging.Logger, level: int = logging.INFO):
         logger.setLevel(orig_level)
     else:  # No need to patch
         yield
+
+
+def log_iterable(
+    items: Iterable,
+    header: str,
+    singular_header: str,
+) -> str:
+    """Log a list.
+
+    Args:
+        items: The iterable
+        singular_header: To use if list has length 1
+        header: To use otherwise
+    """
+    if len(items) == 0:
+        raise ValueError('This function does not take len==0 iterables')
+    elif len(items) == 1:
+        return singular_header.format(items[0])
+    else:
+        msg = header + '\n    * '
+        msg += '\n    * '.join(items)
+        return msg

cylc/flow/scripts/lint.py

@@ -78,7 +78,7 @@
 from cylc.flow import LOG
 from cylc.flow.exceptions import CylcError
 import cylc.flow.flags
-from cylc.flow.loggingutil import set_timestamps
+from cylc.flow.loggingutil import set_timestamps, log_iterable
 from cylc.flow.option_parsers import (
     CylcOptionParser as COP,
     WORKFLOW_ID_OR_PATH_ARG_DOC
@@ -443,6 +443,7 @@ def list_wrapper(line: str, check: Callable) -> Optional[Dict[str, str]]:
     'S': 'Style'
 }
 LINE_LEN_NO = 'S012'
+EMPTY_FILE = 'S000'   # Reference for a completely empty file.
 # Checks Dictionary fields:
 # TODO: Consider making the checks an object.
 # Key: A unique reference number.
@@ -1144,7 +1145,15 @@ def lint(
     """
     # get the first line
     line_no = 1
-    line = next(lines)
+    try:
+        line = next(lines)
+    except StopIteration:
+        LOG.warning(f'File {file_rel} is empty.')
+        if EMPTY_FILE in counter:
+            counter[EMPTY_FILE] += 1
+        else:
+            counter[EMPTY_FILE] = 1
+        return
     # check if it is a jinja2 shebang
     jinja_shebang = line.strip().lower() == JINJA2_SHEBANG
 
@@ -1340,6 +1349,13 @@ def get_option_parser() -> COP:
         action='store_true',
         default=False,
     )
+    parser.add_option(
+        '--rule', '-R',
+        help='Carry out only specific checks.',
+        choices=list(parse_checks(['style', '728']).keys()),
+        action='append',
+        default=[],
+    )
     parser.add_option(
         '--ruleset', '-r',
         help=(
@@ -1390,6 +1406,13 @@ def main(parser: COP, options: 'Values', target=None) -> None:
         print(get_reference(options.ruleset, 'text'))
         sys.exit(0)
 
+    if options.rule and options.ruleset:
+        LOG.error('arguments --rule and --ruleset are mutually exclusive')
+        sys.exit(1)
+    if options.rule and options.ignores:
+        LOG.error('arguments --rule and --ignores are mutually exclusive')
+        sys.exit(1)
+
     # If target not given assume we are looking at PWD:
     if target is None:
         target = str(Path.cwd())
@@ -1417,6 +1440,14 @@ def main(parser: COP, options: 'Values', target=None) -> None:
         max_line_len=mergedopts[MAX_LINE_LENGTH]
     )
 
+    if options.rule:
+        checks = {k: v for k, v in checks.items() if k in options.rule}
+        LOG.warning(log_iterable(
+            [f'{k}: {v["short"]}' for k, v in checks.items()],
+            header='Checking only:',
+            singular_header='Checking only {}')
+        )
+
     # Check each file matching a pattern:
     counter: Dict[str, int] = {}
     for file in get_cylc_files(target, mergedopts[EXCLUDE]):

tests/integration/scripts/test_lint.py

@@ -0,0 +1,89 @@
+#!/usr/bin/env python3
+# 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 <http://www.gnu.org/licenses/>.
+"""Tests `cylc lint` CLI Utility.
+
+TODO: Consider tests in unit test file for movement to here.
+"""
+
+import pytest
+
+from cylc.flow.scripts.lint import main, get_option_parser
+
+cylc_lint = main.__wrapped__
+
+
+@pytest.fixture
+def setup():
+    parser = get_option_parser()
+    options = parser.get_default_values()
+    return parser, options
+
+
+def test_lint_empty_file(tmp_path, setup, caplog):
+    """Argument --rule is mutually exclusive of either --ignores
+    and --ruleset.
+    """
+    (tmp_path / 'flow.cylc').touch()
+    parser, options = setup
+    with pytest.raises(SystemExit, match="True"):
+        main.__wrapped__(parser, options, str(tmp_path))
+    assert 'File flow.cylc is empty.' in caplog.messages
+
+
+def test_mutually_exclusive_args(tmp_path, setup):
+    """Argument --rule is mutually exclusive of either --ignores
+    and --ruleset.
+    """
+    (tmp_path / 'flow.cylc').write_text('[hi]')
+    parser, options = setup
+    options.rule = ['S002']
+
+    # Rule + Ruleset
+    options.ruleset = '728'
+    with pytest.raises(SystemExit, match="1"):
+        main.__wrapped__(parser, options, str(tmp_path))
+
+    # Rule + Ignores
+    options.ignores = ['S005']
+    options.ruleset = ''
+    with pytest.raises(SystemExit, match="1"):
+        main.__wrapped__(parser, options, str(tmp_path))
+
+
+def test_single_check_cli(setup, tmp_path, caplog):
+    """Demo that CLI --rule option works.
+    """
+    (tmp_path / 'flow.cylc').write_text('    [meta]')
+    parser, options = setup
+
+    # This rule should cause a failure:
+    options.rule = ['S003']
+    with pytest.raises(SystemExit, match="True"):
+        main.__wrapped__(parser, options, str(tmp_path))
+    assert (
+        'Checking only S003: Top level sections should'
+        ' not be indented.'
+    ) in caplog.messages
+
+    # This rule should NOT cause a failure:
+    options.ruleset = ''
+    options.rule = ['S001']
+    with pytest.raises(SystemExit, match="False"):
+        main.__wrapped__(parser, options, str(tmp_path))
+    assert (
+        'Checking only S001: Use multiple spaces, not tabs'
+    ) in caplog.messages

tests/unit/test_loggingutil.py

@@ -31,6 +31,7 @@
     CylcLogFormatter,
     get_reload_start_number,
     get_sorted_logs_by_time,
+    log_iterable,
     set_timestamps,
 )
 
@@ -197,3 +198,18 @@ def test_set_timestamps(capsys):
     assert re.match('^[0-9]{4}', errors[0])
     assert re.match('^WARNING - bar', errors[1])
     assert re.match('^[0-9]{4}', errors[2])
+
+
+def test_log_iterable():
+    # It fails if no items are supplied:
+    with pytest.raises(ValueError, match='^This function does'):
+        log_iterable([], 'Hello', '')
+    # It uses an alternative singular header:
+    assert log_iterable(['foo'], 'Hello', 'Bon Jour: {}') == 'Bon Jour: foo'
+    # It lists items:
+    assert log_iterable(['foo', 'quartet'], 'Hello:', '') == (
+        'Hello:'
+        '\n    * foo'
+        '\n    * quartet'
+    )
+