Skip to content

Commit

Permalink
fix: indeed call the preprocessors in toString/autoimports (#3103)
Browse files Browse the repository at this point in the history
  • Loading branch information
monperrus authored and nharrand committed Sep 16, 2019
1 parent d205f86 commit 408d76d
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 41 deletions.
39 changes: 22 additions & 17 deletions src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
import spoon.reflect.visitor.printer.CommentOffset;

import java.lang.annotation.Annotation;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -177,7 +178,7 @@ public class DefaultJavaPrettyPrinter implements CtVisitor, PrettyPrinter {
/**
* Handle imports of classes.
*/
protected List<Processor<CtCompilationUnit>> preprocessors;
protected final List<Processor<CtElement>> preprocessors = new ArrayList<>();

/**
* Environment which Spoon is executed.
Expand Down Expand Up @@ -315,14 +316,11 @@ protected void exit(CtElement e) {
}

@Override
public DefaultJavaPrettyPrinter prettyprint(CtElement e) {
CtType<?> parent = e.getParent(CtType.class);
if (parent != null) {
// call the validators
calculate(e.getFactory().createCompilationUnit(), Arrays.asList(new CtType<?>[]{parent}));
}
public String prettyprint(CtElement e) {
reset();
return scan(e);
applyPreProcessors(e);
scan(e);
return this.getResult();
}


Expand Down Expand Up @@ -938,7 +936,11 @@ public <T> void visitUnresolvedImport(CtUnresolvedImport ctUnresolvedImport) {
}

private void writeImportReference(CtTypeReference<?> ref) {
boolean prevIgnoreImplicit = ignoreImplicit;
// force fqn, import are never short
ignoreImplicit = true;
visitCtTypeReference(ref, false);
ignoreImplicit = prevIgnoreImplicit;
}


Expand Down Expand Up @@ -1910,16 +1912,18 @@ public <T> void visitCtUnboundVariableReference(CtUnboundVariableReference<T> re
@Override
public String printCompilationUnit(CtCompilationUnit compilationUnit) {
reset();
List<Processor<CtCompilationUnit>> preprocessors = getPreprocessors();
if (preprocessors != null) {
for (Processor<CtCompilationUnit> preprocessor : preprocessors) {
preprocessor.process(compilationUnit);
}
}
applyPreProcessors(compilationUnit);
scanCompilationUnit(compilationUnit);
return getResult();
}

/** Warning, this may change the state of the object */
public void applyPreProcessors(CtElement el) {
for (Processor<CtElement> preprocessor : preprocessors) {
preprocessor.process(el);
}
}

protected void scanCompilationUnit(CtCompilationUnit compilationUnit) {
scan(compilationUnit);
}
Expand Down Expand Up @@ -2050,14 +2054,15 @@ private PrinterHelper getPrinterHelper() {
/**
* @param preprocessors list of {@link CompilationUnitValidator}, which have to be used to validate and fix model before it's printing
*/
public void setPreprocessors(List<Processor<CtCompilationUnit>> preprocessors) {
this.preprocessors = preprocessors;
public void setPreprocessors(List<Processor<CtElement>> preprocessors) {
this.preprocessors.clear();
this.preprocessors.addAll(preprocessors);
}

/**
* @return list of {@link CompilationUnitValidator}, which are used to validate and fix model before it's printing
*/
public List<Processor<CtCompilationUnit>> getPreprocessors() {
public List<Processor<CtElement>> getPreprocessors() {
return this.preprocessors;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ protected boolean isTypeReferenceToEnclosingType(LexicalScope nameScope, CtTypeR
}

private boolean isSupertypeOfNewClass(CtTypeReference<?> typeRef) {
if (!typeRef.isParentInitialized()) {
return false;
}
CtElement parent = typeRef.getParent();
if (parent instanceof CtClass && ((CtClass) parent).getSuperclass() == typeRef) {
CtElement parent2 = parent.getParent();
Expand Down
12 changes: 8 additions & 4 deletions src/main/java/spoon/reflect/visitor/ImportAnalyzer.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,22 @@
*
*/
@Experimental
abstract class ImportAnalyzer<T extends CtScanner, U> extends AbstractProcessor<CtCompilationUnit> {
abstract class ImportAnalyzer<T extends CtScanner, U> extends AbstractProcessor<CtElement> {

@Override
public void process(CtCompilationUnit cu) {
public void process(CtElement el) {
T scanner = createScanner();
if (scanner instanceof EarlyTerminatingScanner) {
CtScannerListener listener = createScannerListener(scanner);
if (listener != null) {
((EarlyTerminatingScanner) scanner).setListener(listener);
}
}
process(scanner, cu);
if (el instanceof CtCompilationUnit) {
process(scanner, (CtCompilationUnit) el);
} else {
scanner.scan(el);
}
}

protected static void process(CtScanner scanner, CtCompilationUnit cu) {
Expand Down Expand Up @@ -149,7 +153,7 @@ public ScanningMode enter(CtRole role, CtElement element) {
}
if (parent instanceof CtExecutableReference) {
CtElement parent2 = null;
if (element.isParentInitialized()) {
if (parent.isParentInitialized()) {
parent2 = parent.getParent();
}
if (parent2 instanceof CtConstructorCall<?>) {
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/spoon/reflect/visitor/ImportCleaner.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ protected Context getScannerContextInformation(ImportCleanerScanner scanner) {

@Override
protected void handleTargetedExpression(CtTargetedExpression<?, ?> targetedExpression, Context context, CtRole role) {
if (context == null) {
return;
}
CtExpression<?> target = targetedExpression.getTarget();
if (target != null && target.isImplicit()) {
if (target instanceof CtTypeAccess) {
Expand All @@ -84,6 +87,9 @@ protected void handleTargetedExpression(CtTargetedExpression<?, ?> targetedExpre

@Override
protected void handleTypeReference(CtTypeReference<?> reference, Context context, CtRole role) {
if (context == null) {
return;
}
if (reference.isImplicit()) {
/*
* the reference is implicit. E.g. `assertTrue();`
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/spoon/reflect/visitor/LexicalScopeScanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ public <R> void visitCtBlock(CtBlock<R> block) {
}
@Override
public <T> void visitCtLocalVariable(CtLocalVariable<T> localVariable) {
if (parent == null) {
return;
}
parent.addNamedElement(localVariable);
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/spoon/reflect/visitor/PrettyPrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,5 @@ public interface PrettyPrinter {
Map<Integer, Integer> getLineNumberMapping();

/** pretty-prints the element, call {@link #toString()} to get the result */
PrettyPrinter prettyprint(CtElement ctElement);
String prettyprint(CtElement ctElement);
}
5 changes: 2 additions & 3 deletions src/main/java/spoon/support/StandardEnvironment.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import spoon.processing.Processor;
import spoon.processing.ProcessorProperties;
import spoon.reflect.cu.SourcePosition;
import spoon.reflect.declaration.CtCompilationUnit;
import spoon.reflect.declaration.CtElement;
import spoon.reflect.declaration.CtExecutable;
import spoon.reflect.declaration.CtType;
Expand Down Expand Up @@ -645,7 +644,7 @@ public void setCompressionType(CompressionType serializationType) {
@Override
public PrettyPrinter createPrettyPrinterAutoImport() {
DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(this);
List<Processor<CtCompilationUnit>> preprocessors = Collections.unmodifiableList(Arrays.<Processor<CtCompilationUnit>>asList(
List<Processor<CtElement>> preprocessors = Collections.unmodifiableList(Arrays.<Processor<CtElement>>asList(
//try to import as much types as possible
new ForceImportProcessor(),
//remove unused imports first. Do not add new imports at time when conflicts are not resolved
Expand Down Expand Up @@ -674,7 +673,7 @@ public PrettyPrinter createPrettyPrinter() {

if (PRETTY_PRINTING_MODE.FULLYQUALIFIED.equals(prettyPrintingMode)) {
DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(this);
List<Processor<CtCompilationUnit>> preprocessors = Collections.unmodifiableList(Arrays.<Processor<CtCompilationUnit>>asList(
List<Processor<CtElement>> preprocessors = Collections.unmodifiableList(Arrays.<Processor<CtElement>>asList(
//force fully qualified
new ForceFullyQualifiedProcessor(),
//solve conflicts, the current imports are relevant too
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ public void enter(CtElement e) {
@Override
public String prettyprint() {
PrettyPrinter printer = getFactory().getEnvironment().createPrettyPrinterAutoImport();
return printer.prettyprint(this).getResult();
return printer.prettyprint(this);
}

@Override
Expand All @@ -305,6 +305,10 @@ public String toString() {
clone.setParent(this.getParent());
}

if (getFactory().getEnvironment().getPrettyPrintingMode().equals(Environment.PRETTY_PRINTING_MODE.AUTOIMPORT)) {
printer.applyPreProcessors(clone);
}

printer.scan(clone);
} catch (ParentNotInitializedException ignore) {
LOGGER.error(ERROR_MESSAGE_TO_STRING, ignore);
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/spoon/test/imports/ImportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1621,9 +1621,9 @@ public void testMethodChainAutoImports() {
List<CtStatement> statements = ctor.getBody().getStatements();

assertEquals("super(context, attributeSet)", statements.get(0).toString());
assertEquals("mButton = ((Button) (findViewById(R.id.page_button_button)))", statements.get(1).toString());
assertEquals("mCurrentActiveColor = getColor(R.color.c4_active_button_color)", statements.get(2).toString());
assertEquals("mCurrentActiveColor = getResources().getColor(R.color.c4_active_button_color)", statements.get(3).toString());
assertEquals("mCurrentActiveColor = getData().getResources().getColor(R.color.c4_active_button_color)", statements.get(4).toString());
assertEquals("mButton = ((Button) (findViewById(page_button_button)))", statements.get(1).toString());
assertEquals("mCurrentActiveColor = getColor(c4_active_button_color)", statements.get(2).toString());
assertEquals("mCurrentActiveColor = getResources().getColor(c4_active_button_color)", statements.get(3).toString());
assertEquals("mCurrentActiveColor = getData().getResources().getColor(c4_active_button_color)", statements.get(4).toString());
}
}
3 changes: 2 additions & 1 deletion src/test/java/spoon/test/initializers/InitializerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ public void testModelBuildingInitializerNoclasspath() {
CtClass<?> ctClass = model.getElements(new NamedElementFilter<>(CtClass.class, "Utf8HttpResponse")).get(0);

CtAnonymousExecutable ex = ctClass.getElements(new TypeFilter<>(CtAnonymousExecutable.class)).get(0);
assertEquals("org.apache.lucene.util.UnicodeUtil.UTF8Result temp = new org.apache.lucene.util.UnicodeUtil.UTF8Result()",
// we are indeed in autoimport
assertEquals("UnicodeUtil.UTF8Result temp = new UnicodeUtil.UTF8Result()",
ex.getBody().getStatements().get(0).toString());
assertEquals("temp.result = new byte[0]",
ex.getBody().getStatements().get(1).toString());
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/spoon/test/loop/LoopTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ public void testForeachShouldHaveAlwaysABlockInItsBody() throws Exception {
// contract: the implicit block is not pretty printed
String expectedPrettyPrinted = //
"for (Condition<? super T> condition : conditions)" + nl //
+ " this.conditions.add(notNull(condition));" + nl;
+ " conditions.add(notNull(condition));" + nl;
assertEquals(expectedPrettyPrinted, ctLoop.prettyprint());


// contract: the implicit block is viewable in debug mode
String expectedDebug = //
"for (spoon.test.loop.testclasses.Condition<? super T> condition : conditions) {" + nl //
+ " this.conditions.add(spoon.test.loop.testclasses.Join.notNull(condition));" + nl + "}";
+ " conditions.add(spoon.test.loop.testclasses.Join.notNull(condition));" + nl + "}";

assertEquals(expectedDebug, ctLoop.toString());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,12 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() {
compiler.addInputSource(new File("./src/test/java/spoon/test/prettyprinter/testclasses/sub/TypeIdentifierCollision.java"));
compiler.addInputSource(new File("./src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java"));
compiler.build();
//apply auto import validators
launcher.prettyprint();

final CtClass<?> aClass = (CtClass<?>) factory.Type().get(spoon.test.prettyprinter.testclasses.TypeIdentifierCollision.class);

String expected =
"public void setFieldUsingExternallyDefinedEnumWithSameNameAsLocal() {" + nl
+ " localField = spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.ENUM.E1.ordinal();" + nl
+ " localField = TypeIdentifierCollision.ENUM.E1.ordinal();" + nl
+ "}";

String computed = aClass.getMethodsByName("setFieldUsingExternallyDefinedEnumWithSameNameAsLocal").get(0).toString();
Expand All @@ -227,10 +225,10 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() {

expected =
"public void setFieldOfClassWithSameNameAsTheCompilationUnitClass() {" + nl
+ " spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.globalField = localField;" + nl
+ " TypeIdentifierCollision.globalField = localField;" + nl
+ "}";

computed = aClass.getMethodsByName("setFieldOfClassWithSameNameAsTheCompilationUnitClass").get(0).prettyprint();
computed = aClass.getMethodsByName("setFieldOfClassWithSameNameAsTheCompilationUnitClass").get(0).toString();
assertEquals("The static field of an external type with the same identifier as the compilation unit is printed with FQN", expected, computed);

expected =
Expand All @@ -246,7 +244,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() {

expected =
"public enum ENUM {" + nl + nl
+ " E1(spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.globalField, spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.ENUM.E1);" + nl
+ " E1(TypeIdentifierCollision.globalField, TypeIdentifierCollision.ENUM.E1);" + nl
+ " final int NUM;" + nl + nl
+ " final Enum<?> e;" + nl + nl
+ " private ENUM(int num, Enum<?> e) {" + nl
Expand All @@ -255,7 +253,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() {
+ " }" + nl
+ "}";

computed = aClass.getNestedType("ENUM").prettyprint();
computed = aClass.getNestedType("ENUM").toString();
assertEquals(expected, computed);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import spoon.reflect.code.CtStatement;
import spoon.reflect.declaration.CtClass;
import spoon.reflect.declaration.CtCompilationUnit;
import spoon.reflect.declaration.CtElement;
import spoon.reflect.declaration.CtField;
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.declaration.CtType;
Expand Down Expand Up @@ -180,7 +181,7 @@ private void testSniper(String testClass, Consumer<CtType<?>> transformation, Bi
launcher.addInputResource(getResourcePath(testClass));
launcher.getEnvironment().setPrettyPrinterCreator(() -> {
SniperJavaPrettyPrinter printer = new SniperJavaPrettyPrinter(launcher.getEnvironment());
printer.setPreprocessors(Collections.unmodifiableList(Arrays.<Processor<CtCompilationUnit>>asList(
printer.setPreprocessors(Collections.unmodifiableList(Arrays.<Processor<CtElement>>asList(
//remove unused imports first. Do not add new imports at time when conflicts are not resolved
new ImportCleaner().setCanAddImports(false),
//solve conflicts, the current imports are relevant too
Expand Down

0 comments on commit 408d76d

Please sign in to comment.