From 7a0539993a7e36139a48f87e761bc1bc3b060e20 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Sat, 1 Dec 2018 08:04:59 +0100 Subject: [PATCH 01/25] refactor: handling of imports --- src/main/java/spoon/compiler/Environment.java | 16 + .../spoon/reflect/declaration/CtElement.java | 9 + ...stractCompilationUnitImportsProcessor.java | 224 ++++++++++ .../visitor/DefaultImportComparator.java | 60 +++ .../visitor/DefaultJavaPrettyPrinter.java | 389 +++++++++--------- .../reflect/visitor/ElementPrinterHelper.java | 2 + .../visitor/ForceFullyQualifiedProcessor.java | 129 ++++++ .../reflect/visitor/ForceImportProcessor.java | 93 +++++ .../spoon/reflect/visitor/ImportScanner.java | 1 + .../reflect/visitor/ImportScannerImpl.java | 1 + .../reflect/visitor/ImportValidator.java | 352 ++++++++++++++++ .../reflect/visitor/MinimalImportScanner.java | 1 + .../visitor/NameConflictValidator.java | 311 ++++++++++++++ .../spoon/reflect/visitor/PrettyPrinter.java | 11 + .../spoon/support/StandardEnvironment.java | 53 ++- .../compiler/jdt/JDTBasedSpoonCompiler.java | 10 +- .../compiler/jdt/JDTCommentBuilder.java | 51 ++- .../support/compiler/jdt/JDTTreeBuilder.java | 18 +- .../compiler/jdt/JDTTreeBuilderHelper.java | 118 +++++- .../support/compiler/jdt/ParentExiter.java | 2 +- .../compiler/jdt/ReferenceBuilder.java | 17 +- .../reflect/declaration/CtElementImpl.java | 18 +- .../sniper/SniperJavaPrettyPrinter.java | 44 +- .../spoon/reflect/visitor/CtScannerTest.java | 17 +- src/test/java/spoon/test/api/APITest.java | 2 +- .../java/spoon/test/comment/CommentTest.java | 39 +- .../test/compilation/CompilationTest.java | 70 ++-- .../ConstructorCallTest.java | 2 +- .../constructorcallnewclass/NewClassTest.java | 3 +- .../test/fieldaccesses/FieldAccessTest.java | 7 +- .../spoon/test/generics/GenericsTest.java | 13 +- .../spoon/test/imports/ImportScannerTest.java | 2 +- .../java/spoon/test/imports/ImportTest.java | 60 ++- .../imports/name_scope/NameScopeTest.java | 2 +- .../imports/testclasses/StaticNoOrdered.java | 2 + .../java/spoon/test/javadoc/JavaDocTest.java | 3 +- .../jdtimportbuilder/ImportBuilderTest.java | 11 +- .../java/spoon/test/lambda/LambdaTest.java | 22 +- .../test/position/TestSourceFragment.java | 68 ++- .../testclasses/FooSourceFragments.java | 18 +- .../DefaultPrettyPrinterTest.java | 37 +- .../spoon/test/prettyprinter/PrinterTest.java | 4 +- .../test/prettyprinter/TestSniperPrinter.java | 24 +- .../testclasses/ImportStatic.java | 7 +- .../test/reference/TypeReferenceTest.java | 16 +- .../test/targeted/TargetedExpressionTest.java | 4 +- .../AccessFullyQualifiedFieldTest.java | 5 +- .../spoon/test/visibility/VisibilityTest.java | 2 +- 48 files changed, 1973 insertions(+), 397 deletions(-) create mode 100644 src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java create mode 100644 src/main/java/spoon/reflect/visitor/DefaultImportComparator.java create mode 100644 src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java create mode 100644 src/main/java/spoon/reflect/visitor/ForceImportProcessor.java create mode 100644 src/main/java/spoon/reflect/visitor/ImportValidator.java create mode 100644 src/main/java/spoon/reflect/visitor/NameConflictValidator.java diff --git a/src/main/java/spoon/compiler/Environment.java b/src/main/java/spoon/compiler/Environment.java index 2e0256427ac..909b612a013 100644 --- a/src/main/java/spoon/compiler/Environment.java +++ b/src/main/java/spoon/compiler/Environment.java @@ -14,6 +14,7 @@ import spoon.processing.ProcessingManager; import spoon.processing.Processor; import spoon.processing.ProcessorProperties; +import spoon.reflect.declaration.CtCompilationUnit; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtMethod; import spoon.reflect.visitor.PrettyPrinter; @@ -24,6 +25,7 @@ import java.io.File; import java.nio.charset.Charset; +import java.util.List; import java.util.function.Supplier; /** @@ -430,4 +432,18 @@ void report(Processor processor, Level level, * contains multiple times the same class */ void setIgnoreDuplicateDeclarations(boolean ignoreDuplicateDeclarations); + + /** + * @return list of {@link Processor}, which are used to validate and fix model before it's printing + * + * Note: by default the validators depends on {@link #isAutoImports()} + */ + List> getCompilationUnitValidators(); + + /** + * @param compilationUnitValidators list of {@link Processor}, which have to be used to validate and fix model before it's printing + * + * Note: once this method is called, the calling of {@link #setAutoImports(boolean)} makes no sense + */ + void setCompilationUnitValidators(List> compilationUnitValidators); } diff --git a/src/main/java/spoon/reflect/declaration/CtElement.java b/src/main/java/spoon/reflect/declaration/CtElement.java index 665601a50fc..47bfe5860a4 100644 --- a/src/main/java/spoon/reflect/declaration/CtElement.java +++ b/src/main/java/spoon/reflect/declaration/CtElement.java @@ -391,4 +391,13 @@ List getAnnotatedChildren( */ List getDirectChildren(); + /** + * @return pretty printed source code of this element. + * + * Note: `implicit` elements are not printed. + * The model is not modified by printing. + * It means it doesn't try to fix model inconsistencies (if any) + * so invalid model causes printing of invalid sources. + */ + String print(); } diff --git a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java new file mode 100644 index 00000000000..1fc865cb1db --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java @@ -0,0 +1,224 @@ +/** + * Copyright (C) 2006-2018 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and + * abiding by the rules of distribution of free software. You can use, modify + * and/or redistribute the software under the terms of the CeCILL-C license as + * circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had + * knowledge of the CeCILL-C license and that you accept its terms. + */ +package spoon.reflect.visitor; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import spoon.SpoonException; +import spoon.processing.AbstractProcessor; +import spoon.processing.Processor; +import spoon.reflect.code.CtConstructorCall; +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtTargetedExpression; +import spoon.reflect.declaration.CtCompilationUnit; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtPackage; +import spoon.reflect.path.CtRole; +import spoon.reflect.reference.CtExecutableReference; +import spoon.reflect.reference.CtFieldReference; +import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.reference.CtVariableReference; +import spoon.reflect.visitor.chain.CtScannerListener; +import spoon.reflect.visitor.chain.ScanningMode; +import spoon.support.Experimental; + +/** + * Internal generic {@link Processor} of {@link CtCompilationUnit}, which scans CtCompilationUnit modules, packages and types + * with purpose to find type references and expressions which might influence import directives. + */ +@Experimental +abstract class AbstractCompilationUnitImportsProcessor extends AbstractProcessor { + + @Override + public void process(CtCompilationUnit cu) { + process(createScanner(), cu); + } + + protected static void process(CtScanner scanner, CtCompilationUnit cu) { + scanner.enter(cu); + switch (cu.getUnitType()) { + case MODULE_DECLARATION: + case UNKNOWN: + break; + case PACKAGE_DECLARATION: + // we need to compute imports only for package annotations and package comments + // we don't want to get all imports coming from content of package + CtPackage pack = cu.getDeclaredPackage(); + scanner.scan(pack.getAnnotations()); + break; + case TYPE_DECLARATION: + for (CtTypeReference typeRef : cu.getDeclaredTypeReferences()) { + scanner.scan(typeRef.getTypeDeclaration()); + } + break; + } + scanner.exit(cu); + } + + protected abstract T createRawScanner(); + + protected CtScannerListener createScannerListener(T scanner) { + return new ScannerListener(scanner); + } + + //The set of roles whose values are always kept implicit + protected static Set IGNORED_ROLES_WHEN_IMPLICIT = new HashSet<>(Arrays.asList( + //e.g. List s = new ArrayList(); + CtRole.TYPE_ARGUMENT, + //e.g. List + CtRole.BOUNDING_TYPE, + //e.g. (/*implicit type of parameter*/ p) -> {} + CtRole.TYPE + )); + + protected class ScannerListener implements CtScannerListener { + protected T scanner; + protected Set ignoredRoles = IGNORED_ROLES_WHEN_IMPLICIT; + + ScannerListener(T scanner) { + super(); + this.scanner = scanner; + } + + @Override + public ScanningMode enter(CtRole role, CtElement element) { + if (element == null) { + return ScanningMode.SKIP_ALL; + } + if (role == CtRole.VARIABLE && element instanceof CtVariableReference) { + //ignore variable reference of field access. The accessType is relevant here instead. + return ScanningMode.SKIP_ALL; + } + if (element.isParentInitialized()) { + CtElement parent = element.getParent(); + if (role == CtRole.DECLARING_TYPE && element instanceof CtTypeReference) { + if (parent instanceof CtFieldReference) { + //ignore the declaring type of field reference. It is not relevant for Imports + return ScanningMode.SKIP_ALL; + } + if (parent instanceof CtExecutableReference) { + /* + * ignore the declaring type of type executable like + * anVariable.getSomeInstance().callMethod() + * The declaring type of `callMethod` method is not relevant for Imports + */ + return ScanningMode.SKIP_ALL; + } else if (parent instanceof CtTypeReference) { + //continue. This is relevant for import + } else { + //May be this can never happen + throw new SpoonException("Check this case. Is it relvant or not?"); + } + } + if (role == CtRole.TYPE && element instanceof CtTypeReference) { + if (parent instanceof CtFieldReference) { + //ignore the type of field references. It is not relevant for Imports + return ScanningMode.SKIP_ALL; + } + if (parent instanceof CtExecutableReference) { + CtElement parent2 = null; + if (element.isParentInitialized()) { + parent2 = parent.getParent(); + } + if (parent2 instanceof CtConstructorCall) { + //new SomeType(); is relevant for import + //continue + } else { + /* + * ignore the return type of executable reference. It is not relevant for Imports + * anVariable.getSomeInstance().callMethod() + * The return type `callMethod` method is not relevant for Imports + */ + return ScanningMode.SKIP_ALL; + } + } + /* + * CtTypeReference + * CtMethod, CtField, ... + * continue. This is relevant for import + */ + } + if (role == CtRole.ARGUMENT_TYPE) { + /* + * ignore the type of parameter of CtExecutableReference + * It is not relevant for Imports. + */ + return ScanningMode.SKIP_ALL; + } + } + if (element.isImplicit() && ignoredRoles.contains(role)) { + //ignore implicit actual type arguments + return ScanningMode.SKIP_ALL; + } + onEnter(getContext(scanner), role, element); + return ScanningMode.NORMAL; + } + } + + protected abstract U getContext(T scanner); + + protected T createScanner() { + T scanner = createRawScanner(); + if (scanner instanceof EarlyTerminatingScanner) { + CtScannerListener listener = createScannerListener(scanner); + if (listener != null) { + ((EarlyTerminatingScanner) scanner).setListener(listener); + } + } + return scanner; + } + + protected void onEnter(U context, CtRole role, CtElement element) { + + if (element instanceof CtTargetedExpression) { + CtTargetedExpression targetedExpression = (CtTargetedExpression) element; + CtExpression target = targetedExpression.getTarget(); + if (target == null) { + return; + } + handleTargetedExpression(context, role, targetedExpression, target); + } else if (element instanceof CtTypeReference) { + //we have to visit only PURE CtTypeReference. No CtArrayTypeReference, CtTypeParameterReference, ... + element.accept(new CtAbstractVisitor() { + @Override + public void visitCtTypeReference(CtTypeReference reference) { + handleTypeReference(context, role, (CtTypeReference) element); + } + }); + } + } + + protected abstract void handleTypeReference(U context, CtRole role, CtTypeReference element); + + protected abstract void handleTargetedExpression(U context, CtRole role, CtTargetedExpression targetedExpression, CtExpression target); + + /** + * @return parent of `element`, but only if it's type is `type` + */ + protected static T getParentIfType(CtElement element, Class type) { + if (element == null || !element.isParentInitialized()) { + return null; + } + CtElement parent = element.getParent(); + if (type.isInstance(parent)) { + return type.cast(parent); + } + return null; + } +} diff --git a/src/main/java/spoon/reflect/visitor/DefaultImportComparator.java b/src/main/java/spoon/reflect/visitor/DefaultImportComparator.java new file mode 100644 index 00000000000..954978af0db --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/DefaultImportComparator.java @@ -0,0 +1,60 @@ +/** + * Copyright (C) 2006-2018 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and + * abiding by the rules of distribution of free software. You can use, modify + * and/or redistribute the software under the terms of the CeCILL-C license as + * circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had + * knowledge of the CeCILL-C license and that you accept its terms. + */ +package spoon.reflect.visitor; + +import java.util.Comparator; + +import spoon.reflect.declaration.CtImport; +import spoon.reflect.declaration.CtImportKind; + + +/** + * Defines order of imports: + * 1) imports are ordered alphabetically + * 2) static imports are last + */ +public class DefaultImportComparator implements Comparator { + + @Override + public int compare(CtImport imp1, CtImport imp2) { + int dif = getImportKindOrder(imp1.getImportKind()) - getImportKindOrder(imp2.getImportKind()); + if (dif != 0) { + return dif; + } + String str1 = removeSuffixSemicolon(imp1.toString()); + String str2 = removeSuffixSemicolon(imp2.toString()); + return str1.compareTo(str2); + } + + private static String removeSuffixSemicolon(String str) { + if (str.endsWith(";")) { + return str.substring(0, str.length() - 1); + } + return str; + } + + private int getImportKindOrder(CtImportKind importKind) { + switch (importKind) { + case TYPE: + case ALL_TYPES: + return 0; + default: + return 1; + } + } + +} diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 560251ac903..c82acdfb7f9 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -24,7 +24,6 @@ import spoon.reflect.code.CtCodeSnippetExpression; import spoon.reflect.code.CtCodeSnippetStatement; import spoon.reflect.code.CtComment; -import spoon.reflect.code.CtComment.CommentType; import spoon.reflect.code.CtConditional; import spoon.reflect.code.CtConstructorCall; import spoon.reflect.code.CtContinue; @@ -91,7 +90,6 @@ import spoon.reflect.declaration.CtTypeParameter; import spoon.reflect.declaration.CtUsedService; import spoon.reflect.declaration.CtVariable; -import spoon.reflect.declaration.ModifierKind; import spoon.reflect.declaration.ParentNotInitializedException; import spoon.reflect.reference.CtArrayTypeReference; import spoon.reflect.reference.CtCatchVariableReference; @@ -108,16 +106,15 @@ import spoon.reflect.reference.CtUnboundVariableReference; import spoon.reflect.reference.CtWildcardReference; import spoon.reflect.visitor.PrintingContext.Writable; -import spoon.reflect.visitor.filter.PotentialVariableDeclarationFunction; import spoon.reflect.visitor.printer.CommentOffset; import java.lang.annotation.Annotation; +import java.util.Arrays; import java.util.Collection; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; /** * A visitor for generating Java code from the program compile-time model. @@ -176,15 +173,10 @@ public class DefaultJavaPrettyPrinter implements CtVisitor, PrettyPrinter { */ private PrintingContext context = new PrintingContext(); - /** get the import scanner of this pretty printer */ - public ImportScanner getImportsContext() { - return importsContext; - } - /** * Handle imports of classes. */ - private ImportScanner importsContext; + protected List> preprocessors; /** * Environment which Spoon is executed. @@ -207,9 +199,9 @@ public ImportScanner getImportsContext() { protected CtCompilationUnit sourceCompilationUnit; /** - * Imports computed + * Ignores isImplicit attribute on model and always prints fully qualified names */ - Set imports; + protected boolean forceFullyQualified = false; public boolean inlineElseIf = true; @@ -218,13 +210,7 @@ public ImportScanner getImportsContext() { */ public DefaultJavaPrettyPrinter(Environment env) { this.env = env; - this.imports = new HashSet<>(); setPrinterTokenWriter(new DefaultTokenWriter(new PrinterHelper(env))); - if (env.isAutoImports()) { - this.importsContext = new ImportScannerImpl(); - } else { - this.importsContext = new MinimalImportScanner(); - } } /** @@ -311,16 +297,6 @@ protected void exitCtExpression(CtExpression e) { } } - /** - * Make the imports for a given type. - * - */ - private Collection computeImports(CtType type) { - context.currentTopLevel = type; - importsContext.computeImports(context.currentTopLevel); - return importsContext.getAllImports(); - } - /** * This method is called by {@link #scan(CtElement)} when entering a scanned element. * To be overridden to implement specific behavior. @@ -758,36 +734,6 @@ public void visitCtFieldWrite(CtFieldWrite fieldWrite) { printCtFieldAccess(fieldWrite); } - private boolean isImported(CtFieldReference fieldReference) { - CtImport fieldImport = fieldReference.getFactory().createImport(fieldReference); - - if (this.imports.contains(fieldImport)) { - return true; - } else { - if (fieldReference.getDeclaringType() == null) { - return false; - } - CtTypeMemberWildcardImportReference staticTypeMemberReference = fieldReference.getFactory().Type().createTypeMemberWildcardImportReference(fieldReference.getDeclaringType()); - CtImport staticClassImport = fieldReference.getFactory().createImport(staticTypeMemberReference); - return this.imports.contains(staticClassImport); - } - } - - private boolean isImported(CtExecutableReference executableReference) { - CtImport executableImport = executableReference.getFactory().createImport(executableReference); - - if (this.imports.contains(executableImport)) { - return true; - } else { - if (executableReference.getDeclaringType() == null) { - return false; - } - CtTypeMemberWildcardImportReference staticTypeMemberReference = executableReference.getFactory().Type().createTypeMemberWildcardImportReference(executableReference.getDeclaringType()); - CtImport staticClassImport = executableReference.getFactory().createImport(staticTypeMemberReference); - return this.imports.contains(staticClassImport); - } - } - private void printCtFieldAccess(CtFieldAccess f) { enterCtExpression(f); try (Writable _context = context.modify()) { @@ -796,36 +742,11 @@ private void printCtFieldAccess(CtFieldAccess f) { } CtExpression target = f.getTarget(); if (target != null) { - boolean isInitializeStaticFinalField = isInitializeStaticFinalField(f.getTarget()); - boolean isStaticField = f.getVariable().isStatic(); - boolean isImportedField = this.isImported(f.getVariable()); - - if (!isInitializeStaticFinalField && !(isStaticField && isImportedField)) { - if (target.isImplicit() && !(f.getVariable().getFieldDeclaration() == null && this.env.getNoClasspath())) { - /* - * target is implicit, check whether there is no conflict with an local variable, catch variable or parameter - * in case of conflict make it explicit, otherwise the field access is shadowed by that variable. - * Search for potential variable declaration until we found a class which declares or inherits this field - */ - final CtField field = f.getVariable().getFieldDeclaration(); - if (field != null) { - final String fieldName = field.getSimpleName(); - CtVariable var = f.getVariable().map(new PotentialVariableDeclarationFunction(fieldName)).first(); - if (var != field) { - //another variable declaration was found which is hiding the field declaration for this field access. Make the field access explicit - target.setImplicit(false); - } - } else { - //There is a model inconsistency - printer.writeComment(f.getFactory().createComment("ERROR: Missing field \"" + f.getVariable().getSimpleName() + "\", please check your model. The code may not compile.", CommentType.BLOCK)).writeSpace(); - } - } // the implicit drives the separator - if (!target.isImplicit()) { + if (shouldPrintTarget(target)) { scan(target); printer.writeSeparator("."); } - } _context.ignoreStaticAccess(true); } scan(f.getVariable()); @@ -833,25 +754,25 @@ private void printCtFieldAccess(CtFieldAccess f) { exitCtExpression(f); } - /** - * Check if the target expression is a static final field initialized in a static anonymous block. - */ - private boolean isInitializeStaticFinalField(CtExpression targetExp) { - final CtElement parent; - final CtAnonymousExecutable anonymousParent; - try { - parent = targetExp.getParent(); - anonymousParent = targetExp.getParent(CtAnonymousExecutable.class); - } catch (ParentNotInitializedException e) { + private boolean shouldPrintTarget(CtExpression target) { + if (target == null) { return false; } - return parent instanceof CtFieldWrite - && targetExp.equals(((CtFieldWrite) parent).getTarget()) - && anonymousParent != null - && ((CtFieldWrite) parent).getVariable() != null - && ((CtFieldWrite) parent).getVariable().getDeclaringType() != null - && ((CtFieldWrite) parent).getVariable().getModifiers().contains(ModifierKind.STATIC) - && ((CtFieldWrite) parent).getVariable().getModifiers().contains(ModifierKind.FINAL); + if (!target.isImplicit()) { + //target is not implicit, we always print it + return true; + } + //target is implicit, we should not print it + if (!forceFullyQualified) { + //fully qualified mode is not forced so we should not print implicit target + return false; + } + //forceFullyQualified is ON, we should print full qualified names + if (target instanceof CtThisAccess) { + //the implicit this access is never printed even in forceFullyQualified mode + return false; + } + return true; } @Override @@ -945,50 +866,69 @@ public void visitCtImport(CtImport ctImport) { if (ctImport.getImportKind() != null) { printer.writeKeyword("import"); printer.writeSpace(); + ctImport.accept(new CtImportVisitor() { - switch (ctImport.getImportKind()) { - case TYPE: - visitCtTypeReference((CtTypeReference) ctImport.getReference()); - break; + @Override + public void visitTypeImport(CtTypeReference typeReference) { + writeImportReference(typeReference); + } - case METHOD: + @Override + public void visitMethodImport(CtExecutableReference execRef) { printer.writeKeyword("static"); printer.writeSpace(); - visitCtExecutableReference((CtExecutableReference) ctImport.getReference()); - break; + if (execRef.getDeclaringType() != null) { + writeImportReference(execRef.getDeclaringType()); + printer.writeSeparator("."); + } + printer.writeIdentifier(execRef.getSimpleName()); + } - case FIELD: + @Override + public void visitFieldImport(CtFieldReference fieldReference) { printer.writeKeyword("static"); printer.writeSpace(); - visitCtFieldReference((CtFieldReference) ctImport.getReference()); - break; + if (fieldReference.getDeclaringType() != null) { + writeImportReference(fieldReference.getDeclaringType()); + printer.writeSeparator("."); + } + printer.writeIdentifier(fieldReference.getSimpleName()); + } - case ALL_TYPES: - visitCtPackageReference((CtPackageReference) ctImport.getReference()); + @Override + public void visitAllTypesImport(CtPackageReference packageReference) { + visitCtPackageReference(packageReference); printer.writeSeparator("."); printer.writeIdentifier("*"); - break; + } - case ALL_STATIC_MEMBERS: + @Override + public void visitAllStaticMembersImport(CtTypeMemberWildcardImportReference typeReference) { printer.writeKeyword("static"); printer.writeSpace(); - visitCtTypeReference(((CtTypeMemberWildcardImportReference) ctImport.getReference()).getTypeReference()); + writeImportReference(typeReference.getTypeReference()); printer.writeSeparator("."); printer.writeIdentifier("*"); - break; - case UNRESOLVED: - CtUnresolvedImport ctUnresolvedImport = (CtUnresolvedImport) ctImport; + } + + @Override + public void visitUnresolvedImport(CtUnresolvedImport ctUnresolvedImport) { if (ctUnresolvedImport.isStatic()) { printer.writeKeyword("static"); printer.writeSpace(); } printer.writeCodeSnippet(ctUnresolvedImport.getUnresolvedReference()); + } } printer.writeSeparator(";"); - printer.writeln(); } } + private void writeImportReference(CtTypeReference ref) { + visitCtTypeReference(ref, false); + } + + @Override public void visitCtModule(CtModule module) { enter(module); @@ -1062,24 +1002,46 @@ public void visitCtUsedService(CtUsedService usedService) { @Override public void visitCtCompilationUnit(CtCompilationUnit compilationUnit) { + CtCompilationUnit outerCompilationUnit = this.sourceCompilationUnit; + try { + this.sourceCompilationUnit = compilationUnit; + elementPrinterHelper.writeComment(compilationUnit, CommentOffset.BEFORE); switch (compilationUnit.getUnitType()) { case MODULE_DECLARATION: - //TODO print module declaration + CtModule module = compilationUnit.getDeclaredModule(); + scan(module); break; case PACKAGE_DECLARATION: - //TODO print package declaration + CtPackage pack = compilationUnit.getDeclaredPackage(); + scan(pack); + //note: the package-info.java may contain type declarations too break; case TYPE_DECLARATION: - calculate(compilationUnit, compilationUnit.getDeclaredTypes()); + scan(compilationUnit.getPackageDeclaration()); + for (CtImport imprt : compilationUnit.getImports()) { + scan(imprt); + printer.writeln(); + } + for (CtType t : compilationUnit.getDeclaredTypes()) { + scan(t); + } break; default: - throw new SpoonException("Cannot print compilation unit of type " + compilationUnit.getUnitType()); + throw new SpoonException("Unexpected compilation unit type: " + compilationUnit.getUnitType()); + } + elementPrinterHelper.writeComment(compilationUnit, CommentOffset.AFTER); + } finally { + this.sourceCompilationUnit = outerCompilationUnit; } } @Override public void visitCtPackageDeclaration(CtPackageDeclaration packageDeclaration) { - elementPrinterHelper.writePackageLine(packageDeclaration.getReference().getQualifiedName()); + CtPackageReference ctPackage = packageDeclaration.getReference(); + elementPrinterHelper.writeComment(ctPackage, CommentOffset.BEFORE); + if (!ctPackage.isUnnamedPackage()) { + elementPrinterHelper.writePackageLine(ctPackage.getQualifiedName()); + } } @Override @@ -1278,13 +1240,12 @@ public void visitCtInvocation(CtInvocation invocation) { } } else { // It's a method invocation - boolean isImported = this.isImported(invocation.getExecutable()); - if (!isImported) { + if (invocation.getTarget() != null && (forceFullyQualified || !invocation.getTarget().isImplicit())) { try (Writable _context = context.modify()) { if (invocation.getTarget() instanceof CtTypeAccess) { _context.ignoreGenerics(true); } - if (invocation.getTarget() != null && !invocation.getTarget().isImplicit()) { + if (shouldPrintTarget(invocation.getTarget())) { scan(invocation.getTarget()); printer.writeSeparator("."); } @@ -1581,13 +1542,15 @@ public void visitCtOperatorAssignment(CtOperatorAssignment ref) { - if (importsContext.isImported(ref) // If my.pkg.Something is imported - || (this.env.isAutoImports() && ref.getPackage() != null && "java.lang".equals(ref.getPackage().getSimpleName())) // or that we are in java.lang - ) { - for (CacheBasedConflictFinder typeContext : context.currentThis) { - //A) we are in the context of a class which is also called "Something", - if (typeContext.getSimpleName().equals(ref.getSimpleName()) - && !Objects.equals(typeContext.getPackage(), ref.getPackage())) { - return true; - } - //B) we are in the context of a class which defines field which is also called "Something", - // we should still use qualified version my.pkg.Something - if (typeContext.hasFieldConflict(ref.getSimpleName()) - || typeContext.hasNestedTypeConflict(ref.getSimpleName()) // fix of #2369 - ) { - return true; - } - } - return false; - } else { - return true; + return forceFullyQualified || !ref.isImplicitParent(); } - } @Override @@ -1796,7 +1739,7 @@ public void visitCtTypeReference(CtTypeReference ref) { @Override public void visitCtTypeAccess(CtTypeAccess typeAccess) { - if (typeAccess.isImplicit()) { + if (!forceFullyQualified && typeAccess.isImplicit()) { return; } enterCtExpression(typeAccess); @@ -1809,7 +1752,7 @@ private void visitCtTypeReferenceWithoutGenerics(CtTypeReference ref) { } private void visitCtTypeReference(CtTypeReference ref, boolean withGenerics) { - if (ref.isImplicit()) { + if (!isPrintTypeReference(ref)) { return; } if (ref.isPrimitive()) { @@ -1822,7 +1765,7 @@ private void visitCtTypeReference(CtTypeReference ref, boolean withGenerics) if (!context.ignoreEnclosingClass() && !ref.isLocalType()) { //compute visible type which can be used to print access path to ref CtTypeReference accessType = ref.getAccessType(); - if (!accessType.isAnonymous()) { + if (!accessType.isAnonymous() && isPrintTypeReference(accessType)) { try (Writable _context = context.modify()) { if (!withGenerics) { _context.ignoreGenerics(true); @@ -1856,6 +1799,34 @@ private void visitCtTypeReference(CtTypeReference ref, boolean withGenerics) } } + private boolean isPrintTypeReference(CtTypeReference accessType) { + if (!accessType.isImplicit()) { + //always print explicit type refs + return true; + } + if (forceFullyQualified) { + //print access type always if fully qualified mode is forced + return true; + } + if (context.forceWildcardGenerics() && accessType.getTypeDeclaration().getFormalCtTypeParameters().size() > 0) { + //print access type if access type is generic and we have to force wildcard generics + /* + * E.g. + * class A { + * class B { + * } + * boolean m(Object o) { + * return o instanceof B; //compilation error + * return o instanceof A.B; // OK + * return o instanceof A.B; // OK + * } + * } + */ + return true; + } + return false; + } + @Override public void visitCtUnaryOperator(CtUnaryOperator operator) { enterCtStatement(operator); @@ -1924,29 +1895,38 @@ public void visitCtUnboundVariableReference(CtUnboundVariableReference re } @Override - public String printPackageInfo(CtPackage pack) { + public String printCompilationUnit(CtCompilationUnit compilationUnit) { reset(); - elementPrinterHelper.writeComment(pack); + List> preprocessors = getPreprocessors(); + if (preprocessors != null) { + for (Processor preprocessor : preprocessors) { + preprocessor.process(compilationUnit); + } + } + scanCompilationUnit(compilationUnit); + return getResult(); + } - // we need to compute imports only for annotations - // we don't want to get all imports coming from content of package - for (CtAnnotation annotation : pack.getAnnotations()) { - this.importsContext.computeImports(annotation); + protected void scanCompilationUnit(CtCompilationUnit compilationUnit) { + scan(compilationUnit); } - elementPrinterHelper.writeAnnotations(pack); - if (!pack.isUnnamedPackage()) { - elementPrinterHelper.writePackageLine(pack.getQualifiedName()); + @Override + public String printPackageInfo(CtPackage pack) { + CtCompilationUnit cu = pack.getFactory().CompilationUnit().getOrCreate(pack); + return printCompilationUnit(cu); } - elementPrinterHelper.writeImports(this.importsContext.getAllImports()); - return printer.getPrinterHelper().toString(); - } @Override public String printModuleInfo(CtModule module) { - reset(); - scan(module); - return this.getResult(); + CtCompilationUnit cu = module.getFactory().CompilationUnit().getOrCreate(module); + return printCompilationUnit(cu); + } + + @Override + public String printTypes(CtType... type) { + calculate(null, Arrays.asList(type)); + return getResult(); } @Override @@ -1957,17 +1937,13 @@ public String getResult() { private void reset() { printer.reset(); context = new PrintingContext(); - if (env.isAutoImports()) { - this.importsContext = new ImportScannerImpl(); - } else { - this.importsContext = new MinimalImportScanner(); } - } /** * Write the compilation unit header. */ + @Deprecated public DefaultJavaPrettyPrinter writeHeader(List> types, Collection imports) { elementPrinterHelper.writeHeader(types, imports); return this; @@ -1976,6 +1952,7 @@ public DefaultJavaPrettyPrinter writeHeader(List> types, Collection> types) { elementPrinterHelper.writeFooter(types); return this; @@ -1983,22 +1960,39 @@ public DefaultJavaPrettyPrinter writeFooter(List> types) { @Override public void calculate(CtCompilationUnit sourceCompilationUnit, List> types) { + if (types.isEmpty()) { + return; + } // reset the importsContext to avoid errors with multiple CU - reset(); - - this.sourceCompilationUnit = sourceCompilationUnit; - this.imports = new HashSet<>(); - if (sourceCompilationUnit != null) { - this.importsContext.initWithImports(sourceCompilationUnit.getImports()); + if (sourceCompilationUnit == null) { + CtType type = types.get(0); + sourceCompilationUnit = type.getFactory().CompilationUnit().getOrCreate(type); + } + if (!hasSameTypes(sourceCompilationUnit, types)) { + //the provided CU has different types, then these which has to be printed + //clone CU and assign it expected types + sourceCompilationUnit = sourceCompilationUnit.clone(); + sourceCompilationUnit.setDeclaredTypes(types); + } + CtPackageReference packRef = types.get(0).getPackage().getReference(); + if (!packRef.equals(sourceCompilationUnit.getPackageDeclaration().getReference())) { + //the type was cloned and moved to different package. Adapt package reference of compilation unit too + sourceCompilationUnit.getPackageDeclaration().setReference(packRef); + } + printCompilationUnit(sourceCompilationUnit); } - for (CtType t : types) { - imports.addAll(computeImports(t)); + private boolean hasSameTypes(CtCompilationUnit compilationUnit, List> types) { + List> cuTypes = compilationUnit.getDeclaredTypeReferences(); + if (cuTypes.size() != types.size()) { + return false; } - this.writeHeader(types, imports); - printTypes(types); + Set cuQnames = cuTypes.stream().map(CtTypeReference::getQualifiedName).collect(Collectors.toSet()); + Set qnames = types.stream().map(CtType::getQualifiedName).collect(Collectors.toSet()); + return cuQnames.equals(qnames); } + @Deprecated protected void printTypes(List> types) { for (CtType t : types) { scan(t); @@ -2036,4 +2030,25 @@ public DefaultJavaPrettyPrinter setPrinterTokenWriter(TokenWriter tokenWriter) { private PrinterHelper getPrinterHelper() { return printer.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> preprocessors) { + this.preprocessors = preprocessors; + } + + /** + * @return list of {@link CompilationUnitValidator}, which are used to validate and fix model before it's printing + */ + public List> getPreprocessors() { + return this.preprocessors; + } + + /** + * @param forceFullyQualified true to ignore `isImplicit` attribute on model and always print fully qualified names + */ + public void setForceFullyQualified(boolean forceFullyQualified) { + this.forceFullyQualified = forceFullyQualified; +} } diff --git a/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java b/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java index cad0a13d8ff..a492a0388e6 100644 --- a/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java +++ b/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java @@ -345,6 +345,7 @@ public void writeImports(Collection imports) { /** * Write the compilation unit header. */ + @Deprecated public void writeHeader(List> types, Collection imports) { if (!types.isEmpty()) { for (CtType ctType : types) { @@ -363,6 +364,7 @@ public void writeHeader(List> types, Collection imports) { /** * Write the compilation unit footer. */ + @Deprecated public void writeFooter(List> types) { if (!types.isEmpty()) { for (CtType ctType : types) { diff --git a/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java b/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java new file mode 100644 index 00000000000..439e64877ad --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java @@ -0,0 +1,129 @@ +/** + * Copyright (C) 2006-2018 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and + * abiding by the rules of distribution of free software. You can use, modify + * and/or redistribute the software under the terms of the CeCILL-C license as + * circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had + * knowledge of the CeCILL-C license and that you accept its terms. + */ +package spoon.reflect.visitor; + +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtNewClass; +import spoon.reflect.code.CtTargetedExpression; +import spoon.reflect.code.CtThisAccess; +import spoon.reflect.code.CtTypeAccess; +import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtType; +import spoon.reflect.path.CtRole; +import spoon.reflect.reference.CtTypeReference; +import spoon.support.Experimental; + + +/** + * Removes all imports from compilation unit and forces fully qualified identifiers + */ +@Experimental +public class ForceFullyQualifiedProcessor extends AbstractCompilationUnitImportsProcessor { + + @Override + protected LexicalScopeScanner createRawScanner() { + return new LexicalScopeScanner(); + } + + @Override + protected LexicalScope getContext(LexicalScopeScanner scanner) { + return scanner.getCurrentLexicalScope(); + } + + @Override + protected void handleTypeReference(LexicalScope nameScope, CtRole role, CtTypeReference reference) { + if (reference.isImplicitParent() || reference.isImplicit()) { + if (isThisAccess(reference)) { + //do not force FQ names in this access + return; + } + if (isTypeReferenceToEnclosingType(nameScope, reference)) { + //it is the reference to the enclosing class + //do not use FQ names for that + return; + } + if (isSupertypeOfNewClass(reference)) { + //it is a super type of new anonymous class + //do not use FQ names for that + return; + } + //force fully qualified name + reference.setImplicit(false); + reference.setImplicitParent(false); + } + } + + protected boolean isTypeReferenceToEnclosingType(LexicalScope nameScope, CtTypeReference reference) { +// String qName = reference.getQualifiedName(); +// return Boolean.TRUE == nameScope.forEachElementByName(reference.getSimpleName(), named -> { +// if (named instanceof CtType) { +// CtType type = (CtType) named; +// if (qName.equals(type.getQualifiedName())) { +// return Boolean.TRUE; +// } +// } +// return null; +// }); + //expected by LambdaTest + CtType enclosingType = reference.getParent(CtType.class); + return enclosingType == null ? false : reference.getQualifiedName().equals(enclosingType.getQualifiedName()); + } + + private boolean isSupertypeOfNewClass(CtTypeReference typeRef) { + CtElement parent = typeRef.getParent(); + if (parent instanceof CtClass && ((CtClass) parent).getSuperclass() == typeRef) { + CtElement parent2 = parent.getParent(); + if (parent2 instanceof CtNewClass) { + return true; + } + } + return false; + } + + @Override + protected void handleTargetedExpression(LexicalScope nameScope, CtRole role, CtTargetedExpression targetedExpression, CtExpression target) { + if (!target.isImplicit()) { + return; + } + if (target instanceof CtThisAccess) { + //the non implicit this access is not forced + return; + } + if (target instanceof CtTypeAccess) { + CtTypeAccess typeAccess = (CtTypeAccess) target; + if (isThisAccess(typeAccess)) { + //do not force FQ names for `this` access + return; + } + if (isTypeReferenceToEnclosingType(nameScope, typeAccess.getAccessedType())) { + //it is the reference to the enclosing class + //do not use FQ names for that + return; + } + } + target.setImplicit(false); + } + + private boolean isThisAccess(CtTypeReference typeRef) { + return getParentIfType(getParentIfType(typeRef, CtTypeAccess.class), CtThisAccess.class) != null; + } + + private boolean isThisAccess(CtTypeAccess typeAccess) { + return getParentIfType(typeAccess, CtThisAccess.class) != null; + } +} diff --git a/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java b/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java new file mode 100644 index 00000000000..8f94fc8dbdf --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java @@ -0,0 +1,93 @@ +/** + * Copyright (C) 2006-2018 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and + * abiding by the rules of distribution of free software. You can use, modify + * and/or redistribute the software under the terms of the CeCILL-C license as + * circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had + * knowledge of the CeCILL-C license and that you accept its terms. + */ +package spoon.reflect.visitor; + +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtTargetedExpression; +import spoon.reflect.code.CtThisAccess; +import spoon.reflect.code.CtTypeAccess; +import spoon.reflect.declaration.CtType; +import spoon.reflect.path.CtRole; +import spoon.reflect.reference.CtTypeReference; +import spoon.support.Experimental; + + +/** + * Marks package references implicit so all type will get imported + */ +@Experimental +public class ForceImportProcessor extends AbstractCompilationUnitImportsProcessor { + + @Override + protected LexicalScopeScanner createRawScanner() { + return new LexicalScopeScanner(); + } + + @Override + protected LexicalScope getContext(LexicalScopeScanner scanner) { + return scanner.getCurrentLexicalScope(); + } + + @Override + protected void handleTypeReference(LexicalScope nameScope, CtRole role, CtTypeReference reference) { + if (reference.getPackage() != null) { + //force import of package of top level types only + reference.setImplicitParent(true); + } else { + //it is a reference to an child type + //if it is a reference in scope of parent type declaration then make it implicit, else keep it as it is + CtType contextType = reference.getParent(CtType.class); + if (contextType != null) { + CtType topLevelType = contextType.getTopLevelType(); + CtTypeReference referenceDeclaringType = reference.getDeclaringType(); + if (referenceDeclaringType != null && referenceDeclaringType.getQualifiedName().equals(topLevelType.getQualifiedName())) { + //the reference to direct child type has to be made implicit + reference.setImplicitParent(true); + return; + } else { + //reference to deeper nested child type or to child type from different type has to be kept as it is + } + } + //else it is a reference to a child type from different compilation unit + //keep it as it is (usually explicit) + //but we have to mark top level declaring type as imported, because it is not visited individually + CtTypeReference topLevelTypeRef = reference; + while (topLevelTypeRef != null && topLevelTypeRef.getPackage() == null) { + topLevelTypeRef = topLevelTypeRef.getDeclaringType(); + } + if (topLevelTypeRef != null) { + topLevelTypeRef.setImplicitParent(true); + } + } + } + + + protected void handleTargetedExpression(LexicalScope nameScope, CtRole role, CtTargetedExpression targetedExpression, CtExpression target) { + if (target.isImplicit()) { + return; + } + if (target instanceof CtThisAccess) { + //force implicit this access + target.setImplicit(true); + return; + } + if (target instanceof CtTypeAccess) { + CtTypeAccess typeAccess = (CtTypeAccess) target; + //we might force import of static fields and methods here ... but it is not wanted by default + } + } +} diff --git a/src/main/java/spoon/reflect/visitor/ImportScanner.java b/src/main/java/spoon/reflect/visitor/ImportScanner.java index 737f189411a..c8c31078064 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/ImportScanner.java @@ -17,6 +17,7 @@ * The import scanner API might still change in future release, that's why it is marked as experimental. */ @Experimental +@Deprecated public interface ImportScanner { /** diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 22cec0de8f4..4053698efe1 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -55,6 +55,7 @@ /** * A scanner that calculates the imports for a given model. */ +@Deprecated public class ImportScannerImpl extends CtScanner implements ImportScanner { private static final Collection namesPresentInJavaLang8 = diff --git a/src/main/java/spoon/reflect/visitor/ImportValidator.java b/src/main/java/spoon/reflect/visitor/ImportValidator.java new file mode 100644 index 00000000000..2c70250eb4f --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/ImportValidator.java @@ -0,0 +1,352 @@ +/** + * Copyright (C) 2006-2018 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and + * abiding by the rules of distribution of free software. You can use, modify + * and/or redistribute the software under the terms of the CeCILL-C license as + * circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had + * knowledge of the CeCILL-C license and that you accept its terms. + */ +package spoon.reflect.visitor; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +import spoon.SpoonException; +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtFieldAccess; +import spoon.reflect.code.CtInvocation; +import spoon.reflect.code.CtTargetedExpression; +import spoon.reflect.code.CtTypeAccess; +import spoon.reflect.code.CtVariableAccess; +import spoon.reflect.declaration.CtCompilationUnit; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtImport; +import spoon.reflect.declaration.CtImportKind; +import spoon.reflect.declaration.CtPackage; +import spoon.reflect.factory.Factory; +import spoon.reflect.path.CtRole; +import spoon.reflect.reference.CtExecutableReference; +import spoon.reflect.reference.CtFieldReference; +import spoon.reflect.reference.CtPackageReference; +import spoon.reflect.reference.CtReference; +import spoon.reflect.reference.CtTypeMemberWildcardImportReference; +import spoon.reflect.reference.CtTypeReference; +import spoon.support.Experimental; +import spoon.support.util.ModelList; +import spoon.support.visitor.ClassTypingContext; + + +/** + * Updates list of import statements of compilation unit following {@link CtElement#isImplicit()}. + * It doesn't call {@link CtElement#setImplicit(boolean)}. + * It doesn't fix wrong used implicit which causes conflicts. The fixing is task of {@link NameConflictValidator} + */ +@Experimental +public class ImportValidator extends AbstractCompilationUnitImportsProcessor { + + private Comparator importComparator; + private boolean canAddImports = true; + private boolean canRemoveImports = true; + + @Override + protected MyScanner createRawScanner() { + return new MyScanner(); + } + + @Override + protected Context getContext(MyScanner scanner) { + return scanner.context; + } + + @Override + protected void handleTargetedExpression(Context context, CtRole role, CtTargetedExpression targetedExpression, CtExpression target) { + if (target != null && target.isImplicit()) { + if (target instanceof CtTypeAccess) { + if (targetedExpression instanceof CtFieldAccess) { + context.addImport(((CtFieldAccess) targetedExpression).getVariable()); + } else if (targetedExpression instanceof CtInvocation) { + CtExecutableReference execRef = ((CtInvocation) targetedExpression).getExecutable(); + if (execRef.isStatic()) { + context.addImport(execRef); + } + } + } else if (target instanceof CtVariableAccess) { + throw new SpoonException("TODO"); + } + } + } + + @Override + protected void handleTypeReference(Context context, CtRole role, CtTypeReference reference) { + if (reference.isImplicit()) { + /* + * the reference is implicit. E.g. `assertTrue();` + * where type `org.junit.Assert` is implicit + */ + CtTargetedExpression targetedExpr = getParentIfType(getParentIfType(reference, CtTypeAccess.class), CtTargetedExpression.class); + if (targetedExpr != null) { + if (targetedExpr instanceof CtInvocation) { + CtInvocation invocation = (CtInvocation) targetedExpr; + //import static method + context.addImport(invocation.getExecutable()); + } else if (targetedExpr instanceof CtFieldAccess) { + //import static field + CtFieldAccess fieldAccess = (CtFieldAccess) targetedExpr; + context.addImport(fieldAccess.getVariable()); + } + } + //else do nothing. E.g. in case of implicit type of lambda parameter + //`(e) -> {...}` + } else if (reference.isImplicitParent()) { + /* + * the package is implicit. E.g. `Assert.assertTrue` + * where package `org.junit` is implicit + */ + context.addImport(reference); + } + } + + class Context { + private CtCompilationUnit compilationUnit; + private Map computedImports; + private String packageQName; + private Set typeRefQNames; + + Context(CtCompilationUnit cu) { + this.compilationUnit = cu; + CtPackage pckg = cu.getDeclaredPackage(); + if (pckg != null) { + this.packageQName = pckg.getReference().getQualifiedName(); + } + this.typeRefQNames = cu.getDeclaredTypeReferences().stream().map(CtTypeReference::getQualifiedName).collect(Collectors.toSet()); + computedImports = new HashMap<>(); + } + + Factory getFactory() { + return compilationUnit.getFactory(); + } + + void addImport(CtReference ref) { + if (ref == null) { + return; + } + //check that we do not add reference to a local type + CtTypeReference typeRef; + if (ref instanceof CtExecutableReference) { + typeRef = ((CtExecutableReference) ref).getDeclaringType(); + } else if (ref instanceof CtFieldReference) { + typeRef = ((CtFieldReference) ref).getDeclaringType(); + } else if (ref instanceof CtTypeReference) { + typeRef = (CtTypeReference) ref; + } else { + throw new SpoonException("Unexpected reference type " + ref.getClass()); + } + CtTypeReference topLevelTypeRef = typeRef.getTopLevelType(); + if (typeRefQNames.contains(topLevelTypeRef.getQualifiedName())) { + //it is reference to a type of this CompilationUnit. Do not add it + return; + } + CtPackageReference packageRef = topLevelTypeRef.getPackage(); + if (packageRef == null) { + throw new SpoonException("Type reference has no package"); + } + if ("java.lang".equals(packageRef.getQualifiedName())) { + //java.lang is always imported implicitly. Ignore it + return; + } + if (Objects.equals(packageQName, packageRef.getQualifiedName())) { + //it is reference to a type of the same package. Do not add it + return; + } + String importRefID = getImportRefID(ref); + if (!computedImports.containsKey(importRefID)) { + computedImports.put(importRefID, getFactory().Type().createImport(ref)); + } + } + + void onComilationUnitProcessed(CtCompilationUnit compilationUnit) { + ModelList existingImports = compilationUnit.getImports(); + Set computedImports = new HashSet<>(this.computedImports.values()); + for (CtImport oldImport : new ArrayList<>(existingImports)) { + if (!computedImports.remove(oldImport)) { + if (oldImport.getImportKind() == CtImportKind.ALL_TYPES) { + if (removeAllTypeImportWithPackage(computedImports, ((CtPackageReference) oldImport.getReference()).getQualifiedName())) { + //this All types import still imports some type. Keep it + continue; + } + } + if (oldImport.getImportKind() == CtImportKind.ALL_STATIC_MEMBERS) { + if (removeAllStaticTypeMembersImportWithType(computedImports, ((CtTypeMemberWildcardImportReference) oldImport.getReference()).getTypeReference())) { + //this All types import still imports some type. Keep it + continue; + } + } + //the import doesn't exist in computed imports. Remove it + if (canRemoveImports) { + existingImports.remove(oldImport); + } + } + } + + //add new imports + if (canAddImports) { + existingImports.addAll(computedImports); + } + if (importComparator != null) { + existingImports.set(existingImports.stream().sorted(importComparator).collect(Collectors.toList())); + } + } + } + + /** + * @return fast unique identification of reference. It is not the same like printing of import, because it needs to handle access path. + */ + private static String getImportRefID(CtReference ref) { + if (ref == null) { + throw new SpoonException("Null import refrence"); + } + if (ref instanceof CtFieldReference) { + CtFieldReference fieldRef = (CtFieldReference) ref; + return fieldRef.getDeclaringType().getQualifiedName() + "." + fieldRef.getSimpleName(); + } + if (ref instanceof CtExecutableReference) { + CtExecutableReference execRef = (CtExecutableReference) ref; + return execRef.getDeclaringType().getQualifiedName() + "." + execRef.getSimpleName(); + } + if (ref instanceof CtTypeMemberWildcardImportReference) { + CtTypeMemberWildcardImportReference wildRef = (CtTypeMemberWildcardImportReference) ref; + return wildRef.getTypeReference().getQualifiedName() + ".*"; + } + if (ref instanceof CtTypeReference) { + CtTypeReference typeRef = (CtTypeReference) ref; + return typeRef.getQualifiedName(); + } + throw new SpoonException("Unexpected import type: " + ref.getClass()); + } + + /** + * removes all type imports with the same package from the `imports` + * @return true if at least one import with the same package exists + */ + private boolean removeAllTypeImportWithPackage(Set imports, String packageName) { + boolean found = false; + for (Iterator iter = imports.iterator(); iter.hasNext();) { + CtImport newImport = (CtImport) iter.next(); + if (newImport.getImportKind() == CtImportKind.TYPE) { + CtTypeReference typeRef = (CtTypeReference) newImport.getReference(); + if (typeRef.getPackage() != null && packageName.equals(typeRef.getPackage().getQualifiedName())) { + found = true; + if (canRemoveImports) { + iter.remove(); + } + } + } + } + return found; + } + + /** + * removes all static type member imports with the same type from `imports` + * @return true if at least one import with the same type exists + */ + private boolean removeAllStaticTypeMembersImportWithType(Set imports, CtTypeReference typeRef) { + //the cached type hierarchy of typeRef + ClassTypingContext contextOfTypeRef = new ClassTypingContext(typeRef); + Iterator iter = imports.iterator(); + class Visitor extends CtAbstractImportVisitor { + boolean found = false; + @Override + public void visitFieldImport(CtFieldReference fieldReference) { + checkType(fieldReference.getDeclaringType()); + } + @Override + public void visitMethodImport(CtExecutableReference executableReference) { + checkType(executableReference.getDeclaringType()); + } + void checkType(CtTypeReference importTypeRef) { + if (contextOfTypeRef.isSubtypeOf(importTypeRef)) { + found = true; + if (canRemoveImports) { + iter.remove(); + } + } + } + } + Visitor visitor = new Visitor(); + while (iter.hasNext()) { + iter.next().accept(visitor); + } + return visitor.found; + } + + class MyScanner extends EarlyTerminatingScanner { + Context context; + @Override + protected void enter(CtElement e) { + if (e instanceof CtCompilationUnit) { + context = new Context((CtCompilationUnit) e); + } + } + + @Override + protected void exit(CtElement e) { + if (e instanceof CtCompilationUnit) { + context.onComilationUnitProcessed((CtCompilationUnit) e); + } + } + } + + /** + * @return true if this processor is allowed to add new imports + */ + public boolean isCanAddImports() { + return canAddImports; + } + + /** + * @param canAddImports true if this processor is allowed to add new imports + */ + public ImportValidator setCanAddImports(boolean canAddImports) { + this.canAddImports = canAddImports; + return this; + } + + /** + * @return true if this processor is allowed to remove imports + */ + public boolean isCanRemoveImports() { + return canRemoveImports; + } + + /** + * @param canRemoveImports true if this processor is allowed to remove imports + */ + public ImportValidator setCanRemoveImports(boolean canRemoveImports) { + this.canRemoveImports = canRemoveImports; + return this; + } + + public Comparator getImportComparator() { + return importComparator; + } + + public ImportValidator setImportComparator(Comparator importComparator) { + this.importComparator = importComparator; + return this; + } +} diff --git a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java index 3844e611c1e..dc6f86676d0 100644 --- a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java @@ -14,6 +14,7 @@ * A scanner dedicated to import only the necessary packages, @see spoon.test.variable.AccessFullyQualifiedTest * */ +@Deprecated public class MinimalImportScanner extends ImportScannerImpl implements ImportScanner { /** * This method use @link{ImportScannerImpl#isTypeInCollision} to import a ref only if there is a collision diff --git a/src/main/java/spoon/reflect/visitor/NameConflictValidator.java b/src/main/java/spoon/reflect/visitor/NameConflictValidator.java new file mode 100644 index 00000000000..fe9139bf5c0 --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/NameConflictValidator.java @@ -0,0 +1,311 @@ +/** + * Copyright (C) 2006-2018 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and + * abiding by the rules of distribution of free software. You can use, modify + * and/or redistribute the software under the terms of the CeCILL-C license as + * circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had + * knowledge of the CeCILL-C license and that you accept its terms. + */ +package spoon.reflect.visitor; + +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtFieldAccess; +import spoon.reflect.code.CtInvocation; +import spoon.reflect.code.CtTargetedExpression; +import spoon.reflect.code.CtTypeAccess; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtExecutable; +import spoon.reflect.declaration.CtField; +import spoon.reflect.declaration.CtMethod; +import spoon.reflect.declaration.CtNamedElement; +import spoon.reflect.declaration.CtType; +import spoon.reflect.declaration.CtTypeMember; +import spoon.reflect.path.CtRole; +import spoon.reflect.reference.CtExecutableReference; +import spoon.reflect.reference.CtFieldReference; +import spoon.reflect.reference.CtPackageReference; +import spoon.reflect.reference.CtTypeReference; +import spoon.support.Experimental; + + +/** + * Detects conflict of + * + * 1) Example: conflict of field name with an variable name and fixes it by making field target explicit. + * 
+ * class A {
+ *  int xxx;
+ *  void m(String xxx) {
+ *    this.xxx //the target `this.` must be explicit, otherwise parameter `String xxx` hides it
+ *  }
+ * }
+ *
 + * + * 2) Example: conflict of package name with an variable name and fixes it by making field target implicit. + * 
+ * class A {
+ *  int com;
+ *  void m() {
+ *    com.package.Type.doSomething(); //the package `com` is in conflict with field `com`. Must be imported
+ *  }
+ * }
+ *
 + * and fixes them by call of {@link CtElement#setImplicit(boolean)} and {@link CtTypeReference#setImplicitParent(boolean)} + */ +@Experimental +public class NameConflictValidator extends AbstractCompilationUnitImportsProcessor { + + @Override + protected LexicalScopeScanner createRawScanner() { + return new LexicalScopeScanner(); + } + + @Override + protected LexicalScope getContext(LexicalScopeScanner scanner) { + return scanner.getCurrentLexicalScope(); + } + + @Override + protected void handleTargetedExpression(LexicalScope nameScope, CtRole role, CtTargetedExpression targetedExpression, CtExpression target) { + if (targetedExpression instanceof CtFieldAccess) { + CtFieldAccess fieldAccess = (CtFieldAccess) targetedExpression; + if (target.isImplicit()) { + /* + * target is implicit, check whether there is no conflict with an local variable, catch variable or parameter + * in case of conflict make it explicit, otherwise the field access is shadowed by that variable. + * Search for potential variable declaration until we found a class which declares or inherits this field + */ + final CtField field = fieldAccess.getVariable().getFieldDeclaration(); + if (field != null) { + final String fieldName = field.getSimpleName(); + nameScope.forEachElementByName(fieldName, named -> { + if (named instanceof CtMethod) { + //the methods with same name are no problem for field access + return null; + } + if (named == field) { + return true; + } + //another variable declaration was found which is hiding the field declaration for this field access. Make the field access explicit + target.setImplicit(false); + return false; + }); + } + } + if (!target.isImplicit()) { + //the target should be visible in sources + if (target instanceof CtTypeAccess) { + //the type has to be visible in sources + CtTypeAccess typeAccess = (CtTypeAccess) target; + CtTypeReference accessedTypeRef = typeAccess.getAccessedType(); + checkConflictOfTypeReference(nameScope, accessedTypeRef); + } + } + } + } + + @Override + protected void handleTypeReference(LexicalScope nameScope, CtRole role, CtTypeReference ref) { + if (ref.isImplicit()) { + /* + * the reference is implicit. E.g. `assertTrue();` + * when the type `org.junit.Assert` is implicit + */ + //check if targeted expression is in conflict + CtTargetedExpression targetedExpr = getParentIfType(getParentIfType(ref, CtTypeAccess.class), CtTargetedExpression.class); + if (targetedExpr instanceof CtInvocation) { + CtInvocation invocation = (CtInvocation) targetedExpr; + CtExecutableReference importedReference = invocation.getExecutable(); + CtExecutable importedElement = importedReference.getExecutableDeclaration(); + if (importedElement == null) { + //we have no access to executable - probably no class path mode + //What to do? Keep it as it is? Or make it fully qualified? + //keep it as it is for now. + return; + } + if (importedElement instanceof CtMethod) { + //check if statically imported field or method simple name is not in conflict + nameScope.forEachElementByName(importedReference.getSimpleName(), named -> { + if (named instanceof CtMethod) { + //the method call can be in conflict with Method name only + if (isSameStaticImport(named, importedElement)) { + //we have found import of the same method + return true; + } + ref.setImplicit(false); + ref.setImplicitParent(true); + return false; + } + //no conflict with type or field name + return null; + }); + } + } else if (targetedExpr instanceof CtFieldAccess) { + CtFieldAccess fieldAccess = (CtFieldAccess) targetedExpr; + CtFieldReference importedReference = fieldAccess.getVariable(); + CtElement importedElement = importedReference.getFieldDeclaration(); + if (importedElement == null) { + //we have no access to executable - probably no class path mode + //What to do? Keep it as it is? Or make it fully qualified? + //keep it as it is for now. + return; + } + //check if statically imported field or method simple name is not in conflict + nameScope.forEachElementByName(importedReference.getSimpleName(), named -> { + if (named instanceof CtMethod) { + //field access cannot be in conflict with method name + return null; + } + if (named == importedElement) { + //we have found import of the same field + return true; + } + //else there is a conflict. Make type explicit and package implicit + ref.setImplicit(false); + ref.setImplicitParent(true); + return false; + }); + } + //else do nothing like in case of implicit type of lambda parameter + //`(e) -> {...}` + } + if (!ref.isImplicit() && ref.isImplicitParent()) { + /* + * the package is implicit. E.g. `Assert.assertTrue` + * where package `org.junit` is implicit + */ + String refQName = ref.getQualifiedName(); + //check if type simple name is not in conflict + nameScope.forEachElementByName(ref.getSimpleName(), named -> { + if (named instanceof CtMethod) { + //the methods with same name are no problem for field access + return null; + } + if (named instanceof CtType) { + CtType type = (CtType) named; + if (refQName.equals(type.getQualifiedName())) { + //we have found a declaration of type of the ref. We can use implicit + return true; + } + } + //we have found a variable, field or type of different name + ref.setImplicit(false); + ref.setImplicitParent(false); + return false; + }); + } //else it is already fully qualified + checkConflictOfTypeReference(nameScope, ref); + } + + /** + * if typeRef package name or simple name is in conflict with any name from nameScope then + * solve that conflict by package or type implicit + */ + private void checkConflictOfTypeReference(LexicalScope nameScope, CtTypeReference typeRef) { + if (typeRef == null) { + return; + } + if (!typeRef.isImplicitParent()) { + //we have to print fully qualified type name + //is the first part of package name in conflict with something else? + if (isPackageNameConflict(nameScope, typeRef)) { + //the package must be imported, then simple name might be used in this scope + typeRef.setImplicitParent(true); + if (isSimpleNameConflict(nameScope, typeRef)) { + //there is conflict with simple name too + typeRef.setImplicit(true); + } + } + } else { + if (!typeRef.isImplicit()) { + if (isSimpleNameConflict(nameScope, typeRef)) { + //there is conflict with simple name + //use qualified name + typeRef.setImplicitParent(false); + } + } + } + } + + private boolean isPackageNameConflict(LexicalScope nameScope, CtTypeReference typeRef) { + String fistPackageName = getFirstPackageQName(typeRef); + if (fistPackageName != null) { + return Boolean.TRUE == nameScope.forEachElementByName(fistPackageName, named -> { + if (named instanceof CtMethod) { + //same method name is not a problem + return null; + } + //the package name is in conflict with field name, variable or type name + return Boolean.TRUE; + }); + } + return false; + } + + private boolean isSimpleNameConflict(LexicalScope nameScope, CtTypeReference typeRef) { + String typeQName = typeRef.getQualifiedName(); + //the package name is in conflict check whether type name is in conflict + return Boolean.TRUE == nameScope.forEachElementByName(typeRef.getSimpleName(), named -> { + if (named instanceof CtMethod) { + //same method name is not a problem + return null; + } + if (named instanceof CtType) { + CtType type = (CtType) named; + if (typeQName.equals(type.getQualifiedName())) { + //we found the referenced type declaration -> ok, we can use simple name in this scope + return Boolean.FALSE; + } + //we found another type with same simple name -> conflict + } + //there is conflict with simple name + return Boolean.TRUE; + }); + } + + private String getFirstPackageQName(CtTypeReference typeRef) { + if (typeRef != null) { + CtPackageReference packRef = typeRef.getPackage(); + if (packRef != null) { + String qname = packRef.getQualifiedName(); + if (qname != null && qname.length() > 0) { + int idx = qname.indexOf('.'); + if (idx < 0) { + idx = qname.length(); + } + return qname.substring(0, idx); + } + } + } + return null; + } + /** + * @return true if two methods can come from same static import + * + * For example `import static org.junit.Assert.assertEquals;` + * imports methods with signatures: + * assertEquals(Object, Object) + * assertEquals(Object[], Object[]) + * assertEquals(long, long) + * ... + */ + private static boolean isSameStaticImport(CtNamedElement m1, CtNamedElement m2) { + if (m1 instanceof CtTypeMember && m2 instanceof CtTypeMember) { + if (m1.getSimpleName().equals(m2.getSimpleName())) { + CtType declType1 = ((CtTypeMember) m1).getDeclaringType(); + CtType declType2 = ((CtTypeMember) m2).getDeclaringType(); + //may be we should check isSubTypeOf instead + return declType1 == declType2; + } + } + return false; + } +} diff --git a/src/main/java/spoon/reflect/visitor/PrettyPrinter.java b/src/main/java/spoon/reflect/visitor/PrettyPrinter.java index 92e2a4e2fe3..e702e370143 100644 --- a/src/main/java/spoon/reflect/visitor/PrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/PrettyPrinter.java @@ -18,6 +18,11 @@ */ public interface PrettyPrinter { + /** + * Prints the compilation unit of module-info, package-info or types. + */ + String printCompilationUnit(CtCompilationUnit compilationUnit); + /** * Prints the package info. * It always resets the printing context at the beginning of this process. @@ -30,6 +35,12 @@ public interface PrettyPrinter { */ String printModuleInfo(CtModule module); + /** + * Prints the types of one compilation unit + * It always resets the printing context at the beginning of this process. + */ + String printTypes(CtType... type); + /** * Gets the contents of the compilation unit. */ diff --git a/src/main/java/spoon/support/StandardEnvironment.java b/src/main/java/spoon/support/StandardEnvironment.java index 0d1dba057a4..1d571134c86 100644 --- a/src/main/java/spoon/support/StandardEnvironment.java +++ b/src/main/java/spoon/support/StandardEnvironment.java @@ -22,12 +22,18 @@ 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; import spoon.reflect.declaration.ParentNotInitializedException; +import spoon.reflect.visitor.DefaultImportComparator; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; +import spoon.reflect.visitor.ForceFullyQualifiedProcessor; +import spoon.reflect.visitor.ForceImportProcessor; +import spoon.reflect.visitor.ImportValidator; import spoon.reflect.visitor.PrettyPrinter; +import spoon.reflect.visitor.NameConflictValidator; import spoon.support.compiler.FileSystemFolder; import spoon.support.compiler.SpoonProgress; import spoon.support.modelobs.EmptyModelChangeListener; @@ -42,6 +48,7 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -110,6 +117,8 @@ public class StandardEnvironment implements Serializable, Environment { private Supplier prettyPrinterCreator; + private List> compilationUnitValidators; + /** * Creates a new environment with a null default file * generator. @@ -626,7 +635,9 @@ public PrettyPrinter createPrettyPrinter() { if (prettyPrinterCreator == null) { // DJPP is the default mode // fully backward compatible - return new DefaultJavaPrettyPrinter(this); + DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(this); + printer.setPreprocessors(getCompilationUnitValidators()); + return printer; } return prettyPrinterCreator.get(); } @@ -645,4 +656,44 @@ public boolean isIgnoreDuplicateDeclarations() { public void setIgnoreDuplicateDeclarations(boolean ignoreDuplicateDeclarations) { this.ignoreDuplicateDeclarations = ignoreDuplicateDeclarations; } + + @Override + public List> getCompilationUnitValidators() { + if (compilationUnitValidators == null) { + if (isAutoImports()) { + return Collections.unmodifiableList(Arrays.>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 + new ImportValidator().setCanAddImports(false), + //solve conflicts, the current imports are relevant too + new NameConflictValidator(), + //compute final imports + new ImportValidator().setImportComparator(new DefaultImportComparator()) + )); + } else { + return Collections.unmodifiableList(Arrays.>asList( + //force fully qualified + new ForceFullyQualifiedProcessor(), + //remove unused imports first. Do not add new imports at time when conflicts are not resolved + new ImportValidator().setCanAddImports(false), + //solve conflicts, the current imports are relevant too + new NameConflictValidator(), + //compute final imports + new ImportValidator().setImportComparator(new DefaultImportComparator()) + )); + } + } + return Collections.unmodifiableList(compilationUnitValidators); + } + + @Override + public void setCompilationUnitValidators(List> compilationUnitValidators) { + if (compilationUnitValidators != null) { + this.compilationUnitValidators = new ArrayList<>(compilationUnitValidators); + } else { + //use default validators again + this.compilationUnitValidators = null; + } + } } diff --git a/src/main/java/spoon/support/compiler/jdt/JDTBasedSpoonCompiler.java b/src/main/java/spoon/support/compiler/jdt/JDTBasedSpoonCompiler.java index cf86bd9494d..99f8df007f6 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTBasedSpoonCompiler.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTBasedSpoonCompiler.java @@ -422,12 +422,10 @@ protected void buildModel(CompilationUnitDeclaration[] units) { // we need first to go through the whole model before getting the right reference for imports unit.traverse(builder, unit.scope); }); - if (getFactory().getEnvironment().isAutoImports()) { - //we need first imports before we can place comments. Mainly comments on imports need that - forEachCompilationUnit(unitList, SpoonProgress.Process.IMPORT, unit -> { - new JDTImportBuilder(unit, factory).build(); - }); - } + //we need first imports before we can place comments. Mainly comments on imports need that + forEachCompilationUnit(unitList, SpoonProgress.Process.IMPORT, unit -> { + new JDTImportBuilder(unit, factory).build(); + }); if (getFactory().getEnvironment().isCommentsEnabled()) { forEachCompilationUnit(unitList, SpoonProgress.Process.COMMENT_LINKING, unit -> { new JDTCommentBuilder(unit, factory).build(); diff --git a/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java b/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java index 8192b0a21b6..2b1cbc218ec 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java @@ -31,12 +31,15 @@ import spoon.reflect.declaration.CtAnnotationType; import spoon.reflect.declaration.CtAnonymousExecutable; import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtCompilationUnit; import spoon.reflect.declaration.CtConstructor; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtField; +import spoon.reflect.declaration.CtImport; import spoon.reflect.declaration.CtInterface; import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtModule; +import spoon.reflect.declaration.CtPackageDeclaration; import spoon.reflect.declaration.CtParameter; import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.CtTypeMember; @@ -45,8 +48,8 @@ import spoon.reflect.factory.Factory; import spoon.reflect.reference.CtReference; import spoon.reflect.visitor.CtInheritanceScanner; -import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; +import spoon.reflect.visitor.EarlyTerminatingScanner; import java.io.BufferedReader; import java.io.File; @@ -185,17 +188,25 @@ private CtElement addCommentToNear(final CtComment comment, final Collection> types = spoonUnit.getDeclaredTypes(); + if (types.size() > 0) { + types.get(0).addComment(comment); + return; + } + } else if (file.getName().equals(DefaultJavaPrettyPrinter.JAVA_PACKAGE_DECLARATION)) { + //all compilation unit comments and package declaration comments are in package + //other comments can belong to imports or types declared in package-info.java file spoonUnit.getDeclaredPackage().addComment(comment); - } else if (file != null && file.getName().equals(DefaultJavaPrettyPrinter.JAVA_MODULE_DECLARATION)) { + return; + } else if (file.getName().equals(DefaultJavaPrettyPrinter.JAVA_MODULE_DECLARATION)) { spoonUnit.getDeclaredModule().addComment(comment); - } else { - comment.setCommentType(CtComment.CommentType.FILE); - addCommentToNear(comment, new ArrayList<>(spoonUnit.getDeclaredTypes())); + return; } - return; } // visitor that inserts the comment in the element CtInheritanceScanner insertionVisitor = new CtInheritanceScanner() { @@ -316,6 +327,21 @@ public void visitCtAnnotationType(CtAnnotationType e) visitInterfaceType(e); } + @Override + public void visitCtCompilationUnit(CtCompilationUnit compilationUnit) { + compilationUnit.addComment(comment); + } + + @Override + public void visitCtPackageDeclaration(CtPackageDeclaration packageDeclaration) { + packageDeclaration.addComment(comment); + } + + @Override + public void visitCtImport(CtImport ctImport) { + ctImport.addComment(comment); + } + @Override public void scanCtVariable(CtVariable e) { e.addComment(comment); @@ -475,7 +501,7 @@ public void visitCtAnnotation(CtAnnotation e) { * @return the parent of the comment */ private CtElement findCommentParent(CtComment comment) { - class FindCommentParentScanner extends CtScanner { + class FindCommentParentScanner extends EarlyTerminatingScanner { public CtElement commentParent; private int start; @@ -484,6 +510,7 @@ class FindCommentParentScanner extends CtScanner { FindCommentParentScanner(int start, int end) { this.start = start; this.end = end; + setVisitCompilationUnitContent(true); } private boolean isCommentBetweenElementPosition(CtElement element) { @@ -518,11 +545,7 @@ public void scan(CtElement element) { comment.getPosition().getSourceStart(), comment.getPosition().getSourceEnd()); - if (!spoonUnit.getDeclaredTypes().isEmpty()) { - findCommentParentScanner.scan(spoonUnit.getDeclaredTypes()); - } else if (spoonUnit.getDeclaredModuleReference() != null) { - findCommentParentScanner.scan(spoonUnit.getDeclaredModuleReference().getDeclaration()); - } + findCommentParentScanner.scan(spoonUnit); return findCommentParentScanner.commentParent; } diff --git a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java index cb4df940f7f..a1fa0054ec2 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java @@ -838,7 +838,6 @@ public boolean visit(CompilationUnitDeclaration compilationUnitDeclaration, Comp return true; } - @Override public boolean visit(ReferenceExpression referenceExpression, BlockScope blockScope) { context.enter(helper.createExecutableReferenceExpression(referenceExpression), referenceExpression); @@ -957,7 +956,9 @@ public boolean visit(ArrayTypeReference arrayTypeReference, BlockScope scope) { CtTypeReference objectCtTypeReference = references.buildTypeReference(arrayTypeReference, scope); final CtTypeAccess typeAccess = factory.Code().createTypeAccess(objectCtTypeReference); if (typeAccess.getAccessedType() instanceof CtArrayTypeReference) { - ((CtArrayTypeReference) typeAccess.getAccessedType()).getArrayType().setAnnotations(this.references.buildTypeReference(arrayTypeReference, scope).getAnnotations()); + CtTypeReference arrayType = ((CtArrayTypeReference) typeAccess.getAccessedType()).getArrayType(); + arrayType.setAnnotations(this.references.buildTypeReference(arrayTypeReference, scope).getAnnotations()); + arrayType.setImplicitParent(true); } context.enter(typeAccess, arrayTypeReference); return true; @@ -1468,7 +1469,10 @@ public boolean visit(QualifiedNameReference qualifiedNameRef, BlockScope scope) context.enter(helper.createVariableAccess(qualifiedNameRef), qualifiedNameRef); return true; } else if (qualifiedNameRef.binding instanceof TypeBinding) { - context.enter(factory.Code().createTypeAccessWithoutCloningReference(references.getTypeReference((TypeBinding) qualifiedNameRef.binding)), qualifiedNameRef); + TypeBinding typeBinding = (TypeBinding) qualifiedNameRef.binding; + CtTypeReference typeRef = references.getTypeReference(typeBinding); + helper.handleImplicit(typeBinding.getPackage(), qualifiedNameRef, null, typeRef); + context.enter(factory.Code().createTypeAccessWithoutCloningReference(typeRef), qualifiedNameRef); return true; } else if (qualifiedNameRef.binding instanceof ProblemBinding) { if (context.stack.peek().element instanceof CtInvocation) { @@ -1526,7 +1530,7 @@ public boolean visit(SingleNameReference singleNameReference, BlockScope scope) } else if (singleNameReference.binding instanceof VariableBinding) { context.enter(helper.createVariableAccess(singleNameReference), singleNameReference); } else if (singleNameReference.binding instanceof TypeBinding) { - context.enter(factory.Code().createTypeAccessWithoutCloningReference(references.getTypeReference((TypeBinding) singleNameReference.binding)), singleNameReference); + context.enter(factory.Code().createTypeAccessWithoutCloningReference(references.getTypeReference((TypeBinding) singleNameReference.binding).setImplicitParent(true)), singleNameReference); } else if (singleNameReference.binding instanceof ProblemBinding) { if (context.stack.peek().element instanceof CtInvocation && Character.isUpperCase(CharOperation.charToString(singleNameReference.token).charAt(0))) { context.enter(helper.createTypeAccessNoClasspath(singleNameReference), singleNameReference); @@ -1614,7 +1618,11 @@ public boolean visit(SingleTypeReference singleTypeReference, BlockScope scope) context.enter(helper.createCatchVariable(singleTypeReference), singleTypeReference); return true; } - context.enter(factory.Code().createTypeAccessWithoutCloningReference(references.buildTypeReference(singleTypeReference, scope)), singleTypeReference); + CtTypeReference typeRef = references.buildTypeReference(singleTypeReference, scope); + if (typeRef != null) { + typeRef.setImplicitParent(true); + } + context.enter(factory.Code().createTypeAccessWithoutCloningReference(typeRef), singleTypeReference); return true; } diff --git a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java index e25c13582b9..5c57948edd8 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java @@ -15,6 +15,7 @@ import org.eclipse.jdt.internal.compiler.ast.OpensStatement; import org.eclipse.jdt.internal.compiler.ast.ProvidesStatement; import org.eclipse.jdt.internal.compiler.ast.QualifiedNameReference; +import org.eclipse.jdt.internal.compiler.ast.QualifiedTypeReference; import org.eclipse.jdt.internal.compiler.ast.ReferenceExpression; import org.eclipse.jdt.internal.compiler.ast.RequiresStatement; import org.eclipse.jdt.internal.compiler.ast.SingleNameReference; @@ -28,9 +29,11 @@ import org.eclipse.jdt.internal.compiler.lookup.MissingTypeBinding; import org.eclipse.jdt.internal.compiler.lookup.PackageBinding; import org.eclipse.jdt.internal.compiler.lookup.ProblemBinding; +import org.eclipse.jdt.internal.compiler.lookup.ProblemReferenceBinding; import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding; import org.eclipse.jdt.internal.compiler.lookup.TypeBinding; import org.eclipse.jdt.internal.compiler.lookup.VariableBinding; +import spoon.SpoonException; import spoon.reflect.code.CtCatchVariable; import spoon.reflect.code.CtExecutableReferenceExpression; import spoon.reflect.code.CtExpression; @@ -346,7 +349,7 @@ CtFieldAccess createFieldAccess(SingleNameReference singleNameReference) if (va.getVariable() != null) { final CtFieldReference ref = va.getVariable(); if (ref.isStatic() && !ref.getDeclaringType().isAnonymous()) { - va.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(ref.getDeclaringType())); + va.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(ref.getDeclaringType(), true)); } else if (!ref.isStatic()) { va.setTarget(jdtTreeBuilder.getFactory().Code().createThisAccess(jdtTreeBuilder.getReferencesBuilder().getTypeReference(singleNameReference.actualReceiverType), true)); } @@ -373,7 +376,7 @@ CtFieldAccess createFieldAccessNoClasspath(SingleNameReference singleName final CtReference declaring = jdtTreeBuilder.getReferencesBuilder().getDeclaringReferenceFromImports(singleNameReference.token); if (declaring instanceof CtTypeReference && va.getVariable() != null) { final CtTypeReference declaringRef = (CtTypeReference) declaring; - va.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(declaringRef)); + va.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(declaringRef, true)); va.getVariable().setDeclaringType(declaringRef); va.getVariable().setStatic(true); } @@ -448,7 +451,6 @@ public boolean onAccess(char[][] tokens, int index) { fieldReference.setDeclaringType(jdtTreeBuilder.getReferencesBuilder().getTypeReference(receiverType)); } } - CtTypeAccess typeAccess = jdtTreeBuilder.getFactory().Code().createTypeAccess(fieldReference.getDeclaringType()); if (qualifiedNameReference.indexOfFirstFieldBinding > 1) { // the array sourcePositions contains the position of each element of the qualifiedNameReference @@ -456,13 +458,121 @@ public boolean onAccess(char[][] tokens, int index) { long[] positions = qualifiedNameReference.sourcePositions; typeAccess.setPosition( jdtTreeBuilder.getPositionBuilder().buildPosition(qualifiedNameReference.sourceStart(), (int) (positions[qualifiedNameReference.indexOfFirstFieldBinding - 1] >>> 32) - 2)); + handleImplicit( + qualifiedNameReference.actualReceiverType.getPackage(), + qualifiedNameReference, fieldReference.getSimpleName(), typeAccess.getAccessedType()); } else { typeAccess.setImplicit(qualifiedNameReference.isImplicitThis()); } - return typeAccess; } + static void handleImplicit(PackageBinding packageBinding, QualifiedNameReference qualifiedNameReference, String simpleName, CtTypeReference typeRef) { + handleImplicit( + packageBinding, + qualifiedNameReference.tokens, + qualifiedNameReference.otherBindings == null ? 0 : qualifiedNameReference.otherBindings.length, + qualifiedNameReference, simpleName, typeRef); + } + + static void handleImplicit(QualifiedTypeReference qualifiedTypeReference, CtTypeReference typeRef) { + if (qualifiedTypeReference.resolvedType instanceof ProblemReferenceBinding || qualifiedTypeReference.resolvedType == null) { + //cannot detect implicitness of parts of qualifiedTypeReference, when its type is not known + return; + } + handleImplicit( + qualifiedTypeReference.resolvedType.getPackage(), + qualifiedTypeReference.tokens, + 0, + qualifiedTypeReference, null, typeRef); + } + + private static void handleImplicit(PackageBinding packageBinding, char[][] tokens, int countOfOtherBindings, Object qualifiedNameReference, String simpleName, CtTypeReference typeRef) { + char[][] packageNames = null; + if (packageBinding != null) { + packageNames = packageBinding.compoundName; + } + CtTypeReference originTypeRef = typeRef; + //get component type of arrays + while (typeRef instanceof CtArrayTypeReference) { + typeRef = ((CtArrayTypeReference) typeRef).getComponentType(); + } + int off = tokens.length - 1; + off = off - countOfOtherBindings; + if (off > 0) { + if (simpleName != null) { + if (!simpleName.equals(new String(tokens[off]))) { + throw new SpoonException("Unexpected field reference simple name: \"" + new String(tokens[off]) + "\" expected: \"" + simpleName + "\""); + } + off--; + } + while (off >= 0) { + String token = new String(tokens[off]); + if (!typeRef.getSimpleName().equals(token)) { + /* + * Actually it may happen when type reference full qualified name + * (e.g. spoon.test.imports.testclasses.internal.SuperClass.InnerClassProtected) doesn't match with access path + * (e.g. spoon.test.imports.testclasses.internal.ChildClass.InnerClassProtected) + * In such rare case we cannot detect and set implicitness + */ + typeRef.getFactory().getEnvironment().debugMessage("Compiler's type path: " + qualifiedNameReference + " doesn't matches with Spoon qualified type name: " + originTypeRef); + return; + //throw new SpoonException("Unexpected type reference simple name: \"" + token + "\" expected: \"" + typeRef.getSimpleName() + "\""); + } + CtTypeReference declTypeRef = typeRef.getDeclaringType(); + if (declTypeRef != null) { + typeRef = declTypeRef; + off--; + continue; + } + CtPackageReference packageRef = typeRef.getPackage(); + if (packageRef != null) { + if (packageNames != null && packageNames.length == off) { + //there is full package name + //keep it explicit + return; + } + if (off == 0) { + //the package name is implicit + packageRef.setImplicit(true); + return; + } + throw new SpoonException("Unexpected QualifiedNameReference tokens " + qualifiedNameReference + " for typeRef: " + originTypeRef); + } + } + typeRef.setImplicit(true); + } + } + +// +// private boolean isFullyQualified(QualifiedNameReference qualifiedNameReference) { +// char[][] tokens = qualifiedNameReference.tokens; +// PackageBinding packageBinding = qualifiedNameReference.actualReceiverType.getPackage(); +// char[][] packageNames = null; +// if (packageBinding != null) { +// packageNames = packageBinding.compoundName; +// } +// int idx = 0; +// if (packageNames != null) { +// while (idx < packageNames.length) { +// if (idx >= tokens.length) { +// return false; +// } +// if (!Arrays.equals(tokens[idx], packageNames[idx])) { +// return false; +// } +// idx++; +// } +// } +// if (idx >= tokens.length) { +// return false; +// } +// if (!Arrays.equals(tokens[idx], qualifiedNameReference.actualReceiverType.sourceName())) { +// return false; +// } +// return true; +// } + /** * Creates a type access from its qualified name. * diff --git a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java index faaffe273f2..38e29041246 100644 --- a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java +++ b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java @@ -652,7 +652,7 @@ public void visitCtInvocation(CtInvocation invocation) { if (child instanceof CtThisAccess) { final CtTypeReference declaringType = invocation.getExecutable().getDeclaringType(); if (declaringType != null && invocation.getExecutable().isStatic() && child.isImplicit()) { - invocation.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(declaringType, declaringType.isAnonymous())); + invocation.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(declaringType, true)); } else { invocation.setTarget((CtThisAccess) child); } diff --git a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java index bff1df4cccb..d4baee989b3 100644 --- a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java @@ -21,6 +21,7 @@ import org.eclipse.jdt.internal.compiler.ast.QualifiedNameReference; import org.eclipse.jdt.internal.compiler.ast.QualifiedTypeReference; import org.eclipse.jdt.internal.compiler.ast.SingleNameReference; +import org.eclipse.jdt.internal.compiler.ast.SingleTypeReference; import org.eclipse.jdt.internal.compiler.ast.TypeReference; import org.eclipse.jdt.internal.compiler.ast.Wildcard; import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants; @@ -218,6 +219,12 @@ private CtTypeReference buildTypeReferenceInternal(CtTypeReference typ this.jdtTreeBuilder.getContextBuilder().exit(type); currentReference = currentReference.getDeclaringType(); } + //detect whether something is implicit + if (type instanceof SingleTypeReference) { + typeReference.setImplicitParent(true); + } else if (type instanceof QualifiedTypeReference) { + jdtTreeBuilder.getHelper().handleImplicit((QualifiedTypeReference) type, typeReference); + } return typeReference; } @@ -492,9 +499,15 @@ CtTypeReference getTypeReference(TypeBinding binding, TypeReference ref) CtTypeReference ctRef = getTypeReference(binding); if (ctRef != null && isCorrectTypeReference(ref)) { insertGenericTypesInNoClasspathFromJDTInSpoon(ref, ctRef); - return ctRef; + } else { + ctRef = getTypeReference(ref); + } + if (ref instanceof SingleTypeReference) { + ctRef.setImplicitParent(true); + } else if (ref instanceof QualifiedTypeReference) { + jdtTreeBuilder.getHelper().handleImplicit((QualifiedTypeReference) ref, ctRef); } - return getTypeReference(ref); + return ctRef; } CtTypeReference getTypeParameterReference(TypeBinding binding, TypeReference ref) { diff --git a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java index 469bc386df5..0525728ebe5 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java @@ -14,7 +14,6 @@ import spoon.reflect.cu.SourcePosition; import spoon.reflect.declaration.CtAnnotation; import spoon.reflect.declaration.CtElement; -import spoon.reflect.declaration.CtImport; import spoon.reflect.declaration.CtNamedElement; import spoon.reflect.declaration.CtShadowable; import spoon.reflect.declaration.CtType; @@ -278,14 +277,21 @@ public void enter(CtElement e) { @Override public String toString() { + return print(true); + } + + @Override + public String print() { + return print(false); + } + + private String print(boolean forceFullyQualified) { DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(getFactory().getEnvironment()); + printer.setForceFullyQualified(forceFullyQualified); + //the printer is created here directly without any ImportValidator + //which would change the model content - it is not wanted! String errorMessage = ""; try { - // we do not want to compute imports of for CtImport and CtReference - // as it may change the print of a reference - if (!(this instanceof CtImport) && !(this instanceof CtReference)) { - printer.getImportsContext().computeImports(this); - } printer.scan(this); } catch (ParentNotInitializedException ignore) { LOGGER.error(ERROR_MESSAGE_TO_STRING, ignore); diff --git a/src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java b/src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java index 8f6c4bc3315..4130276c841 100644 --- a/src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java +++ b/src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java @@ -6,9 +6,8 @@ package spoon.support.sniper; import java.util.ArrayDeque; -import java.util.Collection; +import java.util.Collections; import java.util.Deque; -import java.util.List; import spoon.OutputType; import spoon.SpoonException; @@ -16,8 +15,6 @@ import spoon.reflect.code.CtComment; import spoon.reflect.declaration.CtCompilationUnit; import spoon.reflect.declaration.CtElement; -import spoon.reflect.declaration.CtImport; -import spoon.reflect.declaration.CtType; import spoon.reflect.path.CtRole; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; import spoon.reflect.visitor.PrettyPrinter; @@ -100,10 +97,16 @@ private TokenWriter createTokenWriterListener(TokenWriter tokenWriter) { } @Override - public void calculate(CtCompilationUnit sourceCompilationUnit, List> types) { + protected void scanCompilationUnit(CtCompilationUnit compilationUnit) { //use line separator of origin source file - setLineSeparator(detectLineSeparator(sourceCompilationUnit.getOriginalSourceCode())); - super.calculate(sourceCompilationUnit, types); + setLineSeparator(detectLineSeparator(compilationUnit.getOriginalSourceCode())); + runInContext(new SourceFragmentContextList(mutableTokenWriter, + compilationUnit, + Collections.singletonList(compilationUnit.getOriginalSourceFragment()), + new ChangeResolver(getChangeCollector(), compilationUnit)), + () -> { + super.scanCompilationUnit(compilationUnit); + }); } private static final String CR = "\r"; @@ -133,25 +136,6 @@ private String detectLineSeparator(String text) { return System.getProperty("line.separator"); } - @Override - public DefaultJavaPrettyPrinter writeHeader(List> types, Collection imports) { - //run compilation unit header using pretty printer. The sniper mode is not supported for header yet. - runInContext(SourceFragmentContextPrettyPrint.INSTANCE, - () -> super.writeHeader(types, imports)); - return this; - } - - @Override - protected void printTypes(List> types) { - ElementSourceFragment rootFragment = sourceCompilationUnit.getOriginalSourceFragment(); - runInContext(new SourceFragmentContextList(mutableTokenWriter, null, rootFragment.getChildrenFragments(), getChangeResolver()), - () -> { - for (CtType t : types) { - scan(t); - } - }); - } - /** * Called for each printed token * @param tokenType the type of {@link TokenWriter} method @@ -172,8 +156,8 @@ public void printSourceFragment(SourceFragment fragment, Boolean isModified) { CollectionSourceFragment csf = (CollectionSourceFragment) fragment; //we started scanning of collection of elements SourceFragmentContext listContext = csf.isOrdered() - ? new SourceFragmentContextList(mutableTokenWriter, null, csf.getItems(), changeResolver) - : new SourceFragmentContextSet(mutableTokenWriter, null, csf.getItems(), changeResolver); + ? new SourceFragmentContextList(mutableTokenWriter, null, csf.getItems(), getChangeResolver()) + : new SourceFragmentContextSet(mutableTokenWriter, null, csf.getItems(), getChangeResolver()); //push the context of this collection sourceFragmentContextStack.push(listContext); isCollectionStarted = true; @@ -276,8 +260,8 @@ private void scanInternal(CtRole role, CtElement element, SourceFragment fragmen CollectionSourceFragment csf = (CollectionSourceFragment) fragment; //we started scanning of collection of elements SourceFragmentContext listContext = csf.isOrdered() - ? new SourceFragmentContextList(mutableTokenWriter, element, csf.getItems(), changeResolver) - : new SourceFragmentContextSet(mutableTokenWriter, element, csf.getItems(), changeResolver); + ? new SourceFragmentContextList(mutableTokenWriter, element, csf.getItems(), getChangeResolver()) + : new SourceFragmentContextSet(mutableTokenWriter, element, csf.getItems(), getChangeResolver()); //push the context of this collection sourceFragmentContextStack.push(listContext); //and scan first element of that collection again in new context of that collection diff --git a/src/test/java/spoon/reflect/visitor/CtScannerTest.java b/src/test/java/spoon/reflect/visitor/CtScannerTest.java index 6c64b7b9378..069bc8bef12 100644 --- a/src/test/java/spoon/reflect/visitor/CtScannerTest.java +++ b/src/test/java/spoon/reflect/visitor/CtScannerTest.java @@ -25,6 +25,7 @@ import spoon.metamodel.Metamodel; import spoon.reflect.code.CtFieldRead; import spoon.reflect.code.CtInvocation; +import spoon.reflect.cu.CompilationUnit; import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtMethod; @@ -220,7 +221,7 @@ public boolean matches(CtInvocation element) { @Test public void testScan() { - // contract: all AST nodes are visisted through method "scan" + // contract: all AST nodes are visited through method "scan" Launcher launcher; launcher = new Launcher(); launcher.getEnvironment().setNoClasspath(true); @@ -256,14 +257,22 @@ public void exit(CtElement o) { super.exit(o); } }); + + //top comment of file belongs to compilation unit which is not visited by standard scanning + //so count comments of these compilation units + int countOfCommentsInCompilationUnits = 0; + for (CompilationUnit cu : launcher.getFactory().CompilationUnit().getMap().values()) { + countOfCommentsInCompilationUnits += cu.getComments().size(); + } + // interesting, this is never called because of covariance, only CtElement or Collection is called assertEquals(0, counter.nObject); // this is a coarse-grain check to see if the scanner changes // no more exec ref in paramref // also takes into account the comments - assertEquals(3655, counter.nElement); - assertEquals(2435, counter.nEnter); - assertEquals(2435, counter.nExit); + assertEquals(3655, counter.nElement + countOfCommentsInCompilationUnits); + assertEquals(2435, counter.nEnter + countOfCommentsInCompilationUnits); + assertEquals(2435, counter.nExit + countOfCommentsInCompilationUnits); // contract: all AST nodes which are part of Collection or Map are visited first by method "scan(Collection|Map)" and then by method "scan(CtElement)" Counter counter2 = new Counter(); diff --git a/src/test/java/spoon/test/api/APITest.java b/src/test/java/spoon/test/api/APITest.java index ec088bd29b0..ecb640f65a8 100644 --- a/src/test/java/spoon/test/api/APITest.java +++ b/src/test/java/spoon/test/api/APITest.java @@ -445,7 +445,7 @@ public void testOneLinerIntro() { assertEquals("A", l.getSimpleName()); assertEquals(1, l.getMethods().size()); assertEquals("m", l.getMethodsByName("m").get(0).getSimpleName()); - assertEquals("System.out.println(\"yeah\")", l.getMethodsByName("m").get(0).getBody().getStatement(0).toString()); + assertEquals("java.lang.System.out.println(\"yeah\")", l.getMethodsByName("m").get(0).getBody().getStatement(0).toString()); } @Test diff --git a/src/test/java/spoon/test/comment/CommentTest.java b/src/test/java/spoon/test/comment/CommentTest.java index 8cb6ba5ead3..0165103bb57 100644 --- a/src/test/java/spoon/test/comment/CommentTest.java +++ b/src/test/java/spoon/test/comment/CommentTest.java @@ -276,9 +276,10 @@ public void testRemoveComment() { Factory f = getSpoonFactory(); CtClass type = (CtClass) f.Type().get(InlineComment.class); List comments = type.getComments(); - assertEquals(6, comments.size()); + List compilationUnitComments = type.getPosition().getCompilationUnit().getComments(); + assertEquals(6, comments.size() + compilationUnitComments.size()); type.removeComment(comments.get(0)); - assertEquals(5, type.getComments().size()); + assertEquals(5, type.getComments().size() + + compilationUnitComments.size()); } @Test @@ -286,25 +287,25 @@ public void testInLineComment() { Factory f = getSpoonFactory(); CtClass type = (CtClass) f.Type().get(InlineComment.class); String strType = type.toString(); + + List compilationUnitComments = type.getPosition().getCompilationUnit().getComments(); + assertEquals(2, compilationUnitComments.size()); + assertEquals(CtComment.CommentType.BLOCK, compilationUnitComments.get(0).getCommentType()); + assertEquals("Top File\nLine 2", compilationUnitComments.get(0).getContent()); + assertEquals("Bottom File", compilationUnitComments.get(1).getContent()); List comments = type.getElements(new TypeFilter<>(CtComment.class)); // verify that the number of comment present in the AST is correct - assertEquals(69, comments.size()); + assertEquals(67, comments.size()); // verify that all comments present in the AST are printed for (CtComment comment : comments) { - if (comment.getCommentType() == CtComment.CommentType.FILE) { - // the header of the file is not printed with the toString - continue; - } assertNotNull(comment.getParent()); assertTrue(comment.toString() + ":" + comment.getParent() + " is not printed", strType.contains(comment.toString())); } - assertEquals(6, type.getComments().size()); - assertEquals(CtComment.CommentType.FILE, type.getComments().get(0).getCommentType()); - assertEquals(createFakeComment(f, "comment class"), type.getComments().get(1)); - assertEquals("Bottom File", type.getComments().get(5).getContent()); + assertEquals(4, type.getComments().size()); + assertEquals(createFakeComment(f, "comment class"), type.getComments().get(0)); CtField field = type.getField("field"); assertEquals(4, field.getComments().size()); @@ -484,24 +485,23 @@ public void testBlockComment() { Factory f = getSpoonFactory(); CtClass type = (CtClass) f.Type().get(BlockComment.class); String strType = type.toString(); + + List compilationUnitComments = type.getPosition().getCompilationUnit().getComments(); + assertEquals(2, compilationUnitComments.size()); + assertEquals("Bottom File", compilationUnitComments.get(1).getContent()); List comments = type.getElements(new TypeFilter<>(CtComment.class)); // verify that the number of comment present in the AST is correct - assertEquals(52, comments.size()); + assertEquals(50, comments.size()); // verify that all comments present in the AST are printed for (CtComment comment : comments) { - if (comment.getCommentType() == CtComment.CommentType.FILE) { - // the header of the file is not printed with the toString - continue; - } assertNotNull(comment.getParent()); assertTrue(comment.toString() + ":" + comment.getParent() + " is not printed", strType.contains(comment.toString())); } - assertEquals(5, type.getComments().size()); - assertEquals(createFakeBlockComment(f, "comment class"), type.getComments().get(1)); - assertEquals("Bottom File", type.getComments().get(4).getContent()); + assertEquals(3, type.getComments().size()); + assertEquals(createFakeBlockComment(f, "comment class"), type.getComments().get(0)); CtField field = type.getField("field"); assertEquals(2, field.getComments().size()); @@ -799,6 +799,7 @@ public void testAddCommentsToSnippet() { @Test public void testDocumentationContract() throws Exception { // contract: all metamodel classes must be commented with an example. + final Launcher launcher = new Launcher(); launcher.getEnvironment().setNoClasspath(true); launcher.getEnvironment().setCommentEnabled(true); diff --git a/src/test/java/spoon/test/compilation/CompilationTest.java b/src/test/java/spoon/test/compilation/CompilationTest.java index 51c17c5fa24..03cb1d91ba9 100644 --- a/src/test/java/spoon/test/compilation/CompilationTest.java +++ b/src/test/java/spoon/test/compilation/CompilationTest.java @@ -69,36 +69,36 @@ public class CompilationTest { - @Test - public void compileTestWithImportStaticWildcard() { + @Test + public void compileTestWithImportStaticWildcard() { /* Test the compilation of a java file with an import static with wildcard */ - Launcher launcher = new Launcher(); - launcher.addInputResource("src/test/resources/compilation/"); - launcher.getEnvironment().setShouldCompile(true); - launcher.getEnvironment().setAutoImports(true); - launcher.getEnvironment().setNoClasspath(true); - launcher.getEnvironment().setCommentEnabled(true); - launcher.getEnvironment().setBinaryOutputDirectory("target/spooned-classes/"); - launcher.getEnvironment().setSourceOutputDirectory(new File("target/spooned/")); - launcher.run(); - - SpoonModelBuilder compiler = launcher.createCompiler(); - boolean compile = compiler.compile(SpoonModelBuilder.InputType.CTTYPES); - final String nl = System.getProperty("line.separator"); - assertTrue( - nl + "the compilation should succeed: " + nl + - ((JDTBasedSpoonCompiler) compiler).getProblems() - .stream() - .filter(IProblem::isError) - .map(CategorizedProblem::toString) - .collect(Collectors.joining(nl)), - compile - ); - } + Launcher launcher = new Launcher(); + launcher.addInputResource("src/test/resources/compilation/"); + launcher.getEnvironment().setShouldCompile(true); + launcher.getEnvironment().setAutoImports(true); + launcher.getEnvironment().setNoClasspath(true); + launcher.getEnvironment().setCommentEnabled(true); + launcher.getEnvironment().setBinaryOutputDirectory("target/spooned-classes/"); + launcher.getEnvironment().setSourceOutputDirectory(new File("target/spooned/")); + launcher.run(); + + SpoonModelBuilder compiler = launcher.createCompiler(); + boolean compile = compiler.compile(SpoonModelBuilder.InputType.CTTYPES); + final String nl = System.getProperty("line.separator"); + assertTrue( + nl + "the compilation should succeed: " + nl + + ((JDTBasedSpoonCompiler) compiler).getProblems() + .stream() + .filter(IProblem::isError) + .map(CategorizedProblem::toString) + .collect(Collectors.joining(nl)), + compile + ); + } @Test public void compileCommandLineTest() { @@ -300,7 +300,7 @@ public CompilationUnit[] getCompilationUnits() { public void testPrecompile() { // without precompile Launcher l = new Launcher(); - l.setArgs(new String[] {"--noclasspath", "-i", "src/test/resources/compilation/"}); + l.setArgs(new String[]{"--noclasspath", "-i", "src/test/resources/compilation/"}); l.buildModel(); CtClass klass = l.getFactory().Class().get("compilation.Bar"); // without precompile, actualClass does not exist (an exception is thrown) @@ -311,7 +311,7 @@ public void testPrecompile() { // with precompile Launcher l2 = new Launcher(); - l2.setArgs(new String[] {"--precompile", "--noclasspath", "-i", "src/test/resources/compilation/"}); + l2.setArgs(new String[]{"--precompile", "--noclasspath", "-i", "src/test/resources/compilation/"}); l2.buildModel(); CtClass klass2 = l2.getFactory().Class().get("compilation.Bar"); // with precompile, actualClass is not null @@ -321,7 +321,7 @@ public void testPrecompile() { // precompile can be used to compile processors on the fly Launcher l3 = new Launcher(); - l3.setArgs(new String[] {"--precompile", "--noclasspath", "-i", "src/test/resources/compilation/", "-p", "compilation.SimpleProcessor"}); + l3.setArgs(new String[]{"--precompile", "--noclasspath", "-i", "src/test/resources/compilation/", "-p", "compilation.SimpleProcessor"}); l3.run(); } @@ -353,9 +353,9 @@ public void testClassLoader() throws Exception { @Test public void testSingleClassLoader() throws Exception { /* - * contract: the environment exposes a classloader configured by the spoonclass path, - * there is one class loader, so the loaded classes are compatible - */ + * contract: the environment exposes a classloader configured by the spoonclass path, + * there is one class loader, so the loaded classes are compatible + */ Launcher launcher = new Launcher(); launcher.addInputResource(new FileSystemFolder("./src/test/resources/classloader-test")); File outputBinDirectory = new File("./target/classloader-test"); @@ -431,9 +431,9 @@ public void visitCtTypeReference(CtTypeReference reference) { } catch (SpoonClassNotFoundException ignore) { } } }); - + //JDK 9 has implicit constructor, while JDK 8 has not - assertTrue(l.size()>=2); + assertTrue(l.size() >= 2); assertTrue(l.contains("KJHKY")); assertSame(MyClassLoader.class, launcher.getEnvironment().getInputClassLoader().getClass()); } @@ -445,7 +445,7 @@ public void testURLClassLoader() throws Exception { String expected = "target/classes/"; File f = new File(expected); - URL[] urls = { f.toURL() }; + URL[] urls = {f.toURL()}; URLClassLoader urlClassLoader = new URLClassLoader(urls); Launcher launcher = new Launcher(); launcher.getEnvironment().setInputClassLoader(urlClassLoader); @@ -465,7 +465,7 @@ public void testURLClassLoaderWithOtherResourcesThanOnlyFiles() throws Exception File f = new File(file); URL url = new URL(distantJar); - URL[] urls = { f.toURL(), url }; + URL[] urls = {f.toURL(), url}; URLClassLoader urlClassLoader = new URLClassLoader(urls); Launcher launcher = new Launcher(); try { diff --git a/src/test/java/spoon/test/constructorcallnewclass/ConstructorCallTest.java b/src/test/java/spoon/test/constructorcallnewclass/ConstructorCallTest.java index 4f04b4d293d..e2c7530a4ce 100644 --- a/src/test/java/spoon/test/constructorcallnewclass/ConstructorCallTest.java +++ b/src/test/java/spoon/test/constructorcallnewclass/ConstructorCallTest.java @@ -107,7 +107,7 @@ public void testConstructorCallWithGenericArray() { assertEquals("", implicitArrayTyped.toString()); assertEquals("AtomicLong[]", implicitArrayTyped.getSimpleName()); assertTrue(implicitArrayTyped.getComponentType().isImplicit()); - assertEquals("", implicitArrayTyped.getComponentType().toString()); + assertEquals("", implicitArrayTyped.getComponentType().print()); assertEquals("AtomicLong", implicitArrayTyped.getComponentType().getSimpleName()); } diff --git a/src/test/java/spoon/test/constructorcallnewclass/NewClassTest.java b/src/test/java/spoon/test/constructorcallnewclass/NewClassTest.java index ba5027444ba..1ef74fff7f2 100644 --- a/src/test/java/spoon/test/constructorcallnewclass/NewClassTest.java +++ b/src/test/java/spoon/test/constructorcallnewclass/NewClassTest.java @@ -195,8 +195,7 @@ public void testCtNewClassInNoClasspath() { assertEquals("With", anonymousClass.getSuperclass().getSimpleName()); assertEquals("org.apache.lucene.store.Lock$With", anonymousClass.getSuperclass().getQualifiedName()); assertEquals("Lock", anonymousClass.getSuperclass().getDeclaringType().getSimpleName()); - //superclass is implicit so toString is empty - assertEquals("", anonymousClass.getSuperclass().toString()); + assertEquals("org.apache.lucene.store.Lock.With", anonymousClass.getSuperclass().toString()); assertEquals("1", anonymousClass.getSimpleName()); assertEquals("2", secondNewClass.getAnonymousClass().getSimpleName()); assertEquals(1, anonymousClass.getMethods().size()); diff --git a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java index 414f2c21425..e9d30a076ea 100644 --- a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java +++ b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java @@ -43,6 +43,7 @@ import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; import spoon.reflect.visitor.Query; +import spoon.reflect.visitor.NameConflictValidator; import spoon.reflect.visitor.filter.NamedElementFilter; import spoon.reflect.visitor.filter.TypeFilter; import spoon.test.fieldaccesses.testclasses.B; @@ -436,8 +437,8 @@ public void testGetReference() { final CtClass aClass = launcher.getFactory().Class().get(B.class); // now static fields are used with the name of the parent class - assertEquals("A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).toString()); - assertEquals("finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).toString()); + assertEquals("A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).print()); + assertEquals("finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).print()); } @Test public void testFieldAccessAutoExplicit() throws Exception { @@ -449,6 +450,8 @@ public void testFieldAccessAutoExplicit() throws Exception { assertEquals("age", ageFR.getParent().toString()); //add local variable declaration which hides the field declaration method.getBody().insertBegin((CtStatement) mouse.getFactory().createCodeSnippetStatement("int age = 1").compile()); + //run model validator to fix the problem + new NameConflictValidator().process(mouse.getPosition().getCompilationUnit()); //now the field access must use explicit "this." assertEquals("this.age", ageFR.getParent().toString()); } diff --git a/src/test/java/spoon/test/generics/GenericsTest.java b/src/test/java/spoon/test/generics/GenericsTest.java index d6d544e2827..d6f67811cb4 100644 --- a/src/test/java/spoon/test/generics/GenericsTest.java +++ b/src/test/java/spoon/test/generics/GenericsTest.java @@ -49,12 +49,10 @@ import spoon.reflect.reference.CtTypeParameterReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.reference.CtWildcardReference; -import spoon.reflect.visitor.DefaultJavaPrettyPrinter; import spoon.reflect.visitor.filter.AbstractFilter; import spoon.reflect.visitor.filter.NamedElementFilter; import spoon.reflect.visitor.filter.ReferenceTypeFilter; import spoon.reflect.visitor.filter.TypeFilter; -import spoon.support.StandardEnvironment; import spoon.support.comparator.CtLineElementComparator; import spoon.support.reflect.reference.CtTypeParameterReferenceImpl; import spoon.support.reflect.reference.CtTypeReferenceImpl; @@ -175,7 +173,7 @@ public void testDiamond1() { // the diamond is resolved to String but we don't print it, so we use the fully qualified name. assertTrue(val.getType().getActualTypeArguments().get(0).isImplicit()); - assertEquals("", val.getType().getActualTypeArguments().get(0).toString()); + assertEquals("", val.getType().getActualTypeArguments().get(0).print()); assertEquals("java.lang.String", val.getType().getActualTypeArguments().get(0).getQualifiedName()); assertEquals("new java.util.ArrayList<>()",val.toString()); } @@ -291,21 +289,18 @@ public void testBugCommonCollection() throws Exception { CtField x = type.getElements(new NamedElementFilter<>(CtField.class,"x")) .get(0); CtTypeReference ref = x.getType(); - DefaultJavaPrettyPrinter pp = new DefaultJavaPrettyPrinter( - new StandardEnvironment()); // qualifed name assertEquals("java.util.Map$Entry", ref.getQualifiedName()); - // toString uses DefaultJavaPrettyPrinter + // toString uses DefaultJavaPrettyPrinter with forced fully qualified names assertEquals("java.util.Map.Entry", ref.toString()); - // now visitCtTypeReference assertSame(java.util.Map.class, ref.getDeclaringType() .getActualClass()); - pp.visitCtTypeReference(ref); - assertEquals("java.util.Map.Entry", pp.getResult()); + //print prints model following implicit same like in origin source + assertEquals("Map.Entry", ref.print()); CtField y = type.getElements(new NamedElementFilter<>(CtField.class,"y")) .get(0); diff --git a/src/test/java/spoon/test/imports/ImportScannerTest.java b/src/test/java/spoon/test/imports/ImportScannerTest.java index 1bbd2312652..63409ebebf8 100644 --- a/src/test/java/spoon/test/imports/ImportScannerTest.java +++ b/src/test/java/spoon/test/imports/ImportScannerTest.java @@ -338,7 +338,7 @@ public void testImportByJavaDoc() throws Exception { //check that there is still javadoc comment which contains "List" assertTrue(type.getMethodsByName("m").get(0).getComments().toString().contains("List")); { - DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(type.getFactory().getEnvironment()); + PrettyPrinter printer = type.getFactory().getEnvironment().createPrettyPrinter(); printer.calculate(type.getPosition().getCompilationUnit(), Arrays.asList(type)); assertFalse(printer.getResult().contains("import java.util.List;")); } diff --git a/src/test/java/spoon/test/imports/ImportTest.java b/src/test/java/spoon/test/imports/ImportTest.java index 135f367c67b..3223013427f 100644 --- a/src/test/java/spoon/test/imports/ImportTest.java +++ b/src/test/java/spoon/test/imports/ImportTest.java @@ -77,9 +77,11 @@ import spoon.test.imports.testclasses.internal.ChildClass; import spoon.testing.utils.ModelUtils; +import java.io.BufferedReader; import java.io.File; import java.io.FileReader; import java.io.IOException; +import java.io.StringReader; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -298,7 +300,7 @@ public void testSpoonWithImports() { 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); + assertTrue(printByPrinter(anotherClass).indexOf("InnerClass extends ChildClass.InnerClassProtected")>0); importScanner = new ImportScannerImpl(); importScanner.computeImports(classWithInvocation); // java.lang imports are also computed @@ -446,7 +448,7 @@ public void testImportStaticAndFieldAccessWithImport() { final CtType aTacos = launcher.getFactory().Type().get(Tacos.class); final CtStatement assignment = aTacos.getMethod("m").getBody().getStatement(0); assertTrue(assignment instanceof CtLocalVariable); - assertEquals("Constants.CONSTANT.foo", ((CtLocalVariable) assignment).getAssignment().toString()); + assertEquals("Constants.CONSTANT.foo", printByPrinter(((CtLocalVariable) assignment).getAssignment())); } @Test @@ -686,11 +688,15 @@ public void testNestedAccessPathWithTypedParameterWithImports() { CtClass mm = launcher.getFactory().Class().get("spoon.test.imports.testclasses2.AbstractMapBasedMultimap"); CtClass mmwli = launcher.getFactory().Class().get("spoon.test.imports.testclasses2.AbstractMapBasedMultimap$WrappedList$WrappedListIterator"); - assertEquals("private class WrappedListIterator extends AbstractMapBasedMultimap.WrappedCollection.WrappedIterator {}",mmwli.toString()); + //toString prints fully qualified names + assertEquals("private class WrappedListIterator extends spoon.test.imports.testclasses2.AbstractMapBasedMultimap.WrappedCollection.WrappedIterator {}",mmwli.toString()); + //pretty printer prints optimized names + assertEquals("private class WrappedListIterator extends WrappedIterator {}",printByPrinter(mmwli)); assertTrue(mm.toString().contains("AbstractMapBasedMultimap.WrappedCollection.WrappedIterator")); CtClass mmwliother = launcher.getFactory().Class().get("spoon.test.imports.testclasses2.AbstractMapBasedMultimap$OtherWrappedList$WrappedListIterator"); - assertEquals("private class WrappedListIterator extends AbstractMapBasedMultimap.OtherWrappedList.WrappedIterator {}",mmwliother.toString()); + assertEquals("private class WrappedListIterator extends spoon.test.imports.testclasses2.AbstractMapBasedMultimap.OtherWrappedList.WrappedIterator {}",mmwliother.toString()); + assertEquals("private class WrappedListIterator extends WrappedIterator {}",printByPrinter(mmwliother)); } @Test @@ -724,7 +730,10 @@ public void testNestedStaticPathWithTypedParameterWithImports() { fail(e.getMessage()); } CtClass mm = launcher.getFactory().Class().get("spoon.test.imports.testclasses2.Interners"); - assertTrue(mm.toString().contains("List list;")); + //printer does not add FQ for inner types + assertTrue(printByPrinter(mm).contains("List list;")); + //to string forces fully qualified + assertTrue(mm.toString().contains("java.util.List list;")); } @Test @@ -758,7 +767,7 @@ public void testDeepNestedStaticPathWithTypedParameterWithImports() { fail(e.getMessage()); } CtClass mm = launcher.getFactory().Class().get("spoon.test.imports.testclasses2.StaticWithNested"); - assertTrue("new StaticWithNested.StaticNested.StaticNested2();", mm.toString().contains("new StaticWithNested.StaticNested.StaticNested2();")); + assertTrue("new StaticNested2();", printByPrinter(mm).contains("new StaticNested2();")); } private Factory getFactory(String...inputs) { @@ -1327,6 +1336,24 @@ public void testSortImportPutStaticImportAfterTypeImport() { int totalImports = nbStandardImports + nbStaticImports; assertEquals("Exactly "+totalImports+" should have been counted.", (nbStandardImports+nbStaticImports), countImports); + //contract: each `assertEquals` calls is using implicit type. + assertContainsLine("assertEquals(\"bla\", \"truc\");", output); + assertContainsLine("assertEquals(7, 12);", output); + assertContainsLine("assertEquals(new String[0], new String[0]);", output); + } + + private void assertContainsLine(String expectedLine, String output) { + try (BufferedReader br = new BufferedReader(new StringReader(output))) { + String line; + while ((line = br.readLine()) != null) { + if (line.trim().equals(expectedLine)) { + return; + } + } + } catch (IOException e) { + throw new RuntimeException(e); + } + fail("Missing line: " + expectedLine); } @Test @@ -1538,9 +1565,26 @@ public void testBug2369_autoimports() { " }" + nl + "" + nl + " public static void main(String[] args) {" + nl + - " System.out.println(JavaLongUse.method());" + nl + + " System.out.println(method());" + nl + " }" + nl + - "}", launcher.getFactory().Type().get("spoon.test.imports.testclasses.JavaLongUse").toString()); + "}", printByPrinter(launcher.getFactory().Type().get("spoon.test.imports.testclasses.JavaLongUse"))); + } + + public static String printByPrinter(CtElement element) { + DefaultJavaPrettyPrinter pp = (DefaultJavaPrettyPrinter) element.getFactory().getEnvironment().createPrettyPrinter(); + //this call applies print validators, which modifies model before printing + //and then it prints everything + String printedCU = pp.printCompilationUnit(element.getPosition().getCompilationUnit()); + //this code just prints required element (the validators already modified model in previous command) + String printedElement = element.print(); + //check that element is printed same like it would be done by printer + //but we have to ignore indentation first + assertTrue(removeIndentation(printedCU).indexOf(removeIndentation(printedElement)) >= 0); + return printedElement; + } + + private static String removeIndentation(String str) { + return str.replaceAll("(?m)^\\s+", ""); } @Test diff --git a/src/test/java/spoon/test/imports/name_scope/NameScopeTest.java b/src/test/java/spoon/test/imports/name_scope/NameScopeTest.java index fd90ae5ec3f..d66ed700555 100644 --- a/src/test/java/spoon/test/imports/name_scope/NameScopeTest.java +++ b/src/test/java/spoon/test/imports/name_scope/NameScopeTest.java @@ -89,7 +89,7 @@ public ScanningMode enter(CtElement element) { //contract: the local variables are visible after they are declared checkThatScopeContains(scopes[0], Arrays.asList(), "count"); - checkThatScopeContains(scopes[0], Arrays.asList("String theme"), "theme"); + checkThatScopeContains(scopes[0], Arrays.asList("java.lang.String theme"), "theme"); checkThatScopeContains(scopes[0], Arrays.asList(methodDraw), "draw"); checkThatScopeContains(scopes[0], Arrays.asList(typeTereza), "Tereza"); checkThatScopeContains(scopes[0], Arrays.asList(fieldMichal), "michal"); diff --git a/src/test/java/spoon/test/imports/testclasses/StaticNoOrdered.java b/src/test/java/spoon/test/imports/testclasses/StaticNoOrdered.java index bba8e7d2403..dca437c01e1 100644 --- a/src/test/java/spoon/test/imports/testclasses/StaticNoOrdered.java +++ b/src/test/java/spoon/test/imports/testclasses/StaticNoOrdered.java @@ -15,6 +15,8 @@ public class StaticNoOrdered { public void testMachin() { assertEquals("bla","truc"); + assertEquals(7,12); + assertEquals(new String[0],new String[0]); Test test = new Test() { @Override public Class annotationType() { diff --git a/src/test/java/spoon/test/javadoc/JavaDocTest.java b/src/test/java/spoon/test/javadoc/JavaDocTest.java index 0473615b83a..ea9d92c37e0 100644 --- a/src/test/java/spoon/test/javadoc/JavaDocTest.java +++ b/src/test/java/spoon/test/javadoc/JavaDocTest.java @@ -34,6 +34,7 @@ import spoon.reflect.factory.Factory; import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.filter.TypeFilter; +import spoon.test.imports.ImportTest; import spoon.test.javadoc.testclasses.Bar; import java.util.List; @@ -70,7 +71,7 @@ public void testJavaDocReprint() { + " public CtAnnotationType create(CtPackage owner, String simpleName) {" + System.lineSeparator() + " return null;" + System.lineSeparator() + " }" + System.lineSeparator() - + "}", aClass.toString()); + + "}", ImportTest.printByPrinter(aClass)); // contract: getDocComment never returns null, it returns an empty string if no comment assertEquals("", aClass.getDocComment()); diff --git a/src/test/java/spoon/test/jdtimportbuilder/ImportBuilderTest.java b/src/test/java/spoon/test/jdtimportbuilder/ImportBuilderTest.java index d1efe95b7ab..16ba2354238 100644 --- a/src/test/java/spoon/test/jdtimportbuilder/ImportBuilderTest.java +++ b/src/test/java/spoon/test/jdtimportbuilder/ImportBuilderTest.java @@ -45,8 +45,6 @@ */ public class ImportBuilderTest { - private static final String nl = System.getProperty("line.separator"); - @Test public void testWithNoImport() { // contract: when the source code has no import, none is created when building model @@ -75,7 +73,7 @@ public void testWithSimpleImport() { assertEquals(1, imports.size()); CtImport ref = imports.iterator().next(); - assertEquals("import spoon.test.annotation.testclasses.GlobalAnnotation;" + nl, ref.toString()); + assertEquals("import spoon.test.annotation.testclasses.GlobalAnnotation;", ref.toString()); assertTrue(ref.getReference() instanceof CtTypeReference); CtTypeReference refType = (CtTypeReference) ref.getReference(); @@ -88,7 +86,8 @@ public void testWithSimpleImportNoAutoimport() { Launcher spoon = new Launcher(); spoon.addInputResource("./src/test/java/spoon/test/imports/testclasses/ClassWithInvocation.java"); spoon.getEnvironment().setAutoImports(false); - spoon.buildModel(); + //build and print model. During printing the autoImport==false validators are applied + spoon.run(); CtClass classA = spoon.getFactory().Class().get(ClassWithInvocation.class); CompilationUnit unitA = spoon.getFactory().CompilationUnit().getMap().get(classA.getPosition().getFile().getPath()); @@ -180,7 +179,7 @@ public void testWithStaticInheritedImport() { assertEquals(1, imports.size()); CtImport ctImport = imports.iterator().next(); assertEquals(CtImportKind.ALL_STATIC_MEMBERS, ctImport.getImportKind()); - assertEquals("import static spoon.test.jdtimportbuilder.testclasses.staticimport.DependencySubClass.*;" + nl, ctImport.toString()); + assertEquals("import static spoon.test.jdtimportbuilder.testclasses.staticimport.DependencySubClass.*;", ctImport.toString()); } @Test @@ -201,6 +200,6 @@ public void testWithImportFromItf() { CtImport ctImport = imports.iterator().next(); assertEquals(CtImportKind.ALL_STATIC_MEMBERS, ctImport.getImportKind()); - assertEquals("import static jdtimportbuilder.itf.DumbItf.*;" + nl, ctImport.toString()); + assertEquals("import static jdtimportbuilder.itf.DumbItf.*;", ctImport.toString()); } } diff --git a/src/test/java/spoon/test/lambda/LambdaTest.java b/src/test/java/spoon/test/lambda/LambdaTest.java index 81a0299f36f..b117ce573a8 100644 --- a/src/test/java/spoon/test/lambda/LambdaTest.java +++ b/src/test/java/spoon/test/lambda/LambdaTest.java @@ -26,6 +26,7 @@ import spoon.reflect.code.CtLambda; import spoon.reflect.code.CtTypeAccess; import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtInterface; import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtParameter; @@ -42,6 +43,7 @@ import spoon.reflect.visitor.filter.LambdaFilter; import spoon.reflect.visitor.filter.NamedElementFilter; import spoon.reflect.visitor.filter.TypeFilter; +import spoon.test.imports.ImportTest; import spoon.test.lambda.testclasses.Bar; import spoon.test.lambda.testclasses.Foo; import spoon.test.lambda.testclasses.Intersection; @@ -290,7 +292,7 @@ public boolean matches(CtIf element) { + " java.lang.System.err.println(\"Enjoy, you have more than 18.\");" + System .lineSeparator() + "}"; - assertEquals("Condition must be well printed", expected, condition.toString()); + assertEquals("Condition must be well printed", expected, printByPrinter(condition)); } @Test @@ -307,7 +309,8 @@ public void testTypeParameterOfLambdaWithoutType() { final CtParameter ctParameterFirstLambda = lambda1.getParameters().get(0); assertEquals("s", ctParameterFirstLambda.getSimpleName()); assertTrue(ctParameterFirstLambda.getType().isImplicit()); - assertEquals("", ctParameterFirstLambda.getType().toString()); + assertEquals("", ctParameterFirstLambda.getType().print()); + assertEquals("spoon.test.lambda.testclasses.Bar.SingleSubscriber", ctParameterFirstLambda.getType().toString()); assertEquals("SingleSubscriber", ctParameterFirstLambda.getType().getSimpleName()); } @Test @@ -334,7 +337,8 @@ public void testTypeParameterWithImplicitArrayType() { final CtArrayTypeReference typeParameter = (CtArrayTypeReference) ctParameter.getType(); assertTrue(typeParameter.getComponentType().isImplicit()); - assertEquals("", typeParameter.getComponentType().toString()); + assertEquals("", typeParameter.getComponentType().print()); + assertEquals("java.lang.Object", typeParameter.getComponentType().toString()); assertEquals("Object", typeParameter.getComponentType().getSimpleName()); } @@ -346,13 +350,15 @@ public void testLambdaWithPrimitiveParameter() { final CtParameter firstParam = lambda.getParameters().get(0); assertEquals("rs", firstParam.getSimpleName()); assertTrue(firstParam.getType().isImplicit()); - assertEquals("", firstParam.getType().toString()); + assertEquals("", firstParam.getType().print()); + assertEquals("java.sql.ResultSet", firstParam.getType().toString()); assertEquals("ResultSet", firstParam.getType().getSimpleName()); final CtParameter secondParam = lambda.getParameters().get(1); assertEquals("i", secondParam.getSimpleName()); assertTrue(secondParam.getType().isImplicit()); - assertEquals("", secondParam.getType().toString()); + assertEquals("", secondParam.getType().print()); + assertEquals("int", secondParam.getType().toString()); assertEquals("int", secondParam.getType().getSimpleName()); } @@ -493,7 +499,11 @@ private void assertParameterIsNamedBy(String name, CtParameter parameter) { } private void assertIsWellPrinted(String expected, CtLambda lambda) { - assertEquals("Lambda must be well printed", expected, lambda.toString()); + assertEquals("Lambda must be well printed", expected, printByPrinter(lambda)); + } + + private static String printByPrinter(CtElement element) { + return ImportTest.printByPrinter(element); } // note that the lambda number in simple name depends on the classloader diff --git a/src/test/java/spoon/test/position/TestSourceFragment.java b/src/test/java/spoon/test/position/TestSourceFragment.java index 389a04b3a78..540a2794f2a 100644 --- a/src/test/java/spoon/test/position/TestSourceFragment.java +++ b/src/test/java/spoon/test/position/TestSourceFragment.java @@ -19,7 +19,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; -import static spoon.testing.utils.ModelUtils.buildClass; +import static org.junit.Assert.assertTrue; + import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -34,11 +35,12 @@ import spoon.reflect.code.CtConstructorCall; import spoon.reflect.code.CtFieldRead; import spoon.reflect.code.CtFieldWrite; -import spoon.reflect.code.CtInvocation; import spoon.reflect.code.CtLocalVariable; import spoon.reflect.code.CtNewClass; import spoon.reflect.cu.CompilationUnit; import spoon.reflect.cu.SourcePosition; +import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtCompilationUnit; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtType; import spoon.reflect.factory.Factory; @@ -49,7 +51,6 @@ import spoon.support.reflect.cu.CompilationUnitImpl; import spoon.support.reflect.cu.position.SourcePositionImpl; import spoon.test.position.testclasses.AnnonymousClassNewIface; -import spoon.test.position.testclasses.Expressions; import spoon.test.position.testclasses.FooField; import spoon.test.position.testclasses.FooSourceFragments; import spoon.test.position.testclasses.NewArrayList; @@ -207,6 +208,57 @@ public void testExactSourceFragments() throws Exception { checkElementFragments(((CtAssignment)foo.getMethodsByName("m5").get(0).getBody().getStatement(0)).getAssignment(),"7.2"); } + + @Test + public void testSourceFragmentsOfCompilationUnit() throws Exception { + //contract: SourceFragments of compilation unit children like, package declaration, imports, types + final Launcher launcher = new Launcher(); + launcher.getEnvironment().setNoClasspath(false); + launcher.getEnvironment().setCommentEnabled(true); + SpoonModelBuilder comp = launcher.createCompiler(); + comp.addInputSources(SpoonResourceHelper.resources("./src/test/java/" + FooSourceFragments.class.getName().replace('.', '/') + ".java")); + comp.build(); + Factory f = comp.getFactory(); + + final CtType foo = f.Type().get(FooSourceFragments.class); + CtCompilationUnit compilationUnit = foo.getPosition().getCompilationUnit(); + + ElementSourceFragment fragment = compilationUnit.getOriginalSourceFragment(); + List children = fragment.getChildrenFragments(); + assertEquals(11, children.size()); + assertEquals("/**\n" + + " * Javadoc at top of file\n" + + " */", children.get(0).getSourceCode()); + assertEquals("\n", children.get(1).getSourceCode()); + assertEquals("/* comment before package declaration*/\n" + + "package spoon.test.position.testclasses;", children.get(2).getSourceCode()); + assertEquals("\n\n", children.get(3).getSourceCode()); + assertEquals("/*\n" + + " * Comment before import\n" + + " */\n" + + "import java.lang.Deprecated;", children.get(4).getSourceCode()); + assertEquals("\n\n", children.get(5).getSourceCode()); + assertEquals("import java.lang.Class;", children.get(6).getSourceCode()); + assertEquals("\n\n", children.get(7).getSourceCode()); + assertTrue(((ElementSourceFragment) children.get(8)).getElement() instanceof CtClass); + assertStartsWith("/*\n" + + " * Comment before type\n" + + " */\n" + + "public class FooSourceFragments", children.get(8).getSourceCode()); + assertEndsWith("//after last type member\n}", children.get(8).getSourceCode()); + assertEquals("\n\n", children.get(9).getSourceCode()); + assertEquals("//comment at the end of file", children.get(10).getSourceCode()); + } + + + private void assertStartsWith(String expectedPrefix, String real) { + assertEquals(expectedPrefix, real.substring(0, Math.min(expectedPrefix.length(), real.length()))); + } + + private void assertEndsWith(String expectedSuffix, String real) { + int len = real.length(); + assertEquals(expectedSuffix, real.substring(Math.max(0, len - expectedSuffix.length()), len)); + } @Test public void testSourceFragmentsOfFieldAccess() throws Exception { @@ -302,6 +354,8 @@ public void testSourceFragmentsOfNewAnnonymousClass() throws Exception { " }", children.get(2).getSourceCode().replaceAll("\\r|\\n", "")); assertEquals(" ", children.get(3).getSourceCode().replaceAll("\\r|\\n", "")); assertEquals("}", children.get(4).getSourceCode()); + + } } @@ -336,14 +390,14 @@ private static void assertGroupsEqual(Object[] expectedFragments, List { + return "\"" + item.toString() + "\""; + }).collect(Collectors.joining(";")), groupedChildrenFragments.stream().map(item -> { if (item instanceof CollectionSourceFragment) { CollectionSourceFragment csf = (CollectionSourceFragment) item; return "group("+ toCodeStrings(csf.getItems()).toString() + ")"; } - return item.getSourceCode(); - }).collect(Collectors.toList())); + return "\"" + item.getSourceCode() + "\""; + }).collect(Collectors.joining(";"))); } private static List toCodeStrings(List csf) { diff --git a/src/test/java/spoon/test/position/testclasses/FooSourceFragments.java b/src/test/java/spoon/test/position/testclasses/FooSourceFragments.java index 5b3deb9cdf4..ee8ebb83352 100644 --- a/src/test/java/spoon/test/position/testclasses/FooSourceFragments.java +++ b/src/test/java/spoon/test/position/testclasses/FooSourceFragments.java @@ -1,5 +1,19 @@ +/** + * Javadoc at top of file + */ +/* comment before package declaration*/ package spoon.test.position.testclasses; +/* + * Comment before import + */ +import java.lang.Deprecated; + +import java.lang.Class; + +/* + * Comment before type + */ public class FooSourceFragments { void m1(int x) { if(x > 0){this.getClass();}else{/*empty*/} @@ -28,5 +42,7 @@ void m4() { void m5(double f) { f = 7.2; } + //after last type member +} -} \ No newline at end of file +//comment at the end of file \ No newline at end of file diff --git a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java index c9f2b5fe960..f0592905d25 100644 --- a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java +++ b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java @@ -30,6 +30,7 @@ import spoon.reflect.code.CtStatement; import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtConstructor; +import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtParameter; import spoon.reflect.declaration.CtType; @@ -41,6 +42,7 @@ import spoon.reflect.visitor.filter.NamedElementFilter; import spoon.reflect.visitor.filter.TypeFilter; import spoon.support.JavaOutputProcessor; +import spoon.test.imports.ImportTest; import spoon.test.prettyprinter.testclasses.AClass; import spoon.testing.utils.ModelUtils; @@ -123,7 +125,10 @@ public void testPrintAClassWithImports() { + "}"; final CtClass aClass = (CtClass) factory.Type().get(AClass.class); - assertEquals(expected, aClass.toString()); + //TODO remove that after implicit is set correctly for these cases + assertTrue(factory.getEnvironment().createPrettyPrinter().printTypes(aClass).contains(expected)); + + assertEquals(expected, aClass.print()); final CtConstructorCall constructorCall = aClass.getElements(new TypeFilter>(CtConstructorCall.class)).get(0); @@ -149,7 +154,7 @@ public void testPrintAMethodWithImports() { + "}"; final CtClass aClass = (CtClass) factory.Type().get(AClass.class); - assertEquals(expected, aClass.getMethodsByName("aMethod").get(0).toString()); + assertEquals(expected, printByPrinter(aClass.getMethodsByName("aMethod").get(0))); final CtConstructorCall constructorCall = aClass.getElements(new TypeFilter>(CtConstructorCall.class)) @@ -174,7 +179,7 @@ public void testPrintAMethodWithGeneric() { final String expected = "public List aMethodWithGeneric() {" + System.lineSeparator() + " return new ArrayList<>();" + System.lineSeparator() + "}"; - assertEquals(expected, aClass.getMethodsByName("aMethodWithGeneric").get(0).toString()); + assertEquals(expected, printByPrinter(aClass.getMethodsByName("aMethodWithGeneric").get(0))); final CtConstructorCall constructorCall = aClass.getElements(new TypeFilter>(CtConstructorCall.class)) @@ -186,6 +191,10 @@ public void testPrintAMethodWithGeneric() { assertEquals("Object", ctTypeReference.getSimpleName()); } + private static String printByPrinter(CtElement element) { + return ImportTest.printByPrinter(element); + } + @Test public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() { final Launcher launcher = new Launcher(); @@ -195,6 +204,8 @@ 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); @@ -206,12 +217,12 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() { String computed = aClass.getMethodsByName("setFieldUsingExternallyDefinedEnumWithSameNameAsLocal").get(0).toString(); assertEquals("We use FQN for E1", expected, computed); - expected = //This is correct however it could be more concise. + expected = "public void setFieldUsingLocallyDefinedEnum() {" + nl - + " localField = TypeIdentifierCollision.ENUM.E1.ordinal();" + nl + + " localField = ENUM.E1.ordinal();" + nl + "}"; - computed = aClass.getMethodsByName("setFieldUsingLocallyDefinedEnum").get(0).toString(); + computed = aClass.getMethodsByName("setFieldUsingLocallyDefinedEnum").get(0).print(); assertEquals(expected, computed); expected = @@ -219,18 +230,18 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() { + " spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.globalField = localField;" + nl + "}"; - computed = aClass.getMethodsByName("setFieldOfClassWithSameNameAsTheCompilationUnitClass").get(0).toString(); + computed = aClass.getMethodsByName("setFieldOfClassWithSameNameAsTheCompilationUnitClass").get(0).print(); assertEquals("The static field of an external type with the same identifier as the compilation unit is printed with FQN", expected, computed); - expected = //This is correct however it could be more concise. + expected = "public void referToTwoInnerClassesWithTheSameName() {" + nl - + " TypeIdentifierCollision.Class0.ClassA.VAR0 = TypeIdentifierCollision.Class0.ClassA.getNum();" + nl - + " TypeIdentifierCollision.Class1.ClassA.VAR1 = TypeIdentifierCollision.Class1.ClassA.getNum();" + nl + + " ClassA.VAR0 = ClassA.getNum();" + nl + + " Class1.ClassA.VAR1 = Class1.ClassA.getNum();" + nl + "}"; //Ensure the ClassA of Class0 takes precedence over an import statement for ClassA in Class1, and its identifier can be the short version. - computed = aClass.getMethodsByName("referToTwoInnerClassesWithTheSameName").get(0).toString(); + computed = aClass.getMethodsByName("referToTwoInnerClassesWithTheSameName").get(0).print(); assertEquals("where inner types have the same identifier only one may be shortened and the other should be fully qualified", expected, computed); expected = @@ -244,7 +255,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() { + " }" + nl + "}"; - computed = aClass.getNestedType("ENUM").toString(); + computed = aClass.getNestedType("ENUM").print(); assertEquals(expected, computed); } @@ -284,7 +295,7 @@ public void printClassCreatedWithSpoon() throws Exception { File javaFile = new File(pathname); assertTrue(javaFile.exists()); - assertEquals("package foo;" + nl + nl + nl + "class Bar {}" + nl + nl, + assertEquals("package foo;" + nl + "class Bar {}", IOUtils.toString(new FileInputStream(javaFile), "UTF-8")); } diff --git a/src/test/java/spoon/test/prettyprinter/PrinterTest.java b/src/test/java/spoon/test/prettyprinter/PrinterTest.java index 97cbba19e92..84b0a4f30bd 100644 --- a/src/test/java/spoon/test/prettyprinter/PrinterTest.java +++ b/src/test/java/spoon/test/prettyprinter/PrinterTest.java @@ -258,8 +258,8 @@ public void testPrintingOfOrphanFieldReference() throws Exception { //delete the field, so the model is broken. //It may happen during substitution operations and then it is helpful to display descriptive error message type.getField("testedField").delete(); - //contract: printer fails with descriptive exception and not with NPE - assertEquals("/* ERROR: Missing field \"testedField\", please check your model. The code may not compile. */ testedField = 1", type.getMethodsByName("failingMethod").get(0).getBody().getStatement(0).toString()); + //contract: printer doesn't fail, but prints the field reference even if there is no declaration visible + assertEquals("testedField = 1", type.getMethodsByName("failingMethod").get(0).getBody().getStatement(0).toString()); } private final Set separators = new HashSet<>(Arrays.asList("->","::","...")); diff --git a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java index 81b60f482db..70672bc0c82 100644 --- a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java +++ b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java @@ -18,14 +18,18 @@ import org.junit.Test; import spoon.Launcher; +import spoon.processing.Processor; import spoon.reflect.code.CtStatement; import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtCompilationUnit; import spoon.reflect.declaration.CtField; import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.ModifierKind; import spoon.reflect.factory.Factory; import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.visitor.ImportValidator; +import spoon.reflect.visitor.NameConflictValidator; import spoon.support.modelobs.ChangeCollector; import spoon.support.sniper.SniperJavaPrettyPrinter; import spoon.test.prettyprinter.testclasses.ToBeChanged; @@ -35,6 +39,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; +import java.util.Arrays; import java.util.Collections; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -175,8 +180,17 @@ private void testSniper(String testClass, Consumer> transformation, Bi Launcher launcher = new Launcher(); launcher.addInputResource(getResourcePath(testClass)); launcher.getEnvironment().setPrettyPrinterCreator(() -> { - return new SniperJavaPrettyPrinter(launcher.getEnvironment()); } - ); + SniperJavaPrettyPrinter printer = new SniperJavaPrettyPrinter(launcher.getEnvironment()); + printer.setPreprocessors(Collections.unmodifiableList(Arrays.>asList( + //remove unused imports first. Do not add new imports at time when conflicts are not resolved + new ImportValidator().setCanAddImports(false), + //solve conflicts, the current imports are relevant too + new NameConflictValidator(), + //compute final imports + new ImportValidator() + ))); + return printer; + }); launcher.buildModel(); Factory f = launcher.getFactory(); @@ -228,12 +242,6 @@ private static String getResourcePath(String className) { private void assertIsPrintedWithExpectedChanges(CtType ctClass, String printedSource, String... regExpReplacements) { assertEquals(0, regExpReplacements.length % 2); String originSource = ctClass.getPosition().getCompilationUnit().getOriginalSourceCode(); - //TODO REMOVE THIS BLOCK AFTER SNIPER PRINTING OF IMPORTS WORKS - { - //skip imports, which are not handled well yet - originSource = sourceWithoutImports(originSource); - printedSource = sourceWithoutImports(printedSource); - } //apply all expected replacements using Regular expressions int nrChanges = regExpReplacements.length / 2; for (int i = 0; i < nrChanges; i++) { diff --git a/src/test/java/spoon/test/prettyprinter/testclasses/ImportStatic.java b/src/test/java/spoon/test/prettyprinter/testclasses/ImportStatic.java index f34fa181cd5..65f2db9a5b7 100644 --- a/src/test/java/spoon/test/prettyprinter/testclasses/ImportStatic.java +++ b/src/test/java/spoon/test/prettyprinter/testclasses/ImportStatic.java @@ -4,13 +4,18 @@ import static org.junit.Assert.assertTrue; +import static java.lang.System.out; + /** * Created by urli on 30/01/2017. */ public class ImportStatic { - public static void main(String[] args) throws Exception { + public static void main(String[] args, java.lang.String[] args2, + String args3, java.lang.String args4) throws Exception { assertTrue("blabla".equals("toto")); + out.println(Constants.READY); System.out.println(Constants.READY); + java.lang.System.out.println(Constants.READY); } } diff --git a/src/test/java/spoon/test/reference/TypeReferenceTest.java b/src/test/java/spoon/test/reference/TypeReferenceTest.java index b0a50d48182..5a921f2b415 100644 --- a/src/test/java/spoon/test/reference/TypeReferenceTest.java +++ b/src/test/java/spoon/test/reference/TypeReferenceTest.java @@ -666,18 +666,18 @@ public void testTypeReferenceImplicitParent() throws Exception { // contract: CtTypeReference#isImplicitParent can be used read / write implicit value of the parent CtType type = ModelUtils.buildClass(SuperAccess.class); CtTypeReference typeRef = type.getSuperclass(); - assertFalse(typeRef.isImplicitParent()); - assertFalse(typeRef.getPackage().isImplicit()); - - //calling of setImplicitParent influences implicitnes of parent - typeRef.setImplicitParent(true); assertTrue(typeRef.isImplicitParent()); assertTrue(typeRef.getPackage().isImplicit()); - - //calling of setImplicit on parent influences return value of isImplicitParent - typeRef.getPackage().setImplicit(false); + + //calling of setImplicitParent influences implicitnes of parent + typeRef.setImplicitParent(false); assertFalse(typeRef.isImplicitParent()); assertFalse(typeRef.getPackage().isImplicit()); + + //calling of setImplicit on parent influences return value of isImplicitParent + typeRef.getPackage().setImplicit(true); + assertTrue(typeRef.isImplicitParent()); + assertTrue(typeRef.getPackage().isImplicit()); } @Test diff --git a/src/test/java/spoon/test/targeted/TargetedExpressionTest.java b/src/test/java/spoon/test/targeted/TargetedExpressionTest.java index 95001e5db73..e9f154c8b4e 100644 --- a/src/test/java/spoon/test/targeted/TargetedExpressionTest.java +++ b/src/test/java/spoon/test/targeted/TargetedExpressionTest.java @@ -191,8 +191,8 @@ public void testTargetsOfStaticFieldAccess() throws Exception { final List> staticElements = staticInit.getElements(new TypeFilter<>(CtFieldAccess.class)); assertEquals(1, staticElements.size()); - // Changing behaviour when writing static field, it is now writed using the class name - assertEqualsFieldAccess(new ExpectedTargetedExpression().type(CtFieldWrite.class).declaringType(expectedType).target(expectedTypeAccess).result("p"), staticElements.get(0)); + // Changing behaviour when writing static field, it is now written using the class name + assertEqualsFieldAccess(new ExpectedTargetedExpression().type(CtFieldWrite.class).declaringType(expectedType).target(expectedTypeAccess).result("spoon.test.targeted.testclasses.Foo.p"), staticElements.get(0)); } @Test diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index a8ecd76b831..f5c5bffac9b 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -90,8 +90,9 @@ public void testNoFQNWhenUsedInInnerClassAndShadowedByLocalVariable() { String output = "target/spooned-" + this.getClass().getSimpleName() + "-StaticMethod/"; String pathResource = "src/test/java/spoon/test/variable/testclasses/BurritosStaticMethod.java"; String result = this.buildResourceAndReturnResult(pathResource, output); - - assertTrue("The inner class should contain call using import", result.contains(" toto();")); + //the package name `spoon.test.variable.testclasses` cannot be used in FQN mode because it is shadowed by local variable `spoon` + //so use at least Type name + assertTrue("The inner class should contain call using import", result.contains(" BurritosStaticMethod.toto();")); canBeBuilt(output, 7); } diff --git a/src/test/java/spoon/test/visibility/VisibilityTest.java b/src/test/java/spoon/test/visibility/VisibilityTest.java index 910828839c3..e19e9520408 100644 --- a/src/test/java/spoon/test/visibility/VisibilityTest.java +++ b/src/test/java/spoon/test/visibility/VisibilityTest.java @@ -172,6 +172,6 @@ public boolean matches(CtInvocation element) { }).get(0); assertNotNull(ctInvocation.getTarget()); assertTrue(ctInvocation.getTarget().isImplicit()); - assertEquals("bound()", ctInvocation.toString()); + assertEquals("bound()", ctInvocation.print()); } } From 905c591d379adf4a20ca838f00f164dbbb7e67dd Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Sat, 31 Aug 2019 07:58:51 +0100 Subject: [PATCH 02/25] adaptation to spoon master --- .../spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 3 ++- src/main/java/spoon/reflect/visitor/ImportValidator.java | 7 ++++++- .../spoon/support/reflect/declaration/CtImportImpl.java | 4 ++++ .../spoon/support/reflect/declaration/CtTypeImpl.java | 9 ++++----- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index c82acdfb7f9..c92069c90b1 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -8,6 +8,7 @@ import spoon.SpoonException; import spoon.compiler.Environment; import spoon.experimental.CtUnresolvedImport; +import spoon.processing.Processor; import spoon.reflect.code.BinaryOperatorKind; import spoon.reflect.code.CtAnnotationFieldAccess; import spoon.reflect.code.CtArrayAccess; @@ -919,7 +920,7 @@ public void visitUnresolvedImport(CtUnresolvedImport ctUnresolvedImport) { } printer.writeCodeSnippet(ctUnresolvedImport.getUnresolvedReference()); } - } + }); printer.writeSeparator(";"); } } diff --git a/src/main/java/spoon/reflect/visitor/ImportValidator.java b/src/main/java/spoon/reflect/visitor/ImportValidator.java index 2c70250eb4f..684a49e2ddb 100644 --- a/src/main/java/spoon/reflect/visitor/ImportValidator.java +++ b/src/main/java/spoon/reflect/visitor/ImportValidator.java @@ -27,6 +27,7 @@ import java.util.stream.Collectors; import spoon.SpoonException; +import spoon.experimental.CtUnresolvedImport; import spoon.reflect.code.CtExpression; import spoon.reflect.code.CtFieldAccess; import spoon.reflect.code.CtInvocation; @@ -198,7 +199,11 @@ void onComilationUnitProcessed(CtCompilationUnit compilationUnit) { } //the import doesn't exist in computed imports. Remove it if (canRemoveImports) { - existingImports.remove(oldImport); + if (oldImport instanceof CtUnresolvedImport) { + //never remove unresolved imports + } else { + existingImports.remove(oldImport); + } } } } diff --git a/src/main/java/spoon/support/reflect/declaration/CtImportImpl.java b/src/main/java/spoon/support/reflect/declaration/CtImportImpl.java index adc1f02ff83..16459827c09 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtImportImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtImportImpl.java @@ -6,6 +6,7 @@ package spoon.support.reflect.declaration; import spoon.SpoonException; +import spoon.experimental.CtUnresolvedImport; import spoon.reflect.annotations.MetamodelPropertyField; import spoon.reflect.path.CtRole; import spoon.reflect.reference.CtExecutableReference; @@ -90,6 +91,9 @@ public void accept(CtImportVisitor visitor) { case ALL_STATIC_MEMBERS: visitor.visitAllStaticMembersImport((CtTypeMemberWildcardImportReference) localReference); break; + case UNRESOLVED: + visitor.visitUnresolvedImport((CtUnresolvedImport) localReference); + break; default: throw new SpoonException("Unexpected import kind: " + getImportKind()); } diff --git a/src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java b/src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java index d298f0e4276..c810b446498 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java @@ -990,10 +990,9 @@ public boolean isArray() { @Override public String toStringWithImports() { - DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(getFactory().getEnvironment()); - printer.getImportsContext().computeImports(this); - printer.writeHeader(Arrays.asList(new CtType[] {this}), printer.getImportsContext().getAllImports()); - this.accept(printer); - return printer.toString(); + DefaultJavaPrettyPrinter printer = (DefaultJavaPrettyPrinter) getFactory().getEnvironment().createPrettyPrinter(); + //this call applies print validators, which modifies model before printing + //and then it prints everything including package and potentially imports + return printer.printCompilationUnit(this.getPosition().getCompilationUnit()); } } From a87793d8c717e5742b285926e49dbc451cc9bc57 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Sat, 31 Aug 2019 06:56:58 +0100 Subject: [PATCH 03/25] fix tests --- .../java/spoon/test/ctClass/CtClassTest.java | 54 +++++++++---------- .../java/spoon/test/imports/ImportTest.java | 2 +- .../test/initializers/InitializerTest.java | 5 +- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/test/java/spoon/test/ctClass/CtClassTest.java b/src/test/java/spoon/test/ctClass/CtClassTest.java index 8915dbfbb3e..e04cd65e7c1 100644 --- a/src/test/java/spoon/test/ctClass/CtClassTest.java +++ b/src/test/java/spoon/test/ctClass/CtClassTest.java @@ -222,6 +222,8 @@ public void testSpoonShouldInferImplicitPackageInNoClasspath() { @Test public void toStringWithImports() { + String newLine = System.getProperty("line.separator"); + final Launcher launcher2 = new Launcher(); launcher2.addInputResource("./src/test/java/spoon/test/ctClass/"); launcher2.getEnvironment().setNoClasspath(true); @@ -231,39 +233,33 @@ public void toStringWithImports() { aClass2.accept(djpp); // contract: a class can be printed with full context - assertEquals("package spoon.test.ctClass.testclasses;\n" + - "\n" + - "\n" + - "/**\n" + - " * Created by urli on 11/10/2017.\n" + - " */\n" + - "public class AnonymousClass {\n" + - " final int machin = new java.util.Comparator() {\n" + - " @java.lang.Override\n" + - " public int compare(java.lang.Integer o1, java.lang.Integer o2) {\n" + - " return 0;\n" + - " }\n" + - " }.compare(1, 2);\n" + + assertEquals("package spoon.test.ctClass.testclasses;" + newLine + + "/**" + newLine + + " * Created by urli on 11/10/2017." + newLine + + " */" + newLine + + "public class AnonymousClass {" + newLine + + " final int machin = new java.util.Comparator() {" + newLine + + " @java.lang.Override" + newLine + + " public int compare(java.lang.Integer o1, java.lang.Integer o2) {" + newLine + + " return 0;" + newLine + + " }" + newLine + + " }.compare(1, 2);" + newLine + "}", aClass2.toStringWithImports()); // contract: a class can be printed with full context in autoimports aClass2.getFactory().getEnvironment().setAutoImports(true); - assertEquals("package spoon.test.ctClass.testclasses;\n" + - "\n" + - "\n" + - "import java.util.Comparator;\n" + - "\n" + - "\n" + - "/**\n" + - " * Created by urli on 11/10/2017.\n" + - " */\n" + - "public class AnonymousClass {\n" + - " final int machin = new Comparator() {\n" + - " @Override\n" + - " public int compare(Integer o1, Integer o2) {\n" + - " return 0;\n" + - " }\n" + - " }.compare(1, 2);\n" + + assertEquals("package spoon.test.ctClass.testclasses;" + newLine + + "import java.util.Comparator;" + newLine + + "/**" + newLine + + " * Created by urli on 11/10/2017." + newLine + + " */" + newLine + + "public class AnonymousClass {" + newLine + + " final int machin = new Comparator() {" + newLine + + " @Override" + newLine + + " public int compare(Integer o1, Integer o2) {" + newLine + + " return 0;" + newLine + + " }" + newLine + + " }.compare(1, 2);" + newLine + "}", aClass2.toStringWithImports()); } diff --git a/src/test/java/spoon/test/imports/ImportTest.java b/src/test/java/spoon/test/imports/ImportTest.java index 3223013427f..3d81fee7ee9 100644 --- a/src/test/java/spoon/test/imports/ImportTest.java +++ b/src/test/java/spoon/test/imports/ImportTest.java @@ -1626,7 +1626,7 @@ public void testMethodChainAutoImports() { List 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("mButton = ((android.widget.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()); diff --git a/src/test/java/spoon/test/initializers/InitializerTest.java b/src/test/java/spoon/test/initializers/InitializerTest.java index b3c3430791d..cd5472c48e6 100644 --- a/src/test/java/spoon/test/initializers/InitializerTest.java +++ b/src/test/java/spoon/test/initializers/InitializerTest.java @@ -27,6 +27,7 @@ import spoon.reflect.declaration.ModifierKind; import spoon.reflect.visitor.filter.NamedElementFilter; import spoon.reflect.visitor.filter.TypeFilter; +import spoon.test.imports.ImportTest; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -88,10 +89,10 @@ 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("UnicodeUtil.UTF8Result temp = new UnicodeUtil.UTF8Result()", + assertEquals("org.apache.lucene.util.UnicodeUtil.UTF8Result temp = new org.apache.lucene.util.UnicodeUtil.UTF8Result()", ex.getBody().getStatements().get(0).toString()); assertEquals("temp.result = new byte[0]", ex.getBody().getStatements().get(1).toString()); - assertTrue(ctClass.toString().contains("UnicodeUtil.UTF8Result temp = new UnicodeUtil.UTF8Result()")); + assertTrue(ImportTest.printByPrinter(ctClass).contains("UnicodeUtil.UTF8Result temp = new UnicodeUtil.UTF8Result()")); } } From 6e9384d1f6b7dfdf86d008b4f09218afe4def889 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Sat, 31 Aug 2019 09:10:11 +0100 Subject: [PATCH 04/25] test: CompilationTest#testCompileUnresolvedFullyQualifiedName --- .../spoon/test/compilation/CompilationTest.java | 9 +++++++++ .../UnresolvedFullQualifiedType.java | 16 ++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 src/test/resources/compilation2/UnresolvedFullQualifiedType.java diff --git a/src/test/java/spoon/test/compilation/CompilationTest.java b/src/test/java/spoon/test/compilation/CompilationTest.java index 03cb1d91ba9..6eb85d54fec 100644 --- a/src/test/java/spoon/test/compilation/CompilationTest.java +++ b/src/test/java/spoon/test/compilation/CompilationTest.java @@ -496,6 +496,15 @@ public void testCompilationInEmptyDir() throws Exception { assertThat(tempDirPath.toFile().listFiles().length, not(0)); } + + @Test + public void testCompileUnresolvedFullyQualifiedName() { + //contract: the unresolved fully qualified type reference must not cause model building problem + Launcher l = new Launcher(); + l.getEnvironment().setNoClasspath(true); + l.addInputResource("src/test/resources/compilation2/UnresolvedFullQualifiedType.java"); + l.buildModel(); + } @Test public void testBuildAstWithSyntheticMethods() { diff --git a/src/test/resources/compilation2/UnresolvedFullQualifiedType.java b/src/test/resources/compilation2/UnresolvedFullQualifiedType.java new file mode 100644 index 00000000000..f0ccdd5ee99 --- /dev/null +++ b/src/test/resources/compilation2/UnresolvedFullQualifiedType.java @@ -0,0 +1,16 @@ +package spoon.test.compilation.testclasses; + +import static daikon.PptRelation.PptRelationType; + +public final class UnresolvedFullQualifiedType { + static final class ParentRelation implements java.io.Serializable { + daikon.PptRelation.PptRelationType rel_type; + } + + /** + * Parses a ppt parent hierarchy record and returns it. * + */ + private static void m() { + String.valueOf(daikon.PptRelation.PptRelationType.class); + } +} \ No newline at end of file From 7cd21ddde84ce3f477a91d1d764ec9fd75009903 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Sat, 31 Aug 2019 16:07:26 +0100 Subject: [PATCH 05/25] partial fix of 3087 --- .../support/compiler/jdt/JDTTreeBuilderHelper.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java index 5c57948edd8..97fc410de08 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java @@ -31,6 +31,7 @@ import org.eclipse.jdt.internal.compiler.lookup.ProblemBinding; import org.eclipse.jdt.internal.compiler.lookup.ProblemReferenceBinding; import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding; +import org.eclipse.jdt.internal.compiler.lookup.TagBits; import org.eclipse.jdt.internal.compiler.lookup.TypeBinding; import org.eclipse.jdt.internal.compiler.lookup.VariableBinding; import spoon.SpoonException; @@ -537,6 +538,16 @@ private static void handleImplicit(PackageBinding packageBinding, char[][] token packageRef.setImplicit(true); return; } + if ((packageBinding.tagBits & TagBits.HasMissingType) != 0) { + /* + * the packageBinding points to unresolved type like: import static daikon.PptRelation.PptRelationType; + * where packageBinding is 'daikon.PptRelation' + * but it is not clear whether `PptRelation` is type or package. + * JDT compiler expects it is package, while Spoon model expects it is type + */ + //keep it explicit + return; + } throw new SpoonException("Unexpected QualifiedNameReference tokens " + qualifiedNameReference + " for typeRef: " + originTypeRef); } } From b01c6e4f932cd011ccf431f0521841ebc33364a5 Mon Sep 17 00:00:00 2001 From: tdurieux Date: Sat, 31 Aug 2019 20:36:06 +0200 Subject: [PATCH 06/25] Fix auto import --- .../reflect/visitor/ImportValidator.java | 22 +++++++++---------- .../compiler/jdt/JDTBasedSpoonCompiler.java | 2 +- .../compiler/jdt/JDTTreeBuilderHelper.java | 2 +- .../compiler/jdt/ReferenceBuilder.java | 12 +++++++++- .../DefaultPrettyPrinterTest.java | 2 +- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/ImportValidator.java b/src/main/java/spoon/reflect/visitor/ImportValidator.java index 684a49e2ddb..3a800d46fd0 100644 --- a/src/main/java/spoon/reflect/visitor/ImportValidator.java +++ b/src/main/java/spoon/reflect/visitor/ImportValidator.java @@ -16,16 +16,6 @@ */ package spoon.reflect.visitor; -import java.util.ArrayList; -import java.util.Comparator; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; - import spoon.SpoonException; import spoon.experimental.CtUnresolvedImport; import spoon.reflect.code.CtExpression; @@ -51,6 +41,16 @@ import spoon.support.util.ModelList; import spoon.support.visitor.ClassTypingContext; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + /** * Updates list of import statements of compilation unit following {@link CtElement#isImplicit()}. @@ -113,7 +113,7 @@ protected void handleTypeReference(Context context, CtRole role, CtTypeReference } //else do nothing. E.g. in case of implicit type of lambda parameter //`(e) -> {...}` - } else if (reference.isImplicitParent()) { + } else if (reference.isImplicitParent() && !(role.equals(CtRole.DECLARING_TYPE) && reference.getParent() instanceof CtTypeReference)) { /* * the package is implicit. E.g. `Assert.assertTrue` * where package `org.junit` is implicit diff --git a/src/main/java/spoon/support/compiler/jdt/JDTBasedSpoonCompiler.java b/src/main/java/spoon/support/compiler/jdt/JDTBasedSpoonCompiler.java index 99f8df007f6..811c1ee039b 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTBasedSpoonCompiler.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTBasedSpoonCompiler.java @@ -583,7 +583,7 @@ public void reportProblem(CategorizedProblem pb) { public void reportProblemsWhenCompiling(Environment environment) { for (CategorizedProblem problem : getProblems()) { if (problem != null) { - String message = problem.getMessage() + " at " + problem.getOriginatingFileName() + ":" + problem.getSourceLineNumber(); + String message = problem.getMessage() + " at " + new String(problem.getOriginatingFileName()) + ":" + problem.getSourceLineNumber(); if (problem.isError()) { throw new SnippetCompilationError(message); } else { diff --git a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java index 97fc410de08..ec5160b6979 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java @@ -538,7 +538,7 @@ private static void handleImplicit(PackageBinding packageBinding, char[][] token packageRef.setImplicit(true); return; } - if ((packageBinding.tagBits & TagBits.HasMissingType) != 0) { + if (packageBinding == null || (packageBinding.tagBits & TagBits.HasMissingType) != 0) { /* * the packageBinding points to unresolved type like: import static daikon.PptRelation.PptRelationType; * where packageBinding is 'daikon.PptRelation' diff --git a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java index d4baee989b3..d427df3fdee 100644 --- a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java @@ -900,7 +900,17 @@ CtTypeReference getTypeReference(TypeBinding binding, boolean resolveGene } else if (binding instanceof ProblemReferenceBinding) { // Spoon is able to analyze also without the classpath ref = this.jdtTreeBuilder.getFactory().Core().createTypeReference(); - ref.setSimpleName(new String(binding.readableName())); + char[] readableName = binding.readableName(); + StringBuilder sb = new StringBuilder(); + for (int i = readableName.length - 1; i >= 0; i--) { + char c = readableName[i]; + if (c == '.') { + break; + } + sb.append(c); + } + sb.reverse(); + ref.setSimpleName(sb.toString()); final CtReference declaring = this.getDeclaringReferenceFromImports(binding.sourceName()); setPackageOrDeclaringType(ref, declaring); } else if (binding instanceof JDTTreeBuilder.SpoonReferenceBinding) { diff --git a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java index f0592905d25..46e3fae4de4 100644 --- a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java +++ b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java @@ -407,7 +407,7 @@ public void testElseIf() { CtModel model = launcher.buildModel(); CtType a5 = model.getRootPackage().getType("A6"); String result = a5.toStringWithImports(); - String expected = "\n\npublic class A6 {\n" + + String expected = "public class A6 {\n" + " public static void main(String[] args) {\n" + " int a = 1;\n" + " if (a == 1) {\n" + From 09538041854990b516cf34db38fbb4bf8cdb6486 Mon Sep 17 00:00:00 2001 From: tdurieux Date: Sun, 1 Sep 2019 07:50:40 +0200 Subject: [PATCH 07/25] add license --- .../AbstractCompilationUnitImportsProcessor.java | 15 ++------------- .../reflect/visitor/DefaultImportComparator.java | 15 ++------------- .../visitor/ForceFullyQualifiedProcessor.java | 15 ++------------- .../reflect/visitor/ForceImportProcessor.java | 15 ++------------- .../spoon/reflect/visitor/ImportValidator.java | 15 ++------------- .../reflect/visitor/NameConflictValidator.java | 15 ++------------- 6 files changed, 12 insertions(+), 78 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java index 1fc865cb1db..2007b84f859 100644 --- a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java +++ b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java @@ -1,18 +1,7 @@ /** - * Copyright (C) 2006-2018 INRIA and contributors - * Spoon - http://spoon.gforge.inria.fr/ + * Copyright (C) 2006-2019 INRIA and contributors * - * This software is governed by the CeCILL-C License under French law and - * abiding by the rules of distribution of free software. You can use, modify - * and/or redistribute the software under the terms of the CeCILL-C license as - * circulated by CEA, CNRS and INRIA at http://www.cecill.info. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. - * - * The fact that you are presently reading this means that you have had - * knowledge of the CeCILL-C license and that you accept its terms. + * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. */ package spoon.reflect.visitor; diff --git a/src/main/java/spoon/reflect/visitor/DefaultImportComparator.java b/src/main/java/spoon/reflect/visitor/DefaultImportComparator.java index 954978af0db..c6db15fe0d2 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultImportComparator.java +++ b/src/main/java/spoon/reflect/visitor/DefaultImportComparator.java @@ -1,18 +1,7 @@ /** - * Copyright (C) 2006-2018 INRIA and contributors - * Spoon - http://spoon.gforge.inria.fr/ + * Copyright (C) 2006-2019 INRIA and contributors * - * This software is governed by the CeCILL-C License under French law and - * abiding by the rules of distribution of free software. You can use, modify - * and/or redistribute the software under the terms of the CeCILL-C license as - * circulated by CEA, CNRS and INRIA at http://www.cecill.info. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. - * - * The fact that you are presently reading this means that you have had - * knowledge of the CeCILL-C license and that you accept its terms. + * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. */ package spoon.reflect.visitor; diff --git a/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java b/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java index 439e64877ad..ac3e49a49e6 100644 --- a/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java +++ b/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java @@ -1,18 +1,7 @@ /** - * Copyright (C) 2006-2018 INRIA and contributors - * Spoon - http://spoon.gforge.inria.fr/ + * Copyright (C) 2006-2019 INRIA and contributors * - * This software is governed by the CeCILL-C License under French law and - * abiding by the rules of distribution of free software. You can use, modify - * and/or redistribute the software under the terms of the CeCILL-C license as - * circulated by CEA, CNRS and INRIA at http://www.cecill.info. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. - * - * The fact that you are presently reading this means that you have had - * knowledge of the CeCILL-C license and that you accept its terms. + * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. */ package spoon.reflect.visitor; diff --git a/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java b/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java index 8f94fc8dbdf..309de259d0e 100644 --- a/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java +++ b/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java @@ -1,18 +1,7 @@ /** - * Copyright (C) 2006-2018 INRIA and contributors - * Spoon - http://spoon.gforge.inria.fr/ + * Copyright (C) 2006-2019 INRIA and contributors * - * This software is governed by the CeCILL-C License under French law and - * abiding by the rules of distribution of free software. You can use, modify - * and/or redistribute the software under the terms of the CeCILL-C license as - * circulated by CEA, CNRS and INRIA at http://www.cecill.info. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. - * - * The fact that you are presently reading this means that you have had - * knowledge of the CeCILL-C license and that you accept its terms. + * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. */ package spoon.reflect.visitor; diff --git a/src/main/java/spoon/reflect/visitor/ImportValidator.java b/src/main/java/spoon/reflect/visitor/ImportValidator.java index 3a800d46fd0..0796c1d93ed 100644 --- a/src/main/java/spoon/reflect/visitor/ImportValidator.java +++ b/src/main/java/spoon/reflect/visitor/ImportValidator.java @@ -1,18 +1,7 @@ /** - * Copyright (C) 2006-2018 INRIA and contributors - * Spoon - http://spoon.gforge.inria.fr/ + * Copyright (C) 2006-2019 INRIA and contributors * - * This software is governed by the CeCILL-C License under French law and - * abiding by the rules of distribution of free software. You can use, modify - * and/or redistribute the software under the terms of the CeCILL-C license as - * circulated by CEA, CNRS and INRIA at http://www.cecill.info. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. - * - * The fact that you are presently reading this means that you have had - * knowledge of the CeCILL-C license and that you accept its terms. + * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. */ package spoon.reflect.visitor; diff --git a/src/main/java/spoon/reflect/visitor/NameConflictValidator.java b/src/main/java/spoon/reflect/visitor/NameConflictValidator.java index fe9139bf5c0..f31ac02fe98 100644 --- a/src/main/java/spoon/reflect/visitor/NameConflictValidator.java +++ b/src/main/java/spoon/reflect/visitor/NameConflictValidator.java @@ -1,18 +1,7 @@ /** - * Copyright (C) 2006-2018 INRIA and contributors - * Spoon - http://spoon.gforge.inria.fr/ + * Copyright (C) 2006-2019 INRIA and contributors * - * This software is governed by the CeCILL-C License under French law and - * abiding by the rules of distribution of free software. You can use, modify - * and/or redistribute the software under the terms of the CeCILL-C license as - * circulated by CEA, CNRS and INRIA at http://www.cecill.info. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. - * - * The fact that you are presently reading this means that you have had - * knowledge of the CeCILL-C license and that you accept its terms. + * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. */ package spoon.reflect.visitor; From d0a4a1315e4a99677578dca53d68bfad5bb8de7e Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Sun, 1 Sep 2019 08:47:57 +0100 Subject: [PATCH 08/25] move fix to better place and added comment which explains context --- ...stractCompilationUnitImportsProcessor.java | 19 ++++++++++++++++++- .../reflect/visitor/ImportValidator.java | 2 +- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java index 2007b84f859..b7d4d3750c9 100644 --- a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java +++ b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java @@ -76,6 +76,10 @@ protected CtScannerListener createScannerListener(T scanner) { CtRole.TYPE )); + /** + * {@link CtScannerListener} implementation which stops scanning of children on elements, + * which mustn't have influence to compilation unit imports. + */ protected class ScannerListener implements CtScannerListener { protected T scanner; protected Set ignoredRoles = IGNORED_ROLES_WHEN_IMPLICIT; @@ -109,7 +113,20 @@ public ScanningMode enter(CtRole role, CtElement element) { */ return ScanningMode.SKIP_ALL; } else if (parent instanceof CtTypeReference) { - //continue. This is relevant for import + /* + * It looks like this is not needed too. + * + * pvojtechovsky: I am sure it is not wanted in case of + * spoon.test.imports.testclasses.internal.ChildClass.InnerClassProtected + * which extends package protected (and for others invisible class) + * spoon.test.imports.testclasses.internal.SuperClass + * and in this case the import directive must import ...ChildClass and not ...SuperClass, + * because import is using type "access path" and not qualified name of the type. + * + * ... but in other normal cases, I guess the declaring type is used and needed for import! + * ... so I don't understand why SKIP_ALL works in all cases. May be there is missing test case? + */ + return ScanningMode.SKIP_ALL; } else { //May be this can never happen throw new SpoonException("Check this case. Is it relvant or not?"); diff --git a/src/main/java/spoon/reflect/visitor/ImportValidator.java b/src/main/java/spoon/reflect/visitor/ImportValidator.java index 0796c1d93ed..19606786ab4 100644 --- a/src/main/java/spoon/reflect/visitor/ImportValidator.java +++ b/src/main/java/spoon/reflect/visitor/ImportValidator.java @@ -102,7 +102,7 @@ protected void handleTypeReference(Context context, CtRole role, CtTypeReference } //else do nothing. E.g. in case of implicit type of lambda parameter //`(e) -> {...}` - } else if (reference.isImplicitParent() && !(role.equals(CtRole.DECLARING_TYPE) && reference.getParent() instanceof CtTypeReference)) { + } else if (reference.isImplicitParent()) { /* * the package is implicit. E.g. `Assert.assertTrue` * where package `org.junit` is implicit From 819ba02f333cfec7030c18fa1a88c35ec6d2aa7f Mon Sep 17 00:00:00 2001 From: tdurieux Date: Sun, 1 Sep 2019 12:54:20 +0200 Subject: [PATCH 09/25] only skip declaring type when the access is different from the declaration --- .../AbstractCompilationUnitImportsProcessor.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java index b7d4d3750c9..ec9ddf59125 100644 --- a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java +++ b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java @@ -5,10 +5,6 @@ */ package spoon.reflect.visitor; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; - import spoon.SpoonException; import spoon.processing.AbstractProcessor; import spoon.processing.Processor; @@ -27,6 +23,10 @@ import spoon.reflect.visitor.chain.ScanningMode; import spoon.support.Experimental; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + /** * Internal generic {@link Processor} of {@link CtCompilationUnit}, which scans CtCompilationUnit modules, packages and types * with purpose to find type references and expressions which might influence import directives. @@ -126,7 +126,9 @@ public ScanningMode enter(CtRole role, CtElement element) { * ... but in other normal cases, I guess the declaring type is used and needed for import! * ... so I don't understand why SKIP_ALL works in all cases. May be there is missing test case? */ - return ScanningMode.SKIP_ALL; + if (!((CtTypeReference) parent).getAccessType().equals(element)) { + return ScanningMode.SKIP_ALL; + } } else { //May be this can never happen throw new SpoonException("Check this case. Is it relvant or not?"); From 2cedf18804773c984242e07fb2a2c46002fcd98a Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 7 Sep 2019 20:48:11 +0200 Subject: [PATCH 10/25] ok --- .../spoon/reflect/declaration/CtElement.java | 11 ++++++----- .../visitor/DefaultJavaPrettyPrinter.java | 2 +- .../spoon/reflect/visitor/PrettyPrinter.java | 4 ++++ .../reflect/declaration/CtElementImpl.java | 17 +++++++---------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/main/java/spoon/reflect/declaration/CtElement.java b/src/main/java/spoon/reflect/declaration/CtElement.java index 47bfe5860a4..81688d91c82 100644 --- a/src/main/java/spoon/reflect/declaration/CtElement.java +++ b/src/main/java/spoon/reflect/declaration/CtElement.java @@ -5,6 +5,7 @@ */ package spoon.reflect.declaration; +import spoon.compiler.Environment; import spoon.processing.FactoryAccessor; import spoon.reflect.code.CtComment; import spoon.reflect.cu.SourcePosition; @@ -392,12 +393,12 @@ List getAnnotatedChildren( List getDirectChildren(); /** - * @return pretty printed source code of this element. + * @return print source code of this element. * - * Note: `implicit` elements are not printed. - * The model is not modified by printing. - * It means it doesn't try to fix model inconsistencies (if any) - * so invalid model causes printing of invalid sources. + * {@link Environment#getToStringMode()} is not taken into account, it is only taken into account when writing to disk. */ + String toString(); + String print(); + } diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index c92069c90b1..cda391efd74 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -1935,7 +1935,7 @@ public String getResult() { return printer.getPrinterHelper().toString(); } - private void reset() { + public void reset() { printer.reset(); context = new PrintingContext(); } diff --git a/src/main/java/spoon/reflect/visitor/PrettyPrinter.java b/src/main/java/spoon/reflect/visitor/PrettyPrinter.java index e702e370143..5e1d058f291 100644 --- a/src/main/java/spoon/reflect/visitor/PrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/PrettyPrinter.java @@ -6,9 +6,11 @@ package spoon.reflect.visitor; import spoon.reflect.declaration.CtCompilationUnit; +import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtModule; import spoon.reflect.declaration.CtPackage; import spoon.reflect.declaration.CtType; +import spoon.support.reflect.declaration.CtElementImpl; import java.util.List; import java.util.Map; @@ -58,4 +60,6 @@ public interface PrettyPrinter { * code. */ Map getLineNumberMapping(); + + PrettyPrinter scan(CtElement ctElement); } diff --git a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java index 0525728ebe5..5cab0e88b5b 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java @@ -27,12 +27,14 @@ import spoon.reflect.path.CtRole; import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.visitor.CtAbstractVisitor; import spoon.reflect.visitor.CtIterator; import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; import spoon.reflect.visitor.EarlyTerminatingScanner; import spoon.reflect.visitor.Filter; import spoon.reflect.visitor.ModelConsistencyChecker; +import spoon.reflect.visitor.PrettyPrinter; import spoon.reflect.visitor.Query; import spoon.reflect.visitor.chain.CtConsumableFunction; import spoon.reflect.visitor.chain.CtFunction; @@ -275,21 +277,16 @@ public void enter(CtElement e) { return (E) this; } - @Override - public String toString() { - return print(true); - } - @Override public String print() { - return print(false); + PrettyPrinter printer = getFactory().getEnvironment().createPrettyPrinter(); + return printer.scan(this).getResult(); } - private String print(boolean forceFullyQualified) { + @Override + public String toString() { DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(getFactory().getEnvironment()); - printer.setForceFullyQualified(forceFullyQualified); - //the printer is created here directly without any ImportValidator - //which would change the model content - it is not wanted! + printer.setForceFullyQualified(true); String errorMessage = ""; try { printer.scan(this); From bb6c1c6e4c99a48a6be677a9d67adf439291dff2 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 7 Sep 2019 20:50:05 +0200 Subject: [PATCH 11/25] ok --- .../visitor/ForceFullyQualifiedProcessor.java | 22 +- .../reflect/visitor/ForceImportProcessor.java | 15 +- .../spoon/reflect/visitor/ImportAnalyzer.java | 236 ++++++++++++++++++ .../reflect/visitor/ImportValidator.java | 11 +- .../visitor/NameConflictValidator.java | 11 +- 5 files changed, 262 insertions(+), 33 deletions(-) create mode 100644 src/main/java/spoon/reflect/visitor/ImportAnalyzer.java diff --git a/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java b/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java index ac3e49a49e6..5a0c83a6079 100644 --- a/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java +++ b/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java @@ -22,20 +22,20 @@ * Removes all imports from compilation unit and forces fully qualified identifiers */ @Experimental -public class ForceFullyQualifiedProcessor extends AbstractCompilationUnitImportsProcessor { +public class ForceFullyQualifiedProcessor extends ImportAnalyzer { @Override - protected LexicalScopeScanner createRawScanner() { + protected LexicalScopeScanner createScanner() { return new LexicalScopeScanner(); } @Override - protected LexicalScope getContext(LexicalScopeScanner scanner) { + protected LexicalScope getScannerContextInformation(LexicalScopeScanner scanner) { return scanner.getCurrentLexicalScope(); } @Override - protected void handleTypeReference(LexicalScope nameScope, CtRole role, CtTypeReference reference) { + protected void handleTypeReference(CtTypeReference reference, LexicalScope nameScope, CtRole role) { if (reference.isImplicitParent() || reference.isImplicit()) { if (isThisAccess(reference)) { //do not force FQ names in this access @@ -58,17 +58,6 @@ protected void handleTypeReference(LexicalScope nameScope, CtRole role, CtTypeRe } protected boolean isTypeReferenceToEnclosingType(LexicalScope nameScope, CtTypeReference reference) { -// String qName = reference.getQualifiedName(); -// return Boolean.TRUE == nameScope.forEachElementByName(reference.getSimpleName(), named -> { -// if (named instanceof CtType) { -// CtType type = (CtType) named; -// if (qName.equals(type.getQualifiedName())) { -// return Boolean.TRUE; -// } -// } -// return null; -// }); - //expected by LambdaTest CtType enclosingType = reference.getParent(CtType.class); return enclosingType == null ? false : reference.getQualifiedName().equals(enclosingType.getQualifiedName()); } @@ -85,7 +74,8 @@ private boolean isSupertypeOfNewClass(CtTypeReference typeRef) { } @Override - protected void handleTargetedExpression(LexicalScope nameScope, CtRole role, CtTargetedExpression targetedExpression, CtExpression target) { + protected void handleTargetedExpression(CtTargetedExpression targetedExpression, LexicalScope nameScope, CtRole role) { + CtExpression target = targetedExpression.getTarget(); if (!target.isImplicit()) { return; } diff --git a/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java b/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java index 309de259d0e..95469c43bb6 100644 --- a/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java +++ b/src/main/java/spoon/reflect/visitor/ForceImportProcessor.java @@ -16,23 +16,23 @@ /** - * Marks package references implicit so all type will get imported + * Marks all types references as implicit so all types will get imported. */ @Experimental -public class ForceImportProcessor extends AbstractCompilationUnitImportsProcessor { +public class ForceImportProcessor extends ImportAnalyzer { @Override - protected LexicalScopeScanner createRawScanner() { + protected LexicalScopeScanner createScanner() { return new LexicalScopeScanner(); } @Override - protected LexicalScope getContext(LexicalScopeScanner scanner) { + protected LexicalScope getScannerContextInformation(LexicalScopeScanner scanner) { return scanner.getCurrentLexicalScope(); } @Override - protected void handleTypeReference(LexicalScope nameScope, CtRole role, CtTypeReference reference) { + protected void handleTypeReference(CtTypeReference reference, LexicalScope nameScope, CtRole role) { if (reference.getPackage() != null) { //force import of package of top level types only reference.setImplicitParent(true); @@ -64,8 +64,9 @@ protected void handleTypeReference(LexicalScope nameScope, CtRole role, CtTypeRe } } - - protected void handleTargetedExpression(LexicalScope nameScope, CtRole role, CtTargetedExpression targetedExpression, CtExpression target) { + @Override + protected void handleTargetedExpression(CtTargetedExpression targetedExpression, LexicalScope nameScope, CtRole role) { + CtExpression target = targetedExpression.getTarget(); if (target.isImplicit()) { return; } diff --git a/src/main/java/spoon/reflect/visitor/ImportAnalyzer.java b/src/main/java/spoon/reflect/visitor/ImportAnalyzer.java new file mode 100644 index 00000000000..1257fe7b83f --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/ImportAnalyzer.java @@ -0,0 +1,236 @@ +/** + * Copyright (C) 2006-2019 INRIA and contributors + * + * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. + */ +package spoon.reflect.visitor; + +import spoon.SpoonException; +import spoon.processing.AbstractProcessor; +import spoon.processing.Processor; +import spoon.reflect.code.CtConstructorCall; +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtTargetedExpression; +import spoon.reflect.declaration.CtCompilationUnit; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtPackage; +import spoon.reflect.path.CtRole; +import spoon.reflect.reference.CtExecutableReference; +import spoon.reflect.reference.CtFieldReference; +import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.reference.CtVariableReference; +import spoon.reflect.visitor.chain.CtScannerListener; +import spoon.reflect.visitor.chain.ScanningMode; +import spoon.support.Experimental; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +/** + *{@link Processor} of {@link CtCompilationUnit}, which scans CtCompilationUnit modules, packages and types + * with purpose to find type references and expressions which might influence import directives. + * + * Subclasses create a scanner ({@link #createScanner()}) and analyzes the elements to be imported {@link #handleTypeReference} and {@link #handleTargetedExpression(CtTargetedExpression, Object, CtRole)} + * + */ +@Experimental +abstract class ImportAnalyzer extends AbstractProcessor { + + @Override + public void process(CtCompilationUnit cu) { + T scanner = createScanner(); + if (scanner instanceof EarlyTerminatingScanner) { + CtScannerListener listener = createScannerListener(scanner); + if (listener != null) { + ((EarlyTerminatingScanner) scanner).setListener(listener); + } + } + process(scanner, cu); + } + + protected static void process(CtScanner scanner, CtCompilationUnit cu) { + scanner.enter(cu); + switch (cu.getUnitType()) { + case MODULE_DECLARATION: + case UNKNOWN: + break; + case PACKAGE_DECLARATION: + // we need to compute imports only for package annotations and package comments + // we don't want to get all imports coming from content of package + CtPackage pack = cu.getDeclaredPackage(); + scanner.scan(pack.getAnnotations()); + break; + case TYPE_DECLARATION: + for (CtTypeReference typeRef : cu.getDeclaredTypeReferences()) { + scanner.scan(typeRef.getTypeDeclaration()); + } + break; + } + scanner.exit(cu); + } + + protected CtScannerListener createScannerListener(T scanner) { + return new ScannerListener(scanner); + } + + //The set of roles whose values are always kept implicit + protected static Set IGNORED_ROLES_WHEN_IMPLICIT = new HashSet<>(Arrays.asList( + //e.g. List s = new ArrayList(); + CtRole.TYPE_ARGUMENT, + //e.g. List + CtRole.BOUNDING_TYPE, + //e.g. (/*implicit type of parameter*/ p) -> {} + CtRole.TYPE + )); + + /** + * {@link CtScannerListener} implementation which stops scanning of children on elements, + * which mustn't have influence to compilation unit imports. + */ + protected class ScannerListener implements CtScannerListener { + protected T scanner; + protected Set ignoredRoles = IGNORED_ROLES_WHEN_IMPLICIT; + + ScannerListener(T scanner) { + super(); + this.scanner = scanner; + } + + @Override + public ScanningMode enter(CtRole role, CtElement element) { + if (element == null) { + return ScanningMode.SKIP_ALL; + } + if (role == CtRole.VARIABLE && element instanceof CtVariableReference) { + //ignore variable reference of field access. The accessType is relevant here instead. + return ScanningMode.SKIP_ALL; + } + if (element.isParentInitialized()) { + CtElement parent = element.getParent(); + if (role == CtRole.DECLARING_TYPE && element instanceof CtTypeReference) { + if (parent instanceof CtFieldReference) { + //ignore the declaring type of field reference. It is not relevant for Imports + return ScanningMode.SKIP_ALL; + } + if (parent instanceof CtExecutableReference) { + /* + * ignore the declaring type of type executable like + * anVariable.getSomeInstance().callMethod() + * The declaring type of `callMethod` method is not relevant for Imports + */ + return ScanningMode.SKIP_ALL; + } else if (parent instanceof CtTypeReference) { + /* + * It looks like this is not needed too. + * + * pvojtechovsky: I am sure it is not wanted in case of + * spoon.test.imports.testclasses.internal.ChildClass.InnerClassProtected + * which extends package protected (and for others invisible class) + * spoon.test.imports.testclasses.internal.SuperClass + * and in this case the import directive must import ...ChildClass and not ...SuperClass, + * because import is using type "access path" and not qualified name of the type. + * + * ... but in other normal cases, I guess the declaring type is used and needed for import! + * ... so I don't understand why SKIP_ALL works in all cases. May be there is missing test case? + */ + if (!((CtTypeReference) parent).getAccessType().equals(element)) { + return ScanningMode.SKIP_ALL; + } + } else { + //May be this can never happen + throw new SpoonException("Check this case. Is it relvant or not?"); + } + } + if (role == CtRole.TYPE && element instanceof CtTypeReference) { + if (parent instanceof CtFieldReference) { + //ignore the type of field references. It is not relevant for Imports + return ScanningMode.SKIP_ALL; + } + if (parent instanceof CtExecutableReference) { + CtElement parent2 = null; + if (element.isParentInitialized()) { + parent2 = parent.getParent(); + } + if (parent2 instanceof CtConstructorCall) { + //new SomeType(); is relevant for import + //continue + } else { + /* + * ignore the return type of executable reference. It is not relevant for Imports + * anVariable.getSomeInstance().callMethod() + * The return type `callMethod` method is not relevant for Imports + */ + return ScanningMode.SKIP_ALL; + } + } + /* + * CtTypeReference + * CtMethod, CtField, ... + * continue. This is relevant for import + */ + } + if (role == CtRole.ARGUMENT_TYPE) { + /* + * ignore the type of parameter of CtExecutableReference + * It is not relevant for Imports. + */ + return ScanningMode.SKIP_ALL; + } + } + if (element.isImplicit() && ignoredRoles.contains(role)) { + //ignore implicit actual type arguments + return ScanningMode.SKIP_ALL; + } + onEnter(getScannerContextInformation(scanner), role, element); + return ScanningMode.NORMAL; + } + } + + + protected void onEnter(U context, CtRole role, CtElement element) { + + if (element instanceof CtTargetedExpression) { + CtTargetedExpression targetedExpression = (CtTargetedExpression) element; + CtExpression target = targetedExpression.getTarget(); + if (target == null) { + return; + } + handleTargetedExpression(targetedExpression, context, role); + } else if (element instanceof CtTypeReference) { + //we have to visit only PURE CtTypeReference. No CtArrayTypeReference, CtTypeParameterReference, ... + element.accept(new CtAbstractVisitor() { + @Override + public void visitCtTypeReference(CtTypeReference reference) { + handleTypeReference((CtTypeReference) element, context, role); + } + }); + } + } + + /** extract the required information from the scanner to take a decision */ + protected abstract U getScannerContextInformation(T scanner); + + /** creates the scanner that will be used to visit the model */ + protected abstract T createScanner(); + + /** what do we do a type reference? */ + protected abstract void handleTypeReference(CtTypeReference element, U context, CtRole role); + + /** what do we do a target expression (print target or not) ? */ + protected abstract void handleTargetedExpression(CtTargetedExpression targetedExpression, U context, CtRole role); + + /** + * @return parent of `element`, but only if it's type is `type` + */ + protected static T getParentIfType(CtElement element, Class type) { + if (element == null || !element.isParentInitialized()) { + return null; + } + CtElement parent = element.getParent(); + if (type.isInstance(parent)) { + return type.cast(parent); + } + return null; + } +} diff --git a/src/main/java/spoon/reflect/visitor/ImportValidator.java b/src/main/java/spoon/reflect/visitor/ImportValidator.java index 19606786ab4..c846b811235 100644 --- a/src/main/java/spoon/reflect/visitor/ImportValidator.java +++ b/src/main/java/spoon/reflect/visitor/ImportValidator.java @@ -47,24 +47,25 @@ * It doesn't fix wrong used implicit which causes conflicts. The fixing is task of {@link NameConflictValidator} */ @Experimental -public class ImportValidator extends AbstractCompilationUnitImportsProcessor { +public class ImportValidator extends ImportAnalyzer { private Comparator importComparator; private boolean canAddImports = true; private boolean canRemoveImports = true; @Override - protected MyScanner createRawScanner() { + protected MyScanner createScanner() { return new MyScanner(); } @Override - protected Context getContext(MyScanner scanner) { + protected Context getScannerContextInformation(MyScanner scanner) { return scanner.context; } @Override - protected void handleTargetedExpression(Context context, CtRole role, CtTargetedExpression targetedExpression, CtExpression target) { + protected void handleTargetedExpression(CtTargetedExpression targetedExpression, Context context, CtRole role) { + CtExpression target = targetedExpression.getTarget(); if (target != null && target.isImplicit()) { if (target instanceof CtTypeAccess) { if (targetedExpression instanceof CtFieldAccess) { @@ -82,7 +83,7 @@ protected void handleTargetedExpression(Context context, CtRole role, CtTargeted } @Override - protected void handleTypeReference(Context context, CtRole role, CtTypeReference reference) { + protected void handleTypeReference(CtTypeReference reference, Context context, CtRole role) { if (reference.isImplicit()) { /* * the reference is implicit. E.g. `assertTrue();` diff --git a/src/main/java/spoon/reflect/visitor/NameConflictValidator.java b/src/main/java/spoon/reflect/visitor/NameConflictValidator.java index f31ac02fe98..29ed6d176a2 100644 --- a/src/main/java/spoon/reflect/visitor/NameConflictValidator.java +++ b/src/main/java/spoon/reflect/visitor/NameConflictValidator.java @@ -50,20 +50,21 @@ * and fixes them by call of {@link CtElement#setImplicit(boolean)} and {@link CtTypeReference#setImplicitParent(boolean)} */ @Experimental -public class NameConflictValidator extends AbstractCompilationUnitImportsProcessor { +public class NameConflictValidator extends ImportAnalyzer { @Override - protected LexicalScopeScanner createRawScanner() { + protected LexicalScopeScanner createScanner() { return new LexicalScopeScanner(); } @Override - protected LexicalScope getContext(LexicalScopeScanner scanner) { + protected LexicalScope getScannerContextInformation(LexicalScopeScanner scanner) { return scanner.getCurrentLexicalScope(); } @Override - protected void handleTargetedExpression(LexicalScope nameScope, CtRole role, CtTargetedExpression targetedExpression, CtExpression target) { + protected void handleTargetedExpression(CtTargetedExpression targetedExpression, LexicalScope nameScope, CtRole role) { + CtExpression target = targetedExpression.getTarget(); if (targetedExpression instanceof CtFieldAccess) { CtFieldAccess fieldAccess = (CtFieldAccess) targetedExpression; if (target.isImplicit()) { @@ -102,7 +103,7 @@ protected void handleTargetedExpression(LexicalScope nameScope, CtRole role, CtT } @Override - protected void handleTypeReference(LexicalScope nameScope, CtRole role, CtTypeReference ref) { + protected void handleTypeReference(CtTypeReference ref, LexicalScope nameScope, CtRole role) { if (ref.isImplicit()) { /* * the reference is implicit. E.g. `assertTrue();` From 5d828c35f84ad68f2751184261551911ca08ee65 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 7 Sep 2019 20:52:23 +0200 Subject: [PATCH 12/25] ok --- src/main/java/spoon/compiler/Environment.java | 31 +++--- .../spoon/support/StandardEnvironment.java | 98 ++++++++++--------- 2 files changed, 66 insertions(+), 63 deletions(-) diff --git a/src/main/java/spoon/compiler/Environment.java b/src/main/java/spoon/compiler/Environment.java index 909b612a013..4b2f8897b21 100644 --- a/src/main/java/spoon/compiler/Environment.java +++ b/src/main/java/spoon/compiler/Environment.java @@ -8,24 +8,22 @@ import org.apache.log4j.Level; import spoon.OutputType; import spoon.compiler.builder.EncodingProvider; -import spoon.support.modelobs.FineModelChangeListener; import spoon.processing.FileGenerator; import spoon.processing.ProblemFixer; import spoon.processing.ProcessingManager; import spoon.processing.Processor; import spoon.processing.ProcessorProperties; -import spoon.reflect.declaration.CtCompilationUnit; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtMethod; import spoon.reflect.visitor.PrettyPrinter; -import spoon.support.OutputDestinationHandler; import spoon.support.CompressionType; +import spoon.support.OutputDestinationHandler; import spoon.support.compiler.SpoonProgress; +import spoon.support.modelobs.FineModelChangeListener; import spoon.support.sniper.SniperJavaPrettyPrinter; import java.io.File; import java.nio.charset.Charset; -import java.util.List; import java.util.function.Supplier; /** @@ -45,6 +43,10 @@ public interface Environment { */ void setComplianceLevel(int level); + TO_STRING_MODE getToStringMode(); + + void setToStringMode(TO_STRING_MODE toStringMode); + /** * This method should be called to print out a message with a source * position link during the processing. @@ -433,17 +435,14 @@ void report(Processor processor, Level level, */ void setIgnoreDuplicateDeclarations(boolean ignoreDuplicateDeclarations); - /** - * @return list of {@link Processor}, which are used to validate and fix model before it's printing - * - * Note: by default the validators depends on {@link #isAutoImports()} - */ - List> getCompilationUnitValidators(); + public enum TO_STRING_MODE { + /** no preprocessors of the model before pretty-printing */ + VANILLA, - /** - * @param compilationUnitValidators list of {@link Processor}, which have to be used to validate and fix model before it's printing - * - * Note: once this method is called, the calling of {@link #setAutoImports(boolean)} makes no sense - */ - void setCompilationUnitValidators(List> compilationUnitValidators); + /** autoimport mode */ + AUTOIMPORT, + + /** super tailored import to minimize the impact on toString */ + BACKWARD_COMPATIBLE + } } diff --git a/src/main/java/spoon/support/StandardEnvironment.java b/src/main/java/spoon/support/StandardEnvironment.java index 1d571134c86..91a341c8455 100644 --- a/src/main/java/spoon/support/StandardEnvironment.java +++ b/src/main/java/spoon/support/StandardEnvironment.java @@ -21,12 +21,16 @@ import spoon.processing.ProcessingManager; import spoon.processing.Processor; import spoon.processing.ProcessorProperties; +import spoon.reflect.code.CtTypeAccess; 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; import spoon.reflect.declaration.ParentNotInitializedException; +import spoon.reflect.reference.CtPackageReference; +import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.DefaultImportComparator; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; import spoon.reflect.visitor.ForceFullyQualifiedProcessor; @@ -73,7 +77,18 @@ public class StandardEnvironment implements Serializable, Environment { private boolean processingStopped = false; - private boolean autoImports = false; + @Override + public TO_STRING_MODE getToStringMode() { + return toStringMode; + } + + @Override + public void setToStringMode(TO_STRING_MODE toStringMode) { + this.toStringMode = toStringMode; + } + + // the default value is set to maximize backward compatibility + private TO_STRING_MODE toStringMode = TO_STRING_MODE.BACKWARD_COMPATIBLE; private int warningCount = 0; @@ -117,8 +132,6 @@ public class StandardEnvironment implements Serializable, Environment { private Supplier prettyPrinterCreator; - private List> compilationUnitValidators; - /** * Creates a new environment with a null default file * generator. @@ -133,13 +146,16 @@ public void debugMessage(String message) { @Override public boolean isAutoImports() { - return autoImports; + return TO_STRING_MODE.AUTOIMPORT.equals(toStringMode); } @Override public void setAutoImports(boolean autoImports) { - this.autoImports = autoImports; - // TODO: unexpected behaviour could occur, if we reset the autoimport AFTER the pretty printer is created... + if (autoImports == true) { + toStringMode = TO_STRING_MODE.AUTOIMPORT; + } else { + toStringMode = TO_STRING_MODE.BACKWARD_COMPATIBLE; + } } @Override @@ -636,7 +652,35 @@ public PrettyPrinter createPrettyPrinter() { // DJPP is the default mode // fully backward compatible DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(this); - printer.setPreprocessors(getCompilationUnitValidators()); + + if (TO_STRING_MODE.AUTOIMPORT.equals(toStringMode)) { + List> preprocessors = Collections.unmodifiableList(Arrays.>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 + new ImportValidator().setCanAddImports(false), + //solve conflicts, the current imports are relevant too + new NameConflictValidator(), + //compute final imports + new ImportValidator().setImportComparator(new DefaultImportComparator()) + )); + printer.setPreprocessors(preprocessors); + } + + if (TO_STRING_MODE.BACKWARD_COMPATIBLE.equals(toStringMode)) { + List> preprocessors = Collections.unmodifiableList(Arrays.>asList( + //force fully qualified + new ForceFullyQualifiedProcessor(), + //remove unused imports first. Do not add new imports at time when conflicts are not resolved + new ImportValidator().setCanAddImports(false), + //solve conflicts, the current imports are relevant too + new NameConflictValidator(), + //compute final imports + new ImportValidator().setImportComparator(new DefaultImportComparator()) + )); + printer.setPreprocessors(preprocessors); + } + return printer; } return prettyPrinterCreator.get(); @@ -656,44 +700,4 @@ public boolean isIgnoreDuplicateDeclarations() { public void setIgnoreDuplicateDeclarations(boolean ignoreDuplicateDeclarations) { this.ignoreDuplicateDeclarations = ignoreDuplicateDeclarations; } - - @Override - public List> getCompilationUnitValidators() { - if (compilationUnitValidators == null) { - if (isAutoImports()) { - return Collections.unmodifiableList(Arrays.>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 - new ImportValidator().setCanAddImports(false), - //solve conflicts, the current imports are relevant too - new NameConflictValidator(), - //compute final imports - new ImportValidator().setImportComparator(new DefaultImportComparator()) - )); - } else { - return Collections.unmodifiableList(Arrays.>asList( - //force fully qualified - new ForceFullyQualifiedProcessor(), - //remove unused imports first. Do not add new imports at time when conflicts are not resolved - new ImportValidator().setCanAddImports(false), - //solve conflicts, the current imports are relevant too - new NameConflictValidator(), - //compute final imports - new ImportValidator().setImportComparator(new DefaultImportComparator()) - )); - } - } - return Collections.unmodifiableList(compilationUnitValidators); - } - - @Override - public void setCompilationUnitValidators(List> compilationUnitValidators) { - if (compilationUnitValidators != null) { - this.compilationUnitValidators = new ArrayList<>(compilationUnitValidators); - } else { - //use default validators again - this.compilationUnitValidators = null; - } - } } From 08ad51cfe42e355bc77eaa642337fe46b7092e61 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 7 Sep 2019 20:55:40 +0200 Subject: [PATCH 13/25] ok --- src/main/java/spoon/Launcher.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/spoon/Launcher.java b/src/main/java/spoon/Launcher.java index d00ae85909a..1bc68132bd1 100644 --- a/src/main/java/spoon/Launcher.java +++ b/src/main/java/spoon/Launcher.java @@ -446,8 +446,10 @@ protected void processArguments() { // environment initialization environment.setComplianceLevel(jsapActualArgs.getInt("compliance")); environment.setLevel(jsapActualArgs.getString("level")); - environment.setAutoImports(jsapActualArgs.getBoolean("imports")); - + if (jsapActualArgs.getBoolean("imports")) { + environment.setToStringMode(Environment.TO_STRING_MODE.AUTOIMPORT); + } + if (jsapActualArgs.getBoolean("noclasspath")) { Launcher.LOGGER.warn("The usage of --noclasspath argument is now deprecated: noclasspath is now the default behaviour."); } else { From fb4f51c7535f99ce871639e400e871e501ddfd98 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 7 Sep 2019 21:06:56 +0200 Subject: [PATCH 14/25] up --- src/main/java/spoon/reflect/declaration/CtElement.java | 7 +++++-- .../spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 5 +++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/spoon/reflect/declaration/CtElement.java b/src/main/java/spoon/reflect/declaration/CtElement.java index 81688d91c82..da2981be807 100644 --- a/src/main/java/spoon/reflect/declaration/CtElement.java +++ b/src/main/java/spoon/reflect/declaration/CtElement.java @@ -393,12 +393,15 @@ List getAnnotatedChildren( List getDirectChildren(); /** - * @return print source code of this element. + * @return the source code of this element, with full-qualified types. * - * {@link Environment#getToStringMode()} is not taken into account, it is only taken into account when writing to disk. + * {@link Environment#getToStringMode()} is not taken into account */ String toString(); + /** + * @return the source code of this element accorfing to {@link Environment#getToStringMode()}. + */ String print(); } diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index cda391efd74..5d1ec8a946c 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -314,6 +314,11 @@ protected void enter(CtElement e) { protected void exit(CtElement e) { } + @Override + public DefaultJavaPrettyPrinter prettyprint(CtElement e) { + return scan(e); + } + /** * The generic scan method for an element. */ From 4955c75193a1b7f91de006c1d9b8eb111169a57a Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 7 Sep 2019 21:17:31 +0200 Subject: [PATCH 15/25] up --- .../spoon/reflect/declaration/CtElement.java | 6 +- ...stractCompilationUnitImportsProcessor.java | 232 ------------------ .../spoon/reflect/visitor/PrettyPrinter.java | 3 +- .../reflect/declaration/CtElementImpl.java | 4 +- .../ConstructorCallTest.java | 2 +- .../test/fieldaccesses/FieldAccessTest.java | 4 +- .../spoon/test/generics/GenericsTest.java | 5 +- .../java/spoon/test/imports/ImportTest.java | 2 +- .../java/spoon/test/lambda/LambdaTest.java | 8 +- .../DefaultPrettyPrinterTest.java | 10 +- .../spoon/test/visibility/VisibilityTest.java | 2 +- 11 files changed, 24 insertions(+), 254 deletions(-) delete mode 100644 src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java diff --git a/src/main/java/spoon/reflect/declaration/CtElement.java b/src/main/java/spoon/reflect/declaration/CtElement.java index da2981be807..dfebd6ca99e 100644 --- a/src/main/java/spoon/reflect/declaration/CtElement.java +++ b/src/main/java/spoon/reflect/declaration/CtElement.java @@ -393,15 +393,17 @@ List getAnnotatedChildren( List getDirectChildren(); /** - * @return the source code of this element, with full-qualified types. + * @return the source code of this element, with fully-qualified types. * * {@link Environment#getToStringMode()} is not taken into account + * + * Use {@link #prettyprint()} for a nice source code. */ String toString(); /** * @return the source code of this element accorfing to {@link Environment#getToStringMode()}. */ - String print(); + String prettyprint(); } diff --git a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java b/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java deleted file mode 100644 index ec9ddf59125..00000000000 --- a/src/main/java/spoon/reflect/visitor/AbstractCompilationUnitImportsProcessor.java +++ /dev/null @@ -1,232 +0,0 @@ -/** - * Copyright (C) 2006-2019 INRIA and contributors - * - * Spoon is available either under the terms of the MIT License (see LICENSE-MIT.txt) of the Cecill-C License (see LICENSE-CECILL-C.txt). You as the user are entitled to choose the terms under which to adopt Spoon. - */ -package spoon.reflect.visitor; - -import spoon.SpoonException; -import spoon.processing.AbstractProcessor; -import spoon.processing.Processor; -import spoon.reflect.code.CtConstructorCall; -import spoon.reflect.code.CtExpression; -import spoon.reflect.code.CtTargetedExpression; -import spoon.reflect.declaration.CtCompilationUnit; -import spoon.reflect.declaration.CtElement; -import spoon.reflect.declaration.CtPackage; -import spoon.reflect.path.CtRole; -import spoon.reflect.reference.CtExecutableReference; -import spoon.reflect.reference.CtFieldReference; -import spoon.reflect.reference.CtTypeReference; -import spoon.reflect.reference.CtVariableReference; -import spoon.reflect.visitor.chain.CtScannerListener; -import spoon.reflect.visitor.chain.ScanningMode; -import spoon.support.Experimental; - -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; - -/** - * Internal generic {@link Processor} of {@link CtCompilationUnit}, which scans CtCompilationUnit modules, packages and types - * with purpose to find type references and expressions which might influence import directives. - */ -@Experimental -abstract class AbstractCompilationUnitImportsProcessor extends AbstractProcessor { - - @Override - public void process(CtCompilationUnit cu) { - process(createScanner(), cu); - } - - protected static void process(CtScanner scanner, CtCompilationUnit cu) { - scanner.enter(cu); - switch (cu.getUnitType()) { - case MODULE_DECLARATION: - case UNKNOWN: - break; - case PACKAGE_DECLARATION: - // we need to compute imports only for package annotations and package comments - // we don't want to get all imports coming from content of package - CtPackage pack = cu.getDeclaredPackage(); - scanner.scan(pack.getAnnotations()); - break; - case TYPE_DECLARATION: - for (CtTypeReference typeRef : cu.getDeclaredTypeReferences()) { - scanner.scan(typeRef.getTypeDeclaration()); - } - break; - } - scanner.exit(cu); - } - - protected abstract T createRawScanner(); - - protected CtScannerListener createScannerListener(T scanner) { - return new ScannerListener(scanner); - } - - //The set of roles whose values are always kept implicit - protected static Set IGNORED_ROLES_WHEN_IMPLICIT = new HashSet<>(Arrays.asList( - //e.g. List s = new ArrayList(); - CtRole.TYPE_ARGUMENT, - //e.g. List - CtRole.BOUNDING_TYPE, - //e.g. (/*implicit type of parameter*/ p) -> {} - CtRole.TYPE - )); - - /** - * {@link CtScannerListener} implementation which stops scanning of children on elements, - * which mustn't have influence to compilation unit imports. - */ - protected class ScannerListener implements CtScannerListener { - protected T scanner; - protected Set ignoredRoles = IGNORED_ROLES_WHEN_IMPLICIT; - - ScannerListener(T scanner) { - super(); - this.scanner = scanner; - } - - @Override - public ScanningMode enter(CtRole role, CtElement element) { - if (element == null) { - return ScanningMode.SKIP_ALL; - } - if (role == CtRole.VARIABLE && element instanceof CtVariableReference) { - //ignore variable reference of field access. The accessType is relevant here instead. - return ScanningMode.SKIP_ALL; - } - if (element.isParentInitialized()) { - CtElement parent = element.getParent(); - if (role == CtRole.DECLARING_TYPE && element instanceof CtTypeReference) { - if (parent instanceof CtFieldReference) { - //ignore the declaring type of field reference. It is not relevant for Imports - return ScanningMode.SKIP_ALL; - } - if (parent instanceof CtExecutableReference) { - /* - * ignore the declaring type of type executable like - * anVariable.getSomeInstance().callMethod() - * The declaring type of `callMethod` method is not relevant for Imports - */ - return ScanningMode.SKIP_ALL; - } else if (parent instanceof CtTypeReference) { - /* - * It looks like this is not needed too. - * - * pvojtechovsky: I am sure it is not wanted in case of - * spoon.test.imports.testclasses.internal.ChildClass.InnerClassProtected - * which extends package protected (and for others invisible class) - * spoon.test.imports.testclasses.internal.SuperClass - * and in this case the import directive must import ...ChildClass and not ...SuperClass, - * because import is using type "access path" and not qualified name of the type. - * - * ... but in other normal cases, I guess the declaring type is used and needed for import! - * ... so I don't understand why SKIP_ALL works in all cases. May be there is missing test case? - */ - if (!((CtTypeReference) parent).getAccessType().equals(element)) { - return ScanningMode.SKIP_ALL; - } - } else { - //May be this can never happen - throw new SpoonException("Check this case. Is it relvant or not?"); - } - } - if (role == CtRole.TYPE && element instanceof CtTypeReference) { - if (parent instanceof CtFieldReference) { - //ignore the type of field references. It is not relevant for Imports - return ScanningMode.SKIP_ALL; - } - if (parent instanceof CtExecutableReference) { - CtElement parent2 = null; - if (element.isParentInitialized()) { - parent2 = parent.getParent(); - } - if (parent2 instanceof CtConstructorCall) { - //new SomeType(); is relevant for import - //continue - } else { - /* - * ignore the return type of executable reference. It is not relevant for Imports - * anVariable.getSomeInstance().callMethod() - * The return type `callMethod` method is not relevant for Imports - */ - return ScanningMode.SKIP_ALL; - } - } - /* - * CtTypeReference - * CtMethod, CtField, ... - * continue. This is relevant for import - */ - } - if (role == CtRole.ARGUMENT_TYPE) { - /* - * ignore the type of parameter of CtExecutableReference - * It is not relevant for Imports. - */ - return ScanningMode.SKIP_ALL; - } - } - if (element.isImplicit() && ignoredRoles.contains(role)) { - //ignore implicit actual type arguments - return ScanningMode.SKIP_ALL; - } - onEnter(getContext(scanner), role, element); - return ScanningMode.NORMAL; - } - } - - protected abstract U getContext(T scanner); - - protected T createScanner() { - T scanner = createRawScanner(); - if (scanner instanceof EarlyTerminatingScanner) { - CtScannerListener listener = createScannerListener(scanner); - if (listener != null) { - ((EarlyTerminatingScanner) scanner).setListener(listener); - } - } - return scanner; - } - - protected void onEnter(U context, CtRole role, CtElement element) { - - if (element instanceof CtTargetedExpression) { - CtTargetedExpression targetedExpression = (CtTargetedExpression) element; - CtExpression target = targetedExpression.getTarget(); - if (target == null) { - return; - } - handleTargetedExpression(context, role, targetedExpression, target); - } else if (element instanceof CtTypeReference) { - //we have to visit only PURE CtTypeReference. No CtArrayTypeReference, CtTypeParameterReference, ... - element.accept(new CtAbstractVisitor() { - @Override - public void visitCtTypeReference(CtTypeReference reference) { - handleTypeReference(context, role, (CtTypeReference) element); - } - }); - } - } - - protected abstract void handleTypeReference(U context, CtRole role, CtTypeReference element); - - protected abstract void handleTargetedExpression(U context, CtRole role, CtTargetedExpression targetedExpression, CtExpression target); - - /** - * @return parent of `element`, but only if it's type is `type` - */ - protected static T getParentIfType(CtElement element, Class type) { - if (element == null || !element.isParentInitialized()) { - return null; - } - CtElement parent = element.getParent(); - if (type.isInstance(parent)) { - return type.cast(parent); - } - return null; - } -} diff --git a/src/main/java/spoon/reflect/visitor/PrettyPrinter.java b/src/main/java/spoon/reflect/visitor/PrettyPrinter.java index 5e1d058f291..bea536f1f0e 100644 --- a/src/main/java/spoon/reflect/visitor/PrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/PrettyPrinter.java @@ -61,5 +61,6 @@ public interface PrettyPrinter { */ Map getLineNumberMapping(); - PrettyPrinter scan(CtElement ctElement); + /** pretty-prints the element, call {@link #toString()} to get the result */ + PrettyPrinter prettyprint(CtElement ctElement); } diff --git a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java index 5cab0e88b5b..1df5ece9793 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java @@ -278,9 +278,9 @@ public void enter(CtElement e) { } @Override - public String print() { + public String prettyprint() { PrettyPrinter printer = getFactory().getEnvironment().createPrettyPrinter(); - return printer.scan(this).getResult(); + return printer.prettyprint(this).getResult(); } @Override diff --git a/src/test/java/spoon/test/constructorcallnewclass/ConstructorCallTest.java b/src/test/java/spoon/test/constructorcallnewclass/ConstructorCallTest.java index e2c7530a4ce..dc7b32d158e 100644 --- a/src/test/java/spoon/test/constructorcallnewclass/ConstructorCallTest.java +++ b/src/test/java/spoon/test/constructorcallnewclass/ConstructorCallTest.java @@ -107,7 +107,7 @@ public void testConstructorCallWithGenericArray() { assertEquals("", implicitArrayTyped.toString()); assertEquals("AtomicLong[]", implicitArrayTyped.getSimpleName()); assertTrue(implicitArrayTyped.getComponentType().isImplicit()); - assertEquals("", implicitArrayTyped.getComponentType().print()); + assertEquals("", implicitArrayTyped.getComponentType().prettyprint()); assertEquals("AtomicLong", implicitArrayTyped.getComponentType().getSimpleName()); } diff --git a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java index e9d30a076ea..0b5b547b81f 100644 --- a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java +++ b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java @@ -437,8 +437,8 @@ public void testGetReference() { final CtClass aClass = launcher.getFactory().Class().get(B.class); // now static fields are used with the name of the parent class - assertEquals("A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).print()); - assertEquals("finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).print()); + assertEquals("A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).prettyprint()); + assertEquals("finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).prettyprint()); } @Test public void testFieldAccessAutoExplicit() throws Exception { diff --git a/src/test/java/spoon/test/generics/GenericsTest.java b/src/test/java/spoon/test/generics/GenericsTest.java index d6f67811cb4..67d5490b602 100644 --- a/src/test/java/spoon/test/generics/GenericsTest.java +++ b/src/test/java/spoon/test/generics/GenericsTest.java @@ -73,7 +73,6 @@ import spoon.test.generics.testclasses3.ClassThatDefinesANewTypeArgument; import spoon.test.generics.testclasses3.Foo; import spoon.test.generics.testclasses3.GenericConstructor; -import spoon.test.main.MainTest; import spoon.test.generics.testclasses.EnumSetOf; import spoon.test.generics.testclasses.FakeTpl; import spoon.test.generics.testclasses.Lunch; @@ -173,7 +172,7 @@ public void testDiamond1() { // the diamond is resolved to String but we don't print it, so we use the fully qualified name. assertTrue(val.getType().getActualTypeArguments().get(0).isImplicit()); - assertEquals("", val.getType().getActualTypeArguments().get(0).print()); + assertEquals("", val.getType().getActualTypeArguments().get(0).prettyprint()); assertEquals("java.lang.String", val.getType().getActualTypeArguments().get(0).getQualifiedName()); assertEquals("new java.util.ArrayList<>()",val.toString()); } @@ -300,7 +299,7 @@ public void testBugCommonCollection() throws Exception { .getActualClass()); //print prints model following implicit same like in origin source - assertEquals("Map.Entry", ref.print()); + assertEquals("Map.Entry", ref.prettyprint()); CtField y = type.getElements(new NamedElementFilter<>(CtField.class,"y")) .get(0); diff --git a/src/test/java/spoon/test/imports/ImportTest.java b/src/test/java/spoon/test/imports/ImportTest.java index 3d81fee7ee9..aa7c70eeff1 100644 --- a/src/test/java/spoon/test/imports/ImportTest.java +++ b/src/test/java/spoon/test/imports/ImportTest.java @@ -1576,7 +1576,7 @@ public static String printByPrinter(CtElement element) { //and then it prints everything String printedCU = pp.printCompilationUnit(element.getPosition().getCompilationUnit()); //this code just prints required element (the validators already modified model in previous command) - String printedElement = element.print(); + String printedElement = element.prettyprint(); //check that element is printed same like it would be done by printer //but we have to ignore indentation first assertTrue(removeIndentation(printedCU).indexOf(removeIndentation(printedElement)) >= 0); diff --git a/src/test/java/spoon/test/lambda/LambdaTest.java b/src/test/java/spoon/test/lambda/LambdaTest.java index b117ce573a8..371b2a8d191 100644 --- a/src/test/java/spoon/test/lambda/LambdaTest.java +++ b/src/test/java/spoon/test/lambda/LambdaTest.java @@ -309,7 +309,7 @@ public void testTypeParameterOfLambdaWithoutType() { final CtParameter ctParameterFirstLambda = lambda1.getParameters().get(0); assertEquals("s", ctParameterFirstLambda.getSimpleName()); assertTrue(ctParameterFirstLambda.getType().isImplicit()); - assertEquals("", ctParameterFirstLambda.getType().print()); + assertEquals("", ctParameterFirstLambda.getType().prettyprint()); assertEquals("spoon.test.lambda.testclasses.Bar.SingleSubscriber", ctParameterFirstLambda.getType().toString()); assertEquals("SingleSubscriber", ctParameterFirstLambda.getType().getSimpleName()); } @@ -337,7 +337,7 @@ public void testTypeParameterWithImplicitArrayType() { final CtArrayTypeReference typeParameter = (CtArrayTypeReference) ctParameter.getType(); assertTrue(typeParameter.getComponentType().isImplicit()); - assertEquals("", typeParameter.getComponentType().print()); + assertEquals("", typeParameter.getComponentType().prettyprint()); assertEquals("java.lang.Object", typeParameter.getComponentType().toString()); assertEquals("Object", typeParameter.getComponentType().getSimpleName()); } @@ -350,14 +350,14 @@ public void testLambdaWithPrimitiveParameter() { final CtParameter firstParam = lambda.getParameters().get(0); assertEquals("rs", firstParam.getSimpleName()); assertTrue(firstParam.getType().isImplicit()); - assertEquals("", firstParam.getType().print()); + assertEquals("", firstParam.getType().prettyprint()); assertEquals("java.sql.ResultSet", firstParam.getType().toString()); assertEquals("ResultSet", firstParam.getType().getSimpleName()); final CtParameter secondParam = lambda.getParameters().get(1); assertEquals("i", secondParam.getSimpleName()); assertTrue(secondParam.getType().isImplicit()); - assertEquals("", secondParam.getType().print()); + assertEquals("", secondParam.getType().prettyprint()); assertEquals("int", secondParam.getType().toString()); assertEquals("int", secondParam.getType().getSimpleName()); } diff --git a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java index 46e3fae4de4..a3489de79d8 100644 --- a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java +++ b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java @@ -128,7 +128,7 @@ public void testPrintAClassWithImports() { //TODO remove that after implicit is set correctly for these cases assertTrue(factory.getEnvironment().createPrettyPrinter().printTypes(aClass).contains(expected)); - assertEquals(expected, aClass.print()); + assertEquals(expected, aClass.prettyprint()); final CtConstructorCall constructorCall = aClass.getElements(new TypeFilter>(CtConstructorCall.class)).get(0); @@ -222,7 +222,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() { + " localField = ENUM.E1.ordinal();" + nl + "}"; - computed = aClass.getMethodsByName("setFieldUsingLocallyDefinedEnum").get(0).print(); + computed = aClass.getMethodsByName("setFieldUsingLocallyDefinedEnum").get(0).prettyprint(); assertEquals(expected, computed); expected = @@ -230,7 +230,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() { + " spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.globalField = localField;" + nl + "}"; - computed = aClass.getMethodsByName("setFieldOfClassWithSameNameAsTheCompilationUnitClass").get(0).print(); + computed = aClass.getMethodsByName("setFieldOfClassWithSameNameAsTheCompilationUnitClass").get(0).prettyprint(); assertEquals("The static field of an external type with the same identifier as the compilation unit is printed with FQN", expected, computed); expected = @@ -241,7 +241,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() { //Ensure the ClassA of Class0 takes precedence over an import statement for ClassA in Class1, and its identifier can be the short version. - computed = aClass.getMethodsByName("referToTwoInnerClassesWithTheSameName").get(0).print(); + computed = aClass.getMethodsByName("referToTwoInnerClassesWithTheSameName").get(0).prettyprint(); assertEquals("where inner types have the same identifier only one may be shortened and the other should be fully qualified", expected, computed); expected = @@ -255,7 +255,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() { + " }" + nl + "}"; - computed = aClass.getNestedType("ENUM").print(); + computed = aClass.getNestedType("ENUM").prettyprint(); assertEquals(expected, computed); } diff --git a/src/test/java/spoon/test/visibility/VisibilityTest.java b/src/test/java/spoon/test/visibility/VisibilityTest.java index e19e9520408..c48f28ac470 100644 --- a/src/test/java/spoon/test/visibility/VisibilityTest.java +++ b/src/test/java/spoon/test/visibility/VisibilityTest.java @@ -172,6 +172,6 @@ public boolean matches(CtInvocation element) { }).get(0); assertNotNull(ctInvocation.getTarget()); assertTrue(ctInvocation.getTarget().isImplicit()); - assertEquals("bound()", ctInvocation.print()); + assertEquals("bound()", ctInvocation.prettyprint()); } } From 1364276123e322c315476d75b4bf15f285268c71 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 7 Sep 2019 21:35:12 +0200 Subject: [PATCH 16/25] up --- src/main/java/spoon/Launcher.java | 4 ++-- src/main/java/spoon/compiler/Environment.java | 2 +- src/main/java/spoon/reflect/visitor/ImportAnalyzer.java | 2 +- src/main/java/spoon/reflect/visitor/PrettyPrinter.java | 1 - src/main/java/spoon/support/StandardEnvironment.java | 6 +----- .../spoon/support/reflect/declaration/CtElementImpl.java | 1 - 6 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/main/java/spoon/Launcher.java b/src/main/java/spoon/Launcher.java index 1bc68132bd1..a2b327be74e 100644 --- a/src/main/java/spoon/Launcher.java +++ b/src/main/java/spoon/Launcher.java @@ -448,8 +448,8 @@ protected void processArguments() { environment.setLevel(jsapActualArgs.getString("level")); if (jsapActualArgs.getBoolean("imports")) { environment.setToStringMode(Environment.TO_STRING_MODE.AUTOIMPORT); - } - + } + if (jsapActualArgs.getBoolean("noclasspath")) { Launcher.LOGGER.warn("The usage of --noclasspath argument is now deprecated: noclasspath is now the default behaviour."); } else { diff --git a/src/main/java/spoon/compiler/Environment.java b/src/main/java/spoon/compiler/Environment.java index 4b2f8897b21..325fa0e6d2b 100644 --- a/src/main/java/spoon/compiler/Environment.java +++ b/src/main/java/spoon/compiler/Environment.java @@ -435,7 +435,7 @@ void report(Processor processor, Level level, */ void setIgnoreDuplicateDeclarations(boolean ignoreDuplicateDeclarations); - public enum TO_STRING_MODE { + enum TO_STRING_MODE { /** no preprocessors of the model before pretty-printing */ VANILLA, diff --git a/src/main/java/spoon/reflect/visitor/ImportAnalyzer.java b/src/main/java/spoon/reflect/visitor/ImportAnalyzer.java index 1257fe7b83f..9f144defb8d 100644 --- a/src/main/java/spoon/reflect/visitor/ImportAnalyzer.java +++ b/src/main/java/spoon/reflect/visitor/ImportAnalyzer.java @@ -30,7 +30,7 @@ /** *{@link Processor} of {@link CtCompilationUnit}, which scans CtCompilationUnit modules, packages and types * with purpose to find type references and expressions which might influence import directives. - * + * * Subclasses create a scanner ({@link #createScanner()}) and analyzes the elements to be imported {@link #handleTypeReference} and {@link #handleTargetedExpression(CtTargetedExpression, Object, CtRole)} * */ diff --git a/src/main/java/spoon/reflect/visitor/PrettyPrinter.java b/src/main/java/spoon/reflect/visitor/PrettyPrinter.java index bea536f1f0e..44606d608e9 100644 --- a/src/main/java/spoon/reflect/visitor/PrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/PrettyPrinter.java @@ -10,7 +10,6 @@ import spoon.reflect.declaration.CtModule; import spoon.reflect.declaration.CtPackage; import spoon.reflect.declaration.CtType; -import spoon.support.reflect.declaration.CtElementImpl; import java.util.List; import java.util.Map; diff --git a/src/main/java/spoon/support/StandardEnvironment.java b/src/main/java/spoon/support/StandardEnvironment.java index 91a341c8455..a9feb3e9bef 100644 --- a/src/main/java/spoon/support/StandardEnvironment.java +++ b/src/main/java/spoon/support/StandardEnvironment.java @@ -21,23 +21,19 @@ import spoon.processing.ProcessingManager; import spoon.processing.Processor; import spoon.processing.ProcessorProperties; -import spoon.reflect.code.CtTypeAccess; 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; import spoon.reflect.declaration.ParentNotInitializedException; -import spoon.reflect.reference.CtPackageReference; -import spoon.reflect.reference.CtTypeReference; -import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.DefaultImportComparator; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; import spoon.reflect.visitor.ForceFullyQualifiedProcessor; import spoon.reflect.visitor.ForceImportProcessor; import spoon.reflect.visitor.ImportValidator; -import spoon.reflect.visitor.PrettyPrinter; import spoon.reflect.visitor.NameConflictValidator; +import spoon.reflect.visitor.PrettyPrinter; import spoon.support.compiler.FileSystemFolder; import spoon.support.compiler.SpoonProgress; import spoon.support.modelobs.EmptyModelChangeListener; diff --git a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java index 1df5ece9793..0d55a5a957c 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java @@ -27,7 +27,6 @@ import spoon.reflect.path.CtRole; import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; -import spoon.reflect.visitor.CtAbstractVisitor; import spoon.reflect.visitor.CtIterator; import spoon.reflect.visitor.CtScanner; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; From 68bea913ea96b42601a6de3fac7b3ffd3db80668 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sat, 7 Sep 2019 22:05:08 +0200 Subject: [PATCH 17/25] up --- .../spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 7 +++++++ src/test/java/spoon/test/generics/GenericsTest.java | 3 --- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 5d1ec8a946c..66dd3a9a142 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -316,9 +316,16 @@ 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})); + } + reset(); return scan(e); } + /** * The generic scan method for an element. */ diff --git a/src/test/java/spoon/test/generics/GenericsTest.java b/src/test/java/spoon/test/generics/GenericsTest.java index 67d5490b602..ed5665eb5e0 100644 --- a/src/test/java/spoon/test/generics/GenericsTest.java +++ b/src/test/java/spoon/test/generics/GenericsTest.java @@ -298,9 +298,6 @@ public void testBugCommonCollection() throws Exception { assertSame(java.util.Map.class, ref.getDeclaringType() .getActualClass()); - //print prints model following implicit same like in origin source - assertEquals("Map.Entry", ref.prettyprint()); - CtField y = type.getElements(new NamedElementFilter<>(CtField.class,"y")) .get(0); assertEquals("java.util.Map.Entry y;", y.toString()); From f0d9acd978a9ce712f180244cb4662b37abd939e Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sun, 8 Sep 2019 05:55:21 +0200 Subject: [PATCH 18/25] up --- .../spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 66dd3a9a142..f4ec62dd9c1 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -1976,18 +1976,21 @@ public void calculate(CtCompilationUnit sourceCompilationUnit, List> t if (types.isEmpty()) { return; } + CtType type = types.get(0); // reset the importsContext to avoid errors with multiple CU if (sourceCompilationUnit == null) { - CtType type = types.get(0); sourceCompilationUnit = type.getFactory().CompilationUnit().getOrCreate(type); } + if (type.getPackage() == null) { + type.setParent(type.getFactory().Package().getRootPackage()); + } if (!hasSameTypes(sourceCompilationUnit, types)) { //the provided CU has different types, then these which has to be printed //clone CU and assign it expected types sourceCompilationUnit = sourceCompilationUnit.clone(); sourceCompilationUnit.setDeclaredTypes(types); } - CtPackageReference packRef = types.get(0).getPackage().getReference(); + CtPackageReference packRef = type.getPackage().getReference(); if (!packRef.equals(sourceCompilationUnit.getPackageDeclaration().getReference())) { //the type was cloned and moved to different package. Adapt package reference of compilation unit too sourceCompilationUnit.getPackageDeclaration().setReference(packRef); From b924c6e246a644d0f01ba0fa13c9c6dc9d72dccd Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Sun, 8 Sep 2019 06:16:02 +0200 Subject: [PATCH 19/25] up --- doc/launcher.md | 16 ++++++------- src/main/java/spoon/compiler/Environment.java | 2 +- ...mportValidator.java => ImportCleaner.java} | 24 +++++++++---------- ...dator.java => ImportConflictDetector.java} | 4 ++-- .../spoon/support/StandardEnvironment.java | 24 +++++++++---------- .../test/fieldaccesses/FieldAccessTest.java | 4 ++-- .../test/prettyprinter/TestSniperPrinter.java | 10 ++++---- 7 files changed, 42 insertions(+), 42 deletions(-) rename src/main/java/spoon/reflect/visitor/{ImportValidator.java => ImportCleaner.java} (91%) rename src/main/java/spoon/reflect/visitor/{NameConflictValidator.java => ImportConflictDetector.java} (98%) diff --git a/doc/launcher.md b/doc/launcher.md index 04b93502460..f16e894e24c 100644 --- a/doc/launcher.md +++ b/doc/launcher.md @@ -28,22 +28,22 @@ CtModel model = launcher.getModel(); ### Pretty-printing modes -**Autoimport** Spoon can pretty-print code where all classes and methods are fully-qualified. This is not readable for humans but enables fast compilation and is useful when name collisions happen. +Spoon has three pretty-printing modes: + +**Fully-qualified** Spoon can pretty-print code where all classes and methods are fully-qualified. This is the default behavior on `toString()` on AST elements. +This is not readable for humans but is useful when name collisions happen. If `launcher.getEnvironment().getToStringMode() == FULLYQUALIFIED`, the files written on disk are also fully qualified. -```java -launcher.getEnvironment().setAutoImports(false); -``` -The autoimport mode computes the required imports, add the imports in the pretty-printed files, and writes class names unqualified (w/o package names): +**Autoimport** Spoon can pretty-print code where all classes and methods are imported as long as no conflict exists. ```java -// if true, the pretty-printed code is readable without fully-qualified names launcher.getEnvironment().setAutoImports(true); ``` -**Sniper mode** By default, when pretty-printing, Spoon reformats the code according to its own formatting rules. +The autoimport mode computes the required imports, add the imports in the pretty-printed files, and writes class names unqualified (w/o package names). This involves changing the field `implicit` of some elements of the model, through a set of `ImportAnalyzer`, most notable `ImportCleaner` and `ImportConflictDetector`. +When pretty-printing, Spoon reformats the code according to its own formatting rules that can be configured by providing a custom `TokenWriter`. -The sniper mode enables to rewrite only the transformed AST elements, so that the rest of the code is printed identically to the origin version. This is useful to get small diffs after automated refactoring. +**Sniper mode** The sniper mode enables to rewrite only the transformed AST elements, so that the rest of the code is printed identically to the origin version. This is useful to get small diffs after automated refactoring. ```java launcher.getEnvironment().setPrettyPrinterCreator(() -> { diff --git a/src/main/java/spoon/compiler/Environment.java b/src/main/java/spoon/compiler/Environment.java index 325fa0e6d2b..a4035611070 100644 --- a/src/main/java/spoon/compiler/Environment.java +++ b/src/main/java/spoon/compiler/Environment.java @@ -443,6 +443,6 @@ enum TO_STRING_MODE { AUTOIMPORT, /** super tailored import to minimize the impact on toString */ - BACKWARD_COMPATIBLE + FULLYQUALIFIED } } diff --git a/src/main/java/spoon/reflect/visitor/ImportValidator.java b/src/main/java/spoon/reflect/visitor/ImportCleaner.java similarity index 91% rename from src/main/java/spoon/reflect/visitor/ImportValidator.java rename to src/main/java/spoon/reflect/visitor/ImportCleaner.java index c846b811235..4ab352feace 100644 --- a/src/main/java/spoon/reflect/visitor/ImportValidator.java +++ b/src/main/java/spoon/reflect/visitor/ImportCleaner.java @@ -43,23 +43,23 @@ /** * Updates list of import statements of compilation unit following {@link CtElement#isImplicit()}. - * It doesn't call {@link CtElement#setImplicit(boolean)}. - * It doesn't fix wrong used implicit which causes conflicts. The fixing is task of {@link NameConflictValidator} + * Can be configured to add or remove imports using {@link #setCanAddImports(boolean)} and {@link #setCanRemoveImports(boolean)}. + * This does not force some references to be implicit, and doesn't fix the wrong implicit which causes conflicts: this fixing done by {@link ImportConflictDetector} */ @Experimental -public class ImportValidator extends ImportAnalyzer { +public class ImportCleaner extends ImportAnalyzer { private Comparator importComparator; private boolean canAddImports = true; private boolean canRemoveImports = true; @Override - protected MyScanner createScanner() { - return new MyScanner(); + protected ImportCleanerScanner createScanner() { + return new ImportCleanerScanner(); } @Override - protected Context getScannerContextInformation(MyScanner scanner) { + protected Context getScannerContextInformation(ImportCleanerScanner scanner) { return scanner.context; } @@ -170,7 +170,7 @@ void addImport(CtReference ref) { } } - void onComilationUnitProcessed(CtCompilationUnit compilationUnit) { + void onCompilationUnitProcessed(CtCompilationUnit compilationUnit) { ModelList existingImports = compilationUnit.getImports(); Set computedImports = new HashSet<>(this.computedImports.values()); for (CtImport oldImport : new ArrayList<>(existingImports)) { @@ -289,7 +289,7 @@ void checkType(CtTypeReference importTypeRef) { return visitor.found; } - class MyScanner extends EarlyTerminatingScanner { + class ImportCleanerScanner extends EarlyTerminatingScanner { Context context; @Override protected void enter(CtElement e) { @@ -301,7 +301,7 @@ protected void enter(CtElement e) { @Override protected void exit(CtElement e) { if (e instanceof CtCompilationUnit) { - context.onComilationUnitProcessed((CtCompilationUnit) e); + context.onCompilationUnitProcessed((CtCompilationUnit) e); } } } @@ -316,7 +316,7 @@ public boolean isCanAddImports() { /** * @param canAddImports true if this processor is allowed to add new imports */ - public ImportValidator setCanAddImports(boolean canAddImports) { + public ImportCleaner setCanAddImports(boolean canAddImports) { this.canAddImports = canAddImports; return this; } @@ -331,7 +331,7 @@ public boolean isCanRemoveImports() { /** * @param canRemoveImports true if this processor is allowed to remove imports */ - public ImportValidator setCanRemoveImports(boolean canRemoveImports) { + public ImportCleaner setCanRemoveImports(boolean canRemoveImports) { this.canRemoveImports = canRemoveImports; return this; } @@ -340,7 +340,7 @@ public Comparator getImportComparator() { return importComparator; } - public ImportValidator setImportComparator(Comparator importComparator) { + public ImportCleaner setImportComparator(Comparator importComparator) { this.importComparator = importComparator; return this; } diff --git a/src/main/java/spoon/reflect/visitor/NameConflictValidator.java b/src/main/java/spoon/reflect/visitor/ImportConflictDetector.java similarity index 98% rename from src/main/java/spoon/reflect/visitor/NameConflictValidator.java rename to src/main/java/spoon/reflect/visitor/ImportConflictDetector.java index 29ed6d176a2..0bb279cfd91 100644 --- a/src/main/java/spoon/reflect/visitor/NameConflictValidator.java +++ b/src/main/java/spoon/reflect/visitor/ImportConflictDetector.java @@ -26,7 +26,7 @@ /** - * Detects conflict of + * Detects conflicts needed to be required be a fully-qualified name. * * 1) Example: conflict of field name with an variable name and fixes it by making field target explicit. * 
@@ -50,7 +50,7 @@
  * and fixes them by call of {@link CtElement#setImplicit(boolean)} and {@link CtTypeReference#setImplicitParent(boolean)}
  */
 @Experimental
-public class NameConflictValidator extends ImportAnalyzer {
+public class ImportConflictDetector extends ImportAnalyzer {
 
 	@Override
 	protected LexicalScopeScanner createScanner() {
diff --git a/src/main/java/spoon/support/StandardEnvironment.java b/src/main/java/spoon/support/StandardEnvironment.java
index a9feb3e9bef..44565ec2dca 100644
--- a/src/main/java/spoon/support/StandardEnvironment.java
+++ b/src/main/java/spoon/support/StandardEnvironment.java
@@ -31,8 +31,8 @@
 import spoon.reflect.visitor.DefaultJavaPrettyPrinter;
 import spoon.reflect.visitor.ForceFullyQualifiedProcessor;
 import spoon.reflect.visitor.ForceImportProcessor;
-import spoon.reflect.visitor.ImportValidator;
-import spoon.reflect.visitor.NameConflictValidator;
+import spoon.reflect.visitor.ImportCleaner;
+import spoon.reflect.visitor.ImportConflictDetector;
 import spoon.reflect.visitor.PrettyPrinter;
 import spoon.support.compiler.FileSystemFolder;
 import spoon.support.compiler.SpoonProgress;
@@ -84,7 +84,7 @@ public void setToStringMode(TO_STRING_MODE toStringMode) {
 	}
 
 	// the default value is set to maximize backward compatibility
-	private TO_STRING_MODE toStringMode = TO_STRING_MODE.BACKWARD_COMPATIBLE;
+	private TO_STRING_MODE toStringMode = TO_STRING_MODE.FULLYQUALIFIED;
 
 	private int warningCount = 0;
 
@@ -150,7 +150,7 @@ public void setAutoImports(boolean autoImports) {
 		if (autoImports == true) {
 			toStringMode = TO_STRING_MODE.AUTOIMPORT;
 		} else {
-			toStringMode = TO_STRING_MODE.BACKWARD_COMPATIBLE;
+			toStringMode = TO_STRING_MODE.FULLYQUALIFIED;
 		}
 	}
 
@@ -654,27 +654,27 @@ public PrettyPrinter createPrettyPrinter() {
 						//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
-						new ImportValidator().setCanAddImports(false),
+						new ImportCleaner().setCanAddImports(false),
 						//solve conflicts, the current imports are relevant too
-						new NameConflictValidator(),
+						new ImportConflictDetector(),
 						//compute final imports
-						new ImportValidator().setImportComparator(new DefaultImportComparator())
+						new ImportCleaner().setImportComparator(new DefaultImportComparator())
 				));
 				printer.setPreprocessors(preprocessors);
 			}
 
-			if (TO_STRING_MODE.BACKWARD_COMPATIBLE.equals(toStringMode)) {
+			if (TO_STRING_MODE.FULLYQUALIFIED.equals(toStringMode)) {
 				List> preprocessors = Collections.unmodifiableList(Arrays.>asList(
 						//force fully qualified
 						new ForceFullyQualifiedProcessor(),
 						//remove unused imports first. Do not add new imports at time when conflicts are not resolved
-						new ImportValidator().setCanAddImports(false),
+						new ImportCleaner().setCanAddImports(false),
 						//solve conflicts, the current imports are relevant too
-						new NameConflictValidator(),
+						new ImportConflictDetector(),
 						//compute final imports
-						new ImportValidator().setImportComparator(new DefaultImportComparator())
+						new ImportCleaner().setImportComparator(new DefaultImportComparator())
 				));
-				printer.setPreprocessors(preprocessors);
+				//printer.setPreprocessors(preprocessors);
 			}
 
 			return printer;
diff --git a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java
index 0b5b547b81f..70a55c19b80 100644
--- a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java
+++ b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java
@@ -43,7 +43,7 @@
 import spoon.reflect.visitor.CtScanner;
 import spoon.reflect.visitor.DefaultJavaPrettyPrinter;
 import spoon.reflect.visitor.Query;
-import spoon.reflect.visitor.NameConflictValidator;
+import spoon.reflect.visitor.ImportConflictDetector;
 import spoon.reflect.visitor.filter.NamedElementFilter;
 import spoon.reflect.visitor.filter.TypeFilter;
 import spoon.test.fieldaccesses.testclasses.B;
@@ -451,7 +451,7 @@ public void testFieldAccessAutoExplicit() throws Exception {
  		//add local variable declaration which hides the field declaration 
  		method.getBody().insertBegin((CtStatement) mouse.getFactory().createCodeSnippetStatement("int age = 1").compile());
  		//run model validator to fix the problem
- 		new NameConflictValidator().process(mouse.getPosition().getCompilationUnit());
+ 		new ImportConflictDetector().process(mouse.getPosition().getCompilationUnit());
 		//now the field access must use explicit "this."
  		assertEquals("this.age", ageFR.getParent().toString());
 	}
diff --git a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java
index 70672bc0c82..4601378938f 100644
--- a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java
+++ b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java
@@ -28,8 +28,8 @@
 import spoon.reflect.declaration.ModifierKind;
 import spoon.reflect.factory.Factory;
 import spoon.reflect.reference.CtTypeReference;
-import spoon.reflect.visitor.ImportValidator;
-import spoon.reflect.visitor.NameConflictValidator;
+import spoon.reflect.visitor.ImportCleaner;
+import spoon.reflect.visitor.ImportConflictDetector;
 import spoon.support.modelobs.ChangeCollector;
 import spoon.support.sniper.SniperJavaPrettyPrinter;
 import spoon.test.prettyprinter.testclasses.ToBeChanged;
@@ -183,11 +183,11 @@ private void testSniper(String testClass, Consumer> transformation, Bi
 			SniperJavaPrettyPrinter printer = new SniperJavaPrettyPrinter(launcher.getEnvironment());
 			printer.setPreprocessors(Collections.unmodifiableList(Arrays.>asList(
 					//remove unused imports first. Do not add new imports at time when conflicts are not resolved
-					new ImportValidator().setCanAddImports(false),
+					new ImportCleaner().setCanAddImports(false),
 					//solve conflicts, the current imports are relevant too
-					new NameConflictValidator(),
+					new ImportConflictDetector(),
 					//compute final imports
-					new ImportValidator()
+					new ImportCleaner()
 				)));
 			return printer;
 		});

From 2b20decbcd84611dbec35c77d94571ddff2c37c0 Mon Sep 17 00:00:00 2001
From: Martin Monperrus 
Date: Sun, 8 Sep 2019 06:27:39 +0200
Subject: [PATCH 20/25] up

---
 doc/comments.md                                      | 2 +-
 doc/launcher.md                                      | 2 +-
 src/main/java/spoon/compiler/Environment.java        | 6 +++---
 src/main/java/spoon/support/StandardEnvironment.java | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/doc/comments.md b/doc/comments.md
index 2e15bafb64a..ef08099751a 100644
--- a/doc/comments.md
+++ b/doc/comments.md
@@ -19,7 +19,7 @@ We also try to understand to which element they are attached.
 We use some simple heuristics that work well in nominal cases but cannot address all specific cases.
 You can retrieve the comments of each `CtElement` via the API `CtElement.getComments()` which returns a `List`.
 
-The parsing of the comments can be enabled in the Environment via the option `Environment.setCommentsEnable(boolean)` or the command line argument `--enable-comments` (or `-c`).  
+The parsing of the comments can be enabled in the Environment via the option `Environment.setCommentEnabled(boolean)` or the command line argument `--enable-comments` (or `-c`).  
 
 ## Javadoc Comments
 
diff --git a/doc/launcher.md b/doc/launcher.md
index f16e894e24c..bf9c1280d7c 100644
--- a/doc/launcher.md
+++ b/doc/launcher.md
@@ -51,7 +51,7 @@ launcher.getEnvironment().setPrettyPrinterCreator(() -> {
   }
 );
 ```
-
+**Comments** In addition, depending on the value of `Environment#getCommentEnabled`, the comments are removed or kept from the Java files saved to disk (call `Environment#setCommentEnabled(true)` to keep comments).
 
 ## The MavenLauncher class
 
diff --git a/src/main/java/spoon/compiler/Environment.java b/src/main/java/spoon/compiler/Environment.java
index a4035611070..a9d6a4c3014 100644
--- a/src/main/java/spoon/compiler/Environment.java
+++ b/src/main/java/spoon/compiler/Environment.java
@@ -436,13 +436,13 @@ void report(Processor processor, Level level,
 	void setIgnoreDuplicateDeclarations(boolean ignoreDuplicateDeclarations);
 
 	enum TO_STRING_MODE {
-		/** no preprocessors of the model before pretty-printing */
+		/** no preprocessors are applied to the model before pretty-printing, this is the default behavior of @link {@link CtElement#toString()}. */
 		VANILLA,
 
-		/** autoimport mode */
+		/** autoimport mode, adds as many imports as possible */
 		AUTOIMPORT,
 
-		/** super tailored import to minimize the impact on toString */
+		/** force everything to be fully-qualified */
 		FULLYQUALIFIED
 	}
 }
diff --git a/src/main/java/spoon/support/StandardEnvironment.java b/src/main/java/spoon/support/StandardEnvironment.java
index 44565ec2dca..ebd28cefc6a 100644
--- a/src/main/java/spoon/support/StandardEnvironment.java
+++ b/src/main/java/spoon/support/StandardEnvironment.java
@@ -674,7 +674,7 @@ public PrettyPrinter createPrettyPrinter() {
 						//compute final imports
 						new ImportCleaner().setImportComparator(new DefaultImportComparator())
 				));
-				//printer.setPreprocessors(preprocessors);
+				printer.setPreprocessors(preprocessors);
 			}
 
 			return printer;

From 5a6e6d2912ecee0920e3ac0b2b67139c99436dae Mon Sep 17 00:00:00 2001
From: Martin Monperrus 
Date: Sun, 8 Sep 2019 06:47:55 +0200
Subject: [PATCH 21/25] up

---
 .../visitor/DefaultJavaPrettyPrinter.java      | 18 +++++++++---------
 .../spoon/support/StandardEnvironment.java     |  2 --
 .../reflect/declaration/CtElementImpl.java     |  2 +-
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
index f4ec62dd9c1..60431987144 100644
--- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
+++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
@@ -202,7 +202,7 @@ public class DefaultJavaPrettyPrinter implements CtVisitor, PrettyPrinter {
 	/**
 	 * Ignores isImplicit attribute on model and always prints fully qualified names
 	 */
-	protected boolean forceFullyQualified = false;
+	protected boolean ignoreImplicit = false;
 
 	public boolean inlineElseIf = true;
 
@@ -776,7 +776,7 @@ private boolean shouldPrintTarget(CtExpression target) {
 			return true;
 		}
 		//target is implicit, we should not print it
-		if (!forceFullyQualified) {
+		if (!ignoreImplicit) {
 			//fully qualified mode is not forced so we should not print implicit target
 			return false;
 		}
@@ -1253,7 +1253,7 @@ public  void visitCtInvocation(CtInvocation invocation) {
 			}
 		} else {
 			// It's a method invocation
-			if (invocation.getTarget() != null && (forceFullyQualified || !invocation.getTarget().isImplicit())) {
+			if (invocation.getTarget() != null && (ignoreImplicit || !invocation.getTarget().isImplicit())) {
 				try (Writable _context = context.modify()) {
 					if (invocation.getTarget() instanceof CtTypeAccess) {
 						_context.ignoreGenerics(true);
@@ -1731,7 +1731,7 @@ public void visitCtWildcardReference(CtWildcardReference wildcardReference) {
 	}
 
 	private boolean printQualified(CtTypeReference ref) {
-		return forceFullyQualified || !ref.isImplicitParent();
+		return ignoreImplicit || !ref.isImplicitParent();
 		}
 
 
@@ -1752,7 +1752,7 @@ public  void visitCtTypeReference(CtTypeReference ref) {
 
 	@Override
 	public  void visitCtTypeAccess(CtTypeAccess typeAccess) {
-		if (!forceFullyQualified && typeAccess.isImplicit()) {
+		if (!ignoreImplicit && typeAccess.isImplicit()) {
 			return;
 		}
 		enterCtExpression(typeAccess);
@@ -1817,7 +1817,7 @@ private boolean isPrintTypeReference(CtTypeReference accessType) {
 			//always print explicit type refs
 			return true;
 		}
-		if (forceFullyQualified) {
+		if (ignoreImplicit) {
 			//print access type always if fully qualified mode is forced
 			return true;
 		}
@@ -2062,9 +2062,9 @@ public List> getPreprocessors() {
 	}
 
 	/**
-	 * @param forceFullyQualified true to ignore `isImplicit` attribute on model and always print fully qualified names
+	 * @param ignoreImplicit true to ignore `isImplicit` attribute on model and always print fully qualified names
 	 */
-	public void setForceFullyQualified(boolean forceFullyQualified) {
-		this.forceFullyQualified = forceFullyQualified;
+	public void setIgnoreImplicit(boolean ignoreImplicit) {
+		this.ignoreImplicit = ignoreImplicit;
 }
 }
diff --git a/src/main/java/spoon/support/StandardEnvironment.java b/src/main/java/spoon/support/StandardEnvironment.java
index ebd28cefc6a..61467b9ef28 100644
--- a/src/main/java/spoon/support/StandardEnvironment.java
+++ b/src/main/java/spoon/support/StandardEnvironment.java
@@ -667,8 +667,6 @@ public PrettyPrinter createPrettyPrinter() {
 				List> preprocessors = Collections.unmodifiableList(Arrays.>asList(
 						//force fully qualified
 						new ForceFullyQualifiedProcessor(),
-						//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
 						new ImportConflictDetector(),
 						//compute final imports
diff --git a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java
index 0d55a5a957c..b9fe96e8f5b 100644
--- a/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java
+++ b/src/main/java/spoon/support/reflect/declaration/CtElementImpl.java
@@ -285,7 +285,7 @@ public String prettyprint() {
 	@Override
 	public String toString() {
 		DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(getFactory().getEnvironment());
-		printer.setForceFullyQualified(true);
+		printer.setIgnoreImplicit(true);
 		String errorMessage = "";
 		try {
 			printer.scan(this);

From b9905316d09d60b0a06be583294dad8ee4fb46cb Mon Sep 17 00:00:00 2001
From: Martin Monperrus 
Date: Sun, 8 Sep 2019 07:55:09 +0200
Subject: [PATCH 22/25] up

---
 src/main/java/spoon/reflect/declaration/CtElement.java | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/main/java/spoon/reflect/declaration/CtElement.java b/src/main/java/spoon/reflect/declaration/CtElement.java
index dfebd6ca99e..b45bc528dd6 100644
--- a/src/main/java/spoon/reflect/declaration/CtElement.java
+++ b/src/main/java/spoon/reflect/declaration/CtElement.java
@@ -393,16 +393,15 @@  List getAnnotatedChildren(
 	List getDirectChildren();
 
 	/**
-	 * @return the source code of this element, with fully-qualified types.
+	 * @return a debug version of the source code of this element, with fully-qualified types. Warning, some implicit elements are not shown in the string.
 	 *
-	 * {@link Environment#getToStringMode()} is not taken into account
-	 *
-	 * Use {@link #prettyprint()} for a nice source code.
+	 * {@link Environment#getToStringMode()} is not taken into account, use {@link #prettyprint()} for a nice version of the source code.
 	 */
 	String toString();
 
 	/**
 	 * @return the source code of this element accorfing to {@link Environment#getToStringMode()}.
+	 * Warning: this is not side-effect free, this triggers some {@link spoon.reflect.visitor.ImportAnalyzer} which would change the model: add/remove imports, change the value `implicit` of some model elements, etc.
 	 */
 	String prettyprint();
 

From 8979b5f04298d1a253829624dd7091d2c71b6bf1 Mon Sep 17 00:00:00 2001
From: Martin Monperrus 
Date: Sun, 8 Sep 2019 07:56:18 +0200
Subject: [PATCH 23/25] up

---
 src/main/java/spoon/reflect/declaration/CtElement.java | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/main/java/spoon/reflect/declaration/CtElement.java b/src/main/java/spoon/reflect/declaration/CtElement.java
index b45bc528dd6..2d9a3a2f99f 100644
--- a/src/main/java/spoon/reflect/declaration/CtElement.java
+++ b/src/main/java/spoon/reflect/declaration/CtElement.java
@@ -20,6 +20,7 @@
 import spoon.support.DerivedProperty;
 import spoon.reflect.annotations.PropertyGetter;
 import spoon.reflect.annotations.PropertySetter;
+import spoon.support.Experimental;
 
 import java.io.Serializable;
 import java.lang.annotation.Annotation;
@@ -403,6 +404,7 @@  List getAnnotatedChildren(
 	 * @return the source code of this element accorfing to {@link Environment#getToStringMode()}.
 	 * Warning: this is not side-effect free, this triggers some {@link spoon.reflect.visitor.ImportAnalyzer} which would change the model: add/remove imports, change the value `implicit` of some model elements, etc.
 	 */
+	@Experimental
 	String prettyprint();
 
 }

From 75ff0a6f6035a1b8b77127298b3190328833c720 Mon Sep 17 00:00:00 2001
From: Martin Monperrus 
Date: Sun, 8 Sep 2019 07:58:33 +0200
Subject: [PATCH 24/25] up

---
 src/main/java/spoon/Launcher.java             |  2 +-
 src/main/java/spoon/compiler/Environment.java |  7 ++++---
 .../spoon/reflect/declaration/CtElement.java  |  4 ++--
 .../spoon/support/StandardEnvironment.java    | 20 +++++++++----------
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/src/main/java/spoon/Launcher.java b/src/main/java/spoon/Launcher.java
index a2b327be74e..2e6d06f4f93 100644
--- a/src/main/java/spoon/Launcher.java
+++ b/src/main/java/spoon/Launcher.java
@@ -447,7 +447,7 @@ protected void processArguments() {
 		environment.setComplianceLevel(jsapActualArgs.getInt("compliance"));
 		environment.setLevel(jsapActualArgs.getString("level"));
 		if (jsapActualArgs.getBoolean("imports")) {
-			environment.setToStringMode(Environment.TO_STRING_MODE.AUTOIMPORT);
+			environment.setPrettyPrintingMode(Environment.PRETTY_PRINTING_MODE.AUTOIMPORT);
 		}
 
 		if (jsapActualArgs.getBoolean("noclasspath")) {
diff --git a/src/main/java/spoon/compiler/Environment.java b/src/main/java/spoon/compiler/Environment.java
index a9d6a4c3014..5746e9cb44f 100644
--- a/src/main/java/spoon/compiler/Environment.java
+++ b/src/main/java/spoon/compiler/Environment.java
@@ -43,9 +43,9 @@ public interface Environment {
 	 */
 	void setComplianceLevel(int level);
 
-	TO_STRING_MODE getToStringMode();
+	PRETTY_PRINTING_MODE getPrettyPrintingMode();
 
-	void setToStringMode(TO_STRING_MODE toStringMode);
+	void setPrettyPrintingMode(PRETTY_PRINTING_MODE prettyPrintingMode);
 
 	/**
 	 * This method should be called to print out a message with a source
@@ -435,7 +435,8 @@ void report(Processor processor, Level level,
 	 */
 	void setIgnoreDuplicateDeclarations(boolean ignoreDuplicateDeclarations);
 
-	enum TO_STRING_MODE {
+	/** Drives the model is pretty-printed to disk, or when {@link CtElement#prettyprint()} is called */
+	enum PRETTY_PRINTING_MODE {
 		/** no preprocessors are applied to the model before pretty-printing, this is the default behavior of @link {@link CtElement#toString()}. */
 		VANILLA,
 
diff --git a/src/main/java/spoon/reflect/declaration/CtElement.java b/src/main/java/spoon/reflect/declaration/CtElement.java
index 2d9a3a2f99f..9c66a150796 100644
--- a/src/main/java/spoon/reflect/declaration/CtElement.java
+++ b/src/main/java/spoon/reflect/declaration/CtElement.java
@@ -396,12 +396,12 @@  List getAnnotatedChildren(
 	/**
 	 * @return a debug version of the source code of this element, with fully-qualified types. Warning, some implicit elements are not shown in the string.
 	 *
-	 * {@link Environment#getToStringMode()} is not taken into account, use {@link #prettyprint()} for a nice version of the source code.
+	 * {@link Environment#getPrettyPrintingMode()} is not taken into account, use {@link #prettyprint()} for a nice version of the source code.
 	 */
 	String toString();
 
 	/**
-	 * @return the source code of this element accorfing to {@link Environment#getToStringMode()}.
+	 * @return the source code of this element accorfing to {@link Environment#getPrettyPrintingMode()}.
 	 * Warning: this is not side-effect free, this triggers some {@link spoon.reflect.visitor.ImportAnalyzer} which would change the model: add/remove imports, change the value `implicit` of some model elements, etc.
 	 */
 	@Experimental
diff --git a/src/main/java/spoon/support/StandardEnvironment.java b/src/main/java/spoon/support/StandardEnvironment.java
index 61467b9ef28..99dc8be4393 100644
--- a/src/main/java/spoon/support/StandardEnvironment.java
+++ b/src/main/java/spoon/support/StandardEnvironment.java
@@ -74,17 +74,17 @@ public class StandardEnvironment implements Serializable, Environment {
 	private boolean processingStopped = false;
 
 	@Override
-	public TO_STRING_MODE getToStringMode() {
-		return toStringMode;
+	public PRETTY_PRINTING_MODE getPrettyPrintingMode() {
+		return prettyPrintingMode;
 	}
 
 	@Override
-	public void setToStringMode(TO_STRING_MODE toStringMode) {
-		this.toStringMode = toStringMode;
+	public void setPrettyPrintingMode(PRETTY_PRINTING_MODE prettyPrintingMode) {
+		this.prettyPrintingMode = prettyPrintingMode;
 	}
 
 	// the default value is set to maximize backward compatibility
-	private TO_STRING_MODE toStringMode = TO_STRING_MODE.FULLYQUALIFIED;
+	private PRETTY_PRINTING_MODE prettyPrintingMode = PRETTY_PRINTING_MODE.FULLYQUALIFIED;
 
 	private int warningCount = 0;
 
@@ -142,15 +142,15 @@ public void debugMessage(String message) {
 
 	@Override
 	public boolean isAutoImports() {
-		return TO_STRING_MODE.AUTOIMPORT.equals(toStringMode);
+		return PRETTY_PRINTING_MODE.AUTOIMPORT.equals(prettyPrintingMode);
 	}
 
 	@Override
 	public void setAutoImports(boolean autoImports) {
 		if (autoImports == true) {
-			toStringMode = TO_STRING_MODE.AUTOIMPORT;
+			prettyPrintingMode = PRETTY_PRINTING_MODE.AUTOIMPORT;
 		} else {
-			toStringMode = TO_STRING_MODE.FULLYQUALIFIED;
+			prettyPrintingMode = PRETTY_PRINTING_MODE.FULLYQUALIFIED;
 		}
 	}
 
@@ -649,7 +649,7 @@ public PrettyPrinter createPrettyPrinter() {
 			// fully backward compatible
 			DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(this);
 
-			if (TO_STRING_MODE.AUTOIMPORT.equals(toStringMode)) {
+			if (PRETTY_PRINTING_MODE.AUTOIMPORT.equals(prettyPrintingMode)) {
 				List> preprocessors = Collections.unmodifiableList(Arrays.>asList(
 						//try to import as much types as possible
 						new ForceImportProcessor(),
@@ -663,7 +663,7 @@ public PrettyPrinter createPrettyPrinter() {
 				printer.setPreprocessors(preprocessors);
 			}
 
-			if (TO_STRING_MODE.FULLYQUALIFIED.equals(toStringMode)) {
+			if (PRETTY_PRINTING_MODE.FULLYQUALIFIED.equals(prettyPrintingMode)) {
 				List> preprocessors = Collections.unmodifiableList(Arrays.>asList(
 						//force fully qualified
 						new ForceFullyQualifiedProcessor(),

From 2dd73cd9f8d2515eb66c36b08e85b6bee9834b86 Mon Sep 17 00:00:00 2001
From: Martin Monperrus 
Date: Sun, 8 Sep 2019 08:14:23 +0200
Subject: [PATCH 25/25] up

---
 src/main/java/spoon/compiler/Environment.java              | 2 +-
 .../reflect/visitor/ForceFullyQualifiedProcessor.java      | 2 +-
 .../java/spoon/test/fieldaccesses/FieldAccessTest.java     | 4 ++--
 src/test/java/spoon/test/generics/GenericsTest.java        | 7 ++++---
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/main/java/spoon/compiler/Environment.java b/src/main/java/spoon/compiler/Environment.java
index 5746e9cb44f..9573040488f 100644
--- a/src/main/java/spoon/compiler/Environment.java
+++ b/src/main/java/spoon/compiler/Environment.java
@@ -435,7 +435,7 @@ void report(Processor processor, Level level,
 	 */
 	void setIgnoreDuplicateDeclarations(boolean ignoreDuplicateDeclarations);
 
-	/** Drives the model is pretty-printed to disk, or when {@link CtElement#prettyprint()} is called */
+	/** Drives how the model is pretty-printed to disk, or when {@link CtElement#prettyprint()} is called */
 	enum PRETTY_PRINTING_MODE {
 		/** no preprocessors are applied to the model before pretty-printing, this is the default behavior of @link {@link CtElement#toString()}. */
 		VANILLA,
diff --git a/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java b/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java
index 5a0c83a6079..83e09e0aa51 100644
--- a/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java
+++ b/src/main/java/spoon/reflect/visitor/ForceFullyQualifiedProcessor.java
@@ -19,7 +19,7 @@
 
 
 /**
- * Removes all imports from compilation unit and forces fully qualified identifiers
+ * Forces fully qualified identifiers by making many elements explicit (by calling setImplicit(false)).
  */
 @Experimental
 public class ForceFullyQualifiedProcessor extends ImportAnalyzer {
diff --git a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java
index 70a55c19b80..14bf277f5e9 100644
--- a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java
+++ b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java
@@ -437,8 +437,8 @@ public void testGetReference() {
 		final CtClass aClass = launcher.getFactory().Class().get(B.class);
 
 		// now static fields are used with the name of the parent class
-		assertEquals("A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).prettyprint());
-		assertEquals("finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).prettyprint());
+		assertEquals("spoon.test.fieldaccesses.testclasses.A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).toString());
+		assertEquals("spoon.test.fieldaccesses.testclasses.B.finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).toString());
 	}
 	@Test
 	public void testFieldAccessAutoExplicit() throws Exception {
diff --git a/src/test/java/spoon/test/generics/GenericsTest.java b/src/test/java/spoon/test/generics/GenericsTest.java
index ed5665eb5e0..674ec49acb7 100644
--- a/src/test/java/spoon/test/generics/GenericsTest.java
+++ b/src/test/java/spoon/test/generics/GenericsTest.java
@@ -170,11 +170,12 @@ public void testDiamond1() {
 		CtField f = clazz.getFields().get(0);
 		CtConstructorCall val = (CtConstructorCall) f.getDefaultExpression();
 
-		// the diamond is resolved to String but we don't print it, so we use the fully qualified name.
+		// the diamond is resolved to String but we don't prettyprint it in the diamond.
 		assertTrue(val.getType().getActualTypeArguments().get(0).isImplicit());
-		assertEquals("", val.getType().getActualTypeArguments().get(0).prettyprint());
+		assertEquals("java.lang.String", val.getType().getActualTypeArguments().get(0).toString());
 		assertEquals("java.lang.String", val.getType().getActualTypeArguments().get(0).getQualifiedName());
-		assertEquals("new java.util.ArrayList<>()",val.toString());
+		assertEquals("java.util.ArrayList<>", val.getType().prettyprint());
+		assertEquals("new java.util.ArrayList<>()",val.prettyprint());
 	}
 
 	@Test