Type checker awareness of partially-unknown collections

[apparentlymart]

Earlier we made the evaluator support collections (lists and maps) that have a mixture of known and unknown members. However, since the type checker was not also aware of this it was short-circuiting as before and skipping type checking of any expression dependent on a value from a partially-unknown collection.

As well as missing some conversions, which was masked by the evaluator's own short-circuiting as expected, this also caused us to skip the conversion of arithmetic to built-in functions, which then caused the evaluator to fail since it doesn't have any support for direct evaluation of arithmetic.

This follows the same principle as the evalator changes: there's no longer a global rule that any partially-unknown value gets flattened to a total unknown when read from a variable, so each node type separately must deal with the possibility of unknowns and decide what their behavior will be in that case.

For most node types the behavior is simple: any unknowns mean that the result is itself unknown. For index, we optimistically assume that unknown values will become known values of the correct type on a future pass and so we let them pass through, which is safe because of our existing changes to how the evaluator supports unknowns.

This also includes a reorganization of the helper functions VariableListElementTypesAreHomogenous and VariableMapValueTypesAreHomogenous to make the different cases easier to follow in the presence of unknowns.

---

This PR also includes a bonus commit to fix incorrect handling of unknowns in the Output evaluator, which would result in a crash in certain cases due to this same type checker short-circuit behavior.

[apparentlymart]

This fixes the bug introduced by #51.

[mitchellh]

In the scope of this review I can't claim to know the full impact of this, but this stood out to me as a different behavior. Is this fully understood?

[jbardin]

I would add, is this actually an error, or should it return TypeUnknown?

[apparentlymart]

This is the same behavior, just implemented in a different way. It used to build a slice of types and then check if the slice was empty at the end of the function, but now it just takes care of that up-front.

[apparentlymart]

It could in principle return TypeUnknown, but that would be the first instance of an unknown value originating from anywhere other than a variable access, and so may have unintended consequences I'd rather not deal with here.

In the long run I think the correct behavior is for HIL to keep track of the element type of lists and maps as part of the type itself, rather than attempting to infer it from content. This is what #42 was about, but I ended up closing that with the intent of folding it into the more holistic language work we're hoping to do in the not-too-distant future.

[salimane]

@apparentlymart does this change means you can never work with empty lists ?
for example :

variable "my_var" {
  type = "list"
  default = []
}
resource "aws_instance" "main" {
  count = "${var.count}"
  private_ip = "${length(var.my_var) == 0 ? "" : var.my_var[count.index]}"
}

the above is failing in Terraform v0.10.0 with :

* module.bob.aws_instance.main: At column 54, line 1: list "var.my_var" does not have any elements so cannot determine type. in:

${length(var.my_var) == 0 ? "" : var.my_var[count.index]}

[apparentlymart]

@salimane that was not a consequence of this change, but it is true in certain scenarios. Any time the HIL type checker needs to know the element type of a list it requires at least one element to infer it, since the type system isn't strong enough to retain that information as part of the type. (This is a consequence of the fact that Terraform only supported strings at first, and some parts of its design are still not entirely type-aware.)

The change in #42 was intended to address that, but it ended up being trickier than first thought so it ended up rolling into a bigger set of language improvements we're currently designing that make broader changes to the architecture of the evaluator to address this and other gotchas by consistently handling types throughout. We'll have more to share on that soon once we have some more concrete plans, but this issue in particular is uncontroversial and will be fixed as part of it!

[salimane]

@apparentlymart thanks for the context .

But is there a workaround for the errors I'm getting above ?

[apparentlymart]

In that specific scenario it looks like you expect that var.my_var should always have var.count elements, in which case I think it should be safe to write that config as follows:

variable "my_var" {
  type = "list"
  default = []
}
resource "aws_instance" "main" {
  count = "${var.count}"
  private_ip = "${var.my_var[count.index]}"
}

When count is zero, the expressions within the resource block are not evaluated at all, so there's no need to guard for the failure in that case.

If you're count doesn't necessarily match the length of my_var, things get a little more tricky. The key to working around this is to avoid things that cause Terraform to do an element type check, and indexing is one of those things. I'm having trouble constructing a real example to illustrate this, but I think it can be worked around by exploiting the fact that passing a list to a function doesn't currently require an element type check and thus forcing the list to have at least one element:

concat(var.my_var, list(""))

Unfortunately due to other current HIL limitations (which will also be fixed by the forthcoming changes, thankfully) it's not possible to use the [...] operator on a function result, so it'd be necessary to pass that concat result to the element function to extract an element instead.

[salimane]

@apparentlymart thank you very much for the workaround 🍺

[salimane]

@apparentlymart actually the workaround didn't work with terraform v0.10.0, I'm still getting

variable "my_var" {
  type = "list"
  default = []
}
resource "aws_instance" "main" {
  count = "${var.count}"
  private_ip = "${var.my_var[count.index]}"
}

* module.bob.aws_instance.main: At column 18, line 1: list "var.my_var" does not have any elements so cannot determine type. in:

${var.my_var[count.index]}

I

[apparentlymart]

Hmm that's strange, @salimane. I'm afraid I'm not sure why Terraform would be evaluating the expression in the case where count = 0; possibly it's getting tripped up in one of the early processing steps. 😖

In that case, doing something with concatenation of an empty string onto the end of your list is probably the best we can do for a workaround for now.