Bug 533339 - incorrect Null type mismatch warning when using Guava's

checkNotNull, even with severity set to Ignore

Change-Id: Idc1f1d55a277b35ee1e03c0c484327f1a2b4f24d

5 files changed, 73 insertions, 10 deletions

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 48d2f60fa3..4e6725bf70 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
@@ -5594,6 +5594,11 @@ public void testTypeVariable7a() {
 		"	<U> U m(I1<U> in) { return in.get(); }\n" + 
 		"	           ^\n" + 
 		"Null constraint mismatch: The type \'U\' is not a valid substitute for the type parameter \'@NonNull T\'\n" + 
+		"----------\n" + 
+		"2. WARNING in X.java (at line 6)\n" + 
+		"	@Nullable String s = m(() -> \"OK\");\n" + 
+		"	                       ^^^^^^^^^^\n" + 
+		"Contradictory null annotations: function type was inferred as \'@NonNull @Nullable String ()\', but only one of \'@NonNull\' and \'@Nullable\' can be effective at any location\n" + 
 		"----------\n",
 		"OK");
 }
@@ -5624,10 +5629,10 @@ public void testTypeVariable7err() {
 		"	                           ^^^^^^^^\n" + 
 		"Null type mismatch (type annotations): required \'U\' but this expression has type \'@Nullable U\', where \'U\' is a free type variable\n" + 
 		"----------\n" + 
-		"3. WARNING in X.java (at line 6)\n" + 
+		"3. ERROR in X.java (at line 6)\n" + 
 		"	@NonNull String s = m(() -> \"\");\n" + 
-		"	                    ^^^^^^^^^^^\n" + 
-		"Null type safety (type annotations): The expression of type \'String\' needs unchecked conversion to conform to \'@NonNull String\'\n" + 
+		"	                      ^^^^^^^^\n" + 
+		"Contradictory null annotations: function type was inferred as \'@Nullable @NonNull String ()\', but only one of \'@NonNull\' and \'@Nullable\' can be effective at any location\n" + 
 		"----------\n");
 }
 //Bug 435570 - [1.8][null] @NonNullByDefault illegally tries to affect "throws E"
@@ -7149,7 +7154,7 @@ public void testBug448777() {
 		"2. ERROR in DoubleInference.java (at line 13)\n" + 
 		"	return applyWith(i -> i, \"hallo\");\n" + 
 		"	                 ^^^^^^\n" + 
-		"The target type of this expression must be a functional interface\n" + 
+		"Contradictory null annotations: function type was inferred as \'@Nullable @NonNull String (@Nullable @NonNull String)\', but only one of \'@NonNull\' and \'@Nullable\' can be effective at any location\n" + 
 		"----------\n" + 
 		"3. ERROR in DoubleInference.java (at line 15)\n" + 
 		"	void test2(Func<String> f1, Func<@NonNull String> f2) {\n" + 
@@ -17472,4 +17477,52 @@ public void testBug531040() {
 		"----------\n"
 	);
 }
+public void testBug533339() {
+	runNegativeTestWithLibs(
+		new String[] {
+			"Test.java",
+			"import org.eclipse.jdt.annotation.NonNull;\n" + 
+			"import org.eclipse.jdt.annotation.Nullable;\n" + 
+			"\n" + 
+			"public class Test {\n" + 
+			"\n" + 
+			"	interface Foo {\n" + 
+			"\n" + 
+			"		@Nullable\n" + 
+			"		String getString();\n" + 
+			"	}\n" + 
+			"\n" + 
+			"	class Bar {\n" + 
+			"\n" + 
+			"		Bar(@NonNull String s) {\n" + 
+			"		}\n" + 
+			"	}\n" + 
+			"\n" + 
+			"	Bar hasWarning(Foo foo) {\n" + 
+			"		@NonNull String s = checkNotNull(foo.getString());\n" + 
+			"		return new Bar(s);// Null type mismatch: required '@NonNull String' but the provided value is inferred as @Nullable\n" + 
+			"	}\n" + 
+			"\n" + 
+			"	Bar hasNoWarning(Foo foo) {\n" + 
+			"		return new Bar(checkNotNull(foo.getString()));// no warning when s is inlined\n" + 
+			"	}\n" +
+			"	static <T> T checkNotNull(T reference) {\n" +
+			"		if (reference == null) throw new NullPointerException();\n" +
+			"		return reference;\n" +
+			"	}\n" + 
+			"}\n"
+		}, 
+		getCompilerOptions(),
+		"----------\n" + 
+		"1. WARNING in Test.java (at line 19)\n" + 
+		"	@NonNull String s = checkNotNull(foo.getString());\n" + 
+		"	                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
+		"Null type safety (type annotations): The expression of type \'String\' needs unchecked conversion to conform to \'@NonNull String\'\n" + 
+		"----------\n" + 
+		"2. WARNING in Test.java (at line 24)\n" + 
+		"	return new Bar(checkNotNull(foo.getString()));// no warning when s is inlined\n" + 
+		"	               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
+		"Null type safety (type annotations): The expression of type \'String\' needs unchecked conversion to conform to \'@NonNull String\'\n" + 
+		"----------\n");
+}
 }
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FunctionalExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FunctionalExpression.java
index 2972c23489..64ed970872 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FunctionalExpression.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FunctionalExpression.java
@@ -192,7 +192,7 @@ public abstract class FunctionalExpression extends Expression {
 			blockScope.problemReporter().targetTypeIsNotAFunctionalInterface(this);
 			return null;
 		}
-		if (!sam.isValidBinding()) {
+		if (!sam.isValidBinding() && sam.problemId() != ProblemReasons.ContradictoryNullAnnotations) {
 			return reportSamProblem(blockScope, sam);
 		}
 		
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java
index 5c0ff3f67b..54d49f16bc 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java
@@ -349,7 +349,15 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
 			return;
 		}
 
-		// store the constant for final locals
+		boolean mayRequireTypeInference = scope.environment().usesNullTypeAnnotations() 
+				&& variableType.isValidBinding()
+				&& (this.initialization instanceof Invocation || this.initialization instanceof ConditionalExpression);
+		if (mayRequireTypeInference) {
+			// these are definitely no constants, so resolving annotations early should be safe
+			resolveAnnotations(scope, this.annotations, this.binding, true);
+			// for type inference having null annotations upfront gives better results
+			variableType = this.type.resolvedType;
+		}
 		if (this.initialization != null) {
 			if (this.initialization instanceof ArrayInitializer) {
 				TypeBinding initializationType = this.initialization.resolveTypeExpecting(scope, variableType);
@@ -400,8 +408,9 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
 					? this.initialization.constant.castTo((variableType.id << 4) + this.initialization.constant.typeID())
 					: Constant.NotAConstant);
 		}
-		// only resolve annotation at the end, for constant to be positioned before (96991)
-		resolveAnnotations(scope, this.annotations, this.binding, true);
+		// if init could be a constant only resolve annotation at the end, for constant to be positioned before (96991)
+		if (!mayRequireTypeInference)
+			resolveAnnotations(scope, this.annotations, this.binding, true);
 		Annotation.isTypeUseCompatible(this.type, scope, this.annotations);
 		if (!scope.validateNullAnnotation(this.binding.tagBits, this.type, this.annotations))
 			this.binding.tagBits &= ~TagBits.AnnotationNullMASK;
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ProblemMethodBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ProblemMethodBinding.java
index 1024b8d9a6..d973cdc1d5 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ProblemMethodBinding.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ProblemMethodBinding.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2016 IBM Corporation and others.
+ * Copyright (c) 2000, 2018 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -38,7 +38,7 @@ public ProblemMethodBinding(MethodBinding closestMatch, char[] selector, TypeBin
 	if (closestMatch != null && problemReason != ProblemReasons.Ambiguous) {
 		this.declaringClass = closestMatch.declaringClass;
 		this.returnType = closestMatch.returnType;
-		if (problemReason == ProblemReasons.InvocationTypeInferenceFailure) {
+		if (problemReason == ProblemReasons.InvocationTypeInferenceFailure || problemReason == ProblemReasons.ContradictoryNullAnnotations) {
 			this.thrownExceptions = closestMatch.thrownExceptions;
 			this.typeVariables = closestMatch.typeVariables;
 			this.modifiers = closestMatch.modifiers;
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 aeed19c409..a319c86b4a 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
@@ -10372,6 +10372,7 @@ public void arrayReferencePotentialNullReference(ArrayReference arrayReference)
 }
 public void nullityMismatchingTypeAnnotation(Expression expression, TypeBinding providedType, TypeBinding requiredType, NullAnnotationMatching status) 
 {
+	if (providedType == requiredType) return; //$IDENTITY-COMPARISON$
 	// try to improve nonnull vs. null:
 	if (providedType.id == TypeIds.T_null || status.nullStatus == FlowInfo.NULL) {
 		nullityMismatchIsNull(expression, requiredType);