Skip to content

Commit

Permalink
Improve error message in painless when parameter must be a constant b…
Browse files Browse the repository at this point in the history
…ut isn't (elastic#68324)
  • Loading branch information
Kxrr committed Jan 15, 2022
1 parent a5e7984 commit b32ae4c
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,31 @@ 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() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

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;
Expand Down Expand Up @@ -66,9 +67,11 @@
import org.elasticsearch.painless.symbol.IRDecorations.IRDInstanceBinding;
import org.elasticsearch.painless.symbol.IRDecorations.IRDMethod;
import org.elasticsearch.painless.symbol.IRDecorations.IRDOperation;
import org.elasticsearch.painless.symbol.IRDecorations.IRDName;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Locale;
import java.util.function.Consumer;

/**
Expand Down Expand Up @@ -1107,13 +1110,19 @@ private void replaceCallWithConstant(
ExpressionNode argNode = irInvokeCallMemberNode.getArgumentNodes().get(i);
IRDConstant constantDecoration = argNode.getDecoration(IRDConstant.class);
if (constantDecoration == null) {
// TODO find a better string to output
throw irInvokeCallMemberNode.getLocation()
.createError(
new IllegalArgumentException(
"all arguments to [" + javaMethod.getName() + "] must be constant but the [" + (i + 1) + "] argument isn't"
)
);
// offering the symbol name in error message (CastNode was evolved from ESymbol)
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
)));
}
args[i] = constantDecoration.getValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public void testClassMethodCompileTimeOnlyVariableParams() {
IllegalArgumentException.class,
() -> scriptEngine.compile(null, "def a = 2; classMul(2, a)", BindingsTestScript.CONTEXT, Collections.emptyMap())
);
assertThat(e.getMessage(), equalTo("all arguments to [classMul] must be constant but the [2] argument isn't"));
assertThat(e.getMessage(), equalTo("All arguments of the [classMul] method must be constants, but the [2nd] argument [a] is not"));
}

public void testClassMethodCompileTimeOnlyThrows() {
Expand Down Expand Up @@ -269,7 +269,7 @@ public void testInstanceMethodCompileTimeOnlyVariableParams() {
IllegalArgumentException.class,
() -> scriptEngine.compile(null, "def a = 2; instanceMul(a, 2)", BindingsTestScript.CONTEXT, Collections.emptyMap())
);
assertThat(e.getMessage(), equalTo("all arguments to [instanceMul] must be constant but the [1] argument isn't"));
assertThat(e.getMessage(), equalTo("All arguments of the [instanceMul] method must be constants, but the [1st] argument [a] is not"));
}

public void testCompileTimeOnlyParameterFoldedToConstant() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.painless;

import org.elasticsearch.test.ESTestCase;

import java.util.Map;

import static java.util.Map.entry;

public class UtilityTests extends ESTestCase {

public static final Map<Integer, String> ORDINALS = Map.ofEntries(
entry(0, "0th"),
entry(1, "1st"),
entry(2, "2nd"),
entry(3, "3rd"),
entry(4, "4th"),
entry(5, "5th"),
entry(10, "10th"),
entry(11, "11th"),
entry(12, "12th"),
entry(13, "13th"),
entry(14, "14th"),
entry(20, "20th"),
entry(21, "21st"),
entry(22, "22nd"),
entry(23, "23rd"),
entry(24, "24th"),
entry(100, "100th"),
entry(101, "101st"),
entry(102, "102nd"),
entry(103, "103rd"),
entry(104, "104th"),
entry(111, "111th"),
entry(112, "112th"),
entry(113, "113th"),
entry(114, "114th"),
entry(1000, "1000th")
);

public void testToOrdinal() {
for (Map.Entry<Integer, String> item : ORDINALS.entrySet()) {
assertEquals(Utility.toOrdinal(item.getKey()), item.getValue());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fetch:
---
mutable pattern:
- do:
catch: /all arguments to \[grok\] must be constant but the \[1\] argument isn't/
catch: /All arguments of the \[grok\] method must be constants, but the \[1st\] argument \[\] is not/
search:
index: http_logs
body:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fetch:
---
mutable pattern:
- do:
catch: /all arguments to \[grok\] must be constant but the \[1\] argument isn't/
catch: /All arguments of the \[grok\] method must be constants, but the \[1st\] argument \[\] is not/
search:
index: http_logs
body:
Expand Down

0 comments on commit b32ae4c

Please sign in to comment.