feat(junit4Asserts): improve replacement of junit4 asserts (#15)

- Rename RunState to RunMode - Fix inverted condition in TransformationEngine - change add of imports to manuell mode and disable in ImportCleaner
MartinWitt · Jan 2, 2022 · e0247b1 · e0247b1
1 parent f7478f0
commit e0247b1
Show file tree

Hide file tree

Showing 10 changed files with 160 additions and 56 deletions.
diff --git a/.../keksdose/spoon/code_solver/RunState.java → ...z/keksdose/spoon/code_solver/RunMode.java b/.../keksdose/spoon/code_solver/RunState.java → ...z/keksdose/spoon/code_solver/RunMode.java
@@ -1,6 +1,6 @@
 
 package xyz.keksdose.spoon.code_solver;
 
-public enum RunState {
+public enum RunMode {
 	FULL, DRY_RUN;
 }
diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/TransformationEngine.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/TransformationEngine.java
@@ -34,7 +34,7 @@ public class TransformationEngine {
 
 	private static final FluentLogger LOGGER = FluentLogger.forEnclosingClass();
 
-	public Changelog applyToGivenPath(String path, RunState runState) {
+	public Changelog applyToGivenPath(String path, RunMode runState) {
 		LOGGER.atInfo().log("Applying transformations to %s", path);
 		Launcher launcher = new Launcher();
 		Environment environment = setEnvironmentOptions(launcher);
@@ -49,7 +49,7 @@ public Changelog applyToGivenPath(String path, RunState runState) {
 			pm.process(model.getAllTypes());
 		} while (listener.isChanged());
 		Collection<CtType<?>> newTypes = model.getAllTypes();
-		if (runState == RunState.DRY_RUN) {
+		if (runState != RunMode.DRY_RUN) {
 			printChangedTypes(environment.createPrettyPrinter(), listener, newTypes);
 		}
 		return listener.getChangelog();
@@ -62,7 +62,7 @@ protected void addProcessors(ProcessingManager pm, ChangeListener listener) {
 		pm.addProcessor(new AssertionsTransformation(listener));
 	}
 
-	public Changelog applyToGivenPath(String path, String typeName, RunState runState) {
+	public Changelog applyToGivenPath(String path, String typeName, RunMode runState) {
 		LOGGER.atInfo().log("Applying transformations to %s", path);
 		Launcher launcher = new Launcher();
 		Environment environment = setEnvironmentOptions(launcher);
@@ -78,7 +78,7 @@ public Changelog applyToGivenPath(String path, String typeName, RunState runStat
 			addProcessors(pm, listener);
 			pm.process(newTypes);
 		} while (listener.isChanged());
-		if (runState == RunState.DRY_RUN) {
+		if (runState != RunMode.DRY_RUN) {
 			printChangedTypes(environment.createPrettyPrinter(), listener, newTypes);
 		}
 		return listener.getChangelog();
@@ -123,7 +123,7 @@ private static Supplier<PrettyPrinter> applyCommonPrinterOptions(
 		Collection<CtTypeReference<?>> existingReferences = model.getElements(e -> true);
 		List<Processor<CtElement>> preprocessors = List.of(//new ImportCleaning()
 			new SelectiveForceImport(existingReferences), new ImportConflictDetector(),
-			new ImportCleaner().setImportComparator(new ImportComparator()), new ImportGrouper()
+			new ImportCleaner().setImportComparator(new ImportComparator()).setCanAddImports(false), new ImportGrouper()
 		// )
 		);
 		return () -> {

diff --git a/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/config/ConfigStore.java b/code-transformation/src/main/java/xyz/keksdose/spoon/code_solver/config/ConfigStore.java
@@ -34,11 +34,11 @@ public String getGitAuthor() {
 	}
 
 	public String getGitEmail() {
-		return property.getProperty("git.email");
+		return property.getProperty("git.mail");
 	}
 
 	public String getGitDefaultBranchName() {
-		return property.getProperty("git.defaultBranchName");
+		return property.getProperty("git.default_branch");
 	}
 
 	public String getGitBranchPrefix() {

diff --git a/...ormation/src/main/java/xyz/keksdose/spoon/code_solver/spoon/ImportAwareSniperPrinter.java b/...ormation/src/main/java/xyz/keksdose/spoon/code_solver/spoon/ImportAwareSniperPrinter.java
@@ -24,14 +24,18 @@ protected ModelList<CtImport> getImports(CtCompilationUnit compilationUnit) {
 	public void visitCtImport(CtImport ctImport) {
 		if (ctImport instanceof NewlineImport) {
 			String s = getPrinterTokenWriter().getPrinterHelper().toString();
-			if (!s.endsWith("\n") || !s.endsWith("\r\n")) {
+			if (!endsWithNewline(s)) {
 				getPrinterTokenWriter().writeln();
 			}
 			return;
 		}
 		super.visitCtImport(ctImport);
 	}
 
+	private boolean endsWithNewline(String s) {
+		return s.endsWith("\n") || s.endsWith("\r\n");
+	}
+
 	@Override
 	public <T> void visitCtLambda(CtLambda<T> lambda) {
 		enterCtExpression(lambda);

diff --git a/...sformation/src/main/java/xyz/keksdose/spoon/code_solver/transformations/ImportHelper.java b/...sformation/src/main/java/xyz/keksdose/spoon/code_solver/transformations/ImportHelper.java
@@ -26,7 +26,7 @@ public static void addImport(String importString, boolean isStatic, CtCompilatio
 	}
 
 	public static void removeImport(String importString, boolean isStatic, CtCompilationUnit unit) {
-		List<CtImport> imports = unit.getImports();
+		List<CtImport> imports = new ArrayList<>(unit.getImports());
 		List<CtImport> removalableImports = new ArrayList<>();
 		for (CtImport ctImport : imports) {
 			if (ctImport.getReference() instanceof CtTypeReference) {
@@ -38,7 +38,6 @@ public static void removeImport(String importString, boolean isStatic, CtCompila
 		}
 		removalableImports.forEach(imports::remove);
 		removalableImports.forEach(CtImport::delete);
-		imports.clear();
 		unit.setImports(imports);
 		ImportVisitor visitor = new ImportVisitor(importString);
 		imports.forEach(i -> i.accept(visitor));

diff --git a/...n/java/xyz/keksdose/spoon/code_solver/transformations/junit/AssertionsTransformation.java b/...n/java/xyz/keksdose/spoon/code_solver/transformations/junit/AssertionsTransformation.java
@@ -3,74 +3,110 @@
 
 import java.util.ArrayList;
 import java.util.List;
+
 import spoon.experimental.CtUnresolvedImport;
 import spoon.reflect.code.CtInvocation;
 import spoon.reflect.code.CtTypeAccess;
-import spoon.reflect.declaration.CtElement;
 import spoon.reflect.declaration.CtImport;
 import spoon.reflect.declaration.CtMethod;
+import spoon.reflect.declaration.CtType;
+import spoon.reflect.reference.CtExecutableReference;
+import spoon.reflect.reference.CtReference;
 import spoon.reflect.visitor.CtAbstractImportVisitor;
 import spoon.reflect.visitor.filter.TypeFilter;
+import spoon.support.util.ModelList;
 import xyz.keksdose.spoon.code_solver.history.Change;
 import xyz.keksdose.spoon.code_solver.history.ChangeListener;
 import xyz.keksdose.spoon.code_solver.transformations.TransformationProcessor;
 
 public class AssertionsTransformation extends TransformationProcessor<CtMethod<?>> {
 
+	/**
+	 *
+	 */
+	private static final String TRANSFORMATION_NAME = "AssertionsTransformation";
+
 	public AssertionsTransformation(ChangeListener listener) {
 		super(listener);
 	}
 
 	@Override
 	public void process(CtMethod<?> method) {
 		if (JunitHelper.isJunit5TestMethod(method)) {
-			var junit4Asserts = method.getElements(new TypeFilter<>(CtInvocation.class))
-					.stream()
-					.filter(v -> v.getTarget() != null)
-					.filter(v -> v.getTarget().getType() != null)
-					.filter(v -> CtTypeAccess.class.isInstance(v.getTarget()))
-					.filter(v -> ((CtTypeAccess<?>) v.getTarget()).getAccessedType() != null)
-					.filter(v -> ((CtTypeAccess<?>) v.getTarget()).getAccessedType()
-							.getQualifiedName()
-							.equals("org.junit.Assert"))
-					.toList();
+			List<CtInvocation<?>> junit4Asserts = getJunit4Asserts(method);
 			if (!junit4Asserts.isEmpty()) {
-				junit4Asserts.forEach(v -> v.getExecutable()
-						.setDeclaringType(getFactory().Type().createReference("org.junit.jupiter.api.Assertions")));
-				CtTypeAccess<?> access = getFactory()
-						.createTypeAccess(getFactory().Type().createReference("org.junit.jupiter.api.Assertions"));
-				for (CtInvocation<?> junit4Assert : junit4Asserts) {
-					junit4Assert.setTarget(null);
-				}
-				List<CtImport> imports = new ArrayList<>();
-				List<CtImport> newImports = new ArrayList<>();
-				method.getPosition()
-						.getCompilationUnit()
-						.getImports()
-						.forEach(v -> v.accept(new CtAbstractImportVisitor() {
+				convertToJunit5(junit4Asserts);
+				adjustImports(method);
+				notifyChangeListener(method);
+			}
+		}
+	}
 
-							@Override
-							public <T> void visitUnresolvedImport(CtUnresolvedImport ctUnresolvedImport) {
-								if (ctUnresolvedImport.getUnresolvedReference().startsWith("org.junit.Assert.")) {
-									imports.add(ctUnresolvedImport);
-									newImports.add(getFactory().createUnresolvedImport(
-										ctUnresolvedImport.getUnresolvedReference()
-												.replace("org.junit.Assert.", "org.junit.jupiter.api.Assertions."),
-										true));
-								}
-							}
-						}));
-				imports.forEach(CtElement::delete);
-				newImports.stream()
-						.filter(v -> !method.getPosition().getCompilationUnit().getImports().contains(v))
-						.forEach(method.getPosition().getCompilationUnit().getImports()::add);
+	private void notifyChangeListener(CtMethod<?> method) {
+		CtType<?> declaringType = method.getDeclaringType();
+		setChanged(declaringType, new Change(createChangeHistory(method), TRANSFORMATION_NAME, declaringType));
+	}
 
-				setChanged(method.getDeclaringType(),
-					new Change(
-						String.format("Transformed junit4 assert to junt 5 assertion in %s", method.getSimpleName()),
-						"AssertionsTransformation", method.getDeclaringType()));
-			}
+	private String createChangeHistory(CtMethod<?> method) {
+		return String.format("Transformed junit4 assert to junt 5 assertion in %s", method.getSimpleName());
+	}
 
+	private void convertToJunit5(List<CtInvocation<?>> junit4Asserts) {
+		for (CtInvocation<?> junit4Assert : junit4Asserts) {
+			junit4Assert.setTarget(null);
+			junit4Assert.getExecutable()
+					.setDeclaringType(getFactory().Type().createReference("org.junit.jupiter.api.Assertions"));
 		}
 	}
+
+	private List<CtInvocation<?>> getJunit4Asserts(CtMethod<?> method) {
+		return method.getElements(new TypeFilter<CtInvocation<?>>(CtInvocation.class))
+				.stream()
+				.filter(v -> v.getTarget() instanceof CtTypeAccess)
+				.filter(v -> v.getTarget().getType() != null)
+				.filter(v -> ((CtTypeAccess<?>) v.getTarget()).getAccessedType() != null)
+				.filter(v -> ((CtTypeAccess<?>) v.getTarget()).getAccessedType()
+						.getQualifiedName()
+						.equals("org.junit.Assert"))
+				.toList();
+	}
+
+	private void adjustImports(CtMethod<?> method) {
+		List<CtImport> imports = new ArrayList<>(getImports(method));
+		List<CtImport> newImports = new ArrayList<>();
+		List<CtReference> references = new ArrayList<>();
+
+		getImports(method).forEach(v -> v.accept(new CtAbstractImportVisitor() {
+			@Override
+			public <T> void visitUnresolvedImport(CtUnresolvedImport ctUnresolvedImport) {
+				if (ctUnresolvedImport.getUnresolvedReference().startsWith("org.junit.Assert.")) {
+					imports.add(ctUnresolvedImport);
+					newImports.add(getFactory().createUnresolvedImport(ctUnresolvedImport.getUnresolvedReference()
+							.replace("org.junit.Assert.", "org.junit.jupiter.api.Assertions."),
+						true));
+				}
+			}
+
+			@Override
+			public <T> void visitMethodImport(CtExecutableReference<T> executableReference) {
+				if (executableReference.getDeclaringType().getQualifiedName().equals("org.junit.Assert")) {
+					references.add(executableReference);
+					newImports.add(getFactory().createUnresolvedImport(
+						"org.junit.jupiter.api.Assertions." + executableReference.getSimpleName(), true));
+				}
+			}
+		}));
+		getImports(method).stream()
+				.filter(v -> references.contains(v.getReference()))
+				.toList()
+				.forEach(getImports(method)::remove);
+		newImports.stream().filter(v -> !getImports(method).contains(v)).forEach(getImports(method)::add);
+		var filteredImports = new ArrayList<>(getImports(method));
+		getImports(method).clear();
+		getImports(method).set(filteredImports);
+	}
+
+	private ModelList<CtImport> getImports(CtMethod<?> method) {
+		return method.getPosition().getCompilationUnit().getImports();
+	}
 }
diff --git a/...on/src/main/java/xyz/keksdose/spoon/code_solver/transformations/junit/TestAnnotation.java b/...on/src/main/java/xyz/keksdose/spoon/code_solver/transformations/junit/TestAnnotation.java
@@ -26,6 +26,8 @@ public void process(CtAnnotation<?> annotation) {
 			adjustImports(annotation, element);
 			annotation.setType(getFactory().createReference("org.junit.jupiter.api.Test"));
 			annotation.setAnnotationType(getFactory().createReference("org.junit.jupiter.api.Test"));
+			annotation.getAnnotationType().setSimplyQualified(true);
+			annotation.getType().setSimplyQualified(true);
 			CtType<?> type = annotation.getParent(CtType.class);
 			type.getReferencedTypes();
 			setChanged(type,

diff --git a/code-transformation/src/test/java/xyz/keksdose/spoon/code_solver/JunitTests.java b/code-transformation/src/test/java/xyz/keksdose/spoon/code_solver/JunitTests.java
@@ -3,6 +3,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static xyz.keksdose.spoon.code_solver.TestHelper.createProcessorSupplier;
+
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
@@ -11,6 +12,8 @@
 import org.junit.jupiter.api.DisplayNameGeneration;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
+
+import xyz.keksdose.spoon.code_solver.transformations.junit.AssertionsTransformation;
 import xyz.keksdose.spoon.code_solver.transformations.junit.TestAnnotation;
 
 @DisplayNameGeneration(CamelCaseToSpaceDisplayNames.class)
@@ -31,4 +34,22 @@ void replaceJunit4WithJunit5TestAnnotation(@TempDir File tempRoot) throws IOExce
 		assertThat(result).contains("import org.junit.jupiter.api.Test;");
 		assertThat(result).doesNotContain("import org.junit.Test;");
 	}
+
+	@Test
+	void replaceJunit4AssertionsWithJunit5(@TempDir File tempRoot) throws IOException {
+		String fileName = "TestAssertions.java";
+		String resourcePath = "projects/junittests/TestAssertions.java";
+		File copy = TestHelper.createCopy(tempRoot, resourcePath, fileName);
+		List<TransformationCreator> transformations = createProcessorSupplier(v -> new TestAnnotation(v),
+			v -> new AssertionsTransformation(v));
+		new TransformationHelper.Builder().path(tempRoot.getAbsolutePath())
+				.className("TestAssertions")
+				.processors(transformations)
+				.apply();
+		String result = Files.readString(copy.toPath());
+		assertThat(result).contains("@Test");
+		assertThat(result).contains("import org.junit.jupiter.api.Test;");
+		assertThat(result).doesNotContain("import org.junit.Test;");
+		assertThat(result).doesNotContain("import org.junit.Assert.");
+	}
 }
diff --git a/code-transformation/src/test/java/xyz/keksdose/spoon/code_solver/TransformationHelper.java b/code-transformation/src/test/java/xyz/keksdose/spoon/code_solver/TransformationHelper.java
@@ -43,7 +43,7 @@ public void applyWithGivenProcessors(String path, String className,
 				Iterable<TransformationCreator> processors) {
 			currentProcessors.clear();
 			processors.forEach(currentProcessors::add);
-			applyToGivenPath(path, className, RunState.FULL);
+			applyToGivenPath(path, className, RunMode.FULL);
 		}
 
 		@Override

diff --git a/code-transformation/src/test/resources/projects/junittests/TestAssertions.java b/code-transformation/src/test/resources/projects/junittests/TestAssertions.java
@@ -0,0 +1,42 @@
+package spoon.reflect.declaration;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import org.junit.Test;
+
+import spoon.Launcher;
+import spoon.reflect.reference.CtExecutableReference;
+import spoon.reflect.visitor.CtScanner;
+
+public class TestAssertions {
+
+  private static class ExecutableReferenceVisitor extends CtScanner {
+
+    int referenceCounter = 0;
+
+    @Override
+    public <T> void visitCtExecutableReference(final CtExecutableReference<T> reference) {
+      final CtExecutable executable = reference.getDeclaration();
+      assertNull(executable);
+
+      referenceCounter++;
+    }
+  }
+
+  @Test
+  public void testUnknownCalls() {
+    final Launcher runLaunch = new Launcher();
+    runLaunch.getEnvironment().setNoClasspath(true);
+    runLaunch.addInputResource("./src/test/resources/noclasspath/UnknownCalls.java");
+    runLaunch.buildModel();
+
+    final CtPackage rootPackage = runLaunch.getFactory().Package().getRootPackage();
+    final ExecutableReferenceVisitor visitor = new ExecutableReferenceVisitor();
+    visitor.scan(rootPackage);
+    // super constructor to Object +
+    // UnknownClass constructor +
+    // UnknownClass method
+    assertEquals(3, visitor.referenceCounter);
+  }
+}