From a88733a090b3e3dd83a50a357cb6a6c83b0d406e Mon Sep 17 00:00:00 2001
From: Oliver Sanders <oliver.sanders@metoffice.gov.uk>
Date: Tue, 23 Apr 2024 10:04:10 +0100
Subject: [PATCH] outputs: better validation errors for alt qualifiers in
 completion exprs

---
 cylc/flow/config.py                        | 21 +++++++++++++++++----
 tests/integration/validate/test_outputs.py | 12 ++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/cylc/flow/config.py b/cylc/flow/config.py
index f5ce67488e6..c4b340a8f0e 100644
--- a/cylc/flow/config.py
+++ b/cylc/flow/config.py
@@ -81,6 +81,7 @@
     is_relative_to,
 )
 from cylc.flow.print_tree import print_tree
+from cylc.flow.task_qualifiers import ALT_QUALIFIERS
 from cylc.flow.simulation import configure_sim_modes
 from cylc.flow.subprocctx import SubFuncContext
 from cylc.flow.task_events_mgr import (
@@ -96,6 +97,7 @@
     get_completion_expression,
     get_optional_outputs,
     get_trigger_completion_variable_maps,
+    trigger_to_completion_variable,
 )
 from cylc.flow.task_trigger import TaskTrigger, Dependency
 from cylc.flow.taskdef import TaskDef
@@ -1096,14 +1098,14 @@ def _check_completion_expression(self, task_name: str, expr: str) -> None:
             return
 
         (
-            trigger_to_completion_variable,
-            completion_variable_to_trigger,
+            _trigger_to_completion_variable,
+            _completion_variable_to_trigger,
         ) = get_trigger_completion_variable_maps(outputs.keys())
 
         # get the optional/required outputs defined in the graph
         graph_optionals = {
             # completion_variable: is_optional
-            trigger_to_completion_variable[trigger]: (
+            _trigger_to_completion_variable[trigger]: (
                 None if is_required is None else not is_required
             )
             for trigger, (_, is_required)
@@ -1136,6 +1138,17 @@ def _check_completion_expression(self, task_name: str, expr: str) -> None:
                     ' expressions, use "succeeded or failed".'
                 )
 
+            for alt_qualifier, qualifier in ALT_QUALIFIERS.items():
+                _alt_compvar = trigger_to_completion_variable(alt_qualifier)
+                _compvar = trigger_to_completion_variable(qualifier)
+                if re.search(rf'\b{_alt_compvar}\b', error):
+                    raise WorkflowConfigError(
+                        f'Error in [runtime][{task_name}]completion:'
+                        f'\n  {expr}'
+                        f'\nUse "{_compvar}" not "{_alt_compvar}" '
+                        'in completion expressions.'
+                    )
+
             raise WorkflowConfigError(
                 # NOTE: str(exc) == "name 'x' is not defined" tested in
                 # tests/integration/test_optional_outputs.py
@@ -1179,7 +1192,7 @@ def _check_completion_expression(self, task_name: str, expr: str) -> None:
 
             # [1] applies only to "submit-failed" and "expired"
 
-            trigger = completion_variable_to_trigger[compvar]
+            trigger = _completion_variable_to_trigger[compvar]
 
             if graph_opt is True and expr_opt is False:
                 raise WorkflowConfigError(
diff --git a/tests/integration/validate/test_outputs.py b/tests/integration/validate/test_outputs.py
index 3ddeba0e96d..a9fd55f0ef2 100644
--- a/tests/integration/validate/test_outputs.py
+++ b/tests/integration/validate/test_outputs.py
@@ -219,6 +219,18 @@ def test_messages(messages, valid, flow, validate):
             'but required in the completion expression',
             id='failed-implicitly-optional-in-graph-required-in-completion',
         ),
+        pytest.param(
+            'foo',
+            '(succeed and x) or failed',
+            'Use "succeeded" not "succeed" in completion expressions',
+            id='alt-compvar1',
+        ),
+        pytest.param(
+            'foo? & foo:submitted?',
+            'submit_fail or succeeded',
+            'Use "submit_failed" not "submit_fail" in completion expressions',
+            id='alt-compvar2',
+        ),
     ]
 )
 def test_completion_expression_invalid(