Skip to content

Commit

Permalink
Painless: improve error message on non-constant (elastic#68517)
Browse files Browse the repository at this point in the history
As of elastic#68088 painless can have methods where all parameters must be
constant. This improves the error message when the parameter isn't. It's
still not super great, but its better and its what we can easilly give
at that point in the compiler.
  • Loading branch information
nik9000 committed Feb 4, 2021
1 parent 95ee440 commit e5b134d
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -841,12 +841,17 @@ private void replaceCallWithConstant(
Object[] args = new Object[irInvokeCallMemberNode.getArgumentNodes().size()];
for (int i = 0; i < irInvokeCallMemberNode.getArgumentNodes().size(); i++) {
ExpressionNode argNode = irInvokeCallMemberNode.getArgumentNodes().get(i);
if (argNode instanceof ConstantNode == false) {
IRDConstant constantDecoration = argNode.getDecoration(IRDConstant.class);
if (constantDecoration == null) {
// TODO find a better string to output
throw irInvokeCallMemberNode.getLocation()
.createError(new IllegalArgumentException("all arguments must be constant but the [" + (i + 1) + "] argument isn't"));
.createError(
new IllegalArgumentException(
"all arguments to [" + javaMethod.getName() + "] must be constant but the [" + (i + 1) + "] argument isn't"
)
);
}
args[i] = ((ConstantNode) argNode).getDecorationValue(IRDConstant.class);
args[i] = constantDecoration.getValue();
}
Object result;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,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 must be constant but the [2] argument isn't"));
assertThat(e.getMessage(), equalTo("all arguments to [classMul] must be constant but the [2] argument isn't"));
}

public void testClassMethodCompileTimeOnlyThrows() {
Expand Down Expand Up @@ -246,7 +246,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 must be constant but the [1] argument isn't"));
assertThat(e.getMessage(), equalTo("all arguments to [instanceMul] must be constant but the [1] argument isn't"));
}

public void testCompileTimeOnlyParameterFoldedToConstant() {
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 must be constant but the \[1\] argument isn't/
catch: /all arguments to \[grok\] must be constant but the \[1\] argument isn't/
search:
index: http_logs
body:
Expand Down

0 comments on commit e5b134d

Please sign in to comment.