Merge pull request #161 from TNG/make_array_type_return_package_name_…

…of_their_component_type Make array type return package name of their component type
TNG · Mar 16, 2019 · 23bf5d2 · 23bf5d2
2 parents f212109 + cf6461f
commit 23bf5d2
Show file tree

Hide file tree

Showing 10 changed files with 120 additions and 38 deletions.
diff --git a/.../src/main/java/com/tngtech/archunit/example/cycle/complexcycles/slice1/UnrelatedEnum.java b/.../src/main/java/com/tngtech/archunit/example/cycle/complexcycles/slice1/UnrelatedEnum.java
@@ -0,0 +1,14 @@
+package com.tngtech.archunit.example.cycle.complexcycles.slice1;
+
+// This was just added for the integration tests since it creates some synthetic byte code
+@SuppressWarnings("SwitchStatementWithTooFewBranches")
+public enum UnrelatedEnum {
+    INSTANCE;
+
+    static void doSomeSwitching(UnrelatedEnum unrelatedEnum) {
+        switch (unrelatedEnum) {
+            case INSTANCE:
+                throw new UnsupportedOperationException();
+        }
+    }
+}
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
@@ -223,11 +223,11 @@ private static class ObjectType extends AbstractType {
             ObjectType(String fullName) {
                 super(fullName, ensureSimpleName(fullName), createPackage(fullName));
             }
+        }
 
-            private static String createPackage(String fullName) {
-                int packageEnd = fullName.lastIndexOf('.');
-                return packageEnd >= 0 ? fullName.substring(0, packageEnd) : "";
-            }
+        private static String createPackage(String fullName) {
+            int packageEnd = fullName.lastIndexOf('.');
+            return packageEnd >= 0 ? fullName.substring(0, packageEnd) : "";
         }
 
         private static class PrimitiveType extends AbstractType {
@@ -249,7 +249,12 @@ public boolean isPrimitive() {
 
         private static class ArrayType extends AbstractType {
             ArrayType(String fullName) {
-                super(fullName, createSimpleName(fullName), "");
+                super(fullName, createSimpleName(fullName), createPackageOfComponentType(fullName));
+            }
+
+            private static String createPackageOfComponentType(String fullName) {
+                String componentType = getCanonicalName(fullName).replace("[]", "");
+                return createPackage(componentType);
             }
 
             private static String createSimpleName(String fullName) {

diff --git a/archunit/src/main/java/com/tngtech/archunit/library/dependencies/Slice.java b/archunit/src/main/java/com/tngtech/archunit/library/dependencies/Slice.java
@@ -47,17 +47,21 @@
  * housing all the classes from the {@code customer} package and so on.
  */
 public final class Slice extends ForwardingSet<JavaClass> implements HasDescription, CanOverrideDescription<Slice> {
+    private final SliceAssignment sliceAssignment;
     private final List<String> matchingGroups;
     private final Description description;
     private final Set<JavaClass> classes;
 
-    private Slice(List<String> matchingGroups, Set<JavaClass> classes) {
-        this(matchingGroups,
+    private Slice(SliceAssignment sliceAssignment, List<String> matchingGroups, Set<JavaClass> classes) {
+        this(sliceAssignment,
+                matchingGroups,
                 new Description("Slice " + Joiner.on(" - ").join(ascendingCaptures(matchingGroups))),
                 classes);
     }
 
-    private Slice(List<String> matchingGroups, Description description, Set<JavaClass> classes) {
+    private Slice(SliceAssignment sliceAssignment, List<String> matchingGroups, Description description,
+            Set<JavaClass> classes) {
+        this.sliceAssignment = sliceAssignment;
         this.matchingGroups = checkNotNull(matchingGroups);
         this.description = checkNotNull(description);
         this.classes = ImmutableSet.copyOf(classes);
@@ -92,22 +96,27 @@ public String getDescription() {
      */
     @Override
     public Slice as(String pattern) {
-        return new Slice(matchingGroups, new Description(pattern), classes);
+        return new Slice(sliceAssignment, matchingGroups, new Description(pattern), classes);
     }
 
     @PublicAPI(usage = ACCESS)
     public Set<Dependency> getDependencies() {
         Set<Dependency> result = new HashSet<>();
         for (JavaClass javaClass : this) {
             for (Dependency dependency : javaClass.getDirectDependenciesFromSelf()) {
-                if (!contains(dependency.getTargetClass())) {
+                if (isNotToOwnSlice(dependency)) {
                     result.add(dependency);
                 }
             }
         }
         return result;
     }
 
+    private boolean isNotToOwnSlice(Dependency dependency) {
+        List<String> dependencyIdentifier = sliceAssignment.getIdentifierOf(dependency.getTargetClass()).getParts();
+        return !dependencyIdentifier.equals(matchingGroups);
+    }
+
     @Override
     public String toString() {
         return getDescription();
@@ -145,14 +154,16 @@ String format(List<String> matchingGroups) {
 
     static class Builder {
         private final List<String> matchingGroups;
+        private final SliceAssignment sliceAssignment;
         private final Set<JavaClass> classes = new HashSet<>();
 
-        private Builder(List<String> matchingGroups) {
+        private Builder(List<String> matchingGroups, SliceAssignment sliceAssignment) {
             this.matchingGroups = matchingGroups;
+            this.sliceAssignment = sliceAssignment;
         }
 
-        static Builder from(List<String> matchingGroups) {
-            return new Builder(matchingGroups);
+        static Builder from(List<String> matchingGroups, SliceAssignment sliceAssignment) {
+            return new Builder(matchingGroups, sliceAssignment);
         }
 
         Builder addClass(JavaClass clazz) {
@@ -161,7 +172,7 @@ Builder addClass(JavaClass clazz) {
         }
 
         Slice build() {
-            return new Slice(matchingGroups, classes);
+            return new Slice(sliceAssignment, matchingGroups, classes);
         }
     }
 }
diff --git a/archunit/src/main/java/com/tngtech/archunit/library/dependencies/Slices.java b/archunit/src/main/java/com/tngtech/archunit/library/dependencies/Slices.java
@@ -225,10 +225,9 @@ public Slices transform(JavaClasses classes) {
         }
 
         private Slices createSlices(JavaClasses classes) {
-            SliceBuilders sliceBuilders = new SliceBuilders();
+            SliceBuilders sliceBuilders = new SliceBuilders(sliceAssignment);
             for (JavaClass clazz : classes) {
-                List<String> identifierParts = sliceAssignment.getIdentifierOf(clazz).getParts();
-                sliceBuilders.add(identifierParts, clazz);
+                sliceBuilders.add(clazz);
             }
             return new Slices(sliceBuilders.build());
         }
@@ -295,18 +294,22 @@ String joinDescription(String first, String second) {
 
     private static class SliceBuilders {
         private final Map<List<String>, Slice.Builder> sliceBuilders = new HashMap<>();
+        private final SliceAssignment sliceAssignment;
 
-        void add(List<String> identifierParts, JavaClass clazz) {
-            if (!identifierParts.isEmpty()) {
-                put(identifierParts, clazz);
-            }
+        SliceBuilders(SliceAssignment sliceAssignment) {
+            this.sliceAssignment = sliceAssignment;
         }
 
-        private void put(List<String> matchingGroups, JavaClass clazz) {
-            if (!sliceBuilders.containsKey(matchingGroups)) {
-                sliceBuilders.put(matchingGroups, Slice.Builder.from(matchingGroups));
+        void add(JavaClass clazz) {
+            List<String> identifierParts = sliceAssignment.getIdentifierOf(clazz).getParts();
+            if (identifierParts.isEmpty()) {
+                return;
+            }
+
+            if (!sliceBuilders.containsKey(identifierParts)) {
+                sliceBuilders.put(identifierParts, Slice.Builder.from(identifierParts, sliceAssignment));
             }
-            sliceBuilders.get(matchingGroups).addClass(clazz);
+            sliceBuilders.get(identifierParts).addClass(clazz);
         }
 
         Set<Slice> build() {

diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java
@@ -4,7 +4,6 @@
 import java.lang.annotation.Retention;
 import java.util.AbstractList;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
@@ -124,14 +123,6 @@ public void inner_class_has_package_of_declaring_class() {
         assertThat(anonymous.getPackageName()).isEqualTo(getClass().getPackage().getName());
     }
 
-    @Test
-    public void Array_class_has_default_package() {
-        JavaClass arrayType = importClassWithContext(Arrays.class)
-                .getMethod("toString", Object[].class).getRawParameterTypes().get(0);
-
-        assertThat(arrayType.getPackageName()).isEmpty();
-    }
-
     @Test
     public void superclasses_are_found() {
         JavaClass clazz = importClassesWithContext(ClassWithTwoFieldsAndTwoMethods.class, SuperClassWithFieldAndMethod.class, Parent.class)

diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java
@@ -85,6 +85,9 @@
 import com.tngtech.archunit.core.importer.testexamples.annotationmethodimport.ClassWithAnnotatedMethods.MethodAnnotationWithIntValue;
 import com.tngtech.archunit.core.importer.testexamples.annotationmethodimport.ClassWithAnnotatedMethods.MethodAnnotationWithStringValue;
 import com.tngtech.archunit.core.importer.testexamples.annotationmethodimport.MethodAnnotationWithArrays;
+import com.tngtech.archunit.core.importer.testexamples.arrays.ClassAccessingOneDimensionalArray;
+import com.tngtech.archunit.core.importer.testexamples.arrays.ClassAccessingTwoDimensionalArray;
+import com.tngtech.archunit.core.importer.testexamples.arrays.ClassUsedInArray;
 import com.tngtech.archunit.core.importer.testexamples.callimport.CallsExternalMethod;
 import com.tngtech.archunit.core.importer.testexamples.callimport.CallsMethodReturningArray;
 import com.tngtech.archunit.core.importer.testexamples.callimport.CallsOtherConstructor;
@@ -168,6 +171,9 @@
 import com.tngtech.archunit.core.importer.testexamples.specialtargets.ClassCallingSpecialTarget;
 import com.tngtech.archunit.testutil.LogTestRule;
 import com.tngtech.archunit.testutil.OutsideOfClassPathRule;
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
 import org.apache.logging.log4j.Level;
 import org.assertj.core.api.Condition;
 import org.junit.After;
@@ -215,9 +221,11 @@
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.constructor;
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.field;
 import static com.tngtech.archunit.testutil.ReflectionTestUtils.method;
+import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assume.assumeTrue;
 
+@RunWith(DataProviderRunner.class)
 public class ClassFileImporterTest {
     @Rule
     public final OutsideOfClassPathRule outsideOfClassPath = new OutsideOfClassPathRule();
@@ -325,6 +333,25 @@ public void creates_JavaPackages_for_each_JavaClass() {
         assertThatClasses(javaPackage.getParent().get().getClasses()).contain(getClass());
     }
 
+    @DataProvider
+    public static Object[][] array_types() {
+        return testForEach(ClassAccessingOneDimensionalArray.class, ClassAccessingTwoDimensionalArray.class);
+    }
+
+    // we want to diverge from the Reflection API in this place, because it is way more useful for dependency checks,
+    // if com.some.SomeArray[].getPackageName() reports 'com.some' instead of '' (which would be ArchUnit's equivalent of null)
+    @Test
+    @UseDataProvider("array_types")
+    public void adds_package_of_component_type_to_arrays(Class<?> classAccessingArray) {
+        JavaClass javaClass = new ClassFileImporter().importPackagesOf(classAccessingArray)
+                .get(classAccessingArray);
+
+        JavaClass arrayType = getOnlyElement(javaClass.getFieldAccessesFromSelf()).getTarget().getRawType();
+
+        assertThat(arrayType.getPackageName()).isEqualTo(ClassUsedInArray.class.getPackage().getName());
+        assertThat(arrayType.getPackage().getName()).isEqualTo(ClassUsedInArray.class.getPackage().getName());
+    }
+
     @Test
     public void imports_fields() throws Exception {
         Set<JavaField> fields = classesIn("testexamples/fieldimport").getFields();

diff --git a/...tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingOneDimensionalArray.java b/...tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingOneDimensionalArray.java
@@ -0,0 +1,9 @@
+package com.tngtech.archunit.core.importer.testexamples.arrays;
+
+public class ClassAccessingOneDimensionalArray {
+    ClassUsedInArray[] array;
+
+    ClassUsedInArray access() {
+        return array[1];
+    }
+}
diff --git a/...tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingTwoDimensionalArray.java b/...tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingTwoDimensionalArray.java
@@ -0,0 +1,10 @@
+package com.tngtech.archunit.core.importer.testexamples.arrays;
+
+@SuppressWarnings({"WeakerAccess", "unused"})
+public class ClassAccessingTwoDimensionalArray {
+    ClassUsedInArray[][] array;
+
+    ClassUsedInArray access() {
+        return array[1][1];
+    }
+}
diff --git a/...rc/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassUsedInArray.java b/...rc/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassUsedInArray.java
@@ -0,0 +1,4 @@
+package com.tngtech.archunit.core.importer.testexamples.arrays;
+
+public class ClassUsedInArray {
+}
diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/Assertions.java b/archunit/src/test/java/com/tngtech/archunit/testutil/Assertions.java
@@ -47,6 +47,7 @@
 import com.tngtech.archunit.testutil.assertion.ArchConditionAssertion;
 import com.tngtech.archunit.testutil.assertion.DependenciesAssertion;
 import com.tngtech.archunit.testutil.assertion.DependencyAssertion;
+import com.tngtech.archunit.testutil.assertion.DescribedPredicateAssertion;
 import com.tngtech.archunit.testutil.assertion.JavaCodeUnitAssertion;
 import com.tngtech.archunit.testutil.assertion.JavaConstructorAssertion;
 import com.tngtech.archunit.testutil.assertion.JavaFieldAssertion;
@@ -56,7 +57,6 @@
 import com.tngtech.archunit.testutil.assertion.JavaMethodAssertion;
 import com.tngtech.archunit.testutil.assertion.JavaMethodsAssertion;
 import com.tngtech.archunit.testutil.assertion.JavaPackagesAssertion;
-import com.tngtech.archunit.testutil.assertion.DescribedPredicateAssertion;
 import org.assertj.core.api.AbstractCharSequenceAssert;
 import org.assertj.core.api.AbstractIterableAssert;
 import org.assertj.core.api.AbstractListAssert;
@@ -321,9 +321,9 @@ public void matches(Class<?> clazz) {
             assertThat(actual.getSimpleName()).as("Simple name of " + actual)
                     .isEqualTo(ensureArrayName(clazz.getSimpleName()));
             assertThat(actual.getPackage().getName()).as("Package of " + actual)
-                    .isEqualTo(clazz.getPackage() != null ? clazz.getPackage().getName() : "");
+                    .isEqualTo(getExpectedPackageName(clazz));
             assertThat(actual.getPackageName()).as("Package name of " + actual)
-                    .isEqualTo(clazz.getPackage() != null ? clazz.getPackage().getName() : "");
+                    .isEqualTo(getExpectedPackageName(clazz));
             assertThat(actual.getModifiers()).as("Modifiers of " + actual)
                     .isEqualTo(JavaModifier.getModifiersForClass(clazz.getModifiers()));
             assertThat(propertiesOf(actual.getAnnotations())).as("Annotations of " + actual)
@@ -627,7 +627,15 @@ private JavaTypeAssertion(JavaType actual) {
         public void isEquivalentTo(Class<?> clazz) {
             assertThat(actual.getName()).as("name").isEqualTo(clazz.getName());
             assertThat(actual.getSimpleName()).as("simple name").isEqualTo(clazz.getSimpleName());
-            assertThat(actual.getPackageName()).as("package").isEqualTo(clazz.getPackage() != null ? clazz.getPackage().getName() : "");
+            String expectedPackageName = getExpectedPackageName(clazz);
+            assertThat(actual.getPackageName()).as("package").isEqualTo(expectedPackageName);
+        }
+    }
+
+    private static String getExpectedPackageName(Class<?> clazz) {
+        if (!clazz.isArray()) {
+            return clazz.getPackage() != null ? clazz.getPackage().getName() : "";
         }
+        return getExpectedPackageName(clazz.getComponentType());
     }
 }