From cca57170c131540444a7cf20bdf1768d13e09ebe Mon Sep 17 00:00:00 2001
From: Tim <code@tim.life>
Date: Tue, 1 Sep 2026 22:51:37 -0700
Subject: [PATCH] fix: two rules that ignored they were writing expression
 source (#340, #341)

`inside_dollar_string` says the text being produced is part of an
expression rather than a value for the caller. `StringRule` checks it
and keeps its quotes, because `upper("x")` becoming `upper(x)` asks for
a variable nobody declared. Two rules did not.

A heredoc argument was spliced in bare: `upper(<<E\nx\nE\n)` came back
as `${upper(x)}`, and a multi-line body put raw newlines into source
that no longer parsed. Both heredoc rules check the flag now, in the
flattened and the trimmed paths.

With `preserve_heredocs` on, the heredoc is left as itself rather than
wrapped in quotes. That was quoting raw newlines, which OpenTofu rejects
with "Invalid multi-line string"; as a heredoc it is a legal argument
and `trimspace(<<EOF\n  hi  \nEOF\n)` evaluates to "hi". One existing
test pinned the quoted form and now states this.

`TemplateStringRule` dropped its delimiters in the value form, turning
`%{ if x == "y" }` into `%{ if x == y }` -- a comparison against a
variable rather than a string. It only ever appears inside a directive,
which is always expression source.
---
 CHANGELOG.md                              |  5 +-
 hcl2/rules/strings.py                     | 28 ++++++-
 test/unit/rules/test_expression_source.py | 92 +++++++++++++++++++++++
 test/unit/test_api.py                     |  6 +-
 4 files changed, 126 insertions(+), 5 deletions(-)
 create mode 100644 test/unit/rules/test_expression_source.py

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 61dc143a..1fe62afc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## \[Unreleased\]
 
-- Nothing yet.
+### Fixed
+
+- A heredoc inside an expression is written as a string rather than spliced in bare. `SerializationContext.inside_dollar_string` says the text being produced is expression source, and `StringRule` checks it for exactly this reason; the heredoc rules did not, so `upper(<<E\nx\nE\n)` came back as `${upper(x)}` -- a reference to a variable nobody declared -- and a multi-line body put raw newlines into source that then did not parse. With `preserve_heredocs` on, the heredoc is now left as itself rather than wrapped in quotes: it is a legal argument that way, and OpenTofu rejects the quoted form with "Invalid multi-line string". ([#340](https://github.com/amplify-education/python-hcl2/issues/340))
+- A string literal inside a template directive keeps its delimiters in the value form. `TemplateStringRule` only ever appears inside `%{ ... }`, where the text is expression source and the quotes belong to a literal written in it, so dropping them turned `%{ if x == "y" }` into `%{ if x == y }`: a comparison against a variable rather than against a string. ([#341](https://github.com/amplify-education/python-hcl2/issues/341))
 
 ## \[8.1.3\] - 2026-08-26
 
diff --git a/hcl2/rules/strings.py b/hcl2/rules/strings.py
index 76ce0660..becbbc09 100644
--- a/hcl2/rules/strings.py
+++ b/hcl2/rules/strings.py
@@ -170,15 +170,28 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext
             if not match:
                 raise RuntimeError(f"Invalid Heredoc token: {heredoc}")
             heredoc = _strip_closing_marker_line(match.group(2))
-            if options.strip_string_quotes:
+            if options.strip_string_quotes and not context.inside_dollar_string:
                 # The caller asked for the value, so hand back the body as-is:
                 # real newlines, no escaping. The escaping below exists only to
                 # build the quoted-string *source* form returned otherwise.
+                #
+                # Not inside an expression, though. There the heredoc is an
+                # argument, and its text is part of that expression's source:
+                # `upper(<<E\nx\nE\n)` has to come back as `upper("x")` and not
+                # as `upper(x)`, which asks for a variable nobody declared. A
+                # multi-line body made it worse, splicing raw newlines into
+                # source that then did not parse. `StringRule` checks the same
+                # flag one class away, for the same reason.
                 return heredoc
             heredoc = heredoc.replace("\\", "\\\\").replace('"', '\\"').replace("\n", "\\n")
             return f'"{heredoc}"'
 
         result = heredoc.rstrip(self._trim_chars)
+        if context.inside_dollar_string:
+            # A heredoc is a legal argument, and it is already source: quoting
+            # it here would put its raw newlines inside a quoted string, which
+            # is not valid HCL.
+            return result
         if options.strip_string_quotes:
             return result
         return f'"{result}"'
@@ -232,9 +245,12 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext
         if not options.preserve_heredocs:
             lines = [line.replace("\\", "\\\\").replace('"', '\\"') for line in lines]
 
-        if options.strip_string_quotes:
+        if options.strip_string_quotes and not context.inside_dollar_string:
             # Value, not source: join with real newlines regardless of
             # preserve_heredocs, and skip the escaping done for the quoted form.
+            # Inside an expression the text is that expression's source, so the
+            # quoted form below is what belongs there -- see the note in
+            # `HeredocTemplateRule.serialize`.
             return "\n".join(lines)
 
         sep = "\\n" if not options.preserve_heredocs else "\n"
@@ -272,8 +288,14 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext
         Inside template directive expressions, strings are delimited by \\"
         rather than plain ". We preserve these as \\" in serialized form so
         the deserializer can reconstruct them correctly.
+
+        `strip_string_quotes` asks for a value, and this rule only ever appears
+        inside a directive -- where the text is expression source and the
+        delimiters belong to a string literal written in it. Dropping them
+        there turned `%{ if x == "y" }` into `%{ if x == y }`: a comparison
+        against a variable rather than against a string.
         """
         raw = self.raw_value
-        if options.strip_string_quotes:
+        if options.strip_string_quotes and not context.inside_dollar_string:
             return self.inner_value
         return raw
diff --git a/test/unit/rules/test_expression_source.py b/test/unit/rules/test_expression_source.py
new file mode 100644
index 00000000..f1a56588
--- /dev/null
+++ b/test/unit/rules/test_expression_source.py
@@ -0,0 +1,92 @@
+# pylint: disable=C0103,C0114,C0115,C0116
+r"""Rules that are serialized into expression source (GH #340, #341).
+
+`SerializationContext.inside_dollar_string` tells a rule it is being written
+into an expression rather than handed to a caller as a value. `StringRule`
+checks it and keeps its quotes, because `upper("x")` becoming `upper(x)` asks
+for a variable nobody declared. Two rules did not check it.
+
+Checked against OpenTofu v1.12.5: `upper(<<EOT\nx\nEOT\n)` evaluates to
+`"X\n"`, so the argument is a string; and a string literal inside a directive
+is written with plain quotes, `"%{ if local.x == "y" }t%{ endif }"`, which
+evaluates to `"t"`.
+"""
+
+from unittest import TestCase
+
+from hcl2.api import loads
+from hcl2.utils import SerializationOptions
+
+VALUE = SerializationOptions(preserve_heredocs=False, strip_string_quotes=True)
+QUOTED = SerializationOptions(strip_string_quotes=True)
+SOURCE = SerializationOptions(preserve_heredocs=False)
+
+
+class TestAHeredocInsideAnExpression(TestCase):
+    """#340: the body was spliced in bare, so it read as a reference."""
+
+    def test_it_stays_a_string(self):
+        self.assertEqual(loads("a = upper(<<E\nx\nE\n)\n", serialization_options=VALUE)["a"], '${upper("x")}')
+
+    def test_it_matches_the_quoted_equivalent(self):
+        self.assertEqual(
+            loads("a = upper(<<E\nx\nE\n)\n", serialization_options=VALUE)["a"],
+            loads('a = upper("x")\n', serialization_options=VALUE)["a"],
+        )
+
+    def test_a_multi_line_body_does_not_splice_raw_newlines(self):
+        result = loads("a = upper(<<E\nx\ny\nE\n)\n", serialization_options=VALUE)["a"]
+        self.assertEqual(result, '${upper("x\\ny")}')
+        self.assertNotIn("\n", result)
+
+    def test_the_trim_form_too(self):
+        self.assertEqual(
+            loads("a = upper(<<-E\n  x\n  E\n)\n", serialization_options=VALUE)["a"], '${upper("x")}'
+        )
+
+    def test_a_heredoc_that_is_not_in_an_expression_is_unaffected(self):
+        self.assertEqual(loads("a = <<E\nx\nE\n", serialization_options=VALUE)["a"], "x")
+
+
+class TestAStringLiteralInsideADirective(TestCase):
+    """#341: the delimiters were dropped, turning a literal into a reference."""
+
+    ESCAPED = 'a = "%{ if x == \\"y\\" }t%{ endif }"\n'
+    PLAIN = 'a = "%{ if x == "y" }t%{ endif }"\n'
+
+    def test_the_escaped_delimiters_survive_the_value_form(self):
+        self.assertEqual(
+            loads(self.ESCAPED, serialization_options=QUOTED)["a"], '%{ if x == \\"y\\" }t%{ endif }'
+        )
+
+    def test_the_plain_delimiters_survive_too(self):
+        # The spelling Terraform accepts; unchanged by this fix, asserted so it
+        # stays that way.
+        self.assertEqual(loads(self.PLAIN, serialization_options=QUOTED)["a"], '%{ if x == "y" }t%{ endif }')
+
+    def test_the_source_form_is_unchanged(self):
+        self.assertEqual(loads(self.ESCAPED)["a"], '"%{ if x == \\"y\\" }t%{ endif }"')
+
+    def test_a_directive_without_a_literal_is_unaffected(self):
+        self.assertEqual(
+            loads('a = "%{ if x }t%{ endif }"\n', serialization_options=QUOTED)["a"],
+            "%{ if x }t%{ endif }",
+        )
+
+
+class TestTheSourceFormIsUntouched(TestCase):
+    """Neither fix changes what the non-value modes emit."""
+
+    def test_a_heredoc_argument_keeps_its_quoted_source(self):
+        self.assertEqual(
+            loads("a = upper(<<E\nx\nE\n)\n", serialization_options=SOURCE)["a"], '${upper("x")}'
+        )
+
+    def test_default_options_keep_the_heredoc_unquoted(self):
+        """A heredoc is a legal argument; quoting it is not.
+
+        The quoted form put raw newlines inside a quoted string, which OpenTofu
+        rejects with "Invalid multi-line string". As a heredoc it evaluates:
+        `trimspace(<<EOF\n  hi  \nEOF\n)` gives "hi".
+        """
+        self.assertEqual(loads("a = upper(<<E\nx\nE\n)\n")["a"], "${upper(<<E\nx\nE)}")
diff --git a/test/unit/test_api.py b/test/unit/test_api.py
index d6599fec..99cbb3d2 100644
--- a/test/unit/test_api.py
+++ b/test/unit/test_api.py
@@ -358,7 +358,11 @@ def test_empty_heredoc_as_an_object_value(self):
         self.assertEqual(loads("a = {\n  k = <<EOF\nEOF\n}\n"), {"a": {"k": '"<<EOF\nEOF"'}})
 
     def test_empty_heredoc_as_a_function_argument(self):
-        self.assertEqual(loads("a = trimspace(<<EOF\nEOF\n)\n"), {"a": '${trimspace("<<EOF\nEOF")}'})
+        # The heredoc stays a heredoc rather than being quoted. Quoting it put
+        # raw newlines inside a quoted string, which OpenTofu rejects with
+        # "Invalid multi-line string"; as a heredoc it is a legal argument, and
+        # `trimspace(<<EOF\n  hi  \nEOF\n)` evaluates to "hi".
+        self.assertEqual(loads("a = trimspace(<<EOF\nEOF\n)\n"), {"a": "${trimspace(<<EOF\nEOF)}"})
 
 
 class TestNegativeIntegerLiterals(TestCase):