Skip to content

Commit

Permalink
fix: the imports are properly computed after processing (#2083)
Browse files Browse the repository at this point in the history
  • Loading branch information
@surli @monperrus
surli authored and monperrus committed Jun 25, 2018
1 parent 9f72470 commit 0bb8fed
Show file tree
Hide file tree
Showing 9 changed files with 250 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1937,7 +1937,7 @@ public void calculate(CompilationUnit sourceCompilationUnit, List<CtType<?>> typ
this.sourceCompilationUnit = sourceCompilationUnit;
this.imports = new HashSet<>();
if (sourceCompilationUnit != null) {
imports.addAll(sourceCompilationUnit.getImports());
this.importsContext.initWithImports(sourceCompilationUnit.getImports());
}

for (CtType<?> t : types) {
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/spoon/reflect/visitor/ImportScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,9 @@ public interface ImportScanner {
* Checks if the type is already imported.
*/
boolean isImported(CtReference ref);

/**
* Specify the original imports to use before computing new imports.
*/
void initWithImports(Collection<CtImport> importCollection);
}
145 changes: 141 additions & 4 deletions src/main/java/spoon/reflect/visitor/ImportScannerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import spoon.reflect.code.CtFieldAccess;
import spoon.reflect.code.CtFieldRead;
import spoon.reflect.code.CtInvocation;
import spoon.reflect.code.CtJavaDoc;
import spoon.reflect.code.CtJavaDocTag;
import spoon.reflect.code.CtLiteral;
import spoon.reflect.declaration.CtAnnotationType;
import spoon.reflect.declaration.CtClass;
Expand Down Expand Up @@ -76,6 +78,7 @@ public class ImportScannerImpl extends CtScanner implements ImportScanner {
private Map<String, Boolean> namesPresentInJavaLang = new HashMap<>();
private Set<String> fieldAndMethodsNames = new HashSet<String>();
private Set<CtTypeReference> exploredReferences = new HashSet<>(); // list of explored references
private Map<CtImport, Boolean> usedImport = new HashMap<>(); // defined if imports had been used or not

@Override
public <T> void visitCtFieldRead(CtFieldRead<T> fieldRead) {
Expand Down Expand Up @@ -120,6 +123,22 @@ public <T> void visitCtTypeReference(CtTypeReference<T> reference) {
typeReference = reference;
} else {
typeReference = reference.getAccessType();

// in case of a reference to an inner class, the user might want to import it
// instead of importing the access type (which is the default behaviour)
// we cannot guess it: so we check against the existing imports
if (!typeReference.equals(reference)) {
for (CtImport ctImport : this.usedImport.keySet()) {
switch (ctImport.getImportKind()) {
case TYPE:
if (ctImport.getReference().getSimpleName().equals(reference.getSimpleName())) {
this.setImportUsed(ctImport);
super.visitCtTypeReference(reference);
}
}
}

}
}

if (!this.isTypeInCollision(typeReference, false)) {
Expand All @@ -132,11 +151,33 @@ public <T> void visitCtTypeReference(CtTypeReference<T> reference) {

@Override
public void scan(CtElement element) {
if (element != null && !element.isImplicit()) {
if (element != null) {
element.accept(this);
}
}

@Override
public void visitCtJavaDoc(CtJavaDoc ctJavaDoc) {
StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append(ctJavaDoc.getContent());

for (CtJavaDocTag ctJavaDocTag : ctJavaDoc.getTags()) {
stringBuilder.append(ctJavaDocTag.getContent());
}

String javadoc = stringBuilder.toString();
for (CtImport ctImport : this.usedImport.keySet()) {
switch (ctImport.getImportKind()) {
case TYPE:
if (javadoc.contains(ctImport.getReference().getSimpleName())) {
this.setImportUsed(ctImport);
}
break;
}
}

}

@Override
public <A extends Annotation> void visitCtAnnotationType(
CtAnnotationType<A> annotationType) {
Expand Down Expand Up @@ -182,10 +223,27 @@ public <T> void visitCtCatchVariable(CtCatchVariable<T> catchVariable) {
super.visitCtCatchVariable(catchVariable);
}

@Override
public void visitCtInvocation(CtInvocation invocation) {
this.scan(invocation.getTypeCasts());
this.scan(invocation.getExecutable());
if (!this.isImportedInMethodImports(invocation.getExecutable())) {
this.scan(invocation.getTarget());
}

this.scan(invocation.getArguments());
}

@Override
public Collection<CtImport> getAllImports() {
Collection<CtImport> listallImports = new ArrayList<>();

for (Map.Entry<CtImport, Boolean> entry : this.usedImport.entrySet()) {
if (entry.getValue()) {
listallImports.add(entry.getKey());
}
}

for (CtReference reference : this.classImports.values()) {
listallImports.add(reference.getFactory().Type().createImport(reference));
}
Expand Down Expand Up @@ -228,6 +286,13 @@ public boolean isImported(CtReference ref) {
}
}

@Override
public void initWithImports(Collection<CtImport> importCollection) {
for (CtImport ctImport : importCollection) {
this.usedImport.put(ctImport, Boolean.FALSE);
}
}

private boolean isThereAnotherClassWithSameNameInAnotherPackage(CtTypeReference<?> ref) {
for (CtTypeReference typeref : this.exploredReferences) {
if (typeref.getSimpleName().equals(ref.getSimpleName()) && !typeref.getQualifiedName().equals(ref.getQualifiedName())) {
Expand Down Expand Up @@ -324,7 +389,9 @@ protected boolean addClassImport(CtTypeReference<?> ref) {
CtPackageReference pack = targetType.getPackage();
if (pack != null && ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) {
// ignore java.lang package
if (!ref.getPackage().getSimpleName().equals("java.lang")) {
if (ref.getPackage().getSimpleName().equals("java.lang")) {
return false;
} else {
// ignore type in same package
if (ref.getPackage().getSimpleName()
.equals(pack.getSimpleName())) {
Expand All @@ -338,7 +405,32 @@ protected boolean addClassImport(CtTypeReference<?> ref) {
return true;
}

private boolean setImportUsed(CtImport ctImport) {
this.usedImport.put(ctImport, true);
return true;
}

protected boolean isImportedInClassImports(CtTypeReference<?> ref) {
for (CtImport ctImport : this.usedImport.keySet()) {
switch (ctImport.getImportKind()) {
case TYPE:
if (ctImport.getReference().equals(ref)) {
return this.setImportUsed(ctImport);
}
break;

case ALL_TYPES:
String packQualifiedName = ref.getPackage().getQualifiedName();
String typeImportQualifiedName = ctImport.getReference().getSimpleName();

typeImportQualifiedName = typeImportQualifiedName.substring(0, typeImportQualifiedName.lastIndexOf("."));
if (packQualifiedName.equals(typeImportQualifiedName)) {
return this.setImportUsed(ctImport);
}
break;
}
}

if (targetType != null) {
CtPackageReference pack = targetType.getPackage();

Expand Down Expand Up @@ -429,8 +521,8 @@ protected boolean addMethodImport(CtExecutableReference ref) {
if (ref.getFactory().getEnvironment().getComplianceLevel() < 5) {
return false;
}
if (this.methodImports.containsKey(ref.getSimpleName())) {
return isImportedInMethodImports(ref);
if (this.isImportedInMethodImports(ref)) {
return true;
}

// if the whole class is imported: no need to import the method.
Expand All @@ -456,6 +548,30 @@ protected boolean addMethodImport(CtExecutableReference ref) {
}

protected boolean isImportedInMethodImports(CtExecutableReference<?> ref) {
for (CtImport ctImport : this.usedImport.keySet()) {
switch (ctImport.getImportKind()) {
case METHOD:
CtExecutableReference execRef = (CtExecutableReference) ctImport.getReference();
CtTypeReference declaringType = execRef.getDeclaringType();
if (execRef.getSimpleName().equals(ref.getSimpleName()) && declaringType != null && declaringType.equals(ref.getDeclaringType())) {
return this.setImportUsed(ctImport);
}
break;

case ALL_STATIC_MEMBERS:
if (ref.getDeclaringType() != null) {
String typeMethodQualifiedName = ref.getDeclaringType().getQualifiedName();
String typeImportQualifiedName = ctImport.getReference().getSimpleName();

typeImportQualifiedName = typeImportQualifiedName.substring(0, typeImportQualifiedName.lastIndexOf("."));
if (typeMethodQualifiedName.equals(typeImportQualifiedName)) {
return this.setImportUsed(ctImport);
}
}
break;
}
}

if (!(ref.isImplicit()) && methodImports.containsKey(ref.getSimpleName())) {
CtExecutableReference<?> exist = methodImports.get(ref.getSimpleName());
if (getSignature(exist).equals(
Expand Down Expand Up @@ -490,6 +606,27 @@ protected boolean addFieldImport(CtFieldReference ref) {
}

protected boolean isImportedInFieldImports(CtFieldReference<?> ref) {
for (CtImport ctImport : this.usedImport.keySet()) {
switch (ctImport.getImportKind()) {
case FIELD:
if (ctImport.getReference().equals(ref)) {
return this.setImportUsed(ctImport);
}
break;

case ALL_STATIC_MEMBERS:
String typeFieldQualifiedName = ref.getQualifiedName();
String typeImportQualifiedName = ctImport.getReference().getSimpleName();

typeFieldQualifiedName = typeFieldQualifiedName.substring(0, typeFieldQualifiedName.lastIndexOf("."));
typeImportQualifiedName = typeImportQualifiedName.substring(0, typeImportQualifiedName.lastIndexOf("."));
if (typeFieldQualifiedName.equals(typeImportQualifiedName)) {
return this.setImportUsed(ctImport);
}
break;
}
}

if (!(ref.isImplicit()) && fieldImports.containsKey(ref.getSimpleName())) {
CtFieldReference<?> exist = fieldImports.get(ref.getSimpleName());
try {
Expand Down
22 changes: 13 additions & 9 deletions src/test/java/spoon/test/imports/ImportScannerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,26 @@ public void testImportOnSpoon() throws IOException {
Launcher.LOGGER.warn("ImportScannerTest: Import scanner imports " + countUnusedImports + " unused imports and misses " + countMissingImports + " imports");

// Uncomment for the complete list
/*
Set<CtType> keys = new HashSet<>(unusedImports.keySet());
keys.addAll(missingImports.keySet());

for (CtType type : keys) {
Set<CtType> missingKeys = new HashSet<>(missingImports.keySet());

for (CtType type : missingKeys) {
System.err.println(type.getQualifiedName());
if (missingImports.containsKey(type)) {
List<String> imports = missingImports.get(type);
for (String anImport : imports) {
System.err.println("\t" + anImport + " missing");
}
}
}

assertEquals("Import scanner missed " + countMissingImports + " imports",0, countMissingImports);

/*
Set<CtType> unusedKeys = new HashSet<>(unusedImports.keySet());
for (CtType type : unusedKeys) {
System.err.println(type.getQualifiedName());
if (unusedImports.containsKey(type)) {
List<String> imports = unusedImports.get(type);
for (String anImport : imports) {
Expand All @@ -162,9 +170,6 @@ public void testImportOnSpoon() throws IOException {
}
}
*/

assertEquals("Import scanner missed " + countMissingImports + " imports",0, countMissingImports);

// FIXME: the unused imports should be resolved
//assertEquals("Import scanner imports " + countUnusedImports + " unused imports",
// 0, countUnusedImports);
Expand Down Expand Up @@ -235,8 +240,7 @@ public void testComputeImportsInClass() throws Exception {
importContext.computeImports(theClass);
Collection<CtImport> imports = importContext.getAllImports();

// java.lang are also computed
assertEquals(4, imports.size());
assertEquals(2, imports.size());
}

@Test
Expand Down
16 changes: 9 additions & 7 deletions src/test/java/spoon/test/imports/ImportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,16 @@ public void testSpoonWithImports() throws Exception {
importScanner = new ImportScannerImpl();
importScanner.computeImports(anotherClass);
//ClientClass needs 2 imports: ChildClass, PublicInterface2
assertEquals(2, importScanner.getAllImports().size());
Collection<CtImport> allImports = importScanner.getAllImports();
assertEquals(2, allImports.size());

//check that printer did not used the package protected class like "SuperClass.InnerClassProtected"
assertTrue(anotherClass.toString().indexOf("InnerClass extends ChildClass.InnerClassProtected")>0);
importScanner = new ImportScannerImpl();
importScanner.computeImports(classWithInvocation);
// java.lang imports are also computed
assertEquals("Spoon ignores the arguments of CtInvocations", 3, importScanner.getAllImports().size());
allImports = importScanner.getAllImports();
assertEquals("Spoon ignores the arguments of CtInvocations", 1, allImports.size());
}

@Test
Expand Down Expand Up @@ -372,10 +374,9 @@ public void testNotImportExecutableType() throws Exception {

Collection<CtImport> imports = importContext.getAllImports();

// java.lang.Object is considered as imported but it will never be output
assertEquals(3, imports.size());
assertEquals(2, imports.size());
Set<String> expectedImports = new HashSet<>(
Arrays.asList("spoon.test.imports.testclasses.internal3.Foo", "java.io.File", "java.lang.Object"));
Arrays.asList("spoon.test.imports.testclasses.internal3.Foo", "java.io.File"));
Set<String> actualImports = imports.stream().map(CtImport::getReference).map(CtReference::toString).collect(Collectors.toSet());
assertEquals(expectedImports, actualImports);
}
Expand Down Expand Up @@ -1202,7 +1203,7 @@ public void testSortImportPutStaticImportAfterTypeImport() {
int countImports = 0;

int nbStaticImports = 2;
int nbStandardImports = 4;
int nbStandardImports = 3;

boolean startStatic = false;

Expand All @@ -1224,7 +1225,8 @@ public void testSortImportPutStaticImportAfterTypeImport() {
}
}

assertEquals("Exactly "+nbStandardImports+nbStaticImports+" should have been counted.", (nbStandardImports+nbStaticImports), countImports);
int totalImports = nbStandardImports + nbStaticImports;
assertEquals("Exactly "+totalImports+" should have been counted.", (nbStandardImports+nbStaticImports), countImports);
}

@Test
Expand Down
Loading

0 comments on commit 0bb8fed

Please sign in to comment.