Attachment #229409 for bug #378724

View | Details | Raw Unified | Return to bug 378724
Collapse All | Expand All




		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);

		}
	}

	// 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();




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;

			case IProblem.RequiredNonNullButProvidedPotentialNull:
			case IProblem.RequiredNonNullButProvidedUnknown:
			case IProblem.RequiredNonNullButProvidedSpecdNullable:
				if (isArgumentProblem == (changeKind != ChangeKind.TARGET)) {
					annotationToAdd= nonNullAnnotationName;
					annotationToRemove= nullableAnnotationName;
				}


		// 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*/, 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 });
	}

			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 changeKind is constantly LOCAL
			switch (problem.getProblemId()) {
				case IProblem.IllegalDefinitionToNonNullParameter:
				case IProblem.IllegalRedefinitionToNonNullParameter:
					break;
				case IProblem.ParameterLackingNonNullAnnotation:
				case IProblem.IllegalReturnNullityRedefinition:
					annotationToAdd= nonNullAnnotationName;

				case IProblem.RequiredNonNullButProvidedPotentialNull:
				case IProblem.RequiredNonNullButProvidedUnknown:
				case IProblem.RequiredNonNullButProvidedSpecdNullable:
					if (isArgumentProblem) {
					if (isComplainingAboutArgument(selectedNode)) {
						annotationToAdd= nonNullAnnotationName;
						annotationToRemove= nullableAnnotationName;
					}
					break;
				// 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.
			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);
			}
		}
	}


Lines 25-35 Link Here

(-)a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsRewriteOperations.java (-14 / +62 lines)
25	import org.eclipse.jdt.core.dom.AST;	25	import org.eclipse.jdt.core.dom.AST;
26	import org.eclipse.jdt.core.dom.ASTNode;	26	import org.eclipse.jdt.core.dom.ASTNode;
27	import org.eclipse.jdt.core.dom.Annotation;	27	import org.eclipse.jdt.core.dom.Annotation;
28	import org.eclipse.jdt.core.dom.BodyDeclaration;
29	import org.eclipse.jdt.core.dom.CompilationUnit;	28	import org.eclipse.jdt.core.dom.CompilationUnit;
30	import org.eclipse.jdt.core.dom.IAnnotationBinding;	29	import org.eclipse.jdt.core.dom.IAnnotationBinding;
31	import org.eclipse.jdt.core.dom.IBinding;	30	import org.eclipse.jdt.core.dom.IBinding;
32	import org.eclipse.jdt.core.dom.IExtendedModifier;	31	import org.eclipse.jdt.core.dom.IExtendedModifier;
		32	import org.eclipse.jdt.core.dom.IMemberValuePairBinding;
33	import org.eclipse.jdt.core.dom.IMethodBinding;	33	import org.eclipse.jdt.core.dom.IMethodBinding;
34	import org.eclipse.jdt.core.dom.ITypeBinding;	34	import org.eclipse.jdt.core.dom.ITypeBinding;
35	import org.eclipse.jdt.core.dom.IVariableBinding;	35	import org.eclipse.jdt.core.dom.IVariableBinding;
Lines 69-74 Link Here
69	String fAnnotationToAdd;	69	String fAnnotationToAdd;
70	String fAnnotationToRemove;	70	String fAnnotationToRemove;
71	boolean fAllowRemove;	71	boolean fAllowRemove;
		72	boolean fRemoveIfNonNullByDefault;
		73	String fNonNullByDefaultName;
72	CompilationUnit fUnit;	74	CompilationUnit fUnit;
73	protected String fKey;	75	protected String fKey;
74	protected String fMessage;	76	protected String fMessage;
Lines 104-109 Link Here
104	return true;	106	return true;
105	}	107	}
106		108
		109	/* Is the given element affected by a @NonNullByDefault. */
		110	boolean hasNonNullDefault(IBinding enclosingElement) {
		111	if (!fRemoveIfNonNullByDefault) return false;
		112	IAnnotationBinding[] annotations = enclosingElement.getAnnotations();
		113	for (int i= 0; i < annotations.length; i++) {
		114	IAnnotationBinding annot = annotations[i];
		115	if (annot.getAnnotationType().getQualifiedName().equals(fNonNullByDefaultName)) {
		116	IMemberValuePairBinding[] pairs= annot.getDeclaredMemberValuePairs();
		117	if (pairs.length > 0) {
		118	// is default cancelled by "false" or "value=false" ?
		119	for (int j= 0; j < pairs.length; j++)
		120	if (pairs[j].getKey() == null \|\| pairs[j].getKey().equals("value")) //$NON-NLS-1$
		121	return (pairs[j].getValue() != Boolean.FALSE);
		122	}
		123	return true;
		124	}
		125	}
		126	if (enclosingElement instanceof IMethodBinding) {
		127	return hasNonNullDefault(((IMethodBinding)enclosingElement).getDeclaringClass());
		128	} else if (enclosingElement instanceof ITypeBinding) {
		129	ITypeBinding typeBinding= (ITypeBinding)enclosingElement;
		130	if (typeBinding.isLocal())
		131	return hasNonNullDefault(typeBinding.getDeclaringMethod());
		132	else if (typeBinding.isMember())
		133	return hasNonNullDefault(typeBinding.getDeclaringClass());
		134	else
		135	return hasNonNullDefault(typeBinding.getPackage());
		136	}
		137	return false;
		138	}
		139
107	public String getMessage() {	140	public String getMessage() {
108	return fMessage;	141	return fMessage;
109	}	142	}
Lines 116-122 Link Here
116	*/	149	*/
117	static class ReturnAnnotationRewriteOperation extends SignatureAnnotationRewriteOperation {	150	static class ReturnAnnotationRewriteOperation extends SignatureAnnotationRewriteOperation {
118		151
119	private final BodyDeclaration fBodyDeclaration;	152	private final MethodDeclaration fBodyDeclaration;
120		153
121	ReturnAnnotationRewriteOperation(CompilationUnit unit, MethodDeclaration method, String annotationToAdd, String annotationToRemove, boolean allowRemove, String message) {	154	ReturnAnnotationRewriteOperation(CompilationUnit unit, MethodDeclaration method, String annotationToAdd, String annotationToRemove, boolean allowRemove, String message) {
122	fUnit= unit;	155	fUnit= unit;
Lines 135-140 Link Here
135	TextEditGroup group= createTextEditGroup(fMessage, cuRewrite);	168	TextEditGroup group= createTextEditGroup(fMessage, cuRewrite);
136	if (!checkExisting(fBodyDeclaration.modifiers(), listRewrite, group))	169	if (!checkExisting(fBodyDeclaration.modifiers(), listRewrite, group))
137	return;	170	return;
		171	if (hasNonNullDefault(fBodyDeclaration.resolveBinding()))
		172	return; // should be safe, as in this case checkExisting() should've already produced a change (remove existing annotation).
138	Annotation newAnnotation= ast.newMarkerAnnotation();	173	Annotation newAnnotation= ast.newMarkerAnnotation();
139	ImportRewrite importRewrite= cuRewrite.getImportRewrite();	174	ImportRewrite importRewrite= cuRewrite.getImportRewrite();
140	String resolvableName= importRewrite.addImport(fAnnotationToAdd);	175	String resolvableName= importRewrite.addImport(fAnnotationToAdd);
Lines 248-254 Link Here
248		283
249	// Entry for QuickFixes:	284	// Entry for QuickFixes:
250	public static SignatureAnnotationRewriteOperation createAddAnnotationOperation(CompilationUnit compilationUnit, IProblemLocation problem, String annotationToAdd, String annotationToRemove,	285	public static SignatureAnnotationRewriteOperation createAddAnnotationOperation(CompilationUnit compilationUnit, IProblemLocation problem, String annotationToAdd, String annotationToRemove,
251	Set<String> handledPositions, boolean thisUnitOnly, boolean allowRemove, ChangeKind changeKind) {	286	Set<String> handledPositions, boolean thisUnitOnly, boolean allowRemove, boolean isArgumentProblem, ChangeKind changeKind) {
252	// precondition:	287	// precondition:
253	// thisUnitOnly => changeKind == LOCAL	288	// thisUnitOnly => changeKind == LOCAL
254	SignatureAnnotationRewriteOperation result;	289	SignatureAnnotationRewriteOperation result;
Lines 256-262 Link Here
256	result= createAddAnnotationToOverriddenOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, allowRemove);	291	result= createAddAnnotationToOverriddenOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, allowRemove);
257	else	292	else
258	result= createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, changeKind == ChangeKind.TARGET,	293	result= createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, changeKind == ChangeKind.TARGET,
259	thisUnitOnly, allowRemove);	294	thisUnitOnly, allowRemove, isArgumentProblem);
260	if (handledPositions != null && result != null) {	295	if (handledPositions != null && result != null) {
261	if (handledPositions.contains(result.getKey()))	296	if (handledPositions.contains(result.getKey()))
262	return null;	297	return null;
Lines 266-272 Link Here
266	}	301	}
267		302
268	private static SignatureAnnotationRewriteOperation createAddAnnotationOperation(CompilationUnit compilationUnit, IProblemLocation problem, String annotationToAdd, String annotationToRemove,	303	private static SignatureAnnotationRewriteOperation createAddAnnotationOperation(CompilationUnit compilationUnit, IProblemLocation problem, String annotationToAdd, String annotationToRemove,
269	boolean changeTargetMethod, boolean thisUnitOnly, boolean allowRemove) {	304	boolean changeTargetMethod, boolean thisUnitOnly, boolean allowRemove, boolean isArgumentProblem) {
270	ICompilationUnit cu= (ICompilationUnit) compilationUnit.getJavaElement();	305	ICompilationUnit cu= (ICompilationUnit) compilationUnit.getJavaElement();
271	if (!JavaModelUtil.is50OrHigher(cu.getJavaProject()))	306	if (!JavaModelUtil.is50OrHigher(cu.getJavaProject()))
272	return null;	307	return null;
Lines 298-306 Link Here
298	annotationNameLabel= BasicElementLabels.getJavaElementName(annotationNameLabel);	333	annotationNameLabel= BasicElementLabels.getJavaElementName(annotationNameLabel);
299		334
300	if (changeTargetMethod) {	335	if (changeTargetMethod) {
301	if (selectedNode.getParent() instanceof MethodInvocation) {	336	MethodInvocation methodInvocation= null;
		337	if (isArgumentProblem) {
		338	if (selectedNode.getParent() instanceof MethodInvocation)
		339	methodInvocation= (MethodInvocation) selectedNode.getParent();
		340	} else {
		341	if (selectedNode instanceof MethodInvocation)
		342	methodInvocation= (MethodInvocation) selectedNode;
		343	}
		344	if (methodInvocation != null) {
302	// DefiniteNullToNonNullParameter \|\| PotentialNullToNonNullParameter	345	// DefiniteNullToNonNullParameter \|\| PotentialNullToNonNullParameter
303	MethodInvocation methodInvocation= (MethodInvocation) selectedNode.getParent();
304	int paramIdx= methodInvocation.arguments().indexOf(selectedNode);	346	int paramIdx= methodInvocation.arguments().indexOf(selectedNode);
305	IMethodBinding methodBinding= methodInvocation.resolveMethodBinding();	347	IMethodBinding methodBinding= methodInvocation.resolveMethodBinding();
306	compilationUnit= findCUForMethod(compilationUnit, cu, methodBinding);	348	compilationUnit= findCUForMethod(compilationUnit, cu, methodBinding);
Lines 311-319 Link Here
311	ASTNode methodDecl= compilationUnit.findDeclaringNode(methodBinding.getKey());	353	ASTNode methodDecl= compilationUnit.findDeclaringNode(methodBinding.getKey());
312	if (methodDecl == null)	354	if (methodDecl == null)
313	return null;	355	return null;
314	String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_target_method_parameter_nullness,	356	if (isArgumentProblem) {
315	new Object[] {methodInvocation.getName(), annotationNameLabel});	357	String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_target_method_parameter_nullness,
316	return new ParameterAnnotationRewriteOperation(compilationUnit, (MethodDeclaration) methodDecl, annotationToAdd, annotationToRemove, paramIdx, allowRemove, message);	358	new Object[] {methodInvocation.getName(), annotationNameLabel});
		359	return new ParameterAnnotationRewriteOperation(compilationUnit, (MethodDeclaration) methodDecl, annotationToAdd, annotationToRemove, paramIdx, allowRemove, message);
		360	} else {
		361	MethodDeclaration declaration = (MethodDeclaration) methodDecl;
		362	String message= Messages.format(FixMessages.NullAnnotationsRewriteOperations_change_method_return_nullness,
		363	new String[] { declaration.getName().getIdentifier(), annotationNameLabel });
		364	return new ReturnAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, allowRemove, message);
		365	}
317	}	366	}
318	} else if (declaringNode instanceof MethodDeclaration) {	367	} else if (declaringNode instanceof MethodDeclaration) {
319	// complaint is in signature of this method	368	// complaint is in signature of this method
Lines 325-331 Link Here
325	case IProblem.IllegalRedefinitionToNonNullParameter:	374	case IProblem.IllegalRedefinitionToNonNullParameter:
326	case IProblem.SpecdNonNullLocalVariableComparisonYieldsFalse:	375	case IProblem.SpecdNonNullLocalVariableComparisonYieldsFalse:
327	case IProblem.RedundantNullCheckOnSpecdNonNullLocalVariable:	376	case IProblem.RedundantNullCheckOnSpecdNonNullLocalVariable:
328	// statements suggest changing parameters:	377	// problems regarding the argument declaration:
329	if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION) {	378	if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION) {
330	String paramName= findAffectedParameterName(selectedNode);	379	String paramName= findAffectedParameterName(selectedNode);
331	if (paramName != null) {	380	if (paramName != null) {
Lines 341-349 Link Here
341	case IProblem.RequiredNonNullButProvidedUnknown:	390	case IProblem.RequiredNonNullButProvidedUnknown:
342	case IProblem.ConflictingNullAnnotations:	391	case IProblem.ConflictingNullAnnotations:
343	case IProblem.ConflictingInheritedNullAnnotations:	392	case IProblem.ConflictingInheritedNullAnnotations:
344	if (NullAnnotationsFix.isComplainingAboutArgument(selectedNode)) {	393	if (isArgumentProblem) {
345	//TODO: duplication (consider chaining isArgumentProblem also into this method)	394	// statement suggests changing parameters:
346	// statements suggest changing parameters:
347	if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION && selectedNode instanceof SimpleName) {	395	if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION && selectedNode instanceof SimpleName) {
348	// don't call findAffectedParameterName(), in this branch we're not interested in any target method	396	// don't call findAffectedParameterName(), in this branch we're not interested in any target method
349	String paramName= ((SimpleName) selectedNode).getIdentifier();	397	String paramName= ((SimpleName) selectedNode).getIdentifier();

Return to bug 378724

Lines 1353-1359 Link Here

(-)a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationsQuickFixTest.java (-1 / +207 lines)
1353	assertEqualString(preview, buf.toString());	1353	assertEqualString(preview, buf.toString());
1354	}	1354	}
1355		1355
1356	// http://bugs.eclipse.org/395555 - [quickfix] Update null annotation quick fixes for bug 388281	1356	// https://bugs.eclipse.org/395555 - [quickfix] Update null annotation quick fixes for bug 388281
1357	// conflict between nullness inherited from different parents	1357	// conflict between nullness inherited from different parents
1358	public void testChangeReturn3() throws Exception {	1358	public void testChangeReturn3() throws Exception {
1359	fJProject1.setOption(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED);	1359	fJProject1.setOption(JavaCore.COMPILER_INHERIT_NULL_ANNOTATIONS, JavaCore.ENABLED);
Lines 1430-1435 Link Here
1430	}	1430	}
1431	}	1431	}
1432		1432
		1433	// https://bugs.eclipse.org/378724 - Null annotations are extremely hard to use in an existing project
		1434	// see comment 12
		1435	// remove @Nullable without adding redundant @NonNull (due to @NonNullByDefault)
		1436	public void testChangeReturn4() throws Exception {
		1437	IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
		1438	StringBuffer buf= new StringBuffer();
		1439	buf.append("package test1;\n");
		1440	buf.append("import org.eclipse.jdt.annotation.*;\n");
		1441	buf.append("@NonNullByDefault\n");
		1442	buf.append("public class E {\n");
		1443	buf.append(" @Nullable Object bar() {\n");
		1444	buf.append(" return new Object();\n");
		1445	buf.append(" }\n");
		1446	buf.append("}\n");
		1447	pack1.createCompilationUnit("E.java", buf.toString(), false, null);
		1448
		1449	buf= new StringBuffer();
		1450	buf.append("package test1;\n");
		1451	buf.append("import org.eclipse.jdt.annotation.*;\n");
		1452	buf.append("public class E2 {\n");
		1453	buf.append(" @NonNull Object foo(E e) {\n");
		1454	buf.append(" return e.bar();\n");
		1455	buf.append(" }\n");
		1456	buf.append("}\n");
		1457	ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
		1458
		1459	CompilationUnit astRoot= getASTRoot(cu);
		1460	ArrayList proposals= collectCorrections(cu, astRoot);
		1461	assertNumberOfProposals(proposals, 3); // includes "add @SW"
		1462
		1463	CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
		1464
		1465	assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@Nullable'");
		1466
		1467	String preview= getPreviewContent(proposal);
		1468
		1469	buf= new StringBuffer();
		1470	buf.append("package test1;\n");
		1471	buf.append("import org.eclipse.jdt.annotation.*;\n");
		1472	buf.append("public class E2 {\n");
		1473	buf.append(" @Nullable Object foo(E e) {\n");
		1474	buf.append(" return e.bar();\n");
		1475	buf.append(" }\n");
		1476	buf.append("}\n");
		1477	assertEqualString(preview, buf.toString());
		1478
		1479	proposal= (CUCorrectionProposal)proposals.get(1);
		1480
		1481	assertEqualString(proposal.getDisplayString(), "Change return type of 'bar(..)' to '@NonNull'");
		1482
		1483	preview= getPreviewContent(proposal);
		1484
		1485	buf= new StringBuffer();
		1486	buf.append("package test1;\n");
		1487	buf.append("import org.eclipse.jdt.annotation.*;\n");
		1488	buf.append("@NonNullByDefault\n");
		1489	buf.append("public class E {\n");
		1490	buf.append(" Object bar() {\n"); // here's the rub: don't add redundant @NonNull, just remove @Nullable
		1491	buf.append(" return new Object();\n");
		1492	buf.append(" }\n");
		1493	buf.append("}\n");
		1494	assertEqualString(preview, buf.toString());
		1495	}
		1496
1497	// https://bugs.eclipse.org/378724 - Null annotations are extremely hard to use in an existing project
1498	// see comment 12
1499	// remove @Nullable without adding redundant @NonNull (due to @NonNullByDefault)
1500	// variant: package-level default
1501	public void testChangeReturn5() throws Exception {
1502	IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
1503
1504	StringBuffer buf= new StringBuffer();
1505	buf.append("@org.eclipse.jdt.annotation.NonNullByDefault\n");
1506	buf.append("package test1;\n");
1507	pack1.createCompilationUnit("package-info.java", buf.toString(), false, null);
1508
1509	buf= new StringBuffer();
1510	buf.append("package test1;\n");
1511	buf.append("import org.eclipse.jdt.annotation.*;\n");
1512	buf.append("public class E {\n");
1513	buf.append(" @Nullable Object bar() {\n");
1514	buf.append(" return new Object();\n");
1515	buf.append(" }\n");
1516	buf.append("}\n");
1517	pack1.createCompilationUnit("E.java", buf.toString(), false, null);
1518
1519	buf= new StringBuffer();
1520	buf.append("package test1;\n");
1521	buf.append("public class E2 {\n");
1522	buf.append(" public Object foo(E e) {\n"); // non-null by default
1523	buf.append(" return e.bar();\n");
1524	buf.append(" }\n");
1525	buf.append("}\n");
1526	ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
1527
1528	CompilationUnit astRoot= getASTRoot(cu);
1529	ArrayList proposals= collectCorrections(cu, astRoot);
1530	assertNumberOfProposals(proposals, 3); // includes "add @SW"
1531
1532	CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
1533
1534	assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@Nullable'");
1535
1536	String preview= getPreviewContent(proposal);
1537
1538	buf= new StringBuffer();
1539	buf.append("package test1;\n");
1540	buf.append("\n");
1541	buf.append("import org.eclipse.jdt.annotation.Nullable;\n");
1542	buf.append("\n");
1543	buf.append("public class E2 {\n");
1544	buf.append(" public @Nullable Object foo(E e) {\n");
1545	buf.append(" return e.bar();\n");
1546	buf.append(" }\n");
1547	buf.append("}\n");
1548	assertEqualString(preview, buf.toString());
1549
1550	proposal= (CUCorrectionProposal)proposals.get(1);
1551
1552	assertEqualString(proposal.getDisplayString(), "Change return type of 'bar(..)' to '@NonNull'");
1553
1554	preview= getPreviewContent(proposal);
1555
1556	buf= new StringBuffer();
1557	buf.append("package test1;\n");
1558	buf.append("import org.eclipse.jdt.annotation.*;\n");
1559	buf.append("public class E {\n");
1560	buf.append(" Object bar() {\n"); // here's the rub: don't add redundant @NonNull, just remove @Nullable
1561	buf.append(" return new Object();\n");
1562	buf.append(" }\n");
1563	buf.append("}\n");
1564	assertEqualString(preview, buf.toString());
1565	}
1566
1567	// https://bugs.eclipse.org/378724 - Null annotations are extremely hard to use in an existing project
1568	// see comment 12
1569	// remove @Nullable without adding redundant @NonNull (due to @NonNullByDefault)
1570	// variant: cancelled default
1571	public void testChangeReturn6() throws Exception {
1572	IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
1573
1574	StringBuffer buf= new StringBuffer();
1575	buf.append("@org.eclipse.jdt.annotation.NonNullByDefault\n");
1576	buf.append("package test1;\n");
1577	pack1.createCompilationUnit("package-info.java", buf.toString(), false, null);
1578
1579	buf= new StringBuffer();
1580	buf.append("package test1;\n");
1581	buf.append("import org.eclipse.jdt.annotation.*;\n");
1582	buf.append("@NonNullByDefault(false)\n"); // <- HERE
1583	buf.append("public class E {\n");
1584	buf.append(" @Nullable Object bar() {\n");
1585	buf.append(" return new Object();\n");
1586	buf.append(" }\n");
1587	buf.append("}\n");
1588	pack1.createCompilationUnit("E.java", buf.toString(), false, null);
1589
1590	buf= new StringBuffer();
1591	buf.append("package test1;\n");
1592	buf.append("public class E2 {\n");
1593	buf.append(" public Object foo(E e) {\n"); // non-null by default
1594	buf.append(" return e.bar();\n");
1595	buf.append(" }\n");
1596	buf.append("}\n");
1597	ICompilationUnit cu= pack1.createCompilationUnit("E2.java", buf.toString(), false, null);
1598
1599	CompilationUnit astRoot= getASTRoot(cu);
1600	ArrayList proposals= collectCorrections(cu, astRoot);
1601	assertNumberOfProposals(proposals, 3); // includes "add @SW"
1602
1603	CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);
1604
1605	assertEqualString(proposal.getDisplayString(), "Change return type of 'foo(..)' to '@Nullable'");
1606
1607	String preview= getPreviewContent(proposal);
1608
1609	buf= new StringBuffer();
1610	buf.append("package test1;\n");
1611	buf.append("\n");
1612	buf.append("import org.eclipse.jdt.annotation.Nullable;\n");
1613	buf.append("\n");
1614	buf.append("public class E2 {\n");
1615	buf.append(" public @Nullable Object foo(E e) {\n");
1616	buf.append(" return e.bar();\n");
1617	buf.append(" }\n");
1618	buf.append("}\n");
1619	assertEqualString(preview, buf.toString());
1620
1621	proposal= (CUCorrectionProposal)proposals.get(1);
1622
1623	assertEqualString(proposal.getDisplayString(), "Change return type of 'bar(..)' to '@NonNull'");
1624
1625	preview= getPreviewContent(proposal);
1626
1627	buf= new StringBuffer();
1628	buf.append("package test1;\n");
1629	buf.append("import org.eclipse.jdt.annotation.*;\n");
1630	buf.append("@NonNullByDefault(false)\n");
1631	buf.append("public class E {\n");
1632	buf.append(" @NonNull Object bar() {\n"); // here's the rub: DO add redundant @NonNull, default is cancelled here
1633	buf.append(" return new Object();\n");
1634	buf.append(" }\n");
1635	buf.append("}\n");
1636	assertEqualString(preview, buf.toString());
1637	}
1638
1433	public void testRemoveRedundantAnnotation1() throws Exception {	1639	public void testRemoveRedundantAnnotation1() throws Exception {
1434	IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);	1640	IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
1435	StringBuffer buf= new StringBuffer();	1641	StringBuffer buf= new StringBuffer();

Lines 32-37 Link Here

(-)a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsFix.java (-9 / +22 lines)
32	import org.eclipse.jdt.internal.corext.dom.ASTNodes;	32	import org.eclipse.jdt.internal.corext.dom.ASTNodes;
33	import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.ChangeKind;	33	import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.ChangeKind;
34	import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.RemoveRedundantAnnotationRewriteOperation;	34	import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.RemoveRedundantAnnotationRewriteOperation;
		35	import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation;
35	import org.eclipse.jdt.internal.corext.util.JavaModelUtil;	36	import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
36		37
37	import org.eclipse.jdt.ui.cleanup.ICleanUpFix;	38	import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
Lines 106-112 Link Here
106	case IProblem.RequiredNonNullButProvidedPotentialNull:	107	case IProblem.RequiredNonNullButProvidedPotentialNull:
107	case IProblem.RequiredNonNullButProvidedUnknown:	108	case IProblem.RequiredNonNullButProvidedUnknown:
108	case IProblem.RequiredNonNullButProvidedSpecdNullable:	109	case IProblem.RequiredNonNullButProvidedSpecdNullable:
109	if (isArgumentProblem && changeKind != ChangeKind.TARGET) {	110	if (isArgumentProblem == (changeKind != ChangeKind.TARGET)) {
110	annotationToAdd= nonNullAnnotationName;	111	annotationToAdd= nonNullAnnotationName;
111	annotationToRemove= nullableAnnotationName;	112	annotationToRemove= nullableAnnotationName;
112	}	113	}
Lines 122-131 Link Here
122		123
123	// when performing one change at a time we can actually modify another CU than the current one:	124	// when performing one change at a time we can actually modify another CU than the current one:
124	NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation operation= NullAnnotationsRewriteOperations.createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, null,	125	NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation operation= NullAnnotationsRewriteOperations.createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, null,
125	false/thisUnitOnly/, true/allowRemove/, changeKind);	126	false/thisUnitOnly/, true/allowRemove/, isArgumentProblem, changeKind);
126	if (operation == null)	127	if (operation == null)
127	return null;	128	return null;
128		129
		130	if (annotationToAdd == nonNullAnnotationName) {
		131	operation.fRemoveIfNonNullByDefault= true;
		132	operation.fNonNullByDefaultName= getNonNullByDefaultAnnotationName(compilationUnit.getJavaElement(), false);
		133	}
129	return new NullAnnotationsFix(operation.getMessage(), operation.getCompilationUnit(), // note that this uses the findings from createAddAnnotationOperation(..)	134	return new NullAnnotationsFix(operation.getMessage(), operation.getCompilationUnit(), // note that this uses the findings from createAddAnnotationOperation(..)
130	new NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation[] { operation });	135	new NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation[] { operation });
131	}	136	}
Lines 167-176 Link Here
167	IProblemLocation problem= locations[i];	172	IProblemLocation problem= locations[i];
168	if (problem == null)	173	if (problem == null)
169	continue; // problem was filtered out by createCleanUp()	174	continue; // problem was filtered out by createCleanUp()
		175	boolean isArgumentProblem= isComplainingAboutArgument(problem.getCoveredNode(compilationUnit));
170	String annotationToAdd= nullableAnnotationName;	176	String annotationToAdd= nullableAnnotationName;
171	String annotationToRemove= nonNullAnnotationName;	177	String annotationToRemove= nonNullAnnotationName;
172	// cf. createNullAnnotationInSignatureFix() but modifyOverridden is constantly false:	178	// cf. createNullAnnotationInSignatureFix() but changeKind is constantly LOCAL
173	switch (problem.getProblemId()) {	179	switch (problem.getProblemId()) {
		180	case IProblem.IllegalDefinitionToNonNullParameter:
		181	case IProblem.IllegalRedefinitionToNonNullParameter:
		182	break;
174	case IProblem.ParameterLackingNonNullAnnotation:	183	case IProblem.ParameterLackingNonNullAnnotation:
175	case IProblem.IllegalReturnNullityRedefinition:	184	case IProblem.IllegalReturnNullityRedefinition:
176	annotationToAdd= nonNullAnnotationName;	185	annotationToAdd= nonNullAnnotationName;
Lines 180-198 Link Here
180	case IProblem.RequiredNonNullButProvidedPotentialNull:	189	case IProblem.RequiredNonNullButProvidedPotentialNull:
181	case IProblem.RequiredNonNullButProvidedUnknown:	190	case IProblem.RequiredNonNullButProvidedUnknown:
182	case IProblem.RequiredNonNullButProvidedSpecdNullable:	191	case IProblem.RequiredNonNullButProvidedSpecdNullable:
183	ASTNode selectedNode= problem.getCoveredNode(compilationUnit);	192	if (isArgumentProblem) {
184	if (isComplainingAboutArgument(selectedNode)) {
185	annotationToAdd= nonNullAnnotationName;	193	annotationToAdd= nonNullAnnotationName;
186	annotationToRemove= nullableAnnotationName;	194	annotationToRemove= nullableAnnotationName;
187	}	195	}
188	break;	196	break;
189	// all others propose to add @Nullable	197	// all others propose to add @Nullable
190	}	198	}
191	// when performing multiple changes we can only modify the one CU that the CleanUp infrastructure provides to the operation.	199	// when performing multiple changes we can only modify the one CU that the CleanUp infrastructure provides to the operation.
192	CompilationUnitRewriteOperation fix= NullAnnotationsRewriteOperations.createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, handledPositions,	200	SignatureAnnotationRewriteOperation fix= NullAnnotationsRewriteOperations.createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, handledPositions,
193	true/thisUnitOnly/, false/allowRemove/, ChangeKind.LOCAL);	201	true/thisUnitOnly/, false/allowRemove/, isArgumentProblem, ChangeKind.LOCAL);
194	if (fix != null)	202	if (fix != null) {
		203	if (annotationToAdd == nonNullAnnotationName) {
		204	fix.fRemoveIfNonNullByDefault= true;
		205	fix.fNonNullByDefaultName= getNonNullByDefaultAnnotationName(compilationUnit.getJavaElement(), false);
		206	}
195	result.add(fix);	207	result.add(fix);
		208	}
196	}	209	}
197	}	210	}
198		211