From 6a0a9fae4508c87c26dae3f54e7c28a29b64186c Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Tue, 23 May 2023 13:30:48 +0200 Subject: [PATCH 1/7] refactor: Simplify declaration node glue dispatch --- .../java/spoon/support/adaption/DeclarationNode.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/spoon/support/adaption/DeclarationNode.java b/src/main/java/spoon/support/adaption/DeclarationNode.java index b27837d56dd..12d53843a7f 100644 --- a/src/main/java/spoon/support/adaption/DeclarationNode.java +++ b/src/main/java/spoon/support/adaption/DeclarationNode.java @@ -69,12 +69,10 @@ public Optional> resolveTypeParameter(CtTypeParameterReferenc // We try to find a glue node below us to delegate to. Glue nodes do the mapping so we can just // pass it on unchanged. - Optional glueNode = children.stream() - .filter(it -> it.isInducedBy(this.inducedBy)) - .findFirst(); - - if (glueNode.isPresent()) { - return glueNode.get().resolveTypeParameter(reference); + if (!children.isEmpty()) { + // We pick a random child. Well-typed programs will converge to the same solution, no matter + // which path we pick. + return children.iterator().next().resolveTypeParameter(reference); } // If we have no glue node, we need to actually resolve the type parameter as we reached the From bceabb508a75177ee6e5e8eb4b5e094257d417f2 Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Tue, 23 May 2023 13:31:05 +0200 Subject: [PATCH 2/7] fix: Build hierarchies for enclosing classes if they declare used generic --- .../spoon/support/adaption/TypeAdaptor.java | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/main/java/spoon/support/adaption/TypeAdaptor.java b/src/main/java/spoon/support/adaption/TypeAdaptor.java index 9fee036b218..b19f564e45f 100644 --- a/src/main/java/spoon/support/adaption/TypeAdaptor.java +++ b/src/main/java/spoon/support/adaption/TypeAdaptor.java @@ -564,6 +564,11 @@ private DeclarationNode buildHierarchyFrom(CtTypeReference startReference, Ct CtType endType = findDeclaringType(end); Map, DeclarationNode> declarationNodes = new HashMap<>(); + if (needToMoveStartTypeToEnclosingClass(end, endType)) { + startType = moveStartTypeToEnclosingClass(hierarchyStart, endType.getReference()); + startReference = startType.getReference(); + } + DeclarationNode root = buildDeclarationHierarchyFrom( startType.getReference(), endType, @@ -583,6 +588,38 @@ private DeclarationNode buildHierarchyFrom(CtTypeReference startReference, Ct .orElse(null); } + private boolean needToMoveStartTypeToEnclosingClass(CtTypeReference end, CtType endType) { + if (!(end instanceof CtTypeParameterReference)) { + return false; + } + // Declaring type is not the same as the inner type (i.e. the type parameter was declared on an + // enclosing type) + CtType parentType = end.getParent(CtType.class); + if (parentType instanceof CtTypeParameter) { + CtFormalTypeDeclarer declarer = ((CtTypeParameter) parentType).getTypeParameterDeclarer(); + if (declarer instanceof CtType) { + parentType = (CtType) declarer; + } else { + parentType = declarer.getDeclaringType(); + } + } + + return !parentType.getQualifiedName().equals(endType.getQualifiedName()); + } + + private CtType moveStartTypeToEnclosingClass(CtType start, CtTypeReference endRef) { + CtType current = start; + while (current != null) { + if (isSubtype(current, endRef)) { + return current; + } + current = current.getDeclaringType(); + } + throw new SpoonException( + "Did not find a suitable enclosing type to start parameter type adaption from" + ); + } + /** * This method attempts to find a suitable end type for building our hierarchy. *
@@ -598,12 +635,18 @@ private DeclarationNode buildHierarchyFrom(CtTypeReference startReference, Ct */ private CtType findDeclaringType(CtTypeReference reference) { CtType type = null; - if (reference.isParentInitialized()) { + // Prefer declaration to parent. This will be different if the type parameter is declared on an + // enclosing class. + if (reference instanceof CtTypeParameterReference) { + type = reference.getTypeDeclaration(); + } + if (type == null && reference.isParentInitialized()) { type = reference.getParent(CtType.class); } if (type == null) { type = reference.getTypeDeclaration(); } + if (type instanceof CtTypeParameter) { CtFormalTypeDeclarer declarer = ((CtTypeParameter) type).getTypeParameterDeclarer(); if (declarer instanceof CtType) { From 49e957e0004fc7857ef942cdbf870cc9e426c1d0 Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Tue, 23 May 2023 13:31:12 +0200 Subject: [PATCH 3/7] test: Add test --- .../java/spoon/support/TypeAdaptorTest.java | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/test/java/spoon/support/TypeAdaptorTest.java b/src/test/java/spoon/support/TypeAdaptorTest.java index 01b0ec1d82d..a8e7f99717e 100644 --- a/src/test/java/spoon/support/TypeAdaptorTest.java +++ b/src/test/java/spoon/support/TypeAdaptorTest.java @@ -1,5 +1,6 @@ package spoon.support; +import java.util.stream.Collectors; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -12,7 +13,9 @@ import spoon.reflect.declaration.CtTypeParameter; import spoon.reflect.factory.Factory; import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.visitor.filter.TypeFilter; import spoon.support.adaption.TypeAdaptor; +import spoon.test.GitHubIssue; import spoon.testing.utils.ModelTest; import java.util.List; @@ -440,4 +443,53 @@ public void overloaded(T t) {} public void overriden(T t) {} } + + @GitHubIssue(issueNumber = 5226, fixed = true) + void testAdaptingTypeFromEnclosingClass() { + Launcher launcher = new Launcher(); + launcher.getEnvironment().setComplianceLevel(11); + launcher.addInputResource("src/test/java/spoon/support/TypeAdaptorTest.java"); + CtType type = launcher.getFactory() + .Type() + .get(UseGenericFromEnclosingType.class); + @SuppressWarnings("rawtypes") + List methods = type.getElements(new TypeFilter<>(CtMethod.class)) + .stream() + .filter(it -> it.getSimpleName().equals("someMethod")) + .collect(Collectors.toList()); + CtMethod test1Method = methods.stream() + .filter(it -> !it.getDeclaringType().getSimpleName().startsWith("Extends")) + .findAny() + .orElseThrow(); + CtMethod test2Method = methods.stream() + .filter(it -> it.getDeclaringType().getSimpleName().startsWith("Extends")) + .findAny() + .orElseThrow(); + + assertTrue(test2Method.isOverriding(test1Method)); + assertFalse(test1Method.isOverriding(test2Method)); + } + + public static class UseGenericFromEnclosingType { + + public static class Enclosing { + + public class Enclosed { + + void someMethod(S s, T t) { + } + } + } + + public static class ExtendsEnclosing extends Enclosing { + + public class ExtendsEnclosed extends Enclosed { + + @Override + void someMethod(Integer s, String t) { + throw new UnsupportedOperationException(); + } + } + } + } } From 9f1cfa7b0651ab0073f500236105ed1ae2fc96cc Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Tue, 23 May 2023 22:51:14 +0200 Subject: [PATCH 4/7] Silence qodana --- src/main/java/spoon/support/adaption/TypeAdaptor.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/spoon/support/adaption/TypeAdaptor.java b/src/main/java/spoon/support/adaption/TypeAdaptor.java index b19f564e45f..74e45256926 100644 --- a/src/main/java/spoon/support/adaption/TypeAdaptor.java +++ b/src/main/java/spoon/support/adaption/TypeAdaptor.java @@ -559,6 +559,7 @@ private Optional> getDeclaringMethodOrConstructor(CtTypeReferenc return Optional.of((CtExecutable) parent); } + @SuppressWarnings("AssignmentToMethodParameter") private DeclarationNode buildHierarchyFrom(CtTypeReference startReference, CtType startType, CtTypeReference end) { CtType endType = findDeclaringType(end); From 53c3eb78d53cba93279d19fa0aad8f14dfa9c118 Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Tue, 23 May 2023 22:58:05 +0200 Subject: [PATCH 5/7] refactor: Extract common type parameter declarer extraction logic --- .../spoon/support/adaption/TypeAdaptor.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/spoon/support/adaption/TypeAdaptor.java b/src/main/java/spoon/support/adaption/TypeAdaptor.java index 74e45256926..911d4a29d06 100644 --- a/src/main/java/spoon/support/adaption/TypeAdaptor.java +++ b/src/main/java/spoon/support/adaption/TypeAdaptor.java @@ -596,14 +596,7 @@ private boolean needToMoveStartTypeToEnclosingClass(CtTypeReference end, CtTy // Declaring type is not the same as the inner type (i.e. the type parameter was declared on an // enclosing type) CtType parentType = end.getParent(CtType.class); - if (parentType instanceof CtTypeParameter) { - CtFormalTypeDeclarer declarer = ((CtTypeParameter) parentType).getTypeParameterDeclarer(); - if (declarer instanceof CtType) { - parentType = (CtType) declarer; - } else { - parentType = declarer.getDeclaringType(); - } - } + parentType = resolveTypeParameterToDeclarer(parentType); return !parentType.getQualifiedName().equals(endType.getQualifiedName()); } @@ -648,14 +641,21 @@ private CtType findDeclaringType(CtTypeReference reference) { type = reference.getTypeDeclaration(); } - if (type instanceof CtTypeParameter) { - CtFormalTypeDeclarer declarer = ((CtTypeParameter) type).getTypeParameterDeclarer(); + return resolveTypeParameterToDeclarer(type); + } + + private static CtType resolveTypeParameterToDeclarer(CtType parentType) { + if (parentType instanceof CtTypeParameter) { + CtFormalTypeDeclarer declarer = ((CtTypeParameter) parentType).getTypeParameterDeclarer(); if (declarer instanceof CtType) { return (CtType) declarer; + } else { + return declarer.getDeclaringType(); } - return declarer.getDeclaringType(); } - return type; + // Could not resolve type parameter declarer (no class path mode?). + // Type adaption results will not be accurate, this is just a wild (and probably wrong) guess. + return parentType; } private DeclarationNode buildDeclarationHierarchyFrom( From 4252ba240c8b133bec3d275bf0298167da98df81 Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Wed, 24 May 2023 12:56:33 +0200 Subject: [PATCH 6/7] doc: Extend type adaption documentation --- doc/type_adaption.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/doc/type_adaption.md b/doc/type_adaption.md index 07047e4b82e..a6d8a62d8fa 100644 --- a/doc/type_adaption.md +++ b/doc/type_adaption.md @@ -116,6 +116,43 @@ we need to go from any other node in the tree. #### Finding no hierarchy If we can not find any hierarchy we currently return the input type unchanged. +#### Adapting generics of enclosing classes +Java allows classes to use generics of their enclosing class, e.g. +```java +public class Outer { + public static class Inner { + public void bar(A a, B b) {} + } +} +``` +If you now additionally introduce a dual inheritance relationship +```java +public class OuterSub extends Outer { + public static class InnerSub extends Outer.Inner { + public void bar(A1 a, B1 b) {} + } +} +``` +Adapting the method from `Outer.Inner#bar` to `OuterSub.InnerSub#bar` involves +building the hierarchy from `InnerSub` to `Outer.Inner` and then adapting `A1` +and `B1`. +While this hierarchy can be used to resolve `B` to `B1`, it will not be helpful +for adapting `A` to `A1`: This generic type is passed down through a completely +different inheritance chain. + +To solve this, we check whether a type parameter is declared by the class +containing the type reference. +For example, when translating `A1` we notice that even though the usage is in +`InnerSub#bar`, the declaration of the type parameter was in `OuterSub`. +Therefore, we adjust the end of our hierarchy to `Outer` instead of `Inner` and +the start to the innermost enclosing class inheriting from that, which is +`OuterSub` in our example. +Notice that there could be *multiple* matching enclosing classes, but we +have no way to decide which one to use, and just arbitrarily resolve the +ambiguity by picking the first one. + +After this adjustment of the start and end types, the rest of the translation +continues normally. ## Adapting a method to a subclass Closely related to type adaption (but not exactly the same!) is translating From 8a80b6e90619336c127649c49befb53b624fab9d Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Wed, 24 May 2023 18:36:32 +0200 Subject: [PATCH 7/7] doc: Fix inner classes being static --- doc/type_adaption.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/type_adaption.md b/doc/type_adaption.md index a6d8a62d8fa..a9b93db6a6a 100644 --- a/doc/type_adaption.md +++ b/doc/type_adaption.md @@ -120,7 +120,7 @@ If we can not find any hierarchy we currently return the input type unchanged. Java allows classes to use generics of their enclosing class, e.g. ```java public class Outer { - public static class Inner { + public class Inner { public void bar(A a, B b) {} } } @@ -128,7 +128,7 @@ public class Outer { If you now additionally introduce a dual inheritance relationship ```java public class OuterSub extends Outer { - public static class InnerSub extends Outer.Inner { + public class InnerSub extends Outer.Inner { public void bar(A1 a, B1 b) {} } }