We now consider all classes of the graph when creating packages and a…

…ll classes now get their "dependencies to self" attached (even stubs). This means that all dependencies of imported classes will now be sorted into the packages as well and we won't have the surprising case, that we import a package "com.foo" and end up with a case like com.foo.Foo depending on com.foo.Bar[] which in turn is not a member of "com.foo" because it was created as a stub. On the other hand, it is now possible to find elements within JavaClasses.getDefaultPackage().getAllClasses() that are themselves not contained within JavaClasses (because they are just a dependency and not directly imported).

Signed-off-by: Peter Gafert <[email protected]>

archunit/src/main/java/com/tngtech/archunit/core/domain/DomainObjectCreationContext.java

@@ -59,8 +59,10 @@
  */
 @Internal
 public class DomainObjectCreationContext {
-    public static JavaClasses createJavaClasses(Map<String, JavaClass> classes, ImportContext importContext) {
-        return JavaClasses.of(classes, importContext);
+    public static JavaClasses createJavaClasses(
+            Map<String, JavaClass> selectedClasses, Map<String, JavaClass> allClasses, ImportContext importContext) {
+
+        return JavaClasses.of(selectedClasses, allClasses, importContext);
     }
 
     public static JavaClass createJavaClass(JavaClassBuilder builder) {

archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClasses.java

@@ -32,14 +32,15 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Iterables.isEmpty;
 import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
 
 public final class JavaClasses implements DescribedIterable<JavaClass>, CanOverrideDescription<JavaClasses> {
     private final ImmutableMap<String, JavaClass> classes;
     private final JavaPackage defaultPackage;
     private final String description;
 
-    JavaClasses(JavaPackage defaultPackage, Map<String, JavaClass> classes) {
+    private JavaClasses(JavaPackage defaultPackage, Map<String, JavaClass> classes) {
         this(defaultPackage, classes, "classes");
     }
 
@@ -164,18 +165,31 @@ static JavaClasses of(Iterable<JavaClass> classes) {
         for (JavaClass clazz : classes) {
             mapping.put(clazz.getName(), clazz);
         }
-        return new JavaClasses(JavaPackage.from(classes), mapping);
+        JavaPackage defaultPackage = !isEmpty(classes)
+                ? getRoot(classes.iterator().next().getPackage())
+                : JavaPackage.from(classes);
+        return new JavaClasses(defaultPackage, mapping);
     }
 
-    static JavaClasses of(Map<String, JavaClass> classes, ImportContext importContext) {
-        CompletionProcess completionProcess = new CompletionProcess(classes.values(), importContext);
-        JavaPackage defaultPackage = JavaPackage.from(classes.values());
-        for (JavaClass clazz : classes.values()) {
+    private static JavaPackage getRoot(JavaPackage javaPackage) {
+        JavaPackage result = javaPackage;
+        while (result.getParent().isPresent()) {
+            result = result.getParent().get();
+        }
+        return result;
+    }
+
+    static JavaClasses of(
+            Map<String, JavaClass> selectedClasses, Map<String, JavaClass> allClasses, ImportContext importContext) {
+
+        CompletionProcess completionProcess = new CompletionProcess(allClasses.values(), importContext);
+        JavaPackage defaultPackage = JavaPackage.from(allClasses.values());
+        for (JavaClass clazz : allClasses.values()) {
             setPackage(clazz, defaultPackage);
             completionProcess.completeClass(clazz);
         }
         completionProcess.finish();
-        return new JavaClasses(defaultPackage, classes);
+        return new JavaClasses(defaultPackage, selectedClasses);
     }
 
     private static void setPackage(JavaClass clazz, JavaPackage defaultPackage) {

archunit/src/main/java/com/tngtech/archunit/core/importer/ClassGraphCreator.java

@@ -102,7 +102,7 @@ JavaClasses complete() {
         for (RawAccessRecord constructorCallRecord : importRecord.getRawConstructorCallRecords()) {
             tryProcess(constructorCallRecord, AccessRecord.Factory.forConstructorCallRecord(), processedConstructorCallRecords);
         }
-        return createJavaClasses(classes.getDirectlyImported(), this);
+        return createJavaClasses(classes.getDirectlyImported(), classes.getAll(), this);
     }
 
     private void ensureCallTargetsArePresent() {

archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassesTest.java

@@ -1,25 +1,25 @@
 package com.tngtech.archunit.core.domain;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.tngtech.archunit.base.DescribedPredicate;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
 import static com.tngtech.archunit.core.domain.TestUtils.importClassWithContext;
+import static com.tngtech.archunit.core.domain.TestUtils.importClassesWithContext;
+import static com.tngtech.archunit.testutil.Assertions.assertThatClasses;
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class JavaClassesTest {
-    private static final JavaClass SOME_CLASS = importClassWithContext(SomeClass.class);
-    private static final JavaClass SOME_OTHER_CLASS = importClassWithContext(SomeOtherClass.class);
-    private static final ImmutableMap<String, JavaClass> BY_TYPE_NAME = ImmutableMap.of(
-            SomeClass.class.getName(), SOME_CLASS,
-            SomeOtherClass.class.getName(), SOME_OTHER_CLASS);
-    public static final JavaClasses ALL_CLASSES = new JavaClasses(JavaPackage.from(BY_TYPE_NAME.values()), BY_TYPE_NAME);
+    public static final JavaClasses ALL_CLASSES = importClassesWithContext(SomeClass.class, SomeOtherClass.class);
+    private static final JavaClass SOME_CLASS = ALL_CLASSES.get(SomeClass.class);
+    private static final JavaClass SOME_OTHER_CLASS = ALL_CLASSES.get(SomeOtherClass.class);
 
     @Rule
     public final ExpectedException thrown = ExpectedException.none();
@@ -31,6 +31,29 @@ public void restriction_on_classes_should_filter_the_elements() {
         assertThat(onlySomeClass).containsExactly(SOME_CLASS);
     }
 
+    @Test
+    public void restriction_on_classes_should_keep_the_original_package_tree() {
+        JavaClasses restrictedClasses = ALL_CLASSES.that(haveTheNameOf(SomeClass.class));
+
+        JavaPackage javaPackage = restrictedClasses.getPackage(SomeClass.class.getPackage().getName());
+
+        assertThatClasses(javaPackage.getClasses()).contain(SomeOtherClass.class);
+    }
+
+    @Test
+    public void creation_of_JavaClasses_from_existing_classes_should_keep_the_original_package_tree() {
+        JavaClasses classes = JavaClasses.of(singletonList(ALL_CLASSES.get(SomeClass.class)));
+
+        assertThat(classes.getDefaultPackage().getAllClasses()).containsOnlyElementsOf(ALL_CLASSES.getDefaultPackage().getAllClasses());
+    }
+
+    @Test
+    public void creation_of_JavaClasses_from_empty_classes_should_create_empty_default_package() {
+        JavaClasses classes = JavaClasses.of(Collections.<JavaClass>emptySet());
+
+        assertThat(classes.getDefaultPackage().getAllClasses()).isEmpty();
+    }
+
     @Test
     public void restriction_on_classes_should_set_description() {
         JavaClasses onlySomeClass = ALL_CLASSES.that(haveTheNameOf(SomeClass.class));

archunit/src/test/java/com/tngtech/archunit/core/domain/JavaPackageTest.java

@@ -185,8 +185,8 @@ public void iterates_sub_packages() {
 
         JavaPackage java = defaultPackage.getPackage("java");
 
-        assertThatPackages(java.getSubPackages()).containOnlyRelativeNames("lang", "util", "io", "security");
-        assertThatPackages(java.getSubPackages()).containOnlyNames("java.lang", "java.util", "java.io", "java.security");
+        assertThatPackages(java.getSubPackages()).containRelativeNames("lang", "util", "io", "security");
+        assertThatPackages(java.getSubPackages()).containNames("java.lang", "java.util", "java.io", "java.security");
     }
 
     @Test
@@ -205,7 +205,7 @@ public void iterates_all_sub_packages() {
 
         JavaPackage java = defaultPackage.getPackage("java");
 
-        assertThatPackages(java.getAllSubPackages()).matchOnlyPackagesOf(
+        assertThatPackages(java.getAllSubPackages()).containPackagesOf(
                 Object.class, Annotation.class, Collection.class, BlockingQueue.class, Security.class);
     }
 
@@ -221,7 +221,10 @@ public void visit(JavaClass javaClass) {
             }
         });
 
-        assertThatClasses(visitedClasses).matchInAnyOrder(String.class, Serializable.class, Security.class);
+        assertThatClasses(visitedClasses).contain(String.class, Serializable.class, Security.class);
+        for (JavaClass visitedClass : visitedClasses) {
+            assertThat(visitedClass.getSimpleName()).startsWith("S");
+        }
     }
 
     @Test
@@ -236,7 +239,10 @@ public void visit(JavaPackage javaPackage) {
             }
         });
 
-        assertThatPackages(visitedPackages).matchOnlyPackagesOf(Object.class, Annotation.class);
+        assertThatPackages(visitedPackages).containPackagesOf(Object.class, Annotation.class);
+        for (JavaPackage visitedPackage : visitedPackages) {
+            assertThat(visitedPackage.getName()).contains(".lang");
+        }
     }
 
     @Test
@@ -282,13 +288,13 @@ public void has_package_dependencies_to_other_packages() {
         JavaPackage examplePackage = importPackage("packageexamples");
 
         assertThat(examplePackage.getPackage("second").getPackageDependenciesFromSelf())
-                .contains(
+                .containsOnly(
+                        getRoot(examplePackage).getPackage("java.lang"),
                         examplePackage.getPackage("first"),
-                        examplePackage.getPackage("third.sub"))
-                .doesNotContain(examplePackage.getPackage("second"));
+                        examplePackage.getPackage("third.sub"));
 
         assertThat(examplePackage.getPackage("third").getPackageDependenciesFromSelf())
-                .contains(examplePackage.getPackage("first"));
+                .containsOnly(examplePackage.getPackage("first"));
 
         assertThatPackages(examplePackage.getPackage("unrelated").getPackageDependenciesFromSelf())
                 .containOnlyNames("java.lang");
@@ -299,7 +305,7 @@ public void has_package_dependencies_from_other_packages() {
         JavaPackage examplePackage = importPackage("packageexamples");
 
         assertThat(examplePackage.getPackage("first").getPackageDependenciesToSelf())
-                .contains(
+                .containsOnly(
                         examplePackage.getPackage("second"),
                         examplePackage.getPackage("second.sub"),
                         examplePackage.getPackage("third.sub"));
@@ -329,7 +335,10 @@ public void function_GET_CLASSES() {
 
         Iterable<JavaClass> classes = GET_CLASSES.apply(defaultPackage.getPackage("java.lang"));
 
-        assertThatClasses(classes).matchInAnyOrder(Object.class, String.class);
+        assertThatClasses(classes).contain(Object.class, String.class);
+        for (JavaClass javaClass : classes) {
+            assertThat(javaClass.getPackageName()).startsWith("java.lang");
+        }
     }
 
     @Test
@@ -338,7 +347,15 @@ public void function_GET_SUB_PACKAGES() {
 
         Iterable<JavaPackage> packages = GET_SUB_PACKAGES.apply(defaultPackage.getPackage("java.lang"));
 
-        assertThatPackages(packages).matchOnlyPackagesOf(Annotation.class, Field.class);
+        assertThatPackages(packages).containPackagesOf(Annotation.class, Field.class);
+    }
+
+    private JavaPackage getRoot(JavaPackage javaPackage) {
+        JavaPackage result = javaPackage;
+        while (result.getParent().isPresent()) {
+            result = result.getParent().get();
+        }
+        return result;
     }
 
     private Predicate<? super JavaPackage> nameContains(String infix) {

archunit/src/test/java/com/tngtech/archunit/core/domain/TestUtils.java

@@ -79,15 +79,15 @@ public static JavaClass importClassWithContext(Class<?> owner) {
     public static JavaClasses importClassesWithContext(Class<?>... classes) {
         JavaClasses importedHierarchy = importHierarchies(classes);
         final List<String> classNames = JavaClass.namesOf(classes);
-        return importedHierarchy.that(new DescribedPredicate<JavaClass>(importedHierarchy.getDescription()) {
+        return importedHierarchy.that(new DescribedPredicate<JavaClass>("") {
             @Override
             public boolean apply(JavaClass input) {
                 return classNames.contains(input.getName());
             }
-        });
+        }).as(importedHierarchy.getDescription());
     }
 
-    public static JavaMethodCallBuilder newMethodCallBuilder(JavaMethod origin, MethodCallTarget target, int lineNumber) {
+    static JavaMethodCallBuilder newMethodCallBuilder(JavaMethod origin, MethodCallTarget target, int lineNumber) {
         return ImportTestUtils.newMethodCallBuilder(origin, target, lineNumber);
     }
 
@@ -103,7 +103,7 @@ public static MethodCallTarget resolvedTargetFrom(JavaMethod target) {
         return ImportTestUtils.targetFrom(target, Suppliers.ofInstance(Collections.singleton(target)));
     }
 
-    static MethodCallTarget unresolvedTargetFrom(JavaMethod target) {
+    private static MethodCallTarget unresolvedTargetFrom(JavaMethod target) {
         return ImportTestUtils.targetFrom(target, Suppliers.ofInstance(Collections.<JavaMethod>emptySet()));
     }
 

archunit/src/test/java/com/tngtech/archunit/core/domain/packageexamples/second/Second1.java

@@ -4,4 +4,8 @@
 
 public class Second1 {
     First2 first2;
+    // Since we meanwhile consider arrays to be within the package of their component type, we will always run into the problem
+    // that a class is within the same package but was not originally imported.
+    // Those classes should nevertheless have the correct package attached in a consistent way
+    ClassDependingOnOtherSecondClass[] evilArrayCausingClassInSamePackageNotOriginallyImported;
 }

archunit/src/test/java/com/tngtech/archunit/testutil/assertion/JavaPackagesAssertion.java

@@ -3,6 +3,7 @@
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashSet;
+import java.util.Set;
 
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
@@ -27,16 +28,25 @@ private static JavaPackage[] sort(Iterable<JavaPackage> actual) {
         return result;
     }
 
-    public void matchOnlyPackagesOf(Class<?>... classes) {
-        HashSet<String> expectedNames = new HashSet<>();
+    public void containPackagesOf(Class<?>... classes) {
+        Set<String> expectedNames = getExpectedNames(classes);
+        assertThat(getActualNames()).containsAll(expectedNames);
+    }
+
+    private Set<String> getExpectedNames(Class<?>[] classes) {
+        Set<String> expectedNames = new HashSet<>();
         for (Class<?> clazz : classes) {
             expectedNames.add(clazz.getPackage().getName());
         }
-        assertThat(getActualNames()).containsOnlyElementsOf(expectedNames);
+        return expectedNames;
+    }
+
+    public void containRelativeNames(String... relativeNames) {
+        assertThat(getActualRelativeNames()).contains(relativeNames);
     }
 
-    public void containOnlyRelativeNames(String... relativeNames) {
-        assertThat(getActualRelativeNames()).containsOnly(relativeNames);
+    public void containNames(String... names) {
+        assertThat(getActualNames()).contains(names);
     }
 
     public void containOnlyNames(String... names) {