add the possibility to allow empty should per rule #816

This will allow to override the configuration property `archRule.failOnEmptyShould` on a per-rule basis. We overlooked the use case for certain rules that simply expect to have no match when we introduced `failOnEmptyShould`. An example would be rules like `classes().that(areUnwanted()).should().containNumberOfElements(equalTo(0))`. So making this only configurable on a global level renders it useless for a certain set of users. Also, we broke `optionalLayers` in `LayeredArchitecture` and `OnionArchitecture` with the default configuration, since individual access checks are modelled as rules. `LayeredArchitecture` and `OnionArchitecture` already have the concept of `optionalLayers` which is pretty much the same as expressed by `allowEmptyShould`. We now simply translate `allowEmptyShould` to the existing concept, otherwise the two concepts clash (e.g. declare an `optionalLayer` for which the allowed accesses are internally modelled as separate rule, and it still rejects empty classes if `failOnEmptyShould` is set to `true`).

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

@@ -0,0 +1,48 @@
+/*
+ * Copyright 2014-2022 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.lang;
+
+import com.tngtech.archunit.ArchConfiguration;
+
+enum AllowEmptyShould {
+    TRUE {
+        @Override
+        public boolean isAllowed() {
+            return true;
+        }
+    },
+    FALSE {
+        @Override
+        public boolean isAllowed() {
+            return false;
+        }
+    },
+    AS_CONFIGURED {
+        @Override
+        public boolean isAllowed() {
+            return ArchConfiguration.get().getPropertyOrDefault(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME, TRUE.toString())
+                    .equalsIgnoreCase(FALSE.toString());
+        }
+    };
+
+    private static final String FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME = "archRule.failOnEmptyShould";
+
+    abstract boolean isAllowed();
+
+    static AllowEmptyShould fromBoolean(boolean allow) {
+        return allow ? TRUE : FALSE;
+    }
+}

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

@@ -23,7 +23,6 @@
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
-import com.tngtech.archunit.ArchConfiguration;
 import com.tngtech.archunit.Internal;
 import com.tngtech.archunit.PublicAPI;
 import com.tngtech.archunit.base.Optional;
@@ -41,7 +40,6 @@
 import static com.google.common.io.Resources.readLines;
 import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
 import static com.tngtech.archunit.base.ClassLoaders.getCurrentClassLoader;
-import static java.lang.Boolean.TRUE;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 /**
@@ -68,6 +66,18 @@ public interface ArchRule extends CanBeEvaluated, CanOverrideDescription<ArchRul
     @PublicAPI(usage = ACCESS)
     ArchRule because(String reason);
 
+    /**
+     * If set to {@code true} allows the should-clause of this rule to be checked against an empty set of elements.
+     * Otherwise, the rule will fail with a respective message. This is to prevent possible implementation errors,
+     * like filtering for a non-existing package in the that-clause causing an always-passing rule.<br>
+     * Note that this method will override the configuration property {@code archRule.failOnEmptyShould}.
+     *
+     * @param allowEmptyShould Whether the rule fails if the should-clause is evaluated with an empty set of elements
+     * @return A (new) {@link ArchRule} with adjusted {@code allowEmptyShould} behavior
+     */
+    @PublicAPI(usage = ACCESS)
+    ArchRule allowEmptyShould(boolean allowEmptyShould);
+
     @PublicAPI(usage = ACCESS)
     final class Assertions {
         private static final ArchUnitExtensions extensions = new ArchUnitExtensions();
@@ -166,7 +176,7 @@ public EvaluationResult getResult() {
     @Internal
     class Factory {
         public static <T> ArchRule create(final ClassesTransformer<T> classesTransformer, final ArchCondition<T> condition, final Priority priority) {
-            return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.<String>empty());
+            return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.<String>empty(), AllowEmptyShould.AS_CONFIGURED);
         }
 
         public static ArchRule withBecause(ArchRule rule, String reason) {
@@ -184,18 +194,20 @@ private static class SimpleArchRule<T> implements ArchRule {
             private final ClassesTransformer<T> classesTransformer;
             private final ArchCondition<T> condition;
             private final Optional<String> overriddenDescription;
+            private final AllowEmptyShould allowEmptyShould;
 
             private SimpleArchRule(Priority priority, ClassesTransformer<T> classesTransformer, ArchCondition<T> condition,
-                    Optional<String> overriddenDescription) {
+                    Optional<String> overriddenDescription, AllowEmptyShould allowEmptyShould) {
                 this.priority = priority;
                 this.classesTransformer = classesTransformer;
                 this.condition = condition;
                 this.overriddenDescription = overriddenDescription;
+                this.allowEmptyShould = allowEmptyShould;
             }
 
             @Override
             public ArchRule as(String newDescription) {
-                return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.of(newDescription));
+                return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.of(newDescription), allowEmptyShould);
             }
 
             @Override
@@ -208,6 +220,11 @@ public ArchRule because(String reason) {
                 return withBecause(this, reason);
             }
 
+            @Override
+            public ArchRule allowEmptyShould(boolean allowEmptyShould) {
+                return new SimpleArchRule<>(priority, classesTransformer, condition, overriddenDescription, AllowEmptyShould.fromBoolean(allowEmptyShould));
+            }
+
             @Override
             public EvaluationResult evaluate(JavaClasses classes) {
                 Iterable<T> allObjects = classesTransformer.transform(classes);
@@ -223,20 +240,18 @@ public EvaluationResult evaluate(JavaClasses classes) {
             }
 
             private void verifyNoEmptyShouldIfEnabled(Iterable<T> allObjects) {
-                if (isEmpty(allObjects) && isFailOnEmptyShouldEnabled()) {
-                    throw new AssertionError("Rule failed to check any classes. " +
-                            "This means either that no classes have been passed to the rule at all, " +
-                            "or that no classes passed to the rule matched the `that()` clause. " +
-                            "To allow rules being evaluated without checking any classes you can set the ArchUnit property " +
-                            FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME + " = " + false);
+                if (isEmpty(allObjects) && !allowEmptyShould.isAllowed()) {
+                    throw new AssertionError(String.format(
+                            "Rule '%s' failed to check any classes. "
+                                    + "This means either that no classes have been passed to the rule at all, "
+                                    + "or that no classes passed to the rule matched the `that()` clause. "
+                                    + "To allow rules being evaluated without checking any classes you can either "
+                                    + "use `%s.allowEmptyShould(true)` on a single rule or set the configuration property `%s = false` "
+                                    + "to change the behavior globally.",
+                            getDescription(), ArchRule.class.getSimpleName(), FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME));
                 }
             }
 
-            private boolean isFailOnEmptyShouldEnabled() {
-                return ArchConfiguration.get().getPropertyOrDefault(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME, TRUE.toString())
-                        .equals(TRUE.toString());
-            }
-
             @Override
             public String getDescription() {
                 return overriddenDescription.isPresent() ?

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

@@ -82,6 +82,16 @@ public CompositeArchRule because(String reason) {
         return new CompositeArchRule(priority, rules, createBecauseDescription(this, reason));
     }
 
+    @Override
+    @PublicAPI(usage = ACCESS)
+    public ArchRule allowEmptyShould(boolean allowEmptyShould) {
+        ImmutableList.Builder<ArchRule> rulesWithOverriddenAllowEmptyShould = ImmutableList.builder();
+        for (ArchRule rule : rules) {
+            rulesWithOverriddenAllowEmptyShould.add(rule.allowEmptyShould(allowEmptyShould));
+        }
+        return new CompositeArchRule(priority, rulesWithOverriddenAllowEmptyShould.build(), description);
+    }
+
     @Override
     @PublicAPI(usage = ACCESS)
     public EvaluationResult evaluate(JavaClasses classes) {

archunit/src/main/java/com/tngtech/archunit/lang/syntax/ObjectsShouldInternal.java

@@ -86,6 +86,11 @@ public ArchRule because(String reason) {
         return ArchRule.Factory.withBecause(this, reason);
     }
 
+    @Override
+    public ArchRule allowEmptyShould(boolean allowEmptyShould) {
+        return finishedRule.get().allowEmptyShould(allowEmptyShould);
+    }
+
     @Override
     public ArchRule as(String newDescription) {
         return finishedRule.get().as(newDescription);

archunit/src/main/java/com/tngtech/archunit/library/Architectures.java

@@ -109,7 +109,7 @@ public static final class LayeredArchitecture implements ArchRule {
         private final Set<LayerDependencySpecification> dependencySpecifications;
         private final PredicateAggregator<Dependency> irrelevantDependenciesPredicate;
         private final Optional<String> overriddenDescription;
-        private boolean optionalLayers;
+        private final boolean optionalLayers;
 
         private LayeredArchitecture() {
             this(new LayerDefinitions(),
@@ -140,8 +140,7 @@ private LayeredArchitecture(LayerDefinitions layerDefinitions,
          */
         @PublicAPI(usage = ACCESS)
         public LayeredArchitecture withOptionalLayers(boolean optionalLayers) {
-            this.optionalLayers = optionalLayers;
-            return this;
+            return new LayeredArchitecture(layerDefinitions, dependencySpecifications, irrelevantDependenciesPredicate, overriddenDescription, optionalLayers);
         }
 
         private LayeredArchitecture addLayerDefinition(LayerDefinition definition) {
@@ -223,6 +222,8 @@ private void checkEmptyLayers(JavaClasses classes, EvaluationResult result) {
         private EvaluationResult evaluateLayersShouldNotBeEmpty(JavaClasses classes, LayerDefinition layerDefinition) {
             return classes().that(layerDefinitions.containsPredicateFor(layerDefinition.name))
                     .should(notBeEmptyFor(layerDefinition))
+                    // we need to set `allowEmptyShould(true)` to allow the layer not empty check to be evaluated. This will provide a nicer error message.
+                    .allowEmptyShould(true)
                     .evaluate(classes);
         }
 
@@ -232,6 +233,7 @@ private EvaluationResult evaluateDependenciesShouldBeSatisfied(JavaClasses class
                     : onlyHaveDependenciesWhere(targetMatchesIfDependencyIsRelevant(specification.layerName, specification.allowedLayers));
             return classes().that(layerDefinitions.containsPredicateFor(specification.layerName))
                     .should(satisfyLayerDependenciesCondition)
+                    .allowEmptyShould(true)
                     .evaluate(classes);
         }
 
@@ -257,32 +259,6 @@ private DescribedPredicate<Dependency> ifDependencyIsRelevant(DescribedPredicate
                     originPackageMatches;
         }
 
-        private static ArchCondition<JavaClass> notBeEmptyFor(final LayeredArchitecture.LayerDefinition layerDefinition) {
-            return new LayerShouldNotBeEmptyCondition(layerDefinition);
-        }
-
-        private static class LayerShouldNotBeEmptyCondition extends ArchCondition<JavaClass> {
-            private final LayeredArchitecture.LayerDefinition layerDefinition;
-            private boolean empty = true;
-
-            LayerShouldNotBeEmptyCondition(final LayeredArchitecture.LayerDefinition layerDefinition) {
-                super("not be empty");
-                this.layerDefinition = layerDefinition;
-            }
-
-            @Override
-            public void check(JavaClass item, ConditionEvents events) {
-                empty = false;
-            }
-
-            @Override
-            public void finish(ConditionEvents events) {
-                if (empty) {
-                    events.add(violated(layerDefinition, String.format("Layer '%s' is empty", layerDefinition.name)));
-                }
-            }
-        }
-
         @Override
         @PublicAPI(usage = ACCESS)
         public void check(JavaClasses classes) {
@@ -295,6 +271,15 @@ public ArchRule because(String reason) {
             return ArchRule.Factory.withBecause(this, reason);
         }
 
+        /**
+         * This method is equivalent to calling {@link #withOptionalLayers(boolean)}, which should be preferred in this context
+         * as the meaning is easier to understand.
+         */
+        @Override
+        public ArchRule allowEmptyShould(boolean allowEmptyShould) {
+            return withOptionalLayers(allowEmptyShould);
+        }
+
         @Override
         @PublicAPI(usage = ACCESS)
         public LayeredArchitecture as(String newDescription) {
@@ -353,6 +338,32 @@ private void checkLayerNamesExist(String... layerNames) {
             }
         }
 
+        private static ArchCondition<JavaClass> notBeEmptyFor(final LayeredArchitecture.LayerDefinition layerDefinition) {
+            return new LayerShouldNotBeEmptyCondition(layerDefinition);
+        }
+
+        private static class LayerShouldNotBeEmptyCondition extends ArchCondition<JavaClass> {
+            private final LayeredArchitecture.LayerDefinition layerDefinition;
+            private boolean empty = true;
+
+            LayerShouldNotBeEmptyCondition(final LayeredArchitecture.LayerDefinition layerDefinition) {
+                super("not be empty");
+                this.layerDefinition = layerDefinition;
+            }
+
+            @Override
+            public void check(JavaClass item, ConditionEvents events) {
+                empty = false;
+            }
+
+            @Override
+            public void finish(ConditionEvents events) {
+                if (empty) {
+                    events.add(violated(layerDefinition, String.format("Layer '%s' is empty", layerDefinition.name)));
+                }
+            }
+        }
+
         private static final class LayerDefinitions implements Iterable<LayerDefinition> {
             private final Map<String, LayerDefinition> layerDefinitions = new LinkedHashMap<>();
 
@@ -571,6 +582,10 @@ public OnionArchitecture adapter(String name, String... packageIdentifiers) {
             return this;
         }
 
+        /**
+         * @param optionalLayers Whether the different parts of the Onion Architecture (domain models, domain services, ...) should be allowed to be empty.
+         *                       If set to {@code false} the {@link OnionArchitecture OnionArchitecture} will fail if any such layer does not contain any class.
+         */
         @PublicAPI(usage = ACCESS)
         public OnionArchitecture withOptionalLayers(boolean optionalLayers) {
             this.optionalLayers = optionalLayers;
@@ -638,6 +653,15 @@ public ArchRule because(String reason) {
             return ArchRule.Factory.withBecause(this, reason);
         }
 
+        /**
+         * This method is equivalent to calling {@link #withOptionalLayers(boolean)}, which should be preferred in this context
+         * as the meaning is easier to understand.
+         */
+        @Override
+        public ArchRule allowEmptyShould(boolean allowEmptyShould) {
+            return withOptionalLayers(allowEmptyShould);
+        }
+
         @Override
         public OnionArchitecture as(String newDescription) {
             return new OnionArchitecture(domainModelPackageIdentifiers, domainServicePackageIdentifiers,