Merge pull request #97 from databricks/edgar/improve-wrong-message-er…

…ror-during-function-invocation Fix error message for too many arguments with at least one named arg
databricks · Nov 6, 2020 · 2c909ba · 2c909ba
2 parents 1e87561 + 717a377
commit 2c909ba
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 25 deletions.
diff --git a/sjsonnet/src/sjsonnet/Error.scala b/sjsonnet/src/sjsonnet/Error.scala
@@ -64,10 +64,13 @@ object Error {
 
   def failIfNonEmpty(names: collection.BitSet,
                      outerOffset: Int,
-                     formatMsg: (String, String) => String)
+                     formatMsg: (String, String) => String,
+                     // Allows the use of a custom file scope for computing the error message
+                     // for details see: https://github.com/databricks/sjsonnet/issues/83
+                     customFileScope: Option[FileScope] = None)
                     (implicit fileScope: FileScope, eval: EvalErrorScope) = if (names.nonEmpty){
     val plural = if (names.size > 1) "s" else ""
-    val nameSnippet = names.map(fileScope.indexNames).mkString(", ")
+    val nameSnippet = names.map(customFileScope.getOrElse(fileScope).indexNames).mkString(", ")
     fail(formatMsg(plural, nameSnippet), outerOffset)
   }
 

diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala
@@ -287,7 +287,8 @@ object Val{
       Error.failIfNonEmpty(
         repeats,
         outerOffset,
-        (plural, names) => s"Function parameter$plural $names passed more than once"
+        (plural, names) => s"binding parameter a second time: $names",
+        Some(defSiteFileScope)
       )
 
       Error.failIfNonEmpty(

diff --git a/sjsonnet/test/resources/imports/error.too_many_arg_with_named_arg.jsonnet b/sjsonnet/test/resources/imports/error.too_many_arg_with_named_arg.jsonnet
@@ -0,0 +1,2 @@
+local f = import "single_param_function.jsonnet";
+f(1, x = "foo")
diff --git a/sjsonnet/test/resources/imports/single_param_function.jsonnet b/sjsonnet/test/resources/imports/single_param_function.jsonnet
@@ -0,0 +1 @@
+function(x) x
diff --git a/sjsonnet/test/src-jvm/sjsonnet/ErrorTests.scala b/sjsonnet/test/src-jvm/sjsonnet/ErrorTests.scala
@@ -139,20 +139,14 @@ object ErrorTests extends TestSuite{
     )
 
     test("function_duplicate_arg") - check(
-      """sjsonnet.Error: Function parameter x passed more than once
+      """sjsonnet.Error: binding parameter a second time: x
         |    at .(sjsonnet/test/resources/test_suite/error.function_duplicate_arg.jsonnet:17:21)
         |    at .(sjsonnet/test/resources/test_suite/error.function_duplicate_arg.jsonnet:17:21)
         |""".stripMargin
     )
     test("function_duplicate_param") - check(
       """Parse error: Expected no duplicate parameter: x:17:14, found ") x\n"""".stripMargin
     )
-//    test("function_infinite_default") - check(
-//      """sjsonnet.Error: Parameter passed more than once: x
-//        |    at .(sjsonnet/test/resources/test_suite/error.function_duplicate_arg.jsonnet:17:2)
-//        |    at .(sjsonnet/test/resources/test_suite/error.function_duplicate_arg.jsonnet:17:21)
-//        |""".stripMargin
-//    )
     test("function_too_many_args") - check(
       """sjsonnet.Error: Too many args, function has 2 parameter(s)
         |    at .(sjsonnet/test/resources/test_suite/error.function_too_many_args.jsonnet:19:4)
@@ -288,20 +282,12 @@ object ErrorTests extends TestSuite{
          |    at .(sjsonnet/test/resources/imports/error.too_many_arg.jsonnet:3:6)
          |""".stripMargin
     )
-    //    test("overflow") - check(
-//      """sjsonnet.Error: my error message
-//        |    at .(sjsonnet/test/resources/test_suite/error.invariant.simple3.jsonnet:18:10)
-//        |""".stripMargin
-//    )
-//    test("overflow2") - check(
-//      """sjsonnet.Error: my error message
-//        |    at .(sjsonnet/test/resources/test_suite/error.invariant.simple3.jsonnet:18:10)
-//        |""".stripMargin
-//    )
-//    test("overflow3") - check(
-//      """sjsonnet.Error: my error message
-//        |    at .(sjsonnet/test/resources/test_suite/error.invariant.simple3.jsonnet:18:10)
-//        |""".stripMargin
-//    )
+
+    test("too_many_arg_with_named_arg") - checkImports(
+      """|sjsonnet.Error: binding parameter a second time: x
+         |    at .(sjsonnet/test/resources/imports/error.too_many_arg_with_named_arg.jsonnet:2:2)
+         |    at .(sjsonnet/test/resources/imports/error.too_many_arg_with_named_arg.jsonnet:2:2)
+         |""".stripMargin
+    )
   }
 }