From 73bf29072fd313d657b80fe015214b2f1256c5da Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 2 Feb 2017 10:15:03 -0500 Subject: [PATCH] Painless: Fix def invoked qualified method refs (#22918) We were incorrectly resolving qualified method references at run time when invoked on `def`. This lead to errors like `The struct with name [org] has not been defined.` when attempting ``` doc.date.dates.stream().map( org.joda.time.ReadableDateTime::centuryOfEra ).collect(Collectors.toList()) ``` --- .../java/org/elasticsearch/painless/Def.java | 2 +- .../painless/FunctionRefTests.java | 60 ++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java index 6de116da0e9..1438dab084f 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/Def.java @@ -268,7 +268,7 @@ public final class Def { if (lambdaArgs.get(i - 1)) { // decode signature of form 'type.call,2' String signature = (String) args[upTo++]; - int separator = signature.indexOf('.'); + int separator = signature.lastIndexOf('.'); int separator2 = signature.indexOf(','); String type = signature.substring(1, separator); String call = signature.substring(separator+1, separator2); diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java index 7136807bd7e..0f9ef54d614 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java @@ -19,6 +19,13 @@ package org.elasticsearch.painless; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; + +import static java.util.Collections.singletonMap; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.startsWith; + public class FunctionRefTests extends ScriptTestCase { public void testStaticMethodReference() { @@ -37,6 +44,30 @@ public class FunctionRefTests extends ScriptTestCase { assertEquals(2, exec("def l = new ArrayList(); l.add(1); l.add(1); return l.stream().mapToInt(Integer::intValue).sum();")); } + public void testQualifiedStaticMethodReference() { + assertEquals(true, + exec("List l = [true]; l.stream().map(org.elasticsearch.painless.FeatureTest::overloadedStatic).findFirst().get()")); + } + + public void testQualifiedStaticMethodReferenceDef() { + assertEquals(true, + exec("def l = [true]; l.stream().map(org.elasticsearch.painless.FeatureTest::overloadedStatic).findFirst().get()")); + } + + public void testQualifiedVirtualMethodReference() { + long instant = randomLong(); + assertEquals(instant, exec( + "List l = [params.d]; return l.stream().mapToLong(org.joda.time.ReadableDateTime::getMillis).sum()", + singletonMap("d", new DateTime(instant, DateTimeZone.UTC)), true)); + } + + public void testQualifiedVirtualMethodReferenceDef() { + long instant = randomLong(); + assertEquals(instant, exec( + "def l = [params.d]; return l.stream().mapToLong(org.joda.time.ReadableDateTime::getMillis).sum()", + singletonMap("d", new DateTime(instant, DateTimeZone.UTC)), true)); + } + public void testCtorMethodReference() { assertEquals(3.0D, exec("List l = new ArrayList(); l.add(1.0); l.add(2.0); " + @@ -144,10 +175,33 @@ public class FunctionRefTests extends ScriptTestCase { } public void testMethodMissing() { - IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> { - exec("List l = new ArrayList(); l.add(2); l.add(1); l.sort(Integer::bogus); return l.get(0);"); + Exception e = expectScriptThrows(IllegalArgumentException.class, () -> { + exec("List l = [2, 1]; l.sort(Integer::bogus); return l.get(0);"); }); - assertTrue(expected.getMessage().contains("Unknown reference")); + assertThat(e.getMessage(), startsWith("Unknown reference")); + } + + public void testQualifiedMethodMissing() { + Exception e = expectScriptThrows(IllegalArgumentException.class, () -> { + exec("List l = [2, 1]; l.sort(org.joda.time.ReadableDateTime::bogus); return l.get(0);", false); + }); + assertThat(e.getMessage(), startsWith("Unknown reference")); + } + + public void testClassMissing() { + Exception e = expectScriptThrows(IllegalArgumentException.class, () -> { + exec("List l = [2, 1]; l.sort(Bogus::bogus); return l.get(0);", false); + }); + assertThat(e.getMessage(), endsWith("Variable [Bogus] is not defined.")); + } + + public void testQualifiedClassMissing() { + Exception e = expectScriptThrows(IllegalArgumentException.class, () -> { + exec("List l = [2, 1]; l.sort(org.joda.time.BogusDateTime::bogus); return l.get(0);", false); + }); + /* Because the type isn't known and we use the lexer hack this fails to parse. I find this error message confusing but it is the one + * we have... */ + assertEquals("invalid sequence of tokens near ['::'].", e.getMessage()); } public void testNotFunctionalInterface() {