From 1d35233a03c7420fa57ed55cbd520e0dc4bc892b Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 20 Feb 2019 12:53:12 -0800 Subject: [PATCH] configs/configupgrade: Fix up internal HIL conversion functions HIL implemented its type conversions by rewriting its AST to include calls to some undocumented builtin functions. Unfortunately those functions were still explicitly callable if you could figure out the name for them, and so they may have been used in the wild. In particular, __builtin_StringToFloat was used as part of a workaround for a HIL design flaw where it would prefer to convert strings to integers rather than floats when performing arithmetic operations. This issue was, indeed, the main reason for unifying int ant float into a single number type in HCL. Since we published that as a suggested workaround, the upgrade tool ought to fix it up. The other cases have never been documented as a workaround, so they are less likely to appear in the wild, but we might as well fix them up anyway since we already have the conversion functions required to get the same result in the new language. To be safe/conservative, most of these convert to _two_ function calls rather than just one, which ensures that these new expressions retain the behavior of implicitly converting to the source type before running the conversion. The new conversion functions only specify target type, and so cannot guarantee identical results if the argument type does not exactly match what was previously given as the parameter type in HIL. --- .../funcs-replaced/input/funcs-replaced.tf | 10 +++++ .../funcs-replaced/want/funcs-replaced.tf | 10 +++++ configs/configupgrade/upgrade_expr.go | 45 ++++++++++++++++++- 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/configs/configupgrade/test-fixtures/valid/funcs-replaced/input/funcs-replaced.tf b/configs/configupgrade/test-fixtures/valid/funcs-replaced/input/funcs-replaced.tf index 2cea2f8ba8..9f7336f697 100644 --- a/configs/configupgrade/test-fixtures/valid/funcs-replaced/input/funcs-replaced.tf +++ b/configs/configupgrade/test-fixtures/valid/funcs-replaced/input/funcs-replaced.tf @@ -13,4 +13,14 @@ locals { lookup_literal = "${lookup(map("a", "b"), "a")}" lookup_ref = "${lookup(local.map, "a")}" + + # Undocumented HIL implementation details that some users nonetheless relied on. + conv_bool_to_string = "${__builtin_BoolToString(true)}" + conv_float_to_int = "${__builtin_FloatToInt(1.5)}" + conv_float_to_string = "${__builtin_FloatToString(1.5)}" + conv_int_to_float = "${__builtin_IntToFloat(1)}" + conv_int_to_string = "${__builtin_IntToString(1)}" + conv_string_to_int = "${__builtin_StringToInt("1")}" + conv_string_to_float = "${__builtin_StringToFloat("1.5")}" + conv_string_to_bool = "${__builtin_StringToBool("true")}" } diff --git a/configs/configupgrade/test-fixtures/valid/funcs-replaced/want/funcs-replaced.tf b/configs/configupgrade/test-fixtures/valid/funcs-replaced/want/funcs-replaced.tf index 927ed5e47d..1664ee9b4f 100644 --- a/configs/configupgrade/test-fixtures/valid/funcs-replaced/want/funcs-replaced.tf +++ b/configs/configupgrade/test-fixtures/valid/funcs-replaced/want/funcs-replaced.tf @@ -31,4 +31,14 @@ locals { "a" = "b" }["a"] lookup_ref = local.map["a"] + + # Undocumented HIL implementation details that some users nonetheless relied on. + conv_bool_to_string = tostring(tobool(true)) + conv_float_to_int = floor(1.5) + conv_float_to_string = tostring(tonumber(1.5)) + conv_int_to_float = floor(1) + conv_int_to_string = tostring(floor(1)) + conv_string_to_int = floor(tostring("1")) + conv_string_to_float = tonumber(tostring("1.5")) + conv_string_to_bool = tobool(tostring("true")) } diff --git a/configs/configupgrade/upgrade_expr.go b/configs/configupgrade/upgrade_expr.go index 2cc700117d..aa4d2bcbc2 100644 --- a/configs/configupgrade/upgrade_expr.go +++ b/configs/configupgrade/upgrade_expr.go @@ -207,7 +207,7 @@ Value: case int: buf.WriteString(strconv.Itoa(tl)) case float64: - buf.WriteString(strconv.FormatFloat(tl, 'f', 64, 64)) + buf.WriteString(strconv.FormatFloat(tl, 'f', -1, 64)) case bool: if tl { buf.WriteString("true") @@ -398,6 +398,49 @@ Value: buf.WriteByte(']') break Value } + + // HIL used some undocumented special functions to implement certain + // operations, but since those were actually callable in real expressions + // some users inevitably depended on them, so we'll fix them up here. + // These each become two function calls to preserve the old behavior + // of implicitly converting to the source type first. Usage of these + // is relatively rare, so the result doesn't need to be too pretty. + case "__builtin_BoolToString": + buf.WriteString("tostring(tobool(") + buf.Write(argExprs[0]) + buf.WriteString("))") + break Value + case "__builtin_FloatToString": + buf.WriteString("tostring(tonumber(") + buf.Write(argExprs[0]) + buf.WriteString("))") + break Value + case "__builtin_IntToString": + buf.WriteString("tostring(floor(") + buf.Write(argExprs[0]) + buf.WriteString("))") + break Value + case "__builtin_StringToInt": + buf.WriteString("floor(tostring(") + buf.Write(argExprs[0]) + buf.WriteString("))") + break Value + case "__builtin_StringToFloat": + buf.WriteString("tonumber(tostring(") + buf.Write(argExprs[0]) + buf.WriteString("))") + break Value + case "__builtin_StringToBool": + buf.WriteString("tobool(tostring(") + buf.Write(argExprs[0]) + buf.WriteString("))") + break Value + case "__builtin_FloatToInt", "__builtin_IntToFloat": + // Since "floor" already has an implicit conversion of its argument + // to number, and the result is a whole number in either case, + // these ones are easier. (We no longer distinguish int and float + // as types in HCL2, even though HIL did.) + name = "floor" } buf.WriteString(name)