Revert null-safe behavior to error at runtime instead of compiletime (#…

…65099) This reverts a change where null-safe was enhanced to cause a compile-time error instead of a run- time error when the target value was a primitive type. The reason for the reversion is consistency across def/non-def types and versions. I've added a follow up issue to fix this behavior in general (#65098).
elastic · Nov 16, 2020 · 0beffcd · 0beffcd
1 parent afd12fd
commit 0beffcd
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 4 deletions.
diff --git a/...painless/src/main/java/org/elasticsearch/painless/phase/DefaultSemanticAnalysisPhase.java b/...painless/src/main/java/org/elasticsearch/painless/phase/DefaultSemanticAnalysisPhase.java
@@ -2485,11 +2485,14 @@ public void visitDot(EDot userDotNode, SemanticScope semanticScope) {
                             "Field [" + index + "] does not exist for type [" + prefixValueType.getValueCanonicalTypeName() + "]."));
                 }
             } else if (prefixValueType != null && prefixValueType.getValueType() == def.class) {
-                TargetType targetType = semanticScope.getDecoration(userDotNode, TargetType.class);
+                TargetType targetType = userDotNode.isNullSafe() ? null : semanticScope.getDecoration(userDotNode, TargetType.class);
                 // TODO: remove ZonedDateTime exception when JodaCompatibleDateTime is removed
                 valueType = targetType == null || targetType.getTargetType() == ZonedDateTime.class ||
                         semanticScope.getCondition(userDotNode, Explicit.class) ? def.class : targetType.getTargetType();
-                semanticScope.setCondition(userDotNode, DefOptimized.class);
+
+                if (write) {
+                    semanticScope.setCondition(userDotNode, DefOptimized.class);
+                }
             } else {
                 Class<?> prefixType;
                 String prefixCanonicalTypeName;
@@ -2708,7 +2711,10 @@ public void visitBrace(EBrace userBraceNode, SemanticScope semanticScope) {
             // TODO: remove ZonedDateTime exception when JodaCompatibleDateTime is removed
             valueType = targetType == null || targetType.getTargetType() == ZonedDateTime.class ||
                     semanticScope.getCondition(userBraceNode, Explicit.class) ? def.class : targetType.getTargetType();
-            semanticScope.setCondition(userBraceNode, DefOptimized.class);
+
+            if (write) {
+                semanticScope.setCondition(userBraceNode, DefOptimized.class);
+            }
         } else if (Map.class.isAssignableFrom(prefixValueType)) {
             String canonicalClassName = PainlessLookupUtility.typeToCanonicalTypeName(prefixValueType);
 
@@ -2854,7 +2860,7 @@ public void visitCall(ECall userCallNode, SemanticScope semanticScope) {
                 }
             }
 
-            TargetType targetType = semanticScope.getDecoration(userCallNode, TargetType.class);
+            TargetType targetType = userCallNode.isNullSafe() ? null : semanticScope.getDecoration(userCallNode, TargetType.class);
             // TODO: remove ZonedDateTime exception when JodaCompatibleDateTime is removed
             valueType = targetType == null || targetType.getTargetType() == ZonedDateTime.class ||
                     semanticScope.getCondition(userCallNode, Explicit.class) ? def.class : targetType.getTargetType();

diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java
@@ -840,4 +840,10 @@ public void testInvalidFullyQualifiedStaticReferenceType() {
         iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("while (java.util.List) {java.util.List x = 1;}"));
         assertEquals(iae.getMessage(), "value required: instead found unexpected type [java.util.List]");
     }
+
+    public void testInvalidNullSafeBehavior() {
+        expectScriptThrows(ClassCastException.class, () ->
+                exec("def test = ['hostname': 'somehostname']; test?.hostname && params.host.hostname != ''"));
+        expectScriptThrows(NullPointerException.class, () -> exec("params?.host?.hostname && params.host?.hostname != ''"));
+    }
 }