diff --git a/sjsonnet/src/sjsonnet/Evaluator.scala b/sjsonnet/src/sjsonnet/Evaluator.scala index 762cf1c8..e557d81b 100644 --- a/sjsonnet/src/sjsonnet/Evaluator.scala +++ b/sjsonnet/src/sjsonnet/Evaluator.scala @@ -116,7 +116,7 @@ class Evaluator( newScope.bindings(base + i) = b.args match { case null => visitAsLazy(b.rhs)(newScope) case argSpec => - new Lazy(() => visitMethod(b.rhs, argSpec, b.pos)(newScope)) + new Lazy(() => visitMethod(b.rhs, argSpec, b.pos, b.name)(newScope)) } i += 1 } @@ -634,9 +634,10 @@ class Evaluator( Error.fail(s"Field name must be string or null, not ${fieldName.prettyName}", pos) } - def visitMethod(rhs: Expr, params: Params, outerPos: Position)(implicit + def visitMethod(rhs: Expr, params: Params, outerPos: Position, name: String = null)(implicit scope: ValScope): Val.Func = new Val.Func(outerPos, scope, params) { + override def functionName: String = name def evalRhs(vs: ValScope, es: EvalScope, fs: FileScope, pos: Position): Val = visitExpr(rhs)(vs) override def evalDefault(expr: Expr, vs: ValScope, es: EvalScope): Val = visitExpr(expr)(vs) @@ -651,7 +652,7 @@ class Evaluator( case null => new Lazy(() => visitExpr(b.rhs)(scope)) case argSpec => - new Lazy(() => visitMethod(b.rhs, argSpec, b.pos)(scope)) + new Lazy(() => visitMethod(b.rhs, argSpec, b.pos, b.name)(scope)) } i += 1 } diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala index 7ea602d2..0dee3636 100644 --- a/sjsonnet/src/sjsonnet/Val.scala +++ b/sjsonnet/src/sjsonnet/Val.scala @@ -666,6 +666,19 @@ object Val { def evalDefault(expr: Expr, vs: ValScope, es: EvalScope): Val = null + /** Override to provide a function name for error messages. Only called on error paths. */ + def functionName: String = null + + private def functionNamePrefix: String = { + val name = functionName + if (name != null) s" $name" else "" + } + + private def functionNameSuffix: String = { + val name = functionName + if (name != null) s" in function $name" else "" + } + def prettyName = "function" override def exprErrorString: String = "Function" @@ -701,10 +714,16 @@ object Val { while (i < namedNames.length) { val idx = params.paramMap.getOrElse( namedNames(i), - Error.fail(s"Function has no parameter ${namedNames(i)}", outerPos) + Error.fail( + s"Function$functionNamePrefix has no parameter ${namedNames(i)}", + outerPos + ) ) if (argVals(base + idx) != null) - Error.fail(s"binding parameter a second time: ${namedNames(i)}", outerPos) + Error.fail( + s"binding parameter a second time: ${namedNames(i)}$functionNameSuffix", + outerPos + ) argVals(base + idx) = argsL(j) i += 1 j += 1 @@ -712,7 +731,7 @@ object Val { } if (argsL.length > params.names.length) Error.fail( - "Too many args, function has " + params.names.length + " parameter(s)", + "Too many args, function" + functionNamePrefix + " has " + params.names.length + " parameter(s)", outerPos ) if (params.names.length != argsL.length) { // Args missing -> add defaults @@ -735,7 +754,7 @@ object Val { if (missing != null) { val plural = if (missing.size > 1) "s" else "" Error.fail( - s"Function parameter$plural ${missing.mkString(", ")} not bound in call", + s"Function$functionNamePrefix parameter$plural ${missing.mkString(", ")} not bound in call", outerPos ) } @@ -815,7 +834,7 @@ object Val { /** Superclass for standard library functions */ abstract class Builtin( - val functionName: String, + override val functionName: String, paramNames: Array[String], defaults: Array[Expr] = null) extends Func( diff --git a/sjsonnet/test/resources/go_test_suite/native7.jsonnet.golden b/sjsonnet/test/resources/go_test_suite/native7.jsonnet.golden index be7d58ce..a29a6e52 100644 --- a/sjsonnet/test/resources/go_test_suite/native7.jsonnet.golden +++ b/sjsonnet/test/resources/go_test_suite/native7.jsonnet.golden @@ -1,3 +1,3 @@ -sjsonnet.Error: Function has no parameter y +sjsonnet.Error: Function jsonToString has no parameter y at [Apply].(native7.jsonnet:1:27) diff --git a/sjsonnet/test/resources/go_test_suite/optional_args8.jsonnet.golden b/sjsonnet/test/resources/go_test_suite/optional_args8.jsonnet.golden index 687f095a..38b93275 100644 --- a/sjsonnet/test/resources/go_test_suite/optional_args8.jsonnet.golden +++ b/sjsonnet/test/resources/go_test_suite/optional_args8.jsonnet.golden @@ -1,3 +1,3 @@ -sjsonnet.Error: Function has no parameter y +sjsonnet.Error: Function foo has no parameter y at [Apply].(optional_args8.jsonnet:2:4) diff --git a/sjsonnet/test/resources/go_test_suite/std.makeArrayNamed3.jsonnet.golden b/sjsonnet/test/resources/go_test_suite/std.makeArrayNamed3.jsonnet.golden index f00da018..8a7fe105 100644 --- a/sjsonnet/test/resources/go_test_suite/std.makeArrayNamed3.jsonnet.golden +++ b/sjsonnet/test/resources/go_test_suite/std.makeArrayNamed3.jsonnet.golden @@ -1,3 +1,3 @@ -sjsonnet.Error: Function has no parameter blahblah +sjsonnet.Error: Function makeArray has no parameter blahblah at [Apply].(std.makeArrayNamed3.jsonnet:1:14) diff --git a/sjsonnet/test/resources/test_suite/error.function_too_many_args.jsonnet.golden b/sjsonnet/test/resources/test_suite/error.function_too_many_args.jsonnet.golden index 7c14b42f..35d56bc1 100644 --- a/sjsonnet/test/resources/test_suite/error.function_too_many_args.jsonnet.golden +++ b/sjsonnet/test/resources/test_suite/error.function_too_many_args.jsonnet.golden @@ -1,3 +1,3 @@ -sjsonnet.Error: Too many args, function has 2 parameter(s) +sjsonnet.Error: Too many args, function foo has 2 parameter(s) at [Apply3].(error.function_too_many_args.jsonnet:19:4) diff --git a/sjsonnet/test/resources/test_suite/error.trace_one_param.jsonnet.golden b/sjsonnet/test/resources/test_suite/error.trace_one_param.jsonnet.golden index dc0df1bb..77b3c149 100644 --- a/sjsonnet/test/resources/test_suite/error.trace_one_param.jsonnet.golden +++ b/sjsonnet/test/resources/test_suite/error.trace_one_param.jsonnet.golden @@ -1,4 +1,4 @@ -sjsonnet.Error: Function parameter rest not bound in call +sjsonnet.Error: Function trace parameter rest not bound in call at [Apply1].(error.trace_one_param.jsonnet:17:20) at [ValidId v].(error.trace_one_param.jsonnet:19:6) diff --git a/sjsonnet/test/resources/test_suite/error.trace_three_param.jsonnet.golden b/sjsonnet/test/resources/test_suite/error.trace_three_param.jsonnet.golden index 1f716f1a..bbf586ba 100644 --- a/sjsonnet/test/resources/test_suite/error.trace_three_param.jsonnet.golden +++ b/sjsonnet/test/resources/test_suite/error.trace_three_param.jsonnet.golden @@ -1,4 +1,4 @@ -sjsonnet.Error: Too many args, function has 2 parameter(s) +sjsonnet.Error: Too many args, function trace has 2 parameter(s) at [Apply3].(error.trace_three_param.jsonnet:17:20) at [ValidId v].(error.trace_three_param.jsonnet:19:6) diff --git a/sjsonnet/test/resources/test_suite/error.trace_zero_param.jsonnet.golden b/sjsonnet/test/resources/test_suite/error.trace_zero_param.jsonnet.golden index 4b9d16ba..76abf29e 100644 --- a/sjsonnet/test/resources/test_suite/error.trace_zero_param.jsonnet.golden +++ b/sjsonnet/test/resources/test_suite/error.trace_zero_param.jsonnet.golden @@ -1,4 +1,4 @@ -sjsonnet.Error: Function parameters str, rest not bound in call +sjsonnet.Error: Function trace parameters str, rest not bound in call at [Apply0].(error.trace_zero_param.jsonnet:17:20) at [ValidId v].(error.trace_zero_param.jsonnet:19:6) diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala index 76c02da1..17288b79 100644 --- a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala +++ b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala @@ -457,7 +457,7 @@ object EvaluatorTests extends TestSuite { ) } - assert(ex.getMessage.contains("Function parameter y not bound in call")) + assert(ex.getMessage.contains("Function newParams parameter y not bound in call")) } test("invalidParam") { @@ -475,7 +475,7 @@ object EvaluatorTests extends TestSuite { ) } - assert(ex.getMessage.contains("Function has no parameter hello")) + assert(ex.getMessage.contains("Function Person has no parameter hello")) } test("unknownVariable") { @@ -866,6 +866,86 @@ object EvaluatorTests extends TestSuite { } } + test("builtinErrorMessageIncludesFunctionName") { + // Issue #416: error messages for builtin functions should include the function name. + + // Too many args + test("tooManyArgs") { + val ex = assertThrows[Exception] { + eval("std.length([1], [2])", useNewEvaluator = useNewEvaluator) + } + assert(ex.getMessage.contains("Too many args, function length has")) + } + + // Parameter not bound in call (missing required arg) + test("missingRequiredArg") { + val ex = assertThrows[Exception] { + eval("std.substr('hello')", useNewEvaluator = useNewEvaluator) + } + assert(ex.getMessage.contains("Function substr parameter")) + assert(ex.getMessage.contains("not bound in call")) + } + + // Function has no parameter (invalid named arg) + test("invalidNamedArg") { + val ex = assertThrows[Exception] { + eval("std.length(x=[1], noSuchParam=2)", useNewEvaluator = useNewEvaluator) + } + assert(ex.getMessage.contains("Function length has no parameter noSuchParam")) + } + + // Binding parameter a second time + test("duplicateArg") { + val ex = assertThrows[Exception] { + eval("std.pow(2, x=3)", useNewEvaluator = useNewEvaluator) + } + assert(ex.getMessage.contains("binding parameter a second time: x in function pow")) + } + + // User-defined function should include function name from local binding + test("userDefinedFunctionIncludesFunctionName") { + val ex = assertThrows[Exception] { + eval( + """local myFunc(x, y) = x + y; + |myFunc("a") + """.stripMargin, + useNewEvaluator = useNewEvaluator + ) + } + assert(ex.getMessage.contains("Function myFunc parameter y not bound in call")) + } + + // User-defined function: too many args should include function name + test("userDefinedTooManyArgs") { + val ex = assertThrows[Exception] { + eval( + """local add(x, y) = x + y; + |add(1, 2, 3) + """.stripMargin, + useNewEvaluator = useNewEvaluator + ) + } + assert(ex.getMessage.contains("Too many args, function add has 2 parameter(s)")) + } + + // Anonymous function should NOT include function name in error message + test("anonymousFunctionNoFunctionName") { + val ex = assertThrows[Exception] { + eval("(function(x, y) x + y)('a')", useNewEvaluator = useNewEvaluator) + } + // Should be exactly "Function parameter..." without a function name inserted + assert(ex.getMessage.contains("Function parameter y not bound in call")) + } + + // Anonymous function: too many args should NOT include function name + test("anonymousFunctionTooManyArgs") { + val ex = assertThrows[Exception] { + eval("(function(x) x)(1, 2)", useNewEvaluator = useNewEvaluator) + } + assert(ex.getMessage.contains("Too many args, function has 1 parameter(s)")) + } + } + } def tests: Tests = allTests(false).prefix("Evaluator") ++ allTests(true).prefix("NewEvaluator") }