test(SniperPrettyPrinter): test replacement of invocation (#3399)

src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java

@@ -14,11 +14,13 @@
 import spoon.SpoonException;
 import spoon.reflect.code.CtComment;
 import spoon.reflect.cu.SourcePositionHolder;
+import spoon.reflect.declaration.CtModifiable;
 import spoon.reflect.path.CtRole;
 
 import static spoon.support.sniper.internal.ElementSourceFragment.findIndexOfNextFragment;
 import static spoon.support.sniper.internal.ElementSourceFragment.filter;
 import static spoon.support.sniper.internal.ElementSourceFragment.checkCollectionItems;
+import static spoon.support.sniper.internal.ElementSourceFragment.isCommentFragment;
 import static spoon.support.sniper.internal.ElementSourceFragment.isSpaceFragment;
 
 /**
@@ -33,7 +35,7 @@ abstract class AbstractSourceFragmentPrinter implements SourceFragmentPrinter {
 	protected final List<SourceFragment> childFragments;
 	protected final ChangeResolver changeResolver;
 	//no child fragment is current at the beginning
-	private int childFragmentIdx = -1;
+	protected int childFragmentIdx = -1;
 	//this list of skipped tokens, which writes spaces and EOL.
 	//If next element is in origin, then use origin separator actions and ignore this list
 	//If next element is new, then run collected separator actions to print DJPP separators
@@ -51,13 +53,8 @@ public void print(PrinterEvent event) {
 		int index = update(event);
 		if (index != -1) { // means we have found a source code fragment corresponding to this event
 
-			// handling of spaces
-			// hacky but works for now
-			// TODO a refactoring of printSpaces would be better
-			// but there are other bugs of higher priority now
-			childFragmentIdx = prevIndex;
-			printSpaces(index);
-			childFragmentIdx = index;
+			// we print all spaces and comments before this fragment
+			printSpaces(getLastNonSpaceNonCommentBefore(index), index);
 
 			SourceFragment fragment = childFragments.get(index);
 			event.printSourceFragment(fragment, isFragmentModified(fragment));
@@ -90,7 +87,7 @@ public int update(PrinterEvent event) {
 			 * the token did not exist in origin sources. Print spaces made by DJPP
 			 * It can happen e.g. when type parameter like &lt;T&gt; was added. Then bracket tokens are not in origin sources
 			 */
-			printSpaces(-1);
+			printSpaces(childFragmentIdx, -1);
 			event.printSourceFragment(null, ModificationStatus.UNKNOWN);
 			return -1;
 		}
@@ -115,15 +112,16 @@ public void onFinished() {
 	 * Prints spaces before fragment with index `fragmentIndex`
 	 * @param fragmentIndex index of fragment whose prefix spaces has to be printed or -1 if origin source fragment was not found
 	 */
-	protected void printSpaces(int fragmentIndex) {
+	protected void printSpaces(int fromIndex, int fragmentIndex) {
 		if (fragmentIndex < 0) {
 			/*
 			 * the token did not exist in origin sources. Print spaces made by DJPP
 			 * It can happen e.g. when type parameter like &lt;T&gt; was added. Then bracket tokens are not in origin sources
 			 */
+
 			printStandardSpaces();
 		} else {
-			printOriginSpacesUntilFragmentIndex(fragmentIndex);
+			printOriginSpacesUntilFragmentIndex(fromIndex, fragmentIndex);
 		}
 	}
 
@@ -161,14 +159,6 @@ protected ModificationStatus isFragmentModified(SourceFragment fragment) {
 		}
 	}
 
-	/**
-	 * Prints origin whitespaces including comments which prefixes the fragment on index `index`,
-	 * starting with not yet processed spaces
-	 * @param index of non white space fragment
-	 */
-	protected void printOriginSpacesUntilFragmentIndex(int index) {
-		printOriginSpacesUntilFragmentIndex(childFragmentIdx + 1, index);
-	}
 
 	/**
 	 * Prints origin whitespaces including comments which prefixes the fragment on index `index`,
@@ -217,7 +207,6 @@ protected void printOriginSpacesUntilFragmentIndex(int fromIndex, int toIndex) {
 				skipSpaceAfterDeletedElement = true;
 			}
 		}
-		setChildFragmentIdx(toIndex - 1);
 		separatorActions.clear();
 	}
 
@@ -279,10 +268,12 @@ protected int findIndexOfNextChildTokenOfElement(SourcePositionHolder element) {
 	protected int findIFragmentIndexCorrespondingToEvent(PrinterEvent event) {
 		CtRole role = event.getRole();
 		if (role != null) {
-			if (role == CtRole.COMMENT) {
-				return findIndexOfNextChildTokenOfElement(event.getElement());
+			if (event.getElement() instanceof CtModifiable || role == CtRole.MODIFIER) {
+				// using only roles for handling modifiers correctly
+				return findIndexOfNextChildTokenOfRole(childFragmentIdx + 1, role);
 			}
-			return findIndexOfNextChildTokenOfRole(childFragmentIdx + 1, role);
+			return findIndexOfNextChildTokenOfElement(event.getElement());
+
 		}
 		if (event instanceof TokenPrinterEvent) {
 			TokenPrinterEvent tpe = (TokenPrinterEvent) event;
@@ -306,6 +297,17 @@ protected void printStandardSpaces() {
 		separatorActions.clear();
 	}
 
+	private int getLastNonSpaceNonCommentBefore(int index) {
+		for (int i = index - 1; i >= 0; i--) {
+			SourceFragment fragment = childFragments.get(i);
+			if (isSpaceFragment(fragment) || isCommentFragment(fragment)) {
+				continue;
+			}
+			return i + 1;
+		}
+		return 0;
+	}
+
 	@Override
 	public void onPush() {
 	}

src/main/java/spoon/support/sniper/internal/ElementSourceFragment.java

@@ -935,4 +935,12 @@ static <T extends SourceFragment> Predicate<SourceFragment> filter(Class<T> claz
 	static boolean isSpaceFragment(SourceFragment fragment) {
 		return fragment instanceof TokenSourceFragment && ((TokenSourceFragment) fragment).getType() == TokenType.SPACE;
 	}
+
+	/**
+	 * @return true if {@link SourceFragment} represents a comment
+	 */
+	static boolean isCommentFragment(SourceFragment fragment) {
+		return fragment instanceof ElementSourceFragment && ((ElementSourceFragment) fragment).getElement() instanceof CtComment;
+	}
+
 }

src/main/java/spoon/support/sniper/internal/SourceFragmentContextList.java

@@ -7,11 +7,9 @@
  */
 package spoon.support.sniper.internal;
 
-import java.util.List;
-
 import spoon.reflect.declaration.CtElement;
 
-import static spoon.support.sniper.internal.ElementSourceFragment.isSpaceFragment;
+import java.util.List;
 
 /**
  * Handles printing of changes of the ordered list of elements.
@@ -39,22 +37,4 @@ protected int findIFragmentIndexCorrespondingToEvent(PrinterEvent event) {
 		return super.findIFragmentIndexCorrespondingToEvent(event);
 	}
 
-	@Override
-	protected void printOriginSpacesUntilFragmentIndex(int index) {
-		super.printOriginSpacesUntilFragmentIndex(getLastWhiteSpaceBefore(index), index);
-	}
-
-	/**
-	 * @return index of last child fragment which contains space, which is before `index`
-	 */
-	private int getLastWhiteSpaceBefore(int index) {
-		for (int i = index - 1; i >= 0; i--) {
-			SourceFragment fragment = childFragments.get(i);
-			if (isSpaceFragment(fragment)) {
-				continue;
-			}
-			return i + 1;
-		}
-		return 0;
-	}
 }

src/main/java/spoon/support/sniper/internal/SourceFragmentContextNormal.java

@@ -55,6 +55,6 @@ public boolean knowsHowToPrint(PrinterEvent event) {
 	public void onFinished() {
 		//we are at the end of this element. Printer just tries to print something out of this context.
 		//print fragment suffix
-		printSpaces(childFragments.size());
+		printSpaces(childFragmentIdx + 1, childFragments.size());
 	}
 }

src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java

@@ -16,38 +16,35 @@
  */
 package spoon.test.prettyprinter;
 
-import org.junit.Ignore;
 import org.junit.Test;
 import spoon.Launcher;
 import spoon.SpoonException;
-import spoon.compiler.Environment;
-import spoon.processing.AbstractProcessor;
 import spoon.processing.Processor;
-import spoon.processing.ProcessorProperties;
-import spoon.processing.TraversalStrategy;
 import spoon.reflect.CtModel;
+import spoon.reflect.code.CtCodeSnippetExpression;
+import spoon.reflect.code.CtExpression;
+import spoon.reflect.code.CtInvocation;
+import spoon.reflect.code.CtLocalVariable;
 import spoon.reflect.code.CtStatement;
 import spoon.reflect.declaration.CtClass;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.declaration.CtField;
 import spoon.reflect.declaration.CtMethod;
-import spoon.reflect.declaration.CtModule;
 import spoon.reflect.declaration.CtPackage;
 import spoon.reflect.declaration.CtType;
 import spoon.reflect.declaration.ModifierKind;
 import spoon.reflect.factory.Factory;
-import spoon.reflect.reference.CtPackageReference;
+import spoon.reflect.reference.CtExecutableReference;
 import spoon.reflect.reference.CtReference;
 import spoon.reflect.reference.CtTypeReference;
-import spoon.reflect.visitor.CtScanner;
 import spoon.reflect.visitor.ImportCleaner;
 import spoon.reflect.visitor.ImportConflictDetector;
 import spoon.reflect.visitor.filter.TypeFilter;
 import spoon.support.modelobs.ChangeCollector;
 import spoon.support.modelobs.SourceFragmentCreator;
 import spoon.support.sniper.SniperJavaPrettyPrinter;
-import spoon.test.GitHubIssue;
 import spoon.test.prettyprinter.testclasses.OneLineMultipleVariableDeclaration;
+import spoon.test.prettyprinter.testclasses.InvocationReplacement;
 import spoon.test.prettyprinter.testclasses.ToBeChanged;
 
 import java.io.File;
@@ -61,9 +58,7 @@
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
-import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.regex.Matcher;
@@ -75,6 +70,23 @@
 
 public class TestSniperPrinter {
 
+	@Test
+	public void testPrintReplacementOfInvocation() {
+		testSniper(InvocationReplacement.class.getName(), type -> {
+			CtLocalVariable localVariable = (CtLocalVariable) type.getMethodsByName("main").get(0).getBody().getStatements().get(0);
+			CtInvocation invocation = (CtInvocation) localVariable.getAssignment();
+			CtExpression prevTarget = invocation.getTarget();
+			CtCodeSnippetExpression newTarget = type.getFactory().Code().createCodeSnippetExpression("Arrays");
+			CtType arraysClass = type.getFactory().Class().get(Arrays.class);
+			CtMethod method = (CtMethod) arraysClass.getMethodsByName("toString").get(0);
+			CtExecutableReference refToMethod = type.getFactory().Executable().createReference(method);
+			CtInvocation newInvocation = type.getFactory().Code().createInvocation(newTarget, refToMethod, prevTarget);
+			invocation.replace(newInvocation);
+		}, (type, printed) -> {
+			assertIsPrintedWithExpectedChanges(type, printed, "\\QString argStr = args.toString();", "String argStr = Arrays.toString(args);");
+		});
+	}
+
 	@Test
 	public void testPrintOneLineMultipleVariableDeclaration() {
 		// contract: files with joint field declarations can be recompiled after sniper
@@ -84,9 +96,7 @@ public void testPrintOneLineMultipleVariableDeclaration() {
 		}, (type, printed) -> {
 			assertEquals("package spoon.test.prettyprinter.testclasses;\n" +
 					"\n" +
-					"public class OneLineMultipleVariableDeclaration {\n" +
-					"\n" +
-					"\tint a;\n" +
+					"public class OneLineMultipleVariableDeclaration {int a;\n" +
 					"\n" +
 					"\tint c;\n" +
 					"}", printed);
@@ -173,7 +183,7 @@ public void testPrintAfterRemoveOfLastParameter() {
 			//delete last parameter of method `andSomeOtherMethod`
 			type.getMethodsByName("andSomeOtherMethod").get(0).getParameters().get(2).delete();
 		}, (type, printed) -> {
-			assertIsPrintedWithExpectedChanges(type, printed, "\\s*, \\QList<?>[][] ... twoDArrayOfLists\\E", "");
+			assertIsPrintedWithExpectedChanges(type, printed, "\\s*, \\QList<?>[][]... twoDArrayOfLists\\E", "");
 		});
 	}
 

src/test/java/spoon/test/prettyprinter/testclasses/InvocationReplacement.java

@@ -0,0 +1,9 @@
+package spoon.test.prettyprinter.testclasses;
+
+public class InvocationReplacement {
+
+	public static void main(String[] args) {
+		String argStr = args.toString();
+	}
+
+}

src/test/java/spoon/test/prettyprinter/testclasses/ToBeChanged.java

@@ -12,7 +12,7 @@
  */
 public
 @Deprecated
-abstract class /* even this comment stays here together with all SPACES and EOLs*/ ToBeChanged<T, K> /*before extends*/ 
+abstract class /* even this comment stays here together with all SPACES and EOLs*/ ToBeChanged<T, K> /*before extends*/
 	extends ArrayList<T /* let's confuse > it */ > implements List<T>,
 	Cloneable
 {
@@ -29,7 +29,7 @@ abstract class /* even this comment stays here together with all SPACES and EOLs
 
 	public <T, K> void andSomeOtherMethod(
 			int param1,
-			String param2         , List<?>[][] ... twoDArrayOfLists)
+			String param2         , List<?>[][]... twoDArrayOfLists)
 	{/**/
 		System.out.println("aaa"
 				+ "xyz");