Bug 481931 - [compiler][null] Null analysis should consider publicI20190818-1800

static final fields with constructor calls as @NonNull w/o annotations

Change-Id: I15b76706ef1070874f94cbbdda369f80a7684c54

8 files changed, 178 insertions, 5 deletions

diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java
index 07f04a67ea..29c8fd609b 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java
@@ -939,6 +939,7 @@ public void test011_problem_categories() {
 		expectedProblemAttributes.put("RedundantLocalVariableNullAssignment", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM));
 		expectedProblemAttributes.put("RedundantNullAnnotation", new ProblemAttributes(CategorizedProblem.CAT_UNNECESSARY_CODE));
 		expectedProblemAttributes.put("RedundantNullCheckAgainstNonNullType", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM));
+		expectedProblemAttributes.put("RedundantNullCheckOnConstNonNullField", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM));
 		expectedProblemAttributes.put("RedundantNullCheckOnField", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM));
 		expectedProblemAttributes.put("RedundantNullCheckOnNonNullExpression", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM));
 		expectedProblemAttributes.put("RedundantNullCheckOnNonNullSpecdField", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM));
@@ -1887,6 +1888,7 @@ public void test012_compiler_problems_tuning() {
 		expectedProblemAttributes.put("RedundantLocalVariableNullAssignment", new ProblemAttributes(JavaCore.COMPILER_PB_REDUNDANT_NULL_CHECK));
 		expectedProblemAttributes.put("RedundantNullAnnotation", new ProblemAttributes(JavaCore.COMPILER_PB_REDUNDANT_NULL_ANNOTATION));
 		expectedProblemAttributes.put("RedundantNullCheckAgainstNonNullType", new ProblemAttributes(JavaCore.COMPILER_PB_REDUNDANT_NULL_CHECK));
+		expectedProblemAttributes.put("RedundantNullCheckOnConstNonNullField", new ProblemAttributes(JavaCore.COMPILER_PB_REDUNDANT_NULL_CHECK));
 		expectedProblemAttributes.put("RedundantNullCheckOnField", new ProblemAttributes(JavaCore.COMPILER_PB_REDUNDANT_NULL_CHECK));
 		expectedProblemAttributes.put("RedundantNullCheckOnNonNullExpression", new ProblemAttributes(JavaCore.COMPILER_PB_REDUNDANT_NULL_CHECK));
 		expectedProblemAttributes.put("RedundantNullCheckOnNonNullSpecdField", new ProblemAttributes(JavaCore.COMPILER_PB_REDUNDANT_NULL_CHECK));
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java
index 56063c837f..7000f3fd5d 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java
@@ -10641,4 +10641,118 @@ public void testBug499714() {
 		""
 	);
 }
+public void testBug481931_source() {
+	runNegativeTestWithLibs(
+		new String[] {
+			"X.java",
+			"import org.eclipse.jdt.annotation.*;\n" +
+			"public class X {\n" +
+			"	static final String CONST = \"const1\";\n" +
+			"	final String INST_CONST = \"const2\" + CONST;\n" +
+			"	@NonNull String getInstConst() {\n" +
+			"		if (INST_CONST == null) {\n" +
+			"			System.out.println(\"null\");\n" +
+			"		}\n" +
+			"		return INST_CONST;\n" +
+			"	}\n" +
+			"	static @NonNull String getConst() {\n" +
+			"		if (CONST == null) {\n" +
+			"			System.out.println(\"null\");\n" +
+			"		}\n" +
+			"		return CONST;\n" +
+			"	}\n" +
+			"}\n"
+		},
+		getCompilerOptions(),
+		"----------\n" + 
+		"1. ERROR in X.java (at line 6)\n" + 
+		"	if (INST_CONST == null) {\n" + 
+		"	    ^^^^^^^^^^\n" + 
+		"Null comparison always yields false: The field INST_CONST is a nonnull constant\n" + 
+		"----------\n" + 
+		"2. WARNING in X.java (at line 6)\n" + 
+		"	if (INST_CONST == null) {\n" + 
+		"			System.out.println(\"null\");\n" + 
+		"		}\n" + 
+		"	                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
+		"Dead code\n" + 
+		"----------\n" + 
+		"3. ERROR in X.java (at line 12)\n" + 
+		"	if (CONST == null) {\n" + 
+		"	    ^^^^^\n" + 
+		"Null comparison always yields false: The field CONST is a nonnull constant\n" + 
+		"----------\n" + 
+		"4. WARNING in X.java (at line 12)\n" + 
+		"	if (CONST == null) {\n" + 
+		"			System.out.println(\"null\");\n" + 
+		"		}\n" + 
+		"	                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
+		"Dead code\n" + 
+		"----------\n");
+}
+public void testBug481931_binary() {
+	runConformTestWithLibs(
+		new String[] {
+			"test/X.java",
+			"package test;\n" +
+			"public class X {\n" +
+			"	public static final String CONST = \"const1\";\n" +
+			"	public final String INST_CONST = \"const2\" + CONST;\n" +
+			"	X() {}\n" +
+			"	X(int i) {}\n" + // ctors to demonstrate independence of actual initialization
+			"}\n"
+		},
+		getCompilerOptions(),
+		"");
+	Runner runner = new Runner();
+	runner.shouldFlushOutputDirectory = false;
+	runner.testFiles = new String[] {
+			"Y.java",
+			"import org.eclipse.jdt.annotation.*;\n" +
+			"import test.X;\n" +
+			"public class Y {\n" +
+			"	@NonNull String getInstConst(X x) {\n" +
+			"		if (x.INST_CONST == null) {\n" +
+			"			System.out.println(\"null\");\n" +
+			"		}\n" +
+			"		return x.INST_CONST;\n" +
+			"	}\n" +
+			"	static @NonNull String getConst() {\n" +
+			"		if (X.CONST == null) {\n" +
+			"			System.out.println(\"null\");\n" +
+			"		}\n" +
+			"		return X.CONST;\n" +
+			"	}\n" +
+			"}\n"
+		};
+	runner.classLibraries = this.LIBS;
+	runner.customOptions = getCompilerOptions();
+	runner.expectedCompilerLog =
+			"----------\n" + 
+			"1. ERROR in Y.java (at line 5)\n" + 
+			"	if (x.INST_CONST == null) {\n" + 
+			"	      ^^^^^^^^^^\n" + 
+			"Null comparison always yields false: The field INST_CONST is a nonnull constant\n" + 
+			"----------\n" + 
+			"2. WARNING in Y.java (at line 5)\n" + 
+			"	if (x.INST_CONST == null) {\n" + 
+			"			System.out.println(\"null\");\n" + 
+			"		}\n" + 
+			"	                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
+			"Dead code\n" + 
+			"----------\n" + 
+			"3. ERROR in Y.java (at line 11)\n" + 
+			"	if (X.CONST == null) {\n" + 
+			"	      ^^^^^\n" + 
+			"Null comparison always yields false: The field CONST is a nonnull constant\n" + 
+			"----------\n" + 
+			"4. WARNING in Y.java (at line 11)\n" + 
+			"	if (X.CONST == null) {\n" + 
+			"			System.out.println(\"null\");\n" + 
+			"		}\n" + 
+			"	                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
+			"Dead code\n" + 
+			"----------\n";
+	runner.runNegativeTest();
+}
 }
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java
index df38525833..ed6a8d63f0 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java
@@ -17426,14 +17426,58 @@ public void testBug432109() {
 			"}\n"
 		});
 }
-public void testBug435528() {
+public void testBug435528_orig() {
+	Runner runner = new Runner();
+	runner.testFiles =
+		new String[] {
+			"Test.java",
+			"public class Test\n" + 
+			"{\n" + 
+			"   static final String a = \"A\";\n" + 
+			"\n" + 
+			"   static void main(String args[])\n" + 
+			"   {\n" + 
+			"      String x = null;\n" + 
+			"      while (true) {\n" + 
+			"         x = Math.random() < 0.5 ? a : \"BB\";\n" + 
+			"         if (a != null) {\n" + 
+			"            System.out.println(\"s2 value: \" + x);\n" + 
+			"         }\n" + 
+			"         if (x.equals(\"A\")) {\n" + 
+			"            break;\n" + 
+			"         } else {\n" + 
+			"            x = null;\n" + 
+			"         }\n" + 
+			"      }\n" + 
+			"   }\n" + 
+			"}\n"
+		};
+	runner.expectedCompilerLog =
+		"----------\n" + 
+		"1. ERROR in Test.java (at line 10)\n" + 
+		"	if (a != null) {\n" + 
+		"	    ^\n" + 
+		"Null comparison always yields false: The field a is a nonnull constant\n" + 
+		"----------\n" + 
+		"2. WARNING in Test.java (at line 15)\n" + 
+		"	} else {\n" + 
+		"            x = null;\n" + 
+		"         }\n" + 
+		"	       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
+		"Statement unnecessarily nested within else clause. The corresponding then clause does not complete normally\n" + 
+		"----------\n";
+	runner.customOptions = getCompilerOptions();
+	runner.javacTestOptions = JavacTestOptions.Excuse.EclipseHasSomeMoreWarnings;
+	runner.runNegativeTest();
+}
+public void testBug435528_notaconstant() {
 	runConformTest(
 		true/*flush*/,
 		new String[] {
 			"Test.java",
 			"public class Test\n" + 
 			"{\n" + 
-			"   static final String a = \"A\";\n" + 
+			"   static String a	;\n" + 
 			"\n" + 
 			"   static void main(String args[])\n" + 
 			"   {\n" + 
diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations17Test.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations17Test.java
index bdc72f2c31..2558f1a9ad 100644
--- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations17Test.java
+++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/ExternalAnnotations17Test.java
@@ -240,8 +240,8 @@ public class ExternalAnnotations17Test extends ExternalAnnotations18Test {
 				"package libs;\n" + 
 				"\n" +
 				"public interface Lib1 {\n" +
-				"	public static interface Nested {\n" + 
-				"		String one = \"1\";\n" + 
+				"	public static class Nested {\n" + 
+				"		public static String one = \"1\";\n" + 
 				"	}\n" + 
 				"}\n"
 			}, null);
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/core/compiler/IProblem.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/core/compiler/IProblem.java
index dbe2b6067e..15f4535a74 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/core/compiler/IProblem.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/core/compiler/IProblem.java
@@ -1802,6 +1802,8 @@ void setSourceStart(int sourceStart);
 	int FieldComparisonYieldsFalse = Internal + 942;
 	/** @since 3.14 */
 	int RedundantNullDefaultAnnotationModule = Internal + 943;
+	/** @since 3.19 */
+	int RedundantNullCheckOnConstNonNullField = Internal + 944;
 
 	/** @since 3.10 */
 	int ArrayReferencePotentialNullReference = Internal + 951;
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Reference.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Reference.java
index 5eeae470b0..cebb7743ba 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Reference.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Reference.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2016 IBM Corporation and others.
+ * Copyright (c) 2000, 2019 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -26,6 +26,7 @@ import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
 import org.eclipse.jdt.internal.compiler.codegen.Opcodes;
 import org.eclipse.jdt.internal.compiler.flow.FlowContext;
 import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
+import org.eclipse.jdt.internal.compiler.impl.Constant;
 import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
 import org.eclipse.jdt.internal.compiler.lookup.ExtraCompilerModifiers;
 import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
@@ -155,6 +156,8 @@ public int nullStatus(FlowInfo flowInfo, FlowContext flowContext) {
 		return FlowInfo.NON_NULL;
 	FieldBinding fieldBinding = lastFieldBinding();
 	if (fieldBinding != null) {
+		if (fieldBinding.isFinal() && fieldBinding.constant() != Constant.NotAConstant)
+			return FlowInfo.NON_NULL;
 		if (fieldBinding.isNonNull() || flowContext.isNullcheckedFieldAccess(this)) {
 			return FlowInfo.NON_NULL;
 		} else if (fieldBinding.isNullable()) {
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 a7dc401d61..bc42b7703c 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
@@ -167,6 +167,7 @@ import org.eclipse.jdt.internal.compiler.env.AccessRestriction;
 import org.eclipse.jdt.internal.compiler.env.ICompilationUnit;
 import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
 import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
+import org.eclipse.jdt.internal.compiler.impl.Constant;
 import org.eclipse.jdt.internal.compiler.impl.ReferenceContext;
 import org.eclipse.jdt.internal.compiler.impl.StringConstant;
 import org.eclipse.jdt.internal.compiler.lookup.AnnotationBinding;
@@ -434,6 +435,7 @@ public static int getIrritant(int problemID) {
 		case IProblem.NonNullSpecdFieldComparisonYieldsFalse:
 		case IProblem.RedundantNullCheckAgainstNonNullType:
 		case IProblem.RedundantNullCheckOnField:
+		case IProblem.RedundantNullCheckOnConstNonNullField:
 		case IProblem.FieldComparisonYieldsFalse:
 			return CompilerOptions.RedundantNullCheck;
 
@@ -6055,6 +6057,11 @@ public boolean expressionNonNullComparison(Expression expr, boolean checkForNull
 			char[][] nonNullName = this.options.nonNullAnnotationName;
 			arguments = new String[] { new String(field.name), 
 									   new String(nonNullName[nonNullName.length-1]) };
+		} else if (field.constant() != Constant.NotAConstant) {
+			problemId = IProblem.RedundantNullCheckOnConstNonNullField;
+			char[][] nonNullName = this.options.nonNullAnnotationName;
+			arguments = new String[] { new String(field.name), 
+									   new String(nonNullName[nonNullName.length-1]) };
 		} else {
 			// signaling redundancy based on syntactic analysis:
 			problemId = checkForNull
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/messages.properties b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/messages.properties
index 15e133ddeb..33ac72c329 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/messages.properties
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/messages.properties
@@ -815,6 +815,7 @@
 941 = Redundant null check: The field {0} cannot be null at this location (ignoring concurrency)
 942 = Null comparison always yields false: The field {0} cannot be null at this location (ignoring concurrency)
 943 = Nullness default is redundant with a default specified for the enclosing module {0}
+944 = Null comparison always yields false: The field {0} is a nonnull constant
 
 
 951 = Potential null pointer access: array element may be null