From 66d97c5b50956dcbd1feedf36b2591c62dc07c43 Mon Sep 17 00:00:00 2001 From: Manfred Hanke Date: Tue, 15 Dec 2020 22:20:51 +0100 Subject: [PATCH 1/3] initialize non-imported JavaClass with empty dependencies So far, if a user would call `JavaClass.getDirectDependency{From/To}Self()` on a class that was not directly imported (e.g. a class that was not present in the original import, but accessed by a class from that import) it would cause a NPE. While we do want to cut the class graph at that point (i.e. not further track dependencies for classes that are not part of the import), we do not want this exceptional behavior. This change fixes the behavior and sets empty dependencies for all non-directly imported classes, be it classes that have been resolved from the classpath later on, or stub classes, because further resolution has been turned off. Resolves: #397 Signed-off-by: Manfred Hanke --- .../archunit/core/domain/JavaClass.java | 4 +- .../core/importer/ClassFileImporterTest.java | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 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 index 33666da087..e81382ac25 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 @@ -145,8 +145,8 @@ public Set get() { .build(); } }); - private JavaClassDependencies javaClassDependencies; - private ReverseDependencies reverseDependencies; + private JavaClassDependencies javaClassDependencies = new JavaClassDependencies(this); // just for stubs; will be overwritten for imported classes + private ReverseDependencies reverseDependencies = ReverseDependencies.EMPTY; // just for stubs; will be overwritten for imported classes JavaClass(JavaClassBuilder builder) { source = checkNotNull(builder.getSource()); 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 index 2cfd6e1a9e..e95aa75ac4 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java @@ -205,6 +205,8 @@ import static com.tngtech.archunit.testutil.ReflectionTestUtils.field; import static com.tngtech.archunit.testutil.ReflectionTestUtils.method; import static com.tngtech.archunit.testutil.TestUtils.namesOf; +import static com.tngtech.java.junit.dataprovider.DataProviders.$; +import static com.tngtech.java.junit.dataprovider.DataProviders.$$; import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assume.assumeTrue; @@ -1711,6 +1713,48 @@ public void resolve_missing_dependencies_from_classpath_can_be_toogled() throws assertThat(clazz.getSuperClass().get().getMethods()).isEmpty(); } + @DataProvider + public static Object[][] classes_not_directly_imported() { + class Element { + } + @SuppressWarnings("unused") + class DependsOnArray { + Element[] array; + } + ArchConfiguration.get().setResolveMissingDependenciesFromClassPath(true); + JavaClass resolvedFromClasspath = new ClassFileImporter().importClasses(DependsOnArray.class) + .get(DependsOnArray.class).getField("array").getRawType().getComponentType(); + + ArchConfiguration.get().setResolveMissingDependenciesFromClassPath(false); + JavaClass stub = new ClassFileImporter().importClasses(DependsOnArray.class) + .get(DependsOnArray.class).getField("array").getRawType().getComponentType(); + + return $$( + $("Resolved from classpath", resolvedFromClasspath), + $("Stub class", stub) + ); + } + + @Test + @UseDataProvider("classes_not_directly_imported") + public void classes_not_directly_imported_have_empty_dependencies(@SuppressWarnings("unused") String description, JavaClass notDirectlyImported) { + assertThat(notDirectlyImported.getDirectDependenciesFromSelf()).isEmpty(); + assertThat(notDirectlyImported.getDirectDependenciesToSelf()).isEmpty(); + assertThat(notDirectlyImported.getFieldAccessesToSelf()).isEmpty(); + assertThat(notDirectlyImported.getMethodCallsToSelf()).isEmpty(); + assertThat(notDirectlyImported.getConstructorCallsToSelf()).isEmpty(); + assertThat(notDirectlyImported.getAccessesToSelf()).isEmpty(); + assertThat(notDirectlyImported.getFieldsWithTypeOfSelf()).isEmpty(); + assertThat(notDirectlyImported.getMethodsWithParameterTypeOfSelf()).isEmpty(); + assertThat(notDirectlyImported.getMethodsWithReturnTypeOfSelf()).isEmpty(); + assertThat(notDirectlyImported.getMethodThrowsDeclarationsWithTypeOfSelf()).isEmpty(); + assertThat(notDirectlyImported.getConstructorsWithParameterTypeOfSelf()).isEmpty(); + assertThat(notDirectlyImported.getConstructorsWithThrowsDeclarationTypeOfSelf()).isEmpty(); + assertThat(notDirectlyImported.getAnnotationsWithTypeOfSelf()).isEmpty(); + assertThat(notDirectlyImported.getAnnotationsWithParameterTypeOfSelf()).isEmpty(); + assertThat(notDirectlyImported.getInstanceofChecksWithTypeOfSelf()).isEmpty(); + } + @Test public void import_is_resilient_against_broken_class_files() throws Exception { Class expectedClass = getClass(); From 66d8ac44170d8d0700ae969911cba49f361b347c Mon Sep 17 00:00:00 2001 From: Manfred Hanke Date: Thu, 16 Jul 2020 23:28:03 +0200 Subject: [PATCH 2/3] add `JavaClass.isFullyImported()` This method reports whether the class is completed from the ImportContext. Take an import of package `com.foo`, where some class `com.foo.Foo` accesses another class `com.bar.Bar`, then `Foo` would be `isFullyImported() == true`, because it was explicitly covered by the package to be imported, but `Bar` would have `isFullyImported() == false`, because it was not covered by the original package. This method is useful, because it has strong implications, whether the class is fully imported or not. For example only a fully imported classes has its dependencies imported, a class that was just imported as a dependency of a fully imported class will have its dependencies cut off to avoid uncontrollably huge imports. Signed-off-by: Manfred Hanke --- .../archunit/core/domain/JavaClass.java | 13 +++++++ .../core/importer/ClassFileImporterTest.java | 38 ++++++++++--------- 2 files changed, 33 insertions(+), 18 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 index e81382ac25..9e473bddf0 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 @@ -147,6 +147,7 @@ public Set get() { }); private JavaClassDependencies javaClassDependencies = new JavaClassDependencies(this); // just for stubs; will be overwritten for imported classes private ReverseDependencies reverseDependencies = ReverseDependencies.EMPTY; // just for stubs; will be overwritten for imported classes + private boolean fullyImported = false; JavaClass(JavaClassBuilder builder) { source = checkNotNull(builder.getSource()); @@ -1149,6 +1150,17 @@ public Set getInstanceofChecksWithTypeOfSelf() { return reverseDependencies.getInstanceofChecksWithTypeOf(this); } + /** + * @return Whether this class has been fully imported, including all dependencies.
+ * Classes that are only transitively imported are not necessarily fully imported.

+ * Suppose you only import a class {@code Foo} that calls a method of class {@code Bar}. + * Then {@code Bar} is, as a dependency of the fully imported class {@code Foo}, only transitively imported. + */ + @PublicAPI(usage = ACCESS) + public boolean isFullyImported() { + return fullyImported; + } + /** * @param clazz An arbitrary type * @return true, if this {@link JavaClass} represents the same class as the supplied {@link Class}, otherwise false @@ -1301,6 +1313,7 @@ JavaClassDependencies completeFrom(ImportContext context) { codeUnit.completeFrom(context); } javaClassDependencies = new JavaClassDependencies(this); + fullyImported = true; return javaClassDependencies; } 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 index e95aa75ac4..75f59c74a0 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterTest.java @@ -244,6 +244,7 @@ public void imports_simple_class_details() throws Exception { ImportedClasses classes = classesIn("testexamples/simpleimport"); JavaClass javaClass = classes.get(ClassToImportOne.class); + assertThat(javaClass.isFullyImported()).isTrue(); assertThat(javaClass.getName()).as("full name").isEqualTo(ClassToImportOne.class.getName()); assertThat(javaClass.getSimpleName()).as("simple name").isEqualTo(ClassToImportOne.class.getSimpleName()); assertThat(javaClass.getPackageName()).as("package name").isEqualTo(ClassToImportOne.class.getPackage().getName()); @@ -1714,7 +1715,7 @@ public void resolve_missing_dependencies_from_classpath_can_be_toogled() throws } @DataProvider - public static Object[][] classes_not_directly_imported() { + public static Object[][] classes_not_fully_imported() { class Element { } @SuppressWarnings("unused") @@ -1736,23 +1737,24 @@ class DependsOnArray { } @Test - @UseDataProvider("classes_not_directly_imported") - public void classes_not_directly_imported_have_empty_dependencies(@SuppressWarnings("unused") String description, JavaClass notDirectlyImported) { - assertThat(notDirectlyImported.getDirectDependenciesFromSelf()).isEmpty(); - assertThat(notDirectlyImported.getDirectDependenciesToSelf()).isEmpty(); - assertThat(notDirectlyImported.getFieldAccessesToSelf()).isEmpty(); - assertThat(notDirectlyImported.getMethodCallsToSelf()).isEmpty(); - assertThat(notDirectlyImported.getConstructorCallsToSelf()).isEmpty(); - assertThat(notDirectlyImported.getAccessesToSelf()).isEmpty(); - assertThat(notDirectlyImported.getFieldsWithTypeOfSelf()).isEmpty(); - assertThat(notDirectlyImported.getMethodsWithParameterTypeOfSelf()).isEmpty(); - assertThat(notDirectlyImported.getMethodsWithReturnTypeOfSelf()).isEmpty(); - assertThat(notDirectlyImported.getMethodThrowsDeclarationsWithTypeOfSelf()).isEmpty(); - assertThat(notDirectlyImported.getConstructorsWithParameterTypeOfSelf()).isEmpty(); - assertThat(notDirectlyImported.getConstructorsWithThrowsDeclarationTypeOfSelf()).isEmpty(); - assertThat(notDirectlyImported.getAnnotationsWithTypeOfSelf()).isEmpty(); - assertThat(notDirectlyImported.getAnnotationsWithParameterTypeOfSelf()).isEmpty(); - assertThat(notDirectlyImported.getInstanceofChecksWithTypeOfSelf()).isEmpty(); + @UseDataProvider("classes_not_fully_imported") + public void classes_not_fully_imported_have_flag_fullyImported_false_and_empty_dependencies(@SuppressWarnings("unused") String description, JavaClass notFullyImported) { + assertThat(notFullyImported.isFullyImported()).isFalse(); + assertThat(notFullyImported.getDirectDependenciesFromSelf()).isEmpty(); + assertThat(notFullyImported.getDirectDependenciesToSelf()).isEmpty(); + assertThat(notFullyImported.getFieldAccessesToSelf()).isEmpty(); + assertThat(notFullyImported.getMethodCallsToSelf()).isEmpty(); + assertThat(notFullyImported.getConstructorCallsToSelf()).isEmpty(); + assertThat(notFullyImported.getAccessesToSelf()).isEmpty(); + assertThat(notFullyImported.getFieldsWithTypeOfSelf()).isEmpty(); + assertThat(notFullyImported.getMethodsWithParameterTypeOfSelf()).isEmpty(); + assertThat(notFullyImported.getMethodsWithReturnTypeOfSelf()).isEmpty(); + assertThat(notFullyImported.getMethodThrowsDeclarationsWithTypeOfSelf()).isEmpty(); + assertThat(notFullyImported.getConstructorsWithParameterTypeOfSelf()).isEmpty(); + assertThat(notFullyImported.getConstructorsWithThrowsDeclarationTypeOfSelf()).isEmpty(); + assertThat(notFullyImported.getAnnotationsWithTypeOfSelf()).isEmpty(); + assertThat(notFullyImported.getAnnotationsWithParameterTypeOfSelf()).isEmpty(); + assertThat(notFullyImported.getInstanceofChecksWithTypeOfSelf()).isEmpty(); } @Test From 27dae94dce00aa252afa5a5a484ceab51e021436 Mon Sep 17 00:00:00 2001 From: Manfred Hanke Date: Wed, 15 Jul 2020 13:50:41 +0200 Subject: [PATCH 3/3] add method JavaClass.getTransitiveDependenciesFromSelf Signed-off-by: Manfred Hanke --- .../archunit/core/domain/JavaClass.java | 11 ++ .../JavaClassTransitiveDependencies.java | 47 ++++++ .../JavaClassTransitiveDependenciesTest.java | 146 ++++++++++++++++++ 3 files changed, 204 insertions(+) create mode 100644 archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassTransitiveDependencies.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTransitiveDependenciesTest.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 9e473bddf0..74686e19b7 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 @@ -1031,6 +1031,17 @@ public Set getDirectDependenciesFromSelf() { return javaClassDependencies.getDirectDependenciesFromClass(); } + /** + * Returns the transitive closure of all dependencies originating from this class, i.e. its direct dependencies + * and the dependencies from all imported target classes. + * @return all transitive dependencies (including direct dependencies) from this class + * @see #getDirectDependenciesFromSelf() + */ + @PublicAPI(usage = ACCESS) + public Set getTransitiveDependenciesFromSelf() { + return JavaClassTransitiveDependencies.findTransitiveDependenciesFrom(this); + } + /** * Like {@link #getDirectDependenciesFromSelf()}, but instead returns all dependencies where this class * is target. diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassTransitiveDependencies.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassTransitiveDependencies.java new file mode 100644 index 0000000000..2a4e53b304 --- /dev/null +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClassTransitiveDependencies.java @@ -0,0 +1,47 @@ +/* + * Copyright 2014-2020 TNG Technology Consulting GmbH + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.tngtech.archunit.core.domain; + +import com.google.common.collect.ImmutableSet; + +import java.util.HashSet; +import java.util.Set; + +class JavaClassTransitiveDependencies { + private JavaClassTransitiveDependencies() { + } + + static Set findTransitiveDependenciesFrom(JavaClass javaClass) { + ImmutableSet.Builder transitiveDependencies = ImmutableSet.builder(); + Set analyzedClasses = new HashSet<>(); // to avoid infinite recursion for cyclic dependencies + addTransitiveDependenciesFrom(javaClass, transitiveDependencies, analyzedClasses); + return transitiveDependencies.build(); + } + + private static void addTransitiveDependenciesFrom(JavaClass javaClass, ImmutableSet.Builder transitiveDependencies, Set analyzedClasses) { + analyzedClasses.add(javaClass); // currently being analyzed + Set targetClassesToRecurse = new HashSet<>(); + for (Dependency dependency : javaClass.getDirectDependenciesFromSelf()) { + transitiveDependencies.add(dependency); + targetClassesToRecurse.add(dependency.getTargetClass().getBaseComponentType()); + } + for (JavaClass targetClass : targetClassesToRecurse) { + if (!analyzedClasses.contains(targetClass)) { + addTransitiveDependenciesFrom(targetClass, transitiveDependencies, analyzedClasses); + } + } + } +} diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTransitiveDependenciesTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTransitiveDependenciesTest.java new file mode 100644 index 0000000000..94b6a01913 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTransitiveDependenciesTest.java @@ -0,0 +1,146 @@ +package com.tngtech.archunit.core.domain; + +import com.tngtech.archunit.core.importer.ClassFileImporter; +import org.junit.Test; + +import static com.tngtech.archunit.testutil.Assertions.assertThatDependencies; + +public class JavaClassTransitiveDependenciesTest { + + @SuppressWarnings("unused") + static class AcyclicGraph { + static class A { + B b; + C[][] c; + } + + static class B { + Integer i; + } + + static class C { + D d; + } + + static class D { + String s; + } + } + + @Test + public void findsTransitiveDependenciesInAcyclicGraph() { + Class a = AcyclicGraph.A.class; + Class b = AcyclicGraph.B.class; + Class c = AcyclicGraph.C.class; + Class d = AcyclicGraph.D.class; + JavaClasses classes = new ClassFileImporter().importClasses(a, b, c, d); + Class cArray = AcyclicGraph.C[][].class; + + // @formatter:off + assertThatDependencies(classes.get(a).getTransitiveDependenciesFromSelf()) + .contain(a, Object.class) + .contain(a, b) + .contain(b, Object.class) + .contain(b, Integer.class) + .contain(a, cArray) + .contain(c, Object.class) + .contain(c, d) + .contain(d, Object.class) + .contain(d, String.class); + + assertThatDependencies(classes.get(b).getTransitiveDependenciesFromSelf()) + .contain(b, Object.class) + .contain(b, Integer.class); + + assertThatDependencies(classes.get(c).getTransitiveDependenciesFromSelf()) + .contain(c, Object.class) + .contain(c, d) + .contain(d, Object.class) + .contain(d, String.class); + // @formatter:on + } + + @SuppressWarnings("unused") + static class CyclicGraph { + static class A { + B b; + C[][] c; + D d; + } + + static class B { + Integer i; + } + + static class C { + A a; + } + + static class D { + E e; + } + + static class E { + A a; + String s; + } + } + + @Test + public void findsTransitiveDependenciesInCyclicGraph() { + Class a = CyclicGraph.A.class; + Class b = CyclicGraph.B.class; + Class c = CyclicGraph.C.class; + Class d = CyclicGraph.D.class; + Class e = CyclicGraph.E.class; + JavaClasses classes = new ClassFileImporter().importClasses(a, b, c, d, e); + Class cArray = CyclicGraph.C[][].class; + + // @formatter:off + assertThatDependencies(classes.get(a).getTransitiveDependenciesFromSelf()) + .contain(a, Object.class) + .contain(a, b) + .contain(b, Object.class) + .contain(b, Integer.class) + .contain(a, cArray) + .contain(c, Object.class) + .contain(c, a) + .contain(a, d) + .contain(d, Object.class) + .contain(d, e) + .contain(e, Object.class) + .contain(e, a) + .contain(e, String.class); + + assertThatDependencies(classes.get(c).getTransitiveDependenciesFromSelf()) + .contain(c, Object.class) + .contain(c, a) + .contain(a, Object.class) + .contain(a, b) + .contain(b, Object.class) + .contain(b, Integer.class) + .contain(a, cArray) + .contain(a, d) + .contain(d, Object.class) + .contain(d, e) + .contain(e, Object.class) + .contain(e, a) + .contain(e, String.class); + + assertThatDependencies(classes.get(d).getTransitiveDependenciesFromSelf()) + .contain(d, Object.class) + .contain(d, e) + .contain(e, Object.class) + .contain(e, a) + .contain(a, Object.class) + .contain(a, b) + .contain(b, Object.class) + .contain(b, Integer.class) + .contain(a, cArray) + .contain(c, Object.class) + .contain(c, a) + .contain(a, d) + .contain(e, String.class); + // @formatter:on + } +}