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

refactor: change ImportScanner interface to add getAllImports #1339

Merged
merged 3 commits into from
May 31, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ protected void exitCtExpression(CtExpression<?> e) {
*/
public Collection<CtReference> computeImports(CtType<?> type) {
context.currentTopLevel = type;
return importsContext.computeAllImports(context.currentTopLevel);
importsContext.computeAllImports(context.currentTopLevel);
return importsContext.getAllImports();
}

/**
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/spoon/reflect/visitor/ImportScanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public interface ImportScanner {
* Computes import of a {@link spoon.reflect.declaration.CtElement}
*
* @return class imports computed by Spoon, it does not contain static imports
* @deprecated This method signature will change to return void in order to improve encapsulation: Use getAllImports to get the imports after calling it.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the @deprecated should also be outside the javadoc

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

*/
Collection<CtTypeReference<?>> computeImports(CtElement element);

Expand All @@ -40,9 +41,17 @@ public interface ImportScanner {
* (represent a class).
*
* @return imports computed by Spoon, it can be CtTypeReference (for classes), but also CtFieldReference (static field) or CtExecutableReference (static methods)
* @deprecated This method signature will change to return void in order to improve encapsulation: Use getAllImports to get the imports after calling it.
*/
Collection<CtReference> computeAllImports(CtType<?> simpleType);

/**
* Use computeImports or computeAllImports before getting the different imports.
*
* @return the list of computed imports or an empty collection if not imports has been computed.
*/
Collection<CtReference> getAllImports();

/**
* Checks if the type is already imported.
*/
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/spoon/reflect/visitor/ImportScannerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,11 @@ public Collection<CtReference> computeAllImports(CtType<?> simpleType) {
targetType = simpleType.getReference().getTopLevelType();
addClassImport(simpleType.getReference());
scan(simpleType);
return this.getAllImports();
}

@Override
public Collection<CtReference> getAllImports() {
Collection<CtReference> listallImports = new ArrayList<>();
listallImports.addAll(this.classImports.values());
listallImports.addAll(this.fieldImports.values());
Expand Down
14 changes: 11 additions & 3 deletions src/test/java/spoon/test/imports/ImportScannerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import spoon.reflect.declaration.CtType;
import spoon.reflect.factory.Factory;
import spoon.reflect.reference.CtFieldReference;
import spoon.reflect.reference.CtReference;
import spoon.reflect.reference.CtTypeReference;
import spoon.reflect.visitor.ImportScanner;
import spoon.reflect.visitor.ImportScannerImpl;
Expand Down Expand Up @@ -38,7 +39,8 @@ public void testComputeMinimalImportsInClass() throws Exception {
CtType<?> theClass = aFactory.Type().get(qualifiedName);

ImportScanner importContext = new MinimalImportScanner();
Collection<CtTypeReference<?>> imports = importContext.computeImports(theClass);
importContext.computeImports(theClass);
Collection<CtReference> imports = importContext.getAllImports();

assertTrue(imports.isEmpty());
}
Expand All @@ -53,7 +55,8 @@ public void testComputeImportsInClass() throws Exception {
CtType<?> theClass = aFactory.Type().get(qualifiedName);

ImportScanner importContext = new ImportScannerImpl();
Collection<CtTypeReference<?>> imports = importContext.computeImports(theClass);
importContext.computeImports(theClass);
Collection<CtReference> imports = importContext.getAllImports();

// java.lang are also computed
assertEquals(4, imports.size());
Expand All @@ -69,7 +72,8 @@ public void testComputeImportsInClassWithSameName() throws Exception {
CtType<?> theClass = aFactory.Type().get(qualifiedName);

ImportScanner importContext = new ImportScannerImpl();
Collection<CtTypeReference<?>> imports = importContext.computeImports(theClass);
importContext.computeImports(theClass);
Collection<CtReference> imports = importContext.getAllImports();

assertEquals(0, imports.size());
}
Expand Down Expand Up @@ -105,5 +109,9 @@ public void testTargetTypeNull() throws Exception {

ImportScanner importScanner = new MinimalImportScanner();
importScanner.computeImports(fieldRef);

Collection<CtReference> imports = importScanner.getAllImports();

assertEquals(0, imports.size());
}
}
31 changes: 20 additions & 11 deletions src/test/java/spoon/test/imports/ImportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import spoon.reflect.declaration.CtType;
import spoon.reflect.factory.Factory;
import spoon.reflect.reference.CtExecutableReference;
import spoon.reflect.reference.CtReference;
import spoon.reflect.reference.CtTypeReference;
import spoon.reflect.visitor.ImportScanner;
import spoon.reflect.visitor.ImportScannerImpl;
Expand Down Expand Up @@ -248,18 +249,20 @@ public void testSpoonWithImports() throws Exception {
final CtClass<ImportTest> classWithInvocation = launcher.getFactory().Class().get(ClassWithInvocation.class);

ImportScanner importScanner = new ImportScannerImpl();
final Collection<CtTypeReference<?>> imports = importScanner.computeImports(aClass);
assertEquals(2, imports.size());
importScanner.computeImports(aClass);
assertEquals(2, importScanner.getAllImports().size());

importScanner = new ImportScannerImpl();
final Collection<CtTypeReference<?>> imports1 = importScanner.computeImports(anotherClass);
importScanner.computeImports(anotherClass);
//ClientClass needs 2 imports: ChildClass, PublicInterface2
assertEquals(2, imports1.size());
assertEquals(2, importScanner.getAllImports().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();
final Collection<CtTypeReference<?>> imports2 = importScanner.computeImports(classWithInvocation);
importScanner.computeImports(classWithInvocation);
// java.lang imports are also computed
assertEquals("Spoon ignores the arguments of CtInvocations", 3, imports2.size());
assertEquals("Spoon ignores the arguments of CtInvocations", 3, importScanner.getAllImports().size());
}

@Test
Expand Down Expand Up @@ -333,7 +336,9 @@ public void testImportOfInvocationOfPrivateClass() throws Exception {
"./src/test/java/spoon/test/imports/testclasses/Mole.java");

ImportScanner importContext = new ImportScannerImpl();
Collection<CtTypeReference<?>> imports = importContext.computeImports(factory.Class().get(Mole.class));
importContext.computeImports(factory.Class().get(Mole.class));

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

assertEquals(1, imports.size());
assertEquals("spoon.test.imports.testclasses.internal2.Chimichanga", imports.toArray()[0].toString());
Expand All @@ -347,13 +352,15 @@ public void testNotImportExecutableType() throws Exception {
"./src/test/java/spoon/test/imports/testclasses/NotImportExecutableType.java");

ImportScanner importContext = new ImportScannerImpl();
Collection<CtTypeReference<?>> imports = importContext.computeImports(factory.Class().get(NotImportExecutableType.class));
importContext.computeImports(factory.Class().get(NotImportExecutableType.class));

// java.lang.Object is considered as imported but it will never be output
Collection<CtReference> imports = importContext.getAllImports();

// java.lang.Object is considered as imported but it will never be output
assertEquals(3, imports.size());
Set<String> expectedImports = new HashSet<>(
Arrays.asList("spoon.test.imports.testclasses.internal3.Foo", "java.io.File", "java.lang.Object"));
Set<String> actualImports = imports.stream().map(CtTypeReference::toString).collect(Collectors.toSet());
Set<String> actualImports = imports.stream().map(CtReference::toString).collect(Collectors.toSet());
assertEquals(expectedImports, actualImports);
}

Expand All @@ -364,7 +371,9 @@ public void testImportOfInvocationOfStaticMethod() throws Exception {
"./src/test/java/spoon/test/imports/testclasses/Pozole.java");

ImportScanner importContext = new ImportScannerImpl();
Collection<CtTypeReference<?>> imports = importContext.computeImports(factory.Class().get(Pozole.class));
importContext.computeImports(factory.Class().get(Pozole.class));

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

assertEquals(1, imports.size());
assertEquals("spoon.test.imports.testclasses.internal2.Menudo", imports.toArray()[0].toString());
Expand Down