From 201812d64818d0d26982f2250dcbfe0be0308782 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Sun, 18 Feb 2024 11:37:51 +0100 Subject: [PATCH 1/2] fix containsInArchitecture check adding wrong object The corresponding object in a `ConditionEvent` should always be the object that caused the violation, not the condition that checked it. Otherwise, clients that analyse the violating objects can't correctly identify the respective culprit. Signed-off-by: Peter Gafert --- .../archunit/library/Architectures.java | 2 +- .../library/LayeredArchitectureTest.java | 5 ++- .../assertion/ArchRuleCheckAssertion.java | 36 ++++++++++++++++++- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java b/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java index 6ee276c764..45bc47c000 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java @@ -452,7 +452,7 @@ private ArchCondition beContainedInLayers(LayerDefinitions layerDefin @Override public void check(JavaClass javaClass, ConditionEvents events) { if (!ignorePredicate.test(javaClass) && !classContainedInLayers.test(javaClass)) { - events.add(violated(this, String.format("Class <%s> is not contained in architecture", javaClass.getName()))); + events.add(violated(javaClass, String.format("Class <%s> is not contained in architecture", javaClass.getName()))); } } }; diff --git a/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java b/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java index 596c4c223d..f60adc3014 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java @@ -8,6 +8,7 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; +import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.lang.ArchRule; @@ -392,7 +393,9 @@ public void layered_architecture_ensure_all_classes_are_contained_in_architectur .ensureAllClassesAreContainedInArchitecture(); assertThatRule(architectureNotCoveringAllClasses).checking(classes) - .hasOnlyOneViolation("Class <" + Second.class.getName() + "> is not contained in architecture"); + .hasOnlyOneViolation( + object -> object instanceof JavaClass && ((JavaClass) object).isEquivalentTo(Second.class), + "Class <" + Second.class.getName() + "> is not contained in architecture"); LayeredArchitecture architectureCoveringAllClasses = architectureNotCoveringAllClasses .layer("Two").definedBy("..second.."); diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ArchRuleCheckAssertion.java b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ArchRuleCheckAssertion.java index 5895d50fef..25ceb88b51 100644 --- a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ArchRuleCheckAssertion.java +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ArchRuleCheckAssertion.java @@ -1,9 +1,13 @@ package com.tngtech.archunit.testutil.assertion; +import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableSet; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.lang.ArchRule; import com.tngtech.archunit.lang.EvaluationResult; @@ -68,15 +72,35 @@ public ArchRuleCheckAssertion hasNoViolationMatching(String regex) { return this; } - @SuppressWarnings("OptionalGetWithoutIsPresent") public ArchRuleCheckAssertion hasOnlyOneViolation(String violationMessage) { + return hasOnlyOneViolation(__ -> true, violationMessage); + } + + @SuppressWarnings("OptionalGetWithoutIsPresent") + public ArchRuleCheckAssertion hasOnlyOneViolation(Predicate correspondingObjectPredicate, String violationMessage) { assertThat(evaluationResult.getFailureReport().getDetails()).as("Failure report details") .hasSize(1) .first().isEqualTo(violationMessage); assertThat(error.get().getMessage()).contains(violationMessage); + + Set handledViolations = getHandledViolations(evaluationResult); + HandledViolation handledViolation = getOnlyElement(handledViolations); + Object correspondingObject = getOnlyElement(handledViolation.correspondingObjects); + assertThat(correspondingObjectPredicate.test(correspondingObject)) + .as("corresponding object satisfies predicate") + .isTrue(); + assertThat(handledViolation.violationMessage).as("violation message").isEqualTo(violationMessage); return this; } + private Set getHandledViolations(EvaluationResult evaluationResult) { + ImmutableSet.Builder result = ImmutableSet.builder(); + evaluationResult.handleViolations((objects, message) -> { + result.add(new HandledViolation(objects, message)); + }); + return result.build(); + } + public ArchRuleCheckAssertion hasOnlyOneViolationWithStandardPattern(Class violatingClass, String violationDescription) { String violationMessage = toViolationMessage(violatingClass, violationDescription); return hasOnlyOneViolation(violationMessage); @@ -133,4 +157,14 @@ public void hasNoViolation() { private String toViolationMessage(Class violatingClass, String violationDescription) { return "Class <" + violatingClass.getName() + "> " + violationDescription + " in (" + violatingClass.getSimpleName() + ".java:0)"; } + + private static class HandledViolation { + private final Collection correspondingObjects; + private final String violationMessage; + + HandledViolation(Collection correspondingObjects, String violationMessage) { + this.correspondingObjects = correspondingObjects; + this.violationMessage = violationMessage; + } + } } From 771df0615f382bfd1e5e560b1834c60c9f62089d Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Sun, 18 Feb 2024 15:20:22 +0100 Subject: [PATCH 2/2] add automatic conversions to violation handling The extensive freedom of how to create a `ConditionEvent` causes problems for any client that needs more structured results (e.g. tools). `ViolationHandler` provides a convenient API to obtain only those sort of violations that can be handled by a client (e.g. tackling dependencies). However, the type of the `correspondingObject` attached to the `ConditionEvent` can take many forms. From classes to methods, method calls, dependencies or aggregated dependencies (like module dependencies) it's hard to determine the possibilities and handle them all. Furthermore, the type of some objects (e.g. `ComponentDependency` isn't even public, so there is hardly any clean way to handle these objects. To mitigate this a little we now support transparent conversions between compatible objects. E.g. a component dependency can also be considered a set of class dependencies. `ViolationHandler` now allows to be used including these conversions that are implemented by standard ArchUnit objects where reasonable. This allows e.g. to obtain all module dependencies as `Set` for every violation. Signed-off-by: Peter Gafert --- .../com/tngtech/archunit/PublicAPIRules.java | 2 + .../integration/ExamplesIntegrationTest.java | 97 +++++++++---------- ...OnionArchitectureByAnnotationFailures.java | 12 +-- .../testutils/CyclicErrorMatcher.java | 5 + .../archunit/testutils/ExpectedAccess.java | 6 ++ .../testutils/ExpectedModuleDependency.java | 63 ++++++------ .../archunit/testutils/ExpectedRelation.java | 2 +- .../testutils/ExpectedTestFailures.java | 4 +- .../archunit/testutils/HandlingAssertion.java | 35 ++++--- .../testutils/MessageAssertionChain.java | 3 + .../SliceDependencyErrorMatcher.java | 5 + .../tngtech/archunit/core/Convertible.java | 46 +++++++++ .../archunit/core/domain/Dependency.java | 61 +++++++++++- .../archunit/core/domain/JavaAccess.java | 16 ++- .../archunit/lang/EvaluationResult.java | 29 ++++-- .../archunit/lang/ViolationHandler.java | 3 + .../library/cycle_detection/Cycle.java | 13 ++- .../rules/CycleArchCondition.java | 14 ++- .../library/dependencies/SliceDependency.java | 15 ++- .../library/modules/ModuleDependency.java | 14 ++- .../archunit/core/domain/DependencyTest.java | 21 ++++ .../archunit/core/domain/JavaAccessTest.java | 17 ++++ .../archunit/lang/EvaluationResultTest.java | 60 ++++++++++++ .../cycle_detection/CycleInternalTest.java | 71 ++++++++++++++ .../dependencies/SliceDependencyTest.java | 29 ++++++ .../library/dependencies/SliceRuleTest.java | 37 +++++++ .../library/modules/ModuleDependencyTest.java | 30 ++++++ .../modules/syntax/ModuleRuleTest.java | 38 ++++++++ .../test_modules/one/one/ClassOneOne.java | 3 + .../test_modules/two/one/ClassTwoOne.java | 3 + .../tngtech/archunit/testutil/Assertions.java | 6 ++ .../assertion/ConversionAssertion.java | 49 ++++++++++ .../assertion/DependencyAssertion.java | 5 + 33 files changed, 691 insertions(+), 123 deletions(-) create mode 100644 archunit/src/main/java/com/tngtech/archunit/core/Convertible.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/dependencies/SliceDependencyTest.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/modules/ModuleDependencyTest.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ConversionAssertion.java diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/PublicAPIRules.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/PublicAPIRules.java index 7f77edb3c3..0e6793daca 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/PublicAPIRules.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/PublicAPIRules.java @@ -42,6 +42,7 @@ import static com.tngtech.archunit.core.domain.JavaMember.Predicates.declaredIn; import static com.tngtech.archunit.core.domain.JavaModifier.FINAL; import static com.tngtech.archunit.core.domain.JavaModifier.PUBLIC; +import static com.tngtech.archunit.core.domain.JavaModifier.SYNTHETIC; import static com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith; import static com.tngtech.archunit.core.domain.properties.HasModifiers.Predicates.modifier; import static com.tngtech.archunit.core.domain.properties.HasName.Utils.namesOf; @@ -95,6 +96,7 @@ public class PublicAPIRules { public static final ArchRule all_public_API_members_are_accessible = members() .that().areAnnotatedWith(PublicAPI.class) + .and().doNotHaveModifier(SYNTHETIC) .should(bePubliclyAccessible()); @ArchTest diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java index 8fc3430ac8..8a670bcc7d 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java @@ -256,8 +256,7 @@ Stream CodingRulesTest() { expectFailures.ofRule("no classes should use JodaTime, because modern Java projects use the [java.time] API instead") .by(callFromMethod(ClassViolatingCodingRules.class, "jodaTimeIsBad") .toMethod(org.joda.time.DateTime.class, "now") - .inLine(31) - .asDependency()) + .inLine(31)) .by(method(ClassViolatingCodingRules.class, "jodaTimeIsBad") .withReturnType(org.joda.time.DateTime.class)); @@ -636,13 +635,13 @@ Stream DependencyRulesTest() { .ofRule("no classes should depend on upper packages, because that might prevent packages on that level from being split into separate artifacts in a clean way") .by(inheritanceFrom(UseCaseTwoController.class).extending(AbstractController.class)) - .by(callFromConstructor(UseCaseTwoController.class).toConstructor(AbstractController.class).inLine(6).asDependency()) + .by(callFromConstructor(UseCaseTwoController.class).toConstructor(AbstractController.class).inLine(6)) .by(inheritanceFrom(InheritedControllerImpl.class).extending(AbstractController.class)) - .by(callFromConstructor(InheritedControllerImpl.class).toConstructor(AbstractController.class).inLine(5).asDependency()) + .by(callFromConstructor(InheritedControllerImpl.class).toConstructor(AbstractController.class).inLine(5)) .by(inheritanceFrom(InheritedControllerImpl.ChildControl.class).extending(AbstractController.AbstractInnerControl.class)) - .by(callFromConstructor(InheritedControllerImpl.ChildControl.class).toConstructor(AbstractController.AbstractInnerControl.class).inLine(6).asDependency()) - .by(callFromMethod(DaoCallingService.class, "violateLayerRulesTrickily").toConstructor(SomeMediator.class, ServiceViolatingLayerRules.class).inLine(18).asDependency()) - .by(callFromMethod(DaoCallingService.class, "violateLayerRulesTrickily").toMethod(SomeMediator.class, "violateLayerRulesIndirectly").inLine(18).asDependency()) + .by(callFromConstructor(InheritedControllerImpl.ChildControl.class).toConstructor(AbstractController.AbstractInnerControl.class).inLine(6)) + .by(callFromMethod(DaoCallingService.class, "violateLayerRulesTrickily").toConstructor(SomeMediator.class, ServiceViolatingLayerRules.class).inLine(18)) + .by(callFromMethod(DaoCallingService.class, "violateLayerRulesTrickily").toMethod(SomeMediator.class, "violateLayerRulesIndirectly").inLine(18)) .by(annotatedClass(WronglyAnnotated.class).annotatedWith(MyController.class)) .by(inheritanceFrom(VeryCentralCore.class).implementing(SomeOtherBusinessInterface.class)) .by(inheritanceFrom(SomeJpa.class).implementing(SomeDao.class)) @@ -668,16 +667,13 @@ Stream FrozenRulesTest() { .ofRule("no classes should depend on classes that reside in a package '..service..'") .by(callFromMethod(SomeController.class, "doSthController"). toMethod(ServiceViolatingDaoRules.class, "doSthService") - .inLine(11) - .asDependency()) + .inLine(11)) .by(callFromMethod(SomeController.class, "doSthWithSecuredService"). toMethod(ServiceViolatingLayerRules.class, "properlySecured") - .inLine(15) - .asDependency()) + .inLine(15)) .by(callFromMethod(OtherJpa.class, "testConnection") .toMethod(ProxiedConnection.class, "refresh") - .inLine(27) - .asDependency()) + .inLine(27)) .by(method(OtherJpa.class, "testConnection") .checkingInstanceOf(ProxiedConnection.class) .inLine(26)) @@ -685,12 +681,10 @@ Stream FrozenRulesTest() { .ofRule("no classes should depend on classes that are assignable to javax.persistence.EntityManager") .by(callFromMethod(ServiceViolatingDaoRules.class, "illegallyUseEntityManager"). toMethod(EntityManager.class, "persist", Object.class) - .inLine(26) - .asDependency()) + .inLine(26)) .by(callFromMethod(ServiceViolatingDaoRules.class, "illegallyUseEntityManager"). toMethod(ServiceViolatingDaoRules.MyEntityManager.class, "persist", Object.class) - .inLine(27) - .asDependency()) + .inLine(27)) .toDynamicTests(this::withTemporaryViolationStore); } @@ -795,13 +789,13 @@ Stream LayerDependencyRulesTest() { "should depend on classes that reside in a package '..controller..'") .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .getting().field(UseCaseOneTwoController.class, someString) - .inLine(23).asDependency()) + .inLine(23)) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toConstructor(UseCaseTwoController.class) - .inLine(24).asDependency()) + .inLine(24)) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toMethod(UseCaseTwoController.class, doSomethingTwo) - .inLine(25).asDependency()) + .inLine(25)) .by(typeParameter(ServiceHelper.class, "TYPE_PARAMETER_VIOLATING_LAYER_RULE").dependingOn(SomeUtility.class)) .by(typeParameter(ServiceHelper.class, "ANOTHER_TYPE_PARAMETER_VIOLATING_LAYER_RULE").dependingOn(SomeEnum.class)) .by(method(ServiceHelper.class, "violatingLayerRuleByMethodTypeParameters") @@ -847,10 +841,10 @@ Stream LayerDependencyRulesTest() { "depend on classes that reside in a package '..service..'") .by(callFromMethod(DaoCallingService.class, violateLayerRules) .toMethod(ServiceViolatingLayerRules.class, ServiceViolatingLayerRules.doSomething) - .inLine(14).asDependency()) + .inLine(14)) .by(callFromMethod(OtherJpa.class, "testConnection") .toMethod(ProxiedConnection.class, "refresh") - .inLine(27).asDependency()) + .inLine(27)) .by(field(DaoCallingService.class, "service").ofType(ServiceViolatingLayerRules.class)) .by(inheritanceFrom(DaoCallingService.class).implementing(ServiceInterface.class)) .by(method(OtherJpa.class, "testConnection") @@ -861,13 +855,13 @@ Stream LayerDependencyRulesTest() { "only have dependent classes that reside in any package ['..controller..', '..service..']") .by(callFromMethod(DaoCallingService.class, violateLayerRules) .toMethod(ServiceViolatingLayerRules.class, ServiceViolatingLayerRules.doSomething) - .inLine(14).asDependency()) + .inLine(14)) .by(callFromMethod(SomeMediator.class, violateLayerRulesIndirectly) .toMethod(ServiceViolatingLayerRules.class, ServiceViolatingLayerRules.doSomething) - .inLine(15).asDependency()) + .inLine(15)) .by(callFromMethod(OtherJpa.class, "testConnection") .toMethod(ProxiedConnection.class, "refresh") - .inLine(27).asDependency()) + .inLine(27)) .by(inheritanceFrom(DaoCallingService.class).implementing(ServiceInterface.class)) .by(constructor(SomeMediator.class).withParameter(ServiceViolatingLayerRules.class)) .by(field(SomeMediator.class, "service").ofType(ServiceViolatingLayerRules.class)) @@ -880,13 +874,13 @@ Stream LayerDependencyRulesTest() { + "only depend on classes that reside in any package ['..service..', '..persistence..', 'java..', 'javax..']") .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .getting().field(UseCaseOneTwoController.class, someString) - .inLine(23).asDependency()) + .inLine(23)) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toConstructor(UseCaseTwoController.class) - .inLine(24).asDependency()) + .inLine(24)) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toMethod(UseCaseTwoController.class, doSomethingTwo) - .inLine(25).asDependency()) + .inLine(25)) .by(typeParameter(ServiceHelper.class, "TYPE_PARAMETER_VIOLATING_LAYER_RULE").dependingOn(SomeUtility.class)) .by(typeParameter(ServiceHelper.class, "ANOTHER_TYPE_PARAMETER_VIOLATING_LAYER_RULE").dependingOn(SomeEnum.class)) .by(method(ServiceHelper.class, "violatingLayerRuleByMethodTypeParameters") @@ -960,27 +954,27 @@ Stream LayeredArchitectureTest() { .by(callFromMethod(DaoCallingService.class, "violateLayerRules") .toMethod(ServiceViolatingLayerRules.class, "doSomething") .inLine(14) - .asDependency()) + ) .by(callFromMethod(ServiceViolatingLayerRules.class, "illegalAccessToController") .toConstructor(UseCaseTwoController.class) .inLine(24) - .asDependency()) + ) .by(callFromMethod(ServiceViolatingLayerRules.class, "illegalAccessToController") .toMethod(UseCaseTwoController.class, "doSomethingTwo") .inLine(25) - .asDependency()) + ) .by(callFromMethod(ServiceViolatingLayerRules.class, "illegalAccessToController") .getting().field(UseCaseOneTwoController.class, "someString") .inLine(23) - .asDependency()) + ) .by(callFromMethod(OtherJpa.class, "testConnection") .toMethod(ProxiedConnection.class, "refresh") .inLine(27) - .asDependency()) + ) .by(typeParameter(ServiceHelper.class, "TYPE_PARAMETER_VIOLATING_LAYER_RULE").dependingOn(SomeUtility.class)) .by(typeParameter(ServiceHelper.class, "ANOTHER_TYPE_PARAMETER_VIOLATING_LAYER_RULE").dependingOn(SomeEnum.class)) @@ -1036,8 +1030,7 @@ Stream LayeredArchitectureTest() { expectedTestFailures .by(callFromMethod(SomeMediator.class, "violateLayerRulesIndirectly") .toMethod(ServiceViolatingLayerRules.class, "doSomething") - .inLine(15) - .asDependency()) + .inLine(15)) .by(constructor(SomeMediator.class).withParameter(ServiceViolatingLayerRules.class)) .by(field(SomeMediator.class, "service").ofType(ServiceViolatingLayerRules.class)); @@ -1076,24 +1069,24 @@ Stream OnionArchitectureTest() { .inLine(16)) .by(callFromMethod(AdministrationCLI.class, "handle", String[].class, AdministrationPort.class) .toMethod(ProductRepository.class, "getTotalCount") - .inLine(17).asDependency()) + .inLine(17)) .by(callFromMethod(ShoppingController.class, "addToShoppingCart", UUID.class, UUID.class, int.class) .toConstructor(ProductId.class, UUID.class) - .inLine(20).asDependency()) + .inLine(20)) .by(callFromMethod(ShoppingController.class, "addToShoppingCart", UUID.class, UUID.class, int.class) .toConstructor(ShoppingCartId.class, UUID.class) - .inLine(20).asDependency()) + .inLine(20)) .by(method(ShoppingService.class, "addToShoppingCart").withParameter(ProductId.class)) .by(method(ShoppingService.class, "addToShoppingCart").withParameter(ShoppingCartId.class)) .by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class) .toMethod(ShoppingCartRepository.class, "read", ShoppingCartId.class) - .inLine(21).asDependency()) + .inLine(21)) .by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class) .toMethod(ProductRepository.class, "read", ProductId.class) - .inLine(22).asDependency()) + .inLine(22)) .by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class) .toMethod(ShoppingCartRepository.class, "save", ShoppingCart.class) - .inLine(25).asDependency()); + .inLine(25)); ExpectedTestFailures expectedTestFailures = ExpectedTestFailures .forTests( @@ -1146,8 +1139,8 @@ Stream ModulesTest() { BiConsumer expectRespectTheirDeclaredDependenciesViolations = (moduleNames, expected) -> expected - .by(ExpectedModuleDependency.uncontainedFrom(AddressController.class).to(AbstractController.class)) - .by(ExpectedModuleDependency.uncontainedFrom(AddressController.class).to(AbstractController.class)) + .by(ExpectedModuleDependency.uncontained(callFromConstructor(AddressController.class).toConstructor(AbstractController.class).inLine(8))) + .by(ExpectedModuleDependency.uncontained(inheritanceFrom(AddressController.class).extending(AbstractController.class))) .by(ExpectedModuleDependency.fromModule(moduleNames.address()).toModule(moduleNames.catalog()) .including(field(Address.class, "productCatalog").ofType(ProductCatalog.class))) @@ -1163,13 +1156,13 @@ Stream ModulesTest() { .by(ExpectedModuleDependency.fromModule(moduleNames.catalog()).toModule(moduleNames.order()) .including(callFromMethod(ProductCatalog.class, "gonnaDoSomethingIllegalWithOrder") - .toConstructor(Order.class).inLine(12).asDependency()) + .toConstructor(Order.class).inLine(12)) .including(callFromMethod(ProductCatalog.class, "gonnaDoSomethingIllegalWithOrder") - .toMethod(Order.class, "addProducts", Set.class).inLine(16).asDependency())) + .toMethod(Order.class, "addProducts", Set.class).inLine(16))) .by(ExpectedModuleDependency.fromModule(moduleNames.importer()).toModule(moduleNames.customer()) .including(callFromMethod(ProductImport.class, "getCustomer") - .toConstructor(Customer.class).inLine(17).asDependency()) + .toConstructor(Customer.class).inLine(17)) .including(method(ProductImport.class, "getCustomer") .withReturnType(Customer.class))) @@ -1188,7 +1181,7 @@ Stream ModulesTest() { .by(field(Address.class, "productCatalog").ofType(ProductCatalog.class)) .by(field(ProductImport.class, "productCatalog").ofType(ProductCatalog.class)) .by(field(ProductImport.class, "xmlType").ofType(XmlTypes.class)) - .by(callFromMethod(ProductImport.class, "parse", byte[].class).toConstructor(ProductCatalog.class).inLine(21).asDependency()) + .by(callFromMethod(ProductImport.class, "parse", byte[].class).toConstructor(ProductCatalog.class).inLine(21)) .by(method(ProductImport.class, "parse").withReturnType(ProductCatalog.class)); expectedFailures = expectedFailures @@ -1431,11 +1424,11 @@ Stream PlantUmlArchitectureTest() { .by(method(Customer.class, "addOrder") .withParameter(Order.class)) .by(callFromMethod(ProductCatalog.class, "gonnaDoSomethingIllegalWithOrder") - .toConstructor(Order.class).inLine(12).asDependency()) + .toConstructor(Order.class).inLine(12)) .by(callFromMethod(ProductCatalog.class, "gonnaDoSomethingIllegalWithOrder") - .toMethod(Order.class, "addProducts", Set.class).inLine(16).asDependency()) + .toMethod(Order.class, "addProducts", Set.class).inLine(16)) .by(callFromMethod(ProductImport.class, "getCustomer") - .toConstructor(Customer.class).inLine(17).asDependency()) + .toConstructor(Customer.class).inLine(17)) .by(method(ProductImport.class, "getCustomer") .withReturnType(Customer.class)) .by(method(Order.class, "report") @@ -1452,13 +1445,13 @@ Stream PlantUmlArchitectureTest() { .extending(AbstractController.class)) .by(callFromConstructor(AddressController.class) .toConstructor(AbstractController.class) - .inLine(8).asDependency()) + .inLine(8)) .by(field(Product.class, "customer") .ofType(Customer.class)) .by(method(Customer.class, "addOrder") .withParameter(Order.class)) .by(callFromMethod(ProductImport.class, "getCustomer") - .toConstructor(Customer.class).inLine(17).asDependency()) + .toConstructor(Customer.class).inLine(17)) .by(method(ProductImport.class, "getCustomer") .withReturnType(Customer.class)) .by(method(Order.class, "report") diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExpectedOnionArchitectureByAnnotationFailures.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExpectedOnionArchitectureByAnnotationFailures.java index 8099ce0eb9..f86a30ce0a 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExpectedOnionArchitectureByAnnotationFailures.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExpectedOnionArchitectureByAnnotationFailures.java @@ -53,24 +53,24 @@ static void addTo(ExpectedTestFailures expectedTestFailures) { .inLine(18)) .by(callFromMethod(AdministrationCLI.class, "handle", String[].class, AdministrationPort.class) .toMethod(ProductRepository.class, "getTotalCount") - .inLine(19).asDependency()) + .inLine(19)) .by(callFromMethod(ShoppingController.class, "addToShoppingCart", UUID.class, UUID.class, int.class) .toConstructor(ProductId.class, UUID.class) - .inLine(20).asDependency()) + .inLine(20)) .by(callFromMethod(ShoppingController.class, "addToShoppingCart", UUID.class, UUID.class, int.class) .toConstructor(ShoppingCartId.class, UUID.class) - .inLine(20).asDependency()) + .inLine(20)) .by(method(ShoppingService.class, "addToShoppingCart").withParameter(ProductId.class)) .by(method(ShoppingService.class, "addToShoppingCart").withParameter(ShoppingCartId.class)) .by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class) .toMethod(ShoppingCartRepository.class, "read", ShoppingCartId.class) - .inLine(21).asDependency()) + .inLine(21)) .by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class) .toMethod(ProductRepository.class, "read", ProductId.class) - .inLine(22).asDependency()) + .inLine(22)) .by(callFromMethod(ShoppingService.class, "addToShoppingCart", ShoppingCartId.class, ProductId.class, OrderQuantity.class) .toMethod(ShoppingCartRepository.class, "save", ShoppingCart.class) - .inLine(25).asDependency()) + .inLine(25)) .by(constructor(OrderItem.class).withParameter(OrderQuantity.class)) .by(field(OrderItem.class, "quantity").ofType(OrderQuantity.class)); } diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/CyclicErrorMatcher.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/CyclicErrorMatcher.java index 29688a1ae8..5d1a167343 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/CyclicErrorMatcher.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/CyclicErrorMatcher.java @@ -90,6 +90,11 @@ private String detailLinePattern(String string) { return ".*" + quote(string) + ".*"; } + @Override + public void addTo(HandlingAssertion handlingAssertion) { + details.values().forEach(relation -> relation.addTo(handlingAssertion)); + } + @Override public String getDescription() { return String.format("Message contains cycle description '%s' and details '%s'", diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedAccess.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedAccess.java index 10c7e27951..4be5e1719d 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedAccess.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedAccess.java @@ -166,6 +166,9 @@ public ExpectedDependency asDependency() { @Override public void addTo(HandlingAssertion assertion) { assertion.byFieldAccess(this); + if (!getOrigin().getDeclaringClass().equals(getTarget().getDeclaringClass())) { + assertion.byDependency(asDependency()); + } } } @@ -192,6 +195,9 @@ public void addTo(HandlingAssertion assertion) { } else { assertion.byMethodCall(this); } + if (!getOrigin().getDeclaringClass().equals(getTarget().getDeclaringClass())) { + assertion.byDependency(asDependency()); + } } } } diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedModuleDependency.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedModuleDependency.java index daaeb86b1b..c188b3ffc1 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedModuleDependency.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedModuleDependency.java @@ -5,9 +5,8 @@ import java.util.Set; import com.google.common.base.Joiner; -import com.tngtech.archunit.core.domain.Dependency; +import com.tngtech.archunit.testutils.ExpectedRelation.LineAssociation; -import static com.tngtech.archunit.testutils.MessageAssertionChain.matchesLine; import static java.util.regex.Pattern.quote; public class ExpectedModuleDependency implements MessageAssertionChain.Link { @@ -22,8 +21,13 @@ public static ModuleDependencyCreator fromModule(String moduleName) { return new ModuleDependencyCreator(moduleName); } - public static UncontainedCreator uncontainedFrom(Class origin) { - return new UncontainedCreator(origin); + public static MessageAssertionChain.Link uncontained(ExpectedRelation call) { + return new ExpectedUncontainedModuleDependency(call); + } + + @Override + public void addTo(HandlingAssertion handlingAssertion) { + details.forEach(it -> it.addTo(handlingAssertion)); } @Override @@ -58,51 +62,38 @@ public ExpectedModuleDependency toModule(String moduleName) { } } - public static class UncontainedCreator { - private final Class origin; - - private UncontainedCreator(Class origin) { - this.origin = origin; - } + private static class ExpectedUncontainedModuleDependency implements MessageAssertionChain.Link { + private final ExpectedRelation delegate; - public ExpectedUncontainedModuleDependency to(Class target) { - return new ExpectedUncontainedModuleDependency(origin, target); - } - } - - private static class ExpectedUncontainedModuleDependency implements MessageAssertionChain.Link, ExpectedRelation { - private final String dependencyPattern; - private final MessageAssertionChain.Link delegate; - - private ExpectedUncontainedModuleDependency(Class origin, Class target) { - dependencyPattern = String.format("Dependency not contained in any module: .*%s.*%s.*", - quote(origin.getName()), quote(target.getName())); - this.delegate = matchesLine(dependencyPattern); + private ExpectedUncontainedModuleDependency(ExpectedRelation delegate) { + this.delegate = delegate; } @Override public void addTo(HandlingAssertion assertion) { - assertion.byDependency(this); - } - - @Override - public void associateLines(LineAssociation association) { - association.associateIfPatternMatches(dependencyPattern); - } - - @Override - public boolean correspondsTo(Object object) { - return object instanceof Dependency; + delegate.addTo(assertion); } @Override public Result filterMatching(List lines) { - return delegate.filterMatching(lines); + Result.Builder builder = new Result.Builder(); + delegate.associateLines(new LineAssociation() { + @Override + public void associateIfPatternMatches(String pattern) { + builder.matchesLine("Dependency not contained in any module:" + pattern); + } + + @Override + public void associateIfStringIsContained(String string) { + associateIfPatternMatches(".*" + quote(string) + ".*"); + } + }); + return builder.build(lines); } @Override public String getDescription() { - return delegate.getDescription(); + return "Uncontained module dependency: " + delegate; } } } diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedRelation.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedRelation.java index 7cd79aff37..a7902f2c33 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedRelation.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedRelation.java @@ -3,7 +3,7 @@ import com.tngtech.archunit.core.domain.JavaAccess; import com.tngtech.archunit.lang.ConditionEvent; -interface ExpectedRelation { +public interface ExpectedRelation { void addTo(HandlingAssertion assertion); diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedTestFailures.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedTestFailures.java index 787d5fe5e9..b923e681ab 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedTestFailures.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedTestFailures.java @@ -372,9 +372,7 @@ void by(ExpectedDependency inheritance) { void by(MessageAssertionChain.Link assertion) { expectedViolation.by(assertion); - if (assertion instanceof ExpectedRelation) { - ((ExpectedRelation) assertion).addTo(handlingAssertion); - } + assertion.addTo(handlingAssertion); } ExpectedViolationToAssign copy() { diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/HandlingAssertion.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/HandlingAssertion.java index ebe481002a..a98fdd7cfe 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/HandlingAssertion.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/HandlingAssertion.java @@ -5,6 +5,7 @@ import java.util.Iterator; import java.util.Set; import java.util.TreeSet; +import java.util.function.BiPredicate; import com.google.common.base.Joiner; import com.google.common.collect.Sets; @@ -16,14 +17,13 @@ import com.tngtech.archunit.core.domain.JavaMethodCall; import com.tngtech.archunit.lang.EvaluationResult; -import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.common.collect.Sets.union; import static java.lang.System.lineSeparator; import static java.util.Collections.emptySet; import static java.util.Collections.singleton; import static java.util.stream.Collectors.toSet; -class HandlingAssertion { +public class HandlingAssertion { private final Set expectedFieldAccesses; private final Set expectedMethodCalls; private final Set expectedConstructorCalls; @@ -57,8 +57,8 @@ void byMethodCall(ExpectedRelation call) { expectedMethodCalls.add(call); } - void byDependency(ExpectedRelation inheritance) { - expectedDependencies.add(inheritance); + void byDependency(ExpectedRelation dependency) { + expectedDependencies.add(dependency); } static HandlingAssertion ofRule() { @@ -131,15 +131,13 @@ private Set evaluateDependencies(EvaluationResult result) { } private Set removeExpectedAccesses(Collection violatingObjects, Set left) { - Object violatingObject = getOnlyElement(violatingObjects); - for (Iterator actualMethodCalls = left.iterator(); actualMethodCalls.hasNext(); ) { - ExpectedRelation next = actualMethodCalls.next(); - if (next.correspondsTo(violatingObject)) { - actualMethodCalls.remove(); - return emptySet(); - } + removeMatchingElements(violatingObjects, left, (object, relation) -> relation.correspondsTo(object)); + + if (!violatingObjects.isEmpty()) { + return singleton("Unhandled violations: " + violatingObjects); } - return singleton("Unexpected violation handling: " + violatingObject); + + return emptySet(); } private Set errorMessagesFrom(Set set) { @@ -150,6 +148,19 @@ HandlingAssertion copy() { return new HandlingAssertion(expectedFieldAccesses, expectedMethodCalls, expectedConstructorCalls, expectedDependencies); } + private void removeMatchingElements(Collection actual, Collection expected, BiPredicate matches) { + for (Iterator actualIterator = actual.iterator(); actualIterator.hasNext(); ) { + T actualElement = actualIterator.next(); + for (Iterator expectedIterator = expected.iterator(); expectedIterator.hasNext(); ) { + if (matches.test(actualElement, expectedIterator.next())) { + actualIterator.remove(); + expectedIterator.remove(); + break; + } + } + } + } + static class Result { private final Set errorMessages = new TreeSet<>(); diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/MessageAssertionChain.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/MessageAssertionChain.java index f9c423e09c..ad1a7167a6 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/MessageAssertionChain.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/MessageAssertionChain.java @@ -189,6 +189,9 @@ public interface Link { String getDescription(); + default void addTo(HandlingAssertion handlingAssertion) { + } + @Internal class Result { private final boolean matches; diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/SliceDependencyErrorMatcher.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/SliceDependencyErrorMatcher.java index 4e7966f20d..6617a4797c 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/SliceDependencyErrorMatcher.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/SliceDependencyErrorMatcher.java @@ -49,6 +49,11 @@ public Result filterMatching(List lines) { return new Result(true, remainingLines); } + @Override + public void addTo(HandlingAssertion handlingAssertion) { + expectedAccesses.forEach(it -> it.addTo(handlingAssertion)); + } + @Override public String getDescription() { return Joiner.on(System.lineSeparator()).join(ImmutableList.builder() diff --git a/archunit/src/main/java/com/tngtech/archunit/core/Convertible.java b/archunit/src/main/java/com/tngtech/archunit/core/Convertible.java new file mode 100644 index 0000000000..2c1191320e --- /dev/null +++ b/archunit/src/main/java/com/tngtech/archunit/core/Convertible.java @@ -0,0 +1,46 @@ +/* + * Copyright 2014-2024 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; + +import java.util.Set; + +import com.tngtech.archunit.PublicAPI; +import com.tngtech.archunit.core.domain.Dependency; +import com.tngtech.archunit.core.domain.JavaAccess; + +import static com.tngtech.archunit.PublicAPI.Usage.INHERITANCE; + +/** + * Can be implemented to express that this object might also be considered as object(s) of a different type. + * E.g. {@link JavaAccess} and {@link Dependency} (compare {@link #convertTo(Class)}). + */ +@PublicAPI(usage = INHERITANCE) +public interface Convertible { + /** + * Converts this type to a set of other types. + * For example a {@link JavaAccess} can also be + * considered a {@link Dependency}, so javaAccess.convertTo(Dependency.class) + * will yield a set with a single {@link Dependency} representing this access. + * Or a component dependency grouping many class dependencies could be considered a set of exactly + * these class dependencies. + * The result will be an empty set if no conversion is possible + * (e.g. calling javaAccess.convertTo(Integer.class). + * + * @param type The type to convert to + * @return A set of converted elements, empty if no conversion is possible + */ + Set convertTo(Class type); +} diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/Dependency.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/Dependency.java index ca16c2e040..d9759c5b60 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/Dependency.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/Dependency.java @@ -27,6 +27,7 @@ import com.tngtech.archunit.base.ChainableFunction; import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.base.HasDescription; +import com.tngtech.archunit.core.Convertible; import com.tngtech.archunit.core.domain.properties.HasName; import com.tngtech.archunit.core.domain.properties.HasOwner; import com.tngtech.archunit.core.domain.properties.HasSourceCodeLocation; @@ -34,6 +35,8 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; import static com.tngtech.archunit.base.Optionals.asSet; +import static java.util.Collections.emptySet; +import static java.util.Collections.singleton; /** * Represents a dependency of one Java class on another Java class. Such a dependency can occur by either of the @@ -55,7 +58,7 @@ * i.e. origin will never be equal to target. */ @PublicAPI(usage = ACCESS) -public final class Dependency implements HasDescription, Comparable, HasSourceCodeLocation { +public class Dependency implements HasDescription, Comparable, HasSourceCodeLocation, Convertible { private final JavaClass originClass; private final JavaClass targetClass; private final String description; @@ -79,7 +82,9 @@ static Set tryCreateFromAccess(JavaAccess access) { JavaClass targetOwner = access.getTargetOwner(); ImmutableSet.Builder dependencies = ImmutableSet.builder() .addAll(createComponentTypeDependencies(originOwner, access.getOrigin().getDescription(), targetOwner, access.getSourceCodeLocation())); - dependencies.addAll(asSet(tryCreateDependency(originOwner, targetOwner, access.getDescription(), access.getSourceCodeLocation()))); + if (!originOwner.equals(targetOwner) && !targetOwner.isPrimitive()) { + dependencies.add(new Dependency.FromAccess(access)); + } return dependencies.build(); } @@ -270,6 +275,15 @@ public SourceCodeLocation getSourceCodeLocation() { return sourceCodeLocation; } + @Override + @SuppressWarnings("unchecked") // compatibility is explicitly checked + public Set convertTo(Class type) { + if (type.isAssignableFrom(Dependency.class)) { + return (Set) singleton(this); + } + return emptySet(); + } + @Override @PublicAPI(usage = ACCESS) public int compareTo(Dependency o) { @@ -318,6 +332,49 @@ public static JavaClasses toTargetClasses(Iterable dependencies) { return JavaClasses.of(classes); } + private static class FromAccess extends Dependency { + private final JavaAccess access; + + FromAccess(JavaAccess access) { + super(access.getOriginOwner(), access.getTargetOwner(), access.getSourceCodeLocation(), access.getDescription()); + this.access = access; + } + + @Override + @SuppressWarnings("unchecked") // compatibility is explicitly checked + public Set convertTo(Class type) { + if (type.isAssignableFrom(getClass())) { + return (Set) singleton(this); + } + if (type.isAssignableFrom(access.getClass())) { + return (Set) singleton(access); + } + return super.convertTo(type); + } + + @Override + public int hashCode() { + return access.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + final FromAccess other = (FromAccess) obj; + return Objects.equals(this.access, other.access); + } + + @Override + public String toString() { + return getClass().getEnclosingClass().getSimpleName() + "." + super.toString(); + } + } + private static class Origin implements HasOwner, HasDescription { private final JavaClass originClass; private final String originDescription; diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaAccess.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaAccess.java index a061f57f56..627887305f 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaAccess.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaAccess.java @@ -15,12 +15,14 @@ */ package com.tngtech.archunit.core.domain; +import java.util.Collections; import java.util.Set; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.base.ChainableFunction; import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.base.HasDescription; +import com.tngtech.archunit.core.Convertible; import com.tngtech.archunit.core.domain.properties.HasName; import com.tngtech.archunit.core.domain.properties.HasOwner; import com.tngtech.archunit.core.domain.properties.HasOwner.Functions.Get; @@ -33,7 +35,7 @@ @PublicAPI(usage = ACCESS) public abstract class JavaAccess - implements HasName, HasDescription, HasOwner, HasSourceCodeLocation { + implements HasName, HasDescription, HasOwner, HasSourceCodeLocation, Convertible { private final JavaCodeUnit origin; private final TARGET target; @@ -95,6 +97,18 @@ public boolean isDeclaredInLambda() { return declaredInLambda; } + @Override + @SuppressWarnings("unchecked") // compatibility is explicitly checked + public Set convertTo(Class type) { + if (type.isAssignableFrom(getClass())) { + return (Set) Collections.singleton(this); + } + if (type.isAssignableFrom(Dependency.class)) { + return (Set) Dependency.tryCreateFromAccess(this); + } + return Collections.emptySet(); + } + @Override public String toString() { return getClass().getSimpleName() + diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/EvaluationResult.java b/archunit/src/main/java/com/tngtech/archunit/lang/EvaluationResult.java index 17326e3220..7bf289a52a 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/EvaluationResult.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/EvaluationResult.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -30,6 +31,9 @@ import com.google.common.collect.ImmutableSet; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.base.HasDescription; +import com.tngtech.archunit.core.Convertible; +import com.tngtech.archunit.core.domain.Dependency; +import com.tngtech.archunit.core.domain.JavaAccess; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.domain.JavaMethod; @@ -115,6 +119,12 @@ public void add(EvaluationResult part) { * So, in general it is safer to use the wildcard {@code ?} for generic types, unless it is absolutely * certain from the context what the type parameter will be * (for example when only analyzing methods it might be clear that the type parameter will be {@link JavaMethod}). + *

+ * For any {@link ViolationHandler ViolationHandler<T>} violating objects that are not of type T, + * but implement {@link Convertible} will be {@link Convertible#convertTo(Class) converted} to T + * and the result will be passed on to the {@link ViolationHandler}. This makes sense for example for a client + * who wants to handle {@link Dependency}, but the {@link ConditionEvents} corresponding objects are of type + * {@link JavaAccess} which does not share any common meaningful type. * * @param Type of the relevant objects causing violations. E.g. {@code JavaAccess} * @param violationHandler The violation handler that is supposed to handle all violations matching the @@ -141,17 +151,24 @@ private Class componentTypeOf(T[] array) { private ConditionEvent.Handler convertToEventHandler(Class correspondingObjectType, ViolationHandler violationHandler) { return (correspondingObjects, message) -> { - if (allElementTypesMatch(correspondingObjects, correspondingObjectType)) { - // If all elements are assignable to ITEM, covariance of ImmutableList allows this cast - @SuppressWarnings("unchecked") - Collection collection = ImmutableList.copyOf((Collection) correspondingObjects); + Collection collection = getObjectsToHandle(correspondingObjects, correspondingObjectType); + if (!collection.isEmpty()) { violationHandler.handle(collection, message); } }; } - private boolean allElementTypesMatch(Collection violatingObjects, Class supportedElementType) { - return violatingObjects.stream().allMatch(supportedElementType::isInstance); + @SuppressWarnings("unchecked") // compatibility asserted via reflection + private Collection getObjectsToHandle(Collection objects, Class supportedType) { + Set result = new HashSet<>(); + for (Object object : objects) { + if (supportedType.isInstance(object)) { + result.add((T) object); + } else if (object instanceof Convertible) { + result.addAll(((Convertible) object).convertTo(supportedType)); + } + } + return result; } @PublicAPI(usage = ACCESS) diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/ViolationHandler.java b/archunit/src/main/java/com/tngtech/archunit/lang/ViolationHandler.java index 84401ab93e..4c638dd726 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/ViolationHandler.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/ViolationHandler.java @@ -22,6 +22,9 @@ import static com.tngtech.archunit.PublicAPI.State.EXPERIMENTAL; import static com.tngtech.archunit.PublicAPI.Usage.INHERITANCE; +/** + * @see EvaluationResult#handleViolations(ViolationHandler, Object[]) + */ @PublicAPI(usage = INHERITANCE, state = EXPERIMENTAL) public interface ViolationHandler { void handle(Collection violatingObjects, String message); diff --git a/archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/Cycle.java b/archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/Cycle.java index d2ec30d940..d23e20436d 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/Cycle.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/Cycle.java @@ -16,11 +16,14 @@ package com.tngtech.archunit.library.cycle_detection; import java.util.List; +import java.util.Set; import com.tngtech.archunit.PublicAPI; +import com.tngtech.archunit.core.Convertible; import static com.tngtech.archunit.PublicAPI.State.EXPERIMENTAL; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; +import static java.util.stream.Collectors.toSet; /** * A cycle formed by the referenced {@code EDGEs}. A cycle in this context always refers to a "simple" cycle, @@ -31,11 +34,19 @@ * @param The type of the edges forming the cycle */ @PublicAPI(usage = ACCESS, state = EXPERIMENTAL) -public interface Cycle> { +public interface Cycle> extends Convertible { /** * @return The edges of the {@link Cycle} */ @PublicAPI(usage = ACCESS, state = EXPERIMENTAL) List getEdges(); + + @Override + default Set convertTo(Class type) { + return getEdges().stream() + .filter(edge -> edge instanceof Convertible) + .flatMap(edge -> ((Convertible) edge).convertTo(type).stream()) + .collect(toSet()); + } } diff --git a/archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/rules/CycleArchCondition.java b/archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/rules/CycleArchCondition.java index dddc6ba7a7..3281fed71e 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/rules/CycleArchCondition.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/rules/CycleArchCondition.java @@ -33,6 +33,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.SortedSetMultimap; import com.tngtech.archunit.PublicAPI; +import com.tngtech.archunit.core.Convertible; import com.tngtech.archunit.core.domain.Dependency; import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.lang.ArchCondition; @@ -54,8 +55,10 @@ import static com.tngtech.archunit.library.cycle_detection.CycleConfiguration.MAX_NUMBER_OF_CYCLES_TO_DETECT_PROPERTY_NAME; import static com.tngtech.archunit.library.cycle_detection.rules.CycleRuleConfiguration.MAX_NUMBER_OF_DEPENDENCIES_TO_SHOW_PER_EDGE_PROPERTY_NAME; import static java.lang.System.lineSeparator; +import static java.util.Collections.singleton; import static java.util.Comparator.comparing; import static java.util.stream.Collectors.toCollection; +import static java.util.stream.Collectors.toSet; /** * A generic {@link ArchCondition} to check arbitrary {@code COMPONENT}s consisting of {@link JavaClass JavaClasses} @@ -193,7 +196,7 @@ Cycles> findCycles() { } } - private static class ComponentDependency implements Edge { + private static class ComponentDependency implements Edge, Convertible { private final COMPONENT origin; private final COMPONENT target; private final SortedSet classDependencies; @@ -217,6 +220,15 @@ public COMPONENT getTarget() { SortedSet toClassDependencies() { return classDependencies; } + + @Override + @SuppressWarnings("unchecked") // compatibility is explicitly checked + public Set convertTo(Class type) { + if (type.isAssignableFrom(getClass())) { + return (Set) singleton(this); + } + return toClassDependencies().stream().flatMap(it -> it.convertTo(type).stream()).collect(toSet()); + } } private static class EventRecorder { diff --git a/archunit/src/main/java/com/tngtech/archunit/library/dependencies/SliceDependency.java b/archunit/src/main/java/com/tngtech/archunit/library/dependencies/SliceDependency.java index b71e9b296a..86befe73c6 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/dependencies/SliceDependency.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/dependencies/SliceDependency.java @@ -18,20 +18,24 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import com.google.common.base.Joiner; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.base.HasDescription; +import com.tngtech.archunit.core.Convertible; import com.tngtech.archunit.core.domain.Dependency; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; +import static java.util.Collections.singleton; import static java.util.stream.Collectors.toCollection; +import static java.util.stream.Collectors.toSet; import static java.util.stream.StreamSupport.stream; @PublicAPI(usage = ACCESS) -public final class SliceDependency implements HasDescription { +public final class SliceDependency implements HasDescription, Convertible { private final Slice origin; private final SortedSet relevantDependencies; private final Slice target; @@ -79,6 +83,15 @@ private String joinDependencies(Iterable dependencies) { return Joiner.on(System.lineSeparator()).join(parts); } + @Override + @SuppressWarnings("unchecked") // compatibility is explicitly checked + public Set convertTo(Class type) { + if (type.isAssignableFrom(getClass())) { + return (Set) singleton(this); + } + return relevantDependencies.stream().flatMap(it -> it.convertTo(type).stream()).collect(toSet()); + } + @Override public int hashCode() { return Objects.hash(origin, target); diff --git a/archunit/src/main/java/com/tngtech/archunit/library/modules/ModuleDependency.java b/archunit/src/main/java/com/tngtech/archunit/library/modules/ModuleDependency.java index 6c2c60c528..8a5ed17aa2 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/modules/ModuleDependency.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/modules/ModuleDependency.java @@ -21,13 +21,16 @@ import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.base.HasDescription; +import com.tngtech.archunit.core.Convertible; import com.tngtech.archunit.core.domain.Dependency; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.tngtech.archunit.PublicAPI.State.EXPERIMENTAL; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; import static java.lang.System.lineSeparator; +import static java.util.Collections.singleton; import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toSet; /** * A dependency between two {@link ArchModule}s. I.e. {@link #getOrigin() origin} and {@link #getTarget() target} @@ -36,7 +39,7 @@ * {@link Dependency#getTargetClass() target class} resides in the {@link #getTarget() target module}. */ @PublicAPI(usage = ACCESS, state = EXPERIMENTAL) -public final class ModuleDependency implements HasDescription { +public final class ModuleDependency implements HasDescription, Convertible { private final ArchModule origin; private final ArchModule target; private final Set classDependencies; @@ -86,6 +89,15 @@ public String getDescription() { return String.format("Module Dependency [%s -> %s]:%n%s", origin.getName(), target.getName(), classDependencyDescriptions); } + @Override + @SuppressWarnings("unchecked") // compatibility is explicitly checked + public Set convertTo(Class type) { + if (type.isAssignableFrom(getClass())) { + return (Set) singleton(this); + } + return classDependencies.stream().flatMap(it -> it.convertTo(type).stream()).collect(toSet()); + } + @Override public int hashCode() { return Objects.hash(origin.getIdentifier(), target.getIdentifier()); diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/DependencyTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/DependencyTest.java index e12df15b2a..f56e69dbd2 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/domain/DependencyTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/DependencyTest.java @@ -35,8 +35,10 @@ import static com.tngtech.archunit.core.domain.Dependency.Predicates.dependencyTarget; import static com.tngtech.archunit.core.domain.TestUtils.importClassWithContext; import static com.tngtech.archunit.core.domain.TestUtils.importClassesWithContext; +import static com.tngtech.archunit.core.domain.TestUtils.simulateCall; import static com.tngtech.archunit.core.domain.properties.HasType.Predicates.rawType; import static com.tngtech.archunit.testutil.Assertions.assertThat; +import static com.tngtech.archunit.testutil.Assertions.assertThatConversionOf; import static com.tngtech.archunit.testutil.Assertions.assertThatDependencies; import static com.tngtech.archunit.testutil.Assertions.assertThatType; import static com.tngtech.archunit.testutil.assertion.DependenciesAssertion.from; @@ -601,6 +603,25 @@ public void functions() { assertThatType(GET_TARGET_CLASS.apply(createDependency(Origin.class, Target.class))).matches(Target.class); } + @Test + public void convert_dependency_from_access() { + JavaMethodCall call = simulateCall().from(getClass(), "toString").to(Object.class, "toString"); + + Dependency dependency = getOnlyElement(Dependency.tryCreateFromAccess(call)); + + assertThatConversionOf(dependency) + .satisfiesStandardConventions() + .isPossibleToSingleElement(JavaAccess.class, it -> assertThat(it).isEqualTo(call)) + .isPossibleToSingleElement(JavaMethodCall.class, it -> assertThat(it).isEqualTo(call)); + } + + @Test + public void dependency_not_from_access_satisfies_standard_conventions() { + Dependency dependency = createDependency(Origin.class, Target.class); + + assertThatConversionOf(dependency).satisfiesStandardConventions(); + } + private Dependency createDependency(JavaClass origin, JavaClass target) { Dependency dependency = Dependency.fromInheritance(origin, target); assertThatType(dependency.getOriginClass()).as("origin class").isEqualTo(origin); diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaAccessTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaAccessTest.java index f604c69f83..d9e8c84014 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaAccessTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaAccessTest.java @@ -14,6 +14,8 @@ import static com.tngtech.archunit.core.domain.TestUtils.resolvedTargetFrom; import static com.tngtech.archunit.core.domain.TestUtils.simulateCall; import static com.tngtech.archunit.testutil.Assertions.assertThat; +import static com.tngtech.archunit.testutil.Assertions.assertThatConversionOf; +import static com.tngtech.archunit.testutil.Assertions.assertThatDependency; public class JavaAccessTest { @Test @@ -58,6 +60,21 @@ public void origin_predicate() { assertThat(predicate).rejects(anyAccess()); } + @Test + public void convertTo() { + TestJavaAccess access = javaAccessFrom(importClassWithContext(String.class), "toString") + .to(Object.class, "toString") + .inLineNumber(11); + + assertThatConversionOf(access) + .satisfiesStandardConventions() + .isPossibleToSingleElement(Dependency.class, dependency -> { + assertThatDependency(dependency) + .matches(String.class, Object.class) + .hasDescription(access.getDescription()); + }); + } + private TestJavaAccess anyAccess() { return javaAccessFrom(importClassWithContext(SomeClass.Inner.class), "foo") .to(SomeEnum.class, "bar") diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/EvaluationResultTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/EvaluationResultTest.java index bbf428d649..532ad8526b 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/EvaluationResultTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/EvaluationResultTest.java @@ -14,6 +14,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.tngtech.archunit.base.HasDescription; +import com.tngtech.archunit.core.Convertible; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -26,6 +27,8 @@ import static java.nio.file.Files.delete; import static java.nio.file.Files.exists; import static java.util.Arrays.stream; +import static java.util.Collections.emptySet; +import static java.util.Collections.singleton; import static org.assertj.core.api.Assertions.assertThat; public class EvaluationResultTest { @@ -181,6 +184,27 @@ public void ignores_filtered_violations_ignored_by_archunit_ignore_patterns_when assertThat(handledViolations).containsOnly("third one more", "fourth"); } + @Test + public void handles_converted_types() { + ConditionEvents events = events( + SimpleConditionEvent.violated(new CorrectType("correct type"), "message"), + SimpleConditionEvent.violated(new WrongType(), "message"), + SimpleConditionEvent.violated(new ConvertibleToSingleCorrectType("correctly converted"), "message"), + SimpleConditionEvent.violated(new ConvertibleToMultipleCorrectTypes("correctly converted1", "correctly converted2"), "message")); + + EvaluationResult result = evaluationResultWith(events); + + List providedObjects = new ArrayList<>(); + result.handleViolations((Collection types, String __) -> + types.stream().map(it -> it.message).forEach(providedObjects::add)); + + assertThat(providedObjects).containsOnly( + "correct type", + "correctly converted", + "correctly converted1", + "correctly converted2"); + } + private EvaluationResult evaluationResultWith(ConditionEvents events) { return evaluationResultWith(events.getViolating().toArray(new ConditionEvent[0])); } @@ -251,6 +275,42 @@ private static class WrongType { private static class WrongSupertype { } + private static class ConvertibleToSingleCorrectType implements Convertible { + String message; + + ConvertibleToSingleCorrectType(String message) { + this.message = message; + } + + @Override + @SuppressWarnings("unchecked") + public Set convertTo(Class type) { + if (type.isAssignableFrom(CorrectType.class)) { + return (Set) singleton(new CorrectType(message)); + } + return emptySet(); + } + } + + private static class ConvertibleToMultipleCorrectTypes implements Convertible { + private final String message1; + private final String message2; + + ConvertibleToMultipleCorrectTypes(String message1, String message2) { + this.message1 = message1; + this.message2 = message2; + } + + @Override + @SuppressWarnings("unchecked") + public Set convertTo(Class type) { + if (type.isAssignableFrom(CorrectType.class)) { + return (Set) ImmutableSet.of(new CorrectType(message1), new CorrectType(message2)); + } + return emptySet(); + } + } + private static class TestEvent implements ConditionEvent { private final boolean violation; private final List descriptionLines; diff --git a/archunit/src/test/java/com/tngtech/archunit/library/cycle_detection/CycleInternalTest.java b/archunit/src/test/java/com/tngtech/archunit/library/cycle_detection/CycleInternalTest.java index e18cae158b..6f069536c5 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/cycle_detection/CycleInternalTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/cycle_detection/CycleInternalTest.java @@ -1,12 +1,17 @@ package com.tngtech.archunit.library.cycle_detection; import java.util.List; +import java.util.Objects; +import java.util.Set; +import com.tngtech.archunit.core.Convertible; import org.junit.Test; import static com.tngtech.archunit.library.cycle_detection.GraphTest.randomNode; import static com.tngtech.archunit.library.cycle_detection.GraphTest.stringEdge; import static java.util.Arrays.asList; +import static java.util.Collections.emptySet; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -41,4 +46,70 @@ public void minimal_nontrivial_cycle() { assertThat(cycle.getEdges()).hasSize(2); } + + @Test + public void converts_edges() { + CycleInternal> cycle = new CycleInternal<>(asList( + new OriginalEdge("a", "b"), + new OriginalEdge("b", "a")) + ); + + assertThat(cycle.convertTo(ConvertedEdge.class)).containsOnly( + new ConvertedEdge("a", "b"), + new ConvertedEdge("b", "a") + ); + } + + private static class OriginalEdge implements Edge, Convertible { + private final String origin; + private final String target; + + private OriginalEdge(String origin, String target) { + this.origin = origin; + this.target = target; + } + + @Override + public String getOrigin() { + return origin; + } + + @Override + public String getTarget() { + return target; + } + + @Override + @SuppressWarnings("unchecked") + public Set convertTo(Class type) { + return ConvertedEdge.class.isAssignableFrom(type) + ? (Set) singleton(new ConvertedEdge(origin, target)) + : emptySet(); + } + } + + private static class ConvertedEdge { + private final String value; + + public ConvertedEdge(String origin, String target) { + value = origin + "/" + target; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ConvertedEdge that = (ConvertedEdge) o; + return Objects.equals(value, that.value); + } + + @Override + public int hashCode() { + return Objects.hash(value); + } + } } diff --git a/archunit/src/test/java/com/tngtech/archunit/library/dependencies/SliceDependencyTest.java b/archunit/src/test/java/com/tngtech/archunit/library/dependencies/SliceDependencyTest.java new file mode 100644 index 0000000000..0382770201 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/dependencies/SliceDependencyTest.java @@ -0,0 +1,29 @@ +package com.tngtech.archunit.library.dependencies; + +import com.tngtech.archunit.core.domain.Dependency; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import org.junit.Test; + +import static com.tngtech.archunit.library.dependencies.GivenSlicesTest.TEST_CLASSES_PACKAGE; +import static com.tngtech.archunit.testutil.Assertions.assertThatConversionOf; + +public class SliceDependencyTest { + @Test + public void can_be_converted_to_dependencies() { + Slices slices = Slices.matching(TEST_CLASSES_PACKAGE + ".(*)..") + .of(new ClassFileImporter().importPackages(TEST_CLASSES_PACKAGE)); + + Slice origin = getSlice("first", slices); + Slice target = getSlice("second", slices); + + SliceDependency sliceDependency = SliceDependency.of(origin, origin.getDependenciesFromSelf(), target); + + assertThatConversionOf(sliceDependency) + .satisfiesStandardConventions() + .isPossibleTo(Dependency.class); + } + + private Slice getSlice(String name, Slices slices) { + return slices.stream().filter(it -> it.getNamePart(1).equals(name)).findFirst().get(); + } +} \ No newline at end of file diff --git a/archunit/src/test/java/com/tngtech/archunit/library/dependencies/SliceRuleTest.java b/archunit/src/test/java/com/tngtech/archunit/library/dependencies/SliceRuleTest.java index c21a0f844f..78cf89515f 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/dependencies/SliceRuleTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/dependencies/SliceRuleTest.java @@ -1,15 +1,25 @@ package com.tngtech.archunit.library.dependencies; import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.IntStream; import com.google.common.base.Splitter; import com.tngtech.archunit.ArchConfiguration; +import com.tngtech.archunit.core.domain.Dependency; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.library.dependencies.testexamples.completedependencygraph.sevennodes.CompleteSevenNodesGraphRoot; import com.tngtech.archunit.library.dependencies.testexamples.cyclewithunbalanceddependencies.CycleWithUnbalancedDependenciesRoot; +import com.tngtech.archunit.library.testclasses.first.any.pkg.FirstAnyPkgClass; +import com.tngtech.archunit.library.testclasses.first.three.any.FirstThreeAnyClass; +import com.tngtech.archunit.library.testclasses.second.any.pkg.SecondAnyClass; +import com.tngtech.archunit.library.testclasses.second.three.any.SecondThreeAnyClass; +import com.tngtech.archunit.library.testclasses.some.pkg.SomePkgClass; +import com.tngtech.archunit.library.testclasses.some.pkg.sub.SomePkgSubclass; import com.tngtech.archunit.testutil.ArchConfigurationRule; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; @@ -23,9 +33,12 @@ import static com.google.common.math.IntMath.factorial; import static com.tngtech.archunit.library.cycle_detection.CycleConfiguration.MAX_NUMBER_OF_CYCLES_TO_DETECT_PROPERTY_NAME; import static com.tngtech.archunit.library.cycle_detection.rules.CycleRuleTestConfiguration.MAX_NUMBER_OF_DEPENDENCIES_TO_SHOW_PER_EDGE_PROPERTY_NAME; +import static com.tngtech.archunit.library.dependencies.GivenSlicesTest.TEST_CLASSES_PACKAGE; import static com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices; +import static com.tngtech.archunit.testutil.Assertions.assertThatDependencies; 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.lang.System.lineSeparator; import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; @@ -174,6 +187,30 @@ public void allows_empty_should_if_overridden() { ruleWithEmptyShould().allowEmptyShould(true).check(new ClassFileImporter().importClasses(getClass())); } + @DataProvider + public static Object[][] rules() { + return testForEach( + slices().matching(TEST_CLASSES_PACKAGE + ".(*)..").should().notDependOnEachOther(), + slices().matching(TEST_CLASSES_PACKAGE + ".(*)..").should().beFreeOfCycles()); + } + + @Test + @UseDataProvider("rules") + public void handles_violations_as_dependencies(SliceRule rule) { + JavaClasses classes = new ClassFileImporter().importPackages(TEST_CLASSES_PACKAGE); + + Set reportedDependencies = new HashSet<>(); + rule.evaluate(classes).handleViolations( + (Collection dependencies, String __) -> reportedDependencies.addAll(dependencies) + ); + + assertThatDependencies(reportedDependencies) + .contain(FirstThreeAnyClass.class, SecondThreeAnyClass.class) + .contain(FirstAnyPkgClass.class, SomePkgSubclass.class) + .contain(SecondAnyClass.class, FirstAnyPkgClass.class) + .contain(SecondThreeAnyClass.class, SomePkgClass.class); + } + private static SliceRule ruleWithEmptyShould() { return slices().matching("nothing_because_there_is_no_capture_group").should().beFreeOfCycles(); } diff --git a/archunit/src/test/java/com/tngtech/archunit/library/modules/ModuleDependencyTest.java b/archunit/src/test/java/com/tngtech/archunit/library/modules/ModuleDependencyTest.java new file mode 100644 index 0000000000..69d14ffbc8 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/modules/ModuleDependencyTest.java @@ -0,0 +1,30 @@ +package com.tngtech.archunit.library.modules; + +import com.tngtech.archunit.core.domain.Dependency; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.one.ClassOne; +import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.two.ClassTwo; +import org.junit.Test; + +import static com.tngtech.archunit.testutil.Assertions.assertThatConversionOf; + +public class ModuleDependencyTest { + @Test + public void can_be_converted_to_dependencies() { + ArchModules modules = ArchModules.defineByPackages("..test_modules.(*)..") + .modularize(new ClassFileImporter().importPackagesOf(ClassOne.class, ClassTwo.class)); + + ModuleDependency moduleDependency = createDependency(modules, "one", "two"); + + assertThatConversionOf(moduleDependency) + .satisfiesStandardConventions() + .isPossibleTo(Dependency.class); + } + + private static ModuleDependency createDependency(ArchModules modules, String originIdentifier, String targetIdentifier) { + return ModuleDependency.tryCreate( + modules.getByIdentifier(originIdentifier), + modules.getByIdentifier(targetIdentifier) + ).get(); + } +} \ No newline at end of file diff --git a/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/ModuleRuleTest.java b/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/ModuleRuleTest.java index 40d7527185..8dd279187e 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/ModuleRuleTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/ModuleRuleTest.java @@ -1,8 +1,13 @@ package com.tngtech.archunit.library.modules.syntax; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; import java.util.function.Function; import com.tngtech.archunit.base.DescribedFunction; +import com.tngtech.archunit.core.domain.Dependency; +import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.lang.ArchCondition; import com.tngtech.archunit.lang.ConditionEvents; @@ -11,16 +16,25 @@ import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.TestAnnotation; import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.TestAnnotationCustomName; import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.one.ClassOne; +import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.one.one.ClassOneOne; import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.two.ClassTwo; +import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.two.one.ClassTwoOne; +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; import org.junit.Test; +import org.junit.runner.RunWith; import static com.tngtech.archunit.base.DescribedPredicate.alwaysFalse; import static com.tngtech.archunit.base.DescribedPredicate.alwaysTrue; import static com.tngtech.archunit.core.domain.JavaClass.Predicates.equivalentTo; import static com.tngtech.archunit.library.modules.syntax.ModuleDependencyScope.consideringOnlyDependenciesBetweenModules; import static com.tngtech.archunit.library.modules.syntax.ModuleRuleDefinition.modules; +import static com.tngtech.archunit.testutil.Assertions.assertThatDependencies; import static com.tngtech.archunit.testutil.Assertions.assertThatRule; +import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach; +@RunWith(DataProviderRunner.class) public class ModuleRuleTest { @Test @@ -112,6 +126,8 @@ public void ignoring_dependencies_can_be_applied_before_other_methods() { .definedByAnnotation(TestAnnotation.class) .should().respectTheirAllowedDependencies(alwaysFalse(), consideringOnlyDependenciesBetweenModules()) .ignoreDependency(equivalentTo(ClassOne.class), alwaysTrue()) + .ignoreDependency(equivalentTo(ClassOneOne.class), alwaysTrue()) + .ignoreDependency(equivalentTo(ClassTwoOne.class), alwaysTrue()) .because("reason") .as("description") .allowEmptyShould(false) @@ -120,6 +136,28 @@ public void ignoring_dependencies_can_be_applied_before_other_methods() { .hasNoViolation(); } + @DataProvider + public static Object[][] rules() { + return testForEach( + modules().definedByPackages("..test_modules.(*).(*)..").should().respectTheirAllowedDependencies(alwaysFalse(), consideringOnlyDependenciesBetweenModules()), + modules().definedByPackages("..test_modules.(*).(*)..").should().beFreeOfCycles()); + } + + @Test + @UseDataProvider("rules") + public void handles_violations_as_dependencies(ModulesRule rule) { + JavaClasses classes = new ClassFileImporter().importPackagesOf(ClassOne.class, ClassTwo.class); + + Set reportedDependencies = new HashSet<>(); + rule.evaluate(classes).handleViolations( + (Collection dependencies, String __) -> reportedDependencies.addAll(dependencies) + ); + + assertThatDependencies(reportedDependencies) + .contain(ClassOneOne.class, ClassTwoOne.class) + .contain(ClassTwoOne.class, ClassOneOne.class); + } + private static ArchCondition> reportAllAsViolations(Function, String> reportModule) { return new ArchCondition>("report all as violations") { @Override diff --git a/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/testexamples/test_modules/one/one/ClassOneOne.java b/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/testexamples/test_modules/one/one/ClassOneOne.java index beeded1ae2..eeaf289bf2 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/testexamples/test_modules/one/one/ClassOneOne.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/testexamples/test_modules/one/one/ClassOneOne.java @@ -1,5 +1,8 @@ package com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.one.one; +import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.two.one.ClassTwoOne; + @SuppressWarnings("unused") public class ClassOneOne { + ClassTwoOne twoOne; } diff --git a/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/testexamples/test_modules/two/one/ClassTwoOne.java b/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/testexamples/test_modules/two/one/ClassTwoOne.java index d1f9a4a57c..dbbe998d12 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/testexamples/test_modules/two/one/ClassTwoOne.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/testexamples/test_modules/two/one/ClassTwoOne.java @@ -1,5 +1,8 @@ package com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.two.one; +import com.tngtech.archunit.library.modules.syntax.testexamples.test_modules.one.one.ClassOneOne; + @SuppressWarnings("unused") public class ClassTwoOne { + ClassOneOne oneOne; } 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 8cf8b73ecd..2c7cde0807 100644 --- a/archunit/src/test/java/com/tngtech/archunit/testutil/Assertions.java +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/Assertions.java @@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableList; import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.Convertible; import com.tngtech.archunit.core.domain.AccessTarget.FieldAccessTarget; import com.tngtech.archunit.core.domain.Dependency; import com.tngtech.archunit.core.domain.InstanceofCheck; @@ -38,6 +39,7 @@ import com.tngtech.archunit.testutil.assertion.ArchRuleAssertion; import com.tngtech.archunit.testutil.assertion.CodeUnitAccessAssertion; import com.tngtech.archunit.testutil.assertion.ConditionEventsAssertion; +import com.tngtech.archunit.testutil.assertion.ConversionAssertion; import com.tngtech.archunit.testutil.assertion.DependenciesAssertion; import com.tngtech.archunit.testutil.assertion.DependencyAssertion; import com.tngtech.archunit.testutil.assertion.DescribedPredicateAssertion; @@ -171,6 +173,10 @@ public static JavaEnumConstantsAssertion assertThat(JavaEnumConstant[] enumConst return new JavaEnumConstantsAssertion(enumConstants); } + public static ConversionAssertion assertThatConversionOf(Convertible convertible) { + return new ConversionAssertion(convertible); + } + public static JavaClassDescriptorAssertion assertThat(JavaClassDescriptor javaClassDescriptor) { return new JavaClassDescriptorAssertion(javaClassDescriptor); } diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ConversionAssertion.java b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ConversionAssertion.java new file mode 100644 index 0000000000..2e521a9588 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ConversionAssertion.java @@ -0,0 +1,49 @@ +package com.tngtech.archunit.testutil.assertion; + +import java.util.Set; + +import com.tngtech.archunit.core.Convertible; +import org.assertj.core.api.AbstractObjectAssert; + +import static com.google.common.collect.Iterables.getOnlyElement; +import static org.assertj.core.api.Assertions.assertThat; + +public class ConversionAssertion extends AbstractObjectAssert { + public ConversionAssertion(Convertible actual) { + super(actual, ConversionAssertion.class); + } + + public ConversionAssertion isPossibleTo(Class type) { + assertThat(actual.convertTo(type)) + .as(String.format("conversion of %s to compatible type %s", actual.getClass().getName(), type.getName())) + .isNotEmpty(); + return this; + } + + public ConversionAssertion isNotPossibleTo(Class type) { + assertThat(actual.convertTo(type)) + .as(String.format("conversion of %s to incompatible type %s", actual.getClass().getName(), type.getName())) + .isEmpty(); + return this; + } + + public ConversionAssertion isPossibleToSingleElement(Class type, ResultAssertion resultAssertion) { + Set converted = actual.convertTo(type); + assertThat(converted) + .as(String.format("result of converting %s to %s", actual.getClass().getName(), type.getName())) + .hasSize(1); + resultAssertion.assertResult(getOnlyElement(converted)); + return this; + } + + public ConversionAssertion satisfiesStandardConventions() { + class ImpossibleToConvertTo { + } + return isNotPossibleTo(ImpossibleToConvertTo.class) + .isPossibleToSingleElement(Object.class, it -> assertThat(it).isInstanceOf(actual.getClass())); + } + + public interface ResultAssertion { + void assertResult(T result); + } +} \ No newline at end of file diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/DependencyAssertion.java b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/DependencyAssertion.java index 214cb74e82..596a6ff4bb 100644 --- a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/DependencyAssertion.java +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/DependencyAssertion.java @@ -97,6 +97,11 @@ private List> dependencyMatchesTarget(Class targetCl .as("Dependency target matches '%s.class'", targetClass.getSimpleName())); } + public LocationAssertion hasDescription(String description) { + assertThat(actual.getDescription()).isEqualTo(description); + return new LocationAssertion(); + } + public LocationAssertion hasDescription(String originDescription, String dependencyDescription, String targetDescription) { String expectedOriginDependsOnTargetDescription = quote(String.format("<%s> %s <%s>", originDescription, dependencyDescription, targetDescription)); String descriptionPattern = String.format(".+ %s in \\([^ ]+:\\d+\\)", expectedOriginDependsOnTargetDescription);