refactor: Address PR #128 nitpick feedback

- Remove dead len(labels) > 0 guards in include/dependency/unit/stack label
  getters (matchers already guarantee a label)
- Extract firstLabelFromContainingBlock helper in ast/stack to DRY
  GetUnitLabel and GetStackLabel
- Use cty.String comparison instead of FriendlyName() == "string" in
  extractStringValue for idiomatic HCL type checking
- Replace tautological predicate in FindFirstParentMatch test with one
  that actually walks the tree but doesn't match
- Remove meaningless assert.NotNil checks on method values in config
  and stack tests

Co-authored-by: Diogenes Fernandes <diofeher@gmail.com>

Author: yhakbar

internal/ast/ast_test.go

@@ -283,8 +283,11 @@ func TestFindFirstParentMatch(t *testing.T) {
 			content: `locals {
 		foo = "bar"
 	}`,
-			pos:      hcl.Pos{Line: 2, Column: 2},
-			matcher:  func(*ast.IndexedNode) bool { return false },
+			pos: hcl.Pos{Line: 2, Column: 2},
+			matcher: func(n *ast.IndexedNode) bool {
+				block, ok := n.Node.(*hclsyntax.Block)
+				return ok && block.Type == "inputs"
+			},
 			expected: false,
 		},
 	}

internal/ast/config/config.go

@@ -81,12 +81,7 @@ func (c *configAST) GetIncludeLabel(node *ast.IndexedNode) (string, bool) {
 		return "", false
 	}
 
-	name := ""
-	if labels := includeBlock.Node.(*hclsyntax.Block).Labels; len(labels) > 0 {
-		name = labels[0]
-	}
-
-	return name, true
+	return includeBlock.Node.(*hclsyntax.Block).Labels[0], true
 }
 
 // GetDependencyLabel returns the label of the given node, if it is a dependency block
@@ -105,12 +100,7 @@ func (c *configAST) GetDependencyLabel(node *ast.IndexedNode) (string, bool) {
 		return "", false
 	}
 
-	name := ""
-	if labels := depBlock.Node.(*hclsyntax.Block).Labels; len(labels) > 0 {
-		name = labels[0]
-	}
-
-	return name, true
+	return depBlock.Node.(*hclsyntax.Block).Labels[0], true
 }
 
 // GetLocals returns the locals scope

internal/ast/config/config_test.go

@@ -37,13 +37,6 @@ locals {
 	configAST := config.NewConfigAST(indexedAST)
 	require.NotNil(t, configAST)
 
-	// Test interface methods exist and work
-	assert.NotNil(t, configAST.FindNodeAt)
-	assert.NotNil(t, configAST.GetIncludeLabel)
-	assert.NotNil(t, configAST.GetDependencyLabel)
-	assert.NotNil(t, configAST.GetLocals)
-	assert.NotNil(t, configAST.GetIncludes)
-
 	// Test that locals and includes are captured
 	locals := configAST.GetLocals()
 	assert.NotNil(t, locals)
@@ -86,8 +79,6 @@ dependency "vpc" {
 `,
 			testFunc: func(t *testing.T, configAST config.ConfigAST) {
 				t.Helper()
-				// Test that it implements the interface
-				var _ = configAST
 				assert.NotNil(t, configAST)
 			},
 		},

internal/ast/stack/stack.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
+	"github.com/zclconf/go-cty/cty"
 )
 
 // StackAST provides methods for working with terragrunt.stack.hcl files.
@@ -39,42 +40,28 @@ func (s *stackAST) FindNodeAt(pos hcl.Pos) *ast.IndexedNode {
 
 // GetUnitLabel returns the label of the given node, if it is a unit block
 func (s *stackAST) GetUnitLabel(node *ast.IndexedNode) (string, bool) {
-	attr := ast.FindFirstParentMatch(node, ast.IsAttribute)
-	if attr == nil {
-		return "", false
-	}
-
-	unitBlock := ast.FindFirstParentMatch(attr, isUnitBlock)
-	if unitBlock == nil {
-		return "", false
-	}
-
-	name := ""
-	if labels := unitBlock.Node.(*hclsyntax.Block).Labels; len(labels) > 0 {
-		name = labels[0]
-	}
-
-	return name, true
+	return firstLabelFromContainingBlock(node, isUnitBlock)
 }
 
 // GetStackLabel returns the label of the given node, if it is a stack block
 func (s *stackAST) GetStackLabel(node *ast.IndexedNode) (string, bool) {
+	return firstLabelFromContainingBlock(node, isStackBlock)
+}
+
+// firstLabelFromContainingBlock walks up to the containing attribute and then the
+// nearest block matching blockMatcher, returning that block's first label.
+func firstLabelFromContainingBlock(node *ast.IndexedNode, blockMatcher func(*ast.IndexedNode) bool) (string, bool) {
 	attr := ast.FindFirstParentMatch(node, ast.IsAttribute)
 	if attr == nil {
 		return "", false
 	}
 
-	stackBlock := ast.FindFirstParentMatch(attr, isStackBlock)
-	if stackBlock == nil {
+	block := ast.FindFirstParentMatch(attr, blockMatcher)
+	if block == nil {
 		return "", false
 	}
 
-	name := ""
-	if labels := stackBlock.Node.(*hclsyntax.Block).Labels; len(labels) > 0 {
-		name = labels[0]
-	}
-
-	return name, true
+	return block.Node.(*hclsyntax.Block).Labels[0], true
 }
 
 // GetUnitSource returns the source attribute value from a unit block
@@ -162,14 +149,14 @@ func (s *stackAST) getBlockAttribute(node *ast.IndexedNode, blockMatcher func(*a
 func (s *stackAST) extractStringValue(expr hclsyntax.Expression) (string, bool) {
 	switch e := expr.(type) {
 	case *hclsyntax.LiteralValueExpr:
-		if e.Val.Type().FriendlyName() == "string" {
+		if e.Val.Type() == cty.String {
 			return e.Val.AsString(), true
 		}
 	case *hclsyntax.TemplateExpr:
 		// Handle quoted strings which are parsed as TemplateExpr
 		if len(e.Parts) == 1 {
 			if literal, ok := e.Parts[0].(*hclsyntax.LiteralValueExpr); ok {
-				if literal.Val.Type().FriendlyName() == "string" {
+				if literal.Val.Type() == cty.String {
 					return literal.Val.AsString(), true
 				}
 			}

internal/ast/stack/stack_test.go

@@ -39,15 +39,6 @@ stack "nested" {
 	// Create StackAST
 	stackAST := stack.NewStackAST(indexedAST)
 	require.NotNil(t, stackAST)
-
-	// Test interface methods exist and work
-	assert.NotNil(t, stackAST.FindNodeAt)
-	assert.NotNil(t, stackAST.GetUnitLabel)
-	assert.NotNil(t, stackAST.GetStackLabel)
-	assert.NotNil(t, stackAST.GetUnitSource)
-	assert.NotNil(t, stackAST.GetUnitPath)
-	assert.NotNil(t, stackAST.FindUnitAt)
-	assert.NotNil(t, stackAST.FindStackAt)
 }
 
 func TestStackAST_Methods(t *testing.T) {
@@ -82,8 +73,6 @@ stack "nested" {
 `,
 			testFunc: func(t *testing.T, stackAST stack.StackAST) {
 				t.Helper()
-				// Test that it implements the interface
-				var _ = stackAST
 				assert.NotNil(t, stackAST)
 			},
 		},