Unify core functions address handling (#3445)

Signed-off-by: Andrei Ciobanu <andrei.ciobanu@opentofu.org>

internal/addrs/provider_function.go

@@ -62,6 +62,22 @@ func (f Function) IsNamespace(namespace string) bool {
 	return len(f.Namespaces) > 0 && f.Namespaces[0] == namespace
 }
 
+// FullyQualified returns a new [Function] where the [Function.Namespaces] is guaranteed to be filled.
+// For the functions that already have a namespace defined (e.g.: provider::test::func, core::tolist, etc), this
+// method will return the object that was called on.
+// For the functions that have no namespace defined (tolist, tomap, ephemeralasnull, sensitive, etc), this
+// method will return a new struct with the [FunctionNamespaceCore] as the namespace.
+// The purpose of this is to ensure consistency when handling HCL functions addresses.
+func (f Function) FullyQualified() Function {
+	if len(f.Namespaces) > 0 {
+		return f
+	}
+	return Function{
+		Name:       f.Name,
+		Namespaces: []string{FunctionNamespaceCore},
+	}
+}
+
 func (f Function) AsProviderFunction() (pf ProviderFunction, err error) {
 	if !f.IsNamespace(FunctionNamespaceProvider) {
 		// Should always be checked ahead of time!

internal/command/jsonfunction/function.go

@@ -9,7 +9,7 @@ import (
 	"encoding/json"
 	"fmt"
 
-	"github.com/opentofu/opentofu/internal/lang"
+	"github.com/opentofu/opentofu/internal/addrs"
 	"github.com/opentofu/opentofu/internal/tfdiags"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/function"
@@ -58,10 +58,14 @@ func Marshal(f map[string]function.Function) ([]byte, tfdiags.Diagnostics) {
 	signatures := newFunctions()
 
 	for name, v := range f {
-		switch name {
-		case "can", lang.CoreNamespace + "can":
+		// Even though it's not possible to have a provider namespaced function end up in here,
+		// we want to qualify the function name to be sure that we check exactly for the
+		// function that we have custom marshaller for.
+		fqFuncAddr := addrs.ParseFunction(name).FullyQualified().String()
+		switch fqFuncAddr {
+		case addrs.ParseFunction("can").FullyQualified().String():
 			signatures.Signatures[name] = marshalCan(v)
-		case "try", lang.CoreNamespace + "try":
+		case addrs.ParseFunction("try").FullyQualified().String():
 			signatures.Signatures[name] = marshalTry(v)
 		default:
 			signature, err := marshalFunction(v)

internal/command/jsonfunction/function_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/hcl/v2/ext/tryfunc"
 	"github.com/zclconf/go-cty-debug/ctydebug"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/function"
@@ -113,6 +114,72 @@ func TestMarshal(t *testing.T) {
 			"",
 			"Failed to serialize function \"fun\": error",
 		},
+		{
+			"try function marshalled correctly",
+			map[string]function.Function{
+				"try": tryfunc.TryFunc,
+			},
+			`{"format_version":"1.0","function_signatures":{"try":{"return_type":"dynamic","variadic_parameter":{"name":"expressions","type":"dynamic"}}}}`,
+			"",
+		},
+		{
+			"core::try function marshalled correctly",
+			map[string]function.Function{
+				"core::try": tryfunc.TryFunc,
+			},
+			`{"format_version":"1.0","function_signatures":{"core::try":{"return_type":"dynamic","variadic_parameter":{"name":"expressions","type":"dynamic"}}}}`,
+			"",
+		},
+		{
+			// This checks that if a provider contains a function named the same as one of the core with custom marshaller, we identify that correctly
+			"provider::test::try function marshalled correctly",
+			map[string]function.Function{
+				"provider::test::try": function.New(&function.Spec{
+					Params: []function.Parameter{
+						{
+							Name: "list",
+							Type: cty.List(cty.String),
+						},
+					},
+					Type: function.StaticReturnType(cty.List(cty.String)),
+				}),
+			},
+			`{"format_version":"1.0","function_signatures":{"provider::test::try":{"return_type":["list","string"],"parameters":[{"name":"list","type":["list","string"]}]}}}`,
+			"",
+		},
+		{
+			"can function marshalled correctly",
+			map[string]function.Function{
+				"can": tryfunc.CanFunc,
+			},
+			`{"format_version":"1.0","function_signatures":{"can":{"return_type":"bool","parameters":[{"name":"expression","type":"dynamic"}]}}}`,
+			"",
+		},
+		{
+			"core::can function marshalled correctly",
+			map[string]function.Function{
+				"core::can": tryfunc.CanFunc,
+			},
+			`{"format_version":"1.0","function_signatures":{"core::can":{"return_type":"bool","parameters":[{"name":"expression","type":"dynamic"}]}}}`,
+			"",
+		},
+		{
+			// This checks that if a provider contains a function named the same as one of the core with custom marshaller, we identify that correctly
+			"provider::test::can function marshalled correctly",
+			map[string]function.Function{
+				"provider::test::can": function.New(&function.Spec{
+					Params: []function.Parameter{
+						{
+							Name: "list",
+							Type: cty.List(cty.String),
+						},
+					},
+					Type: function.StaticReturnType(cty.List(cty.String)),
+				}),
+			},
+			`{"format_version":"1.0","function_signatures":{"provider::test::can":{"return_type":["list","string"],"parameters":[{"name":"list","type":["list","string"]}]}}}`,
+			"",
+		},
 	}
 
 	for i, test := range tests {

internal/command/metadata_functions.go

@@ -8,6 +8,7 @@ package command
 import (
 	"fmt"
 
+	"github.com/opentofu/opentofu/internal/addrs"
 	"github.com/zclconf/go-cty/cty/function"
 
 	"github.com/opentofu/opentofu/internal/command/jsonfunction"
@@ -15,7 +16,10 @@ import (
 )
 
 var (
-	ignoredFunctions = []string{"map", "list"}
+	ignoredFunctions = []addrs.Function{
+		addrs.ParseFunction("map").FullyQualified(),
+		addrs.ParseFunction("list").FullyQualified(),
+	}
 )
 
 // MetadataFunctionsCommand is a Command implementation that prints out information
@@ -78,8 +82,9 @@ Usage: tofu [global options] metadata functions -json
 `
 
 func isIgnoredFunction(name string) bool {
+	funcAddr := addrs.ParseFunction(name).FullyQualified().String()
 	for _, i := range ignoredFunctions {
-		if i == name || lang.CoreNamespace+i == name {
+		if funcAddr == i.String() {
 			return true
 		}
 	}

internal/command/metadata_functions_test.go

@@ -63,9 +63,12 @@ func TestMetadataFunctions_output(t *testing.T) {
 
 	// test that ignored functions are not part of the json
 	for _, v := range ignoredFunctions {
-		_, ok := got.Signatures[v]
-		if ok {
-			t.Fatalf("found ignored function %q inside output", v)
+		if _, ok := got.Signatures[v.Name]; ok {
+			t.Errorf("found ignored function %q inside output", v)
+		}
+		corePrefixed := v.String()
+		if _, ok := got.Signatures[corePrefixed]; ok {
+			t.Fatalf("found ignored function %q inside output", corePrefixed)
 		}
 	}
 }

internal/lang/functions.go

@@ -25,10 +25,6 @@ var impureFunctions = []string{
 	"uuid",
 }
 
-// CoreNamespace defines the string prefix used for all core namespaced functions
-// TODO: This should probably be replaced with addrs.Function everywhere
-const CoreNamespace = addrs.FunctionNamespaceCore + "::"
-
 // Functions returns the set of functions that should be used to when evaluating
 // expressions in the receiving scope.
 func (s *Scope) Functions() map[string]function.Function {
@@ -63,7 +59,7 @@ func (s *Scope) Functions() map[string]function.Function {
 		}
 		// Copy all stdlib funcs into core:: namespace
 		for _, name := range coreNames {
-			s.funcs[CoreNamespace+name] = s.funcs[name]
+			s.funcs[addrs.ParseFunction(name).FullyQualified().String()] = s.funcs[name]
 		}
 	}
 	s.funcsLock.Unlock()

internal/lang/functions_descriptions_test.go

@@ -6,9 +6,9 @@
 package lang
 
 import (
-	"strings"
 	"testing"
 
+	"github.com/opentofu/opentofu/internal/addrs"
 	"github.com/opentofu/opentofu/internal/lang/funcs"
 )
 
@@ -29,7 +29,7 @@ func TestFunctionDescriptions(t *testing.T) {
 	}
 
 	for name := range allFunctions {
-		_, ok := funcs.DescriptionList[strings.TrimPrefix(name, CoreNamespace)]
+		_, ok := funcs.DescriptionList[addrs.ParseFunction(name).Name]
 		if !ok {
 			t.Errorf("missing DescriptionList entry for function %q", name)
 		}

internal/lang/functions_test.go

@@ -9,13 +9,13 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 	homedir "github.com/mitchellh/go-homedir"
+	"github.com/opentofu/opentofu/internal/addrs"
 	"github.com/zclconf/go-cty/cty"
 
 	"github.com/opentofu/opentofu/internal/experiments"
@@ -1252,11 +1252,12 @@ func TestFunctions(t *testing.T) {
 		// with PureOnly: true and then verify that they return unknown values of a
 		// suitable type.
 		for _, impureFunc := range impureFunctions {
-			delete(allFunctions, impureFunc)
-			delete(allFunctions, CoreNamespace+impureFunc)
+			funcAddr := addrs.ParseFunction(impureFunc)
+			delete(allFunctions, funcAddr.Name)
+			delete(allFunctions, funcAddr.FullyQualified().String())
 		}
 		for f := range scope.Functions() {
-			if _, ok := tests[strings.TrimPrefix(f, CoreNamespace)]; !ok {
+			if _, ok := tests[addrs.ParseFunction(f).Name]; !ok {
 				t.Errorf("Missing test for function %s\n", f)
 			}
 		}
@@ -1344,6 +1345,23 @@ func TestFunctions(t *testing.T) {
 	}
 }
 
+func TestFunctionsPrefixedCorrectly(t *testing.T) {
+	dir := t.TempDir()
+	baseFuncs := makeBaseFunctionTable(dir)
+	s := &Scope{BaseDir: dir}
+	got := s.Functions()
+
+	for name := range baseFuncs {
+		if _, ok := got[name]; !ok {
+			t.Errorf("expected %q function to be in the scope", name)
+		}
+		want := addrs.ParseFunction(name).FullyQualified().String()
+		if _, ok := got[want]; !ok {
+			t.Errorf("expected %q function to be in the scope", want)
+		}
+	}
+}
+
 const (
 	CipherBase64 = "eczGaDhXDbOFRZGhjx2etVzWbRqWDlmq0bvNt284JHVbwCgObiuyX9uV0LSAMY707IEgMkExJqXmsB4OWKxvB7epRB9G/3+F+pcrQpODlDuL9oDUAsa65zEpYF0Wbn7Oh7nrMQncyUPpyr9WUlALl0gRWytOA23S+y5joa4M34KFpawFgoqTu/2EEH4Xl1zo+0fy73fEto+nfkUY+meuyGZ1nUx/+DljP7ZqxHBFSlLODmtuTMdswUbHbXbWneW51D7Jm7xB8nSdiA2JQNK5+Sg5x8aNfgvFTt/m2w2+qpsyFa5Wjeu6fZmXSl840CA07aXbk9vN4I81WmJyblD/ZA=="
 	PrivateKey   = `