lang/funcs: added parseint function

[jeet-parekh]

Added a function to parse a string to an integer for a specified base.

Closes #22584. Also see #22585.

[hashicorp-cla]

All committers have signed the CLA.

[apparentlymart]

Thanks for working on this @jeet-parekh!

Overall this looks great. I just left a few small bits of feedback inline. If you have any questions about them then please let me know!

[jeet-parekh]

@apparentlymart, I have set the type of the first argument as string. But when I test these changes in the terraform console, it accepts both a number, and a string as the first argument. That's confusing me a bit. Why is that happening?

[apparentlymart]

Hi @jeet-parekh!

The Terraform language automatically converts numbers to strings using base 10 when the context requires a string.

In this particular case I agree it seems a bit strange to allow it, since it probably indicates a user misunderstanding what this function is for or how it works: this function is operating on the individual characters of a string, so populating it with an automatic conversion would be a strange thing to do. It's possible to override that default conversion behavior like this:

• Change the parameter type of the first parameter to cty.DynamicPseudoType, which will tell the language runtime to just accept any type and not do any automatic conversions.
• Change the Type field in the function definition to be an inline function (rather than a call to StaticReturnType) and write logic in that function to check that args[0].Type() is cty.String, returning an explicit error if not. The Type function should still ultimately return cty.Number, nil if that check passes, to indicate that the result type will always be a number.

As I was suggesting in my other comments here, we can return this new error using function.NewArgErrorf to allow Terraform to indicate specifically that it's the first argument that is incorrect, doing something like this:

    if !args[0].Type().Equals(cty.String) {
         return cty.Number, function.NewArgErrorf(0, "first argument must be string, not %s", args[0].Type().FriendlyName())
    }

I suggest we retain the automatic type conversion behavior for the second argument (by leaving it specified as cty.Number) because that argument is just a normal parameter interpreted atomically for its value (rather than broken apart and analyzed, as the first argument) and we can therefore remain consistent with the normal language behaviors for that one.

[jeet-parekh]

Hi @apparentlymart,

I changed parseint to use math/big.

What do you think about another function inttostr? It would convert a number to a string for a given base.

[apparentlymart]

Hi @jeet-parekh! I'm going to look at these changes now, but I first wanted to address your question:

For the moment I don't think we should add a function for formatting numbers because formatting numbers to bases 2, 8, 10, and 16 is already supported via the format function and I don't think we have a compelling use-case for formatting to any other bases.

If such a use-case does emerge, I expect we'd prefer to address it via enhancements to format rather than via a new function, since that function is the primary way to produce non-default string representations of non-string values and includes related features like zero-padding and rounding.

[apparentlymart]

Thanks @jeet-parekh!

I made some small changes to fix one failing test, to expand slightly the documentation, and to add some additional tests I used to try out some different inputs. I'm going to merge this now. Thanks again for working on this, and thanks for working through this feedback with me. 🎉

[jeet-parekh]

@apparentlymart, did you make any changes to my code to make the tests pass? What were the changes? Thanks!

[apparentlymart]

Hi @jeet-parekh,

The test failure was from the TestFunctions test using a number instead of a string.

[aajain-crest]

Hi @apparentlymart,

When is this change planned to be released?

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.