Fix cylc lint U013/U015 missing info

changes.d/6214.fix.md

@@ -0,0 +1 @@
+`cylc lint` rules U013 & U015 now tell you which deprecated variables you are using

cylc/flow/scripts/lint.py

@@ -150,7 +150,7 @@
 }
 
 
-LIST_ITEM = '    * '
+LIST_ITEM = '\n    * '
 
 
 deprecated_string_templates = {
@@ -264,7 +264,12 @@
 def check_for_deprecated_environment_variables(
     line: str
 ) -> Union[bool, dict]:
-    """Warn that environment variables with SUITE in are deprecated"""
+    """Warn that environment variables with SUITE in are deprecated
+
+    Examples:
+        >>> check_for_deprecated_environment_variables('CYLC_SUITE_HOST')
+        {'vars': ['CYLC_SUITE_HOST: CYLC_WORKFLOW_HOST']}
+    """
     vars_found = [
         f'{k}: {v}' for k, v in DEPRECATED_ENV_VARS.items()
         if k in line
@@ -277,16 +282,21 @@
     return False
 
 
-def check_for_obsolete_environment_variables(line: str) -> List[str]:
+def check_for_obsolete_environment_variables(line: str) -> Dict[str, List]:
     """Warn that environment variables are obsolete.
 
     Examples:
 
         >>> this = check_for_obsolete_environment_variables
-        >>> this('CYLC_SUITE_DEF_PATH')
-        ['CYLC_SUITE_DEF_PATH']
+        >>> this('script = echo $CYLC_SUITE_DEF_PATH')
+        {'vars': ['CYLC_SUITE_DEF_PATH']}
+        >>> this('script = echo "irrelevent"')
+        {}
     """
-    return [i for i in OBSOLETE_ENV_VARS if i in line]
+    vars_found = [i for i in OBSOLETE_ENV_VARS if i in line]
+    if vars_found:
+        return {'vars': vars_found}
+    return {}
 
 
 def check_for_deprecated_task_event_template_vars(
@@ -298,13 +308,10 @@
         >>> this = check_for_deprecated_task_event_template_vars
 
         >>> this('hello = "My name is %(suite)s"')
-        {'list': '    * %(suite)s ⇒ %(workflow)s'}
+        {'suggest': '\\n    * %(suite)s ⇒ %(workflow)s'}
 
-        >>> expect = {'list': (
-        ...     '    * %(suite)s ⇒ %(workflow)s    * %(task_url)s'
-        ... ' - get ``URL`` (if set in :cylc:conf:`[meta]URL`)')}
-        >>> this('hello = "My name is %(suite)s, %(task_url)s"') == expect
-        True
+        >>> this('hello = "My name is %(suite)s, %(task_url)s"')
+        {'suggest': '\\n    * %(suite)s ⇒ %(workf...cylc:conf:`[meta]URL`)'}
     """
     result = []
     for key, (regex, replacement) in deprecated_string_templates.items():
@@ -315,7 +322,7 @@
             result.append(f'%({key})s ⇒ %({replacement})s')
 
     if result:
-        return {'list': LIST_ITEM + LIST_ITEM.join(result)}
+        return {'suggest': LIST_ITEM + LIST_ITEM.join(result)}
     return None
 
 
@@ -370,7 +377,14 @@
 
 
 def check_lowercase_family_names(line: str) -> bool:
-    """Check for lowercase in family names."""
+    """Check for lowercase in family names.
+
+    Examples:
+        >>> check_lowercase_family_names(' inherit = FOO')
+        False
+        >>> check_lowercase_family_names(' inherit = foo')
+        True
+    """
     match = INHERIT_REGEX.match(line)
     if not match:
         return False
@@ -673,7 +687,7 @@
     },
     'U015': {
         'short': (
-            'Deprecated template variables.'),
+            'Deprecated template variables: {suggest}'),
         'rst': (
             'The following template variables, mostly used in event handlers,'
             'are deprecated, and should be replaced:'

tests/unit/scripts/test_lint_checkers.py

@@ -0,0 +1,95 @@
+#!/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/>.
+"""Test check functions in the `cylc lint` CLI Utility."""
+
+import doctest
+import json
+import pytest
+import re
+
+from cylc.flow.scripts import lint
+
+VARS = re.compile(r'\{(.*)\}')
+
+# Functions in Cylc Lint defined with "check_*
+CHECKERS = [
+    getattr(lint, i) for i in lint.__dir__() if i.startswith('check_')]
+# List of checks defined as checks by Cylc Lint
+ALL_CHECKS = [
+    *lint.MANUAL_DEPRECATIONS.values(),
+    *lint.STYLE_CHECKS.values(),
+]
+
+finder = doctest.DocTestFinder()
+
+
+@pytest.mark.parametrize(
+    'check',
+    # Those checks that have custom checker functions
+    # and a short message with variables to insert:
+    [
+        pytest.param(c, id=c.get('function').__name__) for c in ALL_CHECKS
+        if c.get('function') in CHECKERS
+    ]
+)
+def test_custom_checker_doctests(check):
+    """All check functions have at least one failure doctest
+
+    By forcing each check function to have valid doctests
+    for the case that linting has failed we are able to
+    check that the function outputs the correct information
+    for formatting the short formats.
+    """
+    doctests = finder.find(check['function'])[0]
+
+    msg = f'{check["function"].__name__}: No failure examples in doctest'
+    assert any(i.want for i in doctests.examples if i.want), msg
+
+
+@pytest.mark.parametrize(
+    'ref, check',
+    # Those checks that have custom checker functions
+    # and a short message with variables to insert:
+    [
+        (c.get('function'), c) for c in ALL_CHECKS
+        if c.get('function') in CHECKERS
+        and VARS.findall(c['short'])
+    ]
+)
+def test_custom_checkers_short_formatters(ref, check):
+    """If a check message has a format string assert that the checker
+    function will return a dict to be used in
+    ``check['short'].format(**kwargs)``, based on doctest output.
+
+    ref is useful to allow us to identify the check, even
+    though not used in the test.
+    """
+    doctests = finder.find(check['function'])[0]
+
+    # Filter doctest examples for cases where there is a json parsable
+    # want.
+    examples = [
+        eg for eg in [
+            json.loads(e.want.replace("'", '"'))
+            for e in doctests.examples if e.want
+        ]
+        if eg
+    ]
+
+    # Formatting using the example output changes the check short text:
+    for example in examples:
+        assert check['short'].format(**example) != check['short']