From d4704293986f70301d3d9428f79b282de5e72e98 Mon Sep 17 00:00:00 2001
From: Tom Coleman <15375218+ColemanTom@users.noreply.github.com>
Date: Wed, 17 May 2023 10:36:23 +1000
Subject: [PATCH 1/3] cylc lint now returns error code by default for issues

Previously it would provide a return code 0 always.
By default, it will now return 1 if an issue is found.
New flag --exit-zero changes back to original behaviour.
---
 cylc/flow/scripts/lint.py | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py
index f420eb24f89..e2065e9fe2d 100755
--- a/cylc/flow/scripts/lint.py
+++ b/cylc/flow/scripts/lint.py
@@ -31,6 +31,9 @@
 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.
 
+A non-zero return code will be returned if any issues are identified.
+This can be overridden by providing the "--exit-zero" flag.
+
 Configurations for Cylc lint can also be set in a pyproject.toml file.
 
 """
@@ -38,6 +41,7 @@
 from optparse import Values
 from pathlib import Path
 import re
+import sys
 import tomli
 from typing import Generator, Union
 
@@ -666,6 +670,13 @@ def get_option_parser() -> COP:
         metavar="CODE",
         choices=tuple([f'S{i["index"]:03d}' for i in STYLE_CHECKS.values()])
     )
+    parser.add_option(
+        '--exit-zero',
+        help='Exit with status code "0" even if there are issues.',
+        action='store_true',
+        default=False,
+        dest='exit_zero'
+    )
 
     return parser
 
@@ -678,7 +689,7 @@ def main(parser: COP, options: 'Values', target=None) -> None:
         else:
             rulesets = [options.linter]
         print(get_reference_text(parse_checks(rulesets, reference=True)))
-        exit(0)
+        sys.exit(0)
 
     # If target not given assume we are looking at PWD
     if target is None:
@@ -724,7 +735,9 @@ def main(parser: COP, options: 'Values', target=None) -> None:
             'Lint after renaming '
             '"suite.rc" to "flow.cylc"'
         )
-        exit(0)
+        # Exit with an error code if --exit-zero was not set.
+        # Return codes: sys.exit(True) == 1, sys.exit(False) == 0
+        sys.exit(not options.exit_zero)
     elif not cylc8 and '728' in mergedopts['rulesets']:
         check_names = mergedopts['rulesets']
         check_names.remove('728')
@@ -762,6 +775,11 @@ def main(parser: COP, options: 'Values', target=None) -> None:
 
     print(msg)
 
+    # Exit with an error code if there were warnings and
+    # if --exit-zero was not set.
+    # Return codes: sys.exit(True) == 1, sys.exit(False) == 0
+    sys.exit(count != 0 and not options.exit_zero)
+
 
 # NOTE: use += so that this works with __import__
 # (docstring needed for `cylc help all` output)

From 758193aa5256955c6b3fb2a1a159381c2c43ffe9 Mon Sep 17 00:00:00 2001
From: Tom Coleman <15375218+ColemanTom@users.noreply.github.com>
Date: Wed, 17 May 2023 10:36:56 +1000
Subject: [PATCH 2/3] Update CHANGES.md for cylc lint RC enhancement

---
 CHANGES.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CHANGES.md b/CHANGES.md
index 8acab64893c..bcec72c7d5b 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -21,6 +21,12 @@ for `cylc play` when called by `cylc vip` or `cylc vr`.
 -------------------------------------------------------------------------------
 ## __cylc-8.1.5 (<span actions:bind='release-date'>Awaiting Release</span>)__
 
+### Enhancements
+
+[#5546](https://github.com/cylc/cylc-flow/pull/5546) -
+`cylc lint` will provide a non-zero return code if any issues are identified.
+This can be overridden using the new `--exit-zero` flag.
+
 ### Fixes
 
 [#5228](https://github.com/cylc/cylc-flow/pull/5228) -

From e8e23893ed87548ac868fad9fc9f9876f328d464 Mon Sep 17 00:00:00 2001
From: Tom Coleman <15375218+ColemanTom@users.noreply.github.com>
Date: Wed, 17 May 2023 10:37:42 +1000
Subject: [PATCH 3/3] Add functional tests for cylc lint non-0 return

---
 tests/functional/cylc-lint/00.lint.t      | 22 ++++++++++++++--------
 tests/functional/cylc-lint/01.lint-toml.t |  6 +++---
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/tests/functional/cylc-lint/00.lint.t b/tests/functional/cylc-lint/00.lint.t
index cafaef32d27..f4e1c80b394 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 18
+set_test_number 20
 
 cat > flow.cylc <<__HERE__
 # This is definitely not an OK flow.cylc file.
@@ -29,15 +29,15 @@ __HERE__
 rm etc/global.cylc
 
 TEST_NAME="${TEST_NAME_BASE}.vanilla"
-run_ok "${TEST_NAME}" cylc lint .
+run_fail "${TEST_NAME}" cylc lint .
 named_grep_ok "check-for-error-code" "S004" "${TEST_NAME}.stdout"
 
 TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset"
-run_ok "${TEST_NAME}" cylc lint . -r 728
+run_fail "${TEST_NAME}" cylc lint . -r 728
 named_grep_ok "check-for-error-code" "U024" "${TEST_NAME}.stdout"
 
 TEST_NAME="${TEST_NAME_BASE}.inplace"
-run_ok "${TEST_NAME}" cylc lint . -i
+run_fail "${TEST_NAME}" cylc lint . -i
 named_grep_ok "check-for-error-code-in-file" "U024" flow.cylc
 
 rm flow.cylc
@@ -47,12 +47,18 @@ cat > suite.rc <<__HERE__
 {{FOO}}
 __HERE__
 
-TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset"
-run_ok "${TEST_NAME}" cylc lint . -r 728
+TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset-728"
+run_fail "${TEST_NAME}" cylc lint . -r 728
 named_grep_ok "do-not-upgrade-check-if-compat-mode" "Lint after renaming" "${TEST_NAME}.stderr"
 
-TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset2"
-run_ok "${TEST_NAME}" cylc lint . -r all
+TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset-728-exit-zero"
+run_ok "${TEST_NAME}" cylc lint . -r 728 --exit-zero
+
+TEST_NAME="${TEST_NAME_BASE}.pick-a-ruleset-all"
+run_fail "${TEST_NAME}" cylc lint . -r all
+
+TEST_NAME="${TEST_NAME_BASE}.exit-zero"
+run_ok "${TEST_NAME}" cylc lint --exit-zero .
 
 rm suite.rc
 
diff --git a/tests/functional/cylc-lint/01.lint-toml.t b/tests/functional/cylc-lint/01.lint-toml.t
index 5f0d13ee17d..c31ebd1c4ea 100644
--- a/tests/functional/cylc-lint/01.lint-toml.t
+++ b/tests/functional/cylc-lint/01.lint-toml.t
@@ -47,7 +47,7 @@ __HERE__
 
 # Control tests
 TEST_NAME="it lints without toml file"
-run_ok "${TEST_NAME}" cylc lint
+run_fail "${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}"
@@ -75,7 +75,7 @@ __HERE__
 
 # Test that results are different:
 TEST_NAME="it_lints_with_toml_file"
-run_ok "${TEST_NAME}" cylc lint
+run_fail "${TEST_NAME}" cylc lint
 TESTOUT="${TEST_NAME}.stdout"
 grep_fail "S004" "${TESTOUT}"
 grep_fail "niwa.cylc" "${TESTOUT}"
@@ -93,7 +93,7 @@ cat > flow.cylc <<__HERE__
 __HERE__
 
 TEST_NAME="it_fails_if_max-line-length_set"
-run_ok "${TEST_NAME}" cylc lint
+run_fail "${TEST_NAME}" cylc lint
 named_grep_ok "${TEST_NAME}-line-too-long-message" \
     "\[S008\] flow.cylc:2: line > 4 characters." \
     "${TEST_NAME}.stdout"