Handle multiple missing constant decorations and remove the ordinal code

elastic · Jan 19, 2022 · 84e6198 · 84e6198
1 parent b32ae4c
commit 84e6198
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 95 deletions.
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Utility.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Utility.java
@@ -34,31 +34,5 @@ public static char StringTochar(final String value) {
         return value.charAt(0);
     }
 
-    public static String toOrdinal(int number) {
-        StringBuilder stringBuilder = new StringBuilder();
-        stringBuilder.append(number);
-        int hundredModule = number % 100;
-        int decimalModule = number % 10;
-        if (11 <= hundredModule && hundredModule <= 13) {
-            stringBuilder.append("th");
-        } else {
-            switch (decimalModule) {
-                case 1:
-                    stringBuilder.append("st");
-                    break;
-                case 2:
-                    stringBuilder.append("nd");
-                    break;
-                case 3:
-                    stringBuilder.append("rd");
-                    break;
-                default:
-                    stringBuilder.append("th");
-                    break;
-            }
-        }
-        return stringBuilder.toString();
-    }
-
     private Utility() {}
 }
diff --git a/...c/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java b/...c/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java
@@ -8,9 +8,9 @@
 
 package org.elasticsearch.painless.phase;
 
+import org.elasticsearch.core.Tuple;
 import org.elasticsearch.painless.AnalyzerCaster;
 import org.elasticsearch.painless.Operation;
-import org.elasticsearch.painless.Utility;
 import org.elasticsearch.painless.ir.BinaryImplNode;
 import org.elasticsearch.painless.ir.BinaryMathNode;
 import org.elasticsearch.painless.ir.BooleanNode;
@@ -71,8 +71,11 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Locale;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 
 /**
  * This optimization pass will perform the specified operation on two leafs nodes if they are both
@@ -1106,6 +1109,8 @@ private void replaceCallWithConstant(
         Object receiver
     ) {
         Object[] args = new Object[irInvokeCallMemberNode.getArgumentNodes().size()];
+
+        List<Tuple<Integer,String>> argumentsShouldBeConstants  = new ArrayList<>();
         for (int i = 0; i < irInvokeCallMemberNode.getArgumentNodes().size(); i++) {
             ExpressionNode argNode = irInvokeCallMemberNode.getArgumentNodes().get(i);
             IRDConstant constantDecoration = argNode.getDecoration(IRDConstant.class);
@@ -1114,17 +1119,23 @@ private void replaceCallWithConstant(
                 String argumentName = argNode instanceof CastNode
                     ? ((CastNode) argNode).getChildNode().getDecoration(IRDName.class).getValue()
                     : "";
-                throw irInvokeCallMemberNode
-                    .getLocation()
-                    .createError(new IllegalArgumentException(String.format(
-                        Locale.ROOT,
-                        "All arguments of the [%s] method must be constants, but the [%s] argument [%s] is not",
-                        javaMethod.getName(),
-                        Utility.toOrdinal(i+1),
-                        argumentName
-                    )));
+                argumentsShouldBeConstants.add(Tuple.tuple(i, argumentName));
+            } else {
+                args[i] = constantDecoration.getValue();
             }
-            args[i] = constantDecoration.getValue();
+        }
+        if (argumentsShouldBeConstants.size() > 0 ){
+            throw irInvokeCallMemberNode
+                .getLocation()
+                .createError(new IllegalArgumentException(String.format(
+                    Locale.ROOT,
+                    "All arguments of the [%s] method must be constants, but the following arguments are not: %s",
+                    javaMethod.getName(),
+                    argumentsShouldBeConstants
+                     .stream()
+                     .map(pair -> "argument [" + (pair.v1()+1) +"]"+ (pair.v2().isEmpty() ? "" : " named [" + pair.v2() + "]"))
+                     .collect(Collectors.joining(", "))
+                )));
         }
         Object result;
         try {

diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java
@@ -238,9 +238,10 @@ public void testClassMethodCompileTimeOnly() {
     public void testClassMethodCompileTimeOnlyVariableParams() {
         Exception e = expectScriptThrows(
             IllegalArgumentException.class,
-            () -> scriptEngine.compile(null, "def a = 2; classMul(2, a)", BindingsTestScript.CONTEXT, Collections.emptyMap())
+            () -> scriptEngine.compile(null, "def a = 2; def b = 3; classMul(a, b)", BindingsTestScript.CONTEXT, Collections.emptyMap())
         );
-        assertThat(e.getMessage(), equalTo("All arguments of the [classMul] method must be constants, but the [2nd] argument [a] is not"));
+        assertThat(e.getMessage(), equalTo("All arguments of the [classMul] method must be constants," +
+            " but the following arguments are not: argument [1] named [a], argument [2] named [b]"));
     }
 
     public void testClassMethodCompileTimeOnlyThrows() {
@@ -269,7 +270,8 @@ public void testInstanceMethodCompileTimeOnlyVariableParams() {
             IllegalArgumentException.class,
             () -> scriptEngine.compile(null, "def a = 2; instanceMul(a, 2)", BindingsTestScript.CONTEXT, Collections.emptyMap())
         );
-        assertThat(e.getMessage(), equalTo("All arguments of the [instanceMul] method must be constants, but the [1st] argument [a] is not"));
+        assertThat(e.getMessage(), equalTo("All arguments of the [instanceMul] method must be constants," +
+            " but the following arguments are not: argument [1] named [a]"));
     }
 
     public void testCompileTimeOnlyParameterFoldedToConstant() {

diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/UtilityTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/UtilityTests.java
diff --git a/...e-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/250_grok.yml b/...e-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/250_grok.yml
@@ -80,7 +80,7 @@ fetch:
 ---
 mutable pattern:
   - do:
-      catch: /All arguments of the \[grok\] method must be constants, but the \[1st\] argument \[\] is not/
+      catch: '/All arguments of the \[grok\] method must be constants, but the following arguments are not\: argument \[1\]/'
       search:
         index: http_logs
         body:

diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/250_grok.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/250_grok.yml
@@ -80,7 +80,7 @@ fetch:
 ---
 mutable pattern:
   - do:
-      catch: /All arguments of the \[grok\] method must be constants, but the \[1st\] argument \[\] is not/
+      catch: '/All arguments of the \[grok\] method must be constants, but the following arguments are not\: argument \[1\]/'
       search:
         index: http_logs
         body: