entry_points: remove type uncertainty

* Entry points accept missing or None values for arguments, however,
  this can never happen in practice.
* Set the appropriate type hints and remove code paths which could not
  get executed.

cylc/rose/entry_points.py

@@ -16,6 +16,7 @@
 """Top level module providing entry point functions."""
 
 from pathlib import Path
+from typing import TYPE_CHECKING
 
 from cylc.rose.utilities import (
     copy_config_file,
@@ -27,8 +28,11 @@
     rose_config_exists,
 )
 
+if TYPE_CHECKING:
+    from cylc.flow.option_parsers import Values
 
-def pre_configure(srcdir=None, opts=None, rundir=None) -> dict:
+
+def pre_configure(srcdir: Path, opts: 'Values') -> dict:
     """Run before the Cylc configuration is read."""
     if not srcdir:
         # not sure how this could happen
@@ -51,32 +55,25 @@ def pre_configure(srcdir=None, opts=None, rundir=None) -> dict:
     return plugin_result
 
 
-def post_install(srcdir=None, opts=None, rundir=None) -> bool:
+def post_install(srcdir: Path, rundir: str, opts: 'Values') -> bool:
     """Run after Cylc file installation has completed."""
     from cylc.rose.fileinstall import rose_fileinstall
 
-    if (
-        not srcdir
-        or not rundir
-        or not rose_config_exists(srcdir)
-    ):
+    if not rose_config_exists(srcdir):
         # nothing to do here
         return False
-    srcdir: Path = Path(srcdir)
-    rundir: Path = Path(rundir)
+    _rundir: Path = Path(rundir)
 
     # transfer the rose-suite.conf file
-    copy_config_file(srcdir=srcdir, rundir=rundir)
+    copy_config_file(srcdir=srcdir, rundir=_rundir)
 
     # write cylc-install CLI options to an optional config
-    record_cylc_install_options(
-        srcdir=srcdir, opts=opts, rundir=rundir
-    )
+    record_cylc_install_options(srcdir, _rundir, opts)
 
     # perform file installation
-    config_node = rose_fileinstall(rundir, opts)
+    config_node = rose_fileinstall(_rundir, opts)
     if config_node:
-        dump_rose_log(rundir=rundir, node=config_node)
+        dump_rose_log(rundir=_rundir, node=config_node)
 
     return True
 

cylc/rose/fileinstall.py

@@ -17,16 +17,20 @@
 """Utilities related to performing Rose file installation."""
 
 import os
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Union
 
 from cylc.rose.utilities import rose_config_exists, rose_config_tree_loader
 
 if TYPE_CHECKING:
     from pathlib import Path
-    from cylc.flow.option_parser import Values
+    from cylc.flow.option_parsers import Values
+    from metomi.rose.config import ConfigNode
 
 
-def rose_fileinstall(rundir: 'Path', opts: 'Values'):
+def rose_fileinstall(
+    rundir: 'Path',
+    opts: 'Values',
+) -> 'Union[ConfigNode, bool]':
     """Call Rose Fileinstall.
 
     Args:
@@ -39,7 +43,6 @@ def rose_fileinstall(rundir: 'Path', opts: 'Values'):
         return False
 
     # Load the config tree
-    # TODO: private
     config_tree = rose_config_tree_loader(rundir, opts)
 
     if any(i.startswith('file') for i in config_tree.node.value):

cylc/rose/utilities.py

@@ -24,7 +24,7 @@
 import re
 import shlex
 import shutil
-from typing import Any, Dict, List, Optional, Tuple, Union
+from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union
 
 from cylc.flow import LOG
 from cylc.flow.exceptions import CylcError
@@ -44,6 +44,9 @@
 
 from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros
 
+if TYPE_CHECKING:
+    from cylc.flow.option_parsers import Values
+
 
 SECTIONS = {'jinja2:suite.rc', 'empy:suite.rc', 'template variables'}
 SET_BY_CYLC = 'set by Cylc'
@@ -255,7 +258,7 @@ def rose_config_exists(dir_: Path) -> bool:
     return (dir_ / 'rose-suite.conf').is_file()
 
 
-def rose_config_tree_loader(srcdir=None, opts=None):
+def rose_config_tree_loader(srcdir: Path, opts: 'Values') -> ConfigTree:
     """Get a rose config tree from srcdir.
 
     Args:
@@ -275,15 +278,15 @@ def rose_config_tree_loader(srcdir=None, opts=None):
         opt_conf_keys += shlex.split(opt_conf_keys_env)
 
     # ... or as command line options
-    if opts and 'opt_conf_keys' in dir(opts) and opts.opt_conf_keys:
+    if 'opt_conf_keys' in dir(opts) and opts.opt_conf_keys:
         if isinstance(opts.opt_conf_keys, str):
             opt_conf_keys += opts.opt_conf_keys.split()
         else:
             opt_conf_keys += opts.opt_conf_keys
 
     # Optional definitions
     redefinitions = []
-    if opts and 'defines' in dir(opts) and opts.defines:
+    if 'defines' in dir(opts) and opts.defines:
         redefinitions = opts.defines
 
     # Load the config tree
@@ -451,7 +454,7 @@ def parse_cli_defines(define: str) -> Union[
     return (keys, match['value'], match['state'])
 
 
-def get_cli_opts_node(opts=None, srcdir=None):
+def get_cli_opts_node(srcdir: Path, opts: 'Values'):
     """Create a ConfigNode representing options set on the command line.
 
     Args:
@@ -463,12 +466,13 @@ def get_cli_opts_node(opts=None, srcdir=None):
 
     Example:
         >>> from types import SimpleNamespace
+        >>> from pathlib import Path
         >>> opts = SimpleNamespace(
         ...     opt_conf_keys='A B',
         ...     defines=["[env]FOO=BAR"],
         ...     rose_template_vars=["QUX=BAZ"]
         ... )
-        >>> node = get_cli_opts_node(opts)
+        >>> node = get_cli_opts_node(Path('no/such/dir'), opts)
         >>> node['opts']
         {'value': 'A B', 'state': '!', 'comments': []}
         >>> node['env']['FOO']
@@ -477,9 +481,9 @@ def get_cli_opts_node(opts=None, srcdir=None):
         {'value': 'BAZ', 'state': '', 'comments': []}
     """
     # Unpack info we want from opts:
-    opt_conf_keys = []
-    defines = []
-    rose_template_vars = []
+    opt_conf_keys: list = []
+    defines: list = []
+    rose_template_vars: list = []
     if opts and 'opt_conf_keys' in dir(opts):
         opt_conf_keys = opts.opt_conf_keys or []
     if opts and 'defines' in dir(opts):
@@ -502,21 +506,26 @@ def get_cli_opts_node(opts=None, srcdir=None):
             newconfig.set(*parsed_define)
 
     # For each __suite define__ add define.
-    if srcdir is not None:
-        config_node = rose_config_tree_loader(srcdir, opts).node
-        templating = identify_templating_section(config_node)
-    else:
+    templating: str
+    if not rose_config_exists(srcdir):
         templating = 'template variables'
+    else:
+        templating = identify_templating_section(
+            rose_config_tree_loader(srcdir, opts).node
+        )
 
     for define in rose_template_vars:
-        match = re.match(
+        _match = re.match(
             r'(?P<state>!{0,2})(?P<key>.*)\s*=\s*(?P<value>.*)', define
-        ).groupdict()
+        )
+        if not _match:
+            raise ValueError(f'Invalid define: {define}')
+        _match_groups = _match.groupdict()
         # Guess templating type?
         newconfig.set(
-            keys=[templating, match['key']],
-            value=match['value'],
-            state=match['state']
+            keys=[templating, _match_groups['key']],
+            value=_match_groups['value'],
+            state=_match_groups['state']
         )
 
     # Specialised treatement of optional configs.
@@ -650,7 +659,7 @@ def simplify_opts_strings(opts):
     return ' '.join(reversed(seen_once))
 
 
-def dump_rose_log(rundir, node):
+def dump_rose_log(rundir: Path, node: ConfigNode):
     """Dump a config node to a timestamped file in the ``log`` sub-directory.
 
     Args:
@@ -807,8 +816,7 @@ def deprecation_warnings(config_tree):
 
 def load_rose_config(
     srcdir: Path,
-    opts=None,
-    warn: bool = True,
+    opts: 'Optional[Values]' = None,
 ) -> 'ConfigTree':
     """Load rose configuration from srcdir.
 
@@ -824,20 +832,12 @@ def load_rose_config(
         opts:
             Options object containing specification of optional
             configuarations set by the CLI.
-        warn:
-            Log deprecation warnings if True.
+
+            Note: this is None for "rose stem" usage.
 
     Returns:
-        dict - A dictionary of sections of rose-suite.conf.
-        For each section either a dictionary or None is returned.
-        E.g.
-            {
-                'env': {'MYVAR': 42},
-                'empy:suite.rc': None,
-                'jinja2:suite.rc': {
-                    'myJinja2Var': {'yes': 'it is a dictionary!'}
-                }
-            }
+        The Rose configuration tree for "srcdir".
+
     """
     # Return a blank config dict if srcdir does not exist
     if not rose_config_exists(srcdir):
@@ -858,23 +858,22 @@ def load_rose_config(
 
     # Load the raw config tree
     config_tree = rose_config_tree_loader(srcdir, opts)
-    if warn:
-        deprecation_warnings(config_tree)
+    deprecation_warnings(config_tree)
 
     return config_tree
 
 
-def export_environment(environment):
+def export_environment(environment: Dict[str, str]) -> None:
     # Export environment vars
     for key, val in environment.items():
         os.environ[key] = val
 
 
 def record_cylc_install_options(
-    rundir=None,
-    opts=None,
-    srcdir=None,
-):
+    srcdir: Path,
+    rundir: Path,
+    opts: 'Values',
+) -> Tuple[ConfigNode, ConfigNode]:
     """Create/modify files recording Cylc install config options.
 
     Creates a new config based on CLI options and writes it to the workflow
@@ -887,28 +886,32 @@ def record_cylc_install_options(
     been written in the installed ``rose-suite.conf``.
 
     Args:
-        srcdir (pathlib.Path):
+        srcdir:
             Used to check whether the source directory contains a rose config.
+        rundir:
+            Path to dump the rose-suite-cylc-conf
         opts:
             Cylc option parser object - we want to extract the following
             values:
-            - opt_conf_keys (list or str):
+            - opt_conf_keys (list of str):
                 Equivalent of ``rose suite-run --option KEY``
             - defines (list of str):
                 Equivalent of ``rose suite-run --define KEY=VAL``
             - rose_template_vars (list of str):
                 Equivalent of ``rose suite-run --define-suite KEY=VAL``
-        rundir (pathlib.Path):
-            Path to dump the rose-suite-cylc-conf
 
     Returns:
-        cli_config - Config Node which has been dumped to
-        ``rose-suite-cylc-install.conf``.
-        rose_suite_conf['opts'] - Opts section of the config node dumped to
-        installed ``rose-suite.conf``.
+        Tuple - (cli_config, rose_suite_conf)
+
+        cli_config:
+            The Cylc install config aka "rose-suite-cylc-install.conf".
+        rose_suite_conf:
+            The "opts" section of the config node dumped to
+            installed ``rose-suite.conf``.
+
     """
     # Create a config based on command line options:
-    cli_config = get_cli_opts_node(opts, srcdir)
+    cli_config = get_cli_opts_node(srcdir, opts)
 
     # raise error if CLI config has multiple templating sections
     identify_templating_section(cli_config)

tests/unit/test_config_tree.py

@@ -16,6 +16,7 @@
 """Tests the plugin with Rose suite configurations on the filesystem."""
 
 from io import StringIO
+from pathlib import Path
 from types import SimpleNamespace
 
 from cylc.flow.hostuserutil import get_host
@@ -501,7 +502,7 @@ def test_get_cli_opts_node(opt_confs, defines, rose_template_vars, expect):
     )
     loader = ConfigLoader()
     expect = loader.load(StringIO(expect))
-    result = get_cli_opts_node(opts)
+    result = get_cli_opts_node(Path('no/such/dir'), opts)
     for item in ['env', 'template variables', 'opts']:
         assert result[item] == expect[item]
 

tests/unit/test_functional_post_install.py

@@ -60,7 +60,7 @@ def assert_rose_conf_full_equal(left, right, no_ignore=True):
 
 
 def test_no_rose_suite_conf_in_devdir(tmp_path):
-    result = post_install(srcdir=tmp_path)
+    result = post_install(tmp_path, tmp_path, SimpleNamespace())
     assert result is False
 
 
@@ -82,7 +82,7 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path):
     )
 
     # Run both record_cylc_install options and fileinstall.
-    record_cylc_install_options(opts=opts, rundir=destdir)
+    record_cylc_install_options(srcdir, destdir, opts)
     rose_fileinstall(destdir, opts)
     assert ((destdir / 'installedme').read_text() ==
             'Galileo No! We will not let you go.'
@@ -248,11 +248,7 @@ def fake(*arg, **kwargs):
     loader = ConfigLoader()
 
     # Run the entry point top-level function:
-    rose_suite_cylc_install_node, rose_suite_opts_node = (
-        record_cylc_install_options(
-            rundir=testdir, opts=opts, srcdir=testdir
-        )
-    )
+    record_cylc_install_options(testdir, testdir, opts=opts)
     rose_fileinstall(testdir, opts)
     ritems = sorted([i.relative_to(refdir) for i in refdir.rglob('*')])
     titems = sorted([i.relative_to(testdir) for i in testdir.rglob('*')])
@@ -321,11 +317,7 @@ def test_template_section_conflict(
 
     with pytest.raises(MultipleTemplatingEnginesError) as exc_info:
         # Run the entry point top-level function:
-        rose_suite_cylc_install_node, rose_suite_opts_node = (
-            record_cylc_install_options(
-                rundir=testdir, opts=opts, srcdir=testdir
-            )
-        )
+        record_cylc_install_options(testdir, testdir, opts)
     assert exc_info.match(expect)
 
 
@@ -356,7 +348,7 @@ def test_cylc_no_rose(tmp_path):
     """A Cylc workflow that contains no ``rose-suite.conf`` installs OK.
     """
     from cylc.rose.entry_points import post_install
-    assert post_install(srcdir=tmp_path, rundir=tmp_path) is False
+    assert post_install(tmp_path, tmp_path, SimpleNamespace()) is False
 
 
 def test_copy_config_file_fails(tmp_path):