Attachment 217971 Details for Bug 337977 – fix for bug 331649 comment 47

[patch] fix for bug 331649 comment 47

Bug_337977_improvements.patch (text/plain), 14.78 KB, created by Stephan Herrmann

on 2012-06-27 17:41:45 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Stephan Herrmann

Created: 2012-06-27 17:41:45 EDT

Size: 14.78 KB

patch

obsolete

>diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationQuickFixTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationQuickFixTest.java
>index 553cd59..578a63c 100644
>--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationQuickFixTest.java
>+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationQuickFixTest.java
>@@ -471,4 +471,255 @@
> 		buf.append("}\n");
> 		assertEqualString(preview, buf.toString());
> 	}
>+
>+	// @Nullable argument is used where @NonNull is required -> change to @NonNull
>+	public void testChangeParameter1() 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("    @NonNull Exception e = new Exception();\n");
>+		buf.append("    void foo(@Nullable Exception e1) {\n");
>+		buf.append("        this.e = e1;\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);
>+		CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0);
>+		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("    @NonNull Exception e = new Exception();\n");
>+		buf.append("    void foo(@NonNull Exception e1) {\n");
>+		buf.append("        this.e = e1;\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+	}
>+
>+	// unspec'ed argument is used where @NonNull is required -> change to @NonNull
>+	public void testChangeParameter1a() 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("    @NonNull Exception e = new Exception();\n");
>+		buf.append("    void foo(Exception e1) {\n");
>+		buf.append("        this.e = e1;\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); // other is add @SW
>+		CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0);
>+		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("    @NonNull Exception e = new Exception();\n");
>+		buf.append("    void foo(@NonNull Exception e1) {\n");
>+		buf.append("        this.e = e1;\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+	}
>+
>+	// don't propose to change argument if mismatch is in an assignment to the argument
>+	public void testChangeParameter2() 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 Exception e1) {\n");
>+		buf.append("        e1 = null;\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, 0);
>+	}
>+
>+	// Attempt to override a @Nullable argument with a @NonNull argument 
>+	// -> change to @Nullable
>+	// -> change overridden to @NonNull
>+	public void testChangeParameter3() 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(@Nullable Exception e1) {\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 extends E {\n");
>+		buf.append("    void foo(@NonNull Exception e1) {\n");
>+		buf.append("        e1.printStackTrace();\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);
>+		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 extends E {\n");
>+		buf.append("    void foo(@Nullable Exception e1) {\n"); // change override to accept @Nullable
>+		buf.append("        e1.printStackTrace();\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+
>+		proposal= (CUCorrectionProposal) proposals.get(1);
>+		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(@NonNull Exception e1) {\n"); // change the overridden method to force @NonNull
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+	}
>+
>+	// Attempt to override a @Nullable argument with an unspec'ed argument 
>+	// -> change to @Nullable
>+	public void testChangeParameter3a() 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(@Nullable Exception e1) {\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 extends E {\n");
>+		buf.append("    void foo(Exception e1) {\n");
>+		buf.append("        e1.printStackTrace();\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, 1);
>+		CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0);
>+		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 {\n");
>+		buf.append("    void foo(@Nullable Exception e1) {\n"); // change override to accept @Nullable
>+		buf.append("        e1.printStackTrace();\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+	}
>+
>+	// Attempt to override a @NonNull argument with an unspec'ed argument 
>+	// -> change to @NonNull
>+	// TODO(stephan): here the underlying error is actually debatable / not necessary.
>+	//                @NonNull -> unspec'ed could be seen as safe contravariant overriding
>+	public void testChangeParameter3b() 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 Exception e1) {\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 extends E {\n");
>+		buf.append("    void foo(Exception e1) {\n");
>+		buf.append("        e1.printStackTrace();\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, 1);
>+		CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0);
>+		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 extends E {\n");
>+		buf.append("    void foo(@NonNull Exception e1) {\n"); // change override to accept @Nullable
>+		buf.append("        e1.printStackTrace();\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+	}
>+
>+	// returning @Nullable value from @NonNull method -> change to @Nullable return
>+	public void testChangeReturn1() 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("    @Nullable Object o;\n");
>+		buf.append("    @NonNull Object foo() {\n");
>+		buf.append("        return this.o;\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); // other is: add @SW
>+		CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0);
>+		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("    @Nullable Object o;\n");
>+		buf.append("    @Nullable Object foo() {\n");
>+		buf.append("        return this.o;\n");
>+		buf.append("    }\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+	}
> }
>diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullQuickFixes.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullQuickFixes.java
>index d47e697..959f799 100644
>--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullQuickFixes.java
>+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullQuickFixes.java
>@@ -67,12 +67,15 @@
> 		CompilationUnit astRoot= context.getASTRoot();
> 		ASTNode selectedNode= problem.getCoveringNode(astRoot);
> 
>-		if (isComplainingAboutArgument(selectedNode) || isComplainingAboutReturn(selectedNode))
>-			addNullAnnotationInSignatureProposal(context, problem, proposals, false);
>+		boolean isArgumentProblem = isComplainingAboutArgument(selectedNode);
>+		if (isArgumentProblem || isComplainingAboutReturn(selectedNode))
>+			addNullAnnotationInSignatureProposal(context, problem, proposals, false, isArgumentProblem);
> 	}
> 
>-	public static void addNullAnnotationInSignatureProposal(IInvocationContext context, IProblemLocation problem, Collection<ICommandAccess> proposals, boolean modifyOverridden) {
>-		MyCURewriteOperationsFix fix= createNullAnnotationInSignatureFix(context.getASTRoot(), problem, modifyOverridden);
>+	public static void addNullAnnotationInSignatureProposal(IInvocationContext context, IProblemLocation problem, Collection<ICommandAccess> proposals, 
>+			boolean modifyOverridden, boolean isArgumentProblem) 
>+	{
>+		MyCURewriteOperationsFix fix= createNullAnnotationInSignatureFix(context.getASTRoot(), problem, modifyOverridden, isArgumentProblem);
> 
> 		if (fix != null) {
> 			Image image= JavaPluginImages.get(JavaPluginImages.IMG_CORRECTION_CHANGE);
>@@ -132,7 +135,9 @@
> 		return selectedNode.getParent().getNodeType() == ASTNode.RETURN_STATEMENT;
> 	}
> 
>-	private static MyCURewriteOperationsFix createNullAnnotationInSignatureFix(CompilationUnit compilationUnit, IProblemLocation problem, boolean modifyOverridden) {
>+	private static MyCURewriteOperationsFix createNullAnnotationInSignatureFix(CompilationUnit compilationUnit, IProblemLocation problem, 
>+				boolean modifyOverridden, boolean isArgumentProblem) 
>+	{
> 		String nullableAnnotationName= getNullableAnnotationName(compilationUnit.getJavaElement(), false);
> 		String nonNullAnnotationName= getNonNullAnnotationName(compilationUnit.getJavaElement(), false);
> 		String annotationToAdd= nullableAnnotationName;
>@@ -157,7 +162,11 @@
> 			case IProblem.RequiredNonNullButProvidedNull:
> 			case IProblem.RequiredNonNullButProvidedPotentialNull:
> 			case IProblem.RequiredNonNullButProvidedUnknown:
>-				annotationToAdd= nonNullAnnotationName;
>+			case IProblem.RequiredNonNullButProvidedSpecdNullable:
>+				if (isArgumentProblem) {
>+					annotationToAdd= nonNullAnnotationName;
>+					annotationToRemove= nullableAnnotationName;
>+				}
> 				break;
> 		// all others propose to add @Nullable
> 		}
>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 141488f..887f852 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
>@@ -679,8 +679,8 @@
> 			case IProblem.IllegalReturnNullityRedefinition:
> 			case IProblem.IllegalDefinitionToNonNullParameter:
> 			case IProblem.IllegalRedefinitionToNonNullParameter:
>-				NullQuickFixes.addNullAnnotationInSignatureProposal(context, problem, proposals, false);
>-				NullQuickFixes.addNullAnnotationInSignatureProposal(context, problem, proposals, true);
>+				NullQuickFixes.addNullAnnotationInSignatureProposal(context, problem, proposals, false, true);
>+				NullQuickFixes.addNullAnnotationInSignatureProposal(context, problem, proposals, true, true);
> 				break;
> 			case IProblem.RequiredNonNullButProvidedNull:
> 			case IProblem.RequiredNonNullButProvidedPotentialNull:

Actions: View | Diff

Attachments on bug 337977: 206558 | 217460 | 217707 | 217971 | 218014 | 223525