Add line/column info for DSL runtime non-parse failures (#998)

* Add line/column info for DSL runtime non-parse failures

* Other related callsites

* test cases

* Update already-existing test cases

internal/pkg/dsl/cst/cond.go

@@ -11,11 +11,13 @@ import (
 	"github.com/johnkerl/miller/internal/pkg/dsl"
 	"github.com/johnkerl/miller/internal/pkg/lib"
 	"github.com/johnkerl/miller/internal/pkg/mlrval"
+	"github.com/johnkerl/miller/internal/pkg/parsing/token"
 	"github.com/johnkerl/miller/internal/pkg/runtime"
 )
 
 type CondBlockNode struct {
 	conditionNode      IEvaluable
+	conditionToken     *token.Token
 	statementBlockNode *StatementBlockNode
 }
 
@@ -30,12 +32,14 @@ func (root *RootNode) BuildCondBlockNode(astNode *dsl.ASTNode) (*CondBlockNode,
 	if err != nil {
 		return nil, err
 	}
+	conditionToken := astNode.Children[0].Token
 	statementBlockNode, err := root.BuildStatementBlockNode(astNode.Children[1])
 	if err != nil {
 		return nil, err
 	}
 	condBlockNode := &CondBlockNode{
 		conditionNode:      conditionNode,
+		conditionToken:     conditionToken,
 		statementBlockNode: statementBlockNode,
 	}
 
@@ -56,8 +60,10 @@ func (node *CondBlockNode) Execute(
 	if condition.IsAbsent() {
 		boolValue = false
 	} else if !isBool {
-		// TODO: line-number/token info for the DSL expression.
-		return nil, fmt.Errorf("mlr: conditional expression did not evaluate to boolean.")
+		return nil, fmt.Errorf(
+			"mlr: conditional expression did not evaluate to boolean%s.",
+			dsl.TokenToLocationInfo(node.conditionToken),
+		)
 	}
 
 	if boolValue == true {

internal/pkg/dsl/cst/for.go

@@ -10,6 +10,7 @@ import (
 	"github.com/johnkerl/miller/internal/pkg/dsl"
 	"github.com/johnkerl/miller/internal/pkg/lib"
 	"github.com/johnkerl/miller/internal/pkg/mlrval"
+	"github.com/johnkerl/miller/internal/pkg/parsing/token"
 	"github.com/johnkerl/miller/internal/pkg/runtime"
 )
 
@@ -705,17 +706,19 @@ func (node *ForLoopMultivariableNode) executeInner(
 
 // ================================================================
 type TripleForLoopNode struct {
-	startBlockNode             *StatementBlockNode
-	precontinuationAssignments []IExecutable
-	continuationExpressionNode IEvaluable
-	updateBlockNode            *StatementBlockNode
-	bodyBlockNode              *StatementBlockNode
+	startBlockNode              *StatementBlockNode
+	precontinuationAssignments  []IExecutable
+	continuationExpressionNode  IEvaluable
+	continuationExpressionToken *token.Token
+	updateBlockNode             *StatementBlockNode
+	bodyBlockNode               *StatementBlockNode
 }
 
 func NewTripleForLoopNode(
 	startBlockNode *StatementBlockNode,
 	precontinuationAssignments []IExecutable,
 	continuationExpressionNode IEvaluable,
+	continuationExpressionToken *token.Token,
 	updateBlockNode *StatementBlockNode,
 	bodyBlockNode *StatementBlockNode,
 ) *TripleForLoopNode {
@@ -723,6 +726,7 @@ func NewTripleForLoopNode(
 		startBlockNode,
 		precontinuationAssignments,
 		continuationExpressionNode,
+		continuationExpressionToken,
 		updateBlockNode,
 		bodyBlockNode,
 	}
@@ -793,6 +797,7 @@ func (root *RootNode) BuildTripleForLoopNode(astNode *dsl.ASTNode) (*TripleForLo
 	// for (int i = 0; c += 1, i < 10; i += 1) { ... }
 	var precontinuationAssignments []IExecutable = nil
 	var continuationExpressionNode IEvaluable = nil
+	var continuationExpressionToken *token.Token = nil
 	if len(continuationExpressionASTNode.Children) > 0 { // empty is true
 		n := len(continuationExpressionASTNode.Children)
 		if n > 1 {
@@ -827,6 +832,7 @@ func (root *RootNode) BuildTripleForLoopNode(astNode *dsl.ASTNode) (*TripleForLo
 		}
 		lib.InternalCodingErrorIf(len(bareBooleanASTNode.Children) != 1)
 		continuationExpressionNode, err = root.BuildEvaluableNode(bareBooleanASTNode.Children[0])
+		continuationExpressionToken = bareBooleanASTNode.Children[0].Token
 		if err != nil {
 			return nil, err
 		}
@@ -846,6 +852,7 @@ func (root *RootNode) BuildTripleForLoopNode(astNode *dsl.ASTNode) (*TripleForLo
 		startBlockNode,
 		precontinuationAssignments,
 		continuationExpressionNode,
+		continuationExpressionToken,
 		updateBlockNode,
 		bodyBlockNode,
 	), nil
@@ -892,8 +899,10 @@ func (node *TripleForLoopNode) Execute(state *runtime.State) (*BlockExitPayload,
 			continuationValue := node.continuationExpressionNode.Evaluate(state)
 			boolValue, isBool := continuationValue.GetBoolValue()
 			if !isBool {
-				// TODO: propagate line-number context
-				return nil, fmt.Errorf("mlr: for-loop continuation did not evaluate to boolean.")
+				return nil, fmt.Errorf(
+					"mlr: for-loop continuation did not evaluate to boolean%s.",
+					dsl.TokenToLocationInfo(node.continuationExpressionToken),
+				)
 			}
 			if boolValue == false {
 				break

internal/pkg/dsl/cst/if.go

@@ -10,6 +10,7 @@ import (
 	"github.com/johnkerl/miller/internal/pkg/dsl"
 	"github.com/johnkerl/miller/internal/pkg/lib"
 	"github.com/johnkerl/miller/internal/pkg/mlrval"
+	"github.com/johnkerl/miller/internal/pkg/parsing/token"
 	"github.com/johnkerl/miller/internal/pkg/runtime"
 )
 
@@ -29,6 +30,7 @@ func NewIfChainNode(ifItems []*IfItem) *IfChainNode {
 // statement-block part {...}. For "else", the conditional is nil.
 type IfItem struct {
 	conditionNode      IEvaluable
+	conditionToken     *token.Token
 	statementBlockNode *StatementBlockNode
 }
 
@@ -92,6 +94,7 @@ func (root *RootNode) BuildIfChainNode(astNode *dsl.ASTNode) (*IfChainNode, erro
 			}
 			ifItem := &IfItem{
 				conditionNode:      conditionNode,
+				conditionToken:     astChild.Children[0].Token,
 				statementBlockNode: statementBlockNode,
 			}
 			ifItems = append(ifItems, ifItem)
@@ -126,8 +129,10 @@ func (node *IfChainNode) Execute(state *runtime.State) (*BlockExitPayload, error
 		}
 		boolValue, isBool := condition.GetBoolValue()
 		if !isBool {
-			// TODO: line-number/token info for the DSL expression.
-			return nil, fmt.Errorf("mlr: conditional expression did not evaluate to boolean.")
+			return nil, fmt.Errorf(
+				"mlr: conditional expression did not evaluate to boolean%s.",
+				dsl.TokenToLocationInfo(ifItem.conditionToken),
+			)
 		}
 		if boolValue == true {
 			blockExitPayload, err := ifItem.statementBlockNode.Execute(state)

internal/pkg/dsl/cst/warn.go

@@ -101,16 +101,11 @@ func warnOnASTAux(
 			variableNamesWrittenTo[variableName] = true
 		} else {
 			if !variableNamesWrittenTo[variableName] {
-				// TODO: this would be much more useful with line numbers. :(
-				// That would be a big of work with the parser.  Fortunately,
-				// Miller is designed around low-keystroke little expressions
-				// -- not thousands of lines of Miller-DSL source code -- so
-				// people can look at their few lines of Miller-DSL code and
-				// spot their error.
 				fmt.Fprintf(
 					os.Stderr,
-					"Variable name %s might not have been assigned yet.\n",
+					"Variable name %s might not have been assigned yet%s.\n",
 					variableName,
+					dsl.TokenToLocationInfo(astNode.Token),
 				)
 				ok = false
 			}

internal/pkg/dsl/cst/while.go

@@ -9,21 +9,25 @@ import (
 
 	"github.com/johnkerl/miller/internal/pkg/dsl"
 	"github.com/johnkerl/miller/internal/pkg/lib"
+	"github.com/johnkerl/miller/internal/pkg/parsing/token"
 	"github.com/johnkerl/miller/internal/pkg/runtime"
 )
 
 // ================================================================
 type WhileLoopNode struct {
 	conditionNode      IEvaluable
+	conditionToken     *token.Token
 	statementBlockNode *StatementBlockNode
 }
 
 func NewWhileLoopNode(
 	conditionNode IEvaluable,
+	conditionToken *token.Token,
 	statementBlockNode *StatementBlockNode,
 ) *WhileLoopNode {
 	return &WhileLoopNode{
 		conditionNode:      conditionNode,
+		conditionToken:     conditionToken,
 		statementBlockNode: statementBlockNode,
 	}
 }
@@ -36,6 +40,7 @@ func (root *RootNode) BuildWhileLoopNode(astNode *dsl.ASTNode) (*WhileLoopNode,
 	if err != nil {
 		return nil, err
 	}
+	conditionToken := astNode.Children[0].Token
 	statementBlockNode, err := root.BuildStatementBlockNode(astNode.Children[1])
 	if err != nil {
 		return nil, err
@@ -43,6 +48,7 @@ func (root *RootNode) BuildWhileLoopNode(astNode *dsl.ASTNode) (*WhileLoopNode,
 
 	return NewWhileLoopNode(
 		conditionNode,
+		conditionToken,
 		statementBlockNode,
 	), nil
 }
@@ -53,8 +59,10 @@ func (node *WhileLoopNode) Execute(state *runtime.State) (*BlockExitPayload, err
 		condition := node.conditionNode.Evaluate(state)
 		boolValue, isBool := condition.GetBoolValue()
 		if !isBool {
-			// TODO: line-number/token info for the DSL expression.
-			return nil, fmt.Errorf("mlr: conditional expression did not evaluate to boolean.")
+			return nil, fmt.Errorf(
+				"mlr: conditional expression did not evaluate to boolean%s.",
+				dsl.TokenToLocationInfo(node.conditionToken),
+			)
 		}
 		if boolValue != true {
 			break
@@ -86,15 +94,18 @@ func (node *WhileLoopNode) Execute(state *runtime.State) (*BlockExitPayload, err
 type DoWhileLoopNode struct {
 	statementBlockNode *StatementBlockNode
 	conditionNode      IEvaluable
+	conditionToken     *token.Token
 }
 
 func NewDoWhileLoopNode(
 	statementBlockNode *StatementBlockNode,
 	conditionNode IEvaluable,
+	conditionToken *token.Token,
 ) *DoWhileLoopNode {
 	return &DoWhileLoopNode{
 		statementBlockNode: statementBlockNode,
 		conditionNode:      conditionNode,
+		conditionToken:     conditionToken,
 	}
 }
 
@@ -110,10 +121,12 @@ func (root *RootNode) BuildDoWhileLoopNode(astNode *dsl.ASTNode) (*DoWhileLoopNo
 	if err != nil {
 		return nil, err
 	}
+	conditionToken := astNode.Children[1].Token
 
 	return NewDoWhileLoopNode(
 		statementBlockNode,
 		conditionNode,
+		conditionToken,
 	), nil
 }
 
@@ -143,8 +156,10 @@ func (node *DoWhileLoopNode) Execute(state *runtime.State) (*BlockExitPayload, e
 		condition := node.conditionNode.Evaluate(state)
 		boolValue, isBool := condition.GetBoolValue()
 		if !isBool {
-			// TODO: line-number/token info for the DSL expression.
-			return nil, fmt.Errorf("mlr: conditional expression did not evaluate to boolean.")
+			return nil, fmt.Errorf(
+				"mlr: conditional expression did not evaluate to boolean%s.",
+				dsl.TokenToLocationInfo(node.conditionToken),
+			)
 		}
 		if boolValue == false {
 			break

internal/pkg/dsl/token.go

@@ -0,0 +1,17 @@
+package dsl
+
+import (
+	"fmt"
+
+	"github.com/johnkerl/miller/internal/pkg/parsing/token"
+)
+
+// TokenToLocationInfo is used to track runtime errors back to source-code locations in DSL
+// expressions, so we can have more informative error messages.
+func TokenToLocationInfo(sourceToken *token.Token) string {
+	if sourceToken == nil {
+		return ""
+	} else {
+		return fmt.Sprintf(" at DSL expression line %d column %d", sourceToken.Pos.Line, sourceToken.Pos.Column)
+	}
+}

test/cases/dsl-line-number-column-number/cond/cmd

@@ -0,0 +1 @@
+mlr -n put -f ${CASEDIR}/mlr

test/cases/dsl-line-number-column-number/cond/experr

@@ -0,0 +1 @@
+mlr: conditional expression did not evaluate to boolean at DSL expression line 5 column 3.

test/cases/dsl-line-number-column-number/cond/mlr

@@ -0,0 +1,7 @@
+# line padding
+# line padding
+# line padding
+end {
+  0 {
+  }
+}

test/cases/dsl-line-number-column-number/do-while/cmd

@@ -0,0 +1 @@
+mlr -n put -f ${CASEDIR}/mlr

test/cases/dsl-line-number-column-number/do-while/experr

@@ -0,0 +1 @@
+mlr: conditional expression did not evaluate to boolean at DSL expression line 6 column 12.

test/cases/dsl-line-number-column-number/do-while/mlr

@@ -0,0 +1,7 @@
+# line padding
+# line padding
+# line padding
+end {
+  do {
+  } while (0);
+}

test/cases/dsl-line-number-column-number/for/cmd

@@ -0,0 +1 @@
+mlr -n put -f ${CASEDIR}/mlr

test/cases/dsl-line-number-column-number/for/experr

@@ -0,0 +1 @@
+mlr: for-loop continuation did not evaluate to boolean at DSL expression line 5 column 9.

test/cases/dsl-line-number-column-number/for/mlr

@@ -0,0 +1,7 @@
+# line padding
+# line padding
+# line padding
+end {
+  for (;0;) {
+  }
+}

test/cases/dsl-line-number-column-number/if/cmd

@@ -0,0 +1 @@
+mlr -n put -f ${CASEDIR}/mlr

test/cases/dsl-line-number-column-number/if/experr

@@ -0,0 +1 @@
+mlr: conditional expression did not evaluate to boolean at DSL expression line 5 column 7.

test/cases/dsl-line-number-column-number/if/mlr

@@ -0,0 +1,7 @@
+# line padding
+# line padding
+# line padding
+end {
+  if (0) {
+  }
+}

test/cases/dsl-line-number-column-number/warn/cmd

@@ -0,0 +1 @@
+mlr -n put -w -f ${CASEDIR}/mlr

test/cases/dsl-line-number-column-number/warn/experr

@@ -0,0 +1 @@
+Variable name y might not have been assigned yet at DSL expression line 5 column 7.

test/cases/dsl-line-number-column-number/warn/mlr

@@ -0,0 +1,6 @@
+# line padding
+# line padding
+# line padding
+end {
+  x = y;
+}

test/cases/dsl-line-number-column-number/while/cmd

@@ -0,0 +1 @@
+mlr -n put -f ${CASEDIR}/mlr

test/cases/dsl-line-number-column-number/while/experr

@@ -0,0 +1 @@
+mlr: conditional expression did not evaluate to boolean at DSL expression line 5 column 10.

test/cases/dsl-line-number-column-number/while/mlr

@@ -0,0 +1,7 @@
+# line padding
+# line padding
+# line padding
+end {
+  while (0) {
+  }
+}

test/cases/dsl-warnings/0014/experr

@@ -1 +1 @@
-Variable name x might not have been assigned yet.
+Variable name x might not have been assigned yet at DSL expression line 1 column 5.

test/cases/dsl-warnings/0015/experr

@@ -1 +1 @@
-Variable name x might not have been assigned yet.
+Variable name x might not have been assigned yet at DSL expression line 1 column 5.

test/cases/dsl-warnings/0017/experr

@@ -1,2 +1,2 @@
-Variable name x might not have been assigned yet.
-Variable name y might not have been assigned yet.
+Variable name x might not have been assigned yet at DSL expression line 1 column 5.
+Variable name y might not have been assigned yet at DSL expression line 1 column 9.

test/cases/dsl-warnings/0018/experr

@@ -1,2 +1,2 @@
-Variable name x might not have been assigned yet.
-Variable name y might not have been assigned yet.
+Variable name x might not have been assigned yet at DSL expression line 1 column 5.
+Variable name y might not have been assigned yet at DSL expression line 1 column 9.

test/cases/dsl-warnings/0020/experr

@@ -1 +1 @@
-Variable name i might not have been assigned yet.
+Variable name i might not have been assigned yet at DSL expression line 1 column 3.

test/cases/dsl-warnings/0022/experr

@@ -1 +1 @@
-Variable name m might not have been assigned yet.
+Variable name m might not have been assigned yet at DSL expression line 1 column 20.

test/cases/dsl-warnings/0023/experr

@@ -1 +1 @@
-Variable name m might not have been assigned yet.
+Variable name m might not have been assigned yet at DSL expression line 1 column 27.

test/cases/dsl-warnings/0024/experr

@@ -1 +1 @@
-Variable name m might not have been assigned yet.
+Variable name m might not have been assigned yet at DSL expression line 1 column 19.

test/cases/dsl-warnings/0025/experr

@@ -1 +1 @@
-Variable name m might not have been assigned yet.
+Variable name m might not have been assigned yet at DSL expression line 1 column 26.

todo.txt

@@ -25,6 +25,15 @@ RELEASES
 ================================================================
 FEATURES
 
+----------------------------------------------------------------
+RUNTIME LINE/COLUMN NUMBERS
+
+internal/pkg/dsl/cst/for.go
+895:				// TODO: propagate line-number context
+
+internal/pkg/dsl/cst/if.go
+132:			// TODO: line-number/token info for the DSL expression.
+
 ----------------------------------------------------------------
 STRICT MODE