Skip to content

Fix heredoc bodies losing trailing blank lines/spaces to a greedy rstrip - #318

Merged
kkozik-amplify merged 5 commits into
amplify-education:mainfrom
agu2347:fix-heredoc-trailing-whitespace-issue-316
Aug 24, 2026
Merged

Fix heredoc bodies losing trailing blank lines/spaces to a greedy rstrip#318
kkozik-amplify merged 5 commits into
amplify-education:mainfrom
agu2347:fix-heredoc-trailing-whitespace-issue-316

Conversation

@agu2347

@agu2347 agu2347 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Problem

Heredoc bodies (with preserve_heredocs=False, the flattening path) are right-stripped of all trailing newlines, tabs and spaces, so distinct sources collapse to the same value:

>>> from hcl2.utils import SerializationOptions
>>> opts = SerializationOptions(preserve_heredocs=False)
>>> hcl2.loads('a = <<MARKER\nx\n\nMARKER\n', serialization_options=opts)   # trailing blank line
{'a': '"x"'}
>>> hcl2.loads('a = <<MARKER\nx   \nMARKER\n', serialization_options=opts)  # trailing spaces
{'a': '"x"'}

Both collapse to the same "x" as a plain one-line heredoc, silently discarding a trailing blank line or trailing spaces that are part of the body per the HCL spec.

Fixes #316.

Root cause

hcl2/rules/strings.py, HeredocTemplateRule.serialize's preserve_heredocs=False branch:

heredoc = match.group(2).rstrip(self._trim_chars)  # _trim_chars = "\n\t "

rstrip with a character set removes every trailing character in that set, not just the single newline that separates the last body line from the closing marker.

Fix

Added _strip_single_trailing_newline(), which removes exactly one trailing \n (matching the established one-line-heredoc behavior every existing golden test depends on) and nothing more. Used it in HeredocTemplateRule's flattening path.

Scoped to the plain <<MARKER heredoc only, matching every reproduction case in the issue. I initially also touched the indented <<-MARKER (HeredocTrimTemplateRule) variant, but that broke test_parse_flattens_heredocs -- its rstrip call also throws away the closing marker line's own indentation (not body content), which is unrelated to this bug and has separate, pre-existing golden-test semantics. Left that variant untouched.

Testing

Added test/unit/test_heredoc_trailing_whitespace.py: a trailing blank line, two trailing blank lines, and trailing spaces now each produce distinct, non-collapsed values; a plain one-liner and an interior blank line (both already correct) are confirmed unaffected.

Confirmed each new test fails against the pre-fix code with the exact reported collapse-to-"x" behavior, and passes after the fix. Ran the full existing suite (1391 tests): all pass, zero regressions.

HeredocTemplateRule.serialize's preserve_heredocs=False path did
match.group(2).rstrip("\n\t "), which removes every trailing newline,
tab, and space character -- not just the single newline separating the
last body line from the closing marker. A trailing blank line, multiple
trailing blank lines, and trailing spaces on the last line all collapsed
to the same flattened value as a plain one-line body.

Replace the blanket rstrip with a helper that removes exactly the one
trailing newline that separates content from the marker line, per the
issue's proposed scope. This is limited to the plain <<MARKER heredoc;
the indented <<-MARKER variant has separate, pre-existing
leading-whitespace-trim semantics with its own golden tests, which are
unaffected by this change (verified against the full suite).

Fixes amplify-education#316
@agu2347
agu2347 requested a review from a team as a code owner August 24, 2026 15:09
kkozik-amplify and others added 2 commits August 24, 2026 17:49
…r line out

The blanket rstrip was doing two jobs: dropping the newline that separates
the last body line from the closing marker, and dropping that marker
line's own indentation. Removing only the newline leaves the indentation
behind, so an indented closing marker leaked into the body:

    <<MARKER\nx\n   MARKER   ->  "x\n   "   (was "x")

No existing test flattens a heredoc with an indented closing marker, so
the suite stayed green. The spec is explicit that the marker "may also
have an arbitrary number of spaces preceding it on its line", so that
whitespace is not content.

Splitting the two concerns in _strip_closing_marker_line also removes the
reason to skip <<-MARKER, which still collapsed trailing blank lines and
trailing spaces exactly as amplify-education#316 describes. Its rstrip was the same
double-duty call, which is why swapping in a newline-only strip broke the
dedent; handling the marker line separately does not.

Fixing <<- exposed a second bug that the old rstrip had been masking: a
blank line has zero leading spaces, so once blank lines survive they drag
min_spaces to zero and cancel the dedent for every other line. The spec
measures "any literal string at the start of each line" -- a blank line
has none -- so blank lines are now skipped when measuring.

Tests cover the indented closing marker (spaces and tab, with and without
a trailing blank line) and the <<- variant throughout. The module
docstring is now raw; its `rstrip("\n\t ")` reference was being read as
real control characters.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Heredocs parse in CRLF files once amplify-education#317 lands, so the newline before the
closing marker may be \r\n. Stripping only \n leaves the \r stranded on
the end of the value.

Inert without amplify-education#317 -- no CRLF heredoc parses today -- but it belongs with
this helper rather than with the grammar change that exposes it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
amplify-education#312, amplify-education#311 and amplify-education#313 have landed. Only CHANGELOG.md conflicted, resolved
with the landed entries first and this branch's appended.

hcl2/rules/strings.py auto-merged against amplify-education#313, which rewrote the same
file's string-serialization path. Verified the two coexist: amplify-education#313's
process_escape_sequences and lark_name() dispatch sit alongside this
branch's _strip_closing_marker_line, and the full suite passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
amplify-education#317 has landed since the last update. Only CHANGELOG.md conflicted,
resolved with the landed entries first and this branch's appended.

hcl2/rules/strings.py auto-merged: amplify-education#317's `_trim_chars = "\r\n\t "` for
the preserve path now sits alongside this branch's CRLF-aware
`_strip_closing_marker_line` for the flatten paths. Verified rather than
assumed -- CRLF heredocs flatten and preserve correctly, empty CRLF
heredocs parse, and this branch's own cases (trailing blank line,
trailing spaces, indented closing marker, blank line not cancelling the
`<<-` dedent) all still hold. Full suite: 1504 passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Heredoc bodies lose trailing blank lines and spaces to a greedy rstrip

2 participants