fix getAllInvolvedRawTypes() recursing infinitely

For recursive type parameter definitions like `class Example<T extends Comparable<T>>` 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 <peter.gafert@archunit.org>
TNG · Apr 7, 2024 · f002cbb · f002cbb
1 parent 088d00c
commit f002cbb
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 55 deletions.
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
@@ -657,11 +657,6 @@ public JavaClass toErasure() {
         return this;
     }
 
-    @Override
-    public Set<JavaClass> 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
@@ -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<JavaClass> 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
@@ -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<JavaClass> getAllInvolvedRawTypes();
+    default Set<JavaClass> getAllInvolvedRawTypes() {
+        ImmutableSet.Builder<JavaClass> 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}.<br>

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
@@ -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.<br>
@@ -121,14 +119,6 @@ public JavaClass toErasure() {
         return erasure;
     }
 
-    @Override
-    public Set<JavaClass> 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
@@ -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<JavaClass> 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
@@ -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<JavaClass> getAllInvolvedRawTypes() {
-            return Stream.concat(
-                    type.getAllInvolvedRawTypes().stream(),
-                    typeArguments.stream().map(JavaType::getAllInvolvedRawTypes).flatMap(Set::stream)
-            ).collect(toSet());
-        }
-
         @Override
         public List<JavaType> 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
@@ -45,6 +45,19 @@ T method(List<Map<? extends Number, Set<? super String[][]>>> 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<T extends SomeClass<T>> {
+            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
@@ -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<T extends String & List<Serializable>> {
+            @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);
+    }
+}
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
@@ -132,18 +132,6 @@ class SampleClass<T extends String & List<Serializable[]>> {
         assertThatTypes(typeVariable.getAllInvolvedRawTypes()).matchInAnyOrder(String.class, List.class, Serializable.class);
     }
 
-    @Test
-    public void all_involved_raw_types_of_generic_array() {
-        class SampleClass<T extends String & List<Serializable>> {
-            @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")