Skip to content

Commit

Permalink
fix: Correctly adapt type parameters inherited from enclosing classes (
Browse files Browse the repository at this point in the history
  • Loading branch information
I-Al-Istannen authored May 25, 2023
1 parent a4681da commit ed9f632
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 11 deletions.
37 changes: 37 additions & 0 deletions doc/type_adaption.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<A> {
public class Inner<B> {
public void bar(A a, B b) {}
}
}
```
If you now additionally introduce a dual inheritance relationship
```java
public class OuterSub<A1> extends Outer<A1> {
public class InnerSub<B1> extends Outer.Inner<B1> {
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
Expand Down
10 changes: 4 additions & 6 deletions src/main/java/spoon/support/adaption/DeclarationNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,10 @@ public Optional<CtTypeReference<?>> 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> 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
Expand Down
54 changes: 49 additions & 5 deletions src/main/java/spoon/support/adaption/TypeAdaptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -559,11 +559,17 @@ private Optional<CtExecutable<?>> getDeclaringMethodOrConstructor(CtTypeReferenc
return Optional.of((CtExecutable<?>) parent);
}

@SuppressWarnings("AssignmentToMethodParameter")
private DeclarationNode buildHierarchyFrom(CtTypeReference<?> startReference, CtType<?> startType,
CtTypeReference<?> end) {
CtType<?> endType = findDeclaringType(end);
Map<CtTypeReference<?>, DeclarationNode> declarationNodes = new HashMap<>();

if (needToMoveStartTypeToEnclosingClass(end, endType)) {
startType = moveStartTypeToEnclosingClass(hierarchyStart, endType.getReference());
startReference = startType.getReference();
}

DeclarationNode root = buildDeclarationHierarchyFrom(
startType.getReference(),
endType,
Expand All @@ -583,6 +589,31 @@ 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);
parentType = resolveTypeParameterToDeclarer(parentType);

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.
* <br>
Expand All @@ -598,20 +629,33 @@ 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();

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(
Expand Down
52 changes: 52 additions & 0 deletions src/test/java/spoon/support/TypeAdaptorTest.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -440,4 +443,53 @@ public <T extends CharSequence> void overloaded(T t) {}

public <T extends String> 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<CtMethod> 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<T> {

public class Enclosed<S> {

void someMethod(S s, T t) {
}
}
}

public static class ExtendsEnclosing extends Enclosing<String> {

public class ExtendsEnclosed extends Enclosed<Integer> {

@Override
void someMethod(Integer s, String t) {
throw new UnsupportedOperationException();
}
}
}
}
}

0 comments on commit ed9f632

Please sign in to comment.