Another pass at errors to make results more predictable (#93)

* Another pass at errors to make results more predictable

* More tests

cmd/expressions.go

@@ -49,7 +49,7 @@ func expressionFunction(c *cli.Context) error {
 	compiled, err := builder.Compile(expString)
 
 	if err != nil {
-		fmt.Fprint(os.Stderr, err.Error())
+		fmt.Fprintln(os.Stderr, err.Error())
 		return errors.New("compile error")
 	}
 

pkg/expressions/errors.go

@@ -54,6 +54,15 @@ func (s *CompilerErrors) Unwrap() error {
 	return nil
 }
 
+func (s *CompilerErrors) Is(target error) bool {
+	for _, err := range s.Errors {
+		if errors.Is(err, target) {
+			return true
+		}
+	}
+	return false
+}
+
 func (s *CompilerErrors) add(underlying error, context string, offset int) {
 	s.Errors = append(s.Errors, &DetailedError{underlying, context, offset})
 }

pkg/expressions/keyBuilder_test.go

@@ -45,9 +45,16 @@ func TestManyErrors(t *testing.T) {
 	assert.NotNil(t, kb)
 	assert.Error(t, err)
 	assert.Len(t, err.Errors, 2)
+
+	// Test individually
 	assert.ErrorIs(t, err.Errors[0], ErrorMissingFunction)
 	assert.ErrorIs(t, err.Errors[1], ErrorUnterminated)
+
+	// Test unwrap/is
 	assert.ErrorIs(t, err, ErrorMissingFunction)
+	assert.ErrorIs(t, err, ErrorUnterminated)
+	assert.NotErrorIs(t, err, ErrorEmptyStatement)
+	assert.ErrorIs(t, errors.Unwrap(err), ErrorMissingFunction)
 	assert.NotEmpty(t, err.Error())
 }
 

pkg/expressions/stageAnalysis.go

@@ -27,25 +27,41 @@ func EvalStaticStage(stage KeyBuilderStage) (ret string, ok bool) {
 	return
 }
 
-func EvalStageOrDefault(stage KeyBuilderStage, dflt string) string {
-	if val, ok := EvalStaticStage(stage); ok {
-		return val
-	}
-	return dflt
-}
-
+// Eval a stage, but upon any error, returns default instead
+// Deprecated: Consider using EvalStaticStage for better error detection
 func EvalStageIndexOrDefault(stages []KeyBuilderStage, idx int, dflt string) string {
 	if idx < len(stages) {
-		return EvalStageOrDefault(stages[idx], dflt)
+		if val, ok := EvalStaticStage(stages[idx]); ok {
+			return val
+		}
 	}
 	return dflt
 }
 
-func EvalStageInt(stage KeyBuilderStage, dflt int) (int, bool) {
+// Evals stage and parses as int. If fails for any reason (stage eval or parsing), will return false
+func EvalStageInt(stage KeyBuilderStage) (int, bool) {
 	if s, ok := EvalStaticStage(stage); ok {
 		if v, err := strconv.Atoi(s); err == nil {
 			return v, true
 		}
 	}
-	return dflt, false
+	return 0, false
+}
+
+// Evals stage and parses as int. If fails for any reason (stage eval or parsing), will return false
+func EvalStageInt64(stage KeyBuilderStage) (int64, bool) {
+	if s, ok := EvalStaticStage(stage); ok {
+		if v, err := strconv.ParseInt(s, 10, 64); err == nil {
+			return v, true
+		}
+	}
+	return 0, false
+}
+
+// Helper to EvalStageInt
+func EvalArgInt(stages []KeyBuilderStage, idx int, dflt int) (int, bool) {
+	if idx < len(stages) {
+		return EvalStageInt(stages[idx])
+	}
+	return dflt, true
 }

pkg/expressions/stageAnalysis_test.go

@@ -19,16 +19,6 @@ func testStageNoContext(ret string) KeyBuilderStage {
 	}
 }
 
-func TestEvaluateStageStatic(t *testing.T) {
-	stage := testStageNoContext("test")
-	assert.Equal(t, "test", EvalStageOrDefault(stage, "nope"))
-}
-
-func TestEvaluateStageDynamic(t *testing.T) {
-	stage := testStageUseContext("test")
-	assert.Equal(t, "nope", EvalStageOrDefault(stage, "nope"))
-}
-
 func TestEvaluateStageIndex(t *testing.T) {
 	stages := []KeyBuilderStage{
 		testStageUseContext("test1"),
@@ -41,15 +31,48 @@ func TestEvaluateStageIndex(t *testing.T) {
 }
 
 func TestEvaluationStageInt(t *testing.T) {
-	val, ok := EvalStageInt(testStageNoContext("5"), 1)
+	val, ok := EvalStageInt(testStageNoContext("5"))
 	assert.Equal(t, 5, val)
 	assert.True(t, ok)
 
-	val, ok = EvalStageInt(testStageNoContext("5b"), 1)
-	assert.Equal(t, 1, val)
+	val, ok = EvalStageInt(testStageNoContext("5b"))
+	assert.Equal(t, 0, val)
+	assert.False(t, ok)
+
+	val, ok = EvalStageInt(testStageUseContext("5"))
+	assert.Equal(t, 0, val)
+	assert.False(t, ok)
+}
+
+func TestEvaluationStageInt64(t *testing.T) {
+	val, ok := EvalStageInt64(testStageNoContext("5"))
+	assert.Equal(t, int64(5), val)
+	assert.True(t, ok)
+
+	val, ok = EvalStageInt64(testStageNoContext("5b"))
+	assert.Equal(t, int64(0), val)
 	assert.False(t, ok)
 
-	val, ok = EvalStageInt(testStageUseContext("5"), 1)
-	assert.Equal(t, 1, val)
+	val, ok = EvalStageInt64(testStageUseContext("5"))
+	assert.Equal(t, int64(0), val)
 	assert.False(t, ok)
 }
+
+func TestEvaluateArgInt(t *testing.T) {
+	stages := []KeyBuilderStage{
+		testStageUseContext("5"),
+		testStageNoContext("6"),
+	}
+
+	val, ok := EvalArgInt(stages, 0, 1)
+	assert.False(t, ok)
+	assert.Equal(t, val, 0)
+
+	val, ok = EvalArgInt(stages, 5, 1)
+	assert.True(t, ok)
+	assert.Equal(t, val, 1)
+
+	val, ok = EvalArgInt(stages, 1, 1)
+	assert.True(t, ok)
+	assert.Equal(t, val, 6)
+}

pkg/expressions/stdlib/drawing.go

@@ -8,11 +8,20 @@ import (
 	"strings"
 )
 
+// {color "color" content}
 func kfColor(args []KeyBuilderStage) (KeyBuilderStage, error) {
 	if len(args) != 2 {
 		return stageErrArgCount(args, 2)
 	}
-	colorCode, _ := color.LookupColorByName(EvalStageOrDefault(args[0], ""))
+	colorName, colorNameOk := EvalStaticStage(args[0])
+	if !colorNameOk {
+		return stageArgError(ErrConst, 0)
+	}
+
+	colorCode, hasColor := color.LookupColorByName(colorName)
+	if !hasColor {
+		return stageArgError(ErrEnum, 0)
+	}
 
 	return KeyBuilderStage(func(context KeyBuilderContext) string {
 		return color.Wrap(colorCode, args[1](context))
@@ -25,7 +34,10 @@ func kfRepeat(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgCount(args, 2)
 	}
 
-	char := EvalStageOrDefault(args[0], "|")
+	char, charOk := EvalStaticStage(args[0])
+	if !charOk {
+		return stageArgError(ErrConst, 0)
+	}
 
 	return KeyBuilderStage(func(context KeyBuilderContext) string {
 		count, err := strconv.Atoi(args[1](context))
@@ -42,12 +54,12 @@ func kfBar(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgCount(args, 3)
 	}
 
-	maxVal, err := strconv.ParseInt(EvalStageOrDefault(args[1], ""), 10, 64)
-	if err != nil {
+	maxVal, maxValOk := EvalStageInt64(args[1])
+	if !maxValOk {
 		return stageArgError(ErrNum, 1)
 	}
-	maxLen, err := strconv.ParseInt(EvalStageOrDefault(args[2], ""), 10, 64)
-	if err != nil {
+	maxLen, maxLenOk := EvalStageInt64(args[2])
+	if !maxLenOk {
 		return stageArgError(ErrNum, 2)
 	}
 

pkg/expressions/stdlib/drawing_test.go

@@ -11,16 +11,18 @@ func TestRepeatCharacter(t *testing.T) {
 		"aa bbbb")
 	testExpressionErr(t,
 		mockContext("4"),
-		"{repeat a} {repeat a a}",
-		"<ARGN> <BAD-TYPE>",
-		ErrArgCount)
+		"{repeat a} {repeat a a} {repeat {0} {0}}",
+		"<ARGN> <BAD-TYPE> <CONST>",
+		ErrArgCount, ErrConst)
 }
 
 func TestAddingColor(t *testing.T) {
 	testExpression(t,
 		mockContext("what what"),
 		"{color red {0}}",
 		"what what")
+	testExpressionErr(t, mockContext("what what"), "{color bla {0}}", "<ENUM>", ErrEnum)
+	testExpressionErr(t, mockContext("what what"), "{color {0} {0}}", "<CONST>", ErrConst)
 	testExpressionErr(t,
 		mockContext("what waht"),
 		"{color a}", "<ARGN>", ErrArgCount)
@@ -37,5 +39,6 @@ func TestBarGraphErrorCases(t *testing.T) {
 	testExpressionErr(t, mockContext(), "{bar 1}", "<ARGN>", ErrArgCount)
 	testExpression(t, mockContext(), "{bar a 2 3}", "<BAD-TYPE>")
 	testExpressionErr(t, mockContext(), "{bar 3 {1} {2}}", "<BAD-TYPE>", ErrNum)
+	testExpressionErr(t, mockContext(), "{bar 3 3 {2}}", "<BAD-TYPE>", ErrNum)
 	testExpression(t, mockContext("a"), "{bar {0} 5 5}", "<BAD-TYPE>")
 }

pkg/expressions/stdlib/funcsArithmatic.go

@@ -79,9 +79,9 @@ func kfRound(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgRange(args, "1-2")
 	}
 
-	precision := 0
-	if len(args) >= 2 {
-		precision, _ = EvalStageInt(args[1], 0)
+	precision, precisionOk := EvalArgInt(args, 1, 0)
+	if !precisionOk {
+		return stageArgError(ErrConst, 1)
 	}
 
 	return func(context KeyBuilderContext) string {

pkg/expressions/stdlib/funcsArithmatic_test.go

@@ -41,4 +41,7 @@ func TestFloorCeilRound(t *testing.T) {
 	testExpression(t, mockContext("123.126"), "{round {0} 2}", "123.13")
 
 	testExpressionErr(t, mockContext("123.123"), "{floor {0} b}", "<ARGN>", ErrArgCount)
+	testExpressionErr(t, mockContext("123.123"), "{round {0} 1 2}", "<ARGN>", ErrArgCount)
+	testExpressionErr(t, mockContext("123.123"), "{round {0} {0}}", "<CONST>", ErrConst)
+	testExpressionErr(t, mockContext("123.123"), "{round {0} b}", "<CONST>", ErrConst)
 }

pkg/expressions/stdlib/funcsCommon.go

@@ -24,8 +24,8 @@ func kfBucket(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgCount(args, 2)
 	}
 
-	bucketSize, err := strconv.Atoi(EvalStageOrDefault(args[1], ""))
-	if err != nil {
+	bucketSize, bucketSizeOk := EvalStageInt(args[1])
+	if !bucketSizeOk {
 		return stageArgError(ErrNum, 1)
 	}
 
@@ -44,13 +44,13 @@ func kfClamp(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgCount(args, 3)
 	}
 
-	min, minErr := strconv.Atoi(EvalStageOrDefault(args[1], ""))
-	max, maxErr := strconv.Atoi(EvalStageOrDefault(args[2], ""))
+	min, minOk := EvalStageInt(args[1])
+	max, maxOk := EvalStageInt(args[2])
 
-	if minErr != nil {
+	if !minOk {
 		return stageArgError(ErrNum, 1)
 	}
-	if maxErr != nil {
+	if !maxOk {
 		return stageArgError(ErrNum, 2)
 	}
 

pkg/expressions/stdlib/funcsCommon_test.go

@@ -38,4 +38,7 @@ func TestClamp(t *testing.T) {
 	testExpression(t, mockContext("100", "200", "1000", "-10"),
 		"{clamp {0} 50 200}-{clamp {1} 50 200}-{clamp {2} 50 200}-{clamp {3} 50 200}",
 		"100-200-max-min")
+	testExpressionErr(t, mockContext("0"), "{clamp {0} {0} 1}", "<BAD-TYPE>", ErrNum)
+	testExpressionErr(t, mockContext("0"), "{clamp {0} 1 {0}}", "<BAD-TYPE>", ErrNum)
+	testExpressionErr(t, mockContext("0"), "{clamp {0} 1 2 3}", "<ARGN>", ErrArgCount)
 }

pkg/expressions/stdlib/funcsRange.go

@@ -4,7 +4,6 @@ import (
 	. "rare/pkg/expressions" //lint:ignore ST1001 Legacy
 	"rare/pkg/slicepool"
 	"rare/pkg/stringSplitter"
-	"strconv"
 	"strings"
 )
 
@@ -79,8 +78,8 @@ func kfArraySelect(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgCount(args, 2)
 	}
 
-	index, err := strconv.Atoi(EvalStageOrDefault(args[1], ""))
-	if err != nil {
+	index, indexOk := EvalStageInt(args[1])
+	if !indexOk {
 		return stageArgError(ErrNum, 1)
 	}
 
@@ -172,16 +171,14 @@ func kfArraySlice(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgRange(args, "2-3")
 	}
 
-	sliceStart, ok := EvalStageInt(args[1], 0)
+	sliceStart, ok := EvalStageInt(args[1])
 	if !ok {
 		return stageArgError(ErrConst, 1)
 	}
-	var sliceLen int = -1
-	if len(args) >= 3 {
-		sliceLen, ok = EvalStageInt(args[2], -1)
-		if !ok {
-			return stageArgError(ErrConst, 2)
-		}
+
+	sliceLen, sliceLenOk := EvalArgInt(args, 2, -1)
+	if !sliceLenOk {
+		return stageArgError(ErrConst, 2)
 	}
 
 	return func(context KeyBuilderContext) string {

pkg/expressions/stdlib/funcsRange_test.go

@@ -175,6 +175,13 @@ func TestArraySlice(t *testing.T) {
 		"<ARGN>",
 		ErrArgCount,
 	)
+	testExpressionErr(
+		t,
+		mockContext("0 1 2 5"),
+		`{@join {@slice {@split {0} " "} 1 bla}}`,
+		"<CONST>",
+		ErrConst,
+	)
 }
 
 func TestArrayFilter(t *testing.T) {

pkg/expressions/stdlib/funcsStrings.go

@@ -168,13 +168,9 @@ func kfPercent(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgRange(args, "1-2")
 	}
 
-	decimals := 1
-	if len(args) >= 2 {
-		var ok bool
-		decimals, ok = EvalStageInt(args[1], decimals)
-		if !ok {
-			return stageArgError(ErrConst, 1)
-		}
+	decimals, hasDecimals := EvalArgInt(args, 1, 1)
+	if !hasDecimals {
+		return stageArgError(ErrConst, 1)
 	}
 
 	return func(context KeyBuilderContext) string {

pkg/expressions/stdlib/funcsTime.go

@@ -223,7 +223,12 @@ func kfBucketTime(args []KeyBuilderStage) (KeyBuilderStage, error) {
 		return stageErrArgRange(args, "2-4")
 	}
 
-	bucketFormat := timeBucketToFormat(EvalStageOrDefault(args[1], "day"))
+	bucketName, bucketNameOk := EvalStaticStage(args[1])
+	if !bucketNameOk {
+		return stageArgError(ErrConst, 1)
+	}
+
+	bucketFormat := timeBucketToFormat(bucketName)
 	if bucketFormat == "" {
 		return stageArgError(ErrEnum, 1)
 	}