Rewrite contexts before evaluating them (#287)

* Rewrite contexts before evaluating them

* Precompile context and expression patterns

* Test trim before rewrite

The current contextPattern is quite constraining and would fail the
rewrite of a context with trailing spaces. Triming happens during the
execution of Interpolate, and these tests aim to detect future breaking
changes on this behavior.

Co-authored-by: Casey Lee <cplee@nektos.com>

pkg/runner/expression.go

@@ -17,13 +17,11 @@ import (
 	"gopkg.in/godo.v2/glob"
 )
 
-const prefix = "${{"
+var contextPattern, expressionPattern *regexp.Regexp
-const suffix = "}}"
-
-var pattern *regexp.Regexp
 
 func init() {
-	pattern = regexp.MustCompile(fmt.Sprintf("\\%s.+?%s", prefix, suffix))
+	contextPattern = regexp.MustCompile(`^(\w+(?:\[.+\])*)(?:\.([\w-]+))?(.*)$`)
+	expressionPattern = regexp.MustCompile(`\${{\s*(.+?)\s*}}`)
 }
 
 // NewExpressionEvaluator creates a new evaluator
@@ -54,6 +52,7 @@ func (sc *StepContext) NewExpressionEvaluator() ExpressionEvaluator {
 type ExpressionEvaluator interface {
 	Evaluate(string) (string, error)
 	Interpolate(string) string
+	Rewrite(string) string
 }
 
 type expressionEvaluator struct {
@@ -61,7 +60,12 @@ type expressionEvaluator struct {
 }
 
 func (ee *expressionEvaluator) Evaluate(in string) (string, error) {
-	val, err := ee.vm.Run(in)
+	re := ee.Rewrite(in)
+	if re != in {
+		logrus.Debugf("Evaluating '%s' instead of '%s'", re, in)
+	}
+
+	val, err := ee.vm.Run(re)
 	if err != nil {
 		return "", err
 	}
@@ -76,8 +80,10 @@ func (ee *expressionEvaluator) Interpolate(in string) string {
 
 	out := in
 	for {
-		out = pattern.ReplaceAllStringFunc(in, func(match string) string {
+		out = expressionPattern.ReplaceAllStringFunc(in, func(match string) string {
-			expression := strings.TrimPrefix(strings.TrimSuffix(match, suffix), prefix)
+			// Extract and trim the actual expression inside ${{...}} delimiters
+			expression := expressionPattern.ReplaceAllString(match, "$1")
+			// Evaluate the expression and retrieve errors if any
 			evaluated, err := ee.Evaluate(expression)
 			if err != nil {
 				errList = append(errList, err)
@@ -89,7 +95,7 @@ func (ee *expressionEvaluator) Interpolate(in string) string {
 			break
 		}
 		if out == in {
-			// no replacement occurred, we're done!
+			// No replacement occurred, we're done!
 			break
 		}
 		in = out
@@ -97,6 +103,27 @@ func (ee *expressionEvaluator) Interpolate(in string) string {
 	return out
 }
 
+// Rewrite tries to transform any javascript property accessor into its bracket notation.
+// For instance, "object.property" would become "object['property']".
+func (ee *expressionEvaluator) Rewrite(in string) string {
+	re := in
+	for {
+		matches := contextPattern.FindStringSubmatch(re)
+		if matches == nil {
+			// No global match, we're done!
+			break
+		}
+		if matches[2] == "" {
+			// No property match, we're done!
+			break
+		}
+
+		re = fmt.Sprintf("%s['%s']%s", matches[1], matches[2], matches[3])
+	}
+
+	return re
+}
+
 func (rc *RunContext) newVM() *otto.Otto {
 	configers := []func(*otto.Otto){
 		vmContains,

pkg/runner/expression_test.go

@@ -37,9 +37,21 @@ func TestEvaluate(t *testing.T) {
 			"foo": "bar",
 		},
 		StepResults: map[string]*stepResult{
-			"id1": {
+			"idwithnothing": {
 				Outputs: map[string]string{
-					"foo": "bar",
+					"foowithnothing": "barwithnothing",
+				},
+				Success: true,
+			},
+			"id-with-hyphens": {
+				Outputs: map[string]string{
+					"foo-with-hyphens": "bar-with-hyphens",
+				},
+				Success: true,
+			},
+			"id_with_underscores": {
+				Outputs: map[string]string{
+					"foo_with_underscores": "bar_with_underscores",
 				},
 				Success: true,
 			},
@@ -78,7 +90,9 @@ func TestEvaluate(t *testing.T) {
 		{"github.run_id", "1", ""},
 		{"github.run_number", "1", ""},
 		{"job.status", "success", ""},
-		{"steps.id1.outputs.foo", "bar", ""},
+		{"steps.idwithnothing.outputs.foowithnothing", "barwithnothing", ""},
+		{"steps.id-with-hyphens.outputs.foo-with-hyphens", "bar-with-hyphens", ""},
+		{"steps.id_with_underscores.outputs.foo_with_underscores", "bar_with_underscores", ""},
 		{"runner.os", "Linux", ""},
 		{"matrix.os", "Linux", ""},
 		{"matrix.foo", "bar", ""},
@@ -107,7 +121,9 @@ func TestInterpolate(t *testing.T) {
 			Workdir: ".",
 		},
 		Env: map[string]string{
-			"key": "value",
+			"keywithnothing": "valuewithnothing",
+			"key-with-hyphens": "value-with-hyphens",
+			"key_with_underscores": "value_with_underscores",
 		},
 		Run: &model.Run{
 			JobID: "job1",
@@ -125,7 +141,9 @@ func TestInterpolate(t *testing.T) {
 		out string
 	}{
 		{" ${{1}} to ${{2}} ", " 1 to 2 "},
-		{" ${{ env.key }} ", " value "},
+		{" ${{ env.keywithnothing }} ", " valuewithnothing "},
+		{" ${{ env.key-with-hyphens }} ", " value-with-hyphens "},
+		{" ${{ env.key_with_underscores }} ", " value_with_underscores "},
 		{"${{ env.unknown }}", ""},
 	}
 
@@ -137,3 +155,45 @@ func TestInterpolate(t *testing.T) {
 		})
 	}
 }
+
+func TestRewrite(t *testing.T) {
+	assert := a.New(t)
+
+	rc := &RunContext{
+		Config: &Config{},
+		Run: &model.Run{
+			JobID: "job1",
+			Workflow: &model.Workflow{
+				Jobs: map[string]*model.Job{
+					"job1": {},
+				},
+			},
+		},
+	}
+	ee := rc.NewExpressionEvaluator()
+
+	tables := []struct {
+		in string
+		re string
+	}{
+		{"ecole", "ecole"},
+		{"ecole.centrale", "ecole['centrale']"},
+		{"ecole['centrale']", "ecole['centrale']"},
+		{"ecole.centrale.paris", "ecole['centrale']['paris']"},
+		{"ecole['centrale'].paris", "ecole['centrale']['paris']"},
+		{"ecole.centrale['paris']", "ecole['centrale']['paris']"},
+		{"ecole['centrale']['paris']", "ecole['centrale']['paris']"},
+		{"ecole.centrale-paris", "ecole['centrale-paris']"},
+		{"ecole['centrale-paris']", "ecole['centrale-paris']"},
+		{"ecole.centrale_paris", "ecole['centrale_paris']"},
+		{"ecole['centrale_paris']", "ecole['centrale_paris']"},
+	}
+
+	for _, table := range tables {
+		table := table
+		t.Run(table.in, func(t *testing.T) {
+			re := ee.Rewrite(table.in)
+			assert.Equal(table.re, re, table.in)
+		})
+	}
+}