From 5755fbfc876b8dea336badb384c0568a76d9536b Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 31 Aug 2026 14:02:09 -0700 Subject: [PATCH 01/16] fix: return heredoc bodies that match what Terraform evaluates Three things about a flattened heredoc body differed from the value Terraform and OpenTofu evaluate the same source to. Every expectation added here was produced by running the source through OpenTofu v1.12.5 rather than read off the spec. - The newline terminating the last content line was dropped, so `<HCL2 deserialization and reconstruction.** | -| `preserve_heredocs` | `bool` | `True` | Keep heredocs in their original form, markers included. When `False`, a heredoc becomes a quoted string with its newlines escaped (`'"a\nb"'`); combine with `strip_string_quotes` to get the body as a plain multi-line value instead. | +| `preserve_heredocs` | `bool` | `True` | Keep heredocs in their original form, markers included. When `False`, a heredoc becomes a quoted string with its newlines escaped (`'"a\nb\n"'`); combine with `strip_string_quotes` to get the body as a plain multi-line value instead. Either way the body keeps the newline that terminates its last line, as Terraform's does. | | `force_operation_parentheses` | `bool` | `False` | Force parentheses around all operations | | `preserve_scientific_notation` | `bool` | `True` | Keep scientific notation as-is | | `strip_string_quotes` | `bool` | `False` | Yield string *values* rather than source text: remove surrounding quotes (e.g. `"hello"` instead of `'"hello"'`) and resolve escape sequences (`"a\nb"` becomes a real newline). String literals inside expressions keep their quotes, so `upper("x")` stays `'${upper("x")}'`. **Breaks JSON->HCL2 deserialization and reconstruction.** | @@ -127,7 +127,7 @@ text = dumps(data, deserializer_options=DeserializerOptions( | Field | Type | Default | Description | |---|---|---|---| | `heredocs_to_strings` | `bool` | `False` | Convert heredocs to plain strings | -| `strings_to_heredocs` | `bool` | `False` | Convert strings with `\n` to heredocs | +| `strings_to_heredocs` | `bool` | `False` | Convert newline-terminated strings to heredocs. A value that does not end in a newline is left as a quoted string, because a heredoc body always ends in one and writing it as a heredoc would change the value. | | `object_elements_colon` | `bool` | `False` | Use `:` instead of `=` in object elements | | `object_elements_trailing_comma` | `bool` | `True` | Add trailing commas in object elements | diff --git a/docs/06_migrating_to_v8.md b/docs/06_migrating_to_v8.md index 0c4703bf..467c1d0d 100644 --- a/docs/06_migrating_to_v8.md +++ b/docs/06_migrating_to_v8.md @@ -213,17 +213,18 @@ This restores the v7 dict shape but disables round-trip support and comment pres ```python hcl2.loads('x = <<-EOT\n line1\n line2\n EOT\n', serialization_options=V7_COMPAT) -# {'x': 'line1\nline2'} +# {'x': 'line1\nline2\n'} ``` -Note that `preserve_heredocs=False` on its own — without `strip_string_quotes` — produces the quoted *source* form with escaped newlines (`'"line1\\nline2"'`), because that output is meant to be reconstructable. +Note that `preserve_heredocs=False` on its own — without `strip_string_quotes` — produces the quoted *source* form with escaped newlines (`'"line1\\nline2\\n"'`), because that output is meant to be reconstructable. -Two details of heredoc values are easy to trip over, and both match how HCL itself behaves: +Three details of heredoc values are easy to trip over, and all three match how HCL itself behaves: +- **The body ends with a newline.** Every content line is terminated by its own newline, the last one included, so `< str: + r"""Resolve the escapes a heredoc body carries literally: \n, \" and \\. + + Single-pass, so an escaped backslash cannot combine with the character + after it. + """ + return re.sub( + r'\\(n|"|\\)', + lambda m: "\n" if m.group(1) == "n" else m.group(1), + inner, + ) + + @dataclass class DeserializerOptions: """Options controlling how Python dicts are deserialized into LarkElement trees.""" @@ -71,8 +84,10 @@ class DeserializerOptions: # Convert heredoc values (< LarkRule: return self._deserialize_heredoc(value[1:-1], False) if self.options.strings_to_heredocs: - inner = value[1:-1] - if "\\n" in inner: - return self._deserialize_string_as_heredoc(inner) + content = _unescape_heredoc_body(value[1:-1]) + # A heredoc's closing marker sits on a line of its own, so + # its body always ends with a newline. A value that does not + # cannot be written as one without gaining that character, + # so it stays a quoted string. + if content.endswith("\n"): + return self._deserialize_string_as_heredoc(content) return self._deserialize_string(value) @@ -259,15 +278,9 @@ def _deserialize_heredoc( return HeredocTrimTemplateRule([HEREDOC_TRIM_TEMPLATE(value)]) return HeredocTemplateRule([HEREDOC_TEMPLATE(value)]) - def _deserialize_string_as_heredoc(self, inner: str) -> HeredocTemplateRule: - """Convert a quoted string with escaped newlines back into a heredoc.""" - # Single-pass unescape: \\n → \n, \\" → ", \\\\ → \ - content = re.sub( - r'\\(n|"|\\)', - lambda m: "\n" if m.group(1) == "n" else m.group(1), - inner, - ) - heredoc = f"< HeredocTemplateRule: + """Wrap an unescaped body, already newline-terminated, in heredoc syntax.""" + heredoc = f"< ExprTermRule: diff --git a/hcl2/rules/strings.py b/hcl2/rules/strings.py index 76ce0660..1c37c1d0 100644 --- a/hcl2/rules/strings.py +++ b/hcl2/rules/strings.py @@ -27,23 +27,25 @@ ) -def _strip_closing_marker_line(text: str) -> str: - r"""Drop the closing marker line's indentation and the one newline before it. +def _strip_closing_marker_indent(text: str) -> str: + r"""Drop the whitespace indenting the closing marker on its own line. A heredoc body always ends ``...\n``, where ```` is the - whitespace preceding the closing marker on its own line. The spec allows - "an arbitrary number of spaces preceding it", and neither that indentation - nor the newline separating it from the last content line is part of the - value. The newline may be ``\r\n``, since heredocs parse in CRLF files. - - Everything else is: additional blank lines, and trailing spaces on a - content line. The latter are safe because a content line always ends with - its own newline, so the indentation match never reaches them. This replaces - a blanket ``rstrip("\n\t ")``, which could not tell the two apart and - discarded both. + whitespace preceding the closing marker. The spec allows "an arbitrary + number of spaces preceding it", and that indentation is not part of the + value. + + The newline before it *is*. The spec ends the template where the delimiter + "subsequently appears again on a line of its own", so every content line, + the last one included, is terminated by its own newline: ``< str: def serialize(self, options=SerializationOptions(), context=SerializationContext()) -> Any: """Serialize the trim heredoc, stripping common leading whitespace.""" # See https://github.com/hashicorp/hcl2/blob/master/hcl/hclsyntax/spec.md#template-expressions - # This is a special version of heredocs that are declared with "<<-" - # This will calculate the minimum number of leading spaces in each line of a heredoc - # and then remove that number of spaces from each line - + # This is a special version of heredocs that are declared with "<<-", + # whose body is dedented by the smallest indent any of its lines carries. heredoc = self.heredoc.serialize(options, context) if not options.preserve_heredocs: match = HEREDOC_TRIM_PATTERN.match(heredoc) if not match: raise RuntimeError(f"Invalid Heredoc token: {heredoc}") - heredoc = match.group(2) + lines = self._dedent(_strip_closing_marker_indent(match.group(2))) + if options.strip_string_quotes: + # The caller asked for the value: real newlines, no escaping. + return "\n".join(lines) + escaped = [line.replace("\\", "\\\\").replace('"', '\\"') for line in lines] + return '"' + "\\n".join(escaped) + '"' + + result = heredoc.rstrip(self._trim_chars) + if options.strip_string_quotes: + return result + return f'"{result}"' - heredoc = _strip_closing_marker_line(heredoc) - lines = heredoc.split("\n") + @staticmethod + def _dedent(body: str) -> List[str]: + """Split *body* into lines and remove the common leading whitespace.""" + lines = body.split("\n") - # calculate the min number of leading spaces in each line + # The margin is the smallest indent any content line carries. + # # The spec measures "any literal string at the start of each line", so a # blank line offers no measurement. Counting it as zero would drag the - # minimum down and cancel the dedent for every other line -- which only - # became reachable once blank lines stopped being stripped above. - min_spaces = sys.maxsize + # margin down and cancel the dedent for every other line. + # + # It also says "spaces", but the reference implementation does not read + # that as narrowly: OpenTofu dedents a tab-indented `<<-` heredoc by one + # tab per level. Measuring whitespace characters rather than spaces + # alone matches it, and is identical to counting spaces on the + # space-indented input that reading the letter of the spec would cover. + margin = sys.maxsize for line in lines: if not line.strip(): continue - leading_spaces = len(line) - len(line.lstrip(" ")) - min_spaces = min(min_spaces, leading_spaces) - if min_spaces == sys.maxsize: - min_spaces = 0 - - # trim off that number of leading spaces from each line - lines = [line[min_spaces:] for line in lines] - - if not options.preserve_heredocs: - lines = [line.replace("\\", "\\\\").replace('"', '\\"') for line in lines] - - if options.strip_string_quotes: - # Value, not source: join with real newlines regardless of - # preserve_heredocs, and skip the escaping done for the quoted form. - return "\n".join(lines) - - sep = "\\n" if not options.preserve_heredocs else "\n" - inner = sep.join(lines) - return '"' + inner + '"' + margin = min(margin, len(line) - len(line.lstrip())) + if margin == sys.maxsize: + margin = 0 + + # A line that offered no measurement is left exactly as written -- + # OpenTofu keeps a six-space line inside a four-space heredoc at six + # spaces rather than two. + return [line[margin:] if line.strip() else line for line in lines] class TemplateStringRule(LarkRule): diff --git a/test/integration/specialized/heredocs_flattened.json b/test/integration/specialized/heredocs_flattened.json index 95fb4e55..43c6fd2c 100644 --- a/test/integration/specialized/heredocs_flattened.json +++ b/test/integration/specialized/heredocs_flattened.json @@ -1,16 +1,16 @@ { "locals": [ { - "simple": "\"hello world\"", - "multiline": "\"line1\\nline2\\nline3\"", - "with_quotes": "\"say \\\"hello\\\"\"", - "with_backslashes": "\"path\\\\to\\\\file\"", - "trimmed": "\"indented1\\nindented2\"", - "trimmed_mixed": "\"line1\\n line2\\nline3\"", - "json_content": "\"{\\\"key\\\": \\\"value\\\"}\"", + "simple": "\"hello world\\n\"", + "multiline": "\"line1\\nline2\\nline3\\n\"", + "with_quotes": "\"say \\\"hello\\\"\\n\"", + "with_backslashes": "\"path\\\\to\\\\file\\n\"", + "trimmed": "\"indented1\\nindented2\\n\"", + "trimmed_mixed": "\"line1\\n line2\\nline3\\n\"", + "json_content": "\"{\\\"key\\\": \\\"value\\\"}\\n\"", "empty": "\"\"", "empty_trimmed": "\"\"", - "blank_line_only": "\"\"", + "blank_line_only": "\"\\n\"", "after_empty": "\"still parsed\"", "__is_block__": true } diff --git a/test/integration/specialized/heredocs_restored.tf b/test/integration/specialized/heredocs_restored.tf index 05832d52..a7bad307 100644 --- a/test/integration/specialized/heredocs_restored.tf +++ b/test/integration/specialized/heredocs_restored.tf @@ -1,12 +1,18 @@ locals { - simple = "hello world" + simple = < Date: Mon, 31 Aug 2026 19:53:09 -0700 Subject: [PATCH 02/16] test: add a script that re-derives the heredoc expectations from Terraform `test_heredoc_matches_terraform.py` asserts values that came from running each source through OpenTofu rather than from this library or from the spec. That provenance was a docstring: a reader had to take it on trust, and nothing re-checked it if the reference implementation moved. `bin/heredoc_ground_truth` reads the `CASES` table out of the test module, evaluates every source with `tofu console` (or `terraform console`), and reports any disagreement, exiting non-zero. `--print` emits the evaluated table as Python for pasting. It is not wired into the test run on purpose. The suite must pass without a Terraform binary present, and these values move about as often as the HCL spec does -- this is an audit tool for a reviewer who would rather check than trust, not a gate. Both paths are exercised: all 16 cases agree with OpenTofu v1.12.5, and feeding it the pre-fix value for a case makes it report the mismatch and exit 1. --- bin/heredoc_ground_truth | 108 ++++++++++++++++++++ test/unit/test_heredoc_matches_terraform.py | 7 +- 2 files changed, 114 insertions(+), 1 deletion(-) create mode 100755 bin/heredoc_ground_truth diff --git a/bin/heredoc_ground_truth b/bin/heredoc_ground_truth new file mode 100755 index 00000000..6003ee13 --- /dev/null +++ b/bin/heredoc_ground_truth @@ -0,0 +1,108 @@ +#!/usr/bin/env python +"""Check the heredoc expectations in the test suite against Terraform itself. + +`test/unit/test_heredoc_matches_terraform.py` asserts what a heredoc body +evaluates to. Those values did not come from this library or from reading the +spec -- each one was produced by handing the same source to OpenTofu. That +provenance is a docstring, which a reader has to take on trust and which +nothing re-checks if the reference implementation ever moves. + +This script re-derives them. It reads the `CASES` table out of that test module, +evaluates every source with `tofu console` (or `terraform console`), and +compares. It is not part of the test run: the suite must not depend on a +Terraform binary, and these values change about as often as the HCL spec does. + +Usage: + bin/heredoc_ground_truth # verify; non-zero exit on any mismatch + bin/heredoc_ground_truth --print # print the table as Python, to paste + +Requires `tofu` or `terraform` on PATH. +""" + +import argparse +import json +import os +import shutil +import subprocess +import sys +import tempfile + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")) + +from test.unit.test_heredoc_matches_terraform import CASES # noqa: E402 + +BINARIES = ("tofu", "terraform") + + +def find_binary(): + """Return the first Terraform-compatible binary on PATH, or None.""" + for name in BINARIES: + path = shutil.which(name) + if path: + return path + return None + + +def evaluate(binary, source): + """Return the value `binary` evaluates the given heredoc expression to. + + The source is written as a local rather than an output so that nothing has + to be applied, and `jsonencode` is what carries the exact string back -- + the console's own rendering escapes newlines for display. + """ + with tempfile.TemporaryDirectory() as directory: + # newline="" so a case testing CRLF is written with the bytes it names. + with open(os.path.join(directory, "main.tf"), "w", encoding="utf-8", newline="") as handle: + handle.write("locals {\n x = %s\n}\n" % source) + result = subprocess.run( + [binary, "console"], + cwd=directory, + input="jsonencode(local.x)\n", + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + raise RuntimeError(result.stderr.strip()) + return json.loads(json.loads(result.stdout.strip().splitlines()[-1])) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--print", + dest="print_table", + action="store_true", + help="print the evaluated table as Python instead of verifying", + ) + args = parser.parse_args() + + binary = find_binary() + if binary is None: + print("neither `tofu` nor `terraform` is on PATH", file=sys.stderr) + return 2 + + print("using %s\n" % binary, file=sys.stderr) + mismatches = 0 + for source, expected in CASES: + actual = evaluate(binary, source) + if args.print_table: + print(" (%r, %r)," % (source, actual)) + continue + if actual == expected: + print("ok %r" % source) + else: + mismatches += 1 + print("BAD %r\n expected %r\n %s says %r" % (source, expected, binary, actual)) + + if args.print_table: + return 0 + + # stdout, so it lands after the per-case lines rather than ahead of them + # when the output is piped. + print("\n%d of %d cases disagree" % (mismatches, len(CASES))) + return 1 if mismatches else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/test/unit/test_heredoc_matches_terraform.py b/test/unit/test_heredoc_matches_terraform.py index 6fab9649..6462e768 100644 --- a/test/unit/test_heredoc_matches_terraform.py +++ b/test/unit/test_heredoc_matches_terraform.py @@ -3,7 +3,12 @@ Every expectation in this file was produced by evaluating the same source with OpenTofu v1.12.5 (`tofu console`, `jsonencode` of the resulting local), not by -reading the spec. Three things used to differ: +reading the spec. `bin/heredoc_ground_truth` re-derives the `CASES` table below +from whatever Terraform-compatible binary is on PATH, so that provenance can be +checked rather than taken on trust. It is deliberately not part of the test run: +the suite must not need a Terraform binary to pass. + +Three things used to differ: 1. The newline before the closing marker was dropped, so `< Date: Tue, 1 Sep 2026 16:05:10 -0700 Subject: [PATCH 03/16] fix: escape carriage returns in the flattened heredoc form `preserve_heredocs=False` without `strip_string_quotes` returns the body as quoted-string source -- the text a parser has to read back. Newlines were escaped for that; carriage returns were not. A heredoc from a CRLF file flattened to `"x\ny\n"`, which OpenTofu rejects with "No closing marker was found for the string", so the form documented as reconstructable was not. `\r` is an escape both this package's `process_escape_sequences` and OpenTofu resolve back to a carriage return, so the value survives the round trip unchanged. The trimmed form had the same gap and gets the same treatment. The value form keeps handing back real characters. Two existing CRLF tests asserted the raw-carriage-return output; they now assert the escaped source and say why. --- CHANGELOG.md | 1 + hcl2/rules/strings.py | 9 ++++++-- test/unit/test_crlf.py | 48 +++++++++++++++++++++++++++++++++++++----- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f430960..7c2e8f46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Flattened heredoc bodies match the values Terraform and OpenTofu evaluate the same source to. Three things differed, all checked against OpenTofu v1.12.5 rather than read off the spec: the newline terminating the last content line was dropped (`<b"` with "No closing marker was found for the string". + heredoc = ( + heredoc.replace("\\", "\\\\").replace('"', '\\"').replace("\r", "\\r").replace("\n", "\\n") + ) return f'"{heredoc}"' result = heredoc.rstrip(self._trim_chars) @@ -211,7 +216,7 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext if options.strip_string_quotes: # The caller asked for the value: real newlines, no escaping. return "\n".join(lines) - escaped = [line.replace("\\", "\\\\").replace('"', '\\"') for line in lines] + escaped = [line.replace("\\", "\\\\").replace('"', '\\"').replace("\r", "\\r") for line in lines] return '"' + "\\n".join(escaped) + '"' result = heredoc.rstrip(self._trim_chars) diff --git a/test/unit/test_crlf.py b/test/unit/test_crlf.py index a009cae1..4c72094a 100644 --- a/test/unit/test_crlf.py +++ b/test/unit/test_crlf.py @@ -105,19 +105,21 @@ def test_closing_marker_leaves_no_trailing_carriage_return(self): self.assertTrue(result["a"].endswith('EOF"'), result["a"]) def test_flattening_a_crlf_heredoc_does_not_raise(self): - """The heredoc patterns in utils.py run on an already-parsed token. + r"""The heredoc patterns in utils.py run on an already-parsed token. - Every body line keeps its own `\\r\\n`, the last one included: OpenTofu - evaluates this source to `"x\\r\\ny\\r\\n"`. + Every body line keeps its own `\r\n`, the last one included: OpenTofu + evaluates this source to `"x\r\ny\r\n"`. Both characters are written + escaped, because this form is quoted-string *source* -- see + `TestFlattenedCrlfHeredocsStayValidHcl`. """ options = SerializationOptions(preserve_heredocs=False) result = loads("a = <b"` with "No + closing marker was found for the string", while `"a\rb"` evaluates to a + carriage return, which is what the heredoc body actually held. + + The value form is unaffected: it hands back the body, so its newlines and + carriage returns stay real characters. + """ + + FLAT = SerializationOptions(preserve_heredocs=False) + VALUE = SerializationOptions(preserve_heredocs=False, strip_string_quotes=True) + + def test_heredoc_source_form_escapes_carriage_returns(self): + source = loads("a = < Date: Tue, 1 Sep 2026 18:24:58 -0700 Subject: [PATCH 04/16] fix: resolve \r when writing a heredoc body Escaping carriage returns in the flattened form left the writer half a step behind: `_unescape_heredoc_body` resolved `\n`, `\"` and `\\` but not `\r`, so a heredoc read out of a CRLF file and written back came out holding a literal backslash and an `r`. A heredoc interprets no escape -- its body is the characters themselves -- so that is a different value, and OpenTofu reads it as one. The two halves have to be inverses. Flatten writes `\r` because a quoted string cannot hold a raw carriage return; the writer therefore has to resolve it, exactly as it already resolved `\n` for the same reason. Each half was covered on its own -- flattening a CRLF heredoc, restoring an LF string -- which is why the combination could break with the suite green. The new tests run the whole path: CRLF source, flatten, write, read the value back, against the string OpenTofu evaluates the original file to. Escapes other than these four are still not resolved when writing a heredoc, which is a separate pre-existing defect (#329). --- CHANGELOG.md | 2 +- hcl2/deserializer.py | 14 +++++++++--- test/unit/test_crlf.py | 48 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c2e8f46..8309f67c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Flattened heredoc bodies match the values Terraform and OpenTofu evaluate the same source to. Three things differed, all checked against OpenTofu v1.12.5 rather than read off the spec: the newline terminating the last content line was dropped (`< str: - r"""Resolve the escapes a heredoc body carries literally: \n, \" and \\. + r"""Resolve the escapes a heredoc body carries literally: \n, \r, \" and \\. + + A heredoc interprets no backslash sequence -- its body is the characters + themselves -- so anything the quoted form spelled as an escape has to be + resolved before it is written into one. `\r` is here because the flattened + form escapes carriage returns: without it, a heredoc read out of a CRLF + file and written back came out holding a literal backslash and an `r`. Single-pass, so an escaped backslash cannot combine with the character after it. """ return re.sub( - r'\\(n|"|\\)', - lambda m: "\n" if m.group(1) == "n" else m.group(1), + r'\\(n|r|"|\\)', + lambda m: _HEREDOC_BODY_ESCAPES.get(m.group(1), m.group(1)), inner, ) diff --git a/test/unit/test_crlf.py b/test/unit/test_crlf.py index 4c72094a..3737d698 100644 --- a/test/unit/test_crlf.py +++ b/test/unit/test_crlf.py @@ -15,7 +15,8 @@ from unittest import TestCase -from hcl2.api import loads, parses_to_tree, reconstruct, transform +from hcl2.api import dumps, loads, parses_to_tree, reconstruct, transform +from hcl2.deserializer import DeserializerOptions from hcl2.utils import SerializationOptions CR = "\r" @@ -170,3 +171,48 @@ def test_a_lone_cr_inside_a_line_is_escaped_too(self): # Not a line ending: a carriage return the body carries mid-line. source = loads("a = < str: + flattened = loads(source, serialization_options=self.FLAT) + return dumps(flattened, deserializer_options=self.HEREDOCS) + + def _round_trip(self, source: str) -> str: + restored = self._restore(source) + return loads(restored, serialization_options=self.VALUE)["a"] + + def test_the_value_is_unchanged(self): + self.assertEqual(self._round_trip("a = < Date: Tue, 1 Sep 2026 18:42:21 -0700 Subject: [PATCH 05/16] fix: choose a heredoc delimiter the body cannot close (#330) `strings_to_heredocs` wrote `< str: + """Return a delimiter the body does not close on its own. + + `EOF` unless the body holds a line that would end the heredoc there, in + which case a numbered variant is used. The word matters: a log excerpt, a + shell script or an embedded config is exactly the sort of value people put + in a heredoc, and `EOF` is exactly the word such a payload tends to + contain. Writing one blindly produced a file that no longer parsed. + """ + occupied = set() + for line in content.split("\n"): + match = _CLOSING_MARKER_LINE.fullmatch(line) + if match is not None: + occupied.add(match.group(1)) + + if "EOF" not in occupied: + return "EOF" + + suffix = 1 + while f"EOF_{suffix}" in occupied: + suffix += 1 + return f"EOF_{suffix}" + def _unescape_heredoc_body(inner: str) -> str: r"""Resolve the escapes a heredoc body carries literally: \n, \r, \" and \\. @@ -288,7 +320,8 @@ def _deserialize_heredoc( def _deserialize_string_as_heredoc(self, content: str) -> HeredocTemplateRule: """Wrap an unescaped body, already newline-terminated, in heredoc syntax.""" - heredoc = f"< ExprTermRule: diff --git a/test/unit/test_heredoc_matches_terraform.py b/test/unit/test_heredoc_matches_terraform.py index 6462e768..b920afd5 100644 --- a/test/unit/test_heredoc_matches_terraform.py +++ b/test/unit/test_heredoc_matches_terraform.py @@ -26,7 +26,8 @@ from unittest import TestCase -from hcl2.api import loads +from hcl2.api import dumps, loads +from hcl2.deserializer import DeserializerOptions from hcl2.utils import SerializationOptions _VALUE = SerializationOptions(preserve_heredocs=False, strip_string_quotes=True) @@ -121,3 +122,49 @@ def test_the_two_forms_describe_the_same_string(self): # Re-read the quoted form as HCL and it yields the value back. reread = loads(f"x = {quoted}\n", serialization_options=_VALUE)["x"] self.assertEqual(reread, value) + + +class TestWrittenDelimiterCannotCloseEarly(TestCase): + r"""The delimiter is chosen against the body, not assumed to be `EOF`. + + A log excerpt, a shell script, an embedded config -- the payloads people + put in heredocs -- are exactly the values that contain the word `EOF`. + Writing `< str: + return dumps({"x": value}, deserializer_options=self.HEREDOCS) + + def _round_trip(self, value: str) -> str: + return loads(self._write(value), serialization_options=self.VALUE)["x"] + + def test_an_ordinary_body_still_uses_eof(self): + self.assertEqual(self._write(r'"plain\n"'), "x = < Date: Tue, 1 Sep 2026 18:43:05 -0700 Subject: [PATCH 06/16] docs: the empty string is the exception to the newline rule `strings_to_heredocs` leaves a value that does not end in a newline quoted, and the comments said a heredoc body always ends in one. An empty heredoc does not: `< LarkRule: if self.options.strings_to_heredocs: content = _unescape_heredoc_body(value[1:-1]) # A heredoc's closing marker sits on a line of its own, so - # its body always ends with a newline. A value that does not - # cannot be written as one without gaining that character, - # so it stays a quoted string. + # any body with content in it ends with a newline. A value + # that does not cannot be written as one without gaining + # that character, so it stays a quoted string. + # + # The empty string is the one value this excludes that a + # heredoc could in fact express -- `< Date: Tue, 1 Sep 2026 19:00:51 -0700 Subject: [PATCH 07/16] fix: strip a closing marker indented with any whitespace The dedent measures whitespace rather than spaces and tabs, because that is what OpenTofu does -- it dedents a body indented with a non-breaking space, a vertical tab, a form feed or an ideographic space exactly as it dedents a space-indented one. The closing marker's own indentation was still stripped as `[ \t]*`, so those bodies came back with the marker's indent character appended to the value: `'a\nb\n\xa0'` where OpenTofu evaluates `'a\nb\n'`. It is now any whitespace but a newline, which is the same rule the dedent uses. Trailing spaces on a content line still survive, for the reason they always did: such a line ends with its own newline, and the match cannot cross one. The four cases are in `CASES`, so `bin/heredoc_ground_truth` re-derives them from Terraform along with the rest rather than trusting this reading of the spec. All 20 agree. --- CHANGELOG.md | 1 + hcl2/rules/strings.py | 9 +++++++-- test/unit/test_heredoc_matches_terraform.py | 10 ++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11780ae4..f2e658e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Flattened heredoc bodies match the values Terraform and OpenTofu evaluate the same source to. Three things differed, all checked against OpenTofu v1.12.5 rather than read off the spec: the newline terminating the last content line was dropped (`< str: is ``"line\n"``, which is what Terraform and OpenTofu evaluate it to. Trailing spaces on a content line survive too, because such a line always - ends with its own newline, so the match above never reaches them. This + ends with its own newline, and the match below cannot cross one. This replaced a blanket ``rstrip("\n\t ")``, which could tell none of these apart and discarded all of them. + + The indentation is any whitespace but a newline, not spaces and tabs + alone: a marker indented with a non-breaking space, a vertical tab, a form + feed or an ideographic space is indented as far as OpenTofu is concerned, + and leaving those characters in place appended them to the value. """ - return re.sub(r"[ \t]*\Z", "", text) + return re.sub(r"[^\S\n]*\Z", "", text) class InterpolationRule(LarkRule): diff --git a/test/unit/test_heredoc_matches_terraform.py b/test/unit/test_heredoc_matches_terraform.py index b920afd5..901ea299 100644 --- a/test/unit/test_heredoc_matches_terraform.py +++ b/test/unit/test_heredoc_matches_terraform.py @@ -50,6 +50,16 @@ ("<<-EOT\nEOT", ""), ("< Date: Tue, 1 Sep 2026 19:16:08 -0700 Subject: [PATCH 08/16] fix: a heredoc body cannot hold every value the quoted form can Two cases where writing one produced a file Terraform cannot read. A lone carriage return is not expressible. A heredoc body is read literally, so a `\r` may only appear where one ends a line: OpenTofu rejects `< str: @@ -98,6 +101,19 @@ def _heredoc_delimiter(content: str) -> str: return f"EOF_{suffix}" +def _expressible_as_heredoc(content: str) -> bool: + """Whether *content* can be a heredoc body without changing. + + A heredoc body is read literally, so it can hold a carriage return only + where one ends a line. A lone `\r` makes the file unreadable rather than + merely different: OpenTofu rejects `< str: r"""Resolve the escapes a heredoc body carries literally: \n, \r, \" and \\. @@ -244,7 +260,7 @@ def _deserialize_text(self, value: Any) -> LarkRule: # heredoc could in fact express -- `< Date: Tue, 1 Sep 2026 21:32:42 -0700 Subject: [PATCH 09/16] fix: escapes belong to the span they are written in (#329, #336, #339) Three defects with one shape: a quoted string or a heredoc body is not one run of literal characters, and every path that rewrote such text treated it as one. `$${` and `%%{` are HCL's escapes for a literal `${` and `%{`, exactly as `\"` is for a quote. The value form returned them doubled, so the value differed from the one Terraform reads -- in the single mode whose whole purpose is to give the value. (#336) `strings_to_heredocs` resolved four escapes where the reader resolves nine, because it spelled its own alphabet. A tab written `\t` reached the heredoc body as a backslash and a `t`, and `\uNNNN` fared the same. It now calls `process_escape_sequences`, which is the package's one implementation of that alphabet. (#329) Both the escaping and the unescaping ran over interpolation text. That belongs to an expression, not to this string: OpenTofu reads `"${upper("a")}"` as `A` and rejects `"${upper(\"a\")}"` outright, so escaping through an interpolation produced source the reference implementation will not parse, while resolving through one closed a nested string literal early and changed what the expression said. (#339) `hcl2/template.py` recovers the spans from text in one left-to-right pass. It has to be one pass: a splitter run before escapes are resolved sees the `${` inside `$${` and opens a span that is not there. The grammar already separates these for a quoted string, which is why `StringRule._serialize_part_as_value` could do the right thing by asking each part for its terminal; a heredoc body arrives as one token, so the distinction is recovered rather than given. One existing test asserted the doubled sigil in the value form. It was pinning #336, and now states what OpenTofu evaluates. --- CHANGELOG.md | 3 + hcl2/deserializer.py | 46 +++++----- hcl2/rules/strings.py | 55 +++++++----- hcl2/template.py | 115 +++++++++++++++++++++++++ test/unit/test_api.py | 17 +++- test/unit/test_template_spans.py | 143 +++++++++++++++++++++++++++++++ 6 files changed, 331 insertions(+), 48 deletions(-) create mode 100644 hcl2/template.py create mode 100644 test/unit/test_template_spans.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 256e2d94..eb24f7a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- `$${` and `%%{` resolve to `${` and `%{` in the value form, in both quoted strings and heredocs. They are HCL's escapes for a literal sigil, exactly as `\"` is for a quote, and OpenTofu evaluates `"$${esc}"` to the six characters `${esc}`; returning them doubled made the value differ from the one Terraform reads, in the one mode that promises the value. ([#336](https://github.com/amplify-education/python-hcl2/issues/336)) +- `strings_to_heredocs` resolves every escape the reader does. It knew `\n`, `\r`, `\"` and `\\`, so `"a\tb\n"` was written into the body as a backslash and a `t` -- two characters where Terraform reads one tab -- and `\uNNNN` fared the same. It now uses `process_escape_sequences`, the package's one implementation of that alphabet. ([#329](https://github.com/amplify-education/python-hcl2/issues/329)) +- Escapes are no longer added or resolved inside `${...}`. The text there is expression source, where a nested `"..."` is a string literal of its own: OpenTofu reads `"${upper("a")}"` as `A` and rejects `"${upper(\"a\")}"` outright, so escaping through an interpolation produced source the reference implementation will not parse, and resolving through one closed a nested literal early. Both directions now work on the spans the text is actually made of. ([#339](https://github.com/amplify-education/python-hcl2/issues/339)) - Flattened heredoc bodies match the values Terraform and OpenTofu evaluate the same source to. Three things differed, all checked against OpenTofu v1.12.5 rather than read off the spec: the newline terminating the last content line was dropped (`< str: + r"""Resolve a quoted string's escapes for a body that interprets none. + + A heredoc body is read literally, so anything the quoted form spelled as + an escape has to become the character itself: `\t` a tab, `\u00e9` an + accented e. `process_escape_sequences` is the package's one implementation + of that alphabet, and using it here is what stops this path from resolving + a shorter list than the reader does. + + Only in literal spans. Inside `${...}` the text is expression source, and + an escape there belongs to a string literal written inside the expression: + OpenTofu reads `"${upper("a\"b")}"` as `A"B`, so resolving that `\"` would + close the nested literal early and change what the expression says. + """ + return map_literal_spans(inner, process_escape_sequences) -_HEREDOC_BODY_ESCAPES = {"n": "\n", "r": "\r"} # A line that could end a heredoc: the delimiter word alone, give or take # surrounding spaces and tabs -- and a carriage return, because the body is # split on "\n" and a CRLF line hands back its own `\r`. OpenTofu ends a # heredoc on `EOF\r` exactly as it does on `EOF `, so a CRLF body carrying -# the delimiter has to count. This grammar is stricter than Terraform, whose -# scanner ends the heredoc on `EOF ` while `HEREDOC_TEMPLATE` here requires -# the newline to follow the word itself. The looser reading is the safe one to -# pick a delimiter against: emitting a body that only Terraform would treat as -# closed writes a file this library can read and Terraform cannot. +# the delimiter has to count. _CLOSING_MARKER_LINE = re.compile(r"[ \t]*([a-zA-Z][a-zA-Z0-9._-]*)[ \t\r]*") @@ -114,25 +127,6 @@ def _expressible_as_heredoc(content: str) -> bool: return "\r" not in content.replace("\r\n", "") -def _unescape_heredoc_body(inner: str) -> str: - r"""Resolve the escapes a heredoc body carries literally: \n, \r, \" and \\. - - A heredoc interprets no backslash sequence -- its body is the characters - themselves -- so anything the quoted form spelled as an escape has to be - resolved before it is written into one. `\r` is here because the flattened - form escapes carriage returns: without it, a heredoc read out of a CRLF - file and written back came out holding a literal backslash and an `r`. - - Single-pass, so an escaped backslash cannot combine with the character - after it. - """ - return re.sub( - r'\\(n|r|"|\\)', - lambda m: _HEREDOC_BODY_ESCAPES.get(m.group(1), m.group(1)), - inner, - ) - - @dataclass class DeserializerOptions: """Options controlling how Python dicts are deserialized into LarkElement trees.""" diff --git a/hcl2/rules/strings.py b/hcl2/rules/strings.py index 1e559959..22332554 100644 --- a/hcl2/rules/strings.py +++ b/hcl2/rules/strings.py @@ -17,6 +17,7 @@ STRING_CHARS, TEMPLATE_STRING, ) +from hcl2.template import map_literal_spans, resolve_escaped_markers from hcl2.utils import ( HEREDOC_PATTERN, HEREDOC_TRIM_PATTERN, @@ -53,6 +54,16 @@ def _strip_closing_marker_indent(text: str) -> str: return re.sub(r"[^\S\n]*\Z", "", text) +def _escape_for_quoted_source(text: str) -> str: + r"""Escape literal text so it can sit inside a quoted string. + + A carriage return is escaped alongside the newline: raw, it would break the + quoted string it is being written into. OpenTofu rejects `"ab"` with + "No closing marker was found for the string". + """ + return text.replace("\\", "\\\\").replace('"', '\\"').replace("\r", "\\r").replace("\n", "\\n") + + class InterpolationRule(LarkRule): """Rule for ${expression} interpolation within strings.""" @@ -138,15 +149,22 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext @staticmethod def _serialize_part_as_value(part, options, context) -> str: - """Serialize one part, resolving escapes in literal text only. + """Serialize one part into what the reader sees, by its terminal. - Interpolations and escaped interpolation/directive markers are passed - through untouched: their text is expression source, not literal - content, so an escape inside them is not this string's to resolve. + Literal text has its escapes resolved. An interpolation is passed + through untouched: its text is expression source, not literal content, + so an escape inside it is not this string's to resolve. + + `$${` and `%%{` are neither. They are escapes for a literal `${` and + `%{`, so the value carries the single sigil -- `"$${esc}"` is the six + characters `${esc}` to Terraform, not seven. """ serialized = part.serialize(options, context) - if part.content.lark_name() == "STRING_CHARS": + terminal = part.content.lark_name() + if terminal == "STRING_CHARS": return process_escape_sequences(serialized) + if terminal in ("ESCAPED_INTERPOLATION", "ESCAPED_DIRECTIVE"): + return serialized[1:] return serialized @@ -178,17 +196,15 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext raise RuntimeError(f"Invalid Heredoc token: {heredoc}") heredoc = _strip_closing_marker_indent(match.group(2)) if options.strip_string_quotes: - # 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. - return heredoc - # A carriage return is escaped alongside the newline: raw, it would - # break the quoted string it is being written into. OpenTofu rejects - # `"ab"` with "No closing marker was found for the string". - heredoc = ( - heredoc.replace("\\", "\\\\").replace('"', '\\"').replace("\r", "\\r").replace("\n", "\\n") - ) - return f'"{heredoc}"' + # The caller asked for the value: real newlines, no escaping. + # `$${` and `%%{` are resolved, being escapes for a literal + # `${` and `%{` rather than characters of the value. + return resolve_escaped_markers(heredoc) + # Only the literal spans are escaped. Inside `${...}` the text is + # expression source, and escaping a quote there rewrites someone + # else's code: `${upper("a")}` would become `${upper(\\"a\\")}`, + # which OpenTofu rejects outright. + return '"' + map_literal_spans(heredoc, _escape_for_quoted_source) + '"' result = heredoc.rstrip(self._trim_chars) if options.strip_string_quotes: @@ -217,12 +233,11 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext match = HEREDOC_TRIM_PATTERN.match(heredoc) if not match: raise RuntimeError(f"Invalid Heredoc token: {heredoc}") - lines = self._dedent(_strip_closing_marker_indent(match.group(2))) + body = "\n".join(self._dedent(_strip_closing_marker_indent(match.group(2)))) if options.strip_string_quotes: # The caller asked for the value: real newlines, no escaping. - return "\n".join(lines) - escaped = [line.replace("\\", "\\\\").replace('"', '\\"').replace("\r", "\\r") for line in lines] - return '"' + "\\n".join(escaped) + '"' + return resolve_escaped_markers(body) + return '"' + map_literal_spans(body, _escape_for_quoted_source) + '"' result = heredoc.rstrip(self._trim_chars) if options.strip_string_quotes: diff --git a/hcl2/template.py b/hcl2/template.py new file mode 100644 index 00000000..476604be --- /dev/null +++ b/hcl2/template.py @@ -0,0 +1,115 @@ +"""Splitting template text into the parts that mean different things. + +A quoted string or a heredoc body is not one run of literal characters. HCL +reads `${...}` and `%{...}` as expression source, and `$${`/`%%{` as escapes +for a literal `${`/`%{`. Anything that rewrites such text -- resolving escapes, +adding them, or turning one form into another -- has to know which span it is +looking at, or it corrupts the expression inside. + +The grammar already separates these for a quoted string, which is why +`StringRule._serialize_part_as_value` can do the right thing by asking each +part for its terminal. A heredoc body arrives as one opaque token, so the same +distinction has to be recovered from the text. That is what this does. + +The scan is a single left-to-right pass, because the two questions cannot be +answered separately: a splitter run before escapes are resolved would see the +`${` inside `$${` and open an interpolation that is not there. +""" + +from typing import Callable, Iterator, Tuple + +LITERAL = "literal" +INTERPOLATION = "interpolation" + +_OPENERS = {"$": "${", "%": "%{"} +_ESCAPES = {"$": "$${", "%": "%%{"} + + +def _scan_expression(text: str, start: int) -> int: + """Return the index just past the `}` closing the span opened at *start*. + + Braces nest, and a string literal inside may contain braces of its own or + an escaped quote, so neither can be found by counting alone. + """ + depth = 0 + index = start + length = len(text) + while index < length: + char = text[index] + if char == '"': + index += 1 + while index < length: + if text[index] == "\\": + index += 2 + continue + if text[index] == '"': + break + index += 1 + index += 1 + continue + if char == "{": + depth += 1 + elif char == "}": + depth -= 1 + if depth == 0: + return index + 1 + index += 1 + # Unbalanced: the caller gets the rest as one span rather than an error, + # because a serializer is the wrong place to reject what the parser took. + return length + + +def split_template(text: str) -> Iterator[Tuple[str, str]]: + """Yield `(kind, chunk)` pairs covering *text* exactly once. + + `kind` is `LITERAL` for text the reader treats as characters, including + the `$${` and `%%{` escapes themselves, and `INTERPOLATION` for a `${...}` + or `%{...}` span, whose content is expression source. + """ + index = 0 + literal_start = 0 + length = len(text) + + while index < length: + char = text[index] + if char in _OPENERS: + escape = _ESCAPES[char] + if text.startswith(escape, index): + # An escape is literal text, and consuming it here is what + # stops the `${` inside it from opening a span. + index += len(escape) + continue + if text.startswith(_OPENERS[char], index): + if literal_start != index: + yield LITERAL, text[literal_start:index] + end = _scan_expression(text, index + 1) + yield INTERPOLATION, text[index:end] + index = end + literal_start = index + continue + index += 1 + + if literal_start != length: + yield LITERAL, text[literal_start:length] + + +def resolve_escaped_markers(text: str) -> str: + """Resolve `$${` and `%%{` into the single sigil they stand for. + + Only in literal spans: inside `${...}` the same characters are expression + source, where `$${` does not mean a literal `${`. + """ + return "".join( + chunk.replace("$${", "${").replace("%%{", "%{") if kind == LITERAL else chunk + for kind, chunk in split_template(text) + ) + + +def map_literal_spans(text: str, transform: Callable[[str], str]) -> str: + """Apply *transform* to the literal spans of *text*, leaving the rest alone. + + An escape belongs to the string that carries it, not to an expression + written inside it: escaping or unescaping through an interpolation rewrites + someone else's source and changes what it means. + """ + return "".join(transform(chunk) if kind == LITERAL else chunk for kind, chunk in split_template(text)) diff --git a/test/unit/test_api.py b/test/unit/test_api.py index 0c444152..f4191e26 100644 --- a/test/unit/test_api.py +++ b/test/unit/test_api.py @@ -528,8 +528,21 @@ def test_lone_surrogate_escape_stays_encodable(self): def test_interpolation_is_left_alone(self): self.assertEqual(self._load('a = "pre${var.x}post"\n'), {"a": "pre${var.x}post"}) - def test_escaped_interpolation_marker_is_left_alone(self): - self.assertEqual(self._load('a = "lit $${x}"\n'), {"a": "lit $${x}"}) + def test_escaped_interpolation_marker_resolves_to_one_sigil(self): + r"""`$${` is an escape, so the value carries a single `${`. + + This asserted the doubled form until GH #336. `$${` and `%%{` are what + HCL provides for writing a literal `${` and `%{`, exactly as `\"` is + for a quote: OpenTofu v1.12.5 evaluates `"$${esc}"` to the six + characters `${esc}`. Leaving them doubled made the value differ from + the one Terraform reads, in the one mode that promises the value. + """ + self.assertEqual(self._load('a = "lit $${x}"\n'), {"a": "lit ${x}"}) + self.assertEqual(self._load('a = "lit %%{x}"\n'), {"a": "lit %{x}"}) + + def test_a_doubled_sigil_without_a_brace_is_not_an_escape(self): + """`$$` and `%%` are only escapes in front of `{`.""" + self.assertEqual(self._load('a = "$$notbrace %%either"\n'), {"a": "$$notbrace %%either"}) def test_default_options_still_preserve_source_form(self): """Without the option, the source form is kept for reconstruction.""" diff --git a/test/unit/test_template_spans.py b/test/unit/test_template_spans.py new file mode 100644 index 00000000..890e1093 --- /dev/null +++ b/test/unit/test_template_spans.py @@ -0,0 +1,143 @@ +# pylint: disable=C0103,C0114,C0115,C0116 +r"""Escapes belong to the span they are written in (GH #329, #336, #339). + +A quoted string or a heredoc body is not one run of literal characters. HCL +reads `${...}` and `%{...}` as expression source, and `$${`/`%%{` as escapes +for a literal `${`/`%{`. Three defects came from ignoring that: + +* the writer resolved four escapes where the reader resolves nine (#329), +* `$${` and `%%{` were returned doubled by the value form, which is not what + Terraform evaluates them to (#336), +* and both the escaping and the unescaping ran over interpolation text, which + belongs to an expression rather than to this string (#339). + +Every expectation below was checked against OpenTofu v1.12.5. +""" + +from unittest import TestCase + +from hcl2.api import dumps, loads +from hcl2.deserializer import DeserializerOptions +from hcl2.template import INTERPOLATION, LITERAL, split_template +from hcl2.utils import SerializationOptions + +FLAT = SerializationOptions(preserve_heredocs=False) +VALUE = SerializationOptions(preserve_heredocs=False, strip_string_quotes=True) +QUOTED_VALUE = SerializationOptions(strip_string_quotes=True) +HEREDOCS = DeserializerOptions(strings_to_heredocs=True) + + +class TestSplitTemplate(TestCase): + def test_plain_text_is_one_literal(self): + self.assertEqual(list(split_template("plain")), [(LITERAL, "plain")]) + + def test_an_interpolation_is_its_own_span(self): + self.assertEqual( + list(split_template('a${upper("x")}b')), + [(LITERAL, "a"), (INTERPOLATION, '${upper("x")}'), (LITERAL, "b")], + ) + + def test_an_escaped_marker_stays_literal(self): + # The `${` inside `$${` must not open a span -- which is why the scan + # cannot be split into "find boundaries" then "resolve escapes". + self.assertEqual(list(split_template("a$${esc}b")), [(LITERAL, "a$${esc}b")]) + self.assertEqual(list(split_template("a%%{d}b")), [(LITERAL, "a%%{d}b")]) + + def test_braces_inside_a_nested_string_do_not_close_the_span(self): + self.assertEqual(list(split_template('${ {k = "}"} }')), [(INTERPOLATION, '${ {k = "}"} }')]) + + def test_a_directive_is_expression_source_too(self): + self.assertEqual( + list(split_template("%{ if x }t%{ endif }")), + [ + (INTERPOLATION, "%{ if x }"), + (LITERAL, "t"), + (INTERPOLATION, "%{ endif }"), + ], + ) + + def test_an_unbalanced_span_is_not_an_error(self): + # A serializer is the wrong place to reject what the parser accepted. + self.assertEqual(list(split_template("x ${a")), [(LITERAL, "x "), (INTERPOLATION, "${a")]) + + +class TestTheValueResolvesTheTemplateEscapes(TestCase): + """`$${` and `%%{` stand for a literal `${` and `%{`, so the value has one.""" + + def test_in_a_quoted_string(self): + self.assertEqual(loads('a = "$${esc}"\n', serialization_options=QUOTED_VALUE)["a"], "${esc}") + self.assertEqual(loads('a = "%%{d}"\n', serialization_options=QUOTED_VALUE)["a"], "%{d}") + + def test_in_a_heredoc(self): + self.assertEqual(loads("a = < str: + return dumps({"a": value}, deserializer_options=HEREDOCS) + + def test_a_tab_becomes_a_tab(self): + self.assertEqual(self._body(r'"a\tb\n"'), "a = < Date: Tue, 1 Sep 2026 21:47:16 -0700 Subject: [PATCH 10/16] fix: a brace in a comment does not close an expression The span scan skipped string literals, on the grounds that they are what carries a brace that does not nest. Comments do too, and HCL writes them three ways: OpenTofu evaluates `${1 /* } */ + 2}` to 3, so counting that brace ended the interpolation in the middle of itself and handed the rest back as literal text -- which flattening then escaped, rewriting expression source into something that would not parse. `#` and `//` run to the end of the line, `/* */` to its terminator, and an unterminated one runs to the end rather than hanging. A brace inside a heredoc body written inline in an expression is still counted. That needs the delimiter matched to recognise, and is recorded in the docstring as a known gap rather than left to be discovered. --- CHANGELOG.md | 2 +- hcl2/template.py | 53 +++++++++++++++++++++++++------- test/unit/test_template_spans.py | 41 ++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb24f7a8..67c81031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `$${` and `%%{` resolve to `${` and `%{` in the value form, in both quoted strings and heredocs. They are HCL's escapes for a literal sigil, exactly as `\"` is for a quote, and OpenTofu evaluates `"$${esc}"` to the six characters `${esc}`; returning them doubled made the value differ from the one Terraform reads, in the one mode that promises the value. ([#336](https://github.com/amplify-education/python-hcl2/issues/336)) - `strings_to_heredocs` resolves every escape the reader does. It knew `\n`, `\r`, `\"` and `\\`, so `"a\tb\n"` was written into the body as a backslash and a `t` -- two characters where Terraform reads one tab -- and `\uNNNN` fared the same. It now uses `process_escape_sequences`, the package's one implementation of that alphabet. ([#329](https://github.com/amplify-education/python-hcl2/issues/329)) -- Escapes are no longer added or resolved inside `${...}`. The text there is expression source, where a nested `"..."` is a string literal of its own: OpenTofu reads `"${upper("a")}"` as `A` and rejects `"${upper(\"a\")}"` outright, so escaping through an interpolation produced source the reference implementation will not parse, and resolving through one closed a nested literal early. Both directions now work on the spans the text is actually made of. ([#339](https://github.com/amplify-education/python-hcl2/issues/339)) +- Escapes are no longer added or resolved inside `${...}`. The text there is expression source, where a nested `"..."` is a string literal of its own: OpenTofu reads `"${upper("a")}"` as `A` and rejects `"${upper(\"a\")}"` outright, so escaping through an interpolation produced source the reference implementation will not parse, and resolving through one closed a nested literal early. Both directions now work on the spans the text is actually made of, and the scan knows the three things inside an expression that can carry a non-structural brace: a string literal, and HCL's `#`, `//` and `/* */` comments -- OpenTofu evaluates `${1 /* } */ + 2}` to 3, so counting that brace closed the expression inside itself. ([#339](https://github.com/amplify-education/python-hcl2/issues/339)) - Flattened heredoc bodies match the values Terraform and OpenTofu evaluate the same source to. Three things differed, all checked against OpenTofu v1.12.5 rather than read off the spec: the newline terminating the last content line was dropped (`< int: + """Return the index just past the string literal opening at *index*.""" + length = len(text) + index += 1 + while index < length: + if text[index] == "\\": + index += 2 + continue + if text[index] == '"': + return index + 1 + index += 1 + return length + + +def _skip_comment(text: str, index: int) -> int: + """Return the index just past the comment opening at *index*, or *index*. + + HCL writes them three ways, and all three may hold a brace: `#` and `//` + run to the end of the line, `/* */` to its terminator. OpenTofu evaluates + `${1 /* } */ + 2}` to 3, so a scan that counts that brace closes the + expression in the middle of itself. + """ + if text.startswith("/*", index): + end = text.find("*/", index + 2) + return len(text) if end == -1 else end + 2 + if text.startswith("//", index) or text[index] == "#": + end = text.find("\n", index) + return len(text) if end == -1 else end + return index + + def _scan_expression(text: str, start: int) -> int: """Return the index just past the `}` closing the span opened at *start*. - Braces nest, and a string literal inside may contain braces of its own or - an escaped quote, so neither can be found by counting alone. + Braces nest, and three things inside an expression may carry one that is + not structural: a string literal, a comment, and the body of a heredoc + written inline. The first two are skipped here. A brace in a heredoc body + is not, which is a known gap rather than an oversight -- recognising one + means matching its delimiter, and the case has not been seen in the wild. """ depth = 0 index = start @@ -37,16 +71,13 @@ def _scan_expression(text: str, start: int) -> int: while index < length: char = text[index] if char == '"': - index += 1 - while index < length: - if text[index] == "\\": - index += 2 - continue - if text[index] == '"': - break - index += 1 - index += 1 + index = _skip_string(text, index) continue + if char == "#" or text.startswith("//", index) or text.startswith("/*", index): + skipped = _skip_comment(text, index) + if skipped != index: + index = skipped + continue if char == "{": depth += 1 elif char == "}": diff --git a/test/unit/test_template_spans.py b/test/unit/test_template_spans.py index 890e1093..d23d6175 100644 --- a/test/unit/test_template_spans.py +++ b/test/unit/test_template_spans.py @@ -141,3 +141,44 @@ def test_the_pair_round_trips(self): loads(restored, serialization_options=VALUE)["a"], loads(source, serialization_options=VALUE)["a"], ) + + +class TestBracesThatAreNotStructural(TestCase): + r"""Three things inside an expression may carry a brace that does not nest. + + A string literal was handled from the start. Comments were not, and HCL + writes them three ways: OpenTofu evaluates `${1 /* } */ + 2}` to 3, so a + scan that counts that brace closes the expression inside itself and hands + the rest back as literal text -- which flattening then escapes, rewriting + expression source. + """ + + def test_a_brace_in_a_block_comment(self): + self.assertEqual( + list(split_template("${1 /* } */ + 2}")), [(INTERPOLATION, "${1 /* } */ + 2}")] + ) + + def test_a_brace_in_a_hash_comment(self): + self.assertEqual( + list(split_template('${ foo( # }\n "a") }')), + [(INTERPOLATION, '${ foo( # }\n "a") }')], + ) + + def test_a_brace_in_a_slash_comment(self): + self.assertEqual( + list(split_template("${ a // }\n }")), [(INTERPOLATION, "${ a // }\n }")] + ) + + def test_a_brace_in_a_string_literal(self): + self.assertEqual( + list(split_template('${ {k = "}"} }')), [(INTERPOLATION, '${ {k = "}"} }')] + ) + + def test_an_unterminated_comment_does_not_hang(self): + self.assertEqual(list(split_template("${ a /* } ")), [(INTERPOLATION, "${ a /* } ")]) + + def test_flattening_leaves_a_commented_expression_alone(self): + self.assertEqual( + loads('a = < Date: Tue, 1 Sep 2026 22:28:35 -0700 Subject: [PATCH 11/16] fix: four holes a code review found in the span work A backslash pair is one unit. The scan entered string mode at the quote of a `\"`, read the real closing quote as another escape, and ran to the end of the text -- so a span containing the grammar's own `\"..\"` form swallowed everything after it into one interpolation. Resolving escapes can spell a sigil that was not in the source. `"${foo}"` is the six literal characters `${foo}` to Terraform, because escapes resolve at token level and the result is not rescanned; written into a heredoc body, which is not escaped at all, those characters are a live interpolation. The reverse demotes one. The conversion is refused in both directions now, as it already was for a lone carriage return, by comparing the spans of the source with the spans of the resolved content. An unbalanced span is literal rather than an expression. Calling it an expression meant nothing escaped it, so the heredoc paths wrote raw newlines and unescaped quotes into what the API calls quoted-string source -- turning a loud failure into silent bad output. Two tests pinned the old behaviour and now state this one. `split_template` returns immediately for text holding no `$` or `%`, which is nearly all of it: a scan for two characters answers the question that a per-character loop was answering. `process_escape_ sequences` next door already had the guard. One finding is refuted rather than fixed: escapes inside `$${...}` are not resolved by OpenTofu either. `"$${a\tb}"` evaluates to `${a\tb}` with a literal backslash and a `t`, which is what this returns -- the whole marker is one token, and its interior is not a template. --- hcl2/deserializer.py | 23 +++++-- hcl2/template.py | 33 ++++++++-- test/unit/test_template_spans.py | 103 ++++++++++++++++++++++--------- 3 files changed, 122 insertions(+), 37 deletions(-) diff --git a/hcl2/deserializer.py b/hcl2/deserializer.py index 7c5946c1..b6940d8a 100644 --- a/hcl2/deserializer.py +++ b/hcl2/deserializer.py @@ -60,7 +60,7 @@ FloatLiteral, IntLiteral, ) -from hcl2.template import map_literal_spans +from hcl2.template import INTERPOLATION, map_literal_spans, split_template from hcl2.transformer import RuleTransformer from hcl2.utils import HEREDOC_PATTERN, HEREDOC_TRIM_PATTERN, process_escape_sequences @@ -114,7 +114,12 @@ def _heredoc_delimiter(content: str) -> str: return f"EOF_{suffix}" -def _expressible_as_heredoc(content: str) -> bool: +def _interpolation_spans(text: str) -> List[str]: + """The `${...}` and `%{...}` spans of *text*, in order.""" + return [chunk for kind, chunk in split_template(text) if kind == INTERPOLATION] + + +def _expressible_as_heredoc(content: str, source: str) -> bool: """Whether *content* can be a heredoc body without changing. A heredoc body is read literally, so it can hold a carriage return only @@ -124,7 +129,17 @@ def _expressible_as_heredoc(content: str) -> bool: from is valid and evaluates to that carriage return. Such a value stays quoted, for the same reason one that does not end in a newline does. """ - return "\r" not in content.replace("\r\n", "") + if "\r" in content.replace("\r\n", ""): + return False + + # Resolving escapes can spell a sigil that was not there. `"\u0024\u007bfoo\u007d"` + # is the six literal characters `${foo}` to Terraform -- escapes resolve at + # token level and the result is not rescanned -- but written into a heredoc + # body, which is not escaped at all, those characters are a live + # interpolation. The reverse happens too: `\u0024${b}` resolves to `$${b}`, + # demoting an interpolation to escaped text. Either way the value changes, + # so it stays quoted. + return _interpolation_spans(source) == _interpolation_spans(content) @dataclass @@ -254,7 +269,7 @@ def _deserialize_text(self, value: Any) -> LarkRule: # heredoc could in fact express -- `< int: length = len(text) while index < length: char = text[index] + if char == "\\": + # A backslash pair is one unit. Without this the scan enters string + # mode at the quote of a `\\"`, then reads the real closing quote as + # another escape and runs to the end of the text, swallowing + # everything after the expression into one span. + index += 2 + continue if char == '"': index = _skip_string(text, index) continue @@ -85,9 +92,11 @@ def _scan_expression(text: str, start: int) -> int: if depth == 0: return index + 1 index += 1 - # Unbalanced: the caller gets the rest as one span rather than an error, - # because a serializer is the wrong place to reject what the parser took. - return length + # Unbalanced. Reported as such rather than swallowed: handing the rest + # back as an expression means nothing escapes it, and the heredoc paths + # then emit raw newlines and unescaped quotes into what is supposed to be + # quoted-string source. Treated as literal it is at least well-formed. + return -1 def split_template(text: str) -> Iterator[Tuple[str, str]]: @@ -97,12 +106,23 @@ def split_template(text: str) -> Iterator[Tuple[str, str]]: the `$${` and `%%{` escapes themselves, and `INTERPOLATION` for a `${...}` or `%{...}` span, whose content is expression source. """ + if "$" not in text and "%" not in text: + # The overwhelmingly common case, and the one this used to make + # expensive: a per-character loop where a scan for two characters + # answers the question. `process_escape_sequences` has the same guard. + if text: + yield LITERAL, text + return + index = 0 literal_start = 0 length = len(text) while index < length: char = text[index] + if char == "\\": + index += 2 + continue if char in _OPENERS: escape = _ESCAPES[char] if text.startswith(escape, index): @@ -111,9 +131,14 @@ def split_template(text: str) -> Iterator[Tuple[str, str]]: index += len(escape) continue if text.startswith(_OPENERS[char], index): + end = _scan_expression(text, index + 1) + if end == -1: + # Nothing closes it, so the rest is literal text -- and the + # leading run has to stay unyielded, or the tail below + # repeats it. + break if literal_start != index: yield LITERAL, text[literal_start:index] - end = _scan_expression(text, index + 1) yield INTERPOLATION, text[index:end] index = end literal_start = index diff --git a/test/unit/test_template_spans.py b/test/unit/test_template_spans.py index d23d6175..8ce350ca 100644 --- a/test/unit/test_template_spans.py +++ b/test/unit/test_template_spans.py @@ -56,25 +56,21 @@ def test_a_directive_is_expression_source_too(self): ], ) - def test_an_unbalanced_span_is_not_an_error(self): - # A serializer is the wrong place to reject what the parser accepted. - self.assertEqual(list(split_template("x ${a")), [(LITERAL, "x "), (INTERPOLATION, "${a")]) - - -class TestTheValueResolvesTheTemplateEscapes(TestCase): - """`$${` and `%%{` stand for a literal `${` and `%{`, so the value has one.""" - - def test_in_a_quoted_string(self): - self.assertEqual(loads('a = "$${esc}"\n', serialization_options=QUOTED_VALUE)["a"], "${esc}") - self.assertEqual(loads('a = "%%{d}"\n', serialization_options=QUOTED_VALUE)["a"], "%{d}") - - def test_in_a_heredoc(self): - self.assertEqual(loads("a = < str: + return dumps({"x": value}, deserializer_options=HEREDOCS) + + def test_a_synthesised_sigil_keeps_the_value_quoted(self): + # The value is `\u0024\u007bfoo\u007d\n`, spelled with chr() so this + # test's own source cannot be confused with the characters it means. + esc = chr(92) + "u" + source = '"' + esc + "0024" + esc + "007bfoo" + esc + "007d" + chr(92) + "n" + '"' + self.assertEqual(self._written(source), "x = " + source + "\n") + + def test_a_demoted_interpolation_keeps_the_value_quoted(self): + source = '"' + chr(92) + "u0024" + "${b}" + chr(92) + "n" + '"' + self.assertEqual(self._written(source), "x = " + source + "\n") + + def test_a_real_interpolation_still_converts(self): + self.assertEqual(self._written(r'"${b}\n"'), "x = < Date: Tue, 1 Sep 2026 22:39:57 -0700 Subject: [PATCH 12/16] perf: answer the cheap question first, and pin the escaper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_unescape_heredoc_body` ran before the check that decides whether its result is wanted, so for a document where nothing ends in a newline every pass over every value was discarded. The last two characters answer it: a value ends with a newline only if its source does, escaped or real. Conservative -- `"a\\n"` passes here and is rejected by the check that matters -- and cheap. The escaper had no test at all, which is how the reader and writer drift apart. Its four markers are now pinned against `process_escape_ sequences` reading them back, and the two it does not write -- `\t` and the unicode forms -- are stated as deliberate: those characters are legal inside a quoted string as themselves. The unicode test asserted nothing, because its input held a literal e-acute rather than the escape. It uses `é` and `\U0001F600` now, and fails against the four-escape implementation this replaced. The `$${` and `%%{` expectations are in `CASES`, so `bin/heredoc_ground_truth` re-derives them from OpenTofu with the rest: 23 cases, 0 disagreements. The `${keep}` case is not there on purpose -- an undefined reference has no value for `tofu console` to print. --- hcl2/deserializer.py | 18 +++++++- test/unit/test_heredoc_matches_terraform.py | 5 +++ test/unit/test_template_spans.py | 47 ++++++++++++++++++++- 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/hcl2/deserializer.py b/hcl2/deserializer.py index b6940d8a..b3c82bff 100644 --- a/hcl2/deserializer.py +++ b/hcl2/deserializer.py @@ -65,6 +65,22 @@ from hcl2.utils import HEREDOC_PATTERN, HEREDOC_TRIM_PATTERN, process_escape_sequences +def _may_end_a_heredoc(inner: str) -> bool: + r"""Whether *inner* could possibly resolve to a newline-terminated value. + + Only such a value can be written as a heredoc, and resolving the escapes to + find out costs a pass over the whole string -- for a document where nothing + ends in a newline, every one of those passes is discarded. The last two + characters answer it: a value ends with a newline only if its source ends + with one, escaped or real. + + Conservative on purpose. `"a\\n"` ends with a backslash and an `n` and + passes here, then resolves to those two characters and is rejected by the + check that actually matters. + """ + return inner.endswith("\\n") or inner.endswith("\n") + + def _unescape_heredoc_body(inner: str) -> str: r"""Resolve a quoted string's escapes for a body that interprets none. @@ -258,7 +274,7 @@ def _deserialize_text(self, value: Any) -> LarkRule: if match: return self._deserialize_heredoc(value[1:-1], False) - if self.options.strings_to_heredocs: + if self.options.strings_to_heredocs and _may_end_a_heredoc(value[1:-1]): content = _unescape_heredoc_body(value[1:-1]) # A heredoc's closing marker sits on a line of its own, so # any body with content in it ends with a newline. A value diff --git a/test/unit/test_heredoc_matches_terraform.py b/test/unit/test_heredoc_matches_terraform.py index 901ea299..710efb77 100644 --- a/test/unit/test_heredoc_matches_terraform.py +++ b/test/unit/test_heredoc_matches_terraform.py @@ -60,6 +60,11 @@ ("<<-EOT\n\va\n\vb\n\vEOT", "a\nb\n"), ("<<-EOT\n\fa\n\fb\n\fEOT", "a\nb\n"), ("<<-EOT\n\u3000a\n\u3000b\n\u3000EOT", "a\nb\n"), + # `$${` and `%%{` are escapes for a literal sigil, in a heredoc as much as + # in a quoted string, so the value carries the single form. + ("< Date: Tue, 1 Sep 2026 23:01:10 -0700 Subject: [PATCH 13/16] fix: a directive does not hide the escaped markers inside it `_serialize_part_as_value` decides what to do by asking each part for its terminal, which works while the parts are flat. A template directive is not: the whole `%{ if }...%{ endif }` construct, and everything between, arrives as one part. So a `$${` written inside one never reached the branch that resolves it and stayed doubled -- while the same content in a heredoc resolved, because that path works on text rather than parts. The two source forms disagreed about identical content. Nested parts now go through the same span-aware helper the heredoc path uses: it resolves the markers in the literal stretches and leaves the directives themselves, which are expression source, alone. --- hcl2/rules/strings.py | 8 +++++++- test/unit/test_template_spans.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/hcl2/rules/strings.py b/hcl2/rules/strings.py index 22332554..42d45b8c 100644 --- a/hcl2/rules/strings.py +++ b/hcl2/rules/strings.py @@ -165,7 +165,13 @@ def _serialize_part_as_value(part, options, context) -> str: return process_escape_sequences(serialized) if terminal in ("ESCAPED_INTERPOLATION", "ESCAPED_DIRECTIVE"): return serialized[1:] - return serialized + # Anything else is a nested template rule -- a directive and everything + # it encloses arrive as one part, so a marker written between `%{ if }` + # and `%{ endif }` never reaches the branch above and stayed doubled, + # while the same content in a heredoc resolved. The helper is + # span-aware: it resolves the markers in the literal stretches and + # leaves the directives themselves, which are expression source, alone. + return resolve_escaped_markers(serialized) class HeredocTemplateRule(LarkRule): diff --git a/test/unit/test_template_spans.py b/test/unit/test_template_spans.py index bddafcb3..076ab9dd 100644 --- a/test/unit/test_template_spans.py +++ b/test/unit/test_template_spans.py @@ -272,3 +272,32 @@ def test_a_bmp_escape_becomes_its_character(self): def test_a_wide_escape_becomes_its_character(self): source = '"' + chr(92) + "U0001F600" + chr(92) + "n" + '"' self.assertEqual(dumps({"a": source}, deserializer_options=HEREDOCS), "a = < str: + return loads(f'a = "{body}"\n', serialization_options=QUOTED_VALUE)["a"] + + def _heredoc(self, body: str) -> str: + return loads(f"a = < Date: Wed, 2 Sep 2026 10:53:22 -0700 Subject: [PATCH 14/16] test: name the literal-character test for what it exercises `test_a_unicode_escape_becomes_its_character` fed a literal e-acute rather than a backslash-u escape spelling one, so it passed against the unfixed writer and proved nothing about the escape form its name claimed. `TestTheWriterResolvesUnicodeEscapes` already covers both escape forms, so rename this one to say what it does test -- that a non-ASCII character written literally survives the trip into a heredoc body -- and point the newer class's docstring at it rather than at "the earlier test". --- test/unit/test_template_spans.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/unit/test_template_spans.py b/test/unit/test_template_spans.py index 076ab9dd..b1506b73 100644 --- a/test/unit/test_template_spans.py +++ b/test/unit/test_template_spans.py @@ -95,7 +95,7 @@ def _body(self, value: str) -> str: def test_a_tab_becomes_a_tab(self): self.assertEqual(self._body(r'"a\tb\n"'), "a = < Date: Wed, 2 Sep 2026 11:36:22 -0700 Subject: [PATCH 15/16] fix: a string literal inside an expression is itself a template _skip_string scanned for the next quote, so in `${upper("v${ "{" }w")}` it ended the outer literal at the quote that opens the innermost one. The brace after it was then counted as structural, the span came back unbalanced, and the whole text was handed to the literal path -- where its quotes get escaped, which is the corruption #339 exists to prevent. It now recurses into a nested `${...}` or `%{...}`, and leaves `$${` and `%%{` as the escapes they are. Checked against OpenTofu v1.12.5, which evaluates `"a ${upper("v${ "{" }w")} b"` to `a V{W b` and `"a ${upper("v$${x}w")} b"` to `a V${X}W b`; three levels of nesting and a nested directive are covered too. Found by cross-examination of the review of this branch. --- CHANGELOG.md | 2 +- hcl2/template.py | 29 +++++++++++++++++--- test/unit/test_template_spans.py | 45 ++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67c81031..d8d3fc6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `$${` and `%%{` resolve to `${` and `%{` in the value form, in both quoted strings and heredocs. They are HCL's escapes for a literal sigil, exactly as `\"` is for a quote, and OpenTofu evaluates `"$${esc}"` to the six characters `${esc}`; returning them doubled made the value differ from the one Terraform reads, in the one mode that promises the value. ([#336](https://github.com/amplify-education/python-hcl2/issues/336)) - `strings_to_heredocs` resolves every escape the reader does. It knew `\n`, `\r`, `\"` and `\\`, so `"a\tb\n"` was written into the body as a backslash and a `t` -- two characters where Terraform reads one tab -- and `\uNNNN` fared the same. It now uses `process_escape_sequences`, the package's one implementation of that alphabet. ([#329](https://github.com/amplify-education/python-hcl2/issues/329)) -- Escapes are no longer added or resolved inside `${...}`. The text there is expression source, where a nested `"..."` is a string literal of its own: OpenTofu reads `"${upper("a")}"` as `A` and rejects `"${upper(\"a\")}"` outright, so escaping through an interpolation produced source the reference implementation will not parse, and resolving through one closed a nested literal early. Both directions now work on the spans the text is actually made of, and the scan knows the three things inside an expression that can carry a non-structural brace: a string literal, and HCL's `#`, `//` and `/* */` comments -- OpenTofu evaluates `${1 /* } */ + 2}` to 3, so counting that brace closed the expression inside itself. ([#339](https://github.com/amplify-education/python-hcl2/issues/339)) +- Escapes are no longer added or resolved inside `${...}`. The text there is expression source, where a nested `"..."` is a string literal of its own: OpenTofu reads `"${upper("a")}"` as `A` and rejects `"${upper(\"a\")}"` outright, so escaping through an interpolation produced source the reference implementation will not parse, and resolving through one closed a nested literal early. Both directions now work on the spans the text is actually made of, and the scan knows the things inside an expression that can carry a non-structural brace: a string literal, HCL's `#`, `//` and `/* */` comments, and the nested expressions a string literal may itself contain -- OpenTofu evaluates `${1 /* } */ + 2}` to 3 and `"a ${upper("v${ "{" }w")} b"` to `a V{W b`, so counting either brace closed the expression inside itself. ([#339](https://github.com/amplify-education/python-hcl2/issues/339)) - Flattened heredoc bodies match the values Terraform and OpenTofu evaluate the same source to. Three things differed, all checked against OpenTofu v1.12.5 rather than read off the spec: the newline terminating the last content line was dropped (`< int: - """Return the index just past the string literal opening at *index*.""" + """Return the index just past the string literal opening at *index*. + + A string literal inside an expression is itself a template, so a `${` or + `%{` in it opens a nested expression whose own literals may hold quotes + and braces. OpenTofu evaluates `"a ${upper("v${ "{" }w")} b"` to + `a V{W b`, so taking the next quote as the terminator ended this literal + at the one that *opens* the innermost one, and the brace after it was + then counted as structural. + + The escapes stay escapes here: `"a ${upper("v$${x}w")} b"` is `a V${X}W b`, + so `$${` does not open anything. + """ length = len(text) index += 1 while index < length: - if text[index] == "\\": + char = text[index] + if char == "\\": index += 2 continue - if text[index] == '"': + if char in _OPENERS: + if text.startswith(_ESCAPES[char], index): + index += len(_ESCAPES[char]) + continue + if text.startswith(_OPENERS[char], index): + end = _scan_expression(text, index + 1) + if end == -1: + # Unbalanced, so there is no literal to close either. + return length + index = end + continue + if char == '"': return index + 1 index += 1 return length diff --git a/test/unit/test_template_spans.py b/test/unit/test_template_spans.py index b1506b73..4c09f286 100644 --- a/test/unit/test_template_spans.py +++ b/test/unit/test_template_spans.py @@ -305,3 +305,48 @@ def test_a_directive_without_a_marker_is_unchanged(self): def test_an_interpolation_is_still_left_alone(self): self.assertEqual(self._quoted("pre${var.x}post"), "pre${var.x}post") + + +class TestAStringInsideAnExpressionIsItselfATemplate(TestCase): + r"""A `${` in a nested string literal opens an expression of its own. + + Every expected value below was evaluated by OpenTofu v1.12.5 rather than + read off the spec. Scanning for the next quote ended the outer literal at + the one that *opens* the innermost one, so the brace that followed was + counted as structural and the whole span came back unbalanced -- which + hands expression source to the literal path, where its quotes get escaped. + """ + + def _kinds(self, body: str) -> list: + return [kind for kind, _ in split_template(body)] + + def _spans(self, body: str) -> list: + return list(split_template(body)) + + def test_a_brace_in_a_nested_expressions_string(self): + # tofu: "a ${upper("v${ "{" }w")} b" -> a V{W b + body = 'a ${upper("v${ "{" }w")} b' + self.assertEqual(self._kinds(body), [LITERAL, INTERPOLATION, LITERAL]) + + def test_that_span_covers_the_whole_expression(self): + body = 'a ${upper("v${ "{" }w")} b' + self.assertEqual(self._spans(body)[1], (INTERPOLATION, '${upper("v${ "{" }w")}')) + + def test_three_levels_of_nesting(self): + # tofu: "a ${upper("p${ lower("Q${ "{" }R") }s")} b" -> a PQ{RS b + body = 'a ${upper("p${ lower("Q${ "{" }R") }s")} b' + self.assertEqual(self._kinds(body), [LITERAL, INTERPOLATION, LITERAL]) + + def test_an_escape_in_a_nested_string_opens_nothing(self): + # tofu: "a ${upper("v$${x}w")} b" -> a V${X}W b + body = 'a ${upper("v$${x}w")} b' + self.assertEqual(self._spans(body)[1], (INTERPOLATION, '${upper("v$${x}w")}')) + + def test_a_directive_nested_in_a_string(self): + # tofu: "a ${upper("v%{ if true }y%{ endif }w")} b" -> a VYW b + body = 'a ${upper("v%{ if true }y%{ endif }w")} b' + self.assertEqual(self._kinds(body), [LITERAL, INTERPOLATION, LITERAL]) + + def test_an_unterminated_nested_expression_is_still_literal(self): + # No closing brace anywhere, so nothing is an interpolation. + self.assertEqual(self._kinds('a ${upper("v${ x") b'), [LITERAL]) From 545b8e20ab9724d071ac4eef56d86529eb06421e Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 1 Sep 2026 23:19:28 -0700 Subject: [PATCH 16/16] fix: decline to flatten a heredoc whose interpolation spans lines (#347) The quoted form cannot express one. The newlines inside `${...}` are expression source: OpenTofu rejects an escaped newline there with "This character is not used within the language", and a raw one makes the quoted string span lines, which it rejects as an invalid multi-line string. There is no third spelling. It emitted the raw version, so `preserve_heredocs=False` produced output that neither Terraform nor this library could read -- `loads` of its own result raised `UnexpectedToken` -- with no error at the point it was written. It now hands the heredoc back as it was written, which is the form `preserve_heredocs=True` produces and reads back as that heredoc. So the value survives in the shape that can carry it, and a document round trips unchanged. Declining is the only answer that does not change what the document means. Collapsing the interpolation onto one line is semantically identical for most expressions and silently wrong for one holding a `#` comment or a nested heredoc; raising would break callers flattening documents that happen to contain one, including through the CLI. The value form is untouched -- it hands back the body, which has no such limit. --- CHANGELOG.md | 1 + hcl2/rules/strings.py | 11 +++- hcl2/template.py | 15 +++++ test/unit/test_multiline_interpolation.py | 67 +++++++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 test/unit/test_multiline_interpolation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d8d3fc6f..8cab16a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- A heredoc whose interpolation spans lines is not flattened. The quoted form cannot hold one: the newlines inside `${...}` are expression source, where OpenTofu rejects an escaped newline and a raw one makes the string span lines, which it also rejects. It used to emit the raw version -- output neither Terraform nor this library could read, written with no error -- and now hands the heredoc back in the form `preserve_heredocs=True` produces, which reads back as that heredoc. Declining is the only answer that does not change what the document means. ([#347](https://github.com/amplify-education/python-hcl2/issues/347)) - `$${` and `%%{` resolve to `${` and `%{` in the value form, in both quoted strings and heredocs. They are HCL's escapes for a literal sigil, exactly as `\"` is for a quote, and OpenTofu evaluates `"$${esc}"` to the six characters `${esc}`; returning them doubled made the value differ from the one Terraform reads, in the one mode that promises the value. ([#336](https://github.com/amplify-education/python-hcl2/issues/336)) - `strings_to_heredocs` resolves every escape the reader does. It knew `\n`, `\r`, `\"` and `\\`, so `"a\tb\n"` was written into the body as a backslash and a `t` -- two characters where Terraform reads one tab -- and `\uNNNN` fared the same. It now uses `process_escape_sequences`, the package's one implementation of that alphabet. ([#329](https://github.com/amplify-education/python-hcl2/issues/329)) - Escapes are no longer added or resolved inside `${...}`. The text there is expression source, where a nested `"..."` is a string literal of its own: OpenTofu reads `"${upper("a")}"` as `A` and rejects `"${upper(\"a\")}"` outright, so escaping through an interpolation produced source the reference implementation will not parse, and resolving through one closed a nested literal early. Both directions now work on the spans the text is actually made of, and the scan knows the things inside an expression that can carry a non-structural brace: a string literal, HCL's `#`, `//` and `/* */` comments, and the nested expressions a string literal may itself contain -- OpenTofu evaluates `${1 /* } */ + 2}` to 3 and `"a ${upper("v${ "{" }w")} b"` to `a V{W b`, so counting either brace closed the expression inside itself. ([#339](https://github.com/amplify-education/python-hcl2/issues/339)) diff --git a/hcl2/rules/strings.py b/hcl2/rules/strings.py index 42d45b8c..e9a991a6 100644 --- a/hcl2/rules/strings.py +++ b/hcl2/rules/strings.py @@ -17,7 +17,7 @@ STRING_CHARS, TEMPLATE_STRING, ) -from hcl2.template import map_literal_spans, resolve_escaped_markers +from hcl2.template import has_multi_line_span, map_literal_spans, resolve_escaped_markers from hcl2.utils import ( HEREDOC_PATTERN, HEREDOC_TRIM_PATTERN, @@ -195,6 +195,7 @@ def heredoc(self): def serialize(self, options=SerializationOptions(), context=SerializationContext()) -> Any: """Serialize the heredoc, optionally stripping to a plain string.""" heredoc = self.heredoc.serialize(options, context) + raw = heredoc if not options.preserve_heredocs: match = HEREDOC_PATTERN.match(heredoc) @@ -210,6 +211,11 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext # expression source, and escaping a quote there rewrites someone # else's code: `${upper("a")}` would become `${upper(\\"a\\")}`, # which OpenTofu rejects outright. + if has_multi_line_span(heredoc): + # No quoted spelling exists, so the heredoc is handed back as + # it was written -- the same form `preserve_heredocs=True` + # produces, which reads back as this heredoc. + return f'"{raw.rstrip(self._trim_chars)}"' return '"' + map_literal_spans(heredoc, _escape_for_quoted_source) + '"' result = heredoc.rstrip(self._trim_chars) @@ -234,6 +240,7 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext # This is a special version of heredocs that are declared with "<<-", # whose body is dedented by the smallest indent any of its lines carries. heredoc = self.heredoc.serialize(options, context) + raw = heredoc if not options.preserve_heredocs: match = HEREDOC_TRIM_PATTERN.match(heredoc) @@ -243,6 +250,8 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext if options.strip_string_quotes: # The caller asked for the value: real newlines, no escaping. return resolve_escaped_markers(body) + if has_multi_line_span(body): + return f'"{raw.rstrip(self._trim_chars)}"' return '"' + map_literal_spans(body, _escape_for_quoted_source) + '"' result = heredoc.rstrip(self._trim_chars) diff --git a/hcl2/template.py b/hcl2/template.py index c6dc960d..80d00a01 100644 --- a/hcl2/template.py +++ b/hcl2/template.py @@ -192,3 +192,18 @@ def map_literal_spans(text: str, transform: Callable[[str], str]) -> str: someone else's source and changes what it means. """ return "".join(transform(chunk) if kind == LITERAL else chunk for kind, chunk in split_template(text)) + + +def has_multi_line_span(text: str) -> bool: + """Whether any `${...}` or `%{...}` in *text* runs across a line. + + Such a body has no quoted spelling. The newlines inside the span are + expression source, where OpenTofu rejects an escaped one -- "This character + is not used within the language" -- and a raw one makes the quoted string + span lines, which it rejects as well. So the flattened form cannot express + it, and the only answer that does not change what the document means is to + decline. + """ + return any( + kind == INTERPOLATION and ("\n" in chunk or "\r" in chunk) for kind, chunk in split_template(text) + ) diff --git a/test/unit/test_multiline_interpolation.py b/test/unit/test_multiline_interpolation.py new file mode 100644 index 00000000..09c956b0 --- /dev/null +++ b/test/unit/test_multiline_interpolation.py @@ -0,0 +1,67 @@ +# pylint: disable=C0103,C0114,C0115,C0116 +r"""A heredoc whose interpolation spans lines is not flattened (GH #347). + +`preserve_heredocs=False` returns a heredoc as quoted-string source. That form +cannot hold an interpolation running across lines: the newlines inside `${...}` +are expression source, where OpenTofu rejects an escaped one -- "This character +is not used within the language" -- and a raw one makes the quoted string span +lines, which it rejects as an invalid multi-line string. + +Before, it produced the raw-newline version: output neither Terraform nor this +library could read, written with no error. Declining is the only answer that +does not change what the document means. The two alternatives both do: +collapsing the interpolation onto one line silently breaks an expression +holding a `#` comment or a nested heredoc, and raising breaks callers +flattening documents that contain one. + +The declined form is the one `preserve_heredocs=True` produces, which reads +back as this heredoc -- so the value survives, in the shape that can carry it. +""" + +from unittest import TestCase + +from hcl2.api import dumps, loads +from hcl2.utils import SerializationOptions + +FLAT = SerializationOptions(preserve_heredocs=False) +VALUE = SerializationOptions(preserve_heredocs=False, strip_string_quotes=True) + +MULTI_LINE = "a = <