From fc49afa32d4e3f226b1842b299a7684ec2759b6f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simon=20Lars=C3=A9n?= <slarse@slar.se>
Date: Thu, 25 Mar 2021 06:34:18 +0100
Subject: [PATCH] fix: Fix sniper printer failing to prepend whitespace to
 top-added CtModifiable (#3856)

---
 .../AbstractSourceFragmentPrinter.java        |  5 +-
 .../test/prettyprinter/TestSniperPrinter.java | 68 +++++++++++++++++++
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java b/src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java
index 8619151b319..ba4246711f7 100644
--- a/src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java
+++ b/src/main/java/spoon/support/sniper/internal/AbstractSourceFragmentPrinter.java
@@ -277,8 +277,9 @@ protected int findIndexOfNextChildTokenOfElement(SourcePositionHolder element) {
 	protected int findIFragmentIndexCorrespondingToEvent(PrinterEvent event) {
 		CtRole role = event.getRole();
 		if (role != null) {
-			if (event.getElement() instanceof CtModifiable || role == CtRole.MODIFIER) {
-				// using only roles for handling modifiers correctly
+			if ((event.getElement() instanceof CtModifiable && event.getElement().getPosition().isValidPosition())
+					|| role == CtRole.MODIFIER) {
+				// using only roles for handling modifiers and preexisting modifiables correctly
 				return findIndexOfNextChildTokenOfRole(childFragmentIdx + 1, role);
 			}
 			return findIndexOfNextChildTokenOfElement(event.getElement());
diff --git a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java
index 023168dbc4b..177cf733ae9 100644
--- a/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java
+++ b/src/test/java/spoon/test/prettyprinter/TestSniperPrinter.java
@@ -414,6 +414,74 @@ public void testCalculateCrashesWithInformativeMessageWhenSniperPrinterSetAfterM
 		}
 	}
 
+	@Test
+	public void testWhitespacePrependedToFieldAddedAtTop() {
+		assumeNotWindows(); // FIXME Make test case pass on Windows
+		// contract: newline and indentation must be inserted before a field that's added to the top
+		// of a class body when the class already has other type members.
+
+		Consumer<CtType<?>> addFieldAtTop = type -> {
+			Factory fact = type.getFactory();
+			CtField<?> field = fact.createCtField(
+					"newFieldAtTop", fact.Type().INTEGER_PRIMITIVE, "2");
+			type.addFieldAtTop(field);
+		};
+
+		final String expectedFieldSource = "int newFieldAtTop = 2;";
+		BiConsumer<CtType<?>, String> assertTopAddedFieldOnSeparateLine = (type, result) ->
+				assertThat(result, containsString("{\n    " + expectedFieldSource));
+
+		// it doesn't matter which test resource is used, as long as it has a non-empty class
+		String nonEmptyClass = "TypeMemberComments";
+		testSniper(nonEmptyClass, addFieldAtTop, assertTopAddedFieldOnSeparateLine);
+	}
+
+	@Test
+	public void testWhitespacePrependedToNestedClassAddedAtTop() {
+		assumeNotWindows(); // FIXME Make test case pass on Windows
+		// contract: newline and indentation must be inserted before a nested class that's added to
+		// the top of a class body when the class already has other type members.
+
+		Consumer<CtType<?>> addNestedClassAtTop = type -> {
+			CtClass<?> nestedClass = type.getFactory().createClass("Nested");
+			type.addTypeMemberAt(0, nestedClass);
+		};
+
+		final String expectedClassSource = "class Nested {}";
+		BiConsumer<CtType<?>, String> assertTopAddedClassOnSeparateLine = (type, result) ->
+				assertThat(result, containsString("{\n    " + expectedClassSource));
+
+		// it doesn't matter which test resource is used, as long as it has a non-empty class
+		String nonEmptyClass = "TypeMemberComments";
+		testSniper(nonEmptyClass, addNestedClassAtTop, assertTopAddedClassOnSeparateLine);
+	}
+
+	@Test
+	public void testWhitespacePrependedToLocalVariableAddAtTopOfNonEmptyMethod() {
+		assumeNotWindows(); // FIXME Make test case pass on Windows
+		// contract: newline and indentation must be inserted before a local variable that's added
+		// to the top of a non-empty statement list.
+
+		Consumer<CtType<?>> addLocalVariableAtTopOfMethod = type -> {
+			Factory factory = type.getFactory();
+			CtMethod<?> method = type.getMethods().stream()
+					.filter(m -> !m.getBody().getStatements().isEmpty())
+					.findFirst()
+					.get();
+			CtLocalVariable<?> localVar = factory.createLocalVariable(
+					factory.Type().INTEGER_PRIMITIVE, "localVar", factory.createCodeSnippetExpression("2"));
+			method.getBody().addStatement(0, localVar);
+		};
+
+		final String expectedVariableSource = "int localVar = 2;";
+		BiConsumer<CtType<?>, String> assertTopAddedVariableOnSeparateLine = (type, result) ->
+				assertThat(result, containsString("{\n        " + expectedVariableSource));
+
+		// the test resource must have a class with a non-empty method
+		String classWithNonEmptyMethod = "methodimport.ClassWithStaticMethod";
+		testSniper(classWithNonEmptyMethod, addLocalVariableAtTopOfMethod, assertTopAddedVariableOnSeparateLine);
+	}
+
 	@Test
 	public void testNewlineInsertedBetweenCommentAndTypeMemberWithAddedModifier() {
 		assumeNotWindows(); // FIXME Make test case pass on Windows