From 5755fbfc876b8dea336badb384c0568a76d9536b Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 31 Aug 2026 14:02:09 -0700 Subject: [PATCH 01/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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/17] 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 23:09:59 -0700 Subject: [PATCH 09/17] fix: a heredoc ends its own line wherever it is written (#338) A heredoc ends at its closing marker, on a line of its own, so whatever follows has to start the next line. Inside a list or an object that is the separator, and `EOF,` closes nothing -- the file this library had just written did not parse, here or in Terraform. A top-level attribute survived only because the newline after it comes from the document rather than from the heredoc. The earlier attempt at this appended the newline to the token, which fixed containers and gave every top-level heredoc a blank line, because the reconstructor already supplies one there. The distinction it was missing: `HEREDOC_TEMPLATE` matches through the newline after the marker, so a token that came from the parser already ends the line and a token built by the deserializer does not. Only the second needs help. So the rule lives where the tokens are joined, and asks whether the heredoc just written ended its line rather than assuming either way. Reconstructing a parsed document is byte for byte what it was, which the round-trip fixtures already assert. OpenTofu reads the emitted list back as ["line1\n", "p"]. --- CHANGELOG.md | 1 + hcl2/reconstructor.py | 29 ++++++++++- test/unit/test_heredoc_line_end.py | 77 ++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 test/unit/test_heredoc_line_end.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 256e2d94..eef40378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,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: """Reconstruct a Token node into HCL text fragments.""" result = str(token.value) - if self._should_add_space_before(token, parent_rule_name): + if self._needs_line_after_heredoc(token): + # A heredoc ends at its closing marker, on a line of its own. Any + # token that follows has to start the next line -- inside a list or + # an object that token is the separator, and `EOF,` closes nothing, + # so the file did not parse. A top-level attribute survived only + # because the newline it is followed by comes from the document. + result = "\n" + result + elif self._should_add_space_before(token, parent_rule_name): result = " " + result self._last_token_name = token.type + self._last_token_ended_line = str(token.value).endswith(("\n", "\r\n")) if len(token) != 0: self._last_was_space = result[-1].endswith(" ") or result[-1].endswith("\n") return result + def _needs_line_after_heredoc(self, token: Token) -> bool: + """Whether *token* has to start a new line because a heredoc just ended. + + Only for a heredoc that does not carry its own. `HEREDOC_TEMPLATE` + matches through the newline after the closing marker, so a token that + came from the parser already ends the line; one built by the + deserializer does not, and that is the case where the separator landed + on the marker's line. + """ + if self._last_token_name not in self._heredoc_token_names: + return False + if self._last_token_ended_line: + return False + # Anything that already begins one is fine as it is. + return not str(token.value).startswith(("\n", "\r\n")) + def _reconstruct_node( self, node: Union[Tree, Token], parent_rule_name: Optional[str] = None ) -> List[str]: diff --git a/test/unit/test_heredoc_line_end.py b/test/unit/test_heredoc_line_end.py new file mode 100644 index 00000000..289a78e3 --- /dev/null +++ b/test/unit/test_heredoc_line_end.py @@ -0,0 +1,77 @@ +# pylint: disable=C0103,C0114,C0115,C0116 +r"""A heredoc ends its own line, wherever it is written (GH #338). + +A heredoc ends at its closing marker, on a line of its own, so whatever comes +next has to start the following line. Inside a list or an object that is the +separator, and `EOF,` closes nothing: the file this library had just written +did not parse, here or in Terraform. + +A top-level attribute survived only because the newline after it comes from +the document rather than from the heredoc. + +The distinction the fix turns on: `HEREDOC_TEMPLATE` matches through the +newline after the marker, so a token that came from the parser already ends +the line. One built by the deserializer does not, and that is the only case +that needs help -- which is why reconstructing a parsed document is byte for +byte what it was. + +Checked against OpenTofu v1.12.5: the emitted list reads back as +`["line1\n", "p"]`. +""" + +from unittest import TestCase + +from hcl2.api import dumps, loads +from hcl2.deserializer import DeserializerOptions +from hcl2.utils import SerializationOptions + +HEREDOCS = DeserializerOptions(strings_to_heredocs=True) +FLAT = SerializationOptions(preserve_heredocs=False) + + +class TestAHeredocInAContainer(TestCase): + def _restore(self, source: str) -> str: + return dumps(loads(source, serialization_options=FLAT), deserializer_options=HEREDOCS) + + def test_in_a_list(self): + written = self._restore('a = [< Date: Wed, 23 Sep 2026 11:12:54 -0700 Subject: [PATCH 10/17] fix: any whitespace around the word makes a line a closing marker The delimiter choice counted only spaces and tabs around `EOF`, but OpenTofu ends a heredoc on the word padded with any whitespace: a non-breaking space, a form feed, a vertical tab, an ideographic space or a next-line character. A body holding such a line was written under `< str: diff --git a/test/unit/test_heredoc_matches_terraform.py b/test/unit/test_heredoc_matches_terraform.py index 901ea299..855dbfb3 100644 --- a/test/unit/test_heredoc_matches_terraform.py +++ b/test/unit/test_heredoc_matches_terraform.py @@ -170,6 +170,16 @@ def test_an_indented_marker_line_counts(self): def test_a_trailing_space_marker_line_counts_because_terraform_ends_there(self): self.assertEqual(self._write('"EOF \\nkeeps\\n"'), "x = < Date: Wed, 23 Sep 2026 11:15:12 -0700 Subject: [PATCH 11/17] fix: a trimmed heredoc in a container ends its own line too The check named the grammar's `HEREDOC_TEMPLATE_TRIM`, but the deserializer builds `HEREDOC_TRIM_TEMPLATE`, so a `<<-` heredoc inside a list or an object still had its separator written on the marker line and the output did not parse. The set is now built from the token classes, with the grammar's name kept beside them. --- CHANGELOG.md | 2 +- hcl2/reconstructor.py | 12 +++++++++++- test/unit/test_heredoc_line_end.py | 12 ++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 271d5e33..0cbd4def 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,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: """Reconstruct a Token node into HCL text fragments.""" diff --git a/test/unit/test_heredoc_line_end.py b/test/unit/test_heredoc_line_end.py index 289a78e3..ba9c8d97 100644 --- a/test/unit/test_heredoc_line_end.py +++ b/test/unit/test_heredoc_line_end.py @@ -58,6 +58,18 @@ def test_the_values_survive(self): loads(restored, serialization_options=FLAT), loads(source, serialization_options=FLAT) ) + def test_a_trimmed_heredoc_in_an_object(self): + # The deserializer names this token `HEREDOC_TRIM_TEMPLATE` while the + # grammar calls it `HEREDOC_TEMPLATE_TRIM`; both have to count. + written = dumps({"x": {"j": '"<<-EOT\n bar\n EOT"'}}) + self.assertNotIn("EOT,", written) + loads(written) + + def test_a_trimmed_heredoc_in_a_list(self): + written = dumps({"x": ['"<<-EOT\n bar\n EOT"', '"p"']}) + self.assertNotIn("EOT,", written) + loads(written) + def test_a_top_level_attribute_still_works(self): written = self._restore("a = < Date: Wed, 23 Sep 2026 11:43:23 -0700 Subject: [PATCH 12/17] fix: bound each ground-truth evaluation and report failures per case `tofu console` ran with no timeout, so a binary waiting on a lock or a prompt hung the run with nothing printed; and the first case it rejected raised out of main, losing the remaining cases and the summary. Each case now times out after a minute, and a rejected or stalled case is reported as ERR and counted toward the exit status. --- bin/heredoc_ground_truth | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/bin/heredoc_ground_truth b/bin/heredoc_ground_truth index 6003ee13..96f462b5 100755 --- a/bin/heredoc_ground_truth +++ b/bin/heredoc_ground_truth @@ -33,6 +33,10 @@ from test.unit.test_heredoc_matches_terraform import CASES # noqa: E402 BINARIES = ("tofu", "terraform") +# Seconds one `console` evaluation may take. A binary waiting on a plugin cache +# lock or a prompt would otherwise hang the run with nothing printed. +TIMEOUT_SECONDS = 60 + def find_binary(): """Return the first Terraform-compatible binary on PATH, or None.""" @@ -61,6 +65,7 @@ def evaluate(binary, source): capture_output=True, text=True, check=False, + timeout=TIMEOUT_SECONDS, ) if result.returncode != 0: raise RuntimeError(result.stderr.strip()) @@ -85,7 +90,18 @@ def main(): print("using %s\n" % binary, file=sys.stderr) mismatches = 0 for source, expected in CASES: - actual = evaluate(binary, source) + # One case the binary rejects or stalls on is a result for that case, + # not a reason to lose the rest of the table and the summary. + try: + actual = evaluate(binary, source) + except subprocess.TimeoutExpired: + mismatches += 1 + print("ERR %r\n %s timed out after %ds" % (source, binary, TIMEOUT_SECONDS)) + continue + except RuntimeError as error: + mismatches += 1 + print("ERR %r\n %s: %s" % (source, binary, error)) + continue if args.print_table: print(" (%r, %r)," % (source, actual)) continue @@ -96,7 +112,7 @@ def main(): print("BAD %r\n expected %r\n %s says %r" % (source, expected, binary, actual)) if args.print_table: - return 0 + return 1 if mismatches else 0 # stdout, so it lands after the per-case lines rather than ahead of them # when the output is piped. From 76a4df931eeaa267fbfe717037ce28d7781e5168 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Sep 2026 11:43:23 -0700 Subject: [PATCH 13/17] docs: describe the closing-marker rule by what Terraform accepts The entry contrasted Terraform with this grammar, which a separate change loosens; stating Terraform's rule alone stays true either way. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0899da3..62eb97e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - A carriage return in a flattened heredoc body is written as `\r` rather than left raw. `preserve_heredocs=False` returns quoted-string *source*, and a quoted string cannot hold a literal carriage return: OpenTofu rejects one with "No closing marker was found for the string". A heredoc read out of a CRLF file therefore flattened to source that would not parse again. The value form (`strip_string_quotes=True`) is unchanged and still hands back real carriage returns. `strings_to_heredocs` resolves `\r` when it writes a body, so the two halves stay each other's inverse: a heredoc interprets no escape, so a body carrying a backslash and an `r` would be those two characters rather than the carriage return the value held. - A `<<-` heredoc whose closing marker is indented with something other than spaces or tabs no longer appends that indentation to the value. The dedent already measured whitespace rather than spaces, matching OpenTofu, but the marker's own indent was stripped as `[ \t]*`, so a body indented with a non-breaking space, a vertical tab, a form feed or an ideographic space came back with one of those characters on the end. Four such cases are now in the table that `bin/heredoc_ground_truth` re-derives from OpenTofu. - `strings_to_heredocs` leaves a value carrying a lone carriage return quoted. A heredoc body is read literally, so it can hold a `\r` only where one ends a line: OpenTofu rejects `< Date: Wed, 23 Sep 2026 11:59:12 -0700 Subject: [PATCH 14/17] fix: a heredoc inside an expression loads as the heredoc it is An argument or operand is expression source, but a heredoc there came back quoted with its markers: upper(< Date: Wed, 23 Sep 2026 12:43:05 -0700 Subject: [PATCH 15/17] fix: measure heredoc whitespace as OpenTofu does, not as Python does The <<- margin and the closing-marker indent used Python's whitespace, which includes the information separators U+001C..U+001F. OpenTofu uses Go's unicode.IsSpace, which does not: it leaves <<-EOT\n a\n\x1c b\n EOT undedented, where this measured a margin and dropped the separator. Five ground-truth cases pin both sides. --- CHANGELOG.md | 2 +- hcl2/rules/strings.py | 16 ++++++++++++---- test/unit/test_heredoc_matches_terraform.py | 9 +++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62eb97e4..d14d8f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,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: r"""Drop the whitespace indenting the closing marker on its own line. @@ -50,7 +57,7 @@ def _strip_closing_marker_indent(text: str) -> str: 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"[^\S\n]*\Z", "", text) + return re.sub(r"[^\S\n\x1c-\x1f]*\Z", "", text) class InterpolationRule(LarkRule): @@ -247,16 +254,17 @@ def _dedent(body: str) -> List[str]: # space-indented input that reading the letter of the spec would cover. margin = sys.maxsize for line in lines: - if not line.strip(): + indent = _INDENT.match(line).end() # type: ignore[union-attr] + if indent == len(line): continue - margin = min(margin, len(line) - len(line.lstrip())) + margin = min(margin, indent) 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] + return [line if _INDENT.fullmatch(line) else line[margin:] for line in lines] class TemplateStringRule(LarkRule): diff --git a/test/unit/test_heredoc_matches_terraform.py b/test/unit/test_heredoc_matches_terraform.py index 855dbfb3..d47a4694 100644 --- a/test/unit/test_heredoc_matches_terraform.py +++ b/test/unit/test_heredoc_matches_terraform.py @@ -60,6 +60,15 @@ ("<<-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"), + # "Whitespace" is Go's `unicode.IsSpace`, which is Python's `str.isspace` + # less U+001C..U+001F: Python counts those information separators and Go + # does not. A line led by one has no indent, so the margin is zero and + # nothing is dedented -- or stripped from that line. + ("<<-EOT\n a\n\x1c\x1c b\n EOT", " a\n\x1c\x1c b\n"), + ("<<-EOT\n a\n\x1f b\n EOT", " a\n\x1f b\n"), + ("<<-EOT\n a\n\x1c\n EOT", " a\n\x1c\n"), + ("<<-EOT\n a\n\u2028 b\n EOT", " a\nb\n"), + ("<<-EOT\n a\n\u0085 b\n EOT", " a\nb\n"), ] From d58b5fd073f077037094991a0ade514d100790e1 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Sep 2026 12:43:05 -0700 Subject: [PATCH 16/17] chore: report ground-truth results through logging Results go to stdout and diagnostics to stderr as before, with the same text and exit codes. --- bin/heredoc_ground_truth | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/bin/heredoc_ground_truth b/bin/heredoc_ground_truth index 96f462b5..19958512 100755 --- a/bin/heredoc_ground_truth +++ b/bin/heredoc_ground_truth @@ -21,6 +21,7 @@ Requires `tofu` or `terraform` on PATH. import argparse import json +import logging import os import shutil import subprocess @@ -33,6 +34,11 @@ from test.unit.test_heredoc_matches_terraform import CASES # noqa: E402 BINARIES = ("tofu", "terraform") +# Results go to stdout and diagnostics to stderr, so a piped run keeps the +# table and its summary together while the binary in use stays visible. +REPORT = logging.getLogger("heredoc_ground_truth.report") +DIAGNOSTICS = logging.getLogger("heredoc_ground_truth.diagnostics") + # Seconds one `console` evaluation may take. A binary waiting on a plugin cache # lock or a prompt would otherwise hang the run with nothing printed. TIMEOUT_SECONDS = 60 @@ -72,6 +78,16 @@ def evaluate(binary, source): return json.loads(json.loads(result.stdout.strip().splitlines()[-1])) +def configure_logging(): + """Send each logger to its stream as bare messages, the lines the table is made of.""" + for logger, stream in ((REPORT, sys.stdout), (DIAGNOSTICS, sys.stderr)): + handler = logging.StreamHandler(stream) + handler.setFormatter(logging.Formatter("%(message)s")) + logger.addHandler(handler) + logger.setLevel(logging.INFO) + logger.propagate = False + + def main(): parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) parser.add_argument( @@ -82,12 +98,13 @@ def main(): ) args = parser.parse_args() + configure_logging() binary = find_binary() if binary is None: - print("neither `tofu` nor `terraform` is on PATH", file=sys.stderr) + DIAGNOSTICS.error("neither `tofu` nor `terraform` is on PATH") return 2 - print("using %s\n" % binary, file=sys.stderr) + DIAGNOSTICS.info("using %s\n", binary) mismatches = 0 for source, expected in CASES: # One case the binary rejects or stalls on is a result for that case, @@ -96,27 +113,27 @@ def main(): actual = evaluate(binary, source) except subprocess.TimeoutExpired: mismatches += 1 - print("ERR %r\n %s timed out after %ds" % (source, binary, TIMEOUT_SECONDS)) + REPORT.error("ERR %r\n %s timed out after %ds", source, binary, TIMEOUT_SECONDS) continue except RuntimeError as error: mismatches += 1 - print("ERR %r\n %s: %s" % (source, binary, error)) + REPORT.error("ERR %r\n %s: %s", source, binary, error) continue if args.print_table: - print(" (%r, %r)," % (source, actual)) + REPORT.info(" (%r, %r),", source, actual) continue if actual == expected: - print("ok %r" % source) + REPORT.info("ok %r", source) else: mismatches += 1 - print("BAD %r\n expected %r\n %s says %r" % (source, expected, binary, actual)) + REPORT.warning("BAD %r\n expected %r\n %s says %r", source, expected, binary, actual) if args.print_table: return 1 if mismatches else 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))) + REPORT.info("\n%d of %d cases disagree", mismatches, len(CASES)) return 1 if mismatches else 0 From 4f0756c43928c1d07db8dd531ef5fabb6dbd1ce2 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Sep 2026 12:48:01 -0700 Subject: [PATCH 17/17] fix: build heredoc tokens in the parser's shape, and end object items with them A heredoc built from a dict used its own terminal name for <<- and dropped the newline after the closing marker, so each consumer needed a case for both shapes; the reconstructor had one, and the inline serializer and wrap_tuples/wrap_objects did not. Built tokens now carry the grammar's name and end their line, which makes the reconstructor's special case dead; it is removed, and the formatter drops the break it would otherwise add after one. In an object the heredoc's line break is the separator. OpenTofu rejects a comma at the start of the next line, which is where the previous fix put it, so none is written there: not by the formatter, and not in the inline form used inside an expression. --- CHANGELOG.md | 2 +- hcl2/deserializer.py | 12 +++++- hcl2/formatter.py | 69 ++++++++++++++++++++++++++++-- hcl2/reconstructor.py | 39 +---------------- hcl2/rules/containers.py | 17 +++++--- hcl2/rules/tokens.py | 4 +- test/unit/test_deserializer.py | 8 +++- test/unit/test_heredoc_line_end.py | 62 +++++++++++++++++++++++++++ 8 files changed, 161 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78fe4ae8..1f714680 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,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: + """*heredoc* ending with the newline after its closing marker, as parsed.""" + return heredoc if heredoc.endswith("\n") else heredoc + "\n" + + def _heredoc_delimiter(content: str) -> str: """Return a delimiter the body does not close on its own. @@ -336,6 +341,11 @@ def _deserialize_string_part(self, value: str) -> StringPartRule: def _deserialize_heredoc( self, value: str, trim: bool ) -> Union[HeredocTemplateRule, HeredocTrimTemplateRule]: + # A parsed heredoc token runs through the newline after its closing + # marker, so it ends its own line wherever it is written. The dict form + # drops that newline; putting it back makes a built token the same + # shape as a parsed one, and whatever follows starts the next line. + value = _end_line(value) if trim: return HeredocTrimTemplateRule([HEREDOC_TRIM_TEMPLATE(value)]) return HeredocTemplateRule([HEREDOC_TEMPLATE(value)]) @@ -343,7 +353,7 @@ def _deserialize_heredoc( def _deserialize_string_as_heredoc(self, content: str) -> HeredocTemplateRule: """Wrap an unescaped body, already newline-terminated, in heredoc syntax.""" delimiter = _heredoc_delimiter(content) - heredoc = f"<<{delimiter}\n{content}{delimiter}" + heredoc = f"<<{delimiter}\n{content}{delimiter}\n" return HeredocTemplateRule([HEREDOC_TEMPLATE(heredoc)]) def _deserialize_expression(self, value: str) -> ExprTermRule: diff --git a/hcl2/formatter.py b/hcl2/formatter.py index e30cc1a7..2218c85f 100644 --- a/hcl2/formatter.py +++ b/hcl2/formatter.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from typing import List, Optional -from hcl2.rules.abstract import LarkElement, LarkRule +from hcl2.rules.abstract import LarkElement, LarkRule, LarkToken from hcl2.rules.base import ( AttributeRule, BlockRule, @@ -25,8 +25,17 @@ ForTupleExprRule, ) from hcl2.rules.functions import FunctionCallRule -from hcl2.rules.tokens import COLON, COMMA, LBRACE, LSQB, NL_OR_COMMENT +from hcl2.rules.tokens import ( + COLON, + COMMA, + HEREDOC_TEMPLATE, + HEREDOC_TRIM_TEMPLATE, + LBRACE, + LSQB, + NL_OR_COMMENT, +) from hcl2.rules.whitespace import NewLineOrCommentRule +from hcl2.walk import walk @dataclass @@ -73,6 +82,27 @@ def format_tree(self, tree: LarkElement): """Apply formatting to the given LarkElement tree in place.""" if isinstance(tree, StartRule): self.format_start_rule(tree) + self._join_lines_after_heredocs(tree) + + @staticmethod + def _join_lines_after_heredocs(tree: LarkElement): + """Drop the line break the formatter added right after a heredoc. + + A heredoc token runs through the newline after its closing marker, so + the line is already ended; the break every attribute and element gets + would leave a blank line behind, which the parsed document never had. + """ + previous = None + for node in walk(tree): + if not isinstance(node, LarkToken): + continue + if ( + _is_line_ending_heredoc(previous) + and isinstance(node, NL_OR_COMMENT) # type: ignore[misc] + and str(node.value).startswith("\n") + ): + node.set_value(str(node.value)[1:]) + previous = node def format_start_rule(self, rule: StartRule): """Format the top-level start rule.""" @@ -150,8 +180,9 @@ def format_object_rule(self, rule: ObjectRule, indent_level: int = 0): return new_children = [] - for i, child in enumerate(rule.children): - next_child = rule.children[i + 1] if i + 1 < len(rule.children) else None + children = self._without_commas_after_heredocs(rule.children) + for i, child in enumerate(children): + next_child = children[i + 1] if i + 1 < len(children) else None new_children.append(child) if isinstance(child, LBRACE): # type: ignore[misc] @@ -175,6 +206,21 @@ def format_object_rule(self, rule: ObjectRule, indent_level: int = 0): if self.options.vertically_align_object_elements: self._vertically_align_object_elems(rule) + @staticmethod + def _without_commas_after_heredocs(children: List[LarkElement]) -> List[LarkElement]: + """Drop the comma after an object element that ends with a heredoc. + + The heredoc ends its line, and the line break is what separates object + elements: OpenTofu rejects a comma at the start of the next line with + "Invalid expression". A tuple takes one there, so only objects need it. + """ + result: List[LarkElement] = [] + for child in children: + if isinstance(child, COMMA) and result and _ends_with_heredoc(result[-1]): # type: ignore[misc] + continue + result.append(child) + return result + def format_expression(self, rule: ExprTermRule, indent_level: int = 0): """Dispatch formatting for the inner expression of an ExprTermRule.""" if isinstance(rule.expression, ObjectRule): @@ -321,3 +367,18 @@ def _deindent_last_line(self, times: int = 1): for _ in range(times): if token.value.endswith(" " * self.options.indent_length): token.set_value(token.value[: -self.options.indent_length]) + + +def _is_line_ending_heredoc(node: object) -> bool: + """Whether *node* is a heredoc token running through its closing line.""" + heredoc_types = (HEREDOC_TEMPLATE, HEREDOC_TRIM_TEMPLATE) + return isinstance(node, heredoc_types) and str(node.value).endswith("\n") # type: ignore[arg-type] + + +def _ends_with_heredoc(node: LarkElement) -> bool: + """Whether the last token under *node* is a heredoc that ends its line.""" + last = None + for element in walk(node): + if isinstance(element, LarkToken): + last = element + return _is_line_ending_heredoc(last) diff --git a/hcl2/reconstructor.py b/hcl2/reconstructor.py index 989a295c..166e6c58 100644 --- a/hcl2/reconstructor.py +++ b/hcl2/reconstructor.py @@ -66,7 +66,6 @@ def _reset_state(self): self._last_was_space = True self._current_indent = 0 self._last_token_name = None - self._last_token_ended_line = False self._last_rule_name = None # pylint:disable=R0911,R0912 @@ -289,54 +288,18 @@ def _reconstruct_tree(self, tree: Tree, parent_rule_name: Optional[str] = None) return result - # The grammar calls the trimmed terminal `HEREDOC_TEMPLATE_TRIM` and the - # deserializer builds `HEREDOC_TRIM_TEMPLATE`. A parsed token already ends - # its line, so it is the deserializer's name that decides the output, and - # naming only the grammar's left every `<<-` heredoc it built unhelped. - _heredoc_token_names = frozenset( - { - tokens.HEREDOC_TEMPLATE.lark_name(), - tokens.HEREDOC_TRIM_TEMPLATE.lark_name(), - "HEREDOC_TEMPLATE_TRIM", - } - ) - def _reconstruct_token(self, token: Token, parent_rule_name: Optional[str] = None) -> str: """Reconstruct a Token node into HCL text fragments.""" result = str(token.value) - if self._needs_line_after_heredoc(token): - # A heredoc ends at its closing marker, on a line of its own. Any - # token that follows has to start the next line -- inside a list or - # an object that token is the separator, and `EOF,` closes nothing, - # so the file did not parse. A top-level attribute survived only - # because the newline it is followed by comes from the document. - result = "\n" + result - elif self._should_add_space_before(token, parent_rule_name): + if self._should_add_space_before(token, parent_rule_name): result = " " + result self._last_token_name = token.type - self._last_token_ended_line = str(token.value).endswith(("\n", "\r\n")) if len(token) != 0: self._last_was_space = result[-1].endswith(" ") or result[-1].endswith("\n") return result - def _needs_line_after_heredoc(self, token: Token) -> bool: - """Whether *token* has to start a new line because a heredoc just ended. - - Only for a heredoc that does not carry its own. `HEREDOC_TEMPLATE` - matches through the newline after the closing marker, so a token that - came from the parser already ends the line; one built by the - deserializer does not, and that is the case where the separator landed - on the marker's line. - """ - if self._last_token_name not in self._heredoc_token_names: - return False - if self._last_token_ended_line: - return False - # Anything that already begins one is fine as it is. - return not str(token.value).startswith(("\n", "\r\n")) - def _reconstruct_node( self, node: Union[Tree, Token], parent_rule_name: Optional[str] = None ) -> List[str]: diff --git a/hcl2/rules/containers.py b/hcl2/rules/containers.py index 8b811ce8..86862823 100644 --- a/hcl2/rules/containers.py +++ b/hcl2/rules/containers.py @@ -195,13 +195,20 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext return dict_result with context.modify(inside_dollar_string=True): - str_result = "{" - str_result += ", ".join( - f"{element.key.serialize(options, context)}" - f" = " + items = [ + f"{element.key.serialize(options, context)} = " f"{element.expression.serialize(options, context)}" for element in self.elements - ) + ] + # An item that ends its line -- a heredoc does, through the newline + # after its closing marker -- is already separated from the next: + # OpenTofu takes a line break between object items and rejects a + # comma at the start of a line with "Invalid expression". + str_result = "{" + for index, item in enumerate(items): + if index and not items[index - 1].endswith("\n"): + str_result += ", " + str_result += item str_result += "}" if not context.inside_dollar_string: diff --git a/hcl2/rules/tokens.py b/hcl2/rules/tokens.py index 6fa0f215..6ea809d2 100644 --- a/hcl2/rules/tokens.py +++ b/hcl2/rules/tokens.py @@ -93,7 +93,9 @@ def serialize_conversion(self) -> Callable[[Any], str]: TEMPLATE_STRING = StringToken["TEMPLATE_STRING"] # type: ignore BINARY_OP = StringToken["BINARY_OP"] # type: ignore HEREDOC_TEMPLATE = StringToken["HEREDOC_TEMPLATE"] # type: ignore -HEREDOC_TRIM_TEMPLATE = StringToken["HEREDOC_TRIM_TEMPLATE"] # type: ignore +# The grammar's name: a token the parser reads and one the deserializer builds +# are then the same class, and nothing downstream has to know both spellings. +HEREDOC_TRIM_TEMPLATE = StringToken["HEREDOC_TEMPLATE_TRIM"] # type: ignore NL_OR_COMMENT = StringToken["NL_OR_COMMENT"] # type: ignore # static values EQ = StaticStringToken[("EQ", "=")] # type: ignore diff --git a/test/unit/test_deserializer.py b/test/unit/test_deserializer.py index bf674f5c..942609c7 100644 --- a/test/unit/test_deserializer.py +++ b/test/unit/test_deserializer.py @@ -204,11 +204,15 @@ def test_strings_to_heredocs_with_newline(self): self.assertIsInstance(result, HeredocTemplateRule) def test_strings_to_heredocs_body_is_not_given_an_extra_line(self): - """The value's own trailing newline is the one before the marker.""" + """The value's own trailing newline is the one before the marker. + + The newline after the marker is the token's own, as it is for a parsed + heredoc: the token ends its line wherever it is written. + """ opts = DeserializerOptions(strings_to_heredocs=True) d = _deser(opts) result = d._deserialize_text('"line1\\nline2\\n"') - self.assertEqual(result.heredoc.value, "<