Skip to content

Commit 7c9a92e

Browse files
jjohnstnjjohnstn
authored
Add support for fixing contradictory null annotations (#2921)
- add support for removing contradictory null annotation settings to NullAnnotationsCorrectionProcessor, QuickFixProcessor, NullAnnotationsCorrectionProcessorCore, and NullAnnotationsFixCore - add new remove operation to NullAnnotationsRewriteOperations - add new test to NullAnnotationsQuickFix1d8 - fixes #2919
1 parent a0e0262 commit 7c9a92e

10 files changed

Lines changed: 150 additions & 1 deletion

File tree

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,7 @@ private CorrectionMessages() {
509509
public static String NullAnnotationsCorrectionProcessor_change_local_variable_to_nonNull;
510510
public static String NullAnnotationsCorrectionProcessor_create_packageInfo_with_defaultnullness;
511511
public static String NullAnnotationsCorrectionProcessor_replace_nullable_with_nonnull;
512+
public static String NullAnnotationsCorrectionProcessor_remove_annotation;
512513
public static String PreviewFeaturesSubProcessor_enable_preview_features;
513514
public static String PreviewFeaturesSubProcessor_enable_preview_features_workspace;
514515
public static String PreviewFeaturesSubProcessor_enable_preview_features_info;

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,8 @@ VarargsWarningsSubProcessor_remove_safevarargs_label=Remove @SafeVarargs
542542
NullAnnotationsCorrectionProcessor_change_local_variable_to_nonNull=Declare ''{0}'' as ''@{1}'' to see the root problem
543543
NullAnnotationsCorrectionProcessor_create_packageInfo_with_defaultnullness=Add package-info.java with ''@{0}''
544544
NullAnnotationsCorrectionProcessor_replace_nullable_with_nonnull=Change ''{0}'' to ''@{1}''
545+
NullAnnotationsCorrectionProcessor_remove_annotation=Remove ''@{0}''
546+
545547
PreviewFeaturesSubProcessor_enable_preview_features=Enable preview features on project properties
546548
PreviewFeaturesSubProcessor_enable_preview_features_workspace=Enable preview features on workspace preferences
547549
PreviewFeaturesSubProcessor_enable_preview_features_info=<p>Enable preview features on Java &gt; Compiler properties page

‎org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/NullAnnotationsCorrectionProcessorCore.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,22 @@ public void getRemoveRedundantAnnotationProposal(IInvocationContext context, IPr
142142
proposals.add(fixCorrectionProposalCoreToT(proposal, CORRECTION_CHANGE));
143143
}
144144

145+
public void getRemoveContradictoryAnnotationProposals(IInvocationContext context, IProblemLocation problem, Collection<T> proposals) {
146+
if (problem.getProblemArguments().length == 2) {
147+
NullAnnotationsFixCore fix1= NullAnnotationsFixCore.createRemoveContradictoryNullAnnotationsFix(context.getASTRoot(), problem, problem.getProblemArguments()[0]);
148+
if (fix1 == null)
149+
return;
150+
NullAnnotationsFixCore fix2= NullAnnotationsFixCore.createRemoveContradictoryNullAnnotationsFix(context.getASTRoot(), problem, problem.getProblemArguments()[1]);
151+
if (fix2 == null)
152+
return;
153+
154+
Map<String, String> options= new Hashtable<>();
155+
FixCorrectionProposalCore proposal1= new FixCorrectionProposalCore(fix1, new NullAnnotationsCleanUpCore(options, problem.getProblemId()), IProposalRelevance.REMOVE_REDUNDANT_NULLNESS_ANNOTATION, context);
156+
proposals.add(fixCorrectionProposalCoreToT(proposal1, CORRECTION_CHANGE));
157+
FixCorrectionProposalCore proposal2= new FixCorrectionProposalCore(fix2, new NullAnnotationsCleanUpCore(options, problem.getProblemId()), IProposalRelevance.REMOVE_REDUNDANT_NULLNESS_ANNOTATION, context);
158+
proposals.add(fixCorrectionProposalCoreToT(proposal2, CORRECTION_CHANGE));
159+
}
160+
}
145161
public void getReplaceNullableAnnotationProposal(IInvocationContext context, IProblemLocation problem, Collection<T> proposals) {
146162
NullAnnotationsFixCore fix= NullAnnotationsFixCore.createReplaceNullableFix(context.getASTRoot(), problem);
147163
if (fix == null)

‎org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ private FixMessages() {
148148
public static String NullAnnotationsRewriteOperations_change_overridden_return_nullness;
149149
public static String NullAnnotationsRewriteOperations_remove_redundant_nullness_annotation;
150150
public static String NullAnnotationsRewriteOperations_add_missing_default_nullness_annotation;
151-
151+
public static String NullAnnotationsRewriteOperations_remove_annotation;
152152
public static String ExternalNullAnnotationChangeProposals_add_nullness_annotation;
153153
public static String ExternalNullAnnotationChangeProposals_add_nullness_array_annotation;
154154
public static String ExternalNullAnnotationChangeProposals_remove_nullness_annotation;

‎org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/FixMessages.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ NullAnnotationsRewriteOperations_change_overridden_parameter_nullness=Change par
134134
NullAnnotationsRewriteOperations_change_overridden_return_nullness=Change return type of overridden ''{0}(..)'' to ''@{1}''
135135

136136
NullAnnotationsRewriteOperations_remove_redundant_nullness_annotation=Remove redundant nullness annotation
137+
NullAnnotationsRewriteOperations_remove_annotation=Remove annotation
137138
NullAnnotationsRewriteOperations_add_missing_default_nullness_annotation=Add ''@{0}'' to the package declaration
138139

139140
ExternalNullAnnotationChangeProposals_add_nullness_annotation=Annotate as ''@{0} {1}''

‎org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsFixCore.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@
2626
import org.eclipse.jdt.core.compiler.IProblem;
2727
import org.eclipse.jdt.core.dom.ASTNode;
2828
import org.eclipse.jdt.core.dom.ASTVisitor;
29+
import org.eclipse.jdt.core.dom.AnnotatableType;
2930
import org.eclipse.jdt.core.dom.Annotation;
3031
import org.eclipse.jdt.core.dom.BooleanLiteral;
3132
import org.eclipse.jdt.core.dom.CharacterLiteral;
3233
import org.eclipse.jdt.core.dom.ClassInstanceCreation;
3334
import org.eclipse.jdt.core.dom.CompilationUnit;
3435
import org.eclipse.jdt.core.dom.Expression;
36+
import org.eclipse.jdt.core.dom.FieldDeclaration;
3537
import org.eclipse.jdt.core.dom.IAnnotationBinding;
3638
import org.eclipse.jdt.core.dom.IBinding;
3739
import org.eclipse.jdt.core.dom.IExtendedModifier;
@@ -61,6 +63,7 @@
6163
import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.AddMissingDefaultNullnessRewriteOperation;
6264
import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.Builder;
6365
import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.ChangeKind;
66+
import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.RemoveAnnotationRewriteOperation;
6467
import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.RemoveRedundantAnnotationRewriteOperation;
6568
import org.eclipse.jdt.internal.corext.fix.NullAnnotationsRewriteOperations.SignatureAnnotationRewriteOperation;
6669
import org.eclipse.jdt.internal.corext.fix.TypeAnnotationRewriteOperations.MoveTypeAnnotationRewriteOperation;
@@ -397,6 +400,52 @@ private static void createRemoveRedundantNullAnnotationsOperations(CompilationUn
397400
}
398401
}
399402

403+
public static NullAnnotationsFixCore createRemoveContradictoryNullAnnotationsFix(CompilationUnit astRoot, IProblemLocation problem, String annotationName) {
404+
ASTNode coveringNode= problem.getCoveringNode(astRoot);
405+
if (coveringNode == null) {
406+
return null;
407+
}
408+
coveringNode= coveringNode.getParent();
409+
410+
List<IExtendedModifier> modifiers= new ArrayList<>();
411+
Annotation removeAnnotation= null;
412+
413+
if (coveringNode instanceof SingleVariableDeclaration) {
414+
SingleVariableDeclaration singleVariableDeclaration= (SingleVariableDeclaration) coveringNode;
415+
modifiers= singleVariableDeclaration.modifiers();
416+
} else if (coveringNode instanceof FieldDeclaration) {
417+
FieldDeclaration fieldDeclaration= (FieldDeclaration) coveringNode;
418+
modifiers= fieldDeclaration.modifiers();
419+
} else if (coveringNode instanceof MethodDeclaration) {
420+
MethodDeclaration methodDeclaration= (MethodDeclaration) coveringNode;
421+
modifiers= methodDeclaration.modifiers();
422+
} else if (coveringNode instanceof AnnotatableType annotatableType) {
423+
modifiers= annotatableType.annotations();
424+
} else {
425+
return null;
426+
}
427+
428+
for (IExtendedModifier modifier : modifiers) {
429+
if (modifier instanceof Annotation annotation) {
430+
IAnnotationBinding binding= annotation.resolveAnnotationBinding();
431+
if (binding != null) {
432+
if (binding.getAnnotationType().getQualifiedName().equals(annotationName)) {
433+
removeAnnotation= annotation;
434+
break;
435+
}
436+
}
437+
}
438+
}
439+
440+
if (removeAnnotation != null) {
441+
RemoveAnnotationRewriteOperation op= new RemoveAnnotationRewriteOperation(removeAnnotation);
442+
String label= Messages.format(CorrectionMessages.NullAnnotationsCorrectionProcessor_remove_annotation,
443+
new Object[] {removeAnnotation.getTypeName().getFullyQualifiedName()});
444+
return new NullAnnotationsFixCore(label, astRoot,
445+
new CompilationUnitRewriteOperationWithSourceRange[] { op });
446+
}
447+
return null;
448+
}
400449
// private static boolean isMissingNullAnnotationProblem(int id) {
401450
// return id == IProblem.RequiredNonNullButProvidedNull || id == IProblem.RequiredNonNullButProvidedPotentialNull || id == IProblem.IllegalReturnNullityRedefinition
402451
// || mayIndicateParameterNullcheck(id);
@@ -476,4 +525,5 @@ public boolean visit(MethodDeclaration node) {
476525
root.accept(visitor);
477526
return visitor.getMethodDeclaration();
478527
}
528+
479529
}

‎org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsRewriteOperations.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,24 @@ public void rewriteASTInternal(CompilationUnitRewrite cuRewrite, LinkedProposalM
390390
}
391391
}
392392

393+
static class RemoveAnnotationRewriteOperation extends CompilationUnitRewriteOperationWithSourceRange {
394+
395+
private Annotation fAnnotation;
396+
public RemoveAnnotationRewriteOperation(Annotation annotation) {
397+
fAnnotation= annotation;
398+
}
399+
400+
@Override
401+
public void rewriteASTInternal(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore linkedModel) throws CoreException {
402+
TextEditGroup group= createTextEditGroup(FixMessages.NullAnnotationsRewriteOperations_remove_annotation, cuRewrite);
403+
ASTRewrite astRewrite= cuRewrite.getASTRewrite();
404+
ImportRemover remover= cuRewrite.getImportRemover();
405+
406+
remover.registerRemovedNode(fAnnotation);
407+
astRewrite.remove(fAnnotation, group);
408+
}
409+
}
410+
393411
static class AddMissingDefaultNullnessRewriteOperation extends CompilationUnitRewriteOperationWithSourceRange {
394412

395413
private IProblemLocation fProblem;

‎org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationsQuickFixTest1d8.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2098,4 +2098,57 @@ public void invokeMe(@NonNull String aaa) {
20982098
""");
20992099
}
21002100

2101+
@Test
2102+
public void testGH2919() throws Exception {
2103+
IPackageFragment my= fSourceFolder.createPackageFragment("my", false, null);
2104+
ICompilationUnit cu= my.createCompilationUnit("Test.java",
2105+
"""
2106+
package my;
2107+
import org.eclipse.jdt.annotation.NonNull;
2108+
import org.eclipse.jdt.annotation.Nullable;
2109+
public class Test {
2110+
public @Nullable @NonNull String invokeMe(String aaa) {
2111+
return "abc";
2112+
}
2113+
}
2114+
""",
2115+
false, null);
2116+
2117+
CompilationUnit astRoot= getASTRoot(cu);
2118+
ArrayList<IJavaCompletionProposal> proposals= collectCorrections(cu, astRoot);
2119+
assertNumberOfProposals(proposals, 3);
2120+
CUCorrectionProposal proposal= (CUCorrectionProposal) proposals.get(0);
2121+
2122+
assertEqualString(proposal.getDisplayString(), "Remove '@NonNull'");
2123+
2124+
String preview= getPreviewContent(proposal);
2125+
2126+
assertEqualString(preview,
2127+
"""
2128+
package my;
2129+
import org.eclipse.jdt.annotation.Nullable;
2130+
public class Test {
2131+
public @Nullable String invokeMe(String aaa) {
2132+
return "abc";
2133+
}
2134+
}
2135+
""");
2136+
CUCorrectionProposal proposal2= (CUCorrectionProposal) proposals.get(1);
2137+
2138+
assertEqualString(proposal2.getDisplayString(), "Remove '@Nullable'");
2139+
2140+
preview= getPreviewContent(proposal2);
2141+
2142+
assertEqualString(preview,
2143+
"""
2144+
package my;
2145+
import org.eclipse.jdt.annotation.NonNull;
2146+
public class Test {
2147+
public @NonNull String invokeMe(String aaa) {
2148+
return "abc";
2149+
}
2150+
}
2151+
""");
2152+
}
2153+
21012154
}

‎org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/NullAnnotationsCorrectionProcessor.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ public static void addRemoveRedundantAnnotationProposal(IInvocationContext conte
6060
new NullAnnotationsCorrectionProcessor().getRemoveRedundantAnnotationProposal(context, problem, proposals);
6161
}
6262

63+
public static void addRemoveContradictoryAnnotationProposals(IInvocationContext context, IProblemLocation problem, Collection<ICommandAccess> proposals) {
64+
new NullAnnotationsCorrectionProcessor().getRemoveContradictoryAnnotationProposals(context, problem, proposals);
65+
}
66+
6367
public static void addReplaceNullableAnnotationProposal(IInvocationContext context, IProblemLocation problem, Collection<ICommandAccess> proposals) {
6468
new NullAnnotationsCorrectionProcessor().getReplaceNullableAnnotationProposal(context, problem, proposals);
6569
}

‎org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickFixProcessor.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ public boolean hasCorrections(ICompilationUnit cu, int problemId) {
310310
case IProblem.IllegalAnnotationForBaseType:
311311
case IProblem.MissingNonNullByDefaultAnnotationOnPackage:
312312
case IProblem.NullityMismatchTypeArgument:
313+
case IProblem.ContradictoryNullAnnotations:
313314
case IProblem.UndefinedModule:
314315
case IProblem.PackageDoesNotExistOrIsEmpty:
315316
case IProblem.NotAccessibleType:
@@ -903,6 +904,9 @@ private void process(IInvocationContext context, IProblemLocation problem, Colle
903904
case IProblem.NullableFieldReference:
904905
NullAnnotationsCorrectionProcessor.addExtractCheckedLocalProposal(context, problem, proposals);
905906
break;
907+
case IProblem.ContradictoryNullAnnotations:
908+
NullAnnotationsCorrectionProcessor.addRemoveContradictoryAnnotationProposals(context, problem, proposals);
909+
break;
906910
case IProblem.ConflictingNullAnnotations:
907911
case IProblem.ConflictingInheritedNullAnnotations:
908912
NullAnnotationsCorrectionProcessor.addReturnAndArgumentTypeProposal(context, problem, ChangeKind.LOCAL, proposals);

0 commit comments

Comments
 (0)