From 2d927fababd58459db1d89d743618399ef175461 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Tue, 28 Nov 2017 13:44:52 -0800 Subject: [PATCH] Painless: Fix errors allowing void to be assigned to def. (#27460) --- .../painless/node/EAssignment.java | 6 ++++- .../painless/node/PSubDefCall.java | 7 ++++-- .../painless/node/PSubDefField.java | 1 - .../elasticsearch/painless/AdditionTests.java | 22 +++++++++---------- .../org/elasticsearch/painless/CastTests.java | 9 ++++++++ 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java index 84c6145ac0c..873f109e72d 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java @@ -20,7 +20,6 @@ package org.elasticsearch.painless.node; import org.elasticsearch.painless.DefBootstrap; -import org.elasticsearch.painless.Definition; import org.elasticsearch.painless.Definition.Cast; import org.elasticsearch.painless.Definition.Type; import org.elasticsearch.painless.Globals; @@ -213,6 +212,11 @@ public final class EAssignment extends AExpression { // If the lhs node is a def optimized node we update the actual type to remove the need for a cast. if (lhs.isDefOptimized()) { rhs.analyze(locals); + + if (rhs.actual.clazz == void.class) { + throw createError(new IllegalArgumentException("Right-hand side cannot be a [void] type for assignment.")); + } + rhs.expected = rhs.actual; lhs.updateActual(rhs.actual); // Otherwise, we must adapt the rhs type to the lhs type with a cast. diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefCall.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefCall.java index 89fc169704f..560acaf131e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefCall.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefCall.java @@ -19,9 +19,7 @@ package org.elasticsearch.painless.node; -import java.util.Collections; import org.elasticsearch.painless.DefBootstrap; -import org.elasticsearch.painless.Definition; import org.elasticsearch.painless.Globals; import org.elasticsearch.painless.Locals; import org.elasticsearch.painless.Location; @@ -29,6 +27,7 @@ import org.elasticsearch.painless.MethodWriter; import org.objectweb.asm.Type; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Set; @@ -76,6 +75,10 @@ final class PSubDefCall extends AExpression { totalCaptures += lambda.getCaptureCount(); } + if (expression.actual.clazz == void.class) { + throw createError(new IllegalArgumentException("Argument(s) cannot be of [void] type when calling method [" + name + "].")); + } + expression.expected = expression.actual; arguments.set(argument, expression.cast(locals)); } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefField.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefField.java index c1f0c468e42..99d60c3b73f 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefField.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubDefField.java @@ -20,7 +20,6 @@ package org.elasticsearch.painless.node; import org.elasticsearch.painless.DefBootstrap; -import org.elasticsearch.painless.Definition; import org.elasticsearch.painless.Definition.Type; import org.elasticsearch.painless.Globals; import org.elasticsearch.painless.Locals; diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/AdditionTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/AdditionTests.java index 554da280dda..f124d088bf2 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/AdditionTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/AdditionTests.java @@ -22,7 +22,7 @@ package org.elasticsearch.painless; /** Tests for addition operator across all types */ //TODO: NaN/Inf/overflow/... public class AdditionTests extends ScriptTestCase { - + public void testBasics() throws Exception { assertEquals(3.0, exec("double x = 1; byte y = 2; return x + y;")); } @@ -195,7 +195,7 @@ public class AdditionTests extends ScriptTestCase { assertEquals(1.0+0.0, exec("return 1.0+0.0;")); assertEquals(0.0+0.0, exec("return 0.0+0.0;")); } - + public void testDef() { assertEquals(2, exec("def x = (byte)1; def y = (byte)1; return x + y")); assertEquals(2, exec("def x = (short)1; def y = (byte)1; return x + y")); @@ -253,7 +253,7 @@ public class AdditionTests extends ScriptTestCase { assertEquals(2D, exec("def x = (float)1; def y = (double)1; return x + y")); assertEquals(2D, exec("def x = (double)1; def y = (double)1; return x + y")); } - + public void testDefTypedLHS() { assertEquals(2, exec("byte x = (byte)1; def y = (byte)1; return x + y")); assertEquals(2, exec("short x = (short)1; def y = (byte)1; return x + y")); @@ -311,7 +311,7 @@ public class AdditionTests extends ScriptTestCase { assertEquals(2D, exec("float x = (float)1; def y = (double)1; return x + y")); assertEquals(2D, exec("double x = (double)1; def y = (double)1; return x + y")); } - + public void testDefTypedRHS() { assertEquals(2, exec("def x = (byte)1; byte y = (byte)1; return x + y")); assertEquals(2, exec("def x = (short)1; byte y = (byte)1; return x + y")); @@ -369,19 +369,19 @@ public class AdditionTests extends ScriptTestCase { assertEquals(2D, exec("def x = (float)1; double y = (double)1; return x + y")); assertEquals(2D, exec("def x = (double)1; double y = (double)1; return x + y")); } - + public void testDefNulls() { expectScriptThrows(NullPointerException.class, () -> { - exec("def x = null; int y = 1; return x + y"); + exec("def x = null; int y = 1; return x + y"); }); expectScriptThrows(NullPointerException.class, () -> { - exec("int x = 1; def y = null; return x + y"); + exec("int x = 1; def y = null; return x + y"); }); expectScriptThrows(NullPointerException.class, () -> { - exec("def x = null; def y = 1; return x + y"); + exec("def x = null; def y = 1; return x + y"); }); } - + public void testCompoundAssignment() { // byte assertEquals((byte) 15, exec("byte x = 5; x += 10; return x;")); @@ -406,7 +406,7 @@ public class AdditionTests extends ScriptTestCase { assertEquals(15D, exec("double x = 5.0; x += 10; return x;")); assertEquals(-5D, exec("double x = 5.0; x += -10; return x;")); } - + public void testDefCompoundAssignmentLHS() { // byte assertEquals((byte) 15, exec("def x = (byte)5; x += 10; return x;")); @@ -431,7 +431,7 @@ public class AdditionTests extends ScriptTestCase { assertEquals(15D, exec("def x = 5.0; x += 10; return x;")); assertEquals(-5D, exec("def x = 5.0; x += -10; return x;")); } - + public void testDefCompoundAssignmentRHS() { // byte assertEquals((byte) 15, exec("byte x = 5; def y = 10; x += y; return x;")); diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/CastTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/CastTests.java index 0ca72f993e5..c9954fd7171 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/CastTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/CastTests.java @@ -318,4 +318,13 @@ public class CastTests extends ScriptTestCase { exec("def x = 5L; boolean y = (boolean) (x + x); return y"); }); } + + public void testIllegalVoidCasts() { + expectScriptThrows(IllegalArgumentException.class, () -> { + exec("def map = ['a': 1,'b': 2,'c': 3]; map.c = Collections.sort(new ArrayList(map.keySet()));"); + }); + expectScriptThrows(IllegalArgumentException.class, () -> { + exec("Map map = ['a': 1,'b': 2,'c': 3]; def x = new HashMap(); x.put(1, map.clear());"); + }); + } }