Move not a statement responsibility to user tree expression nodes (#54443)

This removes the "statement" field from the user tree expression nodes 
output. Instead, we delegate responsibility to check whether or not 
something is a statement to each individual user tree expression node 
based on whether or not the result is read from using input from the 
parent. This removes unnecessary state from the output, is more reliable 
as each node can determine its own correct behavior, and results in better 
"not a statement" error messages to give the user an idea of what is 
considered not a statement.

Relates #53702

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/AExpression.java

@@ -65,13 +65,6 @@ public abstract class AExpression extends ANode {
 
     public static class Output {
 
-        /**
-         * Set to true when an expression can be considered a stand alone
-         * statement.  Used to prevent extraneous bytecode. This is always
-         * set by the node as output.
-         */
-        boolean statement = false;
-
         /**
          * Set to the actual type this node is.  Note this variable is always
          * set by the node as output and should only be read from outside of the

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java

@@ -226,7 +226,6 @@ public class EAssignment extends AExpression {
             throw new IllegalStateException("Illegal tree structure.");
         }
 
-        output.statement = true;
         output.actual = input.read ? leftOutput.actual : void.class;
 
         AssignmentNode assignmentNode = new AssignmentNode();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBinary.java

@@ -51,6 +51,11 @@ public class EBinary extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException(
+                "not a statement: result not used from " + operation.name + " operation " + "[" + operation.symbol + "]"));
+        }
+
         Class<?> promote = null;            // promoted type
         Class<?> shiftDistance = null;      // for shifts, the rhs is promoted independently
         boolean originallyExplicit = input.explicit; // record whether there was originally an explicit cast

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBool.java

@@ -47,6 +47,11 @@ public class EBool extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException(
+                "not a statement: result not used from " + operation.name + " operation " + "[" + operation.symbol + "]"));
+        }
+
         Output output = new Output();
 
         Input leftInput = new Input();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EBoolean.java

@@ -40,12 +40,12 @@ public class EBoolean extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Output output = new Output();
-
         if (input.read == false) {
-            throw createError(new IllegalArgumentException("Must read from constant [" + constant + "]."));
+            throw createError(new IllegalArgumentException("not a statement: boolean constant [" + constant + "] not used"));
         }
 
+        Output output = new Output();
+
         output.actual = boolean.class;
 
         ConstantNode constantNode = new ConstantNode();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ECallLocal.java

@@ -167,8 +167,6 @@ public class ECallLocal extends AExpression {
             argumentOutputs.add(argumentOutput);
         }
 
-        output.statement = true;
-
         MemberCallNode memberCallNode = new MemberCallNode();
 
         for (int argument = 0; argument < arguments.size(); ++argument) {

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EComp.java

@@ -50,6 +50,11 @@ public class EComp extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException(
+                "not a statement: result not used from " + operation.name + " operation " + "[" + operation.symbol + "]"));
+        }
+
         Class<?> promotedType;
 
         Output output = new Output();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConditional.java

@@ -48,6 +48,10 @@ public class EConditional extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException("not a statement: result not used from conditional operation [?:]"));
+        }
+
         Output output = new Output();
 
         Input conditionInput = new Input();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EConstant.java

@@ -42,6 +42,10 @@ public class EConstant extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException("not a statement: constant [" + constant + "] not used"));
+        }
+
         Output output = new Output();
 
         if (constant instanceof String) {

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EDecimal.java

@@ -42,14 +42,13 @@ public class EDecimal extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Object constant;
-
-        Output output = new Output();
-
         if (input.read == false) {
-            throw createError(new IllegalArgumentException("Must read from constant [" + value + "]."));
+            throw createError(new IllegalArgumentException("not a statement: decimal constant [" + value + "] not used"));
         }
 
+        Output output = new Output();
+        Object constant;
+
         if (value.endsWith("f") || value.endsWith("F")) {
             try {
                 constant = Float.parseFloat(value.substring(0, value.length() - 1));

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EElvis.java

@@ -46,6 +46,10 @@ public class EElvis extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException("not a statement: result not used from elvis operation [?:]"));
+        }
+
         Output output = new Output();
 
         if (input.expected != null && input.expected.isPrimitive()) {

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EExplicit.java

@@ -43,6 +43,11 @@ public class EExplicit extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException(
+                "not a statement: result not used from explicit cast with target type [" + type + "]"));
+        }
+
         Output output = new Output();
 
         output.actual = scriptRoot.getPainlessLookup().canonicalTypeNameToType(type);

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EInstanceof.java

@@ -46,6 +46,11 @@ public class EInstanceof extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException(
+                "not a statement: result not used from instanceof with target type [" + type + "]"));
+        }
+
         Class<?> resolvedType;
         Class<?> expressionType;
         boolean primitiveExpression;

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EListInit.java

@@ -50,12 +50,11 @@ public class EListInit extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Output output = new Output();
-
         if (input.read == false) {
-            throw createError(new IllegalArgumentException("Must read from list initializer."));
+            throw createError(new IllegalArgumentException("not a statement: result not used from list initializer"));
         }
 
+        Output output = new Output();
         output.actual = ArrayList.class;
 
         PainlessConstructor constructor = scriptRoot.getPainlessLookup().lookupPainlessConstructor(output.actual, 0);

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EMapInit.java

@@ -53,12 +53,11 @@ public class EMapInit extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Output output = new Output();
-
         if (input.read == false) {
-            throw createError(new IllegalArgumentException("Must read from map initializer."));
+            throw createError(new IllegalArgumentException("not a statement: result not used from map initializer"));
         }
 
+        Output output = new Output();
         output.actual = HashMap.class;
 
         PainlessConstructor constructor = scriptRoot.getPainlessLookup().lookupPainlessConstructor(output.actual, 0);

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewArray.java

@@ -49,12 +49,12 @@ public class ENewArray extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Output output = new Output();
-
         if (input.read == false) {
-             throw createError(new IllegalArgumentException("A newly created array must be read from."));
+            throw createError(new IllegalArgumentException("not a statement: result not used from new array"));
         }
 
+        Output output = new Output();
+
         Class<?> clazz = scriptRoot.getPainlessLookup().canonicalTypeNameToType(this.type);
 
         if (clazz == null) {

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENewObj.java

@@ -91,8 +91,6 @@ public class ENewObj extends AExpression {
             argumentOutputs.add(expressionOutput);
         }
 
-        output.statement = true;
-
         NewObjectNode newObjectNode = new NewObjectNode();
 
         for (int i = 0; i < arguments.size(); ++ i) {

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENull.java

@@ -37,12 +37,12 @@ public class ENull extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Output output = new Output();
-
         if (input.read == false) {
-            throw createError(new IllegalArgumentException("Must read from null constant."));
+            throw createError(new IllegalArgumentException("not a statement: null constant not used"));
         }
 
+        Output output = new Output();
+
         if (input.expected != null) {
             if (input.expected.isPrimitive()) {
                 throw createError(new IllegalArgumentException(

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ENumeric.java

@@ -44,14 +44,13 @@ public class ENumeric extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Object constant;
-
-        Output output = new Output();
-
         if (input.read == false) {
-            throw createError(new IllegalArgumentException("Must read from constant [" + value + "]."));
+            throw createError(new IllegalArgumentException("not a statement: numeric constant [" + value + "] not used"));
         }
 
+        Output output = new Output();
+        Object constant;
+
         if (value.endsWith("d") || value.endsWith("D")) {
             if (radix != 10) {
                 throw createError(new IllegalStateException("Illegal tree structure."));

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/ERegex.java

@@ -36,6 +36,7 @@ import org.elasticsearch.painless.symbol.ScriptRoot;
 
 import java.lang.reflect.Modifier;
 import java.util.Arrays;
+import java.util.Objects;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 
@@ -45,24 +46,22 @@ import java.util.regex.PatternSyntaxException;
 public class ERegex extends AExpression {
 
     protected final String pattern;
-    protected final int flags;
+    protected final String flags;
 
-    public ERegex(Location location, String pattern, String flagsString) {
+    public ERegex(Location location, String pattern, String flags) {
         super(location);
 
-        this.pattern = pattern;
-
-        int flags = 0;
-
-        for (int c = 0; c < flagsString.length(); c++) {
-            flags |= flagForChar(flagsString.charAt(c));
-        }
-
+        this.pattern = Objects.requireNonNull(pattern);
         this.flags = flags;
     }
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException(
+                "not a statement: regex constant [" + pattern + "] with flags [" + flags + "] not used"));
+        }
+
         Output output = new Output();
 
         if (scriptRoot.getCompilerSettings().areRegexesEnabled() == false) {
@@ -71,8 +70,10 @@ public class ERegex extends AExpression {
                     + "recursion and long loops."));
         }
 
-        if (input.read == false) {
-            throw createError(new IllegalArgumentException("Regex constant may only be read [" + pattern + "]."));
+        int flags = 0;
+
+        for (int c = 0; c < this.flags.length(); c++) {
+            flags |= flagForChar(this.flags.charAt(c));
         }
 
         try {

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EStatic.java

@@ -42,8 +42,11 @@ public class EStatic extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Output output = new Output();
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException("not a statement: static type [" + type + "] not used"));
+        }
 
+        Output output = new Output();
         output.actual = scriptRoot.getPainlessLookup().canonicalTypeNameToType(type);
 
         if (output.actual == null) {

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EString.java

@@ -42,12 +42,11 @@ public class EString extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
-        Output output = new Output();
-
         if (input.read == false) {
-            throw createError(new IllegalArgumentException("Must read from constant [" + constant + "]."));
+            throw createError(new IllegalArgumentException("not a statement: string constant [" + constant + "] not used"));
         }
 
+        Output output = new Output();
         output.actual = String.class;
 
         ConstantNode constantNode = new ConstantNode();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EUnary.java

@@ -48,6 +48,11 @@ public class EUnary extends AExpression {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, Input input) {
+        if (input.read == false) {
+            throw createError(new IllegalArgumentException(
+                "not a statement: result not used from " + operation.name + " operation " + "[" + operation.symbol + "]"));
+        }
+
         Output output = new Output();
 
         Class<?> promote = null;

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EVariable.java

@@ -54,6 +54,10 @@ public class EVariable extends AStoreable {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, AStoreable.Input input) {
+        if (input.read == false && input.write == false) {
+            throw createError(new IllegalArgumentException("not a statement: variable [" + name + "] not used"));
+        }
+
         Output output = new Output();
 
         Variable variable = scope.getVariable(location, name);

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PBrace.java

@@ -60,6 +60,10 @@ public class PBrace extends AStoreable {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, AStoreable.Input input) {
+        if (input.read == false && input.write == false) {
+            throw createError(new IllegalArgumentException("not a statement: result of brace operator not used"));
+        }
+
         Output output = new Output();
 
         Input prefixInput = new Input();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PCallInvoke.java

@@ -88,8 +88,6 @@ public class PCallInvoke extends AExpression {
         Output subOutput = sub.analyze(classNode, scriptRoot, scope, subInput);
         output.actual = subOutput.actual;
 
-        output.statement = true;
-
         CallNode callNode = new CallNode();
 
         callNode.setLeftNode(prefix.cast(prefixOutput));

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PField.java

@@ -66,6 +66,10 @@ public class PField extends AStoreable {
 
     @Override
     Output analyze(ClassNode classNode, ScriptRoot scriptRoot, Scope scope, AStoreable.Input input) {
+        if (input.read == false && input.write == false) {
+            throw createError(new IllegalArgumentException("not a statement: result of dot operator [.] not used"));
+        }
+
         Output output = new Output();
 
         Input prefixInput = new Input();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/PSubCallInvoke.java

@@ -65,7 +65,6 @@ public class PSubCallInvoke extends AExpression {
             argumentOutputs.add(expressionOutput);
         }
 
-        output.statement = true;
         output.actual = method.returnType;
 
         CallSubNode callSubNode = new CallSubNode();

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SExpression.java

@@ -51,10 +51,6 @@ public class SExpression extends AStatement {
         expressionInput.read = input.lastSource && !isVoid;
         AExpression.Output expressionOutput = expression.analyze(classNode, scriptRoot, scope, expressionInput);
 
-        if ((input.lastSource == false || isVoid) && expressionOutput.statement == false) {
-            throw createError(new IllegalArgumentException("Not a statement."));
-        }
-
         boolean rtn = input.lastSource && isVoid == false && expressionOutput.actual != void.class;
 
         expressionInput.expected = rtn ? rtnType : expressionOutput.actual;

modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SFor.java

@@ -62,10 +62,6 @@ public class SFor extends AStatement {
                 initializerInput.read = false;
                 initializerExpressionOutput = initializer.analyze(classNode, scriptRoot, scope, initializerInput);
 
-                if (initializerExpressionOutput.statement == false) {
-                    throw createError(new IllegalArgumentException("Not a statement."));
-                }
-
                 initializerInput.expected = initializerExpressionOutput.actual;
                 initializer.cast(initializerInput, initializerExpressionOutput);
             } else {
@@ -105,10 +101,6 @@ public class SFor extends AStatement {
             afterthoughtInput.read = false;
             afterthoughtOutput = afterthought.analyze(classNode, scriptRoot, scope, afterthoughtInput);
 
-            if (afterthoughtOutput.statement == false) {
-                throw createError(new IllegalArgumentException("Not a statement."));
-            }
-
             afterthoughtInput.expected = afterthoughtOutput.actual;
             afterthought.cast(afterthoughtInput, afterthoughtOutput);
         }

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

@@ -270,8 +270,9 @@ public class FactoryTests extends ScriptTestCase {
     }
 
     public void testVoidReturn() {
+        scriptEngine.compile("void_return_test", "int x = 1 + 1; return;", VoidReturnTestScript.CONTEXT, Collections.emptyMap());
         IllegalArgumentException iae = expectScriptThrows(IllegalArgumentException.class, () ->
                 scriptEngine.compile("void_return_test", "1 + 1", VoidReturnTestScript.CONTEXT, Collections.emptyMap()));
-        assertEquals(iae.getMessage(), "Not a statement.");
+        assertEquals(iae.getMessage(), "not a statement: result not used from addition operation [+]");
     }
 }

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

@@ -309,4 +309,47 @@ public class WhenThingsGoWrongTests extends ScriptTestCase {
         e = expectScriptThrows(IllegalArgumentException.class, () -> exec("'cat", false));
         assertEquals("unexpected character ['cat].", e.getMessage());
     }
+
+    public void testNotAStatement() {
+        IllegalArgumentException iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("1 * 1; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from multiplication operation [*]");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("boolean x = false; x && true; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from boolean and operation [&&]");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("false; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: boolean constant [false] not used");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("boolean x = false; x == true; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from equals operation [==]");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("boolean x = false; x ? 1 : 2; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from conditional operation [?:]");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("1.1; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: decimal constant [1.1] not used");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("List x = null; x ?: [2]; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from elvis operation [?:]");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("List x = null; (ArrayList)x; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from explicit cast with target type [ArrayList]");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("List x = null; x instanceof List; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from instanceof with target type [List]");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("[]; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from list initializer");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("[:]; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from map initializer");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("new int[] {1, 2}; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from new array");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("null; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: null constant not used");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("1; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: numeric constant [1] not used");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("/a/; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: regex constant [a] with flags [] not used");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("'1'; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: string constant [1] not used");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("+1; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result not used from addition operation [+]");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("boolean x; x; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: variable [x] not used");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("int[] x = new int[] {1}; x[0]; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result of brace operator not used");
+        iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("Integer.MAX_VALUE; return null;"));
+        assertEquals(iae.getMessage(), "not a statement: result of dot operator [.] not used");
+    }
 }