From 159a39d78ed9e72f6608652522685458fe4ad25b Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 14 Feb 2024 14:33:45 -0500
Subject: [PATCH] Create an error type for unknown function diags

Now that we have namespaced functions, and implementations like
Terraform can add functions based on configuration, the reason for an
unknown function call name becomes a little less clear. Because
functions are populated outside of the hcl package scope, there isn't
enough context to provide a useful diagnostic to the user.

We can create a new Diagnostic.Extra value for
FunctionCallUnknownDiagExtra to indicate specifically when a diagnostic
is created due to an unknown function name. This will carry back the
namespace and function name for the caller to inspect, which will allow
refinement of the diagnostic based on information only known to the
caller.
---
 hclsyntax/expression.go                 | 31 ++++++++++++
 hclsyntax/expression_typeparams_test.go | 65 +++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/hclsyntax/expression.go b/hclsyntax/expression.go
index e81553b3..c4a353c4 100644
--- a/hclsyntax/expression.go
+++ b/hclsyntax/expression.go
@@ -252,6 +252,10 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
 			}
 		}
 
+		extraUnknown := &functionCallUnknown{
+			name: e.Name,
+		}
+
 		// For historical reasons, we represent namespaced function names
 		// as strings with :: separating the names. If this was an attempt
 		// to call a namespaced function then we'll try to distinguish
@@ -274,6 +278,9 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
 				}
 			}
 
+			extraUnknown.name = name
+			extraUnknown.namespace = namespace
+
 			if len(avail) == 0 {
 				// TODO: Maybe use nameSuggestion for the other available
 				// namespaces? But that'd require us to go scan the function
@@ -291,6 +298,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
 						Context:     e.Range().Ptr(),
 						Expression:  e,
 						EvalContext: ctx,
+						Extra:       extraUnknown,
 					},
 				}
 			} else {
@@ -308,6 +316,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
 						Context:     e.Range().Ptr(),
 						Expression:  e,
 						EvalContext: ctx,
+						Extra:       extraUnknown,
 					},
 				}
 			}
@@ -331,6 +340,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
 				Context:     e.Range().Ptr(),
 				Expression:  e,
 				EvalContext: ctx,
+				Extra:       extraUnknown,
 			},
 		}
 	}
@@ -678,6 +688,27 @@ func (e *functionCallDiagExtra) FunctionCallError() error {
 	return e.functionCallError
 }
 
+// FunctionCallUnknownDiagExtra is an interface implemented by a value in the Extra
+// field of some diagnostics to indicate when the error was caused by a call to
+// an unknown function.
+type FunctionCallUnknownDiagExtra interface {
+	CalledFunctionName() string
+	CalledFunctionNamespace() string
+}
+
+type functionCallUnknown struct {
+	name      string
+	namespace string
+}
+
+func (e *functionCallUnknown) CalledFunctionName() string {
+	return e.name
+}
+
+func (e *functionCallUnknown) CalledFunctionNamespace() string {
+	return e.namespace
+}
+
 type ConditionalExpr struct {
 	Condition   Expression
 	TrueResult  Expression
diff --git a/hclsyntax/expression_typeparams_test.go b/hclsyntax/expression_typeparams_test.go
index 4ee9a77e..c7fdbd22 100644
--- a/hclsyntax/expression_typeparams_test.go
+++ b/hclsyntax/expression_typeparams_test.go
@@ -24,6 +24,71 @@ func TestExpressionDiagnosticExtra(t *testing.T) {
 		ctx    *hcl.EvalContext
 		assert func(t *testing.T, diags hcl.Diagnostics)
 	}{
+		// Errors for unknown function calls
+		{
+			"boop()",
+			&hcl.EvalContext{
+				Functions: map[string]function.Function{
+					"zap": function.New(&function.Spec{
+						Type: function.StaticReturnType(cty.String),
+						Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
+							return cty.DynamicVal, fmt.Errorf("the expected error")
+						},
+					}),
+				},
+			},
+			func(t *testing.T, diags hcl.Diagnostics) {
+				t.Helper()
+				for _, diag := range diags {
+					extra, ok := hcl.DiagnosticExtra[FunctionCallUnknownDiagExtra](diag)
+					if !ok {
+						continue
+					}
+
+					if got, want := extra.CalledFunctionName(), "boop"; got != want {
+						t.Errorf("wrong called function name %q; want %q", got, want)
+					}
+					ns := extra.CalledFunctionNamespace()
+					if ns != "" {
+						t.Fatal("expected no namespace, got", ns)
+					}
+					return
+				}
+				t.Fatalf("None of the returned diagnostics implement FunctionCallUnknownDiagExtra\n%s", diags.Error())
+			},
+		},
+		{
+			"ns::source::boop()",
+			&hcl.EvalContext{
+				Functions: map[string]function.Function{
+					"zap": function.New(&function.Spec{
+						Type: function.StaticReturnType(cty.String),
+						Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
+							return cty.DynamicVal, fmt.Errorf("the expected error")
+						},
+					}),
+				},
+			},
+			func(t *testing.T, diags hcl.Diagnostics) {
+				t.Helper()
+				for _, diag := range diags {
+					extra, ok := hcl.DiagnosticExtra[FunctionCallUnknownDiagExtra](diag)
+					if !ok {
+						continue
+					}
+
+					if got, want := extra.CalledFunctionName(), "boop"; got != want {
+						t.Errorf("wrong called function name %q; want %q", got, want)
+					}
+					ns := extra.CalledFunctionNamespace()
+					if ns != "ns::source::" {
+						t.Fatal("expected namespace ns::source::, got", ns)
+					}
+					return
+				}
+				t.Fatalf("None of the returned diagnostics implement FunctionCallUnknownDiagExtra\n%s", diags.Error())
+			},
+		},
 		// Error messages describing inconsistent result types for conditional expressions.
 		{
 			"boop()",