summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorStephan Herrmann2013-04-24 10:49:40 (EDT)
committer Dani Megert2013-04-24 10:49:40 (EDT)
commit17c7783177bc359beb03a7575237fd347cc3ba49 (patch)
treee52956c9b71888ac6a91e92c883f6ba4d28ff52d
parent5d6ace937cb325666a8ecd8c450083ad352998d0 (diff)
downloadeclipse.jdt.ui-17c7783177bc359beb03a7575237fd347cc3ba49.zip
eclipse.jdt.ui-17c7783177bc359beb03a7575237fd347cc3ba49.tar.gz
eclipse.jdt.ui-17c7783177bc359beb03a7575237fd347cc3ba49.tar.bz2
Cumulative fix for the following problems:
- bug 400668: [quick fix] The fix change parameter type to @Nonnull generated a null change - bug 405086: [quick fix] don't propose null annotations when those are disabled - bug 395555: [quick fix] Update null annotation quick fixes for bug 388281 - bug 378724: Null annotations are extremely hard to use in an existing project
-rw-r--r--org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationsQuickFixTest.java622
-rw-r--r--org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java5
-rw-r--r--org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties9
-rw-r--r--org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsFix.java60
-rw-r--r--org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsRewriteOperations.java137
-rw-r--r--org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/NullAnnotationsCorrectionProcessor.java16
-rw-r--r--org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickFixProcessor.java36
7 files changed, 818 insertions, 67 deletions
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationsQuickFixTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationsQuickFixTest.java
index 82699f2..234d933 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationsQuickFixTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationsQuickFixTest.java
@@ -78,6 +78,7 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
options.put(JavaCore.COMPILER_PB_NULL_ANNOTATION_INFERENCE_CONFLICT, JavaCore.WARNING);
options.put(JavaCore.COMPILER_PB_NULL_UNCHECKED_CONVERSION, JavaCore.WARNING);
options.put(JavaCore.COMPILER_PB_REDUNDANT_NULL_CHECK, JavaCore.WARNING);
+ options.put(JavaCore.COMPILER_PB_NULL_UNCHECKED_CONVERSION, JavaCore.WARNING);
JavaCore.setOptions(options);
@@ -632,6 +633,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
ArrayList proposals= collectCorrections(cu, astRoot);
assertNumberOfProposals(proposals, 1);
CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'e1' to '@NonNull'");
+
String preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -664,6 +668,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
ArrayList proposals= collectCorrections(cu, astRoot);
assertNumberOfProposals(proposals, 2); // other is add @SW - TODO: check when this is offered
CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'e1' to '@NonNull'");
+
String preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -694,6 +701,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
ArrayList proposals= collectCorrections(cu, astRoot);
assertNumberOfProposals(proposals, 1);
CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'o' to '@NonNull'");
+
String preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -723,6 +733,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
ArrayList proposals= collectCorrections(cu, astRoot);
assertNumberOfProposals(proposals, 2);
CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'o' to '@NonNull'");
+
String preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -782,6 +795,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
ArrayList proposals= collectCorrections(cu, astRoot);
assertNumberOfProposals(proposals, 2);
CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'e1' to '@Nullable'");
+
String preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -795,6 +811,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
assertEqualString(preview, buf.toString());
proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter in overridden 'foo(..)' to '@NonNull'");
+
preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -833,6 +852,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
ArrayList proposals= collectCorrections(cu, astRoot);
assertNumberOfProposals(proposals, 1);
CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'e1' to '@Nullable'");
+
String preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -876,6 +898,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
ArrayList proposals= collectCorrections(cu, astRoot);
assertNumberOfProposals(proposals, 3); // one real change plus two @SuppressWarnings proposals
CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'e1' to '@NonNull'");
+
String preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -884,13 +909,319 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
buf.append("import org.eclipse.jdt.annotation.NonNull;\n");
buf.append("\n");
buf.append("public class E2 extends E {\n");
- buf.append(" void foo(@NonNull Exception e1) {\n"); // change override to accept @Nullable
+ buf.append(" void foo(@NonNull Exception e1) {\n"); // change override to keep @NonNull
buf.append(" e1.printStackTrace();\n");
buf.append(" }\n");
buf.append("}\n");
assertEqualString(preview, buf.toString());
}
+ // http://bugs.eclipse.org/400668 - [quick fix] The fix change parameter type to @Nonnull generated a null change
+ // don't confuse changing arguments of current method and target method
+ // -> split into two proposals
+ public void testChangeParameter4() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@NonNull Object o) {\n");
+ buf.append(" // nop\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E2 {\n");
+ buf.append(" void test(E e, @Nullable Object in) {\n");
+ buf.append(" e.foo(in);\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 2);
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'in' to '@NonNull'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E2 {\n");
+ buf.append(" void test(E e, @NonNull Object in) {\n");
+ buf.append(" e.foo(in);\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+
+ proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter of 'foo(..)' to '@Nullable'");
+
+ preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@Nullable Object o) {\n");
+ buf.append(" // nop\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ }
+
+ // http://bugs.eclipse.org/400668 - [quick fix] The fix change parameter type to @Nonnull generated a null change
+ // don't confuse changing arguments of current method and target method
+ // -> split into two proposals
+ // variant with un-annotated parameter
+ public void testChangeParameter4a() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@NonNull Object o) {\n");
+ buf.append(" // nop\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("public class E2 {\n");
+ buf.append(" void test(E e, Object in) {\n");
+ buf.append(" e.foo(in);\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 3); // third (uninteresting) is "add @SW"
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'in' to '@NonNull'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("\n");
+ buf.append("import org.eclipse.jdt.annotation.NonNull;\n");
+ buf.append("\n");
+ buf.append("public class E2 {\n");
+ buf.append(" void test(E e, @NonNull Object in) {\n");
+ buf.append(" e.foo(in);\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+
+ proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter of 'foo(..)' to '@Nullable'");
+
+ preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@Nullable Object o) {\n");
+ buf.append(" // nop\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ }
+
+ // Bug 405086 - [quick fix] don't propose null annotations when those are disabled
+ public void testChangeParameter5() throws Exception {
+ fJProject1.setOption(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS, JavaCore.DISABLED);
+ try {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(Object o) {\n");
+ buf.append(" if (o == null) return;\n");
+ buf.append(" if (o != null) System.out.print(o.toString());\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 1); // only "add @SW"
+ } finally {
+ fJProject1.setOption(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS, JavaCore.ENABLED);
+ }
+ }
+
+ // Bug 405086 - [quick fix] don't propose null annotations when those are disabled
+ // don't propose a parameter change if there was no parameter annotation being the cause for the warning
+ public void testChangeParameter6() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(Object o) {\n");
+ buf.append(" if (o == null) return;\n");
+ buf.append(" if (o != null) System.out.print(o.toString());\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 1); // only "add @SW"
+ }
+
+ // Bug 405086 - [quick fix] don't propose null annotations when those are disabled
+ // positive case (redundant check)
+ public void testChangeParameter7() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@NonNull Object o) {\n");
+ buf.append(" if (o != null) System.out.print(o.toString());\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 2); // ignore 2nd ("add @SW")
+
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'o' to '@Nullable'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@Nullable Object o) {\n");
+ buf.append(" if (o != null) System.out.print(o.toString());\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ }
+
+ // Bug 405086 - [quick fix] don't propose null annotations when those are disabled
+ // positive case 2 (check always false)
+ public void testChangeParameter8() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@org.eclipse.jdt.annotation.NonNull Object o) {\n");
+ buf.append(" if (o == null) System.out.print(\"NOK\");\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 2); // ignore 2nd ("add @SW")
+
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'o' to '@Nullable'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("\n");
+ buf.append("import org.eclipse.jdt.annotation.Nullable;\n");
+ buf.append("\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@Nullable Object o) {\n");
+ buf.append(" if (o == null) System.out.print(\"NOK\");\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ }
+
+ // http://bugs.eclipse.org/395555 - [quickfix] Update null annotation quick fixes for bug 388281
+ // conflict between inherited and default nullness
+ public void testChangeParameter9() throws Exception {
+ fJProject1.setOption(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED);
+ try {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" void foo(@Nullable Object o) {\n");
+ buf.append(" // nop\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("@NonNullByDefault\n");
+ buf.append("public class E2 extends E {\n");
+ buf.append(" void foo(Object o) {\n");
+ buf.append(" System.out.print(\"E2\");\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 2);
+
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'o' to '@Nullable'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("@NonNullByDefault\n");
+ buf.append("public class E2 extends E {\n");
+ buf.append(" void foo(@Nullable Object o) {\n");
+ buf.append(" System.out.print(\"E2\");\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+
+ proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change parameter 'o' to '@NonNull'");
+
+ preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("@NonNullByDefault\n");
+ buf.append("public class E2 extends E {\n");
+ buf.append(" void foo(@NonNull Object o) {\n");
+ buf.append(" System.out.print(\"E2\");\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ } finally {
+ fJProject1.setOption(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.DISABLED);
+ }
+ }
+
// returning @Nullable value from @NonNull method -> change to @Nullable return
public void testChangeReturn1() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
@@ -949,6 +1280,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
ArrayList proposals= collectCorrections(cu, astRoot);
assertNumberOfProposals(proposals, 2);
CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@NonNull'");
+
String preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -962,6 +1296,9 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
assertEqualString(preview, buf.toString());
proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of overridden 'foo(..)' to '@Nullable'");
+
preview= getPreviewContent(proposal);
buf= new StringBuffer();
@@ -1016,6 +1353,289 @@ public class NullAnnotationsQuickFixTest extends QuickFixTest {
assertEqualString(preview, buf.toString());
}
+ // https://bugs.eclipse.org/395555 - [quickfix] Update null annotation quick fixes for bug 388281
+ // conflict between nullness inherited from different parents
+ public void testChangeReturn3() throws Exception {
+ fJProject1.setOption(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED);
+ try {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" @NonNull Object foo() {\n");
+ buf.append(" // nop\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public interface IE {\n");
+ buf.append(" @Nullable Object foo();\n");
+ buf.append("}\n");
+ pack1.createCompilationUnit("IE.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("public class E2 extends E implements IE {\n");
+ buf.append(" public Object foo() {\n");
+ buf.append(" return this;\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 2);
+
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@Nullable'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("\n");
+ buf.append("import org.eclipse.jdt.annotation.Nullable;\n");
+ buf.append("\n");
+ buf.append("public class E2 extends E implements IE {\n");
+ buf.append(" public @Nullable Object foo() {\n");
+ buf.append(" return this;\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+
+ proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@NonNull'");
+
+ preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("\n");
+ buf.append("import org.eclipse.jdt.annotation.NonNull;\n");
+ buf.append("\n");
+ buf.append("public class E2 extends E implements IE {\n");
+ buf.append(" public @NonNull Object foo() {\n");
+ buf.append(" return this;\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ } finally {
+ fJProject1.setOption(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.DISABLED);
+ }
+ }
+
+ // https://bugs.eclipse.org/378724 - Null annotations are extremely hard to use in an existing project
+ // see comment 12
+ // remove @Nullable without adding redundant @NonNull (due to @NonNullByDefault)
+ public void testChangeReturn4() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+ StringBuffer buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("@NonNullByDefault\n");
+ buf.append("public class E {\n");
+ buf.append(" @Nullable Object bar() {\n");
+ buf.append(" return new Object();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E2 {\n");
+ buf.append(" @NonNull Object foo(E e) {\n");
+ buf.append(" return e.bar();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 3); // includes "add @SW"
+
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@Nullable'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E2 {\n");
+ buf.append(" @Nullable Object foo(E e) {\n");
+ buf.append(" return e.bar();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+
+ proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'bar(..)' to '@NonNull'");
+
+ preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("@NonNullByDefault\n");
+ buf.append("public class E {\n");
+ buf.append(" Object bar() {\n"); // here's the rub: don't add redundant @NonNull, just remove @Nullable
+ buf.append(" return new Object();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ }
+
+ // https://bugs.eclipse.org/378724 - Null annotations are extremely hard to use in an existing project
+ // see comment 12
+ // remove @Nullable without adding redundant @NonNull (due to @NonNullByDefault)
+ // variant: package-level default
+ public void testChangeReturn5() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+
+ StringBuffer buf= new StringBuffer();
+ buf.append("@org.eclipse.jdt.annotation.NonNullByDefault\n");
+ buf.append("package test1;\n");
+ pack1.createCompilationUnit("package-info.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" @Nullable Object bar() {\n");
+ buf.append(" return new Object();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("public class E2 {\n");
+ buf.append(" public Object foo(E e) {\n"); // non-null by default
+ buf.append(" return e.bar();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 3); // includes "add @SW"
+
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@Nullable'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("\n");
+ buf.append("import org.eclipse.jdt.annotation.Nullable;\n");
+ buf.append("\n");
+ buf.append("public class E2 {\n");
+ buf.append(" public @Nullable Object foo(E e) {\n");
+ buf.append(" return e.bar();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+
+ proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'bar(..)' to '@NonNull'");
+
+ preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("public class E {\n");
+ buf.append(" Object bar() {\n"); // here's the rub: don't add redundant @NonNull, just remove @Nullable
+ buf.append(" return new Object();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ }
+
+ // https://bugs.eclipse.org/378724 - Null annotations are extremely hard to use in an existing project
+ // see comment 12
+ // remove @Nullable without adding redundant @NonNull (due to @NonNullByDefault)
+ // variant: cancelled default
+ public void testChangeReturn6() throws Exception {
+ IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+
+ StringBuffer buf= new StringBuffer();
+ buf.append("@org.eclipse.jdt.annotation.NonNullByDefault\n");
+ buf.append("package test1;\n");
+ pack1.createCompilationUnit("package-info.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("@NonNullByDefault(false)\n"); // <- HERE
+ buf.append("public class E {\n");
+ buf.append(" @Nullable Object bar() {\n");
+ buf.append(" return new Object();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("public class E2 {\n");
+ buf.append(" public Object foo(E e) {\n"); // non-null by default
+ buf.append(" return e.bar();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
+
+ CompilationUnit astRoot= getASTRoot(cu);
+ ArrayList proposals= collectCorrections(cu, astRoot);
+ assertNumberOfProposals(proposals, 3); // includes "add @SW"
+
+ CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@Nullable'");
+
+ String preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("\n");
+ buf.append("import org.eclipse.jdt.annotation.Nullable;\n");
+ buf.append("\n");
+ buf.append("public class E2 {\n");
+ buf.append(" public @Nullable Object foo(E e) {\n");
+ buf.append(" return e.bar();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+
+ proposal= (CUCorrectionProposal)proposals.get(1);
+
+ assertEqualString(proposal.getDisplayString(), "Change return type of 'bar(..)' to '@NonNull'");
+
+ preview= getPreviewContent(proposal);
+
+ buf= new StringBuffer();
+ buf.append("package test1;\n");
+ buf.append("import org.eclipse.jdt.annotation.*;\n");
+ buf.append("@NonNullByDefault(false)\n");
+ buf.append("public class E {\n");
+ buf.append(" @NonNull Object bar() {\n"); // here's the rub: DO add redundant @NonNull, default is cancelled here
+ buf.append(" return new Object();\n");
+ buf.append(" }\n");
+ buf.append("}\n");
+ assertEqualString(preview, buf.toString());
+ }
+
public void testRemoveRedundantAnnotation1() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
StringBuffer buf= new StringBuffer();
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java
index 66b3db3..f9c85d4 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java
@@ -7,7 +7,9 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
- * Stephan Herrmann - [quick fix] Add quick fixes for null annotations - https://bugs.eclipse.org/337977
+ * Stephan Herrmann - Contributions for
+ * [quick fix] Add quick fixes for null annotations - https://bugs.eclipse.org/337977
+ * [quick fix] The fix change parameter type to @Nonnull generated a null change - https://bugs.eclipse.org/400668
*******************************************************************************/
package org.eclipse.jdt.internal.corext.fix;
@@ -121,6 +123,7 @@ public final class FixMessages extends NLS {
public static String NullAnnotationsFix_add_annotation_change_name;
public static String NullAnnotationsRewriteOperations_change_method_parameter_nullness;
+ public static String NullAnnotationsRewriteOperations_change_target_method_parameter_nullness;
public static String NullAnnotationsRewriteOperations_change_method_return_nullness;
public static String NullAnnotationsRewriteOperations_change_overridden_parameter_nullness;
public static String NullAnnotationsRewriteOperations_change_overridden_return_nullness;
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties
index 23e5f9e..a5f8dd7 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties
@@ -7,7 +7,9 @@
#
# Contributors:
# IBM Corporation - initial API and implementation
-# Stephan Herrmann - [quick fix] Add quick fixes for null annotations - https://bugs.eclipse.org/337977
+# Stephan Herrmann - Contributions for
+# [quick fix] Add quick fixes for null annotations - https://bugs.eclipse.org/337977
+# [quick fix] The fix change parameter type to @Nonnull generated a null change - https://bugs.eclipse.org/400668
###############################################################################
CleanUpRefactoring_Refactoring_name=Clean Up
CleanUpRefactoring_Initialize_message=Checking preconditions for project ''{0}''
@@ -106,9 +108,10 @@ SortMembersFix_Change_description=Sort Members
SortMembersFix_Fix_description=Sort Members
NullAnnotationsFix_add_annotation_change_name=Add Annotations
-NullAnnotationsRewriteOperations_change_method_parameter_nullness=Change parameter type to ''@{0}''
+NullAnnotationsRewriteOperations_change_method_parameter_nullness=Change parameter ''{0}'' to ''@{1}''
+NullAnnotationsRewriteOperations_change_target_method_parameter_nullness=Change parameter of ''{0}(..)'' to ''@{1}''
NullAnnotationsRewriteOperations_change_method_return_nullness=Change return type of ''{0}(..)'' to ''@{1}''
-NullAnnotationsRewriteOperations_change_overridden_parameter_nullness=Change parameter type in overridden ''{0}(..)'' to ''@{1}''
+NullAnnotationsRewriteOperations_change_overridden_parameter_nullness=Change parameter in overridden ''{0}(..)'' to ''@{1}''
NullAnnotationsRewriteOperations_change_overridden_return_nullness=Change return type of overridden ''{0}(..)'' to ''@{1}''
NullAnnotationsRewriteOperations_remove_redundant_nullness_annotation=Remove redundant nullness annotation
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsFix.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsFix.java
index 5e1037f..ac9d483 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsFix.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsFix.java
@@ -24,11 +24,15 @@ import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.IBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
+import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.dom.SimpleName;
+import org.eclipse.jdt.core.dom.Type;
import org.eclipse.jdt.core.dom.VariableDeclaration;
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
+import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.ChangeKind;
import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.RemoveRedundantAnnotationRewriteOperation;
+import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
@@ -49,6 +53,7 @@ public class NullAnnotationsFix extends CompilationUnitRewriteOperationsFix {
return cu;
}
+ /* recognizes any simple name referring to a parameter binding */
public static boolean isComplainingAboutArgument(ASTNode selectedNode) {
if (!(selectedNode instanceof SimpleName))
return false;
@@ -64,11 +69,19 @@ public class NullAnnotationsFix extends CompilationUnitRewriteOperationsFix {
return false;
}
+ /* recognizes the expression of a return statement and the return type of a method declaration. */
public static boolean isComplainingAboutReturn(ASTNode selectedNode) {
- return selectedNode.getParent().getNodeType() == ASTNode.RETURN_STATEMENT;
+ if (selectedNode.getParent().getNodeType() == ASTNode.RETURN_STATEMENT)
+ return true;
+ while (!(selectedNode instanceof Type)) {
+ if (selectedNode == null) return false;
+ selectedNode = selectedNode.getParent();
+ }
+ return selectedNode.getLocationInParent() == MethodDeclaration.RETURN_TYPE2_PROPERTY;
}
- public static NullAnnotationsFix createNullAnnotationInSignatureFix(CompilationUnit compilationUnit, IProblemLocation problem, boolean modifyOverridden, boolean isArgumentProblem) {
+ public static NullAnnotationsFix createNullAnnotationInSignatureFix(CompilationUnit compilationUnit, IProblemLocation problem,
+ ChangeKind changeKind, boolean isArgumentProblem) {
String nullableAnnotationName= getNullableAnnotationName(compilationUnit.getJavaElement(), false);
String nonNullAnnotationName= getNonNullAnnotationName(compilationUnit.getJavaElement(), false);
String annotationToAdd= nullableAnnotationName;
@@ -78,14 +91,14 @@ public class NullAnnotationsFix extends CompilationUnitRewriteOperationsFix {
case IProblem.IllegalDefinitionToNonNullParameter:
case IProblem.IllegalRedefinitionToNonNullParameter:
// case ParameterLackingNullableAnnotation: // never proposed with modifyOverridden
- if (modifyOverridden) {
+ if (changeKind == ChangeKind.OVERRIDDEN) {
annotationToAdd= nonNullAnnotationName;
annotationToRemove= nullableAnnotationName;
}
break;
case IProblem.ParameterLackingNonNullAnnotation:
case IProblem.IllegalReturnNullityRedefinition:
- if (!modifyOverridden) {
+ if (changeKind != ChangeKind.OVERRIDDEN) {
annotationToAdd= nonNullAnnotationName;
annotationToRemove= nullableAnnotationName;
}
@@ -94,20 +107,30 @@ public class NullAnnotationsFix extends CompilationUnitRewriteOperationsFix {
case IProblem.RequiredNonNullButProvidedPotentialNull:
case IProblem.RequiredNonNullButProvidedUnknown:
case IProblem.RequiredNonNullButProvidedSpecdNullable:
- if (isArgumentProblem) {
+ if (isArgumentProblem == (changeKind != ChangeKind.TARGET)) {
annotationToAdd= nonNullAnnotationName;
annotationToRemove= nullableAnnotationName;
}
break;
- // all others propose to add @Nullable
+ case IProblem.ConflictingNullAnnotations:
+ case IProblem.ConflictingInheritedNullAnnotations:
+ if (changeKind == ChangeKind.INVERSE) {
+ annotationToAdd= nonNullAnnotationName;
+ annotationToRemove= nullableAnnotationName;
+ }
+ // all others propose to add @Nullable
}
// when performing one change at a time we can actually modify another CU than the current one:
NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation operation= NullAnnotationsRewriteOperations.createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, null,
- false/*thisUnitOnly*/, true/*allowRemove*/, modifyOverridden);
+ false/*thisUnitOnly*/, true/*allowRemove*/, isArgumentProblem, changeKind);
if (operation == null)
return null;
+ if (annotationToAdd == nonNullAnnotationName) {
+ operation.fRemoveIfNonNullByDefault= true;
+ operation.fNonNullByDefaultName= getNonNullByDefaultAnnotationName(compilationUnit.getJavaElement(), false);
+ }
return new NullAnnotationsFix(operation.getMessage(), operation.getCompilationUnit(), // note that this uses the findings from createAddAnnotationOperation(..)
new NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation[] { operation });
}
@@ -149,10 +172,14 @@ public class NullAnnotationsFix extends CompilationUnitRewriteOperationsFix {
IProblemLocation problem= locations[i];
if (problem == null)
continue; // problem was filtered out by createCleanUp()
+ boolean isArgumentProblem= isComplainingAboutArgument(problem.getCoveredNode(compilationUnit));
String annotationToAdd= nullableAnnotationName;
String annotationToRemove= nonNullAnnotationName;
- // cf. createNullAnnotationInSignatureFix() but modifyOverridden is constantly false:
+ // cf. createNullAnnotationInSignatureFix() but changeKind is constantly LOCAL
switch (problem.getProblemId()) {
+ case IProblem.IllegalDefinitionToNonNullParameter:
+ case IProblem.IllegalRedefinitionToNonNullParameter:
+ break;
case IProblem.ParameterLackingNonNullAnnotation:
case IProblem.IllegalReturnNullityRedefinition:
annotationToAdd= nonNullAnnotationName;
@@ -162,19 +189,23 @@ public class NullAnnotationsFix extends CompilationUnitRewriteOperationsFix {
case IProblem.RequiredNonNullButProvidedPotentialNull:
case IProblem.RequiredNonNullButProvidedUnknown:
case IProblem.RequiredNonNullButProvidedSpecdNullable:
- ASTNode selectedNode= problem.getCoveredNode(compilationUnit);
- if (isComplainingAboutArgument(selectedNode)) {
+ if (isArgumentProblem) {
annotationToAdd= nonNullAnnotationName;
annotationToRemove= nullableAnnotationName;
}
break;
- // all others propose to add @Nullable
+ // all others propose to add @Nullable
}
// when performing multiple changes we can only modify the one CU that the CleanUp infrastructure provides to the operation.
- CompilationUnitRewriteOperation fix= NullAnnotationsRewriteOperations.createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, handledPositions,
- true/*thisUnitOnly*/, false/*allowRemove*/, false/*modifyOverridden*/);
- if (fix != null)
+ SignatureAnnotationRewriteOperation fix= NullAnnotationsRewriteOperations.createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, handledPositions,
+ true/*thisUnitOnly*/, false/*allowRemove*/, isArgumentProblem, ChangeKind.LOCAL);
+ if (fix != null) {
+ if (annotationToAdd == nonNullAnnotationName) {
+ fix.fRemoveIfNonNullByDefault= true;
+ fix.fNonNullByDefaultName= getNonNullByDefaultAnnotationName(compilationUnit.getJavaElement(), false);
+ }
result.add(fix);
+ }
}
}
@@ -202,6 +233,7 @@ public class NullAnnotationsFix extends CompilationUnitRewriteOperationsFix {
// return problemId == IProblem.NonNullLocalVariableComparisonYieldsFalse || problemId == IProblem.RedundantNullCheckOnNonNullLocalVariable;
// }
+ @SuppressWarnings("unused")
public static boolean hasExplicitNullAnnotation(ICompilationUnit compilationUnit, int offset) {
// FIXME(SH): check for existing annotations disabled due to lack of precision:
// should distinguish what is actually annotated (return? param? which?)
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsRewriteOperations.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsRewriteOperations.java
index adb1fe7..f0074bf 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsRewriteOperations.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsRewriteOperations.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011, 2012 GK Software AG and others.
+ * Copyright (c) 2011, 2013 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
@@ -25,11 +25,11 @@ import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.dom.AST;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.Annotation;
-import org.eclipse.jdt.core.dom.BodyDeclaration;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.IAnnotationBinding;
import org.eclipse.jdt.core.dom.IBinding;
import org.eclipse.jdt.core.dom.IExtendedModifier;
+import org.eclipse.jdt.core.dom.IMemberValuePairBinding;
import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;
@@ -58,10 +58,19 @@ import org.eclipse.jdt.internal.ui.viewsupport.BasicElementLabels;
public class NullAnnotationsRewriteOperations {
+ public enum ChangeKind {
+ LOCAL, // do the normal thing locally in the current method
+ INVERSE, // insert the inverse annotation in the local method
+ OVERRIDDEN, // change the overridden method
+ TARGET // change the target method of a method call
+ }
+
static abstract class SignatureAnnotationRewriteOperation extends CompilationUnitRewriteOperation {
String fAnnotationToAdd;
String fAnnotationToRemove;
boolean fAllowRemove;
+ boolean fRemoveIfNonNullByDefault;
+ String fNonNullByDefaultName;
CompilationUnit fUnit;
protected String fKey;
protected String fMessage;
@@ -97,6 +106,37 @@ public class NullAnnotationsRewriteOperations {
return true;
}
+ /* Is the given element affected by a @NonNullByDefault. */
+ boolean hasNonNullDefault(IBinding enclosingElement) {
+ if (!fRemoveIfNonNullByDefault) return false;
+ IAnnotationBinding[] annotations = enclosingElement.getAnnotations();
+ for (int i= 0; i < annotations.length; i++) {
+ IAnnotationBinding annot = annotations[i];
+ if (annot.getAnnotationType().getQualifiedName().equals(fNonNullByDefaultName)) {
+ IMemberValuePairBinding[] pairs= annot.getDeclaredMemberValuePairs();
+ if (pairs.length > 0) {
+ // is default cancelled by "false" or "value=false" ?
+ for (int j= 0; j < pairs.length; j++)
+ if (pairs[j].getKey() == null || pairs[j].getKey().equals("value")) //$NON-NLS-1$
+ return (pairs[j].getValue() != Boolean.FALSE);
+ }
+ return true;
+ }
+ }
+ if (enclosingElement instanceof IMethodBinding) {
+ return hasNonNullDefault(((IMethodBinding)enclosingElement).getDeclaringClass());
+ } else if (enclosingElement instanceof ITypeBinding) {
+ ITypeBinding typeBinding= (ITypeBinding)enclosingElement;
+ if (typeBinding.isLocal())
+ return hasNonNullDefault(typeBinding.getDeclaringMethod());
+ else if (typeBinding.isMember())
+ return hasNonNullDefault(typeBinding.getDeclaringClass());
+ else
+ return hasNonNullDefault(typeBinding.getPackage());
+ }
+ return false;
+ }
+
public String getMessage() {
return fMessage;
}
@@ -109,7 +149,7 @@ public class NullAnnotationsRewriteOperations {
*/
static class ReturnAnnotationRewriteOperation extends SignatureAnnotationRewriteOperation {
- private final BodyDeclaration fBodyDeclaration;
+ private final MethodDeclaration fBodyDeclaration;
ReturnAnnotationRewriteOperation(CompilationUnit unit, MethodDeclaration method, String annotationToAdd, String annotationToRemove, boolean allowRemove, String message) {
fUnit= unit;
@@ -128,6 +168,8 @@ public class NullAnnotationsRewriteOperations {
TextEditGroup group= createTextEditGroup(fMessage, cuRewrite);
if (!checkExisting(fBodyDeclaration.modifiers(), listRewrite, group))
return;
+ if (hasNonNullDefault(fBodyDeclaration.resolveBinding()))
+ return; // should be safe, as in this case checkExisting() should've already produced a change (remove existing annotation).
Annotation newAnnotation= ast.newMarkerAnnotation();
ImportRewrite importRewrite= cuRewrite.getImportRewrite();
String resolvableName= importRewrite.addImport(fAnnotationToAdd);
@@ -241,9 +283,15 @@ public class NullAnnotationsRewriteOperations {
// Entry for QuickFixes:
public static SignatureAnnotationRewriteOperation createAddAnnotationOperation(CompilationUnit compilationUnit, IProblemLocation problem, String annotationToAdd, String annotationToRemove,
- Set<String> handledPositions, boolean thisUnitOnly, boolean allowRemove, boolean modifyOverridden) {
- SignatureAnnotationRewriteOperation result= modifyOverridden ? createAddAnnotationToOverriddenOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, handledPositions,
- thisUnitOnly, allowRemove) : createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, handledPositions, thisUnitOnly, allowRemove);
+ Set<String> handledPositions, boolean thisUnitOnly, boolean allowRemove, boolean isArgumentProblem, ChangeKind changeKind) {
+ // precondition:
+ // thisUnitOnly => changeKind == LOCAL
+ SignatureAnnotationRewriteOperation result;
+ if (changeKind == ChangeKind.OVERRIDDEN)
+ result= createAddAnnotationToOverriddenOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, allowRemove);
+ else
+ result= createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, changeKind == ChangeKind.TARGET,
+ thisUnitOnly, allowRemove, isArgumentProblem);
if (handledPositions != null && result != null) {
if (handledPositions.contains(result.getKey()))
return null;
@@ -253,7 +301,7 @@ public class NullAnnotationsRewriteOperations {
}
private static SignatureAnnotationRewriteOperation createAddAnnotationOperation(CompilationUnit compilationUnit, IProblemLocation problem, String annotationToAdd, String annotationToRemove,
- Set<String> handledPositions, boolean thisUnitOnly, boolean allowRemove) {
+ boolean changeTargetMethod, boolean thisUnitOnly, boolean allowRemove, boolean isArgumentProblem) {
ICompilationUnit cu= (ICompilationUnit) compilationUnit.getJavaElement();
if (!JavaModelUtil.is50OrHigher(cu.getJavaProject()))
return null;
@@ -274,7 +322,7 @@ public class NullAnnotationsRewriteOperations {
break; // do propose changes even if we already have an annotation
default:
// if this method has annotations, don't change'em
- if (NullAnnotationsFix.hasExplicitNullAnnotation(cu, problem.getOffset()))
+ if (!allowRemove && NullAnnotationsFix.hasExplicitNullAnnotation(cu, problem.getOffset()))
return null;
}
@@ -284,21 +332,38 @@ public class NullAnnotationsRewriteOperations {
annotationNameLabel= annotationToAdd.substring(lastDot + 1);
annotationNameLabel= BasicElementLabels.getJavaElementName(annotationNameLabel);
- if (selectedNode.getParent() instanceof MethodInvocation) {
- // DefiniteNullToNonNullParameter || PotentialNullToNonNullParameter
- MethodInvocation methodInvocation= (MethodInvocation) selectedNode.getParent();
- int paramIdx= methodInvocation.arguments().indexOf(selectedNode);
- IMethodBinding methodBinding= methodInvocation.resolveMethodBinding();
- compilationUnit= findCUForMethod(compilationUnit, cu, methodBinding);
- if (compilationUnit == null)
- return null;
- if (thisUnitOnly && !compilationUnit.getJavaElement().equals(cu))
- return null;
- ASTNode methodDecl= compilationUnit.findDeclaringNode(methodBinding.getKey());
- if (methodDecl == null)
- return null;
- String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_method_parameter_nullness, annotationNameLabel);
- return new ParameterAnnotationRewriteOperation(compilationUnit, (MethodDeclaration) methodDecl, annotationToAdd, annotationToRemove, paramIdx, allowRemove, message);
+ if (changeTargetMethod) {
+ MethodInvocation methodInvocation= null;
+ if (isArgumentProblem) {
+ if (selectedNode.getParent() instanceof MethodInvocation)
+ methodInvocation= (MethodInvocation) selectedNode.getParent();
+ } else {
+ if (selectedNode instanceof MethodInvocation)
+ methodInvocation= (MethodInvocation) selectedNode;
+ }
+ if (methodInvocation != null) {
+ // DefiniteNullToNonNullParameter || PotentialNullToNonNullParameter
+ int paramIdx= methodInvocation.arguments().indexOf(selectedNode);
+ IMethodBinding methodBinding= methodInvocation.resolveMethodBinding();
+ compilationUnit= findCUForMethod(compilationUnit, cu, methodBinding);
+ if (compilationUnit == null)
+ return null;
+ if (thisUnitOnly && !compilationUnit.getJavaElement().equals(cu))
+ return null;
+ ASTNode methodDecl= compilationUnit.findDeclaringNode(methodBinding.getKey());
+ if (methodDecl == null)
+ return null;
+ if (isArgumentProblem) {
+ String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_target_method_parameter_nullness,
+ new Object[] {methodInvocation.getName(), annotationNameLabel});
+ return new ParameterAnnotationRewriteOperation(compilationUnit, (MethodDeclaration) methodDecl, annotationToAdd, annotationToRemove, paramIdx, allowRemove, message);
+ } else {
+ MethodDeclaration declaration = (MethodDeclaration) methodDecl;
+ String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_method_return_nullness,
+ new String[] { declaration.getName().getIdentifier(), annotationNameLabel });
+ return new ReturnAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, allowRemove, message);
+ }
+ }
} else if (declaringNode instanceof MethodDeclaration) {
// complaint is in signature of this method
MethodDeclaration declaration= (MethodDeclaration) declaringNode;
@@ -307,13 +372,14 @@ public class NullAnnotationsRewriteOperations {
case IProblem.ParameterLackingNullableAnnotation:
case IProblem.IllegalDefinitionToNonNullParameter:
case IProblem.IllegalRedefinitionToNonNullParameter:
- case IProblem.NonNullLocalVariableComparisonYieldsFalse:
- case IProblem.RedundantNullCheckOnNonNullLocalVariable:
- // statements suggest changing parameters:
+ case IProblem.SpecdNonNullLocalVariableComparisonYieldsFalse:
+ case IProblem.RedundantNullCheckOnSpecdNonNullLocalVariable:
+ // problems regarding the argument declaration:
if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION) {
String paramName= findAffectedParameterName(selectedNode);
if (paramName != null) {
- String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_method_parameter_nullness, annotationNameLabel);
+ String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_method_parameter_nullness,
+ new Object[] {paramName, annotationNameLabel});
return new ParameterAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, paramName, allowRemove, message);
}
}
@@ -322,13 +388,16 @@ public class NullAnnotationsRewriteOperations {
case IProblem.RequiredNonNullButProvidedPotentialNull:
case IProblem.RequiredNonNullButProvidedSpecdNullable:
case IProblem.RequiredNonNullButProvidedUnknown:
- if (NullAnnotationsFix.isComplainingAboutArgument(selectedNode)) {
- //TODO: duplication
- // statements suggest changing parameters:
- if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION) {
- String paramName= findAffectedParameterName(selectedNode);
+ case IProblem.ConflictingNullAnnotations:
+ case IProblem.ConflictingInheritedNullAnnotations:
+ if (isArgumentProblem) {
+ // statement suggests changing parameters:
+ if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION && selectedNode instanceof SimpleName) {
+ // don't call findAffectedParameterName(), in this branch we're not interested in any target method
+ String paramName= ((SimpleName) selectedNode).getIdentifier();
if (paramName != null) {
- String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_method_parameter_nullness, annotationNameLabel);
+ String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_method_parameter_nullness,
+ new Object[] { paramName, annotationNameLabel });
return new ParameterAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, paramName, allowRemove, message);
}
}
@@ -344,7 +413,7 @@ public class NullAnnotationsRewriteOperations {
}
private static SignatureAnnotationRewriteOperation createAddAnnotationToOverriddenOperation(CompilationUnit compilationUnit, IProblemLocation problem, String annotationToAdd,
- String annotationToRemove, Set<String> handledPositions, boolean thisUnitOnly, boolean allowRemove) {
+ String annotationToRemove, boolean allowRemove) {
ICompilationUnit cu= (ICompilationUnit) compilationUnit.getJavaElement();
if (!JavaModelUtil.is50OrHigher(cu.getJavaProject()))
return null;
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/NullAnnotationsCorrectionProcessor.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/NullAnnotationsCorrectionProcessor.java
index 6dd3808..7140306 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/NullAnnotationsCorrectionProcessor.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/NullAnnotationsCorrectionProcessor.java
@@ -31,6 +31,7 @@ import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.internal.corext.dom.ASTNodes;
import org.eclipse.jdt.internal.corext.fix.NullAnnotationsFix;
+import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.ChangeKind;
import org.eclipse.jdt.ui.text.java.IInvocationContext;
import org.eclipse.jdt.ui.text.java.IProblemLocation;
@@ -46,18 +47,21 @@ import org.eclipse.jdt.internal.ui.text.correction.proposals.FixCorrectionPropos
*/
public class NullAnnotationsCorrectionProcessor {
- public static void addReturnAndArgumentTypeProposal(IInvocationContext context, IProblemLocation problem, Collection<ICommandAccess> proposals) {
+ // pre: changeKind != OVERRIDDEN
+ public static void addReturnAndArgumentTypeProposal(IInvocationContext context, IProblemLocation problem, ChangeKind changeKind,
+ Collection<ICommandAccess> proposals) {
CompilationUnit astRoot= context.getASTRoot();
ASTNode selectedNode= problem.getCoveringNode(astRoot);
boolean isArgumentProblem= NullAnnotationsFix.isComplainingAboutArgument(selectedNode);
if (isArgumentProblem || NullAnnotationsFix.isComplainingAboutReturn(selectedNode))
- addNullAnnotationInSignatureProposal(context, problem, proposals, false, isArgumentProblem);
+ addNullAnnotationInSignatureProposal(context, problem, proposals, changeKind, isArgumentProblem);
}
- public static void addNullAnnotationInSignatureProposal(IInvocationContext context, IProblemLocation problem, Collection<ICommandAccess> proposals, boolean modifyOverridden,
- boolean isArgumentProblem) {
- NullAnnotationsFix fix= NullAnnotationsFix.createNullAnnotationInSignatureFix(context.getASTRoot(), problem, modifyOverridden, isArgumentProblem);
+ public static void addNullAnnotationInSignatureProposal(IInvocationContext context, IProblemLocation problem,
+ Collection<ICommandAccess> proposals, ChangeKind changeKind, boolean isArgumentProblem) {
+ NullAnnotationsFix fix= NullAnnotationsFix.createNullAnnotationInSignatureFix(context.getASTRoot(), problem,
+ changeKind, isArgumentProblem);
if (fix != null) {
Image image= JavaPluginImages.get(JavaPluginImages.IMG_CORRECTION_CHANGE);
@@ -92,7 +96,7 @@ public class NullAnnotationsCorrectionProcessor {
}
};
}
- int relevance= modifyOverridden ? IProposalRelevance.CHANGE_NULLNESS_ANNOTATION_IN_OVERRIDDEN_METHOD : IProposalRelevance.CHANGE_NULLNESS_ANNOTATION; //raise local change above change in overridden method
+ int relevance= (changeKind == ChangeKind.OVERRIDDEN) ? IProposalRelevance.CHANGE_NULLNESS_ANNOTATION_IN_OVERRIDDEN_METHOD : IProposalRelevance.CHANGE_NULLNESS_ANNOTATION; //raise local change above change in overridden method
FixCorrectionProposal proposal= new FixCorrectionProposal(fix, new NullAnnotationsCleanUp(options, problem.getProblemId()), relevance, image, context);
proposals.add(proposal);
}
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickFixProcessor.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickFixProcessor.java
index bdf4b7f..52a04e8 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickFixProcessor.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickFixProcessor.java
@@ -8,7 +8,11 @@
* Contributors:
* IBM Corporation - initial API and implementation
* Benjamin Muskalla <b.muskalla@gmx.net> - [quick fix] Quick fix for missing synchronized modifier - https://bugs.eclipse.org/bugs/show_bug.cgi?id=245250
- * Stephan Herrmann - [quick fix] Add quick fixes for null annotations - https://bugs.eclipse.org/337977
+ * Stephan Herrmann - Contributions for
+ * [quick fix] Add quick fixes for null annotations - https://bugs.eclipse.org/337977
+ * [quick fix] The fix change parameter type to @Nonnull generated a null change - https://bugs.eclipse.org/400668
+ * [quick fix] don't propose null annotations when those are disabled - https://bugs.eclipse.org/405086
+ * [quickfix] Update null annotation quick fixes for bug 388281 - https://bugs.eclipse.org/395555
*******************************************************************************/
package org.eclipse.jdt.internal.ui.text.correction;
@@ -20,10 +24,12 @@ import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.IBuffer;
import org.eclipse.jdt.core.ICompilationUnit;
+import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.compiler.IProblem;
+import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.ChangeKind;
import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
import org.eclipse.jdt.ui.text.java.IInvocationContext;
@@ -248,11 +254,13 @@ public class QuickFixProcessor implements IQuickFixProcessor {
case IProblem.IllegalDefinitionToNonNullParameter:
case IProblem.ParameterLackingNonNullAnnotation:
case IProblem.ParameterLackingNullableAnnotation:
- case IProblem.NonNullLocalVariableComparisonYieldsFalse:
- case IProblem.RedundantNullCheckOnNonNullLocalVariable:
+ case IProblem.SpecdNonNullLocalVariableComparisonYieldsFalse:
+ case IProblem.RedundantNullCheckOnSpecdNonNullLocalVariable:
case IProblem.RedundantNullAnnotation:
case IProblem.UnusedTypeParameter:
case IProblem.NullableFieldReference:
+ case IProblem.ConflictingNullAnnotations:
+ case IProblem.ConflictingInheritedNullAnnotations:
return true;
default:
return SuppressWarningsSubProcessor.hasSuppressWarningsProposal(cu.getJavaProject(), problemId);
@@ -688,8 +696,9 @@ public class QuickFixProcessor implements IQuickFixProcessor {
case IProblem.IllegalReturnNullityRedefinition:
case IProblem.IllegalDefinitionToNonNullParameter:
case IProblem.IllegalRedefinitionToNonNullParameter:
- NullAnnotationsCorrectionProcessor.addNullAnnotationInSignatureProposal(context, problem, proposals, false, true);
- NullAnnotationsCorrectionProcessor.addNullAnnotationInSignatureProposal(context, problem, proposals, true, true);
+ boolean isArgProblem = id != IProblem.IllegalReturnNullityRedefinition;
+ NullAnnotationsCorrectionProcessor.addNullAnnotationInSignatureProposal(context, problem, proposals, ChangeKind.LOCAL, isArgProblem);
+ NullAnnotationsCorrectionProcessor.addNullAnnotationInSignatureProposal(context, problem, proposals, ChangeKind.OVERRIDDEN, isArgProblem);
break;
case IProblem.RequiredNonNullButProvidedSpecdNullable:
case IProblem.RequiredNonNullButProvidedUnknown:
@@ -699,9 +708,15 @@ public class QuickFixProcessor implements IQuickFixProcessor {
case IProblem.RequiredNonNullButProvidedPotentialNull:
case IProblem.ParameterLackingNonNullAnnotation:
case IProblem.ParameterLackingNullableAnnotation:
- case IProblem.NonNullLocalVariableComparisonYieldsFalse:
- case IProblem.RedundantNullCheckOnNonNullLocalVariable:
- NullAnnotationsCorrectionProcessor.addReturnAndArgumentTypeProposal(context, problem, proposals);
+ NullAnnotationsCorrectionProcessor.addReturnAndArgumentTypeProposal(context, problem, ChangeKind.LOCAL, proposals);
+ NullAnnotationsCorrectionProcessor.addReturnAndArgumentTypeProposal(context, problem, ChangeKind.TARGET, proposals);
+ break;
+ case IProblem.SpecdNonNullLocalVariableComparisonYieldsFalse:
+ case IProblem.RedundantNullCheckOnSpecdNonNullLocalVariable:
+ IJavaProject prj = context.getCompilationUnit().getJavaProject();
+ if (prj != null && JavaCore.ENABLED.equals(prj.getOption(JavaCore.COMPILER_ANNOTATION_NULL_ANALYSIS, true))) {
+ NullAnnotationsCorrectionProcessor.addReturnAndArgumentTypeProposal(context, problem, ChangeKind.LOCAL, proposals);
+ }
break;
case IProblem.RedundantNullAnnotation:
case IProblem.RedundantNullDefaultAnnotationPackage:
@@ -715,6 +730,11 @@ public class QuickFixProcessor implements IQuickFixProcessor {
case IProblem.NullableFieldReference:
NullAnnotationsCorrectionProcessor.addExtractCheckedLocalProposal(context, problem, proposals);
break;
+ case IProblem.ConflictingNullAnnotations:
+ case IProblem.ConflictingInheritedNullAnnotations:
+ NullAnnotationsCorrectionProcessor.addReturnAndArgumentTypeProposal(context, problem, ChangeKind.LOCAL, proposals);
+ NullAnnotationsCorrectionProcessor.addReturnAndArgumentTypeProposal(context, problem, ChangeKind.INVERSE, proposals);
+ break;
default:
}
if (JavaModelUtil.is50OrHigher(context.getCompilationUnit().getJavaProject())) {