hclsyntax: Recovery of incomplete ConditionalExpr

[radeksimko]

Background

Similar to some earlier issues, we found some more edge cases while implementing support for ConditionalExpr in the Language Server.

hashicorp/hcl-lang#326

• incomplete ConditionalExpr
  • attr = ? var.foo : var.bar
  • attr = var.foo ? : var.bar
  • attr = var.foo ? var.bar :
  • attr = var.foo ?
• "impeding" incomplete Traversal (with trailing dot) inside otherwise complete ConditionalExpr
  • attr = var. ? var.foo : var.bar
  • attr = var.foo ? var. : var.bar
  • attr = var.foo ? var.bar : var.

As with other reports related to recovery from incomplete configuration, the context is - again - enabling completion. Users would reasonably expect to be offered relevant completion in the above contexts, because it is exactly the nature of completion to complete the incomplete configuration.

Proposal

Regarding the incomplete Traversal specifically - I remember seeing other contexts where the trailing dot is simply ignored and the outer expression is recovered and so is the attribute as whole. I am therefore a little more hopeful we could do something about these edge cases.

The other ones like attr = ? var.foo : var.bar I would think could still be reported as ConditionalExpr but with either nil or some kind of empty expressions (I've seen LiteralValueExpr{} used before in similar contexts, such as attr =).

The only incomplete expression I can see being really difficult or maybe impossible to deal with meaningfully is the one which is missing a control character : (attr = var.foo ? ) because there could be lack of confidence in what kind of expression is it even supposed to be.

[apparentlymart]

Thanks for raising this!

Again I find myself unsure about what we can do here, but it seems worth a try. The challenges with this sort of recovery are always of a similar genre: we need to make some assumptions about what's likely to be coming up so that we can go hunting forward for tokens without any higher-level grammar to guide us, but to try to be conservative so that we don't cause the remaining parsing work to begin in a very strange place in the token stream where the subsequent parsing will behave badly.

In this case it seems like we'd need for the conditional expression parser to check for errors when trying to parse the second expression (the one after the ? token), but retain the resulting expression (if any) anyway. Then we could check whether the very next token (after those consumed by the expression parsing) is :, and if so optimistically assume that we've found the marker for the beginning of the third expression.

As you say, I think predicate ? with something other than a colon after it is a trickier case to handle, but perhaps as a compromise we could think of some other token types that tend to appear immediately after the end of an expression, such as a closing brace or a newline, and if so return a conditional expression with the third expression set to nil to represent its absence, and then continue recovery-mode parsing at the token we used as the heuristic for the end of an expression.

The case where the predicate expression itself is omitted (i.e. the entire expression starts with a ? token) is interesting. I would've expected that to be tricky to handle, but by coincidence the conditional expression happens to be the first expression type we try in the recursive descent and so it would get an opportunity to sniff if the very next token is a ? before it tries to do anything else, and if so assume it was a conditional expression with a missing predicate. That is a little tricky in that it could be broken if we later added another expression that gets visited first, but that seems unlikely and hopefully it would suffice to clearly document it with comments in the relevant parts of the code.