diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b0148ee..992ecc9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Restore `py.typed` marker so type checkers recognize `hcl2` (and `cli`) as typed packages. ([#298](https://github.com/amplify-education/python-hcl2/issues/298)) +- `strip_string_quotes` no longer unquotes string literals nested inside expressions, which produced invalid HCL such as `${upper(x)}` from `upper("x")`. ([#310](https://github.com/amplify-education/python-hcl2/issues/310)) +- `strip_string_quotes` now resolves escape sequences, so the values it yields match what the option documents. ([#308](https://github.com/amplify-education/python-hcl2/issues/308)) ## \[8.1.2\] - 2026-04-10 diff --git a/hcl2/rules/strings.py b/hcl2/rules/strings.py index 4be161cc..e6bec8fb 100644 --- a/hcl2/rules/strings.py +++ b/hcl2/rules/strings.py @@ -21,6 +21,7 @@ HEREDOC_TRIM_PATTERN, SerializationContext, SerializationOptions, + process_escape_sequences, to_dollar_string, ) @@ -92,12 +93,35 @@ def string_parts(self): return self.children[1:-1] def serialize(self, options=SerializationOptions(), context=SerializationContext()) -> Any: - """Serialize to a quoted string.""" + """Serialize to a quoted string. + + `strip_string_quotes` asks for the string's value rather than its + source form, so it applies only where a value is what the caller gets: + a string nested inside an expression is part of that expression's text, + and unquoting it there would produce something that is no longer valid + HCL (`upper("x")` becoming `upper(x)`). + """ + if options.strip_string_quotes and not context.inside_dollar_string: + return "".join( + self._serialize_part_as_value(part, options, context) for part in self.string_parts + ) + inner = "".join(part.serialize(options, context) for part in self.string_parts) - if options.strip_string_quotes: - return inner return '"' + inner + '"' + @staticmethod + def _serialize_part_as_value(part, options, context) -> str: + """Serialize one part, resolving escapes in literal text only. + + 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. + """ + serialized = part.serialize(options, context) + if isinstance(part.content, STRING_CHARS): + return process_escape_sequences(serialized) + return serialized + class HeredocTemplateRule(LarkRule): """Rule for heredoc template strings (< Optional[Tuple[str, int]]: + """Decode a \\uNNNN or \\UNNNNNNNN escape whose marker sits at `index`.""" + width = _UNICODE_ESCAPE_WIDTHS[text[index]] + digits = text[index + 1 : index + 1 + width] + if len(digits) != width or any(char not in _HEX_DIGITS for char in digits): + return None + return chr(int(digits, 16)), index + 1 + width + + +def process_escape_sequences(value: str) -> str: + """Resolve the escape sequences HCL defines inside a quoted template. + + Used when `strip_string_quotes` is set, which asks for the *value* of a + string rather than its source form. Escapes are resolved in a single pass, + so an escaped backslash cannot combine with the character after it: `\\\\n` + is a backslash followed by "n", not a newline. + + An unrecognized escape is preserved verbatim, backslash included. Terraform + rejects those outright, but the grammar here accepts them, and a serializer + is the wrong place to raise an error the parser did not. + """ + if "\\" not in value: + return value + + parts = [] + index = 0 + length = len(value) + while index < length: + char = value[index] + if char != "\\" or index + 1 >= length: + parts.append(char) + index += 1 + continue + + marker = value[index + 1] + if marker in _SIMPLE_ESCAPES: + parts.append(_SIMPLE_ESCAPES[marker]) + index += 2 + continue + if marker in _UNICODE_ESCAPE_WIDTHS: + decoded = _decode_unicode_escape(value, index + 1) + if decoded is not None: + parts.append(decoded[0]) + index = decoded[1] + continue + + parts.append(char) + parts.append(marker) + index += 2 + + return "".join(parts) + + @dataclass class SerializationContext: """Mutable state tracked during serialization traversal.""" diff --git a/test/unit/rules/test_strings.py b/test/unit/rules/test_strings.py index d5eac752..ce65aac3 100644 --- a/test/unit/rules/test_strings.py +++ b/test/unit/rules/test_strings.py @@ -178,6 +178,23 @@ def test_serialize_strip_string_quotes_with_interpolation(self): opts = SerializationOptions(strip_string_quotes=True) self.assertEqual(rule.serialize(opts), "prefix:${var.name}") + def test_serialize_strip_string_quotes_inside_expression_keeps_quotes(self): + """A string nested in an expression is that expression's source text.""" + rule = _make_string([_make_string_part_chars("x")]) + opts = SerializationOptions(strip_string_quotes=True) + ctx = SerializationContext(inside_dollar_string=True) + self.assertEqual(rule.serialize(opts, ctx), '"x"') + + def test_serialize_strip_string_quotes_resolves_escapes(self): + rule = _make_string([_make_string_part_chars(r"say \"hi\"")]) + opts = SerializationOptions(strip_string_quotes=True) + self.assertEqual(rule.serialize(opts), 'say "hi"') + + def test_serialize_without_strip_keeps_escapes_raw(self): + """Default output stays source-shaped so it can be reconstructed.""" + rule = _make_string([_make_string_part_chars(r"say \"hi\"")]) + self.assertEqual(rule.serialize(), r'"say \"hi\""') + # --- HeredocTemplateRule tests --- diff --git a/test/unit/test_api.py b/test/unit/test_api.py index 6af029a5..5ab9a3eb 100644 --- a/test/unit/test_api.py +++ b/test/unit/test_api.py @@ -288,3 +288,75 @@ def test_query_file_object(self): self.assertIsInstance(result, DocumentView) attr = result.attribute("x") self.assertIsNotNone(attr) + + +class TestStripStringQuotes(TestCase): + """`strip_string_quotes=True` asks for values, not source text. + + It is the documented v7-compatibility path, so it has to yield what v7 + yielded: quotes removed from string *values*, escape sequences resolved, + and expressions left as valid HCL. + """ + + _OPTIONS = SerializationOptions(strip_string_quotes=True) + + def _load(self, source): + return loads(source, serialization_options=self._OPTIONS) + + def test_plain_value_loses_its_quotes(self): + self.assertEqual(self._load('a = "plain"\n'), {"a": "plain"}) + + def test_value_in_object_and_list(self): + self.assertEqual( + self._load('a = { k = "v" }\nb = ["x"]\n'), + {"a": {"k": "v"}, "b": ["x"]}, + ) + + def test_function_argument_keeps_its_quotes(self): + """Unquoting here would turn a string into an identifier.""" + self.assertEqual(self._load('a = upper("x")\n'), {"a": '${upper("x")}'}) + + def test_conditional_branches_keep_their_quotes(self): + self.assertEqual( + self._load('a = var.x ? "yes" : "no"\n'), + {"a": '${var.x ? "yes" : "no"}'}, + ) + + def test_comparison_against_a_string_keeps_its_quotes(self): + """An unquoted empty string would leave `s != ` behind.""" + self.assertEqual( + self._load('a = [for s in var.l : upper(s) if s != ""]\n'), + {"a": '${[for s in var.l : upper(s) if s != ""]}'}, + ) + + def test_nested_call_keeps_every_quote(self): + self.assertEqual( + self._load('a = join(",", ["x", "y"])\n'), + {"a": '${join(",", ["x", "y"])}'}, + ) + + def test_escaped_quote_is_resolved(self): + self.assertEqual(self._load(r'a = "quote \"in\" here"' + "\n"), {"a": 'quote "in" here'}) + + def test_escaped_whitespace_is_resolved(self): + self.assertEqual(self._load(r'a = "x\ny\tz"' + "\n"), {"a": "x\ny\tz"}) + + def test_escaped_backslash_is_resolved(self): + self.assertEqual(self._load(r'a = "back\\slash"' + "\n"), {"a": "back\\slash"}) + + def test_escaped_backslash_does_not_combine_with_the_next_character(self): + r"""`\\n` is a backslash followed by "n", not a newline.""" + self.assertEqual(self._load(r'a = "back\\nslash"' + "\n"), {"a": "back\\nslash"}) + + def test_unknown_escape_is_preserved(self): + self.assertEqual(self._load(r'a = "keep \q intact"' + "\n"), {"a": r"keep \q intact"}) + + 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_default_options_still_preserve_source_form(self): + """Without the option, the source form is kept for reconstruction.""" + self.assertEqual(loads(r'a = "line1\nline2"' + "\n"), {"a": r'"line1\nline2"'}) diff --git a/test/unit/test_utils.py b/test/unit/test_utils.py index ab5335c5..5dc04d13 100644 --- a/test/unit/test_utils.py +++ b/test/unit/test_utils.py @@ -5,6 +5,7 @@ SerializationContext, SerializationOptions, is_dollar_string, + process_escape_sequences, to_dollar_string, unwrap_dollar_string, wrap_into_parentheses, @@ -140,3 +141,41 @@ def test_expression_string(self): def test_dollar_expression(self): self.assertEqual(wrap_into_parentheses("${a + b}"), "${(a + b)}") + + +class TestProcessEscapeSequences(TestCase): + """Escape resolution used by `strip_string_quotes`.""" + + def test_no_backslash_is_returned_unchanged(self): + self.assertEqual(process_escape_sequences("plain text"), "plain text") + + def test_simple_escapes(self): + self.assertEqual(process_escape_sequences(r"a\nb\tc\rd"), "a\nb\tc\rd") + + def test_escaped_quote(self): + self.assertEqual(process_escape_sequences(r"say \"hi\""), 'say "hi"') + + def test_escaped_backslash(self): + self.assertEqual(process_escape_sequences(r"back\\slash"), "back\\slash") + + def test_escaped_backslash_is_not_reused_by_the_next_character(self): + r"""A single pass: `\\n` cannot become a newline.""" + self.assertEqual(process_escape_sequences(r"back\\nslash"), "back\\nslash") + + def test_unicode_escape(self): + self.assertEqual(process_escape_sequences(r"café"), "café") + + def test_long_unicode_escape(self): + self.assertEqual(process_escape_sequences(r"\U0001F600"), "\U0001f600") + + def test_malformed_unicode_escape_is_preserved(self): + self.assertEqual(process_escape_sequences(r"\u12"), r"\u12") + + def test_non_hex_unicode_escape_is_preserved(self): + self.assertEqual(process_escape_sequences(r"\uZZZZ"), r"\uZZZZ") + + def test_unknown_escape_is_preserved(self): + self.assertEqual(process_escape_sequences(r"keep \q intact"), r"keep \q intact") + + def test_trailing_backslash_is_preserved(self): + self.assertEqual(process_escape_sequences("trailing\\"), "trailing\\")