Painless: Don't allow casting from void to def (#22969)

Painless can cast anything into the magic type `def` but it
really shouldn't try to cast **nothing** into `def`. That causes
the byte code generation library to freak out a little.

Closes #22908

modules/lang-painless/src/main/java/org/elasticsearch/painless/AnalyzerCaster.java

@@ -722,9 +722,10 @@ public final class AnalyzerCaster {
                 break;
         }
 
-        if (actual.sort == Sort.DEF || expected.sort == Sort.DEF ||
-            expected.clazz.isAssignableFrom(actual.clazz) ||
-            explicit && actual.clazz.isAssignableFrom(expected.clazz)) {
+        if (       actual.sort == Sort.DEF
+                || (actual.sort != Sort.VOID && expected.sort == Sort.DEF)
+                || expected.clazz.isAssignableFrom(actual.clazz)
+                || (explicit && actual.clazz.isAssignableFrom(expected.clazz))) {
             return new Cast(actual, expected, explicit);
         } else {
             throw location.createError(new ClassCastException("Cannot cast from [" + actual.name + "] to [" + expected.name + "]."));

modules/lang-painless/src/main/java/org/elasticsearch/painless/Locals.java

@@ -318,6 +318,19 @@ public final class Locals {
         public int getSlot() {
             return slot;
         }
+
+        @Override
+        public String toString() {
+            StringBuilder b = new StringBuilder();
+            b.append("Variable[type=").append(type);
+            b.append(",name=").append(name);
+            b.append(",slot=").append(slot);
+            if (readonly) {
+                b.append(",readonly");
+            }
+            b.append(']');
+            return b.toString();
+        }
     }
 
     public static final class Parameter {

modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionRefTests.java

@@ -22,7 +22,10 @@ package org.elasticsearch.painless;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 
+import java.lang.invoke.LambdaConversionException;
+
 import static java.util.Collections.singletonMap;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.startsWith;
 
@@ -208,7 +211,7 @@ public class FunctionRefTests extends ScriptTestCase {
         IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
             exec("List l = new ArrayList(); l.add(2); l.add(1); l.add(Integer::bogus); return l.get(0);");
         });
-        assertTrue(expected.getMessage().contains("Cannot convert function reference"));
+        assertThat(expected.getMessage(), containsString("Cannot convert function reference"));
     }
 
     public void testIncompatible() {
@@ -221,7 +224,7 @@ public class FunctionRefTests extends ScriptTestCase {
         IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
             exec("Optional.empty().orElseGet(String::startsWith);");
         });
-        assertTrue(expected.getMessage().contains("Unknown reference"));
+        assertThat(expected.getMessage(), containsString("Unknown reference"));
     }
     
     public void testWrongArityNotEnough() {
@@ -235,13 +238,38 @@ public class FunctionRefTests extends ScriptTestCase {
         IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
             exec("def y = Optional.empty(); return y.orElseGet(String::startsWith);");
         });
-        assertTrue(expected.getMessage().contains("Unknown reference"));
+        assertThat(expected.getMessage(), containsString("Unknown reference"));
     }
     
     public void testWrongArityNotEnoughDef() {
         IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
             exec("def l = new ArrayList(); l.add(2); l.add(1); l.sort(String::isEmpty);");
         });
-        assertTrue(expected.getMessage().contains("Unknown reference"));
+        assertThat(expected.getMessage(), containsString("Unknown reference"));
+    }
+
+    public void testReturnVoid() {
+        Throwable expected = expectScriptThrows(BootstrapMethodError.class, () -> {
+            exec("StringBuilder b = new StringBuilder(); List l = [1, 2]; l.stream().mapToLong(b::setLength);");
+        });
+        assertThat(expected.getCause().getMessage(),
+                containsString("Type mismatch for lambda expected return: void is not convertible to long"));
+    }
+
+    public void testReturnVoidDef() {
+        Exception expected = expectScriptThrows(LambdaConversionException.class, () -> {
+            exec("StringBuilder b = new StringBuilder(); def l = [1, 2]; l.stream().mapToLong(b::setLength);");
+        });
+        assertThat(expected.getMessage(), containsString("Type mismatch for lambda expected return: void is not convertible to long"));
+
+        expected = expectScriptThrows(LambdaConversionException.class, () -> {
+            exec("def b = new StringBuilder(); def l = [1, 2]; l.stream().mapToLong(b::setLength);");
+        });
+        assertThat(expected.getMessage(), containsString("Type mismatch for lambda expected return: void is not convertible to long"));
+
+        expected = expectScriptThrows(LambdaConversionException.class, () -> {
+            exec("def b = new StringBuilder(); List l = [1, 2]; l.stream().mapToLong(b::setLength);");
+        });
+        assertThat(expected.getMessage(), containsString("Type mismatch for lambda expected return: void is not convertible to long"));
     }
 }

modules/lang-painless/src/test/java/org/elasticsearch/painless/FunctionTests.java

@@ -19,6 +19,8 @@
 
 package org.elasticsearch.painless;
 
+import static org.hamcrest.Matchers.containsString;
+
 public class FunctionTests extends ScriptTestCase {
     public void testBasic() {
         assertEquals(5, exec("int get() {5;} get()"));
@@ -49,21 +51,37 @@ public class FunctionTests extends ScriptTestCase {
         Exception expected = expectScriptThrows(IllegalArgumentException.class, () -> {
             exec("void test(int x) {} test()");
         });
-        assertTrue(expected.getMessage().contains("Cannot generate an empty function"));
+        assertThat(expected.getMessage(), containsString("Cannot generate an empty function"));
     }
 
     public void testDuplicates() {
         Exception expected = expectScriptThrows(IllegalArgumentException.class, () -> {
             exec("void test(int x) {x = 2;} void test(def y) {y = 3;} test()");
         });
-        assertTrue(expected.getMessage().contains("Duplicate functions"));
+        assertThat(expected.getMessage(), containsString("Duplicate functions"));
     }
 
     public void testInfiniteLoop() {
         Error expected = expectScriptThrows(PainlessError.class, () -> {
             exec("void test() {boolean x = true; while (x) {}} test()");
         });
-        assertTrue(expected.getMessage().contains(
-            "The maximum number of statements that can be executed in a loop has been reached."));
+        assertThat(expected.getMessage(),
+                containsString("The maximum number of statements that can be executed in a loop has been reached."));
+    }
+
+    public void testReturnVoid() {
+        assertEquals(null, exec("void test(StringBuilder b, int i) {b.setLength(i)} test(new StringBuilder(), 1)"));
+        Exception expected = expectScriptThrows(IllegalArgumentException.class, () -> {
+            exec("int test(StringBuilder b, int i) {b.setLength(i)} test(new StringBuilder(), 1)");
+        });
+        assertEquals("Not all paths provide a return value for method [test].", expected.getMessage());
+        expected = expectScriptThrows(ClassCastException.class, () -> {
+            exec("int test(StringBuilder b, int i) {return b.setLength(i)} test(new StringBuilder(), 1)");
+        });
+        assertEquals("Cannot cast from [void] to [int].", expected.getMessage());
+        expected = expectScriptThrows(ClassCastException.class, () -> {
+            exec("def test(StringBuilder b, int i) {return b.setLength(i)} test(new StringBuilder(), 1)");
+        });
+        assertEquals("Cannot cast from [void] to [def].", expected.getMessage());
     }
 }

modules/lang-painless/src/test/java/org/elasticsearch/painless/LambdaTests.java

@@ -19,9 +19,12 @@
 
 package org.elasticsearch.painless;
 
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 
+import static org.hamcrest.Matchers.containsString;
+
 public class LambdaTests extends ScriptTestCase {
 
     public void testNoArgLambda() {
@@ -231,4 +234,25 @@ public class LambdaTests extends ScriptTestCase {
         assertEquals(false, exec(compare + "compare(() -> { if (params['number'] == 1) { return params['number'] }" +
             "else { return params['key'] } }, 2)", params, true));
     }
+
+    public void testReturnVoid() {
+        Throwable expected = expectScriptThrows(ClassCastException.class, () -> {
+            exec("StringBuilder b = new StringBuilder(); List l = [1, 2]; l.stream().mapToLong(i -> b.setLength(i))");
+        });
+        assertThat(expected.getMessage(), containsString("Cannot cast from [void] to [long]."));
+    }
+
+    public void testReturnVoidDef() {
+        // If we can catch the error at compile time we do
+        Exception expected = expectScriptThrows(ClassCastException.class, () -> {
+            exec("StringBuilder b = new StringBuilder(); def l = [1, 2]; l.stream().mapToLong(i -> b.setLength(i))");
+        });
+        assertThat(expected.getMessage(), containsString("Cannot cast from [void] to [def]."));
+
+        // Otherwise we convert the void into a null
+        assertEquals(Arrays.asList(null, null),
+                exec("def b = new StringBuilder(); def l = [1, 2]; l.stream().map(i -> b.setLength(i)).collect(Collectors.toList())"));
+        assertEquals(Arrays.asList(null, null),
+                exec("def b = new StringBuilder(); List l = [1, 2]; l.stream().map(i -> b.setLength(i)).collect(Collectors.toList())"));
+    }
 }