From cf6461faac2fc7a79c006514f6e01cf9ab3b05fa Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Sat, 16 Mar 2019 02:02:49 +0100 Subject: [PATCH] JavaClasses of Arrays (e.g. com.myapp.SomeClass[]) now have getPackageName() == 'com.myapp' instead of '' like before. This change is due to the fact that users usually want to see a dependency com.foo -> com.bar, if a Class com.foo.Foo declares and array com.bar.Bar[]. Note that array types can never be directly imported and thus will always have attached a simple JavaPackage object containing only themselves (like stubs). Signed-off-by: Peter Gafert --- .../complexcycles/slice1/UnrelatedEnum.java | 14 +++++++++ .../archunit/core/domain/JavaType.java | 15 ++++++---- .../archunit/library/dependencies/Slice.java | 29 +++++++++++++------ .../archunit/library/dependencies/Slices.java | 25 +++++++++------- .../archunit/core/domain/JavaClassTest.java | 9 ------ .../core/importer/ClassFileImporterTest.java | 27 +++++++++++++++++ .../ClassAccessingOneDimensionalArray.java | 9 ++++++ .../ClassAccessingTwoDimensionalArray.java | 10 +++++++ .../testexamples/arrays/ClassUsedInArray.java | 4 +++ .../tngtech/archunit/testutil/Assertions.java | 16 +++++++--- 10 files changed, 120 insertions(+), 38 deletions(-) create mode 100644 archunit-example/example-plain/src/main/java/com/tngtech/archunit/example/cycle/complexcycles/slice1/UnrelatedEnum.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingOneDimensionalArray.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingTwoDimensionalArray.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassUsedInArray.java diff --git a/archunit-example/example-plain/src/main/java/com/tngtech/archunit/example/cycle/complexcycles/slice1/UnrelatedEnum.java b/archunit-example/example-plain/src/main/java/com/tngtech/archunit/example/cycle/complexcycles/slice1/UnrelatedEnum.java new file mode 100644 index 0000000000..20023892c6 --- /dev/null +++ b/archunit-example/example-plain/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 index 6db3615c2c..9ba0345248 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 @@ -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 index f00dd5e9d8..f8b3a10008 100644 --- 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 implements HasDescription, CanOverrideDescription { + private final SliceAssignment sliceAssignment; private final List matchingGroups; private final Description description; private final Set classes; - private Slice(List matchingGroups, Set classes) { - this(matchingGroups, + private Slice(SliceAssignment sliceAssignment, List matchingGroups, Set classes) { + this(sliceAssignment, + matchingGroups, new Description("Slice " + Joiner.on(" - ").join(ascendingCaptures(matchingGroups))), classes); } - private Slice(List matchingGroups, Description description, Set classes) { + private Slice(SliceAssignment sliceAssignment, List matchingGroups, Description description, + Set classes) { + this.sliceAssignment = sliceAssignment; this.matchingGroups = checkNotNull(matchingGroups); this.description = checkNotNull(description); this.classes = ImmutableSet.copyOf(classes); @@ -92,7 +96,7 @@ 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) @@ -100,7 +104,7 @@ public Set getDependencies() { Set result = new HashSet<>(); for (JavaClass javaClass : this) { for (Dependency dependency : javaClass.getDirectDependenciesFromSelf()) { - if (!contains(dependency.getTargetClass())) { + if (isNotToOwnSlice(dependency)) { result.add(dependency); } } @@ -108,6 +112,11 @@ public Set getDependencies() { return result; } + private boolean isNotToOwnSlice(Dependency dependency) { + List dependencyIdentifier = sliceAssignment.getIdentifierOf(dependency.getTargetClass()).getParts(); + return !dependencyIdentifier.equals(matchingGroups); + } + @Override public String toString() { return getDescription(); @@ -145,14 +154,16 @@ String format(List matchingGroups) { static class Builder { private final List matchingGroups; + private final SliceAssignment sliceAssignment; private final Set classes = new HashSet<>(); - private Builder(List matchingGroups) { + private Builder(List matchingGroups, SliceAssignment sliceAssignment) { this.matchingGroups = matchingGroups; + this.sliceAssignment = sliceAssignment; } - static Builder from(List matchingGroups) { - return new Builder(matchingGroups); + static Builder from(List 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 index 012671158c..2bba74bf16 100644 --- 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 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, Slice.Builder> sliceBuilders = new HashMap<>(); + private final SliceAssignment sliceAssignment; - void add(List identifierParts, JavaClass clazz) { - if (!identifierParts.isEmpty()) { - put(identifierParts, clazz); - } + SliceBuilders(SliceAssignment sliceAssignment) { + this.sliceAssignment = sliceAssignment; } - private void put(List matchingGroups, JavaClass clazz) { - if (!sliceBuilders.containsKey(matchingGroups)) { - sliceBuilders.put(matchingGroups, Slice.Builder.from(matchingGroups)); + void add(JavaClass clazz) { + List 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 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 index 58cdb6c51d..d780e85410 100644 --- 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 index 30b8a95c0a..96001dd51e 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 @@ -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 fields = classesIn("testexamples/fieldimport").getFields(); diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingOneDimensionalArray.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingOneDimensionalArray.java new file mode 100644 index 0000000000..ec473b9898 --- /dev/null +++ b/archunit/src/test/java/com/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/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingTwoDimensionalArray.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassAccessingTwoDimensionalArray.java new file mode 100644 index 0000000000..e228183aa4 --- /dev/null +++ b/archunit/src/test/java/com/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/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassUsedInArray.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/testexamples/arrays/ClassUsedInArray.java new file mode 100644 index 0000000000..d2e994c4df --- /dev/null +++ b/archunit/src/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 index 226e7f965a..b853da9614 100644 --- 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()); } }