Improper warning - Potential null pointer access

Fixes eclipse-jdt#1461 Sets the flow info reach mode to FlowInfo.UNREACHABLE_BY_NULLANALYSIS after 'Object o = null;if (Objects.isNull(o)) return;', 'Object o = "";if (Objects.nonNull(o)) return; Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
snjeza · Dec 5, 2023 · 01829a4 · 01829a4
1 parent dc8021b
commit 01829a4
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 1 deletion.
diff --git a/...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java b/...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java
@@ -65,7 +65,9 @@
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.ast;
 
-import static org.eclipse.jdt.internal.compiler.ast.ExpressionContext.*;
+import static org.eclipse.jdt.internal.compiler.ast.ExpressionContext.ASSIGNMENT_CONTEXT;
+import static org.eclipse.jdt.internal.compiler.ast.ExpressionContext.INVOCATION_CONTEXT;
+import static org.eclipse.jdt.internal.compiler.ast.ExpressionContext.VANILLA_CONTEXT;
 
 import java.util.HashMap;
 import java.util.function.BiConsumer;
@@ -75,6 +77,7 @@
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
 import org.eclipse.jdt.internal.compiler.codegen.Opcodes;
+import org.eclipse.jdt.internal.compiler.flow.ConditionalFlowInfo;
 import org.eclipse.jdt.internal.compiler.flow.FlowContext;
 import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
 import org.eclipse.jdt.internal.compiler.flow.UnconditionalFlowInfo;
@@ -223,14 +226,17 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
 				case ARG_NONNULL_IF_TRUE:
 					recordFlowUpdateOnResult(((SingleNameReference) argument).localVariableBinding(), true, false);
 					flowInfo = argument.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits();
+					flowInfo = setUnreachable(flowInfo, flowContext, argument, false);
 					break;
 				case ARG_NONNULL_IF_TRUE_NEGATABLE:
 					recordFlowUpdateOnResult(((SingleNameReference) argument).localVariableBinding(), true, true);
 					flowInfo = argument.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits();
+					flowInfo = setUnreachable(flowInfo, flowContext, argument, false);
 					break;
 				case ARG_NULL_IF_TRUE:
 					recordFlowUpdateOnResult(((SingleNameReference) argument).localVariableBinding(), false, true);
 					flowInfo = argument.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits();
+					flowInfo = setUnreachable(flowInfo, flowContext, argument, true);
 					break;
 				default:
 					flowInfo = argument.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits();
@@ -264,6 +270,30 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
 	flowContext.expireNullCheckedFieldInfo(); // no longer trust this info after any message send
 	return flowInfo;
 }
+
+private FlowInfo setUnreachable(FlowInfo flowInfo, FlowContext flowContext, Expression argument,
+		boolean init) {
+	if (flowInfo instanceof UnconditionalFlowInfo && (flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0) {
+		int status = argument.nullStatus(flowInfo, flowContext);
+		if (status == FlowInfo.NULL) {
+			flowInfo = FlowInfo.conditional(flowInfo.copy(), flowInfo.copy());
+			if (init) {
+				((ConditionalFlowInfo) flowInfo).initsWhenFalse().setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
+			} else {
+				((ConditionalFlowInfo) flowInfo).initsWhenTrue().setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
+			}
+		}
+		if (status == FlowInfo.NON_NULL) {
+			flowInfo = FlowInfo.conditional(flowInfo.copy(), flowInfo.copy());
+			if (init) {
+				((ConditionalFlowInfo) flowInfo).initsWhenTrue.setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
+			} else {
+				((ConditionalFlowInfo) flowInfo).initsWhenFalse.setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
+			}
+		}
+	}
+	return flowInfo;
+}
 public void recordFlowUpdateOnResult(LocalVariableBinding local, boolean nonNullIfTrue, boolean negatable) {
 	this.flowUpdateOnBooleanResult = (f, result) -> {
 		if (result || negatable) {

diff --git a/....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java b/....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java
@@ -18568,4 +18568,33 @@ void m(String packageName, IPS[] packages) {
 			"""
 		});
 }
+// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1461
+public void testGH1461() {
+	if (this.complianceLevel == 0) {
+		buildAllCompliancesTestSuite(testClass());
+		this.complianceLevel = ClassFileConstants.JDK1_8;
+	}
+	if (this.complianceLevel < ClassFileConstants.JDK1_7) return;
+	runNegativeTest(
+		new String[] {
+			"X.java",
+			"import java.util.Objects;\n"
+			+ "public class X {\n"
+			+ "	public void test() { \n"
+			+ "		String name = null;\n"
+			+ "		if (Objects.isNull(name)) { \n"
+			+ "			System.out.println(\"Name is null\");\n"
+			+ "			return;\n"
+			+ "		}\n"
+			+ "		System.out.println(name.substring(0, 4));\n"
+			+ "	}\n"
+			+ "}\n"
+			},
+		"----------\n" +
+		"1. WARNING in X.java (at line 9)\n"
+		+ "	System.out.println(name.substring(0, 4));\n"
+		+ "	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n"
+		+ "Dead code\n"
+		+ "----------\n");
+}
 }