Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ImportCleaner): resolve imports of parent types #4353

Merged
merged 6 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions src/main/java/spoon/reflect/visitor/ImportCleaner.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import spoon.support.Experimental;
import spoon.support.util.ModelList;
import spoon.support.visitor.ClassTypingContext;
import spoon.support.visitor.equals.EqualsVisitor;

import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -176,15 +177,23 @@ void addImport(CtReference ref) {
// we would like to add an import, but we don't know to where
return;
}
if (ref instanceof CtFieldReference<?>
&& !isReferenceToFieldPresentInImports((CtFieldReference<?>) ref)) {
if (ref instanceof CtFieldReference<?> && !isReferencePresentInImports(ref)) {
return;
}
CtTypeReference<?> topLevelTypeRef = typeRef.getTopLevelType();
if (typeRefQNames.contains(topLevelTypeRef.getQualifiedName())) {
//it is reference to a type of this CompilationUnit. Do not add it
return;
}
if (ref instanceof CtTypeReference<?>
&& !isReferencePresentInImports(topLevelTypeRef)
&& topLevelTypeRef != ref
&& !isReferencePresentInImports(ref)) {
// check if a top level type has been imported
// if it has been, we don't need to add a separate import for its subtype
// last condition ensures that if only the subtype has been imported, we do not remove it
return;
}
CtPackageReference packageRef = topLevelTypeRef.getPackage();
if (packageRef == null) {
return;
Expand All @@ -210,10 +219,26 @@ void addImport(CtReference ref) {
}
}

private boolean isReferenceToFieldPresentInImports(CtFieldReference ref) {
private boolean isReferencePresentInImports(CtReference ref) {
return compilationUnit.getImports()
.stream()
.anyMatch(ctImport -> ctImport.getReference() != null && ctImport.getReference().equals(ref));
.anyMatch(ctImport -> ctImport.getReference() != null
&& isEqualAfterSkippingRole(ctImport.getReference(), ref, CtRole.TYPE_ARGUMENT));
}

/**
* Checks if element and other are equal if comparison for `role` value is skipped
*/
private boolean isEqualAfterSkippingRole(CtElement element, CtElement other, CtRole role) {
EqualsVisitor equalsVisitor = new EqualsVisitor();
boolean isEqual = equalsVisitor.checkEquals(element, other);
if (isEqual) {
return true;
}
if (role == equalsVisitor.getNotEqualRole()) {
return true;
}
return false;
}

void onCompilationUnitProcessed(CtCompilationUnit compilationUnit) {
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/spoon/reflect/visitor/ImportCleanerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@

public class ImportCleanerTest {

@Test
void testDoesNotRemoveImportOfSubType() {
// contract: The import cleaner should not remove import of subtype if it is used by its simply qualified name
testImportCleanerDoesNotAlterImports("src/test/resources/importCleaner/DoNotRemoveSubType.java", "importCleaner.DoNotRemoveSubType");
}

@Test
void testDoesNotImportTypeWhoseParentTypeIsAlreadyImported() {
// contract: The import cleaner should not import the subtype if its parent has already been imported
testImportCleanerDoesNotAlterImports("src/test/resources/importCleaner/TypeImportButUseSubType.java", "importCleaner.TypeImportButUseSubType");
}

@Test
void testDoesNotRemoveImportForStaticFieldOfStaticClass() {
// contract: The import cleaner should not remove import of the static field
Expand Down
9 changes: 9 additions & 0 deletions src/test/resources/importCleaner/DoNotRemoveSubType.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package importCleaner;

import java.util.Map.Entry;

public class DoNotRemoveSubType {
public void hey() {
Entry<String, String> f = null;
}
}
Loading