Expression errors (#89)

Add much better errors when compiling and expression fails

pkg/expressions/errors.go

@@ -0,0 +1,67 @@
+package expressions
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+)
+
+var (
+	ErrorUnterminated    = errors.New("non-terminated statement in expression")
+	ErrorEmptyStatement  = errors.New("empty statement in expression")
+	ErrorMissingFunction = errors.New("missing function")
+)
+
+type DetailedError struct {
+	Err     error
+	Context string
+	Index   int
+}
+
+func (s *DetailedError) Error() string {
+	return fmt.Sprintf("At `%s` (%d): %v", s.Context, s.Index, s.Err)
+}
+
+func (s *DetailedError) Unwrap() error {
+	return s.Err
+}
+
+type CompilerErrors struct {
+	Errors     []*DetailedError
+	Expression string
+}
+
+func (s *CompilerErrors) Error() string {
+	if len(s.Errors) == 1 {
+		return s.Errors[0].Error()
+	}
+	var sb strings.Builder
+	sb.WriteString("Compiler Errors in: `")
+	sb.WriteString(s.Expression)
+	sb.WriteString("`\n")
+	for _, e := range s.Errors {
+		sb.WriteString("  ")
+		sb.WriteString(e.Error())
+		sb.WriteString("\n")
+	}
+	return sb.String()
+}
+
+func (s *CompilerErrors) Unwrap() error {
+	return s.Errors[0]
+}
+
+func (s *CompilerErrors) add(underlying error, context string, offset int) {
+	s.Errors = append(s.Errors, &DetailedError{underlying, context, offset})
+}
+
+func (s *CompilerErrors) empty() bool {
+	return len(s.Errors) == 0
+}
+
+// Inherit all errors from another compiler error set, and offset the index eg. if nested compile
+func (s *CompilerErrors) inherit(other *CompilerErrors, offset int) {
+	for _, oe := range other.Errors {
+		s.add(oe.Err, oe.Context, oe.Index+offset)
+	}
+}

pkg/expressions/keyBuilder.go

@@ -1,7 +1,6 @@
 package expressions
 
 import (
-	"errors"
 	"fmt"
 	"strings"
 )
@@ -38,12 +37,18 @@ func (s *KeyBuilder) Funcs(funcs map[string]KeyBuilderFunction) {
 	}
 }
 
-// Compile builds a new key-builder
-func (s *KeyBuilder) Compile(template string) (*CompiledKeyBuilder, error) {
+// Compile builds a new key-builder, returning error(s) on build issues
+// if the CompiledKeyBuilder is not nil, then something is still useable (albeit may have problems)
+func (s *KeyBuilder) Compile(template string) (*CompiledKeyBuilder, *CompilerErrors) {
 	kb := &CompiledKeyBuilder{
 		stages: make([]KeyBuilderStage, 0),
 	}
 
+	errs := CompilerErrors{
+		Expression: template,
+	}
+
+	startStatement := 0
 	inStatement := 0
 	var sb strings.Builder
 	runes := []rune(template)
@@ -59,6 +64,7 @@ func (s *KeyBuilder) Compile(template string) (*CompiledKeyBuilder, error) {
 					kb.stages = append(kb.stages, stageLiteral(sb.String()))
 					sb.Reset()
 				}
+				startStatement = i
 			} else {
 				sb.WriteRune(r)
 			}
@@ -68,23 +74,32 @@ func (s *KeyBuilder) Compile(template string) (*CompiledKeyBuilder, error) {
 			if inStatement == 0 {
 				args := splitTokenizedArguments(sb.String())
 				if len(args) == 0 {
-					return nil, errors.New("empty statement in expression")
+					errs.add(ErrorEmptyStatement, string(runes[startStatement:i+1]), startStatement)
 				} else if len(args) == 1 { // Simple variable keyword like "{1}"
 					kb.stages = append(kb.stages, stageSimpleVariable(args[0]))
 				} else { // Complex function like "{add 1 2}"
 					f := s.functions[args[0]]
 					if f != nil {
-						compiledArgs := make([]KeyBuilderStage, 0)
+						compiledArgs := make([]KeyBuilderStage, 0, len(args)-1)
 						for _, arg := range args[1:] {
 							compiled, err := s.Compile(arg)
 							if err != nil {
-								return nil, err
+								errs.inherit(err, startStatement)
+							}
+							if compiled != nil {
+								compiledArgs = append(compiledArgs, compiled.joinStages())
 							}
-							compiledArgs = append(compiledArgs, compiled.joinStages())
 						}
-						kb.stages = append(kb.stages, f(compiledArgs))
+						stage, err := f(compiledArgs)
+						if err != nil {
+							errs.add(err, sb.String(), startStatement)
+						}
+						if stage != nil {
+							kb.stages = append(kb.stages, stage)
+						}
 					} else {
-						kb.stages = append(kb.stages, stageError(fmt.Sprintf("Err:%s", args[0])))
+						kb.stages = append(kb.stages, stageLiteral(fmt.Sprintf("<Err:%s>", args[0])))
+						errs.add(ErrorMissingFunction, sb.String(), startStatement)
 					}
 				}
 
@@ -98,7 +113,7 @@ func (s *KeyBuilder) Compile(template string) (*CompiledKeyBuilder, error) {
 	}
 
 	if inStatement != 0 {
-		return nil, errors.New("non-terminated statement in expression")
+		errs.add(ErrorUnterminated, string(runes[startStatement:]), startStatement)
 	}
 
 	if sb.Len() > 0 {
@@ -109,6 +124,9 @@ func (s *KeyBuilder) Compile(template string) (*CompiledKeyBuilder, error) {
 		kb = kb.optimize()
 	}
 
+	if !errs.empty() {
+		return kb, &errs
+	}
 	return kb, nil
 }
 

pkg/expressions/keyBuilder_test.go

@@ -2,33 +2,25 @@ package expressions
 
 import (
 	"bytes"
+	"errors"
 	"strconv"
 	"testing"
 	"text/template"
 
 	"github.com/stretchr/testify/assert"
 )
 
-var testData = []string{"ab", "cd", "123"}
-var testKeyData = map[string]string{
-	"test": "testval",
-}
-
-type TestContext struct{}
-
-func (s *TestContext) GetMatch(idx int) string {
-	return testData[idx]
-}
-
-func (s *TestContext) GetKey(key string) string {
-	return testKeyData[key]
+var testContext = KeyBuilderContextArray{
+	Elements: []string{"ab", "cd", "123"},
+	Keys: map[string]string{
+		"test": "testval",
+	},
 }
 
-var testContext = TestContext{}
-
 func TestSimpleKey(t *testing.T) {
-	kb, _ := NewKeyBuilder().Compile("test 123")
+	kb, err := NewKeyBuilder().Compile("test 123")
 	key := kb.BuildKey(&testContext)
+	assert.Nil(t, err)
 	assert.Equal(t, "test 123", key)
 	assert.Equal(t, 1, len(kb.stages))
 }
@@ -43,13 +35,26 @@ func TestSimpleReplacement(t *testing.T) {
 func TestUnterminatedReplacement(t *testing.T) {
 	kb, err := NewKeyBuilder().Compile("{0} is {123")
 	assert.Error(t, err)
-	assert.Nil(t, kb)
+	assert.Len(t, err.Errors, 1)
+	assert.NotEmpty(t, err.Error())
+	assert.NotNil(t, kb) // Still returns workable expression, but with errors
+}
+
+func TestManyErrors(t *testing.T) {
+	kb, err := NewKeyBuilder().Compile("{0} is {abc 1} and {unclosed")
+	assert.NotNil(t, kb)
+	assert.Error(t, err)
+	assert.Len(t, err.Errors, 2)
+	assert.ErrorIs(t, err.Errors[0], ErrorMissingFunction)
+	assert.ErrorIs(t, err.Errors[1], ErrorUnterminated)
+	assert.ErrorIs(t, err, ErrorMissingFunction)
+	assert.NotEmpty(t, err.Error())
 }
 
 func TestEscapedString(t *testing.T) {
-	kb, _ := NewKeyBuilder().Compile("{0} is \\{1\\} cool\\n\\t\\a")
+	kb, _ := NewKeyBuilder().Compile("{0} is \\{1\\} cool\\n\\t\\a\\r")
 	key := kb.BuildKey(&testContext)
-	assert.Equal(t, "ab is {1} cool\n\ta", key)
+	assert.Equal(t, "ab is {1} cool\n\ta\r", key)
 	assert.Equal(t, 2, len(kb.stages))
 }
 
@@ -67,17 +72,19 @@ func TestStringKey(t *testing.T) {
 
 func TestEmptyStatement(t *testing.T) {
 	kb, err := NewKeyBuilder().Compile("{} test")
-	assert.Nil(t, kb)
+	assert.NotNil(t, kb)
 	assert.Error(t, err)
 }
 
+// BenchmarkSimpleReplacement-4   	 7515498	       141.4 ns/op	      24 B/op	       2 allocs/op
 func BenchmarkSimpleReplacement(b *testing.B) {
 	kb, _ := NewKeyBuilder().Compile("{0} is awesome")
 	for n := 0; n < b.N; n++ {
 		kb.BuildKey(&testContext)
 	}
 }
 
+// BenchmarkGoTextTemplate-4   	 3139363	       406.3 ns/op	     160 B/op	       3 allocs/op
 func BenchmarkGoTextTemplate(b *testing.B) {
 	kb, _ := template.New("test").Parse("{a} is awesome")
 	for n := 0; n < b.N; n++ {
@@ -89,15 +96,18 @@ func BenchmarkGoTextTemplate(b *testing.B) {
 // func tests
 
 var simpleFuncs = map[string]KeyBuilderFunction{
-	"addi": func(args []KeyBuilderStage) KeyBuilderStage {
+	"addi": func(args []KeyBuilderStage) (KeyBuilderStage, error) {
+		if len(args) < 2 {
+			return nil, errors.New("expected at least 2 args")
+		}
 		return func(ctx KeyBuilderContext) string {
 			val, _ := strconv.Atoi(args[0](ctx))
 			for i := 1; i < len(args); i++ {
 				aVal, _ := strconv.Atoi(args[i](ctx))
 				val += aVal
 			}
 			return strconv.Itoa(val)
-		}
+		}, nil
 	},
 }
 
@@ -109,6 +119,24 @@ func TestSimpleFuncs(t *testing.T) {
 	assert.Equal(t, "value: 5", kb.BuildKey(&KeyBuilderContextArray{}))
 }
 
+func TestSimpleFuncErrors(t *testing.T) {
+	k := NewKeyBuilder()
+	k.Funcs(simpleFuncs)
+	kb, err := k.Compile("value: {addi 1} {addi 1 2}")
+	assert.Error(t, err)
+	assert.NotNil(t, kb)
+	assert.Equal(t, "value:  3", kb.BuildKey(&KeyBuilderContextArray{}))
+}
+
+func TestDeepFuncError(t *testing.T) {
+	k := NewKeyBuilder()
+	k.Funcs(simpleFuncs)
+	kb, err := k.Compile("value: {addi 1 {addi 1}} {addi 1 2}")
+	assert.Error(t, err)
+	assert.NotNil(t, kb)
+	assert.Equal(t, "value: 1 3", kb.BuildKey(&KeyBuilderContextArray{}))
+}
+
 func TestManyStages(t *testing.T) {
 	k := NewKeyBuilderEx(false)
 	k.Funcs(simpleFuncs)
@@ -117,6 +145,8 @@ func TestManyStages(t *testing.T) {
 	assert.Equal(t, "value: -1 8", kb.BuildKey(&KeyBuilderContextArray{}))
 }
 
+// Optimization
+
 func TestManyStagesOptimize(t *testing.T) {
 	k := NewKeyBuilderEx(true)
 

pkg/expressions/stage.go

@@ -1,7 +1,6 @@
 package expressions
 
 import (
-	"fmt"
 	"strconv"
 	"strings"
 )
@@ -12,7 +11,7 @@ const (
 )
 
 // KeyBuilderFunction defines a helper function at runtime
-type KeyBuilderFunction func([]KeyBuilderStage) KeyBuilderStage
+type KeyBuilderFunction func([]KeyBuilderStage) (KeyBuilderStage, error)
 
 // KeyBuilderStage is a stage within the compiled builder
 type KeyBuilderStage func(KeyBuilderContext) string
@@ -35,13 +34,6 @@ func stageSimpleVariable(s string) KeyBuilderStage {
 	})
 }
 
-func stageError(msg string) KeyBuilderStage {
-	errMessage := fmt.Sprintf("<%s>", msg)
-	return KeyBuilderStage(func(context KeyBuilderContext) string {
-		return errMessage
-	})
-}
-
 // make a delim-separated array
 func MakeArray(args ...string) string {
 	var sb strings.Builder

pkg/expressions/stageAnalysis_test.go

@@ -39,3 +39,9 @@ func TestEvaluateStageIndex(t *testing.T) {
 	assert.Equal(t, "test2", EvalStageIndexOrDefault(stages, 1, "nope"))
 	assert.Equal(t, "nope", EvalStageIndexOrDefault(stages, 2, "nope"))
 }
+
+func TestEvaluationStageInt(t *testing.T) {
+	assert.Equal(t, 5, EvalStageInt(testStageNoContext("5"), 1))
+	assert.Equal(t, 1, EvalStageInt(testStageNoContext("5b"), 1))
+	assert.Equal(t, 1, EvalStageInt(testStageUseContext("5"), 1))
+}