lang, engine: Add a metaparam for catching accidental dollar signs

Let's make our life easier for users!

docs/documentation.md

@@ -274,6 +274,29 @@ and it can't guarantee it if the resource is blocked because of a failed
 pre-requisite resource.
 *XXX: This is currently not implemented!*
 
+#### Dollar
+
+Boolean. Dollar allows you to have a resource name that starts with a `$` sign.
+This is false by default. This helps you catch cases when you write code like:
+
+```mcl
+$foo = "/tmp/file1"
+file "$foo" {} # incorrect!
+```
+
+The above code would ignore the `$foo` variable and attempt to make a file named
+`$foo` which would obviously not work. To correctly interpolate a variable, you
+need to surround the name with curly braces.
+
+```mcl
+$foo = "/tmp/file1"
+file "${foo}" {} # correct!
+```
+
+This meta param is a safety measure to make your life easier. It works for all
+resources. If someone comes up with a resource which would routinely start with
+a dollar sign, then we can revisit the default for this resource kind.
+
 #### Reverse
 
 Boolean. Reverse is a property that some resources can implement that specifies

engine/metaparams.go

@@ -52,6 +52,7 @@ var DefaultMetaParams = &MetaParams{
 	//Sema:  []string{},
 	Rewatch: false,
 	Realize: false, // true would be more awesome, but unexpected for users
+	Dollar:  false,
 }
 
 // MetaRes is the interface a resource must implement to support meta params.
@@ -132,6 +133,13 @@ type MetaParams struct {
 	// the resource is blocked because of a failed pre-requisite resource.
 	// XXX: Not implemented!
 	Realize bool `yaml:"realize"`
+
+	// Dollar allows you to name a resource to start with the dollar
+	// character. We don't allow this by default since it's probably not
+	// needed, and is more likely to be a typo where the user forgot to
+	// interpolate a variable name. In the rare case when it's needed, you
+	// can disable that check with this meta param.
+	Dollar bool `yaml:"dollar"`
 }
 
 // Cmp compares two AutoGroupMeta structs and determines if they're equivalent.
@@ -178,6 +186,9 @@ func (obj *MetaParams) Cmp(meta *MetaParams) error {
 	if obj.Realize != meta.Realize {
 		return fmt.Errorf("values for Realize are different")
 	}
+	if obj.Dollar != meta.Dollar {
+		return fmt.Errorf("values for Dollar are different")
+	}
 
 	return nil
 }
@@ -218,6 +229,7 @@ func (obj *MetaParams) Copy() *MetaParams {
 		Sema:    sema,
 		Rewatch: obj.Rewatch,
 		Realize: obj.Realize,
+		Dollar:  obj.Dollar,
 	}
 }
 

engine/resources.go

@@ -33,6 +33,7 @@ import (
 	"context"
 	"encoding/gob"
 	"fmt"
+	"strings"
 
 	"github.com/purpleidea/mgmt/engine/local"
 	"github.com/purpleidea/mgmt/pgraph"
@@ -272,6 +273,12 @@ func Validate(res Res) error {
 		return errwrap.Wrapf(err, "the Res has an invalid meta param")
 	}
 
+	// TODO: pull dollar prefix from a constant
+	// This catches typos where the user meant to use ${var} interpolation.
+	if !res.MetaParams().Dollar && strings.HasPrefix(res.Name(), "$") {
+		return fmt.Errorf("the Res name starts with a $")
+	}
+
 	return res.Validate()
 }
 

lang/ast/structs.go

@@ -1087,6 +1087,9 @@ func (obj *StmtRes) metaparams(table map[interfaces.Func]types.Value, res engine
 		case "realize":
 			meta.Realize = v.Bool() // must not panic
 
+		case "dollar":
+			meta.Dollar = v.Bool() // must not panic
+
 		case "reverse":
 			if rm != nil {
 				rm.Disabled = !v.Bool() // must not panic
@@ -1150,6 +1153,9 @@ func (obj *StmtRes) metaparams(table map[interfaces.Func]types.Value, res engine
 			if val, exists := v.Struct()["realize"]; exists {
 				meta.Realize = val.Bool() // must not panic
 			}
+			if val, exists := v.Struct()["dollar"]; exists {
+				meta.Dollar = val.Bool() // must not panic
+			}
 			if val, exists := v.Struct()["reverse"]; exists && rm != nil {
 				rm.Disabled = !val.Bool() // must not panic
 			}
@@ -1757,6 +1763,7 @@ func (obj *StmtResMeta) Init(data *interfaces.Data) error {
 	case "sema":
 	case "rewatch":
 	case "realize":
+	case "dollar":
 	case "reverse":
 	case "autoedge":
 	case "autogroup":
@@ -1966,6 +1973,9 @@ func (obj *StmtResMeta) TypeCheck(kind string) ([]*interfaces.UnificationInvaria
 	case "realize":
 		typExpr = types.TypeBool
 
+	case "dollar":
+		typExpr = types.TypeBool
+
 	case "reverse":
 		// TODO: We might want more parameters about how to reverse.
 		typExpr = types.TypeBool
@@ -1982,7 +1992,7 @@ func (obj *StmtResMeta) TypeCheck(kind string) ([]*interfaces.UnificationInvaria
 		// FIXME: allow partial subsets of this struct, and in any order
 		// FIXME: we might need an updated unification engine to do this
 		wrap := func(reverse *types.Type) *types.Type {
-			return types.NewType(fmt.Sprintf("struct{noop bool; retry int; retryreset bool; delay int; poll int; limit float; burst int; reset bool; sema []str; rewatch bool; realize bool; reverse %s; autoedge bool; autogroup bool}", reverse.String()))
+			return types.NewType(fmt.Sprintf("struct{noop bool; retry int; retryreset bool; delay int; poll int; limit float; burst int; reset bool; sema []str; rewatch bool; realize bool; dollar bool; reverse %s; autoedge bool; autogroup bool}", reverse.String()))
 		}
 		// TODO: We might want more parameters about how to reverse.
 		typExpr = wrap(types.TypeBool)

lang/interpolate/parse.rl

@@ -41,6 +41,7 @@ import (
 // Parse performs string interpolation on the input. It returns the list of
 // tokens found. It looks for variables of the format ${foo}. The curly braces
 // are required.
+// XXX: Pull dollar sign and curly chars from VarPrefix and other constants.
 func Parse(data string) (out Stream, _ error) {
 	var (
 		// variables used by Ragel

lang/interpret_test/TestAstFunc1/resdupefields0.txtar

@@ -14,6 +14,7 @@ test "test" {
 		sema => ["foo:1", "bar:3",],
 		rewatch => false,
 		realize => true,
+		dollar => false,
 		reverse => true,
 		autoedge => true,
 		autogroup => true,

lang/interpret_test/TestAstFunc1/resdupefields5.txtar

@@ -14,6 +14,7 @@ test "test" {
 		sema => ["foo:1", "bar:3",],
 		rewatch => false,
 		realize => true,
+		dollar => false,
 		reverse => true,
 		autoedge => true,
 		autogroup => true,
@@ -31,6 +32,7 @@ Edge: const -> composite # autoedge
 Edge: const -> composite # autogroup
 Edge: const -> composite # burst
 Edge: const -> composite # delay
+Edge: const -> composite # dollar
 Edge: const -> composite # limit
 Edge: const -> composite # noop
 Edge: const -> composite # poll
@@ -60,3 +62,4 @@ Vertex: const
 Vertex: const
 Vertex: const
 Vertex: const
+Vertex: const

lang/interpret_test/TestAstFunc2/res4.txtar

@@ -14,6 +14,7 @@ test "t1" {
 	#	sema => ["foo:1", "bar:3",],
 	#	rewatch => false,
 	#	realize => true,
+	#	dollar => false,
 	#	reverse => true,
 	#	autoedge => true,
 	#	autogroup => true,
@@ -29,6 +30,7 @@ test "t1" {
 	Meta:sema => ["foo:1", "bar:3",],
 	Meta:rewatch => false,
 	Meta:realize => true,
+	Meta:dollar => false,
 	Meta:reverse => true,
 	Meta:autoedge => true,
 	Meta:autogroup => true,

lang/interpret_test/TestAstFunc3/dollar1.txtar

@@ -0,0 +1,5 @@
+-- main.mcl --
+$foo = "/tmp/file1"
+test "$foo" {}	# catch simple typos! should be "${foo}"
+-- OUTPUT --
+# err: errValidate: test[$foo] did not Validate: the Res name starts with a $

lang/interpret_test/TestAstFunc3/dollar2.txtar

@@ -0,0 +1,7 @@
+-- main.mcl --
+$foo = "/tmp/file1"
+test "$foo" {
+	Meta:dollar => true, # allow what would normally be a typo!
+}
+-- OUTPUT --
+Vertex: test[$foo]