Skip to content

Commit

Permalink
Fix adding null annotation to qualified type of parameter (#1463)
Browse files Browse the repository at this point in the history 
* Fix adding null annotation to qualified type of parameter

- fix
  NullAnnotationsRewriteOperations.ParameterAnnotationRewriteOperation
  to check for a SimpleType with qualified name or a NameQualifiedType
  and end up with a NameQualifiedType with the new annotation added
- add new tests to NullAnnotationsQuickFixTest
- fixes #1462
  • Loading branch information
@jjohnstn
jjohnstn authored Jun 19, 2024
1 parent e036782 commit b57eccf
Show file tree
Hide file tree
Showing 2 changed files with 202 additions and 33 deletions.
184 changes: 163 additions & 21 deletions ...clipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/NullAnnotationsQuickFixTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2012, 2020 GK Software AG and others.
* Copyright (c) 2012, 2024 GK Software AG and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -938,9 +938,9 @@ void foo(Exception e1) {

String str2= """
package test1;
import org.eclipse.jdt.annotation.Nullable;
public class E2 extends E {
void foo(@Nullable Exception e1) {
e1.printStackTrace();
Expand Down Expand Up @@ -988,9 +988,9 @@ void foo(Exception e1) {

String str2= """
package test1;
import org.eclipse.jdt.annotation.NonNull;
public class E2 extends E {
void foo(@NonNull Exception e1) {
e1.printStackTrace();
Expand Down Expand Up @@ -1105,9 +1105,9 @@ void test(E e, Object in) {

String str2= """
package test1;
import org.eclipse.jdt.annotation.NonNull;
public class E2 {
void test(E e, @NonNull Object in) {
e.foo(in);
Expand Down Expand Up @@ -1245,9 +1245,9 @@ void foo(@org.eclipse.jdt.annotation.NonNull Object o) {

String str1= """
package test1;
import org.eclipse.jdt.annotation.Nullable;
public class E {
void foo(@Nullable Object o) {
if (o == null) System.out.print("NOK");
Expand Down Expand Up @@ -1460,9 +1460,9 @@ Object foo() {

String str2= """
package test1;
import org.eclipse.jdt.annotation.NonNull;
public class E2 extends E {
@NonNull
Object foo() {
Expand Down Expand Up @@ -1522,9 +1522,9 @@ public Object foo() {

String str3= """
package test1;
import org.eclipse.jdt.annotation.Nullable;
public class E2 extends E implements IE {
public @Nullable Object foo() {
return this;
Expand All @@ -1541,9 +1541,9 @@ public class E2 extends E implements IE {

String str4= """
package test1;
import org.eclipse.jdt.annotation.NonNull;
public class E2 extends E implements IE {
public @NonNull Object foo() {
return this;
Expand Down Expand Up @@ -1675,9 +1675,9 @@ public Object foo(E e) {

String str3= """
package test1;
import org.eclipse.jdt.annotation.Nullable;
public class E2 {
public @Nullable Object foo(E e) {
return e.bar();
Expand Down Expand Up @@ -1754,9 +1754,9 @@ public Object foo(E e) {

String str2= """
package test1;
import org.eclipse.jdt.annotation.Nullable;
public class E2 {
public @Nullable Object foo(E e) {
return e.bar();
Expand Down Expand Up @@ -2085,9 +2085,9 @@ public <T extends Number> double foo(boolean b) {

String str1= """
package test1;
import org.eclipse.jdt.annotation.NonNull;
public class E {
public <T extends Number> double foo(boolean b) {
@NonNull
Expand Down Expand Up @@ -2536,6 +2536,148 @@ public class E2 {
}
}
@Test
public void testBug513423a() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
String str= """
package test1;
import org.eclipse.jdt.annotation.NonNullByDefault;
@NonNullByDefault
public class E extends RuntimeException {
private static final long serialVersionUID = 1L;
public void printStackTrace(
// Illegal redefinition of parameter s, inherited method from Throwable
// does not constrain this parameter
java.io.PrintStream s) {
if (s != null) {
synchronized (s) {
s.print(getClass().getName() + ": ");
s.print(getStackTrace());
}
}
}
}
""";
ICompilationUnit cu= pack1.createCompilationUnit("E.java", str, false, null);

CompilationUnit astRoot= getASTRoot(cu);
ArrayList<IJavaCompletionProposal> proposals= collectCorrections(cu, astRoot, 2);
assertNumberOfProposals(proposals, 2); // ignore 2nd ("add @SW")

CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);

assertEqualString(proposal.getDisplayString(), "Change parameter 's' to '@Nullable'");

String preview= getPreviewContent(proposal);

String str1= """
package test1;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
@NonNullByDefault
public class E extends RuntimeException {
private static final long serialVersionUID = 1L;
public void printStackTrace(
// Illegal redefinition of parameter s, inherited method from Throwable
// does not constrain this parameter
java.io.@Nullable PrintStream s) {
if (s != null) {
synchronized (s) {
s.print(getClass().getName() + ": ");
s.print(getStackTrace());
}
}
}
}
""";
assertEqualString(preview, str1);

}
@Test
public void testBug513423b() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
String str0= """
package test1;
import static java.lang.annotation.ElementType.TYPE_USE;
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentinPolicy;
import java.lang.annotation.Target;
@Documented
@Retention(RetentionPolicy.CLASS)
@Target({ TYPE_USE })
public @interface SomeAnnotation {
// marker annotation with no members
}
""";
pack1.createCompilationUnit("SomeAnnotation.java", str0, false, null);

String str= """
package test1;
import org.eclipse.jdt.annotation.NonNullByDefault;
@NonNullByDefault
public class E extends RuntimeException {
private static final long serialVersionUID = 1L;
public void printStackTrace(
// Illegal redefinition of parameter s, inherited method from Throwable
// does not constrain this parameter
java.io.@SomeAnnotation PrintStream s) {
if (s != null) {
synchronized (s) {
s.print(getClass().getName() + ": ");
s.print(getStackTrace());
}
}
}
}
""";
ICompilationUnit cu= pack1.createCompilationUnit("E.java", str, false, null);

CompilationUnit astRoot= getASTRoot(cu);
ArrayList<IJavaCompletionProposal> proposals= collectCorrections(cu, astRoot, 2);
assertNumberOfProposals(proposals, 2); // ignore 2nd ("add @SW")

CUCorrectionProposal proposal= (CUCorrectionProposal)proposals.get(0);

assertEqualString(proposal.getDisplayString(), "Change parameter 's' to '@Nullable'");

String preview= getPreviewContent(proposal);

String str1= """
package test1;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
@NonNullByDefault
public class E extends RuntimeException {
private static final long serialVersionUID = 1L;
public void printStackTrace(
// Illegal redefinition of parameter s, inherited method from Throwable
// does not constrain this parameter
java.io.@SomeAnnotation @Nullable PrintStream s) {
if (s != null) {
synchronized (s) {
s.print(getClass().getName() + ": ");
s.print(getStackTrace());
}
}
}
}
""";
assertEqualString(preview, str1);

}
@Test
public void testGH1294() throws Exception {
IPackageFragment my= fSourceFolder.createPackageFragment("my", false, null);
ICompilationUnit cu= my.createCompilationUnit("Test.java",
Expand Down
51 changes: 39 additions & 12 deletions .../core extension/org/eclipse/jdt/internal/corext/fix/NullAnnotationsRewriteOperations.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2011, 2018 GK Software AG and others.
* Copyright (c) 2011, 2024 GK Software AG and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -51,8 +51,11 @@
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.NameQualifiedType;
import org.eclipse.jdt.core.dom.PackageDeclaration;
import org.eclipse.jdt.core.dom.QualifiedName;
import org.eclipse.jdt.core.dom.SimpleName;
import org.eclipse.jdt.core.dom.SimpleType;
import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
import org.eclipse.jdt.core.dom.StructuralPropertyDescriptor;
import org.eclipse.jdt.core.dom.Type;
Expand Down Expand Up @@ -286,19 +289,43 @@ private ParameterAnnotationRewriteOperation(IMethodBinding methodBinding, List<?
@Override
public void rewriteASTInternal(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore linkedModel) throws CoreException {
AST ast= cuRewrite.getRoot().getAST();
ListRewrite listRewrite= getAnnotationListRewrite(fArgument.getType(), cuRewrite, fArgument, SingleVariableDeclaration.MODIFIERS2_PROPERTY);
Type argType= fArgument.getType();
TextEditGroup group= createTextEditGroup(fMessage, cuRewrite);
if (!checkExisting(listRewrite, group))
return;
if (!fRequireExplicitAnnotation) {
if (hasNonNullDefault(fArgument, fArgument.resolveBinding(), fParameterRank, DefaultLocation.PARAMETER))
return; // should be safe, as in this case checkExisting() should've already produced a change (remove existing annotation).
if (argType instanceof SimpleType simpleType && simpleType.getName().isQualifiedName()) {
Name name2= simpleType.getName();
QualifiedName qualifiedName= (QualifiedName) name2;
ASTRewrite astRewrite= cuRewrite.getASTRewrite();
Name qualifier= (Name) astRewrite.createMoveTarget(qualifiedName.getQualifier());
SimpleName name= (SimpleName) astRewrite.createMoveTarget(qualifiedName.getName());
NameQualifiedType nameQualifiedType= astRewrite.getAST().newNameQualifiedType(qualifier, name);
Annotation newAnnotation= ast.newMarkerAnnotation();
ImportRewrite importRewrite= cuRewrite.getImportRewrite();
String resolvableName= importRewrite.addImport(fAnnotationToAdd);
newAnnotation.setTypeName(ast.newName(resolvableName));
nameQualifiedType.annotations().add(newAnnotation);
astRewrite.replace(simpleType, nameQualifiedType, group);
} else if (argType instanceof NameQualifiedType nameQualifiedType) {
ASTRewrite astRewrite= cuRewrite.getASTRewrite();
ListRewrite listRewrite= astRewrite.getListRewrite(nameQualifiedType, NameQualifiedType.ANNOTATIONS_PROPERTY);
Annotation newAnnotation= ast.newMarkerAnnotation();
ImportRewrite importRewrite= cuRewrite.getImportRewrite();
String resolvableName= importRewrite.addImport(fAnnotationToAdd);
newAnnotation.setTypeName(ast.newName(resolvableName));
listRewrite.insertLast(newAnnotation, group);
} else {
ListRewrite listRewrite= getAnnotationListRewrite(fArgument.getType(), cuRewrite, fArgument, SingleVariableDeclaration.MODIFIERS2_PROPERTY);
if (!checkExisting(listRewrite, group))
return;
if (!fRequireExplicitAnnotation) {
if (hasNonNullDefault(fArgument, fArgument.resolveBinding(), fParameterRank, DefaultLocation.PARAMETER))
return; // should be safe, as in this case checkExisting() should've already produced a change (remove existing annotation).
}
Annotation newAnnotation= ast.newMarkerAnnotation();
ImportRewrite importRewrite= cuRewrite.getImportRewrite();
String resolvableName= importRewrite.addImport(fAnnotationToAdd);
newAnnotation.setTypeName(ast.newName(resolvableName));
listRewrite.insertLast(newAnnotation, group); // null annotation is last modifier, directly preceding the type
}
Annotation newAnnotation= ast.newMarkerAnnotation();
ImportRewrite importRewrite= cuRewrite.getImportRewrite();
String resolvableName= importRewrite.addImport(fAnnotationToAdd);
newAnnotation.setTypeName(ast.newName(resolvableName));
listRewrite.insertLast(newAnnotation, group); // null annotation is last modifier, directly preceding the type
}
}

Expand Down

0 comments on commit b57eccf

Please sign in to comment.