Repository: onicagroup/runway

Issue:
update `runway.cfngin.lookups.handlers.rxref` to use the standardized query syntax
Subtask of #1184 

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/onicagroup/runway/pull/1302?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup) Report
> Merging [#1302](https://codecov.io/gh/onicagroup/runway/pull/1302?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup) (bb4bb09) into [master](https://codecov.io/gh/onicagroup/runway/commit/0dbd1469ade292fe1f13817c44fc182b2214e8c6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup) (0dbd146) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/onicagroup/runway/pull/1302/graphs/tree.svg?width=650&height=150&src=pr&token=Ku28I0RY80&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup)](https://codecov.io/gh/onicagroup/runway/pull/1302?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup)

```diff
@@            Coverage Diff             @@
##           master    #1302      +/-   ##
==========================================
+ Coverage   90.09%   90.10%   +0.01%     
==========================================
  Files         247      247              
  Lines       15375    15391      +16     
==========================================
+ Hits        13852    13868      +16     
  Misses       1523     1523              
```


| [Impacted Files](https://codecov.io/gh/onicagroup/runway/pull/1302?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup) | Coverage Δ | |
|---|---|---|
| [runway/lookups/handlers/cfn.py](https://codecov.io/gh/onicagroup/runway/pull/1302/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup#diff-cnVud2F5L2xvb2t1cHMvaGFuZGxlcnMvY2ZuLnB5) | `100.00% <ø> (ø)` | |
| [runway/cfngin/lookups/handlers/rxref.py](https://codecov.io/gh/onicagroup/runway/pull/1302/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup#diff-cnVud2F5L2Nmbmdpbi9sb29rdXBzL2hhbmRsZXJzL3J4cmVmLnB5) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/onicagroup/runway/pull/1302?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/onicagroup/runway/pull/1302?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup). Last update [0dbd146...bb4bb09](https://codecov.io/gh/onicagroup/runway/pull/1302?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup).


Patch:
diff --git a/docs/source/cfngin/lookups/rxref.rst b/docs/source/cfngin/lookups/rxref.rst
index 7ce61be0..6434582f 100644
--- a/docs/source/cfngin/lookups/rxref.rst
+++ b/docs/source/cfngin/lookups/rxref.rst
@@ -4,11 +4,11 @@
 rxref
 #####
 
-:Query Syntax: ``<stack-name>::<output-name>``
+:Query Syntax: ``<relative-stack-name>.<output-name>[::<arg>=<arg-val>, ...]``
 
 
-The rxref_ lookup type is very similar to the :ref:`xref lookup` type.
-Where the :ref:`xref lookup` type assumes you provided a fully qualified stack name, rxref_, like the :ref:`output lookup` expands and retrieves the output from the given Stack name within the current |namespace|, even if not defined in the CFNgin config you provided it.
+The rxref_ lookup type is very similar to the :ref:`CFNgin cfn lookup` lookup type.
+Where the :ref:`CFNgin cfn lookup` type assumes you provided a fully qualified stack name, rxref_, like the :ref:`output lookup` expands and retrieves the output from the given Stack name within the current |namespace|, even if not defined in the CFNgin config you provided it.
 
 Because there is no requirement to keep all stacks defined within the same CFNgin YAML config, you might need the ability to read outputs from other Stacks deployed by CFNgin into your same account under the same |namespace|.
 rxref_ gives you that ability.
@@ -17,6 +17,18 @@ This is useful if you want to break up very large configs into smaller groupings
 Also, unlike the :ref:`output lookup` type, rxref_ doesn't impact Stack requirements.
 
 
+.. versionchanged:: 2.7.0
+  The ``<relative-stack-name>::<output-name>`` syntax is deprecated to comply with Runway's lookup syntax.
+
+
+
+*********
+Arguments
+*********
+
+This Lookup supports all :ref:`Common Lookup Arguments`.
+
+
 
 *******
 Example
@@ -24,13 +36,15 @@ Example
 
 .. code-block:: yaml
 
-  # in example-us-east-1.env
-  namespace: MyNamespace
+  namespace: namespace
 
-  # in cfngin.yaml
-  ConfVariable: ${rxref my-stack::SomeOutput}
+  stacks:
+    - ...
+      variables:
+        ConfVariable0: ${rxref my-stack.SomeOutput}
+        # both of these lookups are functionally equivalent
+        ConfVariable1: ${cfn namespace-my-stack.SomeOutput}
 
-  # the above would effectively resolve to
-  ConfVariable: ${xref MyNamespace-my-stack::SomeOutput}
 
 Although possible, it is not recommended to use ``rxref`` for stacks defined within the same CFNgin YAML config.
+Doing so would require the use of :attr:`~cfngin.stack.required_by` or :attr:`~cfngin.stack.requires`.
diff --git a/runway/cfngin/lookups/handlers/rxref.py b/runway/cfngin/lookups/handlers/rxref.py
index 70aa8944..6ea4b544 100644
--- a/runway/cfngin/lookups/handlers/rxref.py
+++ b/runway/cfngin/lookups/handlers/rxref.py
@@ -2,28 +2,49 @@
 # pyright: reportIncompatibleMethodOverride=none
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Any
+import logging
+from typing import TYPE_CHECKING, Any, Dict, Tuple
 
 from typing_extensions import Final, Literal
 
 from ....lookups.handlers.base import LookupHandler
-from .output import deconstruct
+from ....lookups.handlers.cfn import CfnLookup
+from ....utils import DOC_SITE
+from .output import OutputQuery, deconstruct
 
 if TYPE_CHECKING:
     from ....context import CfnginContext
     from ...providers.aws.default import Provider
 
+LOGGER = logging.getLogger(__name__)
+
 
 class RxrefLookup(LookupHandler):
     """Rxref lookup."""
 
+    DEPRECATION_MSG = (
+        'lookup query syntax "<relative-stack-name>::<OutputName>" has been deprecated; '
+        "to learn how to use the new lookup query syntax visit "
+        f"{DOC_SITE}/page/cfngin/lookups/rxref.html"
+    )
     TYPE_NAME: Final[Literal["rxref"]] = "rxref"
     """Name that the Lookup is registered as."""
 
+    @classmethod
+    def legacy_parse(cls, value: str) -> Tuple[OutputQuery, Dict[str, str]]:
+        """Retain support for legacy lookup syntax.
+
+        Format of value:
+            <relative-stack-name>::<OutputName>
+
+        """
+        LOGGER.warning("${rxref %s}: %s", value, cls.DEPRECATION_MSG)
+        return deconstruct(value), {}
+
     @classmethod
     def handle(  # pylint: disable=arguments-differ
         cls, value: str, context: CfnginContext, provider: Provider, **_: Any
-    ) -> str:
+    ) -> Any:
         """Fetch an output from the designated stack in the current namespace.
 
         The ``output`` lookup supports fetching outputs from stacks created
@@ -33,16 +54,22 @@ class RxrefLookup(LookupHandler):
         :class:`runway.context.CfnginContext` to expand the fqn of the stack.
 
         Args:
-            value: Parameter(s) given to this lookup. `<stack_name>::<output_name>``
+            value: Parameter(s) given to this lookup. `"<relative-stack-name>.<OutputName>``
             context: Context instance.
             provider: Provider instance.
 
-        Example:
-            ::
-
-                conf_value: ${rxref relative-stack-name::SomeOutputName}
-
         """
-        decon = deconstruct(value)
-        stack_fqn = context.get_fqn(decon.stack_name)
-        return provider.get_output(stack_fqn, decon.output_name)
+        try:
+            raw_query, _args = cls.parse(value)
+            query = OutputQuery(*raw_query.split("."))
+            colon_split = value.split("::", 1)
+            raw_args = colon_split[1] if len(colon_split) > 1 else ""
+        except ValueError:
+            query, _args = cls.legacy_parse(value)
+            raw_args = ""
+        stack_fqn = context.get_fqn(query.stack_name)
+        return CfnLookup.handle(
+            f"{stack_fqn}.{query.output_name}" + (f"::{raw_args}" if raw_args else ""),
+            context=context,
+            provider=provider,
+        )
diff --git a/runway/lookups/handlers/cfn.py b/runway/lookups/handlers/cfn.py
index 910bae8e..61954a5f 100644
--- a/runway/lookups/handlers/cfn.py
+++ b/runway/lookups/handlers/cfn.py
@@ -5,6 +5,7 @@ When specifying the output name, be sure to use the *Logical ID* of
 the output; not the *Export.Name*.
 
 """
+# pyright: reportIncompatibleMethodOverride=none
 from __future__ import annotations
 
 import json


Test patch:
diff --git a/tests/unit/cfngin/lookups/handlers/test_rxref.py b/tests/unit/cfngin/lookups/handlers/test_rxref.py
index 4add562d..b58f1780 100644
--- a/tests/unit/cfngin/lookups/handlers/test_rxref.py
+++ b/tests/unit/cfngin/lookups/handlers/test_rxref.py
@@ -1,33 +1,62 @@
 """Tests for runway.cfngin.lookups.handlers.rxref."""
-# pyright: basic, reportUnknownArgumentType=none, reportUnknownVariableType=none
-import unittest
+# pylint: disable=no-self-use,protected-access
+from __future__ import annotations
 
-from mock import MagicMock
+from typing import TYPE_CHECKING
 
+import pytest
+from mock import Mock
+
+from runway._logging import LogLevels
 from runway.cfngin.lookups.handlers.rxref import RxrefLookup
-from runway.config import CfnginConfig
-from runway.context import CfnginContext
 
+if TYPE_CHECKING:
+    from pytest import LogCaptureFixture
+    from pytest_mock import MockerFixture
 
-class TestRxrefHandler(unittest.TestCase):
-    """Tests for runway.cfngin.lookups.handlers.rxref.RxrefLookup."""
+    from ....factories import MockCFNginContext
 
-    def setUp(self) -> None:
-        """Run before tests."""
-        self.provider = MagicMock()
-        self.context = CfnginContext(config=CfnginConfig.parse_obj({"namespace": "ns"}))
+MODULE = "runway.cfngin.lookups.handlers.rxref"
 
-    def test_rxref_handler(self) -> None:
-        """Test rxref handler."""
-        self.provider.get_output.return_value = "Test Output"
 
-        value = RxrefLookup.handle(
-            "fully-qualified-stack-name::SomeOutput",
-            provider=self.provider,
-            context=self.context,
+class TestRxrefLookup:
+    """Tests for runway.cfngin.lookups.handlers.rxref.RxrefLookup."""
+
+    @pytest.mark.parametrize(
+        "provided, expected",
+        [
+            ("stack-name::Output", "namespace-stack-name.Output"),
+            ("stack-name.Output", "namespace-stack-name.Output"),
+            (
+                "stack-name.Output::default=bar",
+                "namespace-stack-name.Output::default=bar",
+            ),
+        ],
+    )
+    def test_handle(
+        self,
+        cfngin_context: MockCFNginContext,
+        expected: str,
+        mocker: MockerFixture,
+        provided: str,
+    ) -> None:
+        """Test handle."""
+        cfngin_context.config.namespace = "namespace"
+        cfn = mocker.patch(f"{MODULE}.CfnLookup")
+        cfn.handle.return_value = "success"
+        provider = Mock(name="provider")
+        assert RxrefLookup.handle(provided, context=cfngin_context, provider=provider)
+        cfn.handle.assert_called_once_with(
+            expected, context=cfngin_context, provider=provider
         )
-        self.assertEqual(value, "Test Output")
 
-        args = self.provider.get_output.call_args
-        self.assertEqual(args[0][0], "ns-fully-qualified-stack-name")
-        self.assertEqual(args[0][1], "SomeOutput")
+    def test_legacy_parse(
+        self, caplog: LogCaptureFixture, mocker: MockerFixture
+    ) -> None:
+        """Test legacy_parse."""
+        query = "foo"
+        caplog.set_level(LogLevels.WARNING, MODULE)
+        deconstruct = mocker.patch(f"{MODULE}.deconstruct", return_value="success")
+        assert RxrefLookup.legacy_parse(query) == (deconstruct.return_value, {})
+        deconstruct.assert_called_once_with(query)
+        assert f"${{rxref {query}}}: {RxrefLookup.DEPRECATION_MSG}" in caplog.messages