From a92d8ed0a22e1a397f82d282f01b519cf38e9ae9 Mon Sep 17 00:00:00 2001
From: Edgar Fernandes <edgar.fernandes@databricks.com>
Date: Wed, 4 Nov 2020 14:36:23 +0100
Subject: [PATCH] Fix error message for too many arguments whith at least one
 named arg

---
 .gitignore                                    |  1 +
 sjsonnet/src/sjsonnet/Val.scala               |  2 +-
 .../error.too_many_arg_with_named_arg.jsonnet |  2 ++
 .../test/src-jvm/sjsonnet/ErrorTests.scala    | 30 +++++--------------
 .../test/src/sjsonnet/EvaluatorTests.scala    |  7 ++++-
 5 files changed, 18 insertions(+), 24 deletions(-)
 create mode 100644 sjsonnet/test/resources/imports/error.too_many_arg_with_named_arg.jsonnet

diff --git a/.gitignore b/.gitignore
index a3d08f4d..27d8c0c7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,4 +4,5 @@ output/
 .DS_STORE
 .idea_modules
 .idea
+.metals
 out/
diff --git a/sjsonnet/src/sjsonnet/Val.scala b/sjsonnet/src/sjsonnet/Val.scala
index c693d0cb..17fc9e66 100644
--- a/sjsonnet/src/sjsonnet/Val.scala
+++ b/sjsonnet/src/sjsonnet/Val.scala
@@ -287,7 +287,7 @@ 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"
       )
 
       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
new file mode 100644
index 00000000..525dc854
--- /dev/null
+++ b/sjsonnet/test/resources/imports/error.too_many_arg_with_named_arg.jsonnet
@@ -0,0 +1,2 @@
+local foo (x) = x;
+foo(1, x = "bar")
\ No newline at end of file
diff --git a/sjsonnet/test/src-jvm/sjsonnet/ErrorTests.scala b/sjsonnet/test/src-jvm/sjsonnet/ErrorTests.scala
index 9de07ea8..1636edd7 100644
--- a/sjsonnet/test/src-jvm/sjsonnet/ErrorTests.scala
+++ b/sjsonnet/test/src-jvm/sjsonnet/ErrorTests.scala
@@ -139,7 +139,7 @@ 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
@@ -147,12 +147,6 @@ object ErrorTests extends TestSuite{
     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:4)
+         |    at .(sjsonnet/test/resources/imports/error.too_many_arg_with_named_arg.jsonnet:2:4)
+         |""".stripMargin
+    )
   }
 }
diff --git a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala
index 03cadc8e..7409f178 100644
--- a/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala
+++ b/sjsonnet/test/src/sjsonnet/EvaluatorTests.scala
@@ -225,12 +225,17 @@ object EvaluatorTests extends TestSuite{
             |  welcome: 'Hello ' + name + '!',
             |};
             |{
-            |  person2: Person('Bob', hello=123),
+            |  person2: Person(hello=123),
             |}
         """.stripMargin
         )
       }
 
+      /* 
+       * Wrong number of params error takes precedence on invalid param
+       * For that reason, to assert for invalid param we need to pass the 
+       * right number of params in the first place.
+       */
       assert(ex.getMessage.contains("Function has no parameter hello"))
     }