From f002cbb3da7937c4089a6c73008d9d6dafd256f7 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Sun, 17 Mar 2024 22:33:49 +0100 Subject: [PATCH] fix `getAllInvolvedRawTypes()` recursing infinitely For recursive type parameter definitions like `class Example>` the method `JavaClass.getAllInvolvedRawTypes()` causes a `StackOverflowError` at the moment. This is because we have a simple recursion without any short circuit condition if we encounter a type we've already seen before. We can utilize `JavaType.traverseSignature(..)` to solve this, because there the problem is already solved in the visitor pattern implementation. Signed-off-by: Peter Gafert --- .../archunit/core/domain/JavaClass.java | 5 ---- .../core/domain/JavaGenericArrayType.java | 7 ------ .../archunit/core/domain/JavaType.java | 19 ++++++++++++++- .../core/domain/JavaTypeVariable.java | 10 -------- .../core/domain/JavaWildcardType.java | 11 --------- .../core/importer/DomainBuilders.java | 9 ------- .../core/domain/JavaCodeUnitTest.java | 13 ++++++++++ .../core/domain/JavaGenericArrayTypeTest.java | 24 +++++++++++++++++++ .../core/domain/JavaTypeVariableTest.java | 12 ---------- 9 files changed, 55 insertions(+), 55 deletions(-) create mode 100644 archunit/src/test/java/com/tngtech/archunit/core/domain/JavaGenericArrayTypeTest.java diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java index 1e00c5725..044596136 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java @@ -657,11 +657,6 @@ public JavaClass toErasure() { return this; } - @Override - public Set getAllInvolvedRawTypes() { - return ImmutableSet.of(getBaseComponentType()); - } - @Override public void traverseSignature(SignatureVisitor visitor) { SignatureTraversal traversal = SignatureTraversal.from(visitor); diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaGenericArrayType.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaGenericArrayType.java index 92c7ea37d..89c92e089 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaGenericArrayType.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaGenericArrayType.java @@ -15,8 +15,6 @@ */ package com.tngtech.archunit.core.domain; -import java.util.Set; - import com.tngtech.archunit.PublicAPI; import static com.google.common.base.Preconditions.checkNotNull; @@ -69,11 +67,6 @@ public JavaClass toErasure() { return erasure; } - @Override - public Set getAllInvolvedRawTypes() { - return this.componentType.getAllInvolvedRawTypes(); - } - @Override public void traverseSignature(SignatureVisitor visitor) { SignatureTraversal.from(visitor).visitGenericArrayType(this); diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaType.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaType.java index 92f784202..2df7e9614 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaType.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaType.java @@ -22,6 +22,7 @@ import java.util.function.Function; import java.util.function.Supplier; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.base.ChainableFunction; @@ -91,7 +92,23 @@ public interface JavaType extends HasName { * @return All raw types involved in this {@link JavaType} */ @PublicAPI(usage = ACCESS) - Set getAllInvolvedRawTypes(); + default Set getAllInvolvedRawTypes() { + ImmutableSet.Builder result = ImmutableSet.builder(); + traverseSignature(new SignatureVisitor() { + @Override + public Result visitClass(JavaClass type) { + result.add(type); + return CONTINUE; + } + + @Override + public Result visitParameterizedType(JavaParameterizedType type) { + result.add(type.toErasure()); + return CONTINUE; + } + }); + return result.build(); + } /** * Traverses through the signature of this {@link JavaType}.
diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaTypeVariable.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaTypeVariable.java index ce333a71f..1df033425 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaTypeVariable.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaTypeVariable.java @@ -17,7 +17,6 @@ import java.lang.reflect.TypeVariable; import java.util.List; -import java.util.Set; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.base.HasDescription; @@ -29,7 +28,6 @@ import static com.tngtech.archunit.core.domain.properties.HasName.Functions.GET_NAME; import static java.util.Collections.emptyList; import static java.util.stream.Collectors.joining; -import static java.util.stream.Collectors.toSet; /** * Represents a type variable used by generic types and members.
@@ -121,14 +119,6 @@ public JavaClass toErasure() { return erasure; } - @Override - public Set getAllInvolvedRawTypes() { - return this.upperBounds.stream() - .map(JavaType::getAllInvolvedRawTypes) - .flatMap(Set::stream) - .collect(toSet()); - } - @Override public void traverseSignature(SignatureVisitor visitor) { SignatureTraversal.from(visitor).visitTypeVariable(this); diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaWildcardType.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaWildcardType.java index 596296ca1..4c7f2e5d2 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaWildcardType.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaWildcardType.java @@ -17,8 +17,6 @@ import java.lang.reflect.WildcardType; import java.util.List; -import java.util.Set; -import java.util.stream.Stream; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.core.domain.properties.HasUpperBounds; @@ -27,7 +25,6 @@ import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; import static com.tngtech.archunit.core.domain.Formatters.ensureCanonicalArrayTypeName; import static java.util.stream.Collectors.joining; -import static java.util.stream.Collectors.toSet; /** * Represents a wildcard type in a type signature (compare the JLS). @@ -98,14 +95,6 @@ public JavaClass toErasure() { return erasure; } - @Override - public Set getAllInvolvedRawTypes() { - return Stream.concat(upperBounds.stream(), lowerBounds.stream()) - .map(JavaType::getAllInvolvedRawTypes) - .flatMap(Set::stream) - .collect(toSet()); - } - @Override public void traverseSignature(SignatureVisitor visitor) { SignatureTraversal.from(visitor).visitWildcardType(this); diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java index fea803a0c..12b04b32c 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/DomainBuilders.java @@ -88,7 +88,6 @@ import static com.tngtech.archunit.core.domain.properties.HasName.Utils.namesOf; import static java.util.Collections.emptyList; import static java.util.stream.Collectors.joining; -import static java.util.stream.Collectors.toSet; @Internal @SuppressWarnings("UnusedReturnValue") @@ -1217,14 +1216,6 @@ public JavaClass toErasure() { return type.toErasure(); } - @Override - public Set getAllInvolvedRawTypes() { - return Stream.concat( - type.getAllInvolvedRawTypes().stream(), - typeArguments.stream().map(JavaType::getAllInvolvedRawTypes).flatMap(Set::stream) - ).collect(toSet()); - } - @Override public List getActualTypeArguments() { return typeArguments; diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaCodeUnitTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaCodeUnitTest.java index 726c56230..4cb7aadd5 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaCodeUnitTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaCodeUnitTest.java @@ -45,6 +45,19 @@ T method(List>> input) { .matchInAnyOrder(Collection.class, File.class, Serializable.class, List.class, Map.class, Number.class, Set.class, String.class); } + @Test + public void finding_all_involved_raw_types_terminates_for_recursive_type_parameters() { + @SuppressWarnings("unused") + abstract class SomeClass> { + public abstract T method(); + } + + JavaMethod method = new ClassFileImporter().importClass(SomeClass.class).getMethod("method"); + + assertThatTypes(method.getAllInvolvedRawTypes()) + .matchInAnyOrder(SomeClass.class); + } + @Test public void offers_all_calls_from_Self() { JavaMethod method = importClassWithContext(ClassAccessingOtherClass.class).getMethod("access", ClassBeingAccessed.class); diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaGenericArrayTypeTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaGenericArrayTypeTest.java new file mode 100644 index 000000000..fbf846e65 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaGenericArrayTypeTest.java @@ -0,0 +1,24 @@ +package com.tngtech.archunit.core.domain; + +import java.io.Serializable; +import java.util.List; + +import com.tngtech.archunit.core.importer.ClassFileImporter; +import org.junit.Test; + +import static com.tngtech.archunit.testutil.Assertions.assertThatTypes; + +public class JavaGenericArrayTypeTest { + + @Test + public void all_involved_raw_types_of_generic_array() { + class SampleClass> { + @SuppressWarnings("unused") + private T[][] field; + } + + JavaGenericArrayType typeVariable = (JavaGenericArrayType) new ClassFileImporter().importClass(SampleClass.class).getField("field").getType(); + + assertThatTypes(typeVariable.getAllInvolvedRawTypes()).matchInAnyOrder(String.class, List.class, Serializable.class); + } +} \ No newline at end of file diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaTypeVariableTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaTypeVariableTest.java index b5aa0aea0..dc46add21 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaTypeVariableTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaTypeVariableTest.java @@ -132,18 +132,6 @@ class SampleClass> { assertThatTypes(typeVariable.getAllInvolvedRawTypes()).matchInAnyOrder(String.class, List.class, Serializable.class); } - @Test - public void all_involved_raw_types_of_generic_array() { - class SampleClass> { - @SuppressWarnings("unused") - private T[][] field; - } - - JavaGenericArrayType typeVariable = (JavaGenericArrayType) new ClassFileImporter().importClass(SampleClass.class).getField("field").getType(); - - assertThatTypes(typeVariable.getAllInvolvedRawTypes()).matchInAnyOrder(String.class, List.class, Serializable.class); - } - @Test public void toString_unbounded() { @SuppressWarnings("unused")