Bug 560310 - [null] Bogus error message in loop
Change-Id: I9f28022b7c8af9dc08f4d7b3962b5b0f9b0fd013
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java
index a89da88..ae901e2 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullTypeAnnotationTest.java
@@ -9961,7 +9961,7 @@
"2. ERROR in x\\C.java (at line 8)\n" +
" @NonNull Object x = t; // error, should warn?\n" +
" ^\n" +
- "Null type mismatch (type annotations): required \'@NonNull Object\' but this expression has type \'T1 extends @Nullable Number\'\n" +
+ "Null type safety: required \'@NonNull\' but this expression has type \'T1\', a free type variable that may represent a \'@Nullable\' type\n" +
"----------\n" +
"3. ERROR in x\\C.java (at line 10)\n" +
" return t.toString(); // legal???\n" +
@@ -18062,4 +18062,73 @@
};
runner.runConformTest();
}
+public void testBug560310() {
+ runConformTestWithLibs(
+ new String[] {
+ "confusing/Confusing.java",
+ "package confusing;\n" +
+ "\n" +
+ "import java.util.ArrayList;\n" +
+ "\n" +
+ "import org.eclipse.jdt.annotation.NonNullByDefault;\n" +
+ "\n" +
+ "public abstract class Confusing {\n" +
+ " abstract int unannotated(ArrayList<String> list);\n" +
+ "\n" +
+ " @NonNullByDefault\n" +
+ " public void f(boolean x) {\n" +
+ " ArrayList<String> list = x ? null : new ArrayList<>();\n" +
+ "\n" +
+ " while (true) {\n" +
+ " unannotated(list);\n" +
+ " }\n" +
+ " }\n" +
+ "}\n"
+ },
+ getCompilerOptions(),
+ "----------\n" +
+ "1. INFO in confusing\\Confusing.java (at line 15)\n" +
+ " unannotated(list);\n" +
+ " ^^^^\n" +
+ "Unsafe null type conversion (type annotations): The value of type \'ArrayList<@NonNull String>\' is made accessible using the less-annotated type \'ArrayList<String>\'\n" +
+ "----------\n");
+}
+public void testBug560310try_finally() {
+ runConformTestWithLibs(
+ new String[] {
+ "confusing/Confusing.java",
+ "package confusing;\n" +
+ "\n" +
+ "import java.util.ArrayList;\n" +
+ "\n" +
+ "import org.eclipse.jdt.annotation.NonNullByDefault;\n" +
+ "\n" +
+ "public abstract class Confusing {\n" +
+ " abstract int unannotated(ArrayList<String> list);\n" +
+ "\n" +
+ " @NonNullByDefault\n" +
+ " public void f(boolean x) {\n" +
+ " ArrayList<String> list = x ? null : new ArrayList<>();\n" +
+ "\n" +
+ " try {\n" +
+ " unannotated(list);\n" +
+ " } finally {\n" +
+ " unannotated(list);\n" +
+ " }\n" +
+ " }\n" +
+ "}\n"
+ },
+ getCompilerOptions(),
+ "----------\n" +
+ "1. INFO in confusing\\Confusing.java (at line 15)\n" +
+ " unannotated(list);\n" +
+ " ^^^^\n" +
+ "Unsafe null type conversion (type annotations): The value of type \'ArrayList<@NonNull String>\' is made accessible using the less-annotated type \'ArrayList<String>\'\n" +
+ "----------\n" +
+ "2. INFO in confusing\\Confusing.java (at line 17)\n" +
+ " unannotated(list);\n" +
+ " ^^^^\n" +
+ "Unsafe null type conversion (type annotations): The value of type \'ArrayList<@NonNull String>\' is made accessible using the less-annotated type \'ArrayList<String>\'\n" +
+ "----------\n");
+}
}
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/NullAnnotationMatching.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/NullAnnotationMatching.java
index 2be11ef..1054469 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/NullAnnotationMatching.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/NullAnnotationMatching.java
@@ -129,6 +129,14 @@
this.nullStatus = nullStatus;
}
+ /**
+ * For creating updated status during *FlowContext.complainOnDeferred*Checks() once the actual nullStatus is known
+ */
+ public NullAnnotationMatching withNullStatus(int updatedNullStatus) {
+ return updatedNullStatus == this.nullStatus ? this
+ : new NullAnnotationMatching(this.severity, updatedNullStatus, this.superTypeHint);
+ }
+
public boolean isAnyMismatch() { return this.severity.isAnyMismatch(); }
public boolean isUnchecked() { return this.severity == Severity.UNCHECKED || this.severity == Severity.UNCHECKED_TO_UNANNOTATED; }
public boolean isAnnotatedToUnannotated() { return this.severity == Severity.UNCHECKED_TO_UNANNOTATED; }
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java
index e763088..5cd6819 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2000, 2014 IBM Corporation and others.
+ * Copyright (c) 2000, 2020 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -27,6 +27,7 @@
import org.eclipse.jdt.internal.compiler.ast.ASTNode;
import org.eclipse.jdt.internal.compiler.ast.Expression;
+import org.eclipse.jdt.internal.compiler.ast.NullAnnotationMatching;
import org.eclipse.jdt.internal.compiler.ast.Reference;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
@@ -52,6 +53,7 @@
ASTNode[] nullReferences; // Expressions for null checking, Statements for resource analysis
// cast to Expression is safe if corresponding nullCheckType != EXIT_RESOURCE
int[] nullCheckTypes;
+ NullAnnotationMatching[] nullAnnotationStatuses;
int nullCount;
// see also the related field FlowContext#expectedTypes
@@ -210,7 +212,15 @@
int nullStatus = flowInfo.nullStatus(local);
if (nullStatus != FlowInfo.NON_NULL) {
char[][] annotationName = scope.environment().getNonNullAnnotationName();
- scope.problemReporter().nullityMismatch((Expression) location, this.providedExpectedTypes[i][0], this.providedExpectedTypes[i][1], nullStatus, annotationName);
+ TypeBinding providedType = this.providedExpectedTypes[i][0];
+ TypeBinding expectedType = this.providedExpectedTypes[i][1];
+ Expression expression2 = (Expression) location;
+ if (this.nullAnnotationStatuses[i] != null) {
+ this.nullAnnotationStatuses[i] = this.nullAnnotationStatuses[i].withNullStatus(nullStatus);
+ scope.problemReporter().nullityMismatchingTypeAnnotation(expression2, providedType, expectedType, this.nullAnnotationStatuses[i]);
+ } else {
+ scope.problemReporter().nullityMismatch(expression2, providedType, expectedType, nullStatus, annotationName);
+ }
}
break;
case IN_UNBOXING:
@@ -449,12 +459,13 @@
}
@Override
-protected void recordNullReference(LocalVariableBinding local,
- ASTNode expression, int checkType, FlowInfo nullInfo) {
+protected void recordNullReferenceWithAnnotationStatus(LocalVariableBinding local,
+ ASTNode expression, int checkType, FlowInfo nullInfo, NullAnnotationMatching nullAnnotationStatus) {
if (this.nullCount == 0) {
this.nullLocals = new LocalVariableBinding[5];
this.nullReferences = new ASTNode[5];
this.nullCheckTypes = new int[5];
+ this.nullAnnotationStatuses = new NullAnnotationMatching[5];
}
else if (this.nullCount == this.nullLocals.length) {
int newLength = this.nullCount * 2;
@@ -467,9 +478,13 @@
System.arraycopy(this.nullCheckTypes, 0,
this.nullCheckTypes = new int[newLength], 0,
this.nullCount);
+ System.arraycopy(this.nullAnnotationStatuses, 0,
+ this.nullAnnotationStatuses = new NullAnnotationMatching[this.nullCount * 2], 0,
+ this.nullCount);
}
this.nullLocals[this.nullCount] = local;
this.nullReferences[this.nullCount] = expression;
+ this.nullAnnotationStatuses[this.nullCount] = nullAnnotationStatus;
this.nullCheckTypes[this.nullCount++] = checkType;
}
@Override
@@ -480,12 +495,12 @@
recordNullReference(null, expression, IN_UNBOXING, flowInfo);
}
@Override
-protected boolean internalRecordNullityMismatch(Expression expression, TypeBinding providedType, FlowInfo flowInfo, int nullStatus, TypeBinding expectedType, int checkType) {
+protected boolean internalRecordNullityMismatch(Expression expression, TypeBinding providedType, FlowInfo flowInfo, int nullStatus, NullAnnotationMatching nullAnnotationStatus, TypeBinding expectedType, int checkType) {
// cf. decision structure inside FinallyFlowContext.recordUsingNullReference(..)
if (nullStatus == FlowInfo.UNKNOWN ||
((this.tagBits & FlowContext.DEFER_NULL_DIAGNOSTIC) != 0 && nullStatus != FlowInfo.NULL)) {
recordProvidedExpectedTypes(providedType, expectedType, this.nullCount);
- recordNullReference(expression.localVariableBinding(), expression, checkType, flowInfo);
+ recordNullReferenceWithAnnotationStatus(expression.localVariableBinding(), expression, checkType, flowInfo, nullAnnotationStatus);
return true;
}
return false;
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java
index 3d73bad..8fb978d 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2000, 2017 IBM Corporation and others.
+ * Copyright (c) 2000, 2020 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -820,8 +820,33 @@
* Alternatively, a {@link #IN_UNBOXING} check can e requested.
* @param nullInfo the null flow info observed at this first visit of location.
*/
-protected void recordNullReference(LocalVariableBinding local,
+protected final void recordNullReference(LocalVariableBinding local,
ASTNode location, int checkType, FlowInfo nullInfo) {
+ recordNullReferenceWithAnnotationStatus(local, location, checkType, nullInfo, null);
+}
+
+/**
+ * Record a null reference for use by deferred checks. Only looping or
+ * finally contexts really record that information. Other contexts
+ * immediately check for unboxing.
+ * @param local the local variable involved in the check
+ * @param location the location triggering the analysis, for normal null dereference
+ * this is an expression resolving to 'local', for resource leaks it is an
+ * early exit statement.
+ * @param checkType the checkType against which the check must be performed; one of
+ * {@link #CAN_ONLY_NULL CAN_ONLY_NULL}, {@link #CAN_ONLY_NULL_NON_NULL
+ * CAN_ONLY_NULL_NON_NULL}, {@link #MAY_NULL MAY_NULL},
+ * {@link #CAN_ONLY_NON_NULL CAN_ONLY_NON_NULL}, potentially
+ * combined with a context indicator (one of {@link #IN_COMPARISON_NULL},
+ * {@link #IN_COMPARISON_NON_NULL}, {@link #IN_ASSIGNMENT} or {@link #IN_INSTANCEOF}).
+ * <br>
+ * Alternatively, a {@link #IN_UNBOXING} check can e requested.
+ * @param nullInfo the null flow info observed at this first visit of location.
+ * @param nullAnnotationStatus if null annotations are analysed this may hold more information
+ * about the exact kind of problem, can be <code>null</code>
+ */
+protected void recordNullReferenceWithAnnotationStatus(LocalVariableBinding local,
+ ASTNode location, int checkType, FlowInfo nullInfo, NullAnnotationMatching nullAnnotationStatus) {
// default implementation: do nothing
}
@@ -1054,7 +1079,7 @@
if ((this.tagBits & FlowContext.HIDE_NULL_COMPARISON_WARNING) != 0) {
isInsideAssert = FlowContext.HIDE_NULL_COMPARISON_WARNING;
}
- if (currentContext.internalRecordNullityMismatch(expression, providedType, flowInfo, nullStatus, expectedType, ASSIGN_TO_NONNULL | isInsideAssert))
+ if (currentContext.internalRecordNullityMismatch(expression, providedType, flowInfo, nullStatus, annotationStatus, expectedType, ASSIGN_TO_NONNULL | isInsideAssert))
return;
currentContext = currentContext.parent;
}
@@ -1066,7 +1091,7 @@
currentScope.problemReporter().nullityMismatch(expression, providedType, expectedType, nullStatus,
currentScope.environment().getNonNullAnnotationName());
}
-protected boolean internalRecordNullityMismatch(Expression expression, TypeBinding providedType, FlowInfo flowInfo, int nullStatus, TypeBinding expectedType, int checkType) {
+protected boolean internalRecordNullityMismatch(Expression expression, TypeBinding providedType, FlowInfo flowInfo, int nullStatus, NullAnnotationMatching nullAnnotationStatus, TypeBinding expectedType, int checkType) {
// nop, to be overridden in subclasses
return false; // not recorded
}
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java
index 82f40c8..203f568 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2000, 2017 IBM Corporation and others.
+ * Copyright (c) 2000, 2020 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -37,6 +37,7 @@
import org.eclipse.jdt.internal.compiler.ast.ASTNode;
import org.eclipse.jdt.internal.compiler.ast.Expression;
import org.eclipse.jdt.internal.compiler.ast.FakedTrackingVariable;
+import org.eclipse.jdt.internal.compiler.ast.NullAnnotationMatching;
import org.eclipse.jdt.internal.compiler.ast.Reference;
import org.eclipse.jdt.internal.compiler.codegen.BranchLabel;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
@@ -75,6 +76,7 @@
// cast to Expression is safe if corresponding nullCheckType != EXIT_RESOURCE
int[] nullCheckTypes;
UnconditionalFlowInfo[] nullInfos; // detailed null info observed during the first visit of nullReferences[i], or null
+ NullAnnotationMatching[] nullAnnotationStatuses;
int nullCount;
// see also the related field FlowContext#expectedTypes
@@ -408,7 +410,15 @@
int nullStatus = flowInfo.nullStatus(local);
if (nullStatus != FlowInfo.NON_NULL) {
char[][] annotationName = scope.environment().getNonNullAnnotationName();
- scope.problemReporter().nullityMismatch((Expression) location, this.providedExpectedTypes[i][0], this.providedExpectedTypes[i][1], nullStatus, annotationName);
+ TypeBinding providedType = this.providedExpectedTypes[i][0];
+ TypeBinding expectedType = this.providedExpectedTypes[i][1];
+ Expression expression2 = (Expression) location;
+ if (this.nullAnnotationStatuses[i] != null) {
+ this.nullAnnotationStatuses[i] = this.nullAnnotationStatuses[i].withNullStatus(nullStatus);
+ scope.problemReporter().nullityMismatchingTypeAnnotation(expression2, providedType, expectedType, this.nullAnnotationStatuses[i]);
+ } else {
+ scope.problemReporter().nullityMismatch(expression2, providedType, expectedType, nullStatus, annotationName);
+ }
}
break;
case EXIT_RESOURCE:
@@ -563,13 +573,14 @@
}
@Override
-protected void recordNullReference(LocalVariableBinding local,
- ASTNode expression, int checkType, FlowInfo nullInfo) {
+protected void recordNullReferenceWithAnnotationStatus(LocalVariableBinding local,
+ ASTNode expression, int checkType, FlowInfo nullInfo, NullAnnotationMatching nullAnnotationStatus) {
if (this.nullCount == 0) {
this.nullLocals = new LocalVariableBinding[5];
this.nullReferences = new ASTNode[5];
this.nullCheckTypes = new int[5];
this.nullInfos = new UnconditionalFlowInfo[5];
+ this.nullAnnotationStatuses = new NullAnnotationMatching[5];
}
else if (this.nullCount == this.nullLocals.length) {
System.arraycopy(this.nullLocals, 0,
@@ -580,10 +591,13 @@
this.nullCheckTypes = new int[this.nullCount * 2], 0, this.nullCount);
System.arraycopy(this.nullInfos, 0,
this.nullInfos = new UnconditionalFlowInfo[this.nullCount * 2], 0, this.nullCount);
+ System.arraycopy(this.nullAnnotationStatuses, 0,
+ this.nullAnnotationStatuses = new NullAnnotationMatching[this.nullCount * 2], 0, this.nullCount);
}
this.nullLocals[this.nullCount] = local;
this.nullReferences[this.nullCount] = expression;
this.nullCheckTypes[this.nullCount] = checkType;
+ this.nullAnnotationStatuses[this.nullCount] = nullAnnotationStatus;
this.nullInfos[this.nullCount++] = nullInfo != null ? nullInfo.unconditionalCopy() : null;
}
@Override
@@ -781,9 +795,9 @@
}
@Override
- protected boolean internalRecordNullityMismatch(Expression expression, TypeBinding providedType, FlowInfo flowInfo, int nullStatus, TypeBinding expectedType, int checkType) {
+ protected boolean internalRecordNullityMismatch(Expression expression, TypeBinding providedType, FlowInfo flowInfo, int nullStatus, NullAnnotationMatching nullAnnotationStatus, TypeBinding expectedType, int checkType) {
recordProvidedExpectedTypes(providedType, expectedType, this.nullCount);
- recordNullReference(expression.localVariableBinding(), expression, checkType, flowInfo);
+ recordNullReferenceWithAnnotationStatus(expression.localVariableBinding(), expression, checkType, flowInfo, nullAnnotationStatus);
return true;
}
}
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java
index e82038d..fd1f596 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java
@@ -10602,19 +10602,29 @@
nullityMismatchIsNull(expression, requiredType);
return;
}
- // try to improve nonnull vs. nullable:
- if (status.isPotentiallyNullMismatch()
- && (requiredType.tagBits & TagBits.AnnotationNonNull) != 0
- && (providedType.tagBits & TagBits.AnnotationNullable) == 0)
- {
- if(this.options.pessimisticNullAnalysisForFreeTypeVariablesEnabled && providedType.isTypeVariable() && !providedType.hasNullTypeAnnotations()) {
- nullityMismatchIsFreeTypeVariable(providedType, expression.sourceStart, expression.sourceEnd);
+ if ((requiredType.tagBits & TagBits.AnnotationNonNull) != 0) { // some problems need a closer look to report the best possible message:
+ // try to improve nonnull vs. nullable:
+ if (status.isPotentiallyNullMismatch()
+ && (providedType.tagBits & TagBits.AnnotationNullable) == 0)
+ {
+ if(this.options.pessimisticNullAnalysisForFreeTypeVariablesEnabled && providedType.isTypeVariable() && !providedType.hasNullTypeAnnotations()) {
+ nullityMismatchIsFreeTypeVariable(providedType, expression.sourceStart, expression.sourceEnd);
+ return;
+ }
+
+ nullityMismatchPotentiallyNull(expression, requiredType, this.options.nonNullAnnotationName);
return;
}
-
- nullityMismatchPotentiallyNull(expression, requiredType, this.options.nonNullAnnotationName);
- return;
+ VariableBinding var = expression.localVariableBinding();
+ if (var == null && expression instanceof Reference) {
+ var = ((Reference)expression).lastFieldBinding();
+ }
+ if(var != null && var.type.isFreeTypeVariable()) {
+ nullityMismatchVariableIsFreeTypeVariable(var, expression);
+ return;
+ }
}
+
String[] arguments;
String[] shortArguments;
