fix: Use correct target for statically imported method calls

src/main/java/spoon/reflect/visitor/ImportCleaner.java

@@ -89,7 +89,12 @@ protected void handleTargetedExpression(CtTargetedExpression<?, ?> targetedExpre
 		}
 
 		if (targetedExpression instanceof CtFieldRead) {
-			context.addImport(((CtFieldRead<?>) targetedExpression).getVariable());
+			// Type can be null in no-classpath => Just add a computed import to keep existing ones alive
+			// Only add an import for the field if the type of the target is implicit, to prevent adding imports
+			// for fully-qualified field accesses.
+			if (target.getType() == null || target.getType().isImplicit()) {
+				context.addImport(((CtFieldRead<?>) targetedExpression).getVariable());
+			}
 		}
 
 		if (target.isImplicit()) {
@@ -198,8 +203,10 @@ void addImport(CtReference ref) {
 			if (packageRef == null) {
 				return;
 			}
-			if ("java.lang".equals(packageRef.getQualifiedName())) {
-				//java.lang is always imported implicitly. Ignore it
+			if ("java.lang".equals(packageRef.getQualifiedName())
+				&& !isStaticExecutableRef(ref)
+				&& !isStaticFieldRef(ref)) {
+				//java.lang is always imported implicitly. Ignore it, unless it is a static field or method import
 				return;
 			}
 			if (ref instanceof CtTypeReference && Objects.equals(packageQName, packageRef.getQualifiedName()) && !isStaticExecutableRef(ref)) {
@@ -220,10 +227,22 @@ void addImport(CtReference ref) {
 		}
 
 		private boolean isReferencePresentInImports(CtReference ref) {
+			boolean found = compilationUnit.getImports()
+				.stream()
+				.anyMatch(ctImport -> ctImport.getReference() != null
+					&& isEqualAfterSkippingRole(ctImport.getReference(), ref, CtRole.TYPE_ARGUMENT));
+
+			if (found) {
+				return true;
+			}
+			if (!(ref instanceof CtFieldReference)) {
+				return false;
+			}
 			return compilationUnit.getImports()
-					.stream()
-					.anyMatch(ctImport -> ctImport.getReference() != null
-							&& isEqualAfterSkippingRole(ctImport.getReference(), ref, CtRole.TYPE_ARGUMENT));
+				.stream()
+				.filter(it -> it.getReference() instanceof CtTypeMemberWildcardImportReference)
+				.map(it -> (CtTypeMemberWildcardImportReference) it.getReference())
+				.anyMatch(it -> it.getTypeReference().equals(((CtFieldReference<?>) ref).getDeclaringType()));
 		}
 
 		/**
@@ -471,4 +490,8 @@ private static boolean isStaticExecutableRef(CtElement element) {
 		return element instanceof CtExecutableReference<?>
 				&& ((CtExecutableReference<?>) element).isStatic();
 	}
+
+	private static boolean isStaticFieldRef(CtReference ref) {
+		return ref instanceof CtFieldReference<?> && ((CtFieldReference<?>) ref).isStatic();
+	}
 }

src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java

@@ -471,7 +471,9 @@ public boolean onAccess(char[][] tokens, int index) {
 					qualifiedNameReference.actualReceiverType.getPackage(),
 					qualifiedNameReference, fieldReference.getSimpleName(), typeAccess.getAccessedType());
 		} else {
-			typeAccess.setImplicit(qualifiedNameReference.isImplicitThis());
+			if (qualifiedNameReference.isImplicitThis() || qualifiedNameReference.tokens.length == 2) {
+				typeAccess.setImplicit(true);
+			}
 		}
 		return typeAccess;
 	}

src/main/java/spoon/support/compiler/jdt/ParentExiter.java

@@ -720,11 +720,13 @@ public <T> void visitCtInvocation(CtInvocation<T> invocation) {
 		} else if (child instanceof CtExpression) {
 			if (hasChildEqualsToReceiver(invocation) || hasChildEqualsToQualification(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, true));
-					} else {
-						invocation.setTarget((CtThisAccess<?>) child);
+					if (!setTargetFromUnqualifiedAccess(invocation)) {
+						final CtTypeReference<?> declaringType = invocation.getExecutable().getDeclaringType();
+						if (declaringType != null && invocation.getExecutable().isStatic() && child.isImplicit()) {
+							invocation.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(declaringType, true));
+						} else {
+							invocation.setTarget((CtThisAccess<?>) child);
+						}
 					}
 				} else {
 					invocation.setTarget((CtExpression<?>) child);
@@ -737,6 +739,51 @@ public <T> void visitCtInvocation(CtInvocation<T> invocation) {
 		super.visitCtInvocation(invocation);
 	}
 
+	private <T> boolean setTargetFromUnqualifiedAccess(CtInvocation<T> invocation) {
+		// A call to a statically imported method (e.g. assertTrue(false)) is modelled as
+		// "this.assertTrue(false)" by JDT. We need to unscramble that heuristically and replace the
+		// "this" reference with the correct type (e.g. org.junit.api.Assertions)
+
+		// Additionally, references to methods of enclosing classes are also modelled as "this" by JDT.
+		// Compare with Test "correctlySetsThisTargetForUnqualifiedCalls".
+
+		// We need a MessageSend as the parent to resolve the actualType from the receiver
+		if (!(parentPair.node instanceof MessageSend)) {
+			return false;
+		}
+		MessageSend messageSend = (MessageSend) parentPair.node;
+		if (messageSend.actualReceiverType == null || messageSend.receiver.resolvedType == null) {
+			return false;
+		}
+
+		ReferenceBuilder referenceBuilder = jdtTreeBuilder.getReferencesBuilder();
+		CtTypeReference<?> actualReceiverType = referenceBuilder.getTypeReference(messageSend.actualReceiverType);
+		CtTypeReference<?> resolvedReceiverType = referenceBuilder.getTypeReference(messageSend.receiver.resolvedType);
+
+		// If they match we have a normal "this" reference
+		if (actualReceiverType.equals(resolvedReceiverType)) {
+			return false;
+		}
+
+		if (messageSend.binding() == null || !messageSend.binding().isStatic()) {
+			// Emulate outer this access
+			while (resolvedReceiverType != null) {
+				resolvedReceiverType = resolvedReceiverType.getDeclaringType();
+				if (actualReceiverType.equals(resolvedReceiverType)) {
+					invocation.setTarget(jdtTreeBuilder.getFactory().Code().createThisAccess(actualReceiverType, true));
+					return true;
+				}
+			}
+			// I don't think this can happen but let's be conservative and preserve the previous behaviour
+			return false;
+		}
+
+		// If not, we probably had a static import/static outer method reference here and should use the actual type
+		// instead
+		invocation.setTarget(jdtTreeBuilder.getFactory().Code().createTypeAccess(actualReceiverType, true));
+		return true;
+	}
+
 	private <T> boolean hasChildEqualsToQualification(CtInvocation<T> ctInvocation) {
 		if (!(jdtTreeBuilder.getContextBuilder().stack.peek().node instanceof ExplicitConstructorCall)) {
 			return false;

src/test/java/spoon/reflect/visitor/CtScannerTest.java

@@ -271,9 +271,9 @@ public void exit(CtElement o) {
 		// 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 + countOfCommentsInCompilationUnits);
-		assertEquals(2435, counter.nEnter + countOfCommentsInCompilationUnits);
-		assertEquals(2435, counter.nExit + countOfCommentsInCompilationUnits);
+		assertEquals(3631, counter.nElement + countOfCommentsInCompilationUnits);
+		assertEquals(2423, counter.nEnter + countOfCommentsInCompilationUnits);
+		assertEquals(2423, 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();

src/test/java/spoon/reflect/visitor/ImportCleanerTest.java

@@ -51,6 +51,12 @@ void testDoesNotImportInheritedStaticMethod() {
 		testImportCleanerDoesNotAlterImports("./src/test/resources/inherit-static-method", "Derived");
 	}
 
+	@Test
+	void testDoesNotRemoveStaticImports() {
+		// contract: The import cleaner should not remove static imports
+		testImportCleanerDoesNotAlterImports("src/test/resources/importCleaner/StaticImports.java", "test.Test");
+	}
+
 	/**
 	 * Test that processing the target class' compilation unit with the import cleaner does not
 	 * alter the imports.

src/test/java/spoon/test/imports/ImportTest.java

@@ -92,6 +92,8 @@
 import java.util.StringTokenizer;
 
 import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.IsCollectionContaining.hasItem;
@@ -363,7 +365,7 @@ public void testImportStaticAndFieldAccessWithImport() {
 		final CtType<Object> aTacos = launcher.getFactory().Type().get(Tacos.class);
 		final CtStatement assignment = aTacos.getMethod("m").getBody().getStatement(0);
 		assertTrue(assignment instanceof CtLocalVariable);
-		assertEquals("Constants.CONSTANT.foo", printByPrinter(((CtLocalVariable) assignment).getAssignment()));
+		assertEquals("CONSTANT.foo", printByPrinter(((CtLocalVariable) assignment).getAssignment()));
 	}
 
 	@Test
@@ -1773,4 +1775,47 @@ public void testImportsForElementsAnnotatedWithTypeUseAnnotations() {
 				.filter(ctImport -> ctImport.prettyprint().equals("import spoon.test.imports.testclasses.badimportissue3320.source.other.SomeObjectDto;"))
 				.count());
 	}
+
+	@ModelTest("src/test/resources/imports/UnqualifiedCalls.java")
+	void correctlySetsThisTargetForUnqualifiedCalls(Factory factory) {
+		CtType<?> test = factory.Type().get("Test$Inner");
+		CtMethod<?> method = test.getMethodsByName("entrypoint").get(0);
+		CtInvocation<?> actualThisInvocation = method.getBody().getStatement(0);
+		CtInvocation<?> outerThisInvocation = method.getBody().getStatement(1);
+		CtInvocation<?> staticOuterInvocation = method.getBody().getStatement(2);
+		CtInvocation<?> staticInvocation = method.getBody().getStatement(3);
+
+		assertThat(actualThisInvocation.getTarget().isImplicit(), is(true));
+		assertThat(actualThisInvocation.getTarget(), is(instanceOf(CtThisAccess.class)));
+		assertThat(
+			((CtTypeAccess<?>) ((CtThisAccess<?>) actualThisInvocation.getTarget()).getTarget())
+				.getAccessedType()
+				.getQualifiedName(),
+			is("Test$Inner")
+		);
+
+		assertThat(outerThisInvocation.getTarget().isImplicit(), is(true));
+		assertThat(outerThisInvocation.getTarget(), is(instanceOf(CtThisAccess.class)));
+		assertThat(
+			((CtTypeAccess<?>) ((CtThisAccess<?>) outerThisInvocation.getTarget()).getTarget())
+				.getAccessedType()
+				.getQualifiedName(),
+			is("Test")
+		);
+
+		assertThat(staticOuterInvocation.getTarget().isImplicit(), is(true));
+		assertThat(staticOuterInvocation.getTarget(), is(instanceOf(CtTypeAccess.class)));
+		assertThat(
+			((CtTypeAccess<?>) staticOuterInvocation.getTarget()).getAccessedType().getQualifiedName(),
+			is("Test")
+		);
+
+		assertThat(staticInvocation.getTarget().isImplicit(), is(true));
+		assertThat(staticInvocation.getTarget(), is(instanceOf(CtTypeAccess.class)));
+		assertThat(
+			((CtTypeAccess<?>) staticInvocation.getTarget()).getAccessedType().getQualifiedName(),
+			is("java.lang.System")
+		);
+
+	}
 }

src/test/java/spoon/test/targeted/TargetedExpressionTest.java

@@ -56,6 +56,9 @@
 import spoon.test.targeted.testclasses.Tapas;
 import spoon.testing.utils.ModelTest;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -401,13 +404,29 @@ public void testTargetsOfInvInInnerClass() throws Exception {
 		final List<CtInvocation<?>> elements = innerInvMethod.getElements(new TypeFilter<>(CtInvocation.class));
 		assertEquals(8, elements.size());
 		expectedThisAccess.setType(expectedInnerClass);
-		assertEqualsInvocation(new ExpectedTargetedExpression().declaringType(expectedType).target(expectedThisAccess).result("inv()"), elements.get(0));
+		assertThat(elements.get(0).getTarget().isImplicit(), is(true));
+		assertThat(elements.get(0).getTarget(), is(instanceOf(CtThisAccess.class)));
+		assertThat(
+			((CtTypeAccess<?>) ((CtThisAccess<?>) elements.get(0).getTarget()).getTarget())
+				.getAccessedType()
+				.getQualifiedName(),
+			is("spoon.test.targeted.testclasses.Foo")
+		);
+		assertThat(elements.get(0).getExecutable().getSimpleName(), is("inv"));
 		expectedThisAccess.setType(expectedType);
 		assertEqualsInvocation(new ExpectedTargetedExpression().declaringType(expectedType).target(expectedThisAccess).result("this.inv()"), elements.get(1));
 		assertEqualsInvocation(new ExpectedTargetedExpression().declaringType(expectedType).target(fooTypeAccess).result("spoon.test.targeted.testclasses.Foo.staticMethod()"), elements.get(2));
 		assertEqualsInvocation(new ExpectedTargetedExpression().declaringType(expectedType).target(fooTypeAccess).result("spoon.test.targeted.testclasses.Foo.staticMethod()"), elements.get(3));
 		expectedSuperThisAccess.setType(expectedInnerClass);
-		assertEqualsInvocation(new ExpectedTargetedExpression().declaringType(expectedSuperClassType).target(expectedSuperThisAccess).result("superMethod()"), elements.get(4));
+		assertThat(elements.get(4).getTarget().isImplicit(), is(true));
+		assertThat(elements.get(4).getTarget(), is(instanceOf(CtThisAccess.class)));
+		assertThat(
+			((CtTypeAccess<?>) ((CtThisAccess<?>) elements.get(4).getTarget()).getTarget())
+				.getAccessedType()
+				.getQualifiedName(),
+			is("spoon.test.targeted.testclasses.Foo")
+		);
+		assertThat(elements.get(4).getExecutable().getSimpleName(), is("superMethod"));
 		assertEqualsInvocation(new ExpectedTargetedExpression().declaringType(expectedSuperClassType).target(expectedThisAccess).result("this.superMethod()"), elements.get(5));
 		assertEqualsInvocation(new ExpectedTargetedExpression().declaringType(expectedInnerClass).target(expectedInnerClassAccess).result("method()"), elements.get(6));
 		assertEqualsInvocation(new ExpectedTargetedExpression().declaringType(expectedInnerClass).target(expectedInnerClassAccess).result("this.method()"), elements.get(7));

src/test/resources/importCleaner/StaticImports.java

@@ -0,0 +1,15 @@
+package test;
+
+// static field import
+import static java.lang.Integer.MAX_VALUE;
+
+// static method import
+import static java.lang.System.lineSeparator;
+
+public class Test {
+	public int test() {
+		test();
+		lineSeparator();
+		return MAX_VALUE;
+	}
+}

src/test/resources/imports/UnqualifiedCalls.java

@@ -0,0 +1,14 @@
+import static java.lang.System.lineSeparator;
+public class Test {
+	public void outerMethod() {}
+	public static void staticOuterMethod() {}
+
+	public class Inner {
+		void entrypoint() {
+			entrypoint();
+			outerMethod();
+			staticOuterMethod();
+			lineSeparator();
+		}
+	}
+}