From e5b134dae7bf80620dc9bcd9b5d3ef4a0de41137 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 4 Feb 2021 11:08:32 -0500 Subject: [PATCH] Painless: improve error message on non-constant (#68517) As of #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. --- .../DefaultConstantFoldingOptimizationPhase.java | 11 ++++++++--- .../org/elasticsearch/painless/BindingsTests.java | 4 ++-- .../rest-api-spec/test/runtime_fields/250_grok.yml | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java index adcfa3cc0fe52..6b83ba9288aea 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/phase/DefaultConstantFoldingOptimizationPhase.java @@ -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 { 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 index e7e4397faf5cb..c76fcaeaf8d87 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/BindingsTests.java @@ -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() { @@ -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() { diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/250_grok.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/250_grok.yml index 6658853732047..8f369c97ace8d 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/250_grok.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/250_grok.yml @@ -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: