diff --git a/CHANGELOG.md b/CHANGELOG.md index 61dc143a..eef40378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## \[Unreleased\] -- Nothing yet. +### 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 (`<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: + """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 _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 \\. + + 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: @@ -71,8 +140,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 + # 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 -- `< 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.""" + delimiter = _heredoc_delimiter(content) + heredoc = f"<<{delimiter}\n{content}{delimiter}" return HeredocTemplateRule([HEREDOC_TEMPLATE(heredoc)]) def _deserialize_expression(self, value: str) -> ExprTermRule: diff --git a/hcl2/reconstructor.py b/hcl2/reconstructor.py index 166e6c58..5665dd09 100644 --- a/hcl2/reconstructor.py +++ b/hcl2/reconstructor.py @@ -66,6 +66,7 @@ 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 @@ -288,18 +289,44 @@ def _reconstruct_tree(self, tree: Tree, parent_rule_name: Optional[str] = None) return result + _heredoc_token_names = frozenset({"HEREDOC_TEMPLATE", "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._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/hcl2/rules/strings.py b/hcl2/rules/strings.py index 76ce0660..1e559959 100644 --- a/hcl2/rules/strings.py +++ b/hcl2/rules/strings.py @@ -27,23 +27,30 @@ ) -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: ``<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) @@ -197,49 +209,54 @@ def lark_name() -> 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('"', '\\"').replace("\r", "\\r") 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 = <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 = < 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 = < str: + return dumps(loads(source, serialization_options=FLAT), deserializer_options=HEREDOCS) + + def test_in_a_list(self): + written = self._restore('a = [< 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 = <