Static-check fixes from @lespea #1657, batch 1/n (#1703)

pkg/climain/mlrcli_shebang.go

@@ -39,7 +39,7 @@ func maybeInterpolateDashS(args []string) ([]string, error) {
 		return args, nil
 	}
 	if len(args) < 3 {
-		return nil, fmt.Errorf("mlr: -s flag requires a filename after it.")
+		return nil, fmt.Errorf("mlr: -s flag requires a filename after it")
 	}
 
 	// mlr -s scriptfile input1.csv input2.csv

pkg/dsl/cst/block_exit.go

@@ -70,7 +70,7 @@ func (root *RootNode) BuildReturnNode(astNode *dsl.ASTNode) (*ReturnNode, error)
 	} else {
 		lib.InternalCodingErrorIf(true)
 	}
-	return nil, fmt.Errorf("internal coding error: statement should not be reached.")
+	return nil, fmt.Errorf("internal coding error: statement should not be reached")
 }
 
 func (node *ReturnNode) Execute(state *runtime.State) (*BlockExitPayload, error) {

pkg/dsl/cst/builtin_functions.go

@@ -599,7 +599,7 @@ func (root *RootNode) BuildVariadicFunctionCallsiteNode(
 
 	if callsiteArity < builtinFunctionInfo.minimumVariadicArity {
 		return nil, fmt.Errorf(
-			"mlr: function %s takes minimum argument count %d; got %d.\n",
+			"mlr: function %s takes minimum argument count %d; got %d",
 			builtinFunctionInfo.name,
 			builtinFunctionInfo.minimumVariadicArity,
 			callsiteArity,
@@ -609,7 +609,7 @@ func (root *RootNode) BuildVariadicFunctionCallsiteNode(
 	if builtinFunctionInfo.maximumVariadicArity != 0 {
 		if callsiteArity > builtinFunctionInfo.maximumVariadicArity {
 			return nil, fmt.Errorf(
-				"mlr: function %s takes maximum argument count %d; got %d.\n",
+				"mlr: function %s takes maximum argument count %d; got %d",
 				builtinFunctionInfo.name,
 				builtinFunctionInfo.maximumVariadicArity,
 				callsiteArity,
@@ -657,7 +657,7 @@ func (root *RootNode) BuildVariadicFunctionWithStateCallsiteNode(
 
 	if callsiteArity < builtinFunctionInfo.minimumVariadicArity {
 		return nil, fmt.Errorf(
-			"mlr: function %s takes minimum argument count %d; got %d.\n",
+			"mlr: function %s takes minimum argument count %d; got %d",
 			builtinFunctionInfo.name,
 			builtinFunctionInfo.minimumVariadicArity,
 			callsiteArity,
@@ -667,7 +667,7 @@ func (root *RootNode) BuildVariadicFunctionWithStateCallsiteNode(
 	if builtinFunctionInfo.maximumVariadicArity != 0 {
 		if callsiteArity > builtinFunctionInfo.maximumVariadicArity {
 			return nil, fmt.Errorf(
-				"mlr: function %s takes maximum argument count %d; got %d.\n",
+				"mlr: function %s takes maximum argument count %d; got %d",
 				builtinFunctionInfo.name,
 				builtinFunctionInfo.maximumVariadicArity,
 				callsiteArity,

pkg/dsl/cst/cond.go

@@ -61,7 +61,7 @@ func (node *CondBlockNode) Execute(
 		boolValue = false
 	} else if !isBool {
 		return nil, fmt.Errorf(
-			"mlr: conditional expression did not evaluate to boolean%s.",
+			"mlr: conditional expression did not evaluate to boolean%s",
 			dsl.TokenToLocationInfo(node.conditionToken),
 		)
 	}

pkg/dsl/cst/dump.go

@@ -137,7 +137,7 @@ func (root *RootNode) buildDumpxStatementNode(
 			} else if redirectorNode.Type == dsl.NodeTypeRedirectPipe {
 				retval.outputHandlerManager = output.NewPipeWriteHandlerManager(root.recordWriterOptions)
 			} else {
-				return nil, fmt.Errorf("mlr: unhandled redirector node type %s.", string(redirectorNode.Type))
+				return nil, fmt.Errorf("mlr: unhandled redirector node type %s", string(redirectorNode.Type))
 			}
 		}
 	}
@@ -211,7 +211,7 @@ func (node *DumpStatementNode) dumpToFileOrPipe(
 	redirectorTarget := node.redirectorTargetEvaluable.Evaluate(state)
 	if !redirectorTarget.IsString() {
 		return fmt.Errorf(
-			"mlr: output redirection yielded %s, not string.",
+			"mlr: output redirection yielded %s, not string",
 			redirectorTarget.GetTypeName(),
 		)
 	}

pkg/dsl/cst/emit_emitp.go

@@ -171,7 +171,7 @@ func (root *RootNode) buildEmitXStatementNode(
 
 		} else {
 			return nil, fmt.Errorf(
-				"mlr: unlashed-emit node types must be local variables, field names, oosvars, or maps; got %s.",
+				"mlr: unlashed-emit node types must be local variables, field names, oosvars, or maps; got %s",
 				childNode.Type,
 			)
 		}
@@ -181,7 +181,7 @@ func (root *RootNode) buildEmitXStatementNode(
 		for _, childNode := range emittablesNode.Children {
 			if !EMITX_NAMED_NODE_TYPES[childNode.Type] {
 				return nil, fmt.Errorf(
-					"mlr: lashed-emit node types must be local variables, field names, or oosvars; got %s.",
+					"mlr: lashed-emit node types must be local variables, field names, or oosvars; got %s",
 					childNode.Type,
 				)
 			}
@@ -271,7 +271,7 @@ func (root *RootNode) buildEmitXStatementNode(
 			} else if redirectorNode.Type == dsl.NodeTypeRedirectPipe {
 				retval.outputHandlerManager = output.NewPipeWriteHandlerManager(root.recordWriterOptions)
 			} else {
-				return nil, fmt.Errorf("mlr: unhandled redirector node type %s.", string(redirectorNode.Type))
+				return nil, fmt.Errorf("mlr: unhandled redirector node type %s", string(redirectorNode.Type))
 			}
 		}
 	}
@@ -989,7 +989,7 @@ func (node *EmitXStatementNode) emitRecordToFileOrPipe(
 ) error {
 	redirectorTarget := node.redirectorTargetEvaluable.Evaluate(state)
 	if !redirectorTarget.IsString() {
-		return fmt.Errorf("mlr: output redirection yielded %s, not string.", redirectorTarget.GetTypeName())
+		return fmt.Errorf("mlr: output redirection yielded %s, not string", redirectorTarget.GetTypeName())
 	}
 	outputFileName := redirectorTarget.String()
 

pkg/dsl/cst/emitf.go

@@ -119,7 +119,7 @@ func (root *RootNode) BuildEmitFStatementNode(astNode *dsl.ASTNode) (IExecutable
 			} else if redirectorNode.Type == dsl.NodeTypeRedirectPipe {
 				retval.outputHandlerManager = output.NewPipeWriteHandlerManager(root.recordWriterOptions)
 			} else {
-				return nil, fmt.Errorf("mlr: unhandled redirector node type %s.", string(redirectorNode.Type))
+				return nil, fmt.Errorf("mlr: unhandled redirector node type %s", string(redirectorNode.Type))
 			}
 		}
 	}
@@ -163,7 +163,7 @@ func getNameFromNamedNode(astNode *dsl.ASTNode, description string) (string, err
 	} else if astNode.Type == dsl.NodeTypeDirectFieldValue {
 		return string(astNode.Token.Lit), nil
 	}
-	return "", fmt.Errorf("mlr: can't get name of node type \"%s\" for %s.", string(astNode.Type), description)
+	return "", fmt.Errorf(`mlr: can't get name of node type "%s" for %s`, string(astNode.Type), description)
 }
 
 // ----------------------------------------------------------------
@@ -187,7 +187,7 @@ func (node *EmitFStatementNode) emitfToFileOrPipe(
 ) error {
 	redirectorTarget := node.redirectorTargetEvaluable.Evaluate(state)
 	if !redirectorTarget.IsString() {
-		return fmt.Errorf("mlr: output redirection yielded %s, not string.", redirectorTarget.GetTypeName())
+		return fmt.Errorf("mlr: output redirection yielded %s, not string", redirectorTarget.GetTypeName())
 	}
 	outputFileName := redirectorTarget.String()
 

pkg/dsl/cst/for.go

@@ -805,7 +805,7 @@ func (root *RootNode) BuildTripleForLoopNode(astNode *dsl.ASTNode) (*TripleForLo
 			for i := 0; i < n-1; i++ {
 				if continuationExpressionASTNode.Children[i].Type != dsl.NodeTypeAssignment {
 					return nil, fmt.Errorf(
-						"mlr: the non-final triple-for continuation statements must be assignments.",
+						"mlr: the non-final triple-for continuation statements must be assignments",
 					)
 				}
 				precontinuationAssignment, err := root.BuildAssignmentNode(
@@ -822,11 +822,11 @@ func (root *RootNode) BuildTripleForLoopNode(astNode *dsl.ASTNode) (*TripleForLo
 		if bareBooleanASTNode.Type != dsl.NodeTypeBareBoolean {
 			if n == 1 {
 				return nil, fmt.Errorf(
-					"mlr: the triple-for continuation statement must be a bare boolean.",
+					"mlr: the triple-for continuation statement must be a bare boolean",
 				)
 			} else {
 				return nil, fmt.Errorf(
-					"mlr: the final triple-for continuation statement must be a bare boolean.",
+					"mlr: the final triple-for continuation statement must be a bare boolean",
 				)
 			}
 		}
@@ -898,7 +898,7 @@ func (node *TripleForLoopNode) Execute(state *runtime.State) (*BlockExitPayload,
 			boolValue, isBool := continuationValue.GetBoolValue()
 			if !isBool {
 				return nil, fmt.Errorf(
-					"mlr: for-loop continuation did not evaluate to boolean%s.",
+					"mlr: for-loop continuation did not evaluate to boolean%s",
 					dsl.TokenToLocationInfo(node.continuationExpressionToken),
 				)
 			}

pkg/dsl/cst/if.go

@@ -130,7 +130,7 @@ func (node *IfChainNode) Execute(state *runtime.State) (*BlockExitPayload, error
 		boolValue, isBool := condition.GetBoolValue()
 		if !isBool {
 			return nil, fmt.Errorf(
-				"mlr: conditional expression did not evaluate to boolean%s.",
+				"mlr: conditional expression did not evaluate to boolean%s",
 				dsl.TokenToLocationInfo(ifItem.conditionToken),
 			)
 		}

pkg/dsl/cst/lvalues.go

@@ -45,11 +45,11 @@ func (root *RootNode) BuildAssignableNode(
 
 	case dsl.NodeTypeArrayOrMapPositionalNameAccess:
 		return nil, fmt.Errorf(
-			"mlr: '[[...]]' is allowed on assignment left-hand sides only when immediately preceded by '$'.",
+			"mlr: '[[...]]' is allowed on assignment left-hand sides only when immediately preceded by '$'",
 		)
 	case dsl.NodeTypeArrayOrMapPositionalValueAccess:
 		return nil, fmt.Errorf(
-			"mlr: '[[[...]]]' is allowed on assignment left-hand sides only when immediately preceded by '$'.",
+			"mlr: '[[[...]]]' is allowed on assignment left-hand sides only when immediately preceded by '$'",
 		)
 
 	case dsl.NodeTypeArrayOrMapIndexAccess:
@@ -106,7 +106,7 @@ func (node *DirectFieldValueLvalueNode) AssignIndexed(
 	// print inrec attributes. Also, a UDF/UDS invoked from begin/end could try
 	// to access the inrec, and that would get past the validator.
 	if state.Inrec == nil {
-		return fmt.Errorf("there is no current record to assign to.")
+		return fmt.Errorf("there is no current record to assign to")
 	}
 
 	// AssignmentNode checks for absent, so we just assign whatever we get
@@ -205,7 +205,7 @@ func (node *IndirectFieldValueLvalueNode) AssignIndexed(
 	// print inrec attributes. Also, a UDF/UDS invoked from begin/end could try
 	// to access the inrec, and that would get past the validator.
 	if state.Inrec == nil {
-		return fmt.Errorf("there is no current record to assign to.")
+		return fmt.Errorf("there is no current record to assign to")
 	}
 
 	lhsFieldName := node.lhsFieldNameExpression.Evaluate(state)
@@ -298,7 +298,7 @@ func (node *PositionalFieldNameLvalueNode) Assign(
 	// print inrec attributes. Also, a UDF/UDS invoked from begin/end could try
 	// to access the inrec, and that would get past the validator.
 	if state.Inrec == nil {
-		return fmt.Errorf("there is no current record to assign to.")
+		return fmt.Errorf("there is no current record to assign to")
 	}
 
 	lhsFieldIndex := node.lhsFieldIndexExpression.Evaluate(state)
@@ -310,7 +310,7 @@ func (node *PositionalFieldNameLvalueNode) Assign(
 		return nil
 	} else {
 		return fmt.Errorf(
-			"mlr: positional index for $[[...]] assignment must be integer; got %s.",
+			"mlr: positional index for $[[...]] assignment must be integer; got %s",
 			lhsFieldIndex.GetTypeName(),
 		)
 	}
@@ -324,7 +324,7 @@ func (node *PositionalFieldNameLvalueNode) AssignIndexed(
 	// TODO: reconsider this if /when we decide to allow string-slice
 	// assignments.
 	return fmt.Errorf(
-		"mlr: $[[...]] = ... expressions are not indexable.",
+		"mlr: $[[...]] = ... expressions are not indexable",
 	)
 }
 
@@ -416,7 +416,7 @@ func (node *PositionalFieldValueLvalueNode) AssignIndexed(
 	// print inrec attributes. Also, a UDF/UDS invoked from begin/end could try
 	// to access the inrec, and that would get past the validator.
 	if state.Inrec == nil {
-		return fmt.Errorf("there is no current record to assign to.")
+		return fmt.Errorf("there is no current record to assign to")
 	}
 
 	lhsFieldIndex := node.lhsFieldIndexExpression.Evaluate(state)
@@ -434,7 +434,7 @@ func (node *PositionalFieldValueLvalueNode) AssignIndexed(
 			return nil
 		} else {
 			return fmt.Errorf(
-				"mlr: positional index for $[[[...]]] assignment must be integer; got %s.",
+				"mlr: positional index for $[[[...]]] assignment must be integer; got %s",
 				lhsFieldIndex.GetTypeName(),
 			)
 		}
@@ -517,7 +517,7 @@ func (node *FullSrecLvalueNode) AssignIndexed(
 	// print inrec attributes. Also, a UDF/UDS invoked from begin/end could try
 	// to access the inrec, and that would get past the validator.
 	if state.Inrec == nil {
-		return fmt.Errorf("there is no current record to assign to.")
+		return fmt.Errorf("there is no current record to assign to")
 	}
 
 	// AssignmentNode checks for absentness of the rvalue, so we just assign
@@ -787,7 +787,7 @@ func (root *RootNode) BuildLocalVariableLvalueNode(astNode *dsl.ASTNode) (IAssig
 	if astNode.Children == nil { // untyped, like 'x = 3'
 		if root.strictMode {
 			return nil, fmt.Errorf(
-				"mlr: need typedecl such as \"var\", \"str\", \"num\", etc. for variable \"%s\" in strict mode",
+				`mlr: need typedecl such as "var", "str", "num", etc. for variable "%s" in strict mode`,
 				variableName,
 			)
 		}
@@ -1086,7 +1086,7 @@ func (node *EnvironmentVariableLvalueNode) Assign(
 
 	if !name.IsString() {
 		return fmt.Errorf(
-			"assignments to ENV[...] must have string names; got %s \"%s\"\n",
+			`assignments to ENV[...] must have string names; got %s "%s"`,
 			name.GetTypeName(),
 			name.String(),
 		)
@@ -1109,7 +1109,7 @@ func (node *EnvironmentVariableLvalueNode) AssignIndexed(
 	indices []*mlrval.Mlrval,
 	state *runtime.State,
 ) error {
-	return fmt.Errorf("mlr: ENV[...] cannot be indexed.")
+	return fmt.Errorf("mlr: ENV[...] cannot be indexed")
 }
 
 func (node *EnvironmentVariableLvalueNode) Unassign(
@@ -1133,5 +1133,5 @@ func (node *EnvironmentVariableLvalueNode) UnassignIndexed(
 	state *runtime.State,
 ) {
 	// TODO: needs error return
-	//return errors.New("mlr: ENV[...] cannot be indexed.")
+	//return errors.New("mlr: ENV[...] cannot be indexed")
 }

pkg/dsl/cst/print.go

@@ -280,7 +280,7 @@ func (root *RootNode) buildPrintxStatementNode(
 			} else if redirectorNode.Type == dsl.NodeTypeRedirectPipe {
 				retval.outputHandlerManager = output.NewPipeWriteHandlerManager(root.recordWriterOptions)
 			} else {
-				return nil, fmt.Errorf("mlr: unhandled redirector node type %s.", string(redirectorNode.Type))
+				return nil, fmt.Errorf("mlr: unhandled redirector node type %s", string(redirectorNode.Type))
 			}
 		}
 	}
@@ -356,7 +356,7 @@ func (node *PrintStatementNode) printToFileOrPipe(
 ) error {
 	redirectorTarget := node.redirectorTargetEvaluable.Evaluate(state)
 	if !redirectorTarget.IsString() {
-		return fmt.Errorf("mlr: output redirection yielded %s, not string.", redirectorTarget.GetTypeName())
+		return fmt.Errorf("mlr: output redirection yielded %s, not string", redirectorTarget.GetTypeName())
 	}
 	outputFileName := redirectorTarget.String()
 

pkg/dsl/cst/statements.go

@@ -67,9 +67,9 @@ func (root *RootNode) BuildStatementNode(
 		return root.BuildEmitPStatementNode(astNode)
 
 	case dsl.NodeTypeBeginBlock:
-		return nil, fmt.Errorf("mlr: begin blocks may only be declared at top level.")
+		return nil, fmt.Errorf("mlr: begin blocks may only be declared at top level")
 	case dsl.NodeTypeEndBlock:
-		return nil, fmt.Errorf("mlr: end blocks may only be declared at top level.")
+		return nil, fmt.Errorf("mlr: end blocks may only be declared at top level")
 
 	case dsl.NodeTypeIfChain:
 		return root.BuildIfChainNode(astNode)
@@ -89,9 +89,9 @@ func (root *RootNode) BuildStatementNode(
 		return root.BuildTripleForLoopNode(astNode)
 
 	case dsl.NodeTypeNamedFunctionDefinition:
-		return nil, fmt.Errorf("mlr: functions may only be declared at top level.")
+		return nil, fmt.Errorf("mlr: functions may only be declared at top level")
 	case dsl.NodeTypeSubroutineDefinition:
-		return nil, fmt.Errorf("mlr: subroutines may only be declared at top level.")
+		return nil, fmt.Errorf("mlr: subroutines may only be declared at top level")
 	case dsl.NodeTypeSubroutineCallsite:
 		return root.BuildSubroutineCallsiteNode(astNode)
 

pkg/dsl/cst/tee.go

@@ -121,7 +121,7 @@ func (root *RootNode) BuildTeeStatementNode(astNode *dsl.ASTNode) (IExecutable,
 		} else if redirectorNode.Type == dsl.NodeTypeRedirectPipe {
 			retval.outputHandlerManager = output.NewPipeWriteHandlerManager(root.recordWriterOptions)
 		} else {
-			return nil, fmt.Errorf("mlr: unhandled redirector node type %s.", string(redirectorNode.Type))
+			return nil, fmt.Errorf("mlr: unhandled redirector node type %s", string(redirectorNode.Type))
 		}
 	}
 
@@ -138,7 +138,7 @@ func (root *RootNode) BuildTeeStatementNode(astNode *dsl.ASTNode) (IExecutable,
 func (node *TeeStatementNode) Execute(state *runtime.State) (*BlockExitPayload, error) {
 	expression := node.expressionEvaluable.Evaluate(state)
 	if !expression.IsMap() {
-		return nil, fmt.Errorf("mlr: tee-evaluaiton yielded %s, not map.", expression.GetTypeName())
+		return nil, fmt.Errorf("mlr: tee-evaluaiton yielded %s, not map", expression.GetTypeName())
 	}
 	err := node.teeToRedirectFunc(expression.GetMap(), state)
 	return nil, err
@@ -151,7 +151,7 @@ func (node *TeeStatementNode) teeToFileOrPipe(
 ) error {
 	redirectorTarget := node.redirectorTargetEvaluable.Evaluate(state)
 	if !redirectorTarget.IsString() {
-		return fmt.Errorf("mlr: output redirection yielded %s, not string.", redirectorTarget.GetTypeName())
+		return fmt.Errorf("mlr: output redirection yielded %s, not string", redirectorTarget.GetTypeName())
 	}
 	outputFileName := redirectorTarget.String()
 

pkg/dsl/cst/udf.go

@@ -403,15 +403,15 @@ func (root *RootNode) BuildAndInstallUDF(astNode *dsl.ASTNode) error {
 
 	if BuiltinFunctionManagerInstance.LookUp(functionName) != nil {
 		return fmt.Errorf(
-			"mlr: function named \"%s\" must not override a built-in function of the same name.",
+			`mlr: function named "%s" must not override a built-in function of the same name`,
 			functionName,
 		)
 	}
 
 	if !root.allowUDFUDSRedefinitions {
 		if root.udfManager.ExistsByName(functionName) {
 			return fmt.Errorf(
-				"mlr: function named \"%s\" has already been defined.",
+				`mlr: function named "%s" has already been defined`,
 				functionName,
 			)
 		}