From 98b31298b51f0ff071a6b12332c3ea1d11aa3cbc Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 22 Aug 2023 15:26:19 +0100
Subject: [PATCH 1/6] Ensure that a `:suite.rc` is added to the template
 language when creating defines.

---
 CHANGES.md                         |  3 +++
 cylc/rose/stem.py                  |  5 ++--
 cylc/rose/utilities.py             | 22 ++++++++++++++---
 tests/unit/test_config_node.py     | 15 ++++++++++++
 tests/unit/test_rose_stem_units.py | 39 +++++++++++++++++++++++++++++-
 5 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index d5fe9cfd..aa700476 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -13,6 +13,9 @@ ones in. -->
 [#250](https://github.com/cylc/cylc-rose/pull/250) - Prevent project
 name being manually set to an empty string.
 
+[#248](https://github.com/cylc/cylc-rose/pull/248) - Make sure that
+rose stem sets variables in `[jinja2:suite.rc]` not `[jinja2]`.
+
 ## __cylc-rose-1.3.0 (<span actions:bind='release-date'>Released 2023-07-21</span>)__
 
 ### Fixes
diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py
index 49b05efb..f061f9c9 100644
--- a/cylc/rose/stem.py
+++ b/cylc/rose/stem.py
@@ -75,6 +75,7 @@
 )
 
 from cylc.rose.entry_points import get_rose_vars
+from cylc.rose.utilities import id_templating_section
 
 import metomi.rose.config
 from metomi.rose.fs_util import FileSystemUtil
@@ -452,10 +453,10 @@ def process(self):
             self.opts.project.append(project)
 
             if i == 0:
-                # Get the name of the template section to be used:
                 template_type = get_rose_vars(
                     Path(url) / "rose-stem")["templating_detected"]
-                self.template_section = f'[{template_type}]'
+                self.template_section = id_templating_section(
+                    template_type, with_brackets=True)
 
             # Versions of variables with hostname prepended for working copies
             url_host = self._prepend_localhost(url)
diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py
index da367b47..47f3b1ad 100644
--- a/cylc/rose/utilities.py
+++ b/cylc/rose/utilities.py
@@ -21,7 +21,7 @@
 from pathlib import Path
 import re
 import shlex
-from typing import TYPE_CHECKING, Any, List, Tuple, Union
+from typing import TYPE_CHECKING, Any, List, Optional, Tuple, Union
 
 from cylc.flow.hostuserutil import get_host
 from cylc.flow import LOG
@@ -192,13 +192,27 @@ def identify_templating_section(config_node):
             "You should not define more than one templating section. "
             f"You defined:\n\t{'; '.join(defined_sections)}"
         )
-    elif 'jinja2:suite.rc' in defined_sections:
+    elif defined_sections:
+        return id_templating_section(defined_sections.pop())
+    else:
+        return id_templating_section('')
+
+
+def id_templating_section(
+    section: Optional[str] = None,
+    with_brackets: bool = False
+) -> str:
+    """Return a full template section string."""
+    templating = None
+    if section and 'jinja2' in section:
         templating = 'jinja2:suite.rc'
-    elif 'empy:suite.rc' in defined_sections:
+    elif section and 'empy' in section:
         templating = 'empy:suite.rc'
-    else:
+
+    if not templating:
         templating = 'template variables'
 
+    templating = f'[{templating}]' if with_brackets else templating
     return templating
 
 
diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py
index 86bf8232..bcd8d422 100644
--- a/tests/unit/test_config_node.py
+++ b/tests/unit/test_config_node.py
@@ -32,6 +32,7 @@
     deprecation_warnings,
     dump_rose_log,
     identify_templating_section,
+    id_templating_section,
     MultipleTemplatingEnginesError
 )
 
@@ -252,6 +253,20 @@ def test_identify_templating_section(node_, expect, raises):
             identify_templating_section(node)
 
 
+@pytest.mark.parametrize(
+    'input_, expect',
+    (
+        ([None], 'template variables'),
+        (['jinja2'], 'jinja2:suite.rc'),
+        (['jinja2:suite.rc'], 'jinja2:suite.rc'),
+        ([None, True], '[template variables]'),
+        (['jinja2', True], '[jinja2:suite.rc]'),
+    )
+)
+def test_id_templating_section(input_, expect):
+    assert id_templating_section(*input_) == expect
+
+
 @pytest.fixture
 def node_with_ROSE_ORIG_HOST():
     def _inner(comment=''):
diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py
index d88bbdec..0df9edd2 100644
--- a/tests/unit/test_rose_stem_units.py
+++ b/tests/unit/test_rose_stem_units.py
@@ -23,11 +23,12 @@
 from typing import Any, Tuple
 
 from cylc.rose.stem import (
+    _get_rose_stem_opts,
     ProjectNotFoundException,
     RoseStemVersionException,
     RoseSuiteConfNotFoundException,
     StemRunner,
-    get_source_opt_from_args
+    get_source_opt_from_args,
 )
 
 from metomi.rose.reporter import Reporter
@@ -337,3 +338,39 @@ def test_ascertain_project_if_name_supplied(
             ProjectNotFoundException, match='is not a working copy'
         ):
             stemrunner._ascertain_project(item)
+
+
+@pytest.mark.parametrize(
+    'language, expect',
+    (
+        ('empy', '[empy:suite.rc]'),
+        ('jinja2', '[jinja2:suite.rc]'),
+        ('template variables', '[template variables]'),
+    )
+)
+def test_process_template_engine_set_correctly(monkeypatch, language, expect):
+    """Defines are correctly assigned a [<template language>:suite.rc]
+    section.
+
+    https://github.com/cylc/cylc-rose/issues/246
+    """
+    # Mimic expected result from get_rose_vars method:
+    monkeypatch.setattr(
+        'cylc.rose.stem.get_rose_vars',
+        lambda _: {'templating_detected': language}
+    )
+    monkeypatch.setattr(
+        'sys.argv',
+        ['foo', 'bar']
+    )
+
+    # We are not interested in these checks, just in the defines
+    # created by the process method.
+    stemrunner = StemRunner(_get_rose_stem_opts()[1])
+    stemrunner._ascertain_project = lambda _: ['', '', '', '', '']
+    stemrunner._this_suite = lambda: '.'
+    stemrunner._check_suite_version = lambda _: '1'
+    stemrunner.process()
+
+    for define in stemrunner.opts.defines:
+        assert define.startswith(expect)

From 5d4d389b871824b57501eaf600c6dd9969e51336 Mon Sep 17 00:00:00 2001
From: WXTIM <26465611+wxtim@users.noreply.github.com>
Date: Mon, 9 Oct 2023 14:06:11 +0100
Subject: [PATCH 2/6] Raise error if = not in define

---
 CHANGES.md                |  3 +++
 cylc/rose/entry_points.py |  6 ++++++
 cylc/rose/utilities.py    | 44 +++++++++++++++++++++++++++++++++++----
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index d5fe9cfd..2556ffee 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -13,6 +13,9 @@ ones in. -->
 [#250](https://github.com/cylc/cylc-rose/pull/250) - Prevent project
 name being manually set to an empty string.
 
+[#225](https://github.com/cylc/cylc-rose/pull/225) - Prevent totally invalid
+CLI --defines with no = sign.
+
 ## __cylc-rose-1.3.0 (<span actions:bind='release-date'>Released 2023-07-21</span>)__
 
 ### Fixes
diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index 7e5327f4..21c3b66e 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -31,6 +31,7 @@
     dump_rose_log,
     get_rose_vars_from_config_node,
     identify_templating_section,
+    invalid_defines_check,
     rose_config_exists,
     rose_config_tree_loader,
     merge_rose_cylc_suite_install_conf,
@@ -38,6 +39,7 @@
     get_cli_opts_node,
     add_cylc_install_to_rose_conf_node_opts,
 )
+from cylc.flow.flags import cylc7_back_compat
 from cylc.flow.hostuserutil import get_host
 
 
@@ -120,6 +122,10 @@ def get_rose_vars(srcdir=None, opts=None):
             raise NotARoseSuiteException()
         return config
 
+    # Check for definitely invalid defines
+    if opts and hasattr(opts, 'defines') and not cylc7_back_compat:
+        invalid_defines_check(opts.defines)
+
     # Load the raw config tree
     config_tree = rose_config_tree_loader(srcdir, opts)
     deprecation_warnings(config_tree)
diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py
index da367b47..cc607932 100644
--- a/cylc/rose/utilities.py
+++ b/cylc/rose/utilities.py
@@ -25,11 +25,12 @@
 
 from cylc.flow.hostuserutil import get_host
 from cylc.flow import LOG
+from cylc.flow.exceptions import CylcError
 from cylc.flow.flags import cylc7_back_compat
 from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros
 from metomi.rose import __version__ as ROSE_VERSION
 from metomi.isodatetime.datetimeoper import DateTimeOperator
-from metomi.rose.config import ConfigDumper, ConfigNodeDiff, ConfigNode
+from metomi.rose.config import ConfigNodeDiff, ConfigNode, ConfigDumper
 from metomi.rose.config_processor import ConfigProcessError
 from metomi.rose.env import env_var_process, UnboundEnvironmentVariableError
 
@@ -46,7 +47,11 @@
 ALL_MODES = 'all modes'
 
 
-class MultipleTemplatingEnginesError(Exception):
+class MultipleTemplatingEnginesError(CylcError):
+    ...
+
+
+class InvalidDefineError(CylcError):
     ...
 
 
@@ -327,11 +332,42 @@ def merge_rose_cylc_suite_install_conf(old, new):
     return old
 
 
+def invalid_defines_check(defines: List) -> None:
+    """Check for defines which do not contain an = and therefore cannot be
+    valid
+
+    Examples:
+
+        # A single invalid define:
+        >>> import pytest
+        >>> with pytest.raises(InvalidDefineError, match=r'\\* foo'):
+        ...    invalid_defines_check(['foo'])
+
+        # Two invalid defines and one valid one:
+        >>> with pytest.raises(
+        ...     InvalidDefineError, match=r'\\* foo.*\\n.* \\* bar'
+        ... ):
+        ...     invalid_defines_check(['foo', 'bar52', 'baz=442'])
+
+        # No invalid defines
+        >>> invalid_defines_check(['foo=12'])
+    """
+    invalid_defines = []
+    for define in defines:
+        if parse_cli_defines(define) is False:
+            invalid_defines.append(define)
+    if invalid_defines:
+        msg = 'Invalid Suite Defines (should contain an =)'
+        for define in invalid_defines:
+            msg += f'\n * {define}'
+        raise InvalidDefineError(msg)
+
+
 def parse_cli_defines(define: str) -> Union[
-    bool, Tuple[
+    bool, str, Tuple[
         List[Union[str, Any]],
         Union[str, Any],
-        Union[str, Any]
+        Union[str, Any],
     ]
 ]:
     """Parse a define string.

From d35f74fa732fbf0abeaa1c12988bd63875dcbfd6 Mon Sep 17 00:00:00 2001
From: WXTIM <26465611+wxtim@users.noreply.github.com>
Date: Thu, 12 Oct 2023 10:13:53 +0100
Subject: [PATCH 3/6] remove back compatibility check

---
 cylc/rose/entry_points.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py
index 21c3b66e..8da910b0 100644
--- a/cylc/rose/entry_points.py
+++ b/cylc/rose/entry_points.py
@@ -39,7 +39,6 @@
     get_cli_opts_node,
     add_cylc_install_to_rose_conf_node_opts,
 )
-from cylc.flow.flags import cylc7_back_compat
 from cylc.flow.hostuserutil import get_host
 
 
@@ -123,7 +122,7 @@ def get_rose_vars(srcdir=None, opts=None):
         return config
 
     # Check for definitely invalid defines
-    if opts and hasattr(opts, 'defines') and not cylc7_back_compat:
+    if opts and hasattr(opts, 'defines'):
         invalid_defines_check(opts.defines)
 
     # Load the raw config tree

From 8b5d7f2dff9fb63ff77dce1e51d151cc88463199 Mon Sep 17 00:00:00 2001
From: Mark Dawson <mark.dawson@metoffice.gov.uk>
Date: Wed, 18 Oct 2023 14:00:28 +0100
Subject: [PATCH 4/6] added flake8-type-checking (#257)

Co-authored-by: Mark Dawson <mark.dawson@metoffice.gov.ukpython>
---
 CONTRIBUTING.md | 1 +
 mypy.ini        | 4 ++++
 setup.cfg       | 1 +
 3 files changed, 6 insertions(+)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index c4f5b82f..dacd98d3 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -44,6 +44,7 @@ below.
  - Mel Hall
  - Bruno Kinoshita
  - Hilary Oliver
+ - Mark Dawson
 <!-- end-shortlog -->
 
 (All contributors are identifiable with email addresses in the git version
diff --git a/mypy.ini b/mypy.ini
index 51e43ae4..2097c6f1 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -11,3 +11,7 @@ explicit_package_bases = True
 allow_redefinition = True
 strict_equality = True
 show_error_codes = True
+
+# Suppress the following messages:
+# By default the bodies of untyped functions are not checked, consider using --check-untyped-defs
+disable_error_code = annotation-unchecked
\ No newline at end of file
diff --git a/setup.cfg b/setup.cfg
index 15786025..2fe1677c 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -75,6 +75,7 @@ tests =
     flake8-debugger>=4.0.0
     flake8-mutable>=1.2.0
     flake8-simplify>=0.15.1
+    flake8-type-checking; python_version > "3.7"
     mypy>=0.910
     pytest
     pytest-cov

From 9f44894330aadf0ae20343836ec3c6b5996cc910 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Tue, 24 Oct 2023 10:18:47 +0000
Subject: [PATCH 5/6] Prepare release 1.3.1

Workflow: Release stage 1 - create release PR, run: 34
---
 CHANGES.md            | 2 +-
 cylc/rose/__init__.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 96fab350..27af8c00 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -6,7 +6,7 @@ creating a new release entry be sure to copy & paste the span tag with the
 updated. Only the first match gets replaced, so it's fine to leave the old
 ones in. -->
 
-## __cylc-rose-1.3.1 (<span actions:bind='release-date'>Upcoming</span>)__
+## __cylc-rose-1.3.1 (<span actions:bind='release-date'>Released 2023-10-24</span>)__
 
 ### Fixes
 
diff --git a/cylc/rose/__init__.py b/cylc/rose/__init__.py
index 1229ce43..c0d97bee 100644
--- a/cylc/rose/__init__.py
+++ b/cylc/rose/__init__.py
@@ -205,4 +205,4 @@
 
 """
 
-__version__ = '1.3.1.dev'
+__version__ = '1.3.1'

From f27273267f70c38de2972a5e140bbd9ef72f0fc5 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Tue, 24 Oct 2023 11:59:37 +0000
Subject: [PATCH 6/6] Bump dev version

Workflow: Release stage 2 - auto publish, run: 38
---
 cylc/rose/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cylc/rose/__init__.py b/cylc/rose/__init__.py
index c0d97bee..73c675e5 100644
--- a/cylc/rose/__init__.py
+++ b/cylc/rose/__init__.py
@@ -205,4 +205,4 @@
 
 """
 
-__version__ = '1.3.1'
+__version__ = '1.3.2.dev'