Improve ArchCondition evaluation

archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/HandlingAssertion.java

@@ -15,7 +15,6 @@
 import com.tngtech.archunit.core.domain.JavaFieldAccess;
 import com.tngtech.archunit.core.domain.JavaMethodCall;
 import com.tngtech.archunit.lang.EvaluationResult;
-import com.tngtech.archunit.lang.ViolationHandler;
 import com.tngtech.archunit.testutils.ExpectedAccess.ExpectedCall;
 import com.tngtech.archunit.testutils.ExpectedAccess.ExpectedFieldAccess;
 
@@ -26,8 +25,6 @@
 import static java.util.Collections.singleton;
 import static java.util.stream.Collectors.toSet;
 
-@SuppressWarnings("Convert2Lambda")
-        // We need the reified type parameter here
 class HandlingAssertion {
     private final Set<ExpectedRelation> expectedFieldAccesses;
     private final Set<ExpectedRelation> expectedMethodCalls;
@@ -81,53 +78,35 @@ Result evaluate(EvaluationResult evaluationResult) {
         return result;
     }
 
-    // Too bad Java erases types, otherwise a lot of boilerplate could be avoided here :-(
-    // This way we must write explicitly ViolationHandler<ConcreteType> or the bound wont work correctly
     private Set<String> evaluateFieldAccesses(EvaluationResult result) {
         Set<String> errorMessages = new HashSet<>();
         final Set<ExpectedRelation> left = new HashSet<>(this.expectedFieldAccesses);
-        result.handleViolations(new ViolationHandler<JavaFieldAccess>() {
-            @Override
-            public void handle(Collection<JavaFieldAccess> violatingObjects, String message) {
-                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left));
-            }
-        });
+        result.handleViolations((Collection<JavaFieldAccess> violatingObjects, String message) ->
+                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left)));
         return union(errorMessages, errorMessagesFrom(left));
     }
 
     private Set<String> evaluateMethodCalls(EvaluationResult result) {
         Set<String> errorMessages = new HashSet<>();
         final Set<ExpectedRelation> left = new HashSet<>(expectedMethodCalls);
-        result.handleViolations(new ViolationHandler<JavaMethodCall>() {
-            @Override
-            public void handle(Collection<JavaMethodCall> violatingObjects, String message) {
-                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left));
-            }
-        });
+        result.handleViolations((Collection<JavaMethodCall> violatingObjects, String message) ->
+                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left)));
         return union(errorMessages, errorMessagesFrom(left));
     }
 
     private Set<String> evaluateConstructorCalls(EvaluationResult result) {
         Set<String> errorMessages = new HashSet<>();
         final Set<ExpectedRelation> left = new HashSet<>(expectedConstructorCalls);
-        result.handleViolations(new ViolationHandler<JavaConstructorCall>() {
-            @Override
-            public void handle(Collection<JavaConstructorCall> violatingObjects, String message) {
-                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left));
-            }
-        });
+        result.handleViolations((Collection<JavaConstructorCall> violatingObjects, String message) ->
+                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left)));
         return union(errorMessages, errorMessagesFrom(left));
     }
 
     private Set<String> evaluateCalls(EvaluationResult result) {
         Set<String> errorMessages = new HashSet<>();
         final Set<ExpectedRelation> left = new HashSet<>(Sets.union(expectedConstructorCalls, expectedMethodCalls));
-        result.handleViolations(new ViolationHandler<JavaCall<?>>() {
-            @Override
-            public void handle(Collection<JavaCall<?>> violatingObjects, String message) {
-                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left));
-            }
-        });
+        result.handleViolations((Collection<JavaCall<?>> violatingObjects, String message) ->
+                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left)));
         return union(errorMessages, errorMessagesFrom(left));
     }
 
@@ -140,24 +119,16 @@ private Set<String> evaluateAccesses(EvaluationResult result) {
                 addAll(expectedFieldAccesses);
             }
         };
-        result.handleViolations(new ViolationHandler<JavaAccess<?>>() {
-            @Override
-            public void handle(Collection<JavaAccess<?>> violatingObjects, String message) {
-                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left));
-            }
-        });
+        result.handleViolations((Collection<JavaAccess<?>> violatingObjects, String message) ->
+                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left)));
         return union(errorMessages, errorMessagesFrom(left));
     }
 
     private Set<String> evaluateDependencies(EvaluationResult result) {
         Set<String> errorMessages = new HashSet<>();
         final Set<ExpectedRelation> left = new HashSet<>(expectedDependencies);
-        result.handleViolations(new ViolationHandler<Dependency>() {
-            @Override
-            public void handle(Collection<Dependency> violatingObjects, String message) {
-                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left));
-            }
-        });
+        result.handleViolations((Collection<Dependency> violatingObjects, String message) ->
+                errorMessages.addAll(removeExpectedAccesses(violatingObjects, left)));
         return union(errorMessages, errorMessagesFrom(left));
     }
 

archunit/src/main/java/com/tngtech/archunit/lang/ArchCondition.java

@@ -16,19 +16,11 @@
 package com.tngtech.archunit.lang;
 
 import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Set;
-import java.util.TreeSet;
 
-import com.google.common.base.Joiner;
-import com.google.common.base.MoreObjects;
-import com.google.common.collect.ImmutableList;
 import com.tngtech.archunit.PublicAPI;
+import com.tngtech.archunit.lang.conditions.ArchConditions;
 
 import static com.tngtech.archunit.PublicAPI.Usage.INHERITANCE;
-import static java.util.stream.Collectors.joining;
-import static java.util.stream.Collectors.toList;
 
 @PublicAPI(usage = INHERITANCE)
 public abstract class ArchCondition<T> {
@@ -60,11 +52,11 @@ public void finish(ConditionEvents events) {
     }
 
     public ArchCondition<T> and(ArchCondition<? super T> condition) {
-        return new AndCondition<>(this, condition.forSubtype());
+        return ArchConditions.and(this, condition.forSubtype());
     }
 
     public ArchCondition<T> or(ArchCondition<? super T> condition) {
-        return new OrCondition<>(this, condition.forSubtype());
+        return ArchConditions.or(this, condition.forSubtype());
     }
 
     public String getDescription() {
@@ -99,189 +91,4 @@ public String toString() {
     public <U extends T> ArchCondition<U> forSubtype() {
         return (ArchCondition<U>) this;
     }
-
-    private abstract static class JoinCondition<T> extends ArchCondition<T> {
-        private final Collection<ArchCondition<T>> conditions;
-
-        private JoinCondition(String infix, Collection<ArchCondition<T>> conditions) {
-            super(joinDescriptionsOf(infix, conditions));
-            this.conditions = conditions;
-        }
-
-        private static <T> String joinDescriptionsOf(String infix, Collection<ArchCondition<T>> conditions) {
-            return conditions.stream().map(ArchCondition::getDescription).collect(joining(" " + infix + " "));
-        }
-
-        @Override
-        public void init(Collection<T> allObjectsToTest) {
-            for (ArchCondition<T> condition : conditions) {
-                condition.init(allObjectsToTest);
-            }
-        }
-
-        @Override
-        public void finish(ConditionEvents events) {
-            for (ArchCondition<T> condition : conditions) {
-                condition.finish(events);
-            }
-        }
-
-        List<ConditionWithEvents<T>> evaluateConditions(T item) {
-            return conditions.stream().map(condition -> new ConditionWithEvents<>(condition, item)).collect(toList());
-        }
-
-        @Override
-        public String toString() {
-            return getClass().getSimpleName() + "{" + conditions + "}";
-        }
-    }
-
-    private static class ConditionWithEvents<T> {
-        private final ArchCondition<T> condition;
-        private final ConditionEvents events;
-
-        ConditionWithEvents(ArchCondition<T> condition, T item) {
-            this(condition, check(condition, item));
-        }
-
-        ConditionWithEvents(ArchCondition<T> condition, ConditionEvents events) {
-            this.condition = condition;
-            this.events = events;
-        }
-
-        private static <T> ConditionEvents check(ArchCondition<T> condition, T item) {
-            ConditionEvents events = new ConditionEvents();
-            condition.check(item, events);
-            return events;
-        }
-
-        @Override
-        public String toString() {
-            return MoreObjects.toStringHelper(this)
-                    .add("condition", condition)
-                    .add("events", events)
-                    .toString();
-        }
-    }
-
-    private abstract static class JoinConditionEvent<T> implements ConditionEvent {
-        final T correspondingObject;
-        final List<ConditionWithEvents<T>> evaluatedConditions;
-
-        JoinConditionEvent(T correspondingObject, List<ConditionWithEvents<T>> evaluatedConditions) {
-            this.correspondingObject = correspondingObject;
-            this.evaluatedConditions = evaluatedConditions;
-        }
-
-        List<String> getUniqueLinesOfViolations() { // TODO: Sort by line number, then lexicographically
-            final Set<String> result = new TreeSet<>();
-            for (ConditionWithEvents<T> evaluation : evaluatedConditions) {
-                for (ConditionEvent event : evaluation.events) {
-                    if (event.isViolation()) {
-                        result.addAll(event.getDescriptionLines());
-                    }
-                }
-            }
-            return ImmutableList.copyOf(result);
-        }
-
-        @Override
-        public String toString() {
-            return MoreObjects.toStringHelper(this)
-                    .add("evaluatedConditions", evaluatedConditions)
-                    .toString();
-        }
-
-        List<ConditionWithEvents<T>> invert(List<ConditionWithEvents<T>> evaluatedConditions) {
-            return evaluatedConditions.stream().map(this::invert).collect(toList());
-        }
-
-        private ConditionWithEvents<T> invert(ConditionWithEvents<T> evaluation) {
-            ConditionEvents invertedEvents = new ConditionEvents();
-            for (ConditionEvent event : evaluation.events) {
-                event.addInvertedTo(invertedEvents);
-            }
-            return new ConditionWithEvents<>(evaluation.condition, invertedEvents);
-        }
-    }
-
-    private static class AndCondition<T> extends JoinCondition<T> {
-        private AndCondition(ArchCondition<T> first, ArchCondition<T> second) {
-            super("and", ImmutableList.of(first, second));
-        }
-
-        @Override
-        public void check(T item, ConditionEvents events) {
-            events.add(new AndConditionEvent<>(item, evaluateConditions(item)));
-        }
-    }
-
-    private static class OrCondition<T> extends JoinCondition<T> {
-        private OrCondition(ArchCondition<T> first, ArchCondition<T> second) {
-            super("or", ImmutableList.of(first, second));
-        }
-
-        @Override
-        public void check(T item, ConditionEvents events) {
-            events.add(new OrConditionEvent<>(item, evaluateConditions(item)));
-        }
-    }
-
-    private static class AndConditionEvent<T> extends JoinConditionEvent<T> {
-        AndConditionEvent(T item, List<ConditionWithEvents<T>> evaluatedConditions) {
-            super(item, evaluatedConditions);
-        }
-
-        @Override
-        public boolean isViolation() {
-            return evaluatedConditions.stream().anyMatch(evaluation -> evaluation.events.containViolation());
-        }
-
-        @Override
-        public void addInvertedTo(ConditionEvents events) {
-            events.add(new OrConditionEvent<>(correspondingObject, invert(evaluatedConditions)));
-        }
-
-        @Override
-        public List<String> getDescriptionLines() {
-            return getUniqueLinesOfViolations();
-        }
-
-        @Override
-        public void handleWith(final Handler handler) {
-            for (ConditionWithEvents<T> condition : evaluatedConditions) {
-                condition.events.handleViolations(handler::handle);
-            }
-        }
-    }
-
-    private static class OrConditionEvent<T> extends JoinConditionEvent<T> {
-        OrConditionEvent(T item, List<ConditionWithEvents<T>> evaluatedConditions) {
-            super(item, evaluatedConditions);
-        }
-
-        @Override
-        public boolean isViolation() {
-            return evaluatedConditions.stream().allMatch(evaluation -> evaluation.events.containViolation());
-        }
-
-        @Override
-        public void addInvertedTo(ConditionEvents events) {
-            events.add(new AndConditionEvent<>(correspondingObject, invert(evaluatedConditions)));
-        }
-
-        @Override
-        public List<String> getDescriptionLines() {
-            return ImmutableList.of(createMessage());
-        }
-
-        private String createMessage() {
-            return Joiner.on(" and ").join(getUniqueLinesOfViolations());
-        }
-
-        @Override
-        public void handleWith(final Handler handler) {
-            handler.handle(Collections.singleton(correspondingObject), createMessage());
-        }
-    }
 }

archunit/src/main/java/com/tngtech/archunit/lang/ArchRule.java

@@ -227,7 +227,7 @@ public EvaluationResult evaluate(JavaClasses classes) {
                 verifyNoEmptyShouldIfEnabled(allObjects);
 
                 condition.init(allObjects);
-                ConditionEvents events = new ConditionEvents();
+                ConditionEvents events = ConditionEvents.Factory.create();
                 for (T object : allObjects) {
                     condition.check(object, events);
                 }

archunit/src/main/java/com/tngtech/archunit/lang/ConditionEvent.java

@@ -25,6 +25,11 @@
 import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
 import static com.tngtech.archunit.PublicAPI.Usage.INHERITANCE;
 
+/**
+ * An event that occurred while checking an {@link ArchCondition}. This can either be a {@link #isViolation() violation}
+ * or be allowed. An event that is allowed will turn into a violation if it is {@link #invert() inverted}
+ * (e.g. for negation of the rule).
+ */
 @PublicAPI(usage = INHERITANCE)
 public interface ConditionEvent {
     /**
@@ -33,15 +38,13 @@ public interface ConditionEvent {
     boolean isViolation();
 
     /**
-     * Adds the 'opposite' of the event. <br>
-     * E.g. <i>The event is a violation, if some conditions A and B are both true?</i>
-     * <br> {@literal ->} <i>The 'inverted' event is a violation if either A or B (or both) are not true</i><br>
-     * In the most simple case, this is just an equivalent event evaluating {@link #isViolation()}
-     * inverted.
-     *
-     * @param events The events to add the 'inverted self' to
+     * @return the 'opposite' of the event. <br>
+     *         Assume e.g. <i>The event is a violation, if some conditions A and B are both true</i>
+     *         <br> {@literal =>} <i>The 'inverted' event is a violation if either A or B (or both) are not true</i><br>
+     *         In the most simple case, this is just an equivalent event evaluating {@link #isViolation()}
+     *         inverted.
      */
-    void addInvertedTo(ConditionEvents events);
+    ConditionEvent invert();
 
     /**
      * @return A textual description of this event as a list of lines