fix: unresolved JSON strings aren't valid JSON (#274)

When we have a JSON string (like container definitions) if it is
unresolved we are inserting a reference string in there to what was
unresolved but its not valid JSON so if anyone uses `fromjson()` on it
it will fail.

This detects JSON strings and formats it properly while maintaining
the reference.

Co-authored-by: John Anderson <[email protected]>

Author: sontek

gotfparse/pkg/converter/converter.go

@@ -294,7 +294,7 @@ func (t *terraformConverter) getAttributeValue(a *terraform.Attribute) any {
 		}
 
 		if traversalExpr, isTraversal := hclAttr.Expr.(*hclsyntax.ScopeTraversalExpr); isTraversal {
-			return t.handleScopeTraversal(traversalExpr)
+			return t.handleScopeTraversal(traversalExpr, val)
 		}
 
 		if funcExpr, isFuncCall := hclAttr.Expr.(*hclsyntax.FunctionCallExpr); isFuncCall {
@@ -332,7 +332,7 @@ func (t *terraformConverter) handleTemplateExpression(a *terraform.Attribute, te
 }
 
 // handleScopeTraversal processes direct references to resources or attributes
-func (t *terraformConverter) handleScopeTraversal(traversalExpr *hclsyntax.ScopeTraversalExpr) any {
+func (t *terraformConverter) handleScopeTraversal(traversalExpr *hclsyntax.ScopeTraversalExpr, val cty.Value) any {
 	// Only process if we have enough parts for a meaningful reference
 	if len(traversalExpr.Traversal) <= 1 {
 		return nil
@@ -382,9 +382,29 @@ func (t *terraformConverter) handleScopeTraversal(traversalExpr *hclsyntax.Scope
 		}
 	}
 
+	fullRef := refString.String()
+
+	// Special case: If this is an unknown JSON string (detected via refinements),
+	// return a parseable JSON placeholder instead of a reference object.
+	// This allows downstream tools like JMESPath's from_json() to work while
+	// still preserving the reference information for debugging.
+	if !val.IsKnown() && val.Type() == cty.String {
+		if rng := val.Range(); rng.StringPrefix() != "" {
+			prefix := rng.StringPrefix()
+
+			if prefix == "[" {
+				// Return JSON array placeholder with unresolved marker
+				return fmt.Sprintf("[{\"__unresolved__\": \"%s\"}]", fullRef)
+			} else if prefix == "{" {
+				// Return JSON object placeholder with unresolved marker
+				return fmt.Sprintf("{\"__unresolved__\": \"%s\"}", fullRef)
+			}
+		}
+	}
+
 	// Construct the reference object with all available information
 	refObj := map[string]interface{}{
-		"__attribute__": refString.String(),
+		"__attribute__": fullRef,
 	}
 
 	// Always add type information if available

tests/terraform/module-output-json-string/main.tf

@@ -0,0 +1,36 @@
+# Variables without defaults - makes them unresolvable at static analysis time
+variable "root_task_name" {
+  type = string
+  # NO DEFAULT - unresolvable
+}
+
+variable "root_image" {
+  type = string
+  # NO DEFAULT - unresolvable
+}
+
+# Direct module usage - this works fine
+module "container_direct" {
+  source = "./module"
+  name   = "direct-container"
+  image  = "nginx:latest"
+}
+
+resource "aws_ecs_task_definition" "direct" {
+  family                   = "direct-task"
+  network_mode             = "awsvpc"
+  requires_compatibilities = ["FARGATE"]
+  cpu                      = "256"
+  memory                   = "512"
+
+  # Direct module reference - this should work
+  container_definitions = module.container_direct.json_encoded_list
+}
+
+# Nested module usage - this reproduces the issue
+# Pass unresolvable variables to the wrapper module
+module "task_wrapper" {
+  source    = "./modules/task_wrapper"
+  task_name = var.root_task_name  # Unknown!
+  image     = var.root_image      # Unknown!
+}

tests/terraform/module-output-json-string/minimal_module/main.tf

@@ -0,0 +1,37 @@
+variable "container_name" {
+  type = string
+}
+
+variable "map_environment" {
+  type    = map(string)
+  default = null
+}
+
+locals {
+  # This pattern triggers UnknownVal: for loop inside ternary
+  final_environment_vars = var.map_environment != null ? [
+    for k, v in var.map_environment : {
+      name  = k
+      value = v
+    }
+  ] : null
+
+  container_definition = {
+    name        = var.container_name
+    environment = local.final_environment_vars
+  }
+
+  container_definition_without_null = {
+    for k, v in local.container_definition :
+    k => v
+    if v != null
+  }
+
+  final_container_definition = merge(local.container_definition_without_null, {})
+
+  json_map = jsonencode(local.final_container_definition)
+}
+
+output "json_map_encoded_list" {
+  value = "[${local.json_map}]"
+}

tests/terraform/module-output-json-string/module/main.tf

@@ -0,0 +1,36 @@
+variable "name" {
+  type        = string
+  description = "Container name"
+}
+
+variable "image" {
+  type        = string
+  description = "Container image"
+}
+
+locals {
+  container_definition = {
+    name      = var.name
+    image     = var.image
+    essential = true
+    portMappings = [
+      {
+        containerPort = 80
+        protocol      = "tcp"
+      }
+    ]
+  }
+
+  # This simulates what simple modules do - direct jsonencode
+  json_map = jsonencode(local.container_definition)
+}
+
+output "json_encoded_list" {
+  description = "JSON string encoded list of container definitions"
+  value       = "[${local.json_map}]"
+}
+
+output "json_map_object" {
+  description = "Container definition as an object"
+  value       = local.container_definition
+}

tests/terraform/module-output-json-string/modules/task_wrapper/main.tf

@@ -0,0 +1,37 @@
+variable "task_name" {
+  type = string
+  # NO DEFAULT - makes it unresolvable at static analysis time
+}
+
+variable "image" {
+  type = string
+  # NO DEFAULT - makes it unresolvable at static analysis time
+}
+
+variable "environment_vars" {
+  type    = map(string)
+  default = {}
+  # Has a default, but when overridden, makes things unresolvable
+}
+
+# Nested module - testing minimal complexity to trigger UnknownVal
+module "container" {
+  source          = "../../minimal_module"
+  container_name  = var.task_name  # Unknown!
+  container_image = var.image      # Unknown!
+}
+
+resource "aws_ecs_task_definition" "wrapped_task" {
+  family                   = var.task_name
+  network_mode             = "awsvpc"
+  requires_compatibilities = ["FARGATE"]
+  cpu                      = "512"
+  memory                   = "1024"
+
+  # This references a nested module output (cloudposse uses json_map_encoded_list)
+  container_definitions = module.container.json_map_encoded_list
+}
+
+output "task_arn" {
+  value = aws_ecs_task_definition.wrapped_task.arn
+}

tests/test_tfparse.py

@@ -704,3 +704,62 @@ def test_not_wholly_known_foreach(tmp_path):
     assert parsed["locals"][0]["current_month"] is None
     assert parsed["locals"][0]["last_month"] is None
     assert parsed["terraform_data"][0]["for_each"] is None
+
+
+def test_module_output_json_string(tmp_path):
+    """
+    Test that module outputs that should be JSON strings are handled correctly.
+
+    For unresolvable module outputs that have JSON refinements (start with "[" or "{"),
+    we should return a parseable JSON placeholder instead of a reference object.
+
+    This prevents JMESPath errors when using from_json():
+    JMESPathTypeError: In function from_json(), invalid type for value:
+    {'__attribute__': 'module.container.json_map_encoded_list', '__name__': 'container'},
+    expected one of: ['string'], received: "object"
+    """
+    mod_path = init_module("module-output-json-string", tmp_path, run_init=False)
+    parsed = load_from_path(mod_path)
+
+    assert "aws_ecs_task_definition" in parsed
+    assert len(parsed["aws_ecs_task_definition"]) == 2  # direct and wrapped
+
+    # Test 1: Direct module reference - fully resolvable
+    direct_task = [
+        t for t in parsed["aws_ecs_task_definition"] if t["family"] == "direct-task"
+    ][0]
+    direct_container_defs = direct_task["container_definitions"]
+
+    assert isinstance(direct_container_defs, str), (
+        f"Expected direct container_definitions to be a string (JSON-encoded), "
+        f"but got {type(direct_container_defs).__name__}: {direct_container_defs}"
+    )
+
+    import json
+
+    parsed_direct = json.loads(direct_container_defs)
+    assert parsed_direct[0]["name"] == "direct-container"
+
+    # Test 2: Nested module reference - unresolvable, should get JSON placeholder
+    # The wrapped task's family is also unresolvable, so find it by path
+    wrapped_task = [
+        t
+        for t in parsed["aws_ecs_task_definition"]
+        if "task_wrapper" in t.get("__tfmeta", {}).get("path", "")
+    ][0]
+    nested_container_defs = wrapped_task["container_definitions"]
+
+    # Should be a string (JSON), not a reference object
+    assert isinstance(nested_container_defs, str), (
+        f"Expected nested container_definitions to be a string (JSON-encoded), "
+        f"but got {type(nested_container_defs).__name__}: {nested_container_defs}"
+    )
+
+    # Should be parseable JSON with __unresolved__ marker
+    parsed_nested = json.loads(nested_container_defs)
+    assert isinstance(parsed_nested, list), "Should be a JSON array"
+    assert len(parsed_nested) == 1, "Should have one placeholder item"
+    assert "__unresolved__" in parsed_nested[0], "Should have __unresolved__ marker"
+    assert (
+        "module.container.json_map_encoded_list" in parsed_nested[0]["__unresolved__"]
+    ), "Should preserve the reference path"