further adjust quickfixes to current implementation
+ more complete support for fixing illegal override
+ make two proposals if either current or overridden signature could be adjusted
respect that those proposals need to use opposite null annotations
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/.settings/org.eclipse.jdt.core.prefs b/contrib/org.eclipse.objectteams.jdt.nullity/.settings/org.eclipse.jdt.core.prefs
index b9836d6..9fa3c77 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/.settings/org.eclipse.jdt.core.prefs
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/.settings/org.eclipse.jdt.core.prefs
@@ -1,4 +1,4 @@
-#Fri Jan 28 21:26:40 CET 2011
+#Mon Aug 08 23:39:51 CEST 2011
eclipse.preferences.version=1
org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.5
@@ -18,7 +18,7 @@
org.eclipse.jdt.core.compiler.problem.discouragedReference=warning
org.eclipse.jdt.core.compiler.problem.emptyStatement=ignore
org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
-org.eclipse.jdt.core.compiler.problem.fallthroughCase=ignore
+org.eclipse.jdt.core.compiler.problem.fallthroughCase=warning
org.eclipse.jdt.core.compiler.problem.fatalOptionalError=disabled
org.eclipse.jdt.core.compiler.problem.fieldHiding=ignore
org.eclipse.jdt.core.compiler.problem.finalParameterBound=warning
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/FixMessages.java b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/FixMessages.java
index 8a1e38a..2524c15 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/FixMessages.java
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/FixMessages.java
@@ -23,7 +23,7 @@
public static String QuickFixes_declare_method_parameter_nullness;
- public static String QuickFixes_declare_method_return_nullable;
+ public static String QuickFixes_declare_method_return_nullness;
public static String QuickFixes_declare_overridden_parameter_as_nonnull;
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/FixMessages.properties b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/FixMessages.properties
index cc36be8..ec61f27 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/FixMessages.properties
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/FixMessages.properties
@@ -2,6 +2,6 @@
NullAnnotationsCleanUp_add_nonnull_annotation=Add missing @NonNull annotation
QuickFixes_add_annotation_change_name=Add Annotations
QuickFixes_declare_method_parameter_nullness=Declare method parameter as @{0}
-QuickFixes_declare_method_return_nullable=Declare method return as @{0}
+QuickFixes_declare_method_return_nullness=Declare method return as @{0}
QuickFixes_declare_overridden_parameter_as_nonnull=Adjust overridden method from {1}, mark parameter as @{0}
QuickFixes_declare_overridden_return_as_nullable=Adjust overridden method from {1}, mark as returning @{0}
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/QuickFixes.java b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/QuickFixes.java
index 08a4324..98a3b1c 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/QuickFixes.java
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/QuickFixes.java
@@ -29,6 +29,7 @@
import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaElement;
+import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.compiler.IProblem;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.CompilationUnit;
@@ -116,7 +117,8 @@
case IllegalDefinitionToNonNullParameter:
case IllegalRedefinitionToNonNullParameter:
case ParameterLackingNonNullAnnotation:
- addNullAnnotationInSignatureProposal(context, problem, proposals);
+ addNullAnnotationInSignatureProposal(context, problem, proposals, false);
+ addNullAnnotationInSignatureProposal(context, problem, proposals, true);
break;
case RequiredNonNullButProvidedNull:
case RequiredNonNullButProvidedPotentialNull:
@@ -125,15 +127,18 @@
case IProblem.RedundantNullCheckOnNonNullLocalVariable:
if (isComplainingAboutArgument(selectedNode)
|| isComplainingAboutReturn(selectedNode))
- addNullAnnotationInSignatureProposal(context, problem, proposals);
+ addNullAnnotationInSignatureProposal(context, problem, proposals, false);
break;
}
}
@SuppressWarnings("unchecked")
- void addNullAnnotationInSignatureProposal(IInvocationContext context, IProblemLocation problem, @SuppressWarnings("rawtypes") Collection proposals)
+ void addNullAnnotationInSignatureProposal(IInvocationContext context,
+ IProblemLocation problem,
+ @SuppressWarnings("rawtypes") Collection proposals,
+ boolean modifyOverridden)
{
- MyCURewriteOperationsFix fix= createNullAnnotationInSignatureFix(context.getASTRoot(), problem);
+ MyCURewriteOperationsFix fix= createNullAnnotationInSignatureFix(context.getASTRoot(), problem, modifyOverridden);
if (fix != null) {
Image image= JavaPluginImages.get(JavaPluginImages.IMG_CORRECTION_CHANGE);
@@ -184,7 +189,7 @@
return selectedNode.getParent().getNodeType() == ASTNode.RETURN_STATEMENT;
}
- MyCURewriteOperationsFix createNullAnnotationInSignatureFix(CompilationUnit compilationUnit, IProblemLocation problem)
+ MyCURewriteOperationsFix createNullAnnotationInSignatureFix(CompilationUnit compilationUnit, IProblemLocation problem, boolean modifyOverridden)
{
String nullableAnnotationName = getNullableAnnotationName(compilationUnit.getJavaElement(), false);
String nonNullAnnotationName = getNonNullAnnotationName(compilationUnit.getJavaElement(), false);
@@ -195,8 +200,11 @@
case IllegalDefinitionToNonNullParameter:
case IllegalRedefinitionToNonNullParameter:
case ParameterLackingNonNullAnnotation:
- annotationToAdd = nonNullAnnotationName;
- annotationToRemove = nullableAnnotationName;
+ case IllegalReturnNullityRedefinition:
+ if (!modifyOverridden) {
+ annotationToAdd = nonNullAnnotationName;
+ annotationToRemove = nullableAnnotationName;
+ }
break;
// all others propose to add @Nullable
}
@@ -204,7 +212,8 @@
// when performing one change at a time we can actually modify another CU than the current one:
RewriteOperations.SignatureAnnotationRewriteOperation operation =
RewriteOperations.createAddAnnotationOperation(
- compilationUnit, problem, annotationToAdd, annotationToRemove, null, false/*thisUnitOnly*/, true/*allowRemove*/);
+ compilationUnit, problem, annotationToAdd, annotationToRemove, null,
+ false/*thisUnitOnly*/, true/*allowRemove*/, modifyOverridden);
if (operation == null)
return null;
@@ -255,13 +264,15 @@
case IllegalDefinitionToNonNullParameter:
case IllegalRedefinitionToNonNullParameter:
case ParameterLackingNonNullAnnotation:
+ case IllegalReturnNullityRedefinition:
annotationToAdd = nonNullAnnotationName;
annotationToRemove = nullableAnnotationName;
// 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 = RewriteOperations.createAddAnnotationOperation(
- compilationUnit, problem, annotationToAdd, annotationToRemove, handledPositions, true/*thisUnitOnly*/, false);
+ compilationUnit, problem, annotationToAdd, annotationToRemove,
+ handledPositions, true/*thisUnitOnly*/, false/*allowRemove*/, false/*modifyOverridden*/);
if (fix != null)
result.add(fix);
}
diff --git a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/RewriteOperations.java b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/RewriteOperations.java
index ede2791..b4c1272 100644
--- a/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/RewriteOperations.java
+++ b/contrib/org.eclipse.objectteams.jdt.nullity/src/org/eclipse/objectteams/internal/jdt/nullity/quickfix/RewriteOperations.java
@@ -31,6 +31,7 @@
import org.eclipse.jdt.core.dom.MarkerAnnotation;
import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.dom.MethodInvocation;
+import org.eclipse.jdt.core.dom.Name;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclaration;
@@ -211,30 +212,55 @@
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);
+ if (handledPositions != null && result != null) {
+ if (handledPositions.contains(result.getKey()))
+ return null;
+ handledPositions.add(result.getKey());
+ }
+ return result;
+ }
+ static SignatureAnnotationRewriteOperation createAddAnnotationOperation(CompilationUnit compilationUnit,
+ IProblemLocation problem,
+ String annotationToAdd,
+ String annotationToRemove,
+ Set<String> handledPositions,
+ boolean thisUnitOnly,
boolean allowRemove)
{
ICompilationUnit cu= (ICompilationUnit)compilationUnit.getJavaElement();
if (!JavaModelUtil.is50OrHigher(cu.getJavaProject()))
return null;
+
+ ASTNode selectedNode= problem.getCoveringNode(compilationUnit);
+ if (selectedNode == null)
+ return null;
+
+ ASTNode declaringNode= getDeclaringNode(selectedNode);
switch (problem.getProblemId()) {
-// case IllegalReturnNullityRedefinition:
case IllegalDefinitionToNonNullParameter:
// case IllegalRedefinitionToNonNullParameter:
// these affect another method
break;
+ case IllegalReturnNullityRedefinition:
+ if (declaringNode == null)
+ declaringNode = selectedNode;
+ break; // do propose changes even if we already have an annotation
default:
// if this method has annotations, don't change'em
if (QuickFixes.hasExplicitNullAnnotation(cu, problem.getOffset()))
return null;
}
- ASTNode selectedNode= problem.getCoveringNode(compilationUnit);
- if (selectedNode == null)
- return null;
-
SignatureAnnotationRewriteOperation result = null;
- ASTNode declaringNode= getDeclaringNode(selectedNode);
String message = null;
String annotationNameLabel = annotationToAdd;
int lastDot = annotationToAdd.lastIndexOf('.');
@@ -288,36 +314,79 @@
}
}
break;
- case IllegalDefinitionToNonNullParameter:
- case IllegalRedefinitionToNonNullParameter:
- result = createChangeOverriddenParameterOperation(compilationUnit, cu, declaration, selectedNode, allowRemove,
- annotationToAdd, annotationToRemove, annotationNameLabel);
- break;
-// TODO: how do we know which method to change (this or super)?
-// case IllegalReturnNullityRedefinition:
-// result = createChangeOverriddenReturnOperation(compilationUnit, cu, declaration, allowRemove,
-// annotationToAdd, annotationToRemove, annotationNameLabel);
-// break;
+ case IllegalReturnNullityRedefinition:
case RequiredNonNullButProvidedNull:
case RequiredNonNullButProvidedPotentialNull:
- message = Messages.format(FixMessages.QuickFixes_declare_method_return_nullable, annotationNameLabel);
+ message = Messages.format(FixMessages.QuickFixes_declare_method_return_nullness, annotationNameLabel);
result = new ReturnAnnotationRewriteOperation(compilationUnit,
- declaration,
- annotationToAdd,
- annotationToRemove,
- allowRemove,
- message);
+ declaration,
+ annotationToAdd,
+ annotationToRemove,
+ allowRemove,
+ message);
break;
}
}
- if (handledPositions != null && result != null) {
- if (handledPositions.contains(result.getKey()))
- return null;
- handledPositions.add(result.getKey());
- }
return result;
}
+ static SignatureAnnotationRewriteOperation createAddAnnotationToOverriddenOperation(CompilationUnit compilationUnit,
+ IProblemLocation problem,
+ String annotationToAdd,
+ String annotationToRemove,
+ Set<String> handledPositions,
+ boolean thisUnitOnly,
+ boolean allowRemove)
+ {
+ ICompilationUnit cu = (ICompilationUnit) compilationUnit.getJavaElement();
+ if (!JavaModelUtil.is50OrHigher(cu.getJavaProject()))
+ return null;
+
+ ASTNode selectedNode = problem.getCoveringNode(compilationUnit);
+ if (selectedNode == null)
+ return null;
+
+ ASTNode declaringNode = getDeclaringNode(selectedNode);
+
+ switch (problem.getProblemId()) {
+ case IllegalDefinitionToNonNullParameter:
+ case IllegalRedefinitionToNonNullParameter:
+ break;
+ case IllegalReturnNullityRedefinition:
+ if (declaringNode == null)
+ declaringNode = selectedNode;
+ break;
+ default:
+ return null;
+ }
+
+ String annotationNameLabel = annotationToAdd;
+ int lastDot = annotationToAdd.lastIndexOf('.');
+ if (lastDot != -1)
+ annotationNameLabel = annotationToAdd.substring(lastDot + 1);
+ annotationNameLabel = BasicElementLabels.getJavaElementName(annotationNameLabel);
+
+ if (declaringNode instanceof MethodDeclaration) {
+
+ // complaint is in signature of this method
+
+ MethodDeclaration declaration = (MethodDeclaration) declaringNode;
+
+ switch (problem.getProblemId()) {
+ case IllegalDefinitionToNonNullParameter:
+ case IllegalRedefinitionToNonNullParameter:
+ return createChangeOverriddenParameterOperation(compilationUnit, cu, declaration, selectedNode,
+ allowRemove, annotationToAdd, annotationToRemove, annotationNameLabel);
+ case IllegalReturnNullityRedefinition:
+ if (hasNullAnnotation(declaration)) { // don't adjust super if local has no explicit annotation (?)
+ return createChangeOverriddenReturnOperation(compilationUnit, cu, declaration, allowRemove,
+ annotationToAdd, annotationToRemove, annotationNameLabel);
+ }
+ }
+
+ }
+ return null;
+ }
static SignatureAnnotationRewriteOperation createChangeOverriddenParameterOperation(CompilationUnit compilationUnit,
ICompilationUnit cu,
@@ -371,6 +440,27 @@
}
return null;
}
+
+ static boolean hasNullAnnotation(MethodDeclaration decl) {
+ List modifiers = decl.modifiers();
+ String nonnull = QuickFixes.getNonNullAnnotationName(decl.resolveBinding().getJavaElement(), false);
+ String nullable = QuickFixes.getNullableAnnotationName(decl.resolveBinding().getJavaElement(), false);
+ for (Object mod : modifiers) {
+ if (mod instanceof Annotation) {
+ Name annotationName = ((Annotation) mod).getTypeName();
+ String fullyQualifiedName = annotationName.getFullyQualifiedName();
+ if (annotationName.isSimpleName()
+ ? nonnull.endsWith(fullyQualifiedName)
+ : fullyQualifiedName.equals(nonnull))
+ return true;
+ if (annotationName.isSimpleName()
+ ? nullable.endsWith(fullyQualifiedName)
+ : fullyQualifiedName.equals(nullable))
+ return true;
+ }
+ }
+ return false;
+ }
static SignatureAnnotationRewriteOperation createChangeOverriddenReturnOperation(CompilationUnit compilationUnit,
ICompilationUnit cu,
@@ -396,6 +486,9 @@
if (methodDecl == null)
return null;
declaration = (MethodDeclaration) methodDecl;
+// TODO(SH): decide whether we want to propose overwriting existing annotations in super
+// if (hasNullAnnotation(declaration)) // if overridden has explicit declaration don't propose to change it
+// return null;
message = Messages.format(FixMessages.QuickFixes_declare_overridden_return_as_nullable,
new String[] {
annotationNameLabel,