diff options
author | Stephan Herrmann | 2012-11-15 13:05:54 +0000 |
---|---|---|
committer | Stephan Herrmann | 2012-11-15 13:08:00 +0000 |
commit | aff037e4075d974a1198e8a25c9e7f07acc35fc1 (patch) | |
tree | e7d54a4d9ea449be68e6d31dd5fa866abaab10a5 | |
parent | a8ed5c9ce2b125913ea11ffa5f66b79c1153133a (diff) | |
download | eclipse.jdt.core-aff037e4075d974a1198e8a25c9e7f07acc35fc1.tar.gz eclipse.jdt.core-aff037e4075d974a1198e8a25c9e7f07acc35fc1.tar.xz eclipse.jdt.core-aff037e4075d974a1198e8a25c9e7f07acc35fc1.zip |
Bug 388281 - [compiler][null] inheritance of null annotations as an
option
19 files changed, 1175 insertions, 255 deletions
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/BatchCompilerTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/BatchCompilerTest.java index c6dd206604..b8ca08bb8d 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/BatchCompilerTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/BatchCompilerTest.java @@ -24,6 +24,7 @@ * bug 365859 - [compiler][null] distinguish warnings based on flow analysis vs. null annotations * bug 374605 - Unreasonable warning for enum-based switch statements * bug 375366 - ECJ ignores unusedParameterIncludeDocCommentReference unless enableJavadoc option is set + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -1886,6 +1887,7 @@ public void test012b(){ " <argument value=\"---OUTPUT_DIR_PLACEHOLDER---\"/>\n" + " </command_line>\n" + " <options>\n" + + " <option key=\"org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations\" value=\"disabled\"/>\n" + " <option key=\"org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation\" value=\"ignore\"/>\n" + " <option key=\"org.eclipse.jdt.core.compiler.annotation.nonnull\" value=\"org.eclipse.jdt.annotation.NonNull\"/>\n" + " <option key=\"org.eclipse.jdt.core.compiler.annotation.nonnullbydefault\" value=\"org.eclipse.jdt.annotation.NonNullByDefault\"/>\n" + 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 5333b1bc14..a398fb9838 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 @@ -5,6 +5,10 @@ * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html * + * This is an implementation of an early-draft specification developed under the Java + * Community Process (JCP) and is made available for testing and evaluation purposes + * only. The code is not compatible with any specification of the JCP. + * * Contributors: * IBM Corporation - initial API and implementation * Benjamin Muskalla - Contribution for bug 239066 @@ -17,11 +21,7 @@ * bug 374605 - Unreasonable warning for enum-based switch statements * bug 382353 - [1.8][compiler] Implementation property modifiers should be accepted on default methods. * bug 382347 - [1.8][compiler] Compiler accepts incorrect default method inheritance - * - * This is an implementation of an early-draft specification developed under the Java - * Community Process (JCP) and is made available for testing and evaluation purposes - * only. The code is not compatible with any specification of the JCP. - * + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -410,6 +410,8 @@ public void test011_problem_categories() { expectedProblemAttributes.put("ContradictoryNullAnnotations", new ProblemAttributes(CategorizedProblem.CAT_INTERNAL)); expectedProblemAttributes.put("ComparingIdentical", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM)); expectedProblemAttributes.put("ConflictingImport", new ProblemAttributes(CategorizedProblem.CAT_IMPORT)); + expectedProblemAttributes.put("ConflictingNullAnnotations", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM)); + expectedProblemAttributes.put("ConflictingInheritedNullAnnotations", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM)); expectedProblemAttributes.put("ConstructorReferenceNotBelow18", new ProblemAttributes(CategorizedProblem.CAT_SYNTAX)); expectedProblemAttributes.put("ConstructorVarargsArgumentNeedCast", new ProblemAttributes(CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM)); expectedProblemAttributes.put("CorruptedSignature", new ProblemAttributes(CategorizedProblem.CAT_BUILDPATH)); @@ -1136,6 +1138,8 @@ public void test012_compiler_problems_tuning() { expectedProblemAttributes.put("CodeSnippetMissingMethod", SKIP); expectedProblemAttributes.put("ComparingIdentical", new ProblemAttributes(JavaCore.COMPILER_PB_COMPARING_IDENTICAL)); expectedProblemAttributes.put("ConflictingImport", SKIP); + expectedProblemAttributes.put("ConflictingNullAnnotations", new ProblemAttributes(JavaCore.COMPILER_PB_NULL_SPECIFICATION_VIOLATION)); + expectedProblemAttributes.put("ConflictingInheritedNullAnnotations", new ProblemAttributes(JavaCore.COMPILER_PB_NULL_SPECIFICATION_VIOLATION)); expectedProblemAttributes.put("ConstructorReferenceNotBelow18", SKIP); expectedProblemAttributes.put("ContradictoryNullAnnotations", SKIP); expectedProblemAttributes.put("ConstructorVarargsArgumentNeedCast", new ProblemAttributes(JavaCore.COMPILER_PB_VARARGS_ARGUMENT_NEED_CAST)); 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 32708b6ff6..cec84ed209 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 @@ -28,7 +28,7 @@ public NullAnnotationTest(String name) { // Static initializer to specify tests subset using TESTS_* static variables // All specified tests which do not belong to the class are skipped... static { -// TESTS_NAMES = new String[] { "testBug388630" }; +// TESTS_NAMES = new String[] { "testBug388281_09" }; // TESTS_NUMBERS = new int[] { 561 }; // TESTS_RANGE = new int[] { 1, 2049 }; } @@ -3655,4 +3655,538 @@ public void testBug388630_2() { "Potential null pointer access: The variable a may be null at this location\n" + "----------\n"); } + +/* Content of Test388281.jar used in the following tests: + +// === package i (explicit annotations): === +package i; +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; +public interface I { + @NonNull Object m1(@Nullable Object a1); + @Nullable String m2(@NonNull Object a2); + Object m1(@Nullable Object o1, Object o2); +} + +// === package i2 with package-info.java (default annot, canceled in one type): === +@org.eclipse.jdt.annotation.NonNullByDefault +package i2; + +package i2; +public interface I2 { + Object m1(Object a1); + String m2(Object a2); +} + +package i2; +public interface II extends i.I { + String m1(Object o1, Object o2); +} + +package i2; +import org.eclipse.jdt.annotation.NonNullByDefault; +@NonNullByDefault(false) +public interface I2A { + Object m1(Object a1); + String m2(Object a2); +} + +// === package c (no null annotations): === +package c; +public class C1 implements i.I { + public Object m1(Object a1) { + System.out.println(a1.toString()); // (1) + return null; // (2) + } + public String m2(Object a2) { + System.out.println(a2.toString()); + return null; + } + public Object m1(Object o1, Object o2) { + return null; + } +} + +package c; +public class C2 implements i2.I2 { + public Object m1(Object a1) { + return a1; + } + public String m2(Object a2) { + return a2.toString(); + } +} + */ +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// Test whether null annotations from a super interface are respected +// Class and its super interface both read from binary +public void testBug388281_01() { + String path = this.getCompilerTestsPluginDirectoryPath() + File.separator + "workspace" + File.separator + "Test388281.jar"; + String[] libs = new String[this.LIBS.length + 1]; + System.arraycopy(this.LIBS, 0, libs, 0, this.LIBS.length); + libs[this.LIBS.length] = path; + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTest( + new String[] { + "Client.java", + "import c.C1;\n" + + "public class Client {\n" + + " void test(C1 c) {\n" + + " String s = c.m2(null); // (3)\n" + + " System.out.println(s.toUpperCase()); // (4)\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in Client.java (at line 4)\n" + + " String s = c.m2(null); // (3)\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull Object\' but the provided value is null\n" + + "----------\n" + + "2. ERROR in Client.java (at line 5)\n" + + " System.out.println(s.toUpperCase()); // (4)\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n", + libs, + true /* shouldFlush*/, + options); +} + +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// Test whether null annotations from a super interface are respected +// Class from source, its supers (class + super interface) from binary +public void testBug388281_02() { + String path = this.getCompilerTestsPluginDirectoryPath() + File.separator + "workspace" + File.separator + "Test388281.jar"; + String[] libs = new String[this.LIBS.length + 1]; + System.arraycopy(this.LIBS, 0, libs, 0, this.LIBS.length); + libs[this.LIBS.length] = path; + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTest( + new String[] { + "ctest/C.java", + "package ctest;\n" + + "public class C extends c.C1 {\n" + + " @Override\n" + + " public Object m1(Object a1) {\n" + + " System.out.println(a1.toString()); // (1)\n" + + " return null; // (2)\n" + + " }\n" + + " @Override\n" + + " public String m2(Object a2) {\n" + + " System.out.println(a2.toString());\n" + + " return null;\n" + + " }\n" + + "}\n", + "Client.java", + "import ctest.C;\n" + + "public class Client {\n" + + " void test(C c) {\n" + + " String s = c.m2(null); // (3)\n" + + " System.out.println(s.toUpperCase()); // (4)\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in ctest\\C.java (at line 5)\n" + + " System.out.println(a1.toString()); // (1)\n" + + " ^^\n" + + "Potential null pointer access: The variable a1 may be null at this location\n" + + "----------\n" + + "2. ERROR in ctest\\C.java (at line 6)\n" + + " return null; // (2)\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull Object\' but the provided value is null\n" + + "----------\n" + + "----------\n" + + "1. ERROR in Client.java (at line 4)\n" + + " String s = c.m2(null); // (3)\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull Object\' but the provided value is null\n" + + "----------\n" + + "2. ERROR in Client.java (at line 5)\n" + + " System.out.println(s.toUpperCase()); // (4)\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n", + libs, + true /* shouldFlush*/, + options); +} + +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// Test whether null annotations from a super interface trigger an error against the overriding implementation +// Class from source, its super interface from binary +public void testBug388281_03() { + String path = this.getCompilerTestsPluginDirectoryPath() + File.separator + "workspace" + File.separator + "Test388281.jar"; + String[] libs = new String[this.LIBS.length + 1]; + System.arraycopy(this.LIBS, 0, libs, 0, this.LIBS.length); + libs[this.LIBS.length] = path; + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTest( + new String[] { + "ctest/C.java", + "package ctest;\n" + + "public class C implements i.I {\n" + + " public Object m1(Object a1) {\n" + + " System.out.println(a1.toString()); // (1)\n" + + " return null; // (2)\n" + + " }\n" + + " public String m2(Object a2) {\n" + + " System.out.println(a2.toString());\n" + + " return null;\n" + + " }\n" + + " public Object m1(Object a1, Object a2) {\n" + + " System.out.println(a1.toString()); // (3)\n" + + " return null;\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in ctest\\C.java (at line 4)\n" + + " System.out.println(a1.toString()); // (1)\n" + + " ^^\n" + + "Potential null pointer access: The variable a1 may be null at this location\n" + + "----------\n" + + "2. ERROR in ctest\\C.java (at line 5)\n" + + " return null; // (2)\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull Object\' but the provided value is null\n" + + "----------\n" + + "3. ERROR in ctest\\C.java (at line 12)\n" + + " System.out.println(a1.toString()); // (3)\n" + + " ^^\n" + + "Potential null pointer access: The variable a1 may be null at this location\n" + + "----------\n", + libs, + true /* shouldFlush*/, + options); +} + +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// Do inherit even if one parameter/return is annotated +// also features some basic overloading +public void testBug388281_04() { + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTestWithLibs( + true /* shouldFlush*/, + new String[] { + "i/I.java", + "package i;\n" + + "import org.eclipse.jdt.annotation.*;\n" + + "public interface I {\n" + + " @NonNull Object m1(@NonNull Object s1, @Nullable String s2);\n" + + " @Nullable Object m1(@Nullable String s1, @NonNull Object s2);\n" + + "}\n", + "ctest/C.java", + "package ctest;\n" + + "import org.eclipse.jdt.annotation.*;\n" + + "public class C implements i.I {\n" + + " public Object m1(@Nullable Object o1, String s2) {\n" + + " System.out.println(s2.toString()); // (1)\n" + + " return null; // (2)\n" + + " }\n" + + " public @NonNull Object m1(String s1, Object o2) {\n" + + " System.out.println(s1.toString()); // (3)\n" + + " return new Object();\n" + + " }\n" + + "}\n" + }, + options, + "----------\n" + + "1. ERROR in ctest\\C.java (at line 5)\n" + + " System.out.println(s2.toString()); // (1)\n" + + " ^^\n" + + "Potential null pointer access: The variable s2 may be null at this location\n" + + "----------\n" + + "2. ERROR in ctest\\C.java (at line 6)\n" + + " return null; // (2)\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull Object\' but the provided value is null\n" + + "----------\n" + + "3. ERROR in ctest\\C.java (at line 9)\n" + + " System.out.println(s1.toString()); // (3)\n" + + " ^^\n" + + "Potential null pointer access: The variable s1 may be null at this location\n" + + "----------\n"); +} + +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// Test whether null annotations from a super interface trigger an error against the overriding implementation +// Class from source, its super interface from binary +// Super interface subject to package level @NonNullByDefault +public void testBug388281_05() { + String path = this.getCompilerTestsPluginDirectoryPath() + File.separator + "workspace" + File.separator + "Test388281.jar"; + String[] libs = new String[this.LIBS.length + 1]; + System.arraycopy(this.LIBS, 0, libs, 0, this.LIBS.length); + libs[this.LIBS.length] = path; + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTest( + new String[] { + "ctest/C.java", + "package ctest;\n" + + "public class C implements i2.I2 {\n" + + " public Object m1(Object a1) {\n" + + " System.out.println(a1.toString()); // silent\n" + + " return null; // (1)\n" + + " }\n" + + " public String m2(Object a2) {\n" + + " System.out.println(a2.toString());\n" + + " return null; // (2)\n" + + " }\n" + + "}\n", + "Client.java", + "import ctest.C;\n" + + "public class Client {\n" + + " void test(C c) {\n" + + " String s = c.m2(null); // (3)\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in ctest\\C.java (at line 5)\n" + + " return null; // (1)\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull Object\' but the provided value is null\n" + + "----------\n" + + "2. ERROR in ctest\\C.java (at line 9)\n" + + " return null; // (2)\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull String\' but the provided value is null\n" + + "----------\n" + + "----------\n" + + "1. ERROR in Client.java (at line 4)\n" + + " String s = c.m2(null); // (3)\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull Object\' but the provided value is null\n" + + "----------\n", + libs, + true /* shouldFlush*/, + options); +} + +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// Conflicting annotations from several indirect super interfaces must be detected +public void testBug388281_06() { + String path = this.getCompilerTestsPluginDirectoryPath() + File.separator + "workspace" + File.separator + "Test388281.jar"; + String[] libs = new String[this.LIBS.length + 1]; + System.arraycopy(this.LIBS, 0, libs, 0, this.LIBS.length); + libs[this.LIBS.length] = path; + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTest( + new String[] { + "ctest/C.java", + "package ctest;\n" + + "public class C extends c.C2 implements i2.I2A {\n" + // neither super has explicit annotations, + // but C2 inherits those from the default applicable at its super interface i2.I2 + // whereas I2A cancels that same default + "}\n" + }, + "----------\n" + + "1. ERROR in ctest\\C.java (at line 2)\n" + + " public class C extends c.C2 implements i2.I2A {\n" + + " ^\n" + + "The method m2(Object) from C2 cannot implement the corresponding method from I2A due to incompatible nullness constraints\n" + + "----------\n" + + "2. ERROR in ctest\\C.java (at line 2)\n" + + " public class C extends c.C2 implements i2.I2A {\n" + + " ^\n" + + "The method m1(Object) from C2 cannot implement the corresponding method from I2A due to incompatible nullness constraints\n" + + "----------\n", + libs, + true /* shouldFlush*/, + options); +} + +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// report conflict between inheritance and default +public void testBug388281_07() { + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTestWithLibs( + new String[] { + "p1/Super.java", + "package p1;\n" + + "import org.eclipse.jdt.annotation.*;\n" + + "public class Super {\n" + + " public @Nullable Object m(@Nullable Object arg) {\n" + + " return null;" + + " }\n" + + "}\n", + "p2/Sub.java", + "package p2;\n" + + "import org.eclipse.jdt.annotation.*;\n" + + "@NonNullByDefault\n" + + "public class Sub extends p1.Super {\n" + + " @Override\n" + + " public Object m(Object arg) { // (a)+(b) conflict at arg and return\n" + + " System.out.println(arg.toString()); // (1)\n" + + " return null;\n" + + " }\n" + + "}\n", + "Client.java", + "public class Client {\n" + + " void test(p2.Sub s) {\n" + + " Object result = s.m(null);\n" + + " System.out.println(result.toString()); // (2)\n" + + " }\n" + + "}\n" + }, + options, + "----------\n" + + "1. ERROR in p2\\Sub.java (at line 6)\n" + + " public Object m(Object arg) { // (a)+(b) conflict at arg and return\n" + + " ^^^^^^\n" + + "The default \'@NonNull\' conflicts with the inherited \'@Nullable\' annotation in the overridden method from Super \n" + + "----------\n" + + "2. ERROR in p2\\Sub.java (at line 6)\n" + + " public Object m(Object arg) { // (a)+(b) conflict at arg and return\n" + + " ^^^\n" + + "The default \'@NonNull\' conflicts with the inherited \'@Nullable\' annotation in the overridden method from Super \n" + + "----------\n" + + "3. ERROR in p2\\Sub.java (at line 7)\n" + + " System.out.println(arg.toString()); // (1)\n" + + " ^^^\n" + + "Potential null pointer access: The variable arg may be null at this location\n" + + "----------\n" + + "----------\n" + + "1. ERROR in Client.java (at line 4)\n" + + " System.out.println(result.toString()); // (2)\n" + + " ^^^^^^\n" + + "Potential null pointer access: The variable result may be null at this location\n" + + "----------\n"); +} + +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// report conflict between inheritance and default - binary types +public void testBug388281_08() { + String path = this.getCompilerTestsPluginDirectoryPath() + File.separator + "workspace" + File.separator + "Test388281.jar"; + String[] libs = new String[this.LIBS.length + 1]; + System.arraycopy(this.LIBS, 0, libs, 0, this.LIBS.length); + libs[this.LIBS.length] = path; + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTest( + new String[] { + "ctest/Ctest.java", + "package ctest;\n" + + "import org.eclipse.jdt.annotation.*;\n" + + "@NonNullByDefault\n" + + "public class Ctest implements i2.II {\n" + + " public Object m1(@Nullable Object a1) { // silent: conflict at a1 avoided\n" + + " return new Object();\n" + + " }\n" + + " public String m2(Object a2) { // (a) conflict at return\n" + + " return null;\n" + + " }\n" + + " public String m1(Object o1, Object o2) { // (b) conflict at o1\n" + + " System.out.println(o1.toString()); // (1) inherited @Nullable\n" + + " return null; // (2) @NonNullByDefault in i2.II\n" + + " }\n" + + "}\n", + "Client.java", + "public class Client {\n" + + " void test(ctest.Ctest c) {\n" + + " Object result = c.m1(null, null); // (3) 2nd arg @NonNullByDefault from i2.II\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in ctest\\Ctest.java (at line 8)\n" + + " public String m2(Object a2) { // (a) conflict at return\n" + + " ^^^^^^\n" + + "The default \'@NonNull\' conflicts with the inherited \'@Nullable\' annotation in the overridden method from I \n" + + "----------\n" + + "2. ERROR in ctest\\Ctest.java (at line 11)\n" + + " public String m1(Object o1, Object o2) { // (b) conflict at o1\n" + + " ^^\n" + + "The default \'@NonNull\' conflicts with the inherited \'@Nullable\' annotation in the overridden method from II \n" + + "----------\n" + + "3. ERROR in ctest\\Ctest.java (at line 12)\n" + + " System.out.println(o1.toString()); // (1) inherited @Nullable\n" + + " ^^\n" + + "Potential null pointer access: The variable o1 may be null at this location\n" + + "----------\n" + + "4. ERROR in ctest\\Ctest.java (at line 13)\n" + + " return null; // (2) @NonNullByDefault in i2.II\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull String\' but the provided value is null\n" + + "----------\n" + + "----------\n" + + "1. ERROR in Client.java (at line 3)\n" + + " Object result = c.m1(null, null); // (3) 2nd arg @NonNullByDefault from i2.II\n" + + " ^^^^\n" + + "Null type mismatch: required \'@NonNull Object\' but the provided value is null\n" + + "----------\n", + libs, + true, // should flush + options); +} + +// https://bugs.eclipse.org/bugs/show_bug.cgi?id=388281 +// difference between inherited abstract & non-abstract methods +public void testBug388281_09() { + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED); + runNegativeTestWithLibs( + new String[] { + "p1/Super.java", + "package p1;\n" + + "import org.eclipse.jdt.annotation.*;\n" + + "public abstract class Super {\n" + + " public abstract @NonNull Object compatible(@Nullable Object arg);\n" + + " public @Nullable Object incompatible(int dummy, @NonNull Object arg) {\n" + + " return null;" + + " }\n" + + "}\n", + "p1/I.java", + "package p1;\n" + + "import org.eclipse.jdt.annotation.*;\n" + + "public interface I {\n" + + " public @Nullable Object compatible(@NonNull Object arg);\n" + + " public @NonNull Object incompatible(int dummy, @Nullable Object arg);\n" + + "}\n", + "p2/Sub.java", + "package p2;\n" + + "public class Sub extends p1.Super implements p1.I {\n" + + " @Override\n" + + " public Object compatible(Object arg) {\n" + + " return this;" + + " }\n" + + " @Override\n" + + " public Object incompatible(int dummy, Object arg) {\n" + + " return null;" + + " }\n" + + "}\n" + }, + options, + "----------\n" + + "1. ERROR in p2\\Sub.java (at line 4)\n" + + " public Object compatible(Object arg) {\n" + + " ^^^^^^\n" + + "Conflict between inherited null annotations \'@Nullable\' declared in I versus \'@NonNull\' declared in Super \n" + + "----------\n" + + "2. ERROR in p2\\Sub.java (at line 4)\n" + + " public Object compatible(Object arg) {\n" + + " ^^^^^^\n" + + "Conflict between inherited null annotations \'@NonNull\' declared in I versus \'@Nullable\' declared in Super \n" + + "----------\n" + + "3. ERROR in p2\\Sub.java (at line 7)\n" + + " public Object incompatible(int dummy, Object arg) {\n" + + " ^^^^^^\n" + + "Conflict between inherited null annotations \'@NonNull\' declared in I versus \'@Nullable\' declared in Super \n" + + "----------\n" + + "4. ERROR in p2\\Sub.java (at line 7)\n" + + " public Object incompatible(int dummy, Object arg) {\n" + + " ^^^^^^\n" + + "Conflict between inherited null annotations \'@Nullable\' declared in I versus \'@NonNull\' declared in Super \n" + + "----------\n"); +} + } diff --git a/org.eclipse.jdt.core.tests.compiler/workspace/Test388281.jar b/org.eclipse.jdt.core.tests.compiler/workspace/Test388281.jar Binary files differnew file mode 100644 index 0000000000..7c66b721c9 --- /dev/null +++ b/org.eclipse.jdt.core.tests.compiler/workspace/Test388281.jar 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 ca3b4b5caa..d9f0206103 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 @@ -155,6 +155,8 @@ * ShouldReturnValueHintMissingDefault * IllegalModifierForInterfaceDefaultMethod * InheritedDefaultMethodConflictsWithOtherInherited + * ConflictingNullAnnotations + * ConflictingInheritedNullAnnotations *******************************************************************************/ package org.eclipse.jdt.core.compiler; @@ -1543,6 +1545,10 @@ void setSourceStart(int sourceStart); int SpecdNonNullLocalVariableComparisonYieldsFalse = Internal + 932; /** @since 3.8 */ int RequiredNonNullButProvidedSpecdNullable = Internal + 933; + /** @since 3.9 */ + int ConflictingNullAnnotations = MethodRelated + 939; + /** @since 3.9 */ + int ConflictingInheritedNullAnnotations = MethodRelated + 940; // Java 8 work diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java index 8ce46a7734..3f5eb634d5 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java @@ -18,6 +18,7 @@ * bug 365531 - [compiler][null] investigate alternative strategy for internally encoding nullness defaults * bug 382353 - [1.8][compiler] Implementation property modifiers should be accepted on default methods. * bug 392099 - [1.8][compiler][null] Apply null annotation on types for null analysis + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -101,8 +102,10 @@ public abstract class AbstractMethodDeclaration argTypeTagBits = (argument.binding.tagBits & TagBits.AnnotationNullMASK); } if (argTypeTagBits != 0) { - if (this.binding.parameterNonNullness == null) + if (this.binding.parameterNonNullness == null) { this.binding.parameterNonNullness = new Boolean[this.arguments.length]; + this.binding.tagBits |= TagBits.IsNullnessKnown; + } this.binding.parameterNonNullness[i] = Boolean.valueOf(argTypeTagBits == TagBits.AnnotationNonNull); } } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java index 134944583e..0803095c89 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java @@ -18,6 +18,7 @@ * bug 388996 - [compiler][resource] Incorrect 'potential resource leak' * bug 379784 - [compiler] "Method can be static" is not getting reported * bug 379834 - Wrong "method can be static" in presence of qualified super and different staticness of nested super class. + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -38,6 +39,7 @@ import org.eclipse.jdt.internal.compiler.lookup.ExtraCompilerModifiers; import org.eclipse.jdt.internal.compiler.lookup.InvocationSite; import org.eclipse.jdt.internal.compiler.lookup.MethodBinding; import org.eclipse.jdt.internal.compiler.lookup.MissingTypeBinding; +import org.eclipse.jdt.internal.compiler.lookup.ImplicitNullAnnotationVerifier; import org.eclipse.jdt.internal.compiler.lookup.PolymorphicMethodBinding; import org.eclipse.jdt.internal.compiler.lookup.ProblemMethodBinding; import org.eclipse.jdt.internal.compiler.lookup.ProblemReasons; @@ -542,6 +544,12 @@ public TypeBinding resolveType(BlockScope scope) { return null; } + if (compilerOptions.isAnnotationBasedNullAnalysisEnabled && (this.binding.tagBits & TagBits.IsNullnessKnown) == 0) { + // not interested in reporting problems against this.binding: + new ImplicitNullAnnotationVerifier(compilerOptions.inheritNullAnnotations) + .checkImplicitNullAnnotations(this.binding, null/*srcMethod*/, false, scope); + } + if (((this.bits & ASTNode.InsideExpressionStatement) != 0) && this.binding.isPolymorphic()) { // we only set the return type to be void if this method invocation is used inside an expression statement diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java index 3c5a3f61e1..754a1e5110 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java @@ -20,6 +20,7 @@ * bug 370639 - [compiler][resource] restore the default for resource leak warnings * bug 366063 - Compiler should not add synthetic @NonNull annotations * bug 374605 - Unreasonable warning for enum-based switch statements + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.impl; @@ -166,6 +167,7 @@ public class CompilerOptions { static final char[][] DEFAULT_NONNULL_ANNOTATION_NAME = CharOperation.splitOn('.', "org.eclipse.jdt.annotation.NonNull".toCharArray()); //$NON-NLS-1$ static final char[][] DEFAULT_NONNULLBYDEFAULT_ANNOTATION_NAME = CharOperation.splitOn('.', "org.eclipse.jdt.annotation.NonNullByDefault".toCharArray()); //$NON-NLS-1$ public static final String OPTION_ReportMissingNonNullByDefaultAnnotation = "org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation"; //$NON-NLS-1$ + public static final String OPTION_InheritNullAnnotations = "org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations"; //$NON-NLS-1$ /** * Possible values for configurable options */ @@ -430,6 +432,8 @@ public class CompilerOptions { String tolerateIllegalAmbiguousVarargs = System.getProperty("tolerateIllegalAmbiguousVarargsInvocation"); //$NON-NLS-1$ tolerateIllegalAmbiguousVarargsInvocation = tolerateIllegalAmbiguousVarargs != null && tolerateIllegalAmbiguousVarargs.equalsIgnoreCase("true"); //$NON-NLS-1$ } + /** Should null annotations of overridden methods be inherited? */ + public boolean inheritNullAnnotations; // keep in sync with warningTokenToIrritant and warningTokenFromIrritant public final static String[] warningTokens = { @@ -826,6 +830,7 @@ public class CompilerOptions { OPTION_ReportNullUncheckedConversion, OPTION_ReportRedundantNullAnnotation, OPTION_ReportUnusedTypeParameter, + OPTION_InheritNullAnnotations }; return result; } @@ -1123,6 +1128,7 @@ public class CompilerOptions { optionsMap.put(OPTION_NonNullByDefaultAnnotationName, String.valueOf(CharOperation.concatWith(this.nonNullByDefaultAnnotationName, '.'))); optionsMap.put(OPTION_ReportMissingNonNullByDefaultAnnotation, getSeverityString(MissingNonNullByDefaultAnnotation)); optionsMap.put(OPTION_ReportUnusedTypeParameter, getSeverityString(UnusedTypeParameter)); + optionsMap.put(OPTION_InheritNullAnnotations, this.inheritNullAnnotations ? ENABLED : DISABLED); return optionsMap; } @@ -1281,6 +1287,7 @@ public class CompilerOptions { this.nonNullAnnotationName = DEFAULT_NONNULL_ANNOTATION_NAME; this.nonNullByDefaultAnnotationName = DEFAULT_NONNULLBYDEFAULT_ANNOTATION_NAME; this.intendedDefaultNonNullness = 0; + this.inheritNullAnnotations = false; this.analyseResourceLeaks = true; @@ -1611,6 +1618,9 @@ public class CompilerOptions { this.nonNullByDefaultAnnotationName = CharOperation.splitAndTrimOn('.', ((String)optionValue).toCharArray()); } if ((optionValue = optionsMap.get(OPTION_ReportMissingNonNullByDefaultAnnotation)) != null) updateSeverity(MissingNonNullByDefaultAnnotation, optionValue); + if ((optionValue = optionsMap.get(OPTION_InheritNullAnnotations)) != null) { + this.inheritNullAnnotations = ENABLED.equals(optionValue); + } } // Javadoc options diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java index 55676551db..3328269003 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java @@ -19,6 +19,7 @@ * bug 358903 - Filter practically unimportant resource leak warnings * bug 365531 - [compiler][null] investigate alternative strategy for internally encoding nullness defaults * bug 388800 - [1.8][compiler] detect default methods in class files + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -1174,13 +1175,6 @@ void scanMethodForNullAnnotation(IBinaryMethod method, MethodBinding methodBindi if (nullableAnnotationName == null || nonNullAnnotationName == null || nonNullByDefaultAnnotationName == null) return; // not well-configured to use null annotations - int currentDefault = NO_NULL_DEFAULT; - if ((this.tagBits & TagBits.AnnotationNonNullByDefault) != 0) { - currentDefault = NONNULL_BY_DEFAULT; - } else if ((this.tagBits & TagBits.AnnotationNullUnspecifiedByDefault) != 0) { - currentDefault = NULL_UNSPECIFIED_BY_DEFAULT; - } - // return: IBinaryAnnotation[] annotations = method.getAnnotations(); boolean explicitNullness = false; @@ -1192,7 +1186,6 @@ void scanMethodForNullAnnotation(IBinaryMethod method, MethodBinding methodBindi char[][] typeName = CharOperation.splitOn('/', annotationTypeName, 1, annotationTypeName.length-1); // cut of leading 'L' and trailing ';' if (CharOperation.equals(typeName, nonNullByDefaultAnnotationName)) { methodBinding.tagBits |= TagBits.AnnotationNonNullByDefault; - currentDefault = NONNULL_BY_DEFAULT; } if (!explicitNullness && CharOperation.equals(typeName, nonNullAnnotationName)) { methodBinding.tagBits |= TagBits.AnnotationNonNull; @@ -1204,19 +1197,13 @@ void scanMethodForNullAnnotation(IBinaryMethod method, MethodBinding methodBindi } } } - if (!explicitNullness - && (methodBinding.returnType != null && !methodBinding.returnType.isBaseType()) - && currentDefault == NONNULL_BY_DEFAULT) { - methodBinding.tagBits |= TagBits.AnnotationNonNull; - } // parameters: TypeBinding[] parameters = methodBinding.parameters; int numVisibleParams = parameters.length; int numParamAnnotations = method.getAnnotatedParametersCount(); - if (numParamAnnotations > 0 || currentDefault == NONNULL_BY_DEFAULT) { + if (numParamAnnotations > 0) { for (int j = 0; j < numVisibleParams; j++) { - explicitNullness = false; if (numParamAnnotations > 0) { int startIndex = numParamAnnotations - numVisibleParams; IBinaryAnnotation[] paramAnnotations = method.getParameterAnnotations(j+startIndex); @@ -1230,25 +1217,16 @@ void scanMethodForNullAnnotation(IBinaryMethod method, MethodBinding methodBindi if (methodBinding.parameterNonNullness == null) methodBinding.parameterNonNullness = new Boolean[numVisibleParams]; methodBinding.parameterNonNullness[j] = Boolean.TRUE; - explicitNullness = true; break; } else if (CharOperation.equals(typeName, nullableAnnotationName)) { if (methodBinding.parameterNonNullness == null) methodBinding.parameterNonNullness = new Boolean[numVisibleParams]; methodBinding.parameterNonNullness[j] = Boolean.FALSE; - explicitNullness = true; break; } } } } - if (!explicitNullness && currentDefault == NONNULL_BY_DEFAULT) { - if (methodBinding.parameterNonNullness == null) - methodBinding.parameterNonNullness = new Boolean[numVisibleParams]; - if (methodBinding.parameters[j]!= null && !methodBinding.parameters[j].isBaseType()) { - methodBinding.parameterNonNullness[j] = Boolean.TRUE; - } - } } } } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ImplicitNullAnnotationVerifier.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ImplicitNullAnnotationVerifier.java new file mode 100644 index 0000000000..6670cffc04 --- /dev/null +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ImplicitNullAnnotationVerifier.java @@ -0,0 +1,434 @@ +/******************************************************************************* + * Copyright (c) 2012 GK Software AG 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 + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Stephan Herrmann - initial API and implementation + *******************************************************************************/ +package org.eclipse.jdt.internal.compiler.lookup; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.eclipse.jdt.internal.compiler.ast.ASTNode; +import org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration; +import org.eclipse.jdt.internal.compiler.ast.Argument; +import org.eclipse.jdt.internal.compiler.ast.MethodDeclaration; +import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; + +/** + * Extracted slice from MethodVerifier15, which is responsible only for implicit null annotations. + * First, if enabled, it detects overridden methods from which null annotations are inherited. + * Next, also default nullness is filled into remaining empty slots. + * After all implicit annotations have been filled in compatibility is checked and problems are complained. + */ +public class ImplicitNullAnnotationVerifier { + + /** + * Simple record to store nullness info for one argument or return type + * while iterating over a set of overridden methods. + */ + static class InheritedNonNullnessInfo { + Boolean inheritedNonNullness; + MethodBinding annotationOrigin; + } + + // delegate which to ask for recursive analysis of super methods + // can be 'this', but is never a MethodVerifier (to avoid infinite recursion). + ImplicitNullAnnotationVerifier buddyImplicitNullAnnotationsVerifier; + private boolean inheritNullAnnotations; + + public ImplicitNullAnnotationVerifier(boolean inheritNullAnnotations) { + this.buddyImplicitNullAnnotationsVerifier = this; + this.inheritNullAnnotations = inheritNullAnnotations; + } + + // for sub-classes: + ImplicitNullAnnotationVerifier(CompilerOptions options) { + this.buddyImplicitNullAnnotationsVerifier = new ImplicitNullAnnotationVerifier(options.inheritNullAnnotations); + this.inheritNullAnnotations = options.inheritNullAnnotations; + } + + /** + * Check and fill in implicit annotations from overridden methods and from default. + * Precondition: caller has checked whether annotation-based null analysis is enabled. + */ + public void checkImplicitNullAnnotations(MethodBinding currentMethod, AbstractMethodDeclaration srcMethod, boolean complain, Scope scope) { + // check inherited nullness from superclass and superInterfaces + try { + ReferenceBinding currentType = currentMethod.declaringClass; + if (currentType.id == TypeIds.T_JavaLangObject) { + return; + } + boolean needToApplyNonNullDefault = currentMethod.hasNonNullDefault(); + // compatibility & inheritance do not consider constructors / static methods: + boolean isInstanceMethod = !currentMethod.isConstructor() && !currentMethod.isStatic(); + complain &= isInstanceMethod; + if (!needToApplyNonNullDefault + && !complain + && !(this.inheritNullAnnotations && isInstanceMethod)) { + return; // short cut, no work to be done + } + + if (isInstanceMethod) { + List superMethodList = new ArrayList(); + + int paramLen = currentMethod.parameters.length; + findAllOverriddenMethods(currentMethod.original(), currentMethod.selector, paramLen, + currentType, new HashSet(), superMethodList); + + // prepare interim storage for nullness info so we don't pollute currentMethod before we know its conflict-free: + InheritedNonNullnessInfo[] inheritedNonNullnessInfos = new InheritedNonNullnessInfo[paramLen+1]; // index 0 is for the return type + for (int i=0; i<paramLen+1; i++) inheritedNonNullnessInfos[i] = new InheritedNonNullnessInfo(); + + int length = superMethodList.size(); + for (int i = length; --i >= 0;) { + MethodBinding currentSuper = (MethodBinding) superMethodList.get(i); + if ((currentSuper.tagBits & TagBits.IsNullnessKnown) == 0) { + // recurse to prepare currentSuper + checkImplicitNullAnnotations(currentSuper, null, false, scope); // TODO (stephan) complain=true if currentSuper is source method?? + } + checkNullSpecInheritance(currentMethod, srcMethod, needToApplyNonNullDefault, complain, currentSuper, scope, inheritedNonNullnessInfos); + needToApplyNonNullDefault = false; + } + + // transfer collected information into currentMethod: + InheritedNonNullnessInfo info = inheritedNonNullnessInfos[0]; + if (info.inheritedNonNullness == Boolean.TRUE) { + currentMethod.tagBits |= TagBits.AnnotationNonNull; + } else if (info.inheritedNonNullness == Boolean.FALSE) { + currentMethod.tagBits |= TagBits.AnnotationNullable; + } + for (int i=0; i<paramLen; i++) { + info = inheritedNonNullnessInfos[i+1]; + if (info.inheritedNonNullness != null) { + if (currentMethod.parameterNonNullness == null) + currentMethod.parameterNonNullness = new Boolean[paramLen]; + currentMethod.parameterNonNullness[i] = info.inheritedNonNullness; + } + } + + } + if (needToApplyNonNullDefault) { + currentMethod.fillInDefaultNonNullness(srcMethod); + } + } finally { + currentMethod.tagBits |= TagBits.IsNullnessKnown; + } + } + + /* + * Recursively traverse the tree of ancestors but whenever we find a matching method prune the super tree. + * Collect all matching methods in 'result'. + */ + private void findAllOverriddenMethods(MethodBinding original, char[] selector, int suggestedParameterLength, + ReferenceBinding currentType, Set ifcsSeen, List result) + { + if (currentType.id == TypeIds.T_JavaLangObject) + return; + + // superclass: + collectOverriddenMethods(original, selector, suggestedParameterLength, currentType.superclass(), ifcsSeen, result); + + // superInterfaces: + ReferenceBinding[] superInterfaces = currentType.superInterfaces(); + int ifcLen = superInterfaces.length; + for (int i = 0; i < ifcLen; i++) { + ReferenceBinding currentIfc = superInterfaces[i]; + if (ifcsSeen.add(currentIfc.original())) { // process each interface at most once + collectOverriddenMethods(original, selector, suggestedParameterLength, currentIfc, ifcsSeen, result); + } + } + } + + /* collect matching methods from one supertype. */ + private void collectOverriddenMethods(MethodBinding original, char[] selector, int suggestedParameterLength, + ReferenceBinding superType, Set ifcsSeen, List result) + { + MethodBinding [] ifcMethods = superType.getMethods(selector, suggestedParameterLength); + int length = ifcMethods.length; + for (int i=0; i<length; i++) { + MethodBinding currentMethod = ifcMethods[i]; + if (currentMethod.isStatic()) + continue; + if (areParametersEqual(original, currentMethod.original())) { + result.add(currentMethod); + return; // at most one method is overridden from any supertype + } + } + findAllOverriddenMethods(original, selector, suggestedParameterLength, superType, ifcsSeen, result); + } + + /** + * The main algorithm in this class. + * @param currentMethod focus method + * @param srcMethod AST of 'currentMethod' if present + * @param hasNonNullDefault is a @NonNull default applicable at the site of currentMethod? + * @param shouldComplain should we report any errors found? + * (see also comment about flows into this method, below). + * @param inheritedMethod one overridden method from a super type + * @param scope provides context for error reporting etc. + * @param inheritedNonNullnessInfos if non-null, this array of non-null elements is used for + * interim recording of nullness information from inheritedMethod rather than prematurely updating currentMethod. + * Index position 0 is used for the return type, positions i+1 for argument i. + */ + void checkNullSpecInheritance(MethodBinding currentMethod, AbstractMethodDeclaration srcMethod, + boolean hasNonNullDefault, boolean shouldComplain, + MethodBinding inheritedMethod, Scope scope, InheritedNonNullnessInfo[] inheritedNonNullnessInfos) + { + // Note that basically two different flows lead into this method: + // (1) during MethodVerifyer15.checkMethods() we want to report errors (against srcMethod or against the current type) + // In this case this method is directly called from MethodVerifier15 (checkAgainstInheritedMethod / checkConcreteInheritedMethod) + // (2) during on-demand invocation we are mainly interested in the side effects of copying inherited null annotations + // In this case this method is called via checkImplicitNullAnnotations from + // - MessageSend.resolveType(..) + // - SourceTypeBinding.createArgumentBindings(..) + // - recursive calls within this class + // Still we *might* want to complain about problems found (controlled by 'complain') + + if ((inheritedMethod.tagBits & TagBits.IsNullnessKnown) == 0) { + // TODO (stephan): even here we may need to report problems? How to discriminate? + this.buddyImplicitNullAnnotationsVerifier.checkImplicitNullAnnotations(inheritedMethod, null, false, scope); + } + long inheritedBits = inheritedMethod.tagBits; + long inheritedNullnessBits = inheritedBits & (TagBits.AnnotationNonNull|TagBits.AnnotationNullable); + long currentBits = currentMethod.tagBits; + long currentNullnessBits = currentBits & (TagBits.AnnotationNonNull|TagBits.AnnotationNullable); + + LookupEnvironment environment = scope.environment(); + boolean shouldInherit = this.inheritNullAnnotations; + + // return type: + returnType: { + if (currentMethod.returnType == null || currentMethod.returnType.isBaseType()) + break returnType; // no nullness for primitive types + if (currentNullnessBits == 0) { + // unspecified, may fill in either from super or from default + if (shouldInherit) { + if (inheritedNullnessBits != 0) { + if (hasNonNullDefault) { + // both inheritance and default: check for conflict? + if (shouldComplain && inheritedNullnessBits == TagBits.AnnotationNullable) + scope.problemReporter().conflictingNullAnnotations(currentMethod, ((MethodDeclaration) srcMethod).returnType, inheritedMethod); + // still use the inherited bits to avoid incompatibility + } + if (inheritedNonNullnessInfos != null && srcMethod != null) { + recordDeferredInheritedNullness(scope, ((MethodDeclaration) srcMethod).returnType, + inheritedMethod, Boolean.valueOf(inheritedNullnessBits == TagBits.AnnotationNonNull), inheritedNonNullnessInfos[0]); + } else { + // no need to defer, record this info now: + currentMethod.tagBits |= inheritedNullnessBits; + } + break returnType; // compatible by construction, skip complain phase below + } + } + if (hasNonNullDefault) { // conflict with inheritance already checked + currentMethod.tagBits |= (currentNullnessBits = TagBits.AnnotationNonNull); + } + } + if (shouldComplain) { + if ((inheritedNullnessBits & TagBits.AnnotationNonNull) != 0 + && currentNullnessBits != TagBits.AnnotationNonNull) + { + if (srcMethod != null) { + scope.problemReporter().illegalReturnRedefinition(srcMethod, inheritedMethod, + environment.getNonNullAnnotationName()); + } else { + scope.problemReporter().cannotImplementIncompatibleNullness(currentMethod, inheritedMethod); + return; + } + } + } + } + + // parameters: + Argument[] currentArguments = srcMethod == null ? null : srcMethod.arguments; + + int length = 0; + if (currentArguments != null) + length = currentArguments.length; + else if (inheritedMethod.parameterNonNullness != null) + length = inheritedMethod.parameterNonNullness.length; + else if (currentMethod.parameterNonNullness != null) + length = currentMethod.parameterNonNullness.length; + + for (int i = 0; i < length; i++) { + if (currentMethod.parameters[i].isBaseType()) continue; + + Argument currentArgument = currentArguments == null + ? null : currentArguments[i]; + Boolean inheritedNonNullNess = (inheritedMethod.parameterNonNullness == null) + ? null : inheritedMethod.parameterNonNullness[i]; + Boolean currentNonNullNess = (currentMethod.parameterNonNullness == null) + ? null : currentMethod.parameterNonNullness[i]; + + if (currentNonNullNess == null) { + // unspecified, may fill in either from super or from default + if (inheritedNonNullNess != null) { + if (shouldInherit) { + if (hasNonNullDefault) { + // both inheritance and default: check for conflict? + if (shouldComplain + && inheritedNonNullNess == Boolean.FALSE + && currentArgument != null) + { + scope.problemReporter().conflictingNullAnnotations(currentMethod, currentArgument, inheritedMethod); + } + // still use the inherited info to avoid incompatibility + } + if (inheritedNonNullnessInfos != null && srcMethod != null) { + recordDeferredInheritedNullness(scope, srcMethod.arguments[i].type, + inheritedMethod, inheritedNonNullNess, inheritedNonNullnessInfos[i+1]); + } else { + // no need to defer, record this info now: + if (currentMethod.parameterNonNullness == null) + currentMethod.parameterNonNullness = new Boolean[length]; + currentMethod.parameterNonNullness[i] = inheritedNonNullNess; + } + continue; // compatible by construction, skip complain phase below + } + } + if (hasNonNullDefault) { // conflict with inheritance already checked + if (currentMethod.parameterNonNullness == null) + currentMethod.parameterNonNullness = new Boolean[length]; + currentMethod.parameterNonNullness[i] = (currentNonNullNess = Boolean.TRUE); + } + } + if (shouldComplain) { + boolean needNonNull = false; + char[][] annotationName; + if (inheritedNonNullNess == Boolean.TRUE) { + needNonNull = true; + annotationName = environment.getNonNullAnnotationName(); + } else { + annotationName = environment.getNullableAnnotationName(); + } + if (inheritedNonNullNess != Boolean.TRUE // super parameter is not restricted to @NonNull + && currentNonNullNess == Boolean.TRUE) // current parameter is restricted to @NonNull + { + // incompatible + if (currentArgument != null) { + scope.problemReporter().illegalRedefinitionToNonNullParameter( + currentArgument, + inheritedMethod.declaringClass, + (inheritedNonNullNess == null) ? null : environment.getNullableAnnotationName()); + } else { + scope.problemReporter().cannotImplementIncompatibleNullness(currentMethod, inheritedMethod); + } + } else if (inheritedNonNullNess != null + && currentNonNullNess == null) + { + // weak conflict (TODO reconsider this case) + if (currentArgument != null) { + scope.problemReporter().parameterLackingNullAnnotation( + currentArgument, + inheritedMethod.declaringClass, + needNonNull, + annotationName); + } else { + scope.problemReporter().cannotImplementIncompatibleNullness(currentMethod, inheritedMethod); + } + } + } + } + } + /* check for conflicting annotations and record here the info 'inheritedNonNullness' found in 'inheritedMethod'. */ + protected void recordDeferredInheritedNullness(Scope scope, ASTNode location, + MethodBinding inheritedMethod, Boolean inheritedNonNullness, + InheritedNonNullnessInfo nullnessInfo) + { + if (nullnessInfo.inheritedNonNullness != null && nullnessInfo.inheritedNonNullness != inheritedNonNullness) { + scope.problemReporter().conflictingInheritedNullAnnotations(location, + nullnessInfo.inheritedNonNullness.booleanValue(), nullnessInfo.annotationOrigin, + inheritedNonNullness.booleanValue(), inheritedMethod); + // leave previous info intact, so subsequent errors are reported against the same first method + } else { + nullnessInfo.inheritedNonNullness = inheritedNonNullness; + nullnessInfo.annotationOrigin = inheritedMethod; + } + } + + // ==== minimal set of utility methods previously from MethodVerifier15: ==== + + boolean areParametersEqual(MethodBinding one, MethodBinding two) { + TypeBinding[] oneArgs = one.parameters; + TypeBinding[] twoArgs = two.parameters; + if (oneArgs == twoArgs) return true; + + int length = oneArgs.length; + if (length != twoArgs.length) return false; + + + // methods with raw parameters are considered equal to inherited methods + // with parameterized parameters for backwards compatibility, need a more complex check + int i; + foundRAW: for (i = 0; i < length; i++) { + if (!areTypesEqual(oneArgs[i], twoArgs[i])) { + if (oneArgs[i].leafComponentType().isRawType()) { + if (oneArgs[i].dimensions() == twoArgs[i].dimensions() && oneArgs[i].leafComponentType().isEquivalentTo(twoArgs[i].leafComponentType())) { + // raw mode does not apply if the method defines its own type variables + if (one.typeVariables != Binding.NO_TYPE_VARIABLES) + return false; + // one parameter type is raw, hence all parameters types must be raw or non generic + // otherwise we have a mismatch check backwards + for (int j = 0; j < i; j++) + if (oneArgs[j].leafComponentType().isParameterizedTypeWithActualArguments()) + return false; + // switch to all raw mode + break foundRAW; + } + } + return false; + } + } + // all raw mode for remaining parameters (if any) + for (i++; i < length; i++) { + if (!areTypesEqual(oneArgs[i], twoArgs[i])) { + if (oneArgs[i].leafComponentType().isRawType()) + if (oneArgs[i].dimensions() == twoArgs[i].dimensions() && oneArgs[i].leafComponentType().isEquivalentTo(twoArgs[i].leafComponentType())) + continue; + return false; + } else if (oneArgs[i].leafComponentType().isParameterizedTypeWithActualArguments()) { + return false; // no remaining parameter can be a Parameterized type (if one has been converted then all RAW types must be converted) + } + } + return true; + } + boolean areTypesEqual(TypeBinding one, TypeBinding two) { + if (one == two) return true; + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=329584 + switch(one.kind()) { + case Binding.TYPE: + switch (two.kind()) { + case Binding.PARAMETERIZED_TYPE: + case Binding.RAW_TYPE: + if (one == two.erasure()) + return true; + } + break; + case Binding.RAW_TYPE: + case Binding.PARAMETERIZED_TYPE: + switch(two.kind()) { + case Binding.TYPE: + if (one.erasure() == two) + return true; + } + } + + // need to consider X<?> and X<? extends Object> as the same 'type' + if (one.isParameterizedType() && two.isParameterizedType()) + return one.isEquivalentTo(two) && two.isEquivalentTo(one); + + // Can skip this since we resolved each method before comparing it, see computeSubstituteMethod() + // if (one instanceof UnresolvedReferenceBinding) + // return ((UnresolvedReferenceBinding) one).resolvedType == two; + // if (two instanceof UnresolvedReferenceBinding) + // return ((UnresolvedReferenceBinding) two).resolvedType == one; + return false; // all other type bindings are identical + } +} diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java index b600a76fc2..352e279aa8 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java @@ -13,6 +13,7 @@ * bug 365519 - editorial cleanup after bug 186342 and bug 365387 * bug 365662 - [compiler][null] warn on contradictory and redundant null annotations * bug 365531 - [compiler][null] investigate alternative strategy for internally encoding nullness defaults + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -452,10 +453,9 @@ public final char[] constantPoolName() { /** * After method verifier has finished, fill in missing @NonNull specification from the applicable default. */ -protected void fillInDefaultNonNullness() { +protected void fillInDefaultNonNullness(AbstractMethodDeclaration sourceMethod) { if (this.parameterNonNullness == null) this.parameterNonNullness = new Boolean[this.parameters.length]; - AbstractMethodDeclaration sourceMethod = sourceMethod(); boolean added = false; int length = this.parameterNonNullness.length; for (int i = 0; i < length; i++) { @@ -467,7 +467,7 @@ protected void fillInDefaultNonNullness() { if (sourceMethod != null) { sourceMethod.arguments[i].binding.tagBits |= TagBits.AnnotationNonNull; } - } else if (this.parameterNonNullness[i].booleanValue()) { + } else if (sourceMethod != null && this.parameterNonNullness[i].booleanValue()) { sourceMethod.scope.problemReporter().nullAnnotationIsRedundant(sourceMethod, i); } } @@ -478,7 +478,7 @@ protected void fillInDefaultNonNullness() { && (this.tagBits & (TagBits.AnnotationNonNull|TagBits.AnnotationNullable)) == 0) { this.tagBits |= TagBits.AnnotationNonNull; - } else if ((this.tagBits & TagBits.AnnotationNonNull) != 0) { + } else if (sourceMethod != null && (this.tagBits & TagBits.AnnotationNonNull) != 0) { sourceMethod.scope.problemReporter().nullAnnotationIsRedundant(sourceMethod, -1/*signifies method return*/); } } @@ -1176,4 +1176,11 @@ public String toString() { public TypeVariableBinding[] typeVariables() { return this.typeVariables; } +public boolean hasNonNullDefault() { + if ((this.tagBits & TagBits.AnnotationNonNullByDefault) != 0) + return true; + if ((this.tagBits & TagBits.AnnotationNullUnspecifiedByDefault) != 0) + return false; + return this.declaringClass.hasNonNullDefault(); +} } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier.java index 9df2c7154b..a89ab87453 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier.java @@ -15,6 +15,7 @@ * Stephan Herrmann - Contribution for * bug 382347 - [1.8][compiler] Compiler accepts incorrect default method inheritance * bug 388954 - [1.8][compiler] detect default methods in class files + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -25,7 +26,7 @@ import org.eclipse.jdt.internal.compiler.problem.ProblemReporter; import org.eclipse.jdt.internal.compiler.util.HashtableOfObject; import org.eclipse.jdt.internal.compiler.util.SimpleSet; -public class MethodVerifier { +public class MethodVerifier extends ImplicitNullAnnotationVerifier { SourceTypeBinding type; HashtableOfObject inheritedMethods; HashtableOfObject currentMethods; @@ -49,6 +50,7 @@ Binding creation is responsible for reporting all problems with types: - defining an interface as a local type (local types can only be classes) */ MethodVerifier(LookupEnvironment environment) { + super(environment.globalOptions); this.type = null; // Initialized with the public method verify(SourceTypeBinding) this.inheritedMethods = null; this.currentMethods = null; @@ -60,18 +62,6 @@ MethodVerifier(LookupEnvironment environment) { boolean areMethodsCompatible(MethodBinding one, MethodBinding two) { return isParameterSubsignature(one, two) && areReturnTypesCompatible(one, two); } -boolean areParametersEqual(MethodBinding one, MethodBinding two) { - TypeBinding[] oneArgs = one.parameters; - TypeBinding[] twoArgs = two.parameters; - if (oneArgs == twoArgs) return true; - - int length = oneArgs.length; - if (length != twoArgs.length) return false; - - for (int i = 0; i < length; i++) - if (!areTypesEqual(oneArgs[i], twoArgs[i])) return false; - return true; -} boolean areReturnTypesCompatible(MethodBinding one, MethodBinding two) { if (one.returnType == two.returnType) return true; @@ -94,19 +84,6 @@ boolean areReturnTypesCompatible0(MethodBinding one, MethodBinding two) { return one.returnType.isCompatibleWith(two.returnType); } -boolean areTypesEqual(TypeBinding one, TypeBinding two) { - if (one == two) return true; - - // its possible that an UnresolvedReferenceBinding can be compared to its resolved type - // when they're both UnresolvedReferenceBindings then they must be identical like all other types - // all wrappers of UnresolvedReferenceBindings are converted as soon as the type is resolved - // so its not possible to have 2 arrays where one is UnresolvedX[] and the other is X[] - if (one instanceof UnresolvedReferenceBinding) - return ((UnresolvedReferenceBinding) one).resolvedType == two; - if (two instanceof UnresolvedReferenceBinding) - return ((UnresolvedReferenceBinding) two).resolvedType == one; - return false; // all other type bindings are identical -} boolean canSkipInheritedMethods() { if (this.type.superclass() != null && this.type.superclass().isAbstract()) return false; diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java index 9c030ec2c9..fee31e960f 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java @@ -10,6 +10,7 @@ * Stephan Herrmann - Contributions for * bug 186342 - [compiler][null] Using annotations for null checking * bug 365519 - editorial cleanup after bug 186342 and bug 365387 + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -41,50 +42,6 @@ boolean areMethodsCompatible(MethodBinding one, MethodBinding two) { return isParameterSubsignature(one, two); } -boolean areParametersEqual(MethodBinding one, MethodBinding two) { - TypeBinding[] oneArgs = one.parameters; - TypeBinding[] twoArgs = two.parameters; - if (oneArgs == twoArgs) return true; - - int length = oneArgs.length; - if (length != twoArgs.length) return false; - - - // methods with raw parameters are considered equal to inherited methods - // with parameterized parameters for backwards compatibility, need a more complex check - int i; - foundRAW: for (i = 0; i < length; i++) { - if (!areTypesEqual(oneArgs[i], twoArgs[i])) { - if (oneArgs[i].leafComponentType().isRawType()) { - if (oneArgs[i].dimensions() == twoArgs[i].dimensions() && oneArgs[i].leafComponentType().isEquivalentTo(twoArgs[i].leafComponentType())) { - // raw mode does not apply if the method defines its own type variables - if (one.typeVariables != Binding.NO_TYPE_VARIABLES) - return false; - // one parameter type is raw, hence all parameters types must be raw or non generic - // otherwise we have a mismatch check backwards - for (int j = 0; j < i; j++) - if (oneArgs[j].leafComponentType().isParameterizedTypeWithActualArguments()) - return false; - // switch to all raw mode - break foundRAW; - } - } - return false; - } - } - // all raw mode for remaining parameters (if any) - for (i++; i < length; i++) { - if (!areTypesEqual(oneArgs[i], twoArgs[i])) { - if (oneArgs[i].leafComponentType().isRawType()) - if (oneArgs[i].dimensions() == twoArgs[i].dimensions() && oneArgs[i].leafComponentType().isEquivalentTo(twoArgs[i].leafComponentType())) - continue; - return false; - } else if (oneArgs[i].leafComponentType().isParameterizedTypeWithActualArguments()) { - return false; // no remaining parameter can be a Parameterized type (if one has been converted then all RAW types must be converted) - } - } - return true; -} boolean areReturnTypesCompatible(MethodBinding one, MethodBinding two) { if (one.returnType == two.returnType) return true; if (this.type.scope.compilerOptions().sourceLevel >= ClassFileConstants.JDK1_5) { @@ -93,38 +50,6 @@ boolean areReturnTypesCompatible(MethodBinding one, MethodBinding two) { return areTypesEqual(one.returnType.erasure(), two.returnType.erasure()); } } -boolean areTypesEqual(TypeBinding one, TypeBinding two) { - if (one == two) return true; - // https://bugs.eclipse.org/bugs/show_bug.cgi?id=329584 - switch(one.kind()) { - case Binding.TYPE: - switch (two.kind()) { - case Binding.PARAMETERIZED_TYPE: - case Binding.RAW_TYPE: - if (one == two.erasure()) - return true; - } - break; - case Binding.RAW_TYPE: - case Binding.PARAMETERIZED_TYPE: - switch(two.kind()) { - case Binding.TYPE: - if (one.erasure() == two) - return true; - } - } - - // need to consider X<?> and X<? extends Object> as the same 'type' - if (one.isParameterizedType() && two.isParameterizedType()) - return one.isEquivalentTo(two) && two.isEquivalentTo(one); - - // Can skip this since we resolved each method before comparing it, see computeSubstituteMethod() - // if (one instanceof UnresolvedReferenceBinding) - // return ((UnresolvedReferenceBinding) one).resolvedType == two; - // if (two instanceof UnresolvedReferenceBinding) - // return ((UnresolvedReferenceBinding) two).resolvedType == one; - return false; // all other type bindings are identical -} // Given `overridingMethod' which overrides `inheritedMethod' answer whether some subclass method that // differs in erasure from overridingMethod could override `inheritedMethod' protected boolean canOverridingMethodDifferInErasure(MethodBinding overridingMethod, MethodBinding inheritedMethod) { @@ -147,6 +72,11 @@ boolean canSkipInheritedMethods(MethodBinding one, MethodBinding two) { void checkConcreteInheritedMethod(MethodBinding concreteMethod, MethodBinding[] abstractMethods) { super.checkConcreteInheritedMethod(concreteMethod, abstractMethods); boolean analyseNullAnnotations = this.environment.globalOptions.isAnnotationBasedNullAnalysisEnabled; + // TODO (stephan): unclear if this srcMethod is actually needed + AbstractMethodDeclaration srcMethod = null; + if (analyseNullAnnotations && this.type.equals(concreteMethod.declaringClass)) // is currentMethod from the current type? + srcMethod = concreteMethod.sourceMethod(); + boolean hasNonNullDefault = concreteMethod.hasNonNullDefault(); for (int i = 0, l = abstractMethods.length; i < l; i++) { MethodBinding abstractMethod = abstractMethods[i]; if (concreteMethod.isVarargs() != abstractMethod.isVarargs()) @@ -167,8 +97,9 @@ void checkConcreteInheritedMethod(MethodBinding concreteMethod, MethodBinding[] || this.type.superclass.erasure().findSuperTypeOriginatingFrom(originalInherited.declaringClass) == null) this.type.addSyntheticBridgeMethod(originalInherited, concreteMethod.original()); } - if (analyseNullAnnotations && !concreteMethod.isStatic() && !abstractMethod.isStatic()) - checkNullSpecInheritance(concreteMethod, abstractMethod); + if (analyseNullAnnotations && !concreteMethod.isStatic() && !abstractMethod.isStatic()) { + checkNullSpecInheritance(concreteMethod, srcMethod, hasNonNullDefault, true, abstractMethod, this.type.scope, null); + } } } void checkForBridgeMethod(MethodBinding currentMethod, MethodBinding inheritedMethod, MethodBinding[] allInheritedMethods) { @@ -369,100 +300,37 @@ boolean checkInheritedReturnTypes(MethodBinding method, MethodBinding otherMetho void checkAgainstInheritedMethods(MethodBinding currentMethod, MethodBinding[] methods, int length, MethodBinding[] allInheritedMethods) { super.checkAgainstInheritedMethods(currentMethod, methods, length, allInheritedMethods); - if (this.environment.globalOptions.isAnnotationBasedNullAnalysisEnabled) { + CompilerOptions options = this.environment.globalOptions; + if (options.isAnnotationBasedNullAnalysisEnabled + && (currentMethod.tagBits & TagBits.IsNullnessKnown) == 0) + { + // if annotations are inherited these have been checked during STB.resolveTypesFor() (for methods explicit in this.type) + AbstractMethodDeclaration srcMethod = null; + if (this.type.equals(currentMethod.declaringClass)) // is currentMethod from the current type? + srcMethod = currentMethod.sourceMethod(); + boolean hasNonNullDefault = currentMethod.hasNonNullDefault(); for (int i = length; --i >= 0;) if (!currentMethod.isStatic() && !methods[i].isStatic()) - checkNullSpecInheritance(currentMethod, methods[i]); + checkNullSpecInheritance(currentMethod, srcMethod, hasNonNullDefault, true, methods[i], this.type.scope, null); } } -void checkNullSpecInheritance(MethodBinding currentMethod, MethodBinding inheritedMethod) { - // precondition: caller has checked whether annotation-based null analysis is enabled. - long inheritedBits = inheritedMethod.tagBits; - long currentBits = currentMethod.tagBits; - AbstractMethodDeclaration srcMethod = null; - if (this.type.equals(currentMethod.declaringClass)) // is currentMethod from the current type? - srcMethod = currentMethod.sourceMethod(); - - // return type: - if ((inheritedBits & TagBits.AnnotationNonNull) != 0) { - long currentNullBits = currentBits & (TagBits.AnnotationNonNull|TagBits.AnnotationNullable); - if (currentNullBits != TagBits.AnnotationNonNull) { - if (srcMethod != null) { - this.type.scope.problemReporter().illegalReturnRedefinition(srcMethod, inheritedMethod, - this.environment.getNonNullAnnotationName()); - } else { - this.type.scope.problemReporter().cannotImplementIncompatibleNullness(currentMethod, inheritedMethod); - return; - } - } +void checkNullSpecInheritance(MethodBinding currentMethod, AbstractMethodDeclaration srcMethod, + boolean hasNonNullDefault, boolean complain, MethodBinding inheritedMethod, Scope scope, InheritedNonNullnessInfo[] inheritedNonNullnessInfos) +{ + complain &= !currentMethod.isConstructor(); + if (!hasNonNullDefault && !complain && !this.environment.globalOptions.inheritNullAnnotations) { + // nothing to be done, take the shortcut + currentMethod.tagBits |= TagBits.IsNullnessKnown; + return; } - - // parameters: - Argument[] currentArguments = srcMethod == null ? null : srcMethod.arguments; - if (inheritedMethod.parameterNonNullness != null) { - // inherited method has null-annotations, check compatibility: - - int length = inheritedMethod.parameterNonNullness.length; - for (int i = 0; i < length; i++) { - Argument currentArgument = currentArguments == null ? null : currentArguments[i]; - - Boolean inheritedNonNullNess = inheritedMethod.parameterNonNullness[i]; - Boolean currentNonNullNess = (currentMethod.parameterNonNullness == null) - ? null : currentMethod.parameterNonNullness[i]; - if (inheritedNonNullNess != null) { // super has a null annotation - if (currentNonNullNess == null) { // current parameter lacks null annotation - boolean needNonNull = false; - char[][] annotationName; - if (inheritedNonNullNess == Boolean.TRUE) { - needNonNull = true; - annotationName = this.environment.getNonNullAnnotationName(); - } else { - annotationName = this.environment.getNullableAnnotationName(); - } - if (currentArgument != null) { - this.type.scope.problemReporter().parameterLackingNullAnnotation( - currentArgument, - inheritedMethod.declaringClass, - needNonNull, - annotationName); - continue; - } else { - this.type.scope.problemReporter().cannotImplementIncompatibleNullness(currentMethod, inheritedMethod); - break; - } - } - } - if (inheritedNonNullNess != Boolean.TRUE) { // super parameter is not restricted to @NonNull - if (currentNonNullNess == Boolean.TRUE) { // current parameter is restricted to @NonNull - if (currentArgument != null) - this.type.scope.problemReporter().illegalRedefinitionToNonNullParameter( - currentArgument, - inheritedMethod.declaringClass, - inheritedNonNullNess == null - ? null - : this.environment.getNullableAnnotationName()); - else - this.type.scope.problemReporter().cannotImplementIncompatibleNullness(currentMethod, inheritedMethod); - } - } - } - } else if (currentMethod.parameterNonNullness != null) { - // super method has no annotations but current has - for (int i = 0; i < currentMethod.parameterNonNullness.length; i++) { - if (currentMethod.parameterNonNullness[i] == Boolean.TRUE) { // tightening from unconstrained to @NonNull - if (currentArguments != null) { - this.type.scope.problemReporter().illegalRedefinitionToNonNullParameter( - currentArguments[i], - inheritedMethod.declaringClass, - null); - } else { - this.type.scope.problemReporter().cannotImplementIncompatibleNullness(currentMethod, inheritedMethod); - break; - } - } - } + // in this context currentMethod can be inherited, too. Recurse if needed. + if (currentMethod.declaringClass != this.type + && (currentMethod.tagBits & TagBits.IsNullnessKnown) == 0) + { + this.buddyImplicitNullAnnotationsVerifier.checkImplicitNullAnnotations(currentMethod, srcMethod, complain, this.type.scope); } + super.checkNullSpecInheritance(currentMethod, srcMethod, hasNonNullDefault, complain, inheritedMethod, scope, inheritedNonNullnessInfos); } void reportRawReferences() { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java index c38d541528..c1a6073144 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java @@ -13,6 +13,7 @@ * bug 365519 - editorial cleanup after bug 186342 and bug 365387 * bug 358903 - Filter practically unimportant resource leak warnings * bug 365531 - [compiler][null] investigate alternative strategy for internally encoding nullness defaults + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -973,6 +974,23 @@ public boolean hasMemberTypes() { return false; } +/** + * Answer whether a @NonNullByDefault is applicable at the given method binding. + */ +boolean hasNonNullDefault() { + // Note, STB overrides for correctly handling local types + ReferenceBinding currentType = this; + while (currentType != null) { + if ((currentType.tagBits & TagBits.AnnotationNonNullByDefault) != 0) + return true; + if ((currentType.tagBits & TagBits.AnnotationNullUnspecifiedByDefault) != 0) + return false; + currentType = currentType.enclosingType(); + } + // package + return this.getPackage().defaultNullness == NONNULL_BY_DEFAULT; +} + public final boolean hasRestrictedAccess() { return (this.modifiers & ExtraCompilerModifiers.AccRestrictedAccess) != 0; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java index c6704772ec..c5437db9b4 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/SourceTypeBinding.java @@ -18,6 +18,7 @@ * bug 366063 - Compiler should not add synthetic @NonNull annotations * bug 384663 - Package Based Annotation Compilation Error in JDT 3.8/4.2 (works in 3.7.2) * bug 386356 - Type mismatch error with annotations and generics + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -36,6 +37,7 @@ import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration; import org.eclipse.jdt.internal.compiler.ast.TypeParameter; import org.eclipse.jdt.internal.compiler.ast.TypeReference; import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants; +import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; import org.eclipse.jdt.internal.compiler.impl.Constant; import org.eclipse.jdt.internal.compiler.problem.ProblemSeverities; import org.eclipse.jdt.internal.compiler.util.SimpleLookupTable; @@ -1114,6 +1116,18 @@ void initializeForStaticImports() { this.scope.buildMethods(); } +private void initializeNullDefault() { + // ensure nullness defaults are initialized at all enclosing levels: + switch (this.nullnessDefaultInitialized) { + case 0: + getAnnotationTagBits(); // initialize + //$FALL-THROUGH$ + case 1: + getPackage().isViewedAsDeprecated(); // initialize annotations + this.nullnessDefaultInitialized = 2; + } +} + /** * Returns true if a type is identical to another one, * or for generic types, true if compared to its raw type. @@ -1619,8 +1633,10 @@ public MethodBinding resolveTypesFor(MethodBinding method) { typeParameters[i].binding = null; return null; } - if (this.scope.compilerOptions().isAnnotationBasedNullAnalysisEnabled) - createArgumentBindings(method); // need annotations resolved already at this point + CompilerOptions compilerOptions = this.scope.compilerOptions(); + if (compilerOptions.isAnnotationBasedNullAnalysisEnabled) { + createArgumentBindings(method, compilerOptions); // need annotations resolved already at this point + } if (foundReturnTypeProblem) return method; // but its still unresolved with a null return type & is still connected to its method declaration @@ -1640,22 +1656,16 @@ private void rejectTypeAnnotatedVoidMethod(AbstractMethodDeclaration methodDecl) } } } -private void createArgumentBindings(MethodBinding method) { - // ensure nullness defaults are initialized at all enclosing levels: - switch (this.nullnessDefaultInitialized) { - case 0: - getAnnotationTagBits(); // initialize - //$FALL-THROUGH$ - case 1: - getPackage().isViewedAsDeprecated(); // initialize annotations - this.nullnessDefaultInitialized = 2; - } +private void createArgumentBindings(MethodBinding method, CompilerOptions compilerOptions) { + initializeNullDefault(); AbstractMethodDeclaration methodDecl = method.sourceMethod(); if (methodDecl != null) { + // while creating argument bindings we also collect explicit null annotations: if (method.parameters != Binding.NO_PARAMETERS) methodDecl.createArgumentBindings(); - if ((findNonNullDefault(methodDecl.scope, methodDecl.scope.environment()) == NONNULL_BY_DEFAULT)) { - method.fillInDefaultNonNullness(); + // add implicit annotations (inherited(?) & default): + if (compilerOptions.isAnnotationBasedNullAnalysisEnabled) { + new ImplicitNullAnnotationVerifier(compilerOptions.inheritNullAnnotations).checkImplicitNullAnnotations(method, methodDecl, true, this.scope); } } } @@ -1727,16 +1737,11 @@ protected boolean checkRedundantNullnessDefaultOne(ASTNode location, Annotation[ return true; } -/** - * Answer the nullness default applicable at the given method binding. - * Possible values: {@link Binding#NO_NULL_DEFAULT}, {@link Binding#NULL_UNSPECIFIED_BY_DEFAULT}, {@link Binding#NONNULL_BY_DEFAULT}. - * @param currentScope where to start search for lexically enclosing default - * @param environment gateway to options - */ -private int findNonNullDefault(Scope currentScope, LookupEnvironment environment) { +boolean hasNonNullDefault() { // find the applicable default inside->out: SourceTypeBinding currentType = null; + Scope currentScope = this.scope; while (currentScope != null) { switch (currentScope.kind) { case Scope.METHOD_SCOPE: @@ -1744,9 +1749,9 @@ private int findNonNullDefault(Scope currentScope, LookupEnvironment environment if (referenceMethod != null && referenceMethod.binding != null) { long methodTagBits = referenceMethod.binding.tagBits; if ((methodTagBits & TagBits.AnnotationNonNullByDefault) != 0) - return NONNULL_BY_DEFAULT; + return true; if ((methodTagBits & TagBits.AnnotationNullUnspecifiedByDefault) != 0) - return NULL_UNSPECIFIED_BY_DEFAULT; + return false; } break; case Scope.CLASS_SCOPE: @@ -1754,7 +1759,7 @@ private int findNonNullDefault(Scope currentScope, LookupEnvironment environment if (currentType != null) { int foundDefaultNullness = currentType.defaultNullness; if (foundDefaultNullness != NO_NULL_DEFAULT) { - return foundDefaultNullness; + return foundDefaultNullness == NONNULL_BY_DEFAULT; } } break; @@ -1764,13 +1769,10 @@ private int findNonNullDefault(Scope currentScope, LookupEnvironment environment // package if (currentType != null) { - int foundDefaultNullness = currentType.getPackage().defaultNullness; - if (foundDefaultNullness != NO_NULL_DEFAULT) { - return foundDefaultNullness; - } + return currentType.getPackage().defaultNullness == NONNULL_BY_DEFAULT; } - return NO_NULL_DEFAULT; + return false; } public AnnotationHolder retrieveAnnotationHolder(Binding binding, boolean forceInitialization) { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TagBits.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TagBits.java index aa0d422564..a8239d7557 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TagBits.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TagBits.java @@ -14,6 +14,7 @@ * Stephan Herrmann - Contributions for * bug 186342 - [compiler][null] Using annotations for null checking * bug 392099 - [1.8][compiler][null] Apply null annotation on types for null analysis + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -67,6 +68,9 @@ public interface TagBits { long MultiCatchParameter = ASTNode.Bit13; // local long IsResource = ASTNode.Bit14; // local + // have implicit null annotations been collected (inherited(?) & default)? + long IsNullnessKnown = ASTNode.Bit13; // method + // test bits to see if parts of binary types are faulted long AreFieldsSorted = ASTNode.Bit13; long AreFieldsComplete = ASTNode.Bit14; // sorted and all resolved 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 0efa0ef7e2..615b78dea0 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 @@ -24,6 +24,7 @@ * bug 374605 - Unreasonable warning for enum-based switch statements * bug 382353 - [1.8][compiler] Implementation property modifiers should be accepted on default methods. * bug 382347 - [1.8][compiler] Compiler accepts incorrect default method inheritance + * bug 388281 - [compiler][null] inheritance of null annotations as an option *******************************************************************************/ package org.eclipse.jdt.internal.compiler.problem; @@ -327,6 +328,8 @@ public static int getIrritant(int problemID) { case IProblem.ParameterLackingNonNullAnnotation: case IProblem.ParameterLackingNullableAnnotation: case IProblem.CannotImplementIncompatibleNullness: + case IProblem.ConflictingNullAnnotations: + case IProblem.ConflictingInheritedNullAnnotations: return CompilerOptions.NullSpecViolation; case IProblem.RequiredNonNullButProvidedPotentialNull: @@ -8612,6 +8615,44 @@ public void contradictoryNullAnnotations(Annotation annotation) { this.handle(IProblem.ContradictoryNullAnnotations, arguments, shortArguments, annotation.sourceStart, annotation.sourceEnd); } +// conflict default <-> inherited +public void conflictingNullAnnotations(MethodBinding currentMethod, ASTNode location, MethodBinding inheritedMethod) +{ + char[][] nonNullAnnotationName = this.options.nonNullAnnotationName; + char[][] nullableAnnotationName = this.options.nullableAnnotationName; + String[] arguments = { + new String(CharOperation.concatWith(nonNullAnnotationName, '.')), + new String(CharOperation.concatWith(nullableAnnotationName, '.')), + new String(inheritedMethod.declaringClass.readableName()) + }; + String[] shortArguments = { + new String(nonNullAnnotationName[nonNullAnnotationName.length-1]), + new String(nullableAnnotationName[nullableAnnotationName.length-1]), + new String(inheritedMethod.declaringClass.shortReadableName()) + }; + this.handle(IProblem.ConflictingNullAnnotations, arguments, shortArguments, location.sourceStart, location.sourceEnd); +} + +// conflict between different inheriteds +public void conflictingInheritedNullAnnotations(ASTNode location, boolean previousIsNonNull, MethodBinding previousInherited, boolean isNonNull, MethodBinding inheritedMethod) +{ + char[][] previousAnnotationName = previousIsNonNull ? this.options.nonNullAnnotationName : this.options.nullableAnnotationName; + char[][] annotationName = isNonNull ? this.options.nonNullAnnotationName : this.options.nullableAnnotationName; + String[] arguments = { + new String(CharOperation.concatWith(previousAnnotationName, '.')), + new String(previousInherited.declaringClass.readableName()), + new String(CharOperation.concatWith(annotationName, '.')), + new String(inheritedMethod.declaringClass.readableName()) + }; + String[] shortArguments = { + new String(previousAnnotationName[previousAnnotationName.length-1]), + new String(previousInherited.declaringClass.shortReadableName()), + new String(annotationName[annotationName.length-1]), + new String(inheritedMethod.declaringClass.shortReadableName()) + }; + this.handle(IProblem.ConflictingInheritedNullAnnotations, arguments, shortArguments, location.sourceStart, location.sourceEnd); +} + public void illegalAnnotationForBaseType(TypeReference type, Annotation[] annotations, char[] annotationName, long nullAnnotationTagBit) { int typeId = (nullAnnotationTagBit == TagBits.AnnotationNullable) 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 0d765d9bf5..36beb24f76 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 @@ -23,6 +23,7 @@ # bug 374605 - Unreasonable warning for enum-based switch statements # bug 382353 - [1.8][compiler] Implementation property modifiers should be accepted on default methods. # bug 382347 - [1.8][compiler] Compiler accepts incorrect default method inheritance +# bug 388281 - [compiler][null] inheritance of null annotations as an option ############################################################################### 0 = {0} 1 = super cannot be used in java.lang.Object @@ -715,6 +716,8 @@ 931 = Redundant null check: The variable {0} is specified as @{1} 932 = Null comparison always yields false: The variable {0} is specified as @{1} 933 = Null type mismatch: required ''@{0} {1}'' but the provided value is specified as @{2} +939 = The default ''@{0}'' conflicts with the inherited ''@{1}'' annotation in the overridden method from {2} +940 = Conflict between inherited null annotations ''@{0}'' declared in {1} versus ''@{2}'' declared in {3} // Java 8 diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/core/JavaCore.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/core/JavaCore.java index 2125b64d2a..c88a83599c 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/core/JavaCore.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/core/JavaCore.java @@ -95,6 +95,7 @@ * COMPILER_PB_NULL_SPECIFICATION_INSUFFICIENT_INFO * COMPILER_PB_MISSING_ENUM_CASE_DESPITE_DEFAULT * COMPILER_PB_SWITCH_MISSING_DEFAULT_CASE + * COMPILER_INHERIT_NULL_ANNOTATIONS *******************************************************************************/ package org.eclipse.jdt.core; @@ -1686,6 +1687,26 @@ public final class JavaCore extends Plugin { */ public static final String COMPILER_PB_REDUNDANT_NULL_ANNOTATION = PLUGIN_ID + ".compiler.problem.redundantNullAnnotation"; //$NON-NLS-1$ /** + * Compiler option ID: Inheritance of null annotations. + * <p>When enabled, the compiler will check for each method without any explicit null annotations: + * If it overrides a method which has null annotations, it will treat the + * current method as if it had the same annotations as the overridden method.</p> + * <p>Annotation inheritance will use the <em>effective</em> nullness of the overridden method + * after transitively applying inheritance and after applying any default nullness + * (see {@link #COMPILER_NONNULL_BY_DEFAULT_ANNOTATION_NAME}) at the site of the overridden method.</p> + * <p>If different implicit null annotations (from a nonnull default and/or overridden methods) are applicable + * to the same type in a method signature, this is flagged as an error + * and an explicit null annotation must be used to disambiguate.</p> + * <dl> + * <dt>Option id:</dt><dd><code>"org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations"</code></dd> + * <dt>Possible values:</dt><dd><code>{ "disabled", "enabled" }</code></dd> + * <dt>Default:</dt><dd><code>"disabled"</code></dd> + * </dl> + * @since 3.9 + * @category CompilerOptionID + */ + public static final String COMPILER_INHERIT_NULL_ANNOTATIONS = JavaCore.PLUGIN_ID+".compiler.annotation.inheritNullAnnotations"; //$NON-NLS-1$ + /** * Compiler option ID: Setting Source Compatibility Mode. * <p>Specify whether which source level compatibility is used. From 1.4 on, <code>'assert'</code> is a keyword * reserved for assertion support. Also note, than when toggling to 1.4 mode, the target VM |