fix for null analysis issue for conditional expressions - fixes

mpalat · mpalat · commit fb6cd91bbed2 · 2025-05-21T14:38:50.000+05:30
issue eclipse-jdt#3990
diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java
@@ -2089,6 +2089,8 @@ public interface IProblem {
 	/** @since 3.41 */
 	int FieldWithUnresolvedOwningAnnotation = Internal + 989;
 
+	/** @since 3.42 */
+	int RedundantNullCheckNullValueExpression = Internal + 990;
 
 	// Java 8 work
 	/** @since 3.10 */
diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2020 IBM Corporation and others.
+ * Copyright (c) 2000, 2025 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -32,6 +32,7 @@
 import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
 import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
 import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding;
+import org.eclipse.jdt.internal.compiler.lookup.NullTypeBinding;
 import org.eclipse.jdt.internal.compiler.lookup.Scope;
 import org.eclipse.jdt.internal.compiler.lookup.TagBits;
 import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
@@ -80,6 +81,10 @@ && shouldPerformSyntacticAnalsysisFor(scope, reference))
 				if (field != null && (field.type.tagBits & TagBits.IsBaseType) == 0) {
 					flowContext.recordNullCheckedFieldReference((Reference) this.left, 1);
 				}
+			} else if (this.left instanceof ConditionalExpression ce) {
+				if ((ce.resolvedType.tagBits & TagBits.IsBaseType) == 0 || ce.resolvedType instanceof NullTypeBinding) {
+					checkConditionalExpressionComparison(scope, flowContext, flowInfo, rightStatus, this.left);
+				}
 			}
 		}
 		if (!rightNonNullChecked) {
@@ -96,6 +101,10 @@ && shouldPerformSyntacticAnalsysisFor(scope, reference))
 				if (field != null && (field.type.tagBits & TagBits.IsBaseType) == 0) {
 					flowContext.recordNullCheckedFieldReference((Reference) this.right, 1);
 				}
+			} else if (this.right instanceof ConditionalExpression ce) {
+				if ((ce.resolvedType.tagBits & TagBits.IsBaseType) == 0 || ce.resolvedType instanceof NullTypeBinding) {
+					checkConditionalExpressionComparison(scope, flowContext, flowInfo, leftStatus, this.right);
+				}
 			}
 		}
 
@@ -175,6 +184,32 @@ private void checkVariableComparison(BlockScope scope, FlowContext flowContext,
 		// we do not impact enclosing try context because this kind of protection
 		// does not preclude the variable from being null in an enclosing scope
 	}
+	private void checkConditionalExpressionComparison(BlockScope scope,
+			FlowContext flowContext, FlowInfo flowInfo,
+			int nullStatus, Expression reference) {
+
+
+		switch (nullStatus) {
+			case FlowInfo.NULL :
+				if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) {
+					flowContext.flagConditional(scope, reference,
+							FlowContext.CAN_ONLY_NULL | FlowContext.IN_COMPARISON_NULL, flowInfo);
+				} else {
+					flowContext.flagConditional(scope, reference,
+							FlowContext.CAN_ONLY_NULL | FlowContext.IN_COMPARISON_NON_NULL, flowInfo);
+				}
+				break;
+			case FlowInfo.NON_NULL :
+				if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) {
+					flowContext.flagConditional(scope, reference,
+							FlowContext.CAN_ONLY_NULL_NON_NULL | FlowContext.IN_COMPARISON_NON_NULL, flowInfo);
+				} else {
+					flowContext.flagConditional(scope, reference,
+							FlowContext.CAN_ONLY_NULL_NON_NULL | FlowContext.IN_COMPARISON_NULL, flowInfo);
+				}
+				break;
+		}
+	}
 	private void analyzeLocalVariable(Expression exp, FlowInfo flowInfo) {
 		if (exp instanceof SingleNameReference && (exp.bits & Binding.LOCAL) != 0 ) {
 			LocalVariableBinding localBinding = (LocalVariableBinding) ((SingleNameReference ) exp).binding;
diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/flow/FlowContext.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/flow/FlowContext.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2020 IBM Corporation and others.
+ * Copyright (c) 2000, 2025 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -1023,6 +1023,41 @@ else if (flowInfo.cannotBeDefinitelyNullOrNonNull(local)) {
 	}
 }
 
+public void flagConditional(Scope scope, Expression conditionalExpression,
+		 int checkType, FlowInfo flowInfo) {
+
+	// if inside an assert, we will not raise redundant null check warnings
+	checkType |= (this.tagBits & FlowContext.HIDE_NULL_COMPARISON_WARNING);
+	int checkTypeWithoutHideNullWarning = checkType & ~FlowContext.HIDE_NULL_COMPARISON_WARNING_MASK;
+	switch (checkTypeWithoutHideNullWarning) {
+		case FlowContext.CAN_ONLY_NULL_NON_NULL | FlowContext.IN_COMPARISON_NULL:
+			if ((checkType & FlowContext.HIDE_NULL_COMPARISON_WARNING) == 0) {
+				scope.problemReporter().expressionRedundantNullComparison(conditionalExpression, /* isExpressionNull */false );
+			}
+			flowInfo.initsWhenTrue().setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
+			break;
+		case FlowContext.CAN_ONLY_NULL_NON_NULL | FlowContext.IN_COMPARISON_NON_NULL:
+			if ((checkType & FlowContext.HIDE_NULL_COMPARISON_WARNING) == 0) {
+				scope.problemReporter().expressionRedundantNullComparison(conditionalExpression, /* isExpressionNull */false);
+			}
+			flowInfo.initsWhenFalse().setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
+			break;
+
+		case FlowContext.CAN_ONLY_NULL | FlowContext.IN_COMPARISON_NULL:
+			if ((checkType & FlowContext.HIDE_NULL_COMPARISON_WARNING) == 0) {
+				scope.problemReporter().expressionRedundantNullComparison(conditionalExpression, /* isExpressionNull */true);
+			}
+			flowInfo.initsWhenFalse().setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
+			break;
+		case FlowContext.CAN_ONLY_NULL | FlowContext.IN_COMPARISON_NON_NULL:
+			if ((checkType & FlowContext.HIDE_NULL_COMPARISON_WARNING) == 0) {
+				scope.problemReporter().expressionRedundantNullComparison(conditionalExpression, /* isExpressionNull */true);
+			}
+			flowInfo.initsWhenTrue().setReachMode(FlowInfo.UNREACHABLE_BY_NULLANALYSIS);
+			break;
+	}
+}
+
 void removeFinalAssignmentIfAny(Reference reference) {
 	// default implementation: do nothing
 }
diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java
@@ -6061,6 +6061,30 @@ public void localVariableNullComparedToNonNull(LocalVariableBinding local, ASTNo
 		nodeSourceEnd(local, location));
 }
 
+/**
+ * @param expr expression being compared for null or non-null
+ * @param isExpressionNull true if flow info shows null, false if flow info is non-null
+ */
+
+public void expressionRedundantNullComparison(Expression expr, boolean isExpressionNull) {
+	if (expr.resolvedType == null)
+		return;
+
+	int problemId = isExpressionNull ? IProblem.RedundantNullCheckNullValueExpression
+			: IProblem.RedundantNullCheckOnNonNullExpression;
+	int severity = computeSeverity(problemId);
+	if (severity == ProblemSeverities.Ignore) return;
+	String[] arguments = new String[] {new String(expr.toString())};
+	this.handle(
+		problemId,
+		arguments,
+		arguments,
+		severity,
+		nodeSourceStart(expr),
+		nodeSourceEnd(expr));
+}
+
+
 /**
  * @param expr expression being compared for null or nonnull
  * @param checkForNull true if checking for null, false if checking for nonnull
diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties
@@ -876,6 +876,9 @@
 988 = Parameter ''{0}'' of method ''{1}'' has an unresolved annotation ''{2}'' that could be relevant for static analysis
 989 = Field ''{0}'' has an unresolved annotation ''{1}'' that could be relevant for static analysis
 
+# null analysis for conditional expressions
+990 = Redundant null check: this expression can only be null
+
 # Java 8
 1001 = Syntax error, modifiers and annotations are not allowed for the lambda parameter {0} as its type is elided
 1002 = Syntax error, modifiers are not allowed here
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java
