Bug 337977: [quick fix] Add quick fixes for null annotations
- Wording tweaks
- Code style
- Bug fixes
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullAnnotationsCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullAnnotationsCleanUp.java
index c24e171..056b0bc 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullAnnotationsCleanUp.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullAnnotationsCleanUp.java
@@ -83,6 +83,9 @@
// TODO(SH): might set depending on this.handledProblemID, not sure about the benefit
result.put(JavaCore.COMPILER_PB_NULL_SPECIFICATION_VIOLATION, JavaCore.WARNING);
result.put(JavaCore.COMPILER_PB_REDUNDANT_NULL_CHECK, JavaCore.WARNING);
+ result.put(JavaCore.COMPILER_PB_NULL_ANNOTATION_INFERENCE_CONFLICT, JavaCore.WARNING);
+ result.put(JavaCore.COMPILER_PB_NULL_UNCHECKED_CONVERSION, JavaCore.WARNING);
+ result.put(JavaCore.COMPILER_PB_MISSING_NONNULL_BY_DEFAULT_ANNOTATION, JavaCore.WARNING);
return result;
}
@@ -124,14 +127,12 @@
*/
public boolean canFix(ICompilationUnit compilationUnit, IProblemLocation problem) {
int id= problem.getProblemId();
-
if (id == this.handledProblemID) {
// FIXME search specifically: return param (which??)
// if (QuickFixes.hasExplicitNullnessAnnotation(compilationUnit, problem.getOffset()))
// return false;
return true;
}
-
return false;
}
@@ -142,7 +143,6 @@
@Override
public int computeNumberOfFixes(CompilationUnit compilationUnit) {
int result= 0;
-
IProblem[] problems= compilationUnit.getProblems();
for (int i= 0; i < problems.length; i++) {
int id= problems[i].getID();
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullFixMessages.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullFixMessages.java
index 3fd05d4..75d4c0b 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullFixMessages.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullFixMessages.java
@@ -21,10 +21,10 @@
public static String NullAnnotationsCleanUp_add_nonnull_annotation;
public static String QuickFixes_add_annotation_change_name;
- public static String QuickFixes_declare_method_parameter_nullness;
- public static String QuickFixes_declare_method_return_nullness;
- public static String QuickFixes_declare_overridden_parameter_as_nonnull;
- public static String QuickFixes_declare_overridden_return_as_nullable;
+ public static String QuickFixes_change_method_parameter_nullness;
+ public static String QuickFixes_change_method_return_nullness;
+ public static String QuickFixes_change_overridden_parameter_nullness;
+ public static String QuickFixes_change_overridden_return_nullness;
static {
// initialize resource bundle
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullFixMessages.properties b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullFixMessages.properties
index 172c9cb..1e48568 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullFixMessages.properties
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullFixMessages.properties
@@ -12,7 +12,7 @@
NullAnnotationsCleanUp_add_nullable_annotation=Add missing @Nullable annotation
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_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}
+QuickFixes_change_method_parameter_nullness=Change parameter type to ''@{0}''
+QuickFixes_change_method_return_nullness=Change return type of ''{0}(..)'' to ''@{1}''
+QuickFixes_change_overridden_parameter_nullness=Change parameter type in overridden ''{0}(..)'' to ''@{1}''
+QuickFixes_change_overridden_return_nullness=Change return type of overridden ''{0}(..)'' to ''@{1}''
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 63dd01c..5770850 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
@@ -112,9 +112,8 @@
}
public static boolean isComplainingAboutArgument(ASTNode selectedNode) {
- if (!(selectedNode instanceof SimpleName)) {
+ if (!(selectedNode instanceof SimpleName))
return false;
- }
SimpleName nameNode= (SimpleName) selectedNode;
IBinding binding= nameNode.resolveBinding();
if (binding.getKind() == IBinding.VARIABLE && ((IVariableBinding) binding).isParameter())
@@ -173,13 +172,11 @@
// Entry for NullAnnotationsCleanup:
public static ICleanUpFix createCleanUp(CompilationUnit compilationUnit, IProblemLocation[] locations, int problemID) {
-
ICompilationUnit cu= (ICompilationUnit) compilationUnit.getJavaElement();
if (!JavaModelUtil.is50OrHigher(cu.getJavaProject()))
return null;
List<CompilationUnitRewriteOperation> operations= new ArrayList<CompilationUnitRewriteOperation>();
-
if (locations == null) {
org.eclipse.jdt.core.compiler.IProblem[] problems= compilationUnit.getProblems();
locations= new IProblemLocation[problems.length];
@@ -190,10 +187,8 @@
}
createAddNullAnnotationOperations(compilationUnit, locations, operations);
-
if (operations.size() == 0)
return null;
-
CompilationUnitRewriteOperation[] operationsArray= operations.toArray(new CompilationUnitRewriteOperation[operations.size()]);
return new MyCURewriteOperationsFix(NullFixMessages.QuickFixes_add_annotation_change_name, compilationUnit, operationsArray);
}
@@ -215,7 +210,13 @@
case IProblem.IllegalReturnNullityRedefinition:
annotationToAdd= nonNullAnnotationName;
annotationToRemove= nullableAnnotationName;
- // all others propose to add @Nullable
+ break;
+ case IProblem.RequiredNonNullButProvidedNull:
+ case IProblem.RequiredNonNullButProvidedPotentialNull:
+ case IProblem.RequiredNonNullButProvidedUnknown:
+ annotationToAdd= nonNullAnnotationName;
+ 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.
CompilationUnitRewriteOperation fix= NullRewriteOperations.createAddAnnotationOperation(compilationUnit, problem, annotationToAdd, annotationToRemove, handledPositions,
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullRewriteOperations.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullRewriteOperations.java
index 7e6e473..99ac1d0 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullRewriteOperations.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/NullRewriteOperations.java
@@ -58,15 +58,10 @@
static abstract class SignatureAnnotationRewriteOperation extends CompilationUnitRewriteOperation {
String fAnnotationToAdd;
-
String fAnnotationToRemove;
-
boolean fAllowRemove;
-
CompilationUnit fUnit;
-
protected String fKey;
-
protected String fMessage;
/* A globally unique key that identifies the position being annotated (for avoiding double annotations). */
@@ -78,7 +73,6 @@
return fUnit;
}
-
boolean checkExisting(List<IExtendedModifier> existingModifiers, ListRewrite listRewrite, TextEditGroup editGroup) {
for (Object mod : existingModifiers) {
if (mod instanceof MarkerAnnotation) {
@@ -211,12 +205,11 @@
ASTNode selectedNode= problem.getCoveringNode(compilationUnit);
if (selectedNode == null)
return null;
-
ASTNode declaringNode= getDeclaringNode(selectedNode);
switch (problem.getProblemId()) {
case IProblem.IllegalDefinitionToNonNullParameter:
-// case IllegalRedefinitionToNonNullParameter:
+// case IllegalRedefinitionToNonNullParameter:
// these affect another method
break;
case IProblem.IllegalReturnNullityRedefinition:
@@ -229,8 +222,6 @@
return null;
}
- SignatureAnnotationRewriteOperation result= null;
- String message= null;
String annotationNameLabel= annotationToAdd;
int lastDot= annotationToAdd.lastIndexOf('.');
if (lastDot != -1)
@@ -250,14 +241,11 @@
ASTNode methodDecl= compilationUnit.findDeclaringNode(methodBinding.getKey());
if (methodDecl == null)
return null;
- message= Messages.format(NullFixMessages.QuickFixes_declare_method_parameter_nullness, annotationNameLabel);
- result= new ParameterAnnotationRewriteOperation(compilationUnit, (MethodDeclaration) methodDecl, annotationToAdd, annotationToRemove, paramIdx, allowRemove, message);
+ String message= Messages.format(NullFixMessages.QuickFixes_change_method_parameter_nullness, annotationNameLabel);
+ return new ParameterAnnotationRewriteOperation(compilationUnit, (MethodDeclaration) methodDecl, annotationToAdd, annotationToRemove, paramIdx, allowRemove, message);
} else if (declaringNode instanceof MethodDeclaration) {
-
// complaint is in signature of this method
-
MethodDeclaration declaration= (MethodDeclaration) declaringNode;
-
switch (problem.getProblemId()) {
case IProblem.ParameterLackingNonNullAnnotation:
case IProblem.ParameterLackingNullableAnnotation:
@@ -269,8 +257,8 @@
if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION) {
String paramName= findAffectedParameterName(selectedNode);
if (paramName != null) {
- message= Messages.format(NullFixMessages.QuickFixes_declare_method_parameter_nullness, annotationNameLabel);
- result= new ParameterAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, paramName, allowRemove, message);
+ String message= Messages.format(NullFixMessages.QuickFixes_change_method_parameter_nullness, annotationNameLabel);
+ return new ParameterAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, paramName, allowRemove, message);
}
}
break;
@@ -283,26 +271,23 @@
if (declaration.getNodeType() == ASTNode.METHOD_DECLARATION) {
String paramName= findAffectedParameterName(selectedNode);
if (paramName != null) {
- message= Messages.format(NullFixMessages.QuickFixes_declare_method_parameter_nullness, annotationNameLabel);
- result= new ParameterAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, paramName, allowRemove, message);
+ String message= Messages.format(NullFixMessages.QuickFixes_change_method_parameter_nullness, annotationNameLabel);
+ return new ParameterAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, paramName, allowRemove, message);
}
}
break;
}
//$FALL-THROUGH$
case IProblem.IllegalReturnNullityRedefinition:
- message= Messages.format(NullFixMessages.QuickFixes_declare_method_return_nullness, annotationNameLabel);
- result= new ReturnAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, allowRemove, message);
- break;
+ String message= Messages.format(NullFixMessages.QuickFixes_change_method_return_nullness, new String[] { declaration.getName().getIdentifier(), annotationNameLabel });
+ return new ReturnAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, allowRemove, message);
}
-
}
- return result;
+ return null;
}
private static SignatureAnnotationRewriteOperation createAddAnnotationToOverriddenOperation(CompilationUnit compilationUnit, IProblemLocation problem, String annotationToAdd,
- String annotationToRemove,
- Set<String> handledPositions, boolean thisUnitOnly, boolean allowRemove) {
+ String annotationToRemove, Set<String> handledPositions, boolean thisUnitOnly, boolean allowRemove) {
ICompilationUnit cu= (ICompilationUnit) compilationUnit.getJavaElement();
if (!JavaModelUtil.is50OrHigher(cu.getJavaProject()))
return null;
@@ -312,7 +297,6 @@
return null;
ASTNode declaringNode= getDeclaringNode(selectedNode);
-
switch (problem.getProblemId()) {
case IProblem.IllegalDefinitionToNonNullParameter:
case IProblem.IllegalRedefinitionToNonNullParameter:
@@ -332,11 +316,8 @@
annotationNameLabel= BasicElementLabels.getJavaElementName(annotationNameLabel);
if (declaringNode instanceof MethodDeclaration) {
-
// complaint is in signature of this method
-
MethodDeclaration declaration= (MethodDeclaration) declaringNode;
-
switch (problem.getProblemId()) {
case IProblem.IllegalDefinitionToNonNullParameter:
case IProblem.IllegalRedefinitionToNonNullParameter:
@@ -346,15 +327,12 @@
return createChangeOverriddenReturnOperation(compilationUnit, cu, declaration, allowRemove, annotationToAdd, annotationToRemove, annotationNameLabel);
}
}
-
}
return null;
}
private static SignatureAnnotationRewriteOperation createChangeOverriddenParameterOperation(CompilationUnit compilationUnit, ICompilationUnit cu, MethodDeclaration declaration,
ASTNode selectedNode, boolean allowRemove, String annotationToAdd, String annotationToRemove, String annotationNameLabel) {
- SignatureAnnotationRewriteOperation result;
- String message;
IMethodBinding methodDeclBinding= declaration.resolveBinding();
if (methodDeclBinding == null)
return null;
@@ -369,15 +347,13 @@
if (methodDecl == null)
return null;
declaration= (MethodDeclaration) methodDecl;
- message= Messages.format(NullFixMessages.QuickFixes_declare_overridden_parameter_as_nonnull,
- new String[] { annotationNameLabel, BasicElementLabels.getJavaElementName(overridden.getDeclaringClass().getName()) });
+ String message= Messages.format(NullFixMessages.QuickFixes_change_overridden_parameter_nullness, new String[] { overridden.getName(), annotationNameLabel });
String paramName= findAffectedParameterName(selectedNode);
- result= new ParameterAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, paramName, allowRemove, message);
- return result;
+ return new ParameterAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, paramName, allowRemove, message);
}
private static String findAffectedParameterName(ASTNode selectedNode) {
- VariableDeclaration argDecl= selectedNode instanceof VariableDeclaration ? (VariableDeclaration) selectedNode : (VariableDeclaration) ASTNodes.getParent(selectedNode,
+ VariableDeclaration argDecl= (selectedNode instanceof VariableDeclaration) ? (VariableDeclaration) selectedNode : (VariableDeclaration) ASTNodes.getParent(selectedNode,
VariableDeclaration.class);
if (argDecl != null)
return argDecl.getName().getIdentifier();
@@ -408,8 +384,6 @@
private static SignatureAnnotationRewriteOperation createChangeOverriddenReturnOperation(CompilationUnit compilationUnit, ICompilationUnit cu, MethodDeclaration declaration, boolean allowRemove,
String annotationToAdd, String annotationToRemove, String annotationNameLabel) {
- SignatureAnnotationRewriteOperation result;
- String message;
IMethodBinding methodDeclBinding= declaration.resolveBinding();
if (methodDeclBinding == null)
return null;
@@ -427,10 +401,8 @@
// 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(NullFixMessages.QuickFixes_declare_overridden_return_as_nullable,
- new String[] { annotationNameLabel, BasicElementLabels.getJavaElementName(overridden.getDeclaringClass().getName()) });
- result= new ReturnAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, allowRemove, message);
- return result;
+ String message= Messages.format(NullFixMessages.QuickFixes_change_overridden_return_nullness, new String[] { overridden.getName(), annotationNameLabel });
+ return new ReturnAnnotationRewriteOperation(compilationUnit, declaration, annotationToAdd, annotationToRemove, allowRemove, message);
}
private static CompilationUnit findCUForMethod(CompilationUnit compilationUnit, ICompilationUnit cu, IMethodBinding methodBinding) {
