test pkg/exprhelpers: explicit message if the tag "expr_debug" is missing (#3400)

* test pkg/exprhelpers: explicit message if the tag "expr_debug" is missing

* typo

* lint: use build tag expr_debug while linting

* lint

.github/workflows/go-tests.yml

@@ -143,11 +143,11 @@ jobs:
         go generate ./...
         protoc --version
         if [[ $(git status --porcelain) ]]; then
-          echo "Error: Uncommitted changes found after running 'make generate'. Please commit all generated code."
+          echo "Error: Uncommitted changes found after running 'go generate'. Please commit all generated code."
           git diff
           exit 1
         else
-          echo "No changes detected after running 'make generate'."
+          echo "No changes detected after running 'go generate'."
         fi
 
     - name: Create localstack streams

.golangci.yml

@@ -1,5 +1,9 @@
 # https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml
 
+run:
+  build-tags:
+    - expr_debug
+
 linters-settings:
   gci:
     sections:

pkg/exprhelpers/debugger.go

@@ -21,35 +21,35 @@ var IndentStep = 4
 
 // we use this struct to store the output of the expr runtime
 type OpOutput struct {
-	Code string //relevant code part
+	Code string // relevant code part
 
-	CodeDepth  int //level of nesting
+	CodeDepth  int // level of nesting
 	BlockStart bool
 	BlockEnd   bool
 
-	Func        bool //true if it's a function call
+	Func        bool // true if it's a function call
 	FuncName    string
 	Args        []string
 	FuncResults []string
 	//
-	Comparison bool //true if it's a comparison
+	Comparison bool // true if it's a comparison
 	Negated    bool
 	Left       string
 	Right      string
 	//
-	JumpIf  bool //true if it's conditional jump
+	JumpIf  bool // true if it's conditional jump
 	IfTrue  bool
 	IfFalse bool
 	//
-	Condition         bool //true if it's a condition
+	Condition         bool // true if it's a condition
 	ConditionIn       bool
 	ConditionContains bool
-	//used for comparisons, conditional jumps and conditions
+	// used for comparisons, conditional jumps and conditions
 	StrConditionResult string
-	ConditionResult    *bool //should always be present for conditions
+	ConditionResult    *bool // should always be present for conditions
 
 	//
-	Finalized bool //used when a node is finalized, we already fetched result from next OP
+	Finalized bool // used when a node is finalized, we already fetched result from next OP
 }
 
 func (o *OpOutput) String() string {
@@ -57,6 +57,7 @@ func (o *OpOutput) String() string {
 	if o.Code != "" {
 		ret += fmt.Sprintf("[%s]", o.Code)
 	}
+
 	ret += " "
 
 	switch {
@@ -68,19 +69,24 @@ func (o *OpOutput) String() string {
 		if indent < 0 {
 			indent = 0
 		}
+
 		ret = fmt.Sprintf("%*cBLOCK_END [%s]", indent, ' ', o.Code)
+
 		if o.StrConditionResult != "" {
 			ret += fmt.Sprintf(" -> %s", o.StrConditionResult)
 		}
+
 		return ret
-		//A block end can carry a value, for example if it's a count, any, all etc. XXX
+		// A block end can carry a value, for example if it's a count, any, all etc. XXX
 	case o.Func:
 		return ret + fmt.Sprintf("%s(%s) = %s", o.FuncName, strings.Join(o.Args, ", "), strings.Join(o.FuncResults, ", "))
 	case o.Comparison:
 		if o.Negated {
 			ret += "NOT "
 		}
+
 		ret += fmt.Sprintf("%s == %s -> %s", o.Left, o.Right, o.StrConditionResult)
+
 		return ret
 	case o.ConditionIn:
 		return ret + fmt.Sprintf("%s in %s -> %s", o.Args[0], o.Args[1], o.StrConditionResult)
@@ -91,18 +97,23 @@ func (o *OpOutput) String() string {
 			if *o.ConditionResult {
 				return ret + "OR -> false"
 			}
+
 			return ret + "OR -> true"
 		}
+
 		return ret + "OR(?)"
 	case o.JumpIf && o.IfFalse:
 		if o.ConditionResult != nil {
 			if *o.ConditionResult {
 				return ret + "AND -> true"
 			}
+
 			return ret + "AND -> false"
 		}
+
 		return ret + "AND(?)"
 	}
+
 	return ret + ""
 }
 
@@ -135,7 +146,7 @@ func (erp ExprRuntimeDebug) extractCode(ip int, program *vm.Program) string {
 func autoQuote(v any) string {
 	switch x := v.(type) {
 	case string:
-		//let's avoid printing long strings. it can happen ie. when we are debugging expr with `File()` or similar helpers
+		// let's avoid printing long strings. it can happen ie. when we are debugging expr with `File()` or similar helpers
 		if len(x) > 40 {
 			return fmt.Sprintf("%q", x[:40]+"...")
 		} else {
@@ -147,35 +158,40 @@ func autoQuote(v any) string {
 }
 
 func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, parts []string, outputs []OpOutput) ([]OpOutput, error) {
-
 	IdxOut := len(outputs)
 	prevIdxOut := 0
 	currentDepth := 0
 
-	//when there is a function call or comparison, we need to wait for the next instruction to get the result and "finalize" the previous one
+	// when there is a function call or comparison, we need to wait for the next instruction to get the result and "finalize" the previous one
 	if IdxOut > 0 {
 		prevIdxOut = IdxOut - 1
 		currentDepth = outputs[prevIdxOut].CodeDepth
+
 		if outputs[prevIdxOut].Func && !outputs[prevIdxOut].Finalized {
 			stack := vm.Stack
 			num_items := 1
+
 			for i := len(stack) - 1; i >= 0 && num_items > 0; i-- {
 				outputs[prevIdxOut].FuncResults = append(outputs[prevIdxOut].FuncResults, autoQuote(stack[i]))
 				num_items--
 			}
+
 			outputs[prevIdxOut].Finalized = true
 		} else if (outputs[prevIdxOut].Comparison || outputs[prevIdxOut].Condition) && !outputs[prevIdxOut].Finalized {
 			stack := vm.Stack
 			outputs[prevIdxOut].StrConditionResult = fmt.Sprintf("%+v", stack)
+
 			if val, ok := stack[0].(bool); ok {
 				outputs[prevIdxOut].ConditionResult = new(bool)
 				*outputs[prevIdxOut].ConditionResult = val
 			}
+
 			outputs[prevIdxOut].Finalized = true
 		}
 	}
 
 	erp.Logger.Tracef("[STEP %d:%s] (stack:%+v) (parts:%+v) {depth:%d}", ip, parts[1], vm.Stack, parts, currentDepth)
+
 	out := OpOutput{}
 	out.CodeDepth = currentDepth
 	out.Code = erp.extractCode(ip, program)
@@ -188,27 +204,28 @@ func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, part
 	case "OpEnd":
 		out.CodeDepth -= IndentStep
 		out.BlockEnd = true
-		//OpEnd can carry value, if it's any/all/count etc.
+		// OpEnd can carry value, if it's any/all/count etc.
 		if len(vm.Stack) > 0 {
 			out.StrConditionResult = fmt.Sprintf("%v", vm.Stack)
 		}
+
 		outputs = append(outputs, out)
 	case "OpNot":
-		//negate the previous condition
+		// negate the previous condition
 		outputs[prevIdxOut].Negated = true
-	case "OpTrue": //generated when possible ? (1 == 1)
+	case "OpTrue": // generated when possible ? (1 == 1)
 		out.Condition = true
 		out.ConditionResult = new(bool)
 		*out.ConditionResult = true
 		out.StrConditionResult = "true"
 		outputs = append(outputs, out)
-	case "OpFalse": //generated when possible ? (1 != 1)
+	case "OpFalse": // generated when possible ? (1 != 1)
 		out.Condition = true
 		out.ConditionResult = new(bool)
 		*out.ConditionResult = false
 		out.StrConditionResult = "false"
 		outputs = append(outputs, out)
-	case "OpJumpIfTrue": //OR
+	case "OpJumpIfTrue": // OR
 		stack := vm.Stack
 		out.JumpIf = true
 		out.IfTrue = true
@@ -218,78 +235,88 @@ func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, part
 			out.ConditionResult = new(bool)
 			*out.ConditionResult = val
 		}
+
 		outputs = append(outputs, out)
-	case "OpJumpIfFalse": //AND
+	case "OpJumpIfFalse": // AND
 		stack := vm.Stack
 		out.JumpIf = true
 		out.IfFalse = true
 		out.StrConditionResult = fmt.Sprintf("%v", stack[0])
+
 		if val, ok := stack[0].(bool); ok {
 			out.ConditionResult = new(bool)
 			*out.ConditionResult = val
 		}
+
 		outputs = append(outputs, out)
-	case "OpCall1": //Op for function calls
+	case "OpCall1": // Op for function calls
 		out.Func = true
 		out.FuncName = parts[3]
 		stack := vm.Stack
+
 		num_items := 1
 		for i := len(stack) - 1; i >= 0 && num_items > 0; i-- {
 			out.Args = append(out.Args, autoQuote(stack[i]))
 			num_items--
 		}
+
 		outputs = append(outputs, out)
-	case "OpCall2": //Op for function calls
+	case "OpCall2": // Op for function calls
 		out.Func = true
 		out.FuncName = parts[3]
 		stack := vm.Stack
+
 		num_items := 2
 		for i := len(stack) - 1; i >= 0 && num_items > 0; i-- {
 			out.Args = append(out.Args, autoQuote(stack[i]))
 			num_items--
 		}
+
 		outputs = append(outputs, out)
-	case "OpCall3": //Op for function calls
+	case "OpCall3": // Op for function calls
 		out.Func = true
 		out.FuncName = parts[3]
 		stack := vm.Stack
+
 		num_items := 3
 		for i := len(stack) - 1; i >= 0 && num_items > 0; i-- {
 			out.Args = append(out.Args, autoQuote(stack[i]))
 			num_items--
 		}
+
 		outputs = append(outputs, out)
-	//double check OpCallFast and OpCallTyped
+	// double check OpCallFast and OpCallTyped
 	case "OpCallFast", "OpCallTyped":
 		//
-	case "OpCallN": //Op for function calls with more than 3 args
+	case "OpCallN": // Op for function calls with more than 3 args
 		out.Func = true
 		out.FuncName = parts[1]
 		stack := vm.Stack
 
-		//for OpCallN, we get the number of args
+		// for OpCallN, we get the number of args
 		if len(program.Arguments) >= ip {
 			nb_args := program.Arguments[ip]
 			if nb_args > 0 {
-				//we need to skip the top item on stack
+				// we need to skip the top item on stack
 				for i := len(stack) - 2; i >= 0 && nb_args > 0; i-- {
 					out.Args = append(out.Args, autoQuote(stack[i]))
 					nb_args--
 				}
 			}
-		} else { //let's blindly take the items on stack
+		} else { // let's blindly take the items on stack
 			for _, val := range vm.Stack {
 				out.Args = append(out.Args, autoQuote(val))
 			}
 		}
+
 		outputs = append(outputs, out)
-	case "OpEqualString", "OpEqual", "OpEqualInt": //comparisons
+	case "OpEqualString", "OpEqual", "OpEqualInt": // comparisons
 		stack := vm.Stack
 		out.Comparison = true
 		out.Left = autoQuote(stack[0])
 		out.Right = autoQuote(stack[1])
 		outputs = append(outputs, out)
-	case "OpIn": //in operator
+	case "OpIn": // in operator
 		stack := vm.Stack
 		out.Condition = true
 		out.ConditionIn = true
@@ -299,7 +326,7 @@ func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, part
 		out.Args = append(out.Args, autoQuote(stack[0]))
 		out.Args = append(out.Args, autoQuote(stack[1]))
 		outputs = append(outputs, out)
-	case "OpContains": //kind OpIn , but reverse
+	case "OpContains": // kind OpIn , but reverse
 		stack := vm.Stack
 		out.Condition = true
 		out.ConditionContains = true
@@ -310,6 +337,7 @@ func (erp ExprRuntimeDebug) ipDebug(ip int, vm *vm.VM, program *vm.Program, part
 		out.Args = append(out.Args, autoQuote(stack[1]))
 		outputs = append(outputs, out)
 	}
+
 	return outputs, nil
 }
 
@@ -319,10 +347,12 @@ func (erp ExprRuntimeDebug) ipSeek(ip int) []string {
 		if len(parts) == 0 {
 			continue
 		}
+
 		if parts[0] == strconv.Itoa(ip) {
 			return parts
 		}
 	}
+
 	return nil
 }
 
@@ -330,19 +360,23 @@ func Run(program *vm.Program, env interface{}, logger *log.Entry, debug bool) (a
 	if debug {
 		dbgInfo, ret, err := RunWithDebug(program, env, logger)
 		DisplayExprDebug(program, dbgInfo, logger, ret)
+
 		return ret, err
 	}
+
 	return expr.Run(program, env)
 }
 
 func cleanTextForDebug(text string) string {
 	text = strings.Join(strings.Fields(text), " ")
 	text = strings.Trim(text, " \t\n")
+
 	return text
 }
 
 func DisplayExprDebug(program *vm.Program, outputs []OpOutput, logger *log.Entry, ret any) {
 	logger.Debugf("dbg(result=%v): %s", ret, cleanTextForDebug(string(program.Source())))
+
 	for _, output := range outputs {
 		logger.Debugf("%s", output.String())
 	}
@@ -360,46 +394,55 @@ func RunWithDebug(program *vm.Program, env interface{}, logger *log.Entry) ([]Op
 	erp.Lines = lines
 
 	go func() {
-		//We must never return until the execution of the program is done
+		// We must never return until the execution of the program is done
 		var err error
+
 		erp.Logger.Tracef("[START] ip 0")
+
 		ops := erp.ipSeek(0)
 		if ops == nil {
 			log.Warningf("error while debugging expr: failed getting ops for ip 0")
 		}
+
 		if outputs, err = erp.ipDebug(0, vm, program, ops, outputs); err != nil {
 			log.Warningf("error while debugging expr: error while debugging at ip 0")
 		}
+
 		vm.Step()
+
 		for ip := range vm.Position() {
 			ops := erp.ipSeek(ip)
 			if ops == nil {
 				erp.Logger.Tracef("[DONE] ip %d", ip)
 				break
 			}
+
 			if outputs, err = erp.ipDebug(ip, vm, program, ops, outputs); err != nil {
 				log.Warningf("error while debugging expr: error while debugging at ip %d", ip)
 			}
+
 			vm.Step()
 		}
 	}()
 
 	var return_error error
+
 	ret, err := vm.Run(program, env)
-	//if the expr runtime failed, we don't need to wait for the debug to finish
+	// if the expr runtime failed, we don't need to wait for the debug to finish
 	if err != nil {
 		return_error = err
 	}
-	//the overall result of expression is the result of last op ?
+	// the overall result of expression is the result of last op ?
 	if len(outputs) > 0 {
 		lastOutIdx := len(outputs)
 		if lastOutIdx > 0 {
 			lastOutIdx -= 1
 		}
+
 		switch val := ret.(type) {
 		case bool:
 			log.Tracef("completing with bool %t", ret)
-			//if outputs[lastOutIdx].Comparison {
+			// if outputs[lastOutIdx].Comparison {
 			outputs[lastOutIdx].StrConditionResult = fmt.Sprintf("%v", ret)
 			outputs[lastOutIdx].ConditionResult = new(bool)
 			*outputs[lastOutIdx].ConditionResult = val
@@ -412,5 +455,6 @@ func RunWithDebug(program *vm.Program, env interface{}, logger *log.Entry) ([]Op
 	} else {
 		log.Tracef("no output from expr runtime")
 	}
+
 	return outputs, ret, return_error
 }

pkg/exprhelpers/debugger_test.go

@@ -1,3 +1,4 @@
+//go:build expr_debug
 package exprhelpers
 
 import (

pkg/exprhelpers/debuggerstub_test.go

@@ -0,0 +1,10 @@
+//go:build !expr_debug
+package exprhelpers
+
+import (
+	"testing"
+)
+
+func TestFailWithoutExprDebug(t *testing.T) {
+	t.Fatal("To test pkg/exprhelpers, you need the expr_debug build tag")
+}