Skip to content

Handle missing HCL expression types in Terraform parsing#249

Merged
whitemerch merged 5 commits into
mainfrom
chakib.hamie/hcl_expr_handling
Jul 8, 2026
Merged

Handle missing HCL expression types in Terraform parsing#249
whitemerch merged 5 commits into
mainfrom
chakib.hamie/hcl_expr_handling

Conversation

@whitemerch

@whitemerch whitemerch commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Motivation

Terraform HCL parsing still had gaps for a few expression node types that appear in real modules. When those nodes were hit, splat traversals could lose their trailing path, template %{for} blocks could not be rendered, and some synthetic or invalid-expression placeholders fell through to generic unsupported handling.

Changes

Splat traversals

Splat expressions with a trailing attribute or index were truncated because the anonymous splat item in the AST was not handled.

# input
security_group_ids = aws_security_group.app[*].id

# before
"aws_security_group.app[*]"

# after
"aws_security_group.app[*].id"

Template for-loops

%{for} / %{endfor} template directives produce a TemplateJoinExpr in the AST. That shape was unsupported in some parsing paths.

# input
user_data = "%{for v in var.subnets}${v.cidr}%{endfor}"

# before
"__UNSUPPORTED_EXPR__"

# after
"[for v in var.subnets : v.cidr]"

Remaining HCL expression types

AnonSymbolExpr (the synthetic item inside var.list[*].id) and ExprSyntaxError (placeholder for invalid or incomplete expressions) now have explicit handling in the shared HCL expression visitor instead of falling through to the default fallback.

# splat with nested traversal
bucket = aws_s3_bucket.logs[*].bucket

# before → "aws_s3_bucket.logs[*]"
# after  → "aws_s3_bucket.logs[*].bucket"

Author Checklist

  • I have reviewed my own PR.
  • I have added or updated relevant unit tests where necessary. If no tests are added, I've explained why.
  • All new and existing tests pass.
  • I have tested my changes on staging (if applicable).
  • I have updated any relevant documentation (if applicable).

QA Instruction

  1. go test ./pkg/hclexpr/... ./pkg/engine ./pkg/builder/engine/... ./pkg/parser/terraform/...
  2. Splat with traversal: parse var.list[*].id and confirm the rendered path is var.list[*].id, not var.list[*].
  3. Template for-loop: parse %{for v in var.list}${v}%{endfor} and confirm it renders as [for v in var.list : v].

Blast Radius

Datadog IaC Scanner Terraform parsing only: HCL expression rendering in the inspector, engine string conversion, module resolver, and shared hclexpr dispatch.

Additional Notes

N/A

I submit this contribution under the Apache-2.0 license.

…for-loops render correctly across Terraform parsing paths.
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Jul 7, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 82.35%
Overall Coverage: 50.76% (+1.10%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 22e1478 | Docs | Datadog PR Page | Give us feedback!

@whitemerch
whitemerch marked this pull request as ready for review July 7, 2026 16:21
@whitemerch
whitemerch requested a review from a team as a code owner July 7, 2026 16:21
chatgpt-codex-connector[bot]

This comment was marked as resolved.

… and nested traversals are rendered instead of collapsing to \${...}.

@robertohuertasm-datadog robertohuertasm-datadog left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR does address its stated goal. For the common case, this is an improvement:

"%{for v in var.list}${v}%{endfor}"

One edge case to consider: the existing ForExpr renderer appears to mishandle list-style for-expressions with two loop variables:

[for k, v in var.map : k]

KeyExpr is nil for this form because it is a list/tuple comprehension, not an object comprehension, but KeyVar is still populated (k). The current rendering branch only writes ValVar, so it can produce:

[for v in var.map : k]

That bug mostly pre-existed for direct ForExpr rendering. This PR makes it more visible because %{for ...} template directives now route through the same ForExpr renderer, so this template case inherits the issue too:

"%{for k, v in var.map}${k}%{endfor}"

Check engine.go:122 and inspector.go:1598.

Could we preserve KeyVar in the tuple/list branch when it is set, and add a regression test for both direct [for k, v in ...] and template %{for k, v in ...} forms?

In theory this should be a small fix:

b.WriteString("[for ")
if e.KeyVar != "" {
	b.WriteString(e.KeyVar)
	b.WriteString(", ")
}
b.WriteString(e.ValVar)
b.WriteString(" in ")

Comment thread pkg/engine/inspector.go

@robertohuertasm-datadog robertohuertasm-datadog left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@whitemerch
whitemerch merged commit 384e12d into main Jul 8, 2026
19 checks passed
@whitemerch
whitemerch deleted the chakib.hamie/hcl_expr_handling branch July 8, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants