From f716a3a73b330bdba479903eff434cf3659e9f06 Mon Sep 17 00:00:00 2001
From: James Shubin <james@shubin.ca>
Date: Fri, 1 Feb 2019 03:58:02 -0500
Subject: [PATCH] lang: funcs: Rename template functions to remove periods

Due to a limitation in the template library, we need to rename some
functions. It's probably worth looking into modifying this library or
finding an alternate version.
---
 lang/funcs/core/template_func.go              | 20 ++++++++++++++++++-
 lang/funcs/funcs.go                           | 12 +++++++++++
 .../TestAstFunc2/template-0.output            |  1 +
 .../TestAstFunc2/template-0/main.mcl          | 12 +++++++++++
 4 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 lang/interpret_test/TestAstFunc2/template-0.output
 create mode 100644 lang/interpret_test/TestAstFunc2/template-0/main.mcl

diff --git a/lang/funcs/core/template_func.go b/lang/funcs/core/template_func.go
index 3a9a3ee9..71ef9fb5 100644
--- a/lang/funcs/core/template_func.go
+++ b/lang/funcs/core/template_func.go
@@ -21,6 +21,7 @@ import (
 	"bytes"
 	"fmt"
 	"reflect"
+	"strings"
 	"text/template"
 
 	"github.com/purpleidea/mgmt/lang/funcs"
@@ -189,8 +190,8 @@ func (obj *TemplateFunc) run(templateText string, vars types.Value) (string, err
 	// function in the simple package?
 	// TODO: loop through this map in a sorted, deterministic order
 	// XXX: should this use the scope instead (so imports are used properly) ?
-	// XXX: dots are not valid here, so maybe replace dots with underscores so we can do fmt_print???
 	for name, fn := range simple.RegisteredFuncs {
+		name = safename(name) // TODO: rename since we can't include dot
 		if _, exists := funcMap[name]; exists {
 			obj.init.Logf("warning, existing function named: `%s` exists", name)
 			continue
@@ -326,6 +327,23 @@ func (obj *TemplateFunc) Close() error {
 	return nil
 }
 
+// safename renames the functions so they're valid inside the template. This is
+// a limitation of the template library, and it might be worth moving to a new
+// one.
+func safename(name string) string {
+	// TODO: should we pick a different replacement char?
+	char := funcs.ReplaceChar // can't be any of: .-#
+	result := strings.Replace(name, funcs.ModuleSep, char, -1)
+	if result == name {
+		// No change, so add a prefix for package-less functions... This
+		// prevents conflicts from sys.func1 -> sys_func1 which would be
+		// a conflict with a top-level function named sys_func1 which is
+		// now renamed to _sys_func1.
+		return char + name
+	}
+	return result
+}
+
 // wrap builds a function in the format expected by the template engine, and
 // returns it as an interface{}. It does so by wrapping our type system and
 // function API with what is expected from the reflection API. It returns a
diff --git a/lang/funcs/funcs.go b/lang/funcs/funcs.go
index 1c785749..7f41a9ae 100644
--- a/lang/funcs/funcs.go
+++ b/lang/funcs/funcs.go
@@ -30,6 +30,13 @@ const (
 	// example when using `fmt.printf` or `math.sin` this is the char used.
 	// It is included here for convenience when importing this package.
 	ModuleSep = interfaces.ModuleSep
+
+	// ReplaceChar is a special char that is used to replace ModuleSep when
+	// it can't be used for some reason. This currently only happens in the
+	// golang template library. Even with this limitation in that library,
+	// we don't want to allow this as the first or last character in a name.
+	// NOTE: the template library will panic if it is one of: .-#
+	ReplaceChar = "_"
 )
 
 // registeredFuncs is a global map of all possible funcs which can be used. You
@@ -56,6 +63,11 @@ func Register(name string, fn func() interfaces.Func) {
 	if strings.HasPrefix(name, ModuleSep) || strings.HasSuffix(name, ModuleSep) {
 		panic(fmt.Sprintf("a func named %s is invalid", name))
 	}
+	// TODO: this should be added but conflicts with our internal functions
+	// can't start or end with an underscore
+	//if strings.HasPrefix(name, ReplaceChar) || strings.HasSuffix(name, ReplaceChar) {
+	//	panic(fmt.Sprintf("a func named %s is invalid", name))
+	//}
 
 	//gob.Register(fn())
 	registeredFuncs[name] = fn
diff --git a/lang/interpret_test/TestAstFunc2/template-0.output b/lang/interpret_test/TestAstFunc2/template-0.output
new file mode 100644
index 00000000..fddc7367
--- /dev/null
+++ b/lang/interpret_test/TestAstFunc2/template-0.output
@@ -0,0 +1 @@
+Vertex: print[template-0]
diff --git a/lang/interpret_test/TestAstFunc2/template-0/main.mcl b/lang/interpret_test/TestAstFunc2/template-0/main.mcl
new file mode 100644
index 00000000..f3e28249
--- /dev/null
+++ b/lang/interpret_test/TestAstFunc2/template-0/main.mcl
@@ -0,0 +1,12 @@
+import "datetime"
+
+$secplus42 = 42 + $ayear
+
+# note the order of the assignment (year can come later in the code)
+$ayear = 60 * 60 * 24 * 365	# is a year in seconds (31536000)
+
+$tmplvalues = struct{time => $secplus42, hello => "world",}
+
+print "template-0" {
+	msg => template("Hello: {{ .hello }}, 42 sec + 1 year is: {{ .time }} seconds, aka: {{ datetime_print .time }}", $tmplvalues),
+}