diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/AllowEmptyShould.java b/archunit/src/main/java/com/tngtech/archunit/lang/AllowEmptyShould.java new file mode 100644 index 0000000000..e523d4e52b --- /dev/null +++ b/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; + } +} diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/ArchRule.java b/archunit/src/main/java/com/tngtech/archunit/lang/ArchRule.java index 849bf60018..ade2990131 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/ArchRule.java +++ b/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 + * 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 ArchRule create(final ClassesTransformer classesTransformer, final ArchCondition condition, final Priority priority) { - return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.empty()); + return new SimpleArchRule<>(priority, classesTransformer, condition, Optional.empty(), AllowEmptyShould.AS_CONFIGURED); } public static ArchRule withBecause(ArchRule rule, String reason) { @@ -184,18 +194,20 @@ private static class SimpleArchRule implements ArchRule { private final ClassesTransformer classesTransformer; private final ArchCondition condition; private final Optional overriddenDescription; + private final AllowEmptyShould allowEmptyShould; private SimpleArchRule(Priority priority, ClassesTransformer classesTransformer, ArchCondition condition, - Optional overriddenDescription) { + Optional 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 allObjects = classesTransformer.transform(classes); @@ -223,20 +240,18 @@ public EvaluationResult evaluate(JavaClasses classes) { } private void verifyNoEmptyShouldIfEnabled(Iterable 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() ? diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/CompositeArchRule.java b/archunit/src/main/java/com/tngtech/archunit/lang/CompositeArchRule.java index 5a48a15e41..56395b26cc 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/CompositeArchRule.java +++ b/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 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) { diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/ObjectsShouldInternal.java b/archunit/src/main/java/com/tngtech/archunit/lang/syntax/ObjectsShouldInternal.java index f449c04296..57fb96df98 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/syntax/ObjectsShouldInternal.java +++ b/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); 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 3bfbf6ceb2..d430efb02a 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java @@ -109,7 +109,7 @@ public static final class LayeredArchitecture implements ArchRule { private final Set dependencySpecifications; private final PredicateAggregator irrelevantDependenciesPredicate; private final Optional 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 ifDependencyIsRelevant(DescribedPredicate originPackageMatches; } - private static ArchCondition notBeEmptyFor(final LayeredArchitecture.LayerDefinition layerDefinition) { - return new LayerShouldNotBeEmptyCondition(layerDefinition); - } - - private static class LayerShouldNotBeEmptyCondition extends ArchCondition { - 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 notBeEmptyFor(final LayeredArchitecture.LayerDefinition layerDefinition) { + return new LayerShouldNotBeEmptyCondition(layerDefinition); + } + + private static class LayerShouldNotBeEmptyCondition extends ArchCondition { + 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 { private final Map 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, diff --git a/archunit/src/main/java/com/tngtech/archunit/library/dependencies/SliceRule.java b/archunit/src/main/java/com/tngtech/archunit/library/dependencies/SliceRule.java index 91f5c714f7..d35a55351f 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/dependencies/SliceRule.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/dependencies/SliceRule.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import com.tngtech.archunit.PublicAPI; import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.base.Optional; import com.tngtech.archunit.core.domain.Dependency; import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; @@ -44,18 +45,32 @@ public final class SliceRule implements ArchRule { private final List transformations; private final DescribedPredicate ignoreDependency; private final ConditionFactory conditionFactory; + private final Optional allowEmptyShould; SliceRule(Slices.Transformer inputTransformer, Priority priority, ConditionFactory conditionFactory) { - this(inputTransformer, priority, Collections.emptyList(), DescribedPredicate.alwaysFalse(), conditionFactory); + this( + inputTransformer, + priority, + Collections.emptyList(), + DescribedPredicate.alwaysFalse(), + conditionFactory, + Optional.empty()); } - private SliceRule(Slices.Transformer inputTransformer, Priority priority, List transformations, - DescribedPredicate ignoreDependency, ConditionFactory conditionFactory) { + private SliceRule( + Slices.Transformer inputTransformer, + Priority priority, + List transformations, + DescribedPredicate ignoreDependency, + ConditionFactory conditionFactory, + Optional allowEmptyShould + ) { this.inputTransformer = inputTransformer; this.priority = priority; this.transformations = transformations; this.ignoreDependency = ignoreDependency; this.conditionFactory = conditionFactory; + this.allowEmptyShould = allowEmptyShould; } @Override @@ -70,6 +85,12 @@ public SliceRule because(String reason) { return copyWithTransformation(new Because(reason)); } + @Override + @PublicAPI(usage = ACCESS) + public ArchRule allowEmptyShould(boolean allowEmptyShould) { + return new SliceRule(inputTransformer, priority, transformations, ignoreDependency, conditionFactory, Optional.of(allowEmptyShould)); + } + @Override @PublicAPI(usage = ACCESS) public EvaluationResult evaluate(JavaClasses classes) { @@ -100,17 +121,22 @@ public SliceRule ignoreDependency(String origin, String target) { @PublicAPI(usage = ACCESS) public SliceRule ignoreDependency(DescribedPredicate origin, DescribedPredicate target) { - return new SliceRule(inputTransformer, priority, transformations, ignoreDependency.or(dependency(origin, target)), conditionFactory); + return new SliceRule(inputTransformer, priority, transformations, ignoreDependency.or(dependency(origin, target)), conditionFactory, allowEmptyShould); } private SliceRule copyWithTransformation(Transformation transformation) { List newTransformations = ImmutableList.builder().addAll(transformations).add(transformation).build(); - return new SliceRule(inputTransformer, priority, newTransformations, ignoreDependency, conditionFactory); + return new SliceRule(inputTransformer, priority, newTransformations, ignoreDependency, conditionFactory, allowEmptyShould); } private ArchRule getArchRule() { - ArchRule rule = priority(priority).all(inputTransformer).should(conditionFactory.create(inputTransformer, not(ignoreDependency))); + ArchRule rule = priority(priority) + .all(inputTransformer) + .should(conditionFactory.create(inputTransformer, not(ignoreDependency))); + if (allowEmptyShould.isPresent()) { + rule = rule.allowEmptyShould(allowEmptyShould.get()); + } for (Transformation transformation : transformations) { rule = transformation.apply(rule); } diff --git a/archunit/src/main/java/com/tngtech/archunit/library/freeze/FreezingArchRule.java b/archunit/src/main/java/com/tngtech/archunit/library/freeze/FreezingArchRule.java index 609ba4649f..ad382137fb 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/freeze/FreezingArchRule.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/freeze/FreezingArchRule.java @@ -102,6 +102,12 @@ public FreezingArchRule because(String reason) { return new FreezingArchRule(delegate.because(reason), store, matcher); } + @Override + @PublicAPI(usage = ACCESS) + public ArchRule allowEmptyShould(boolean allowEmptyShould) { + return new FreezingArchRule(delegate.allowEmptyShould(allowEmptyShould), store, matcher); + } + @Override @PublicAPI(usage = ACCESS) public FreezingArchRule as(String newDescription) { diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/ArchRuleTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/ArchRuleTest.java index 3d5ce88966..05bb37c0f0 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/ArchRuleTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/ArchRuleTest.java @@ -10,7 +10,7 @@ import com.google.common.base.Joiner; import com.google.common.base.Splitter; import com.google.common.io.Files; -import com.tngtech.archunit.ArchConfiguration; +import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.domain.JavaClassesTest; @@ -33,13 +33,12 @@ import static com.tngtech.archunit.lang.Priority.HIGH; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.all; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; +import static com.tngtech.archunit.testutil.ArchConfigurationRule.FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME; import static com.tngtech.archunit.testutil.TestUtils.toUri; -import static java.lang.Boolean.FALSE; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; public class ArchRuleTest { - private static final String FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME = "archRule.failOnEmptyShould"; @Rule public final ExpectedException thrown = ExpectedException.none(); @@ -169,32 +168,47 @@ public void finish(ConditionEvents events) { } @Test - public void evaluation_fails_because_of_empty_set_of_classes_with_default_fail_on_empty_should() { - archConfigurationRule.removeProperty(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME); - + public void evaluation_fails_because_of_empty_set_of_elements_with_default_fail_on_empty_should() { thrown.expect(AssertionError.class); - thrown.expectMessage("Rule failed to check any classes"); + thrown.expectMessage("failed to check any classes"); thrown.expectMessage(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME); - classes().should(ALWAYS_BE_VALID).evaluate(importClasses()); + createPassingArchRule().evaluate(importEmptyClasses()); } @Test - public void evaluation_fails_because_of_empty_set_of_classes_after_filter_with_default_fail_on_empty_should() { - archConfigurationRule.removeProperty(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME); - + public void evaluation_fails_because_of_empty_set_of_elements_after_that_clause_with_default_fail_on_empty_should() { thrown.expect(AssertionError.class); - thrown.expectMessage("Rule failed to check any classes"); + thrown.expectMessage("failed to check any classes"); thrown.expectMessage(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME); - classes().that().doNotHaveSimpleName(SomeClass.class.getSimpleName()).should(ALWAYS_BE_VALID).evaluate(importClasses(SomeClass.class)); + createPassingArchRule(strings().that(DescribedPredicate.alwaysFalse())).evaluate(importClasses(SomeClass.class)); } @Test - public void evaluation_passes_on_empty_set_of_classes_with_deactivated_fail_on_empty_should() { - ArchConfiguration.get().setProperty(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME, FALSE.toString()); + public void evaluation_passes_on_empty_set_of_elements_with_deactivated_fail_on_empty_should_by_configuration() { + archConfigurationRule.setFailOnEmptyShould(false); - classes().should(ALWAYS_BE_VALID).evaluate(importClasses()); + createPassingArchRule().evaluate(importEmptyClasses()); + } + + @Test + public void evaluation_passes_on_empty_set_of_elements_with_activated_fail_on_empty_should_by_configuration_but_overridden_by_rule() { + archConfigurationRule.setFailOnEmptyShould(true); + + createPassingArchRule().allowEmptyShould(true).evaluate(importEmptyClasses()); + } + + private JavaClasses importEmptyClasses() { + return importClasses(); + } + + private ArchRule createPassingArchRule() { + return createPassingArchRule(strings()); + } + + private ArchRule createPassingArchRule(ClassesTransformer classesTransformer) { + return ArchRule.Factory.create(classesTransformer, ALWAYS_BE_VALID.forSubtype(), Priority.MEDIUM); } private ClassesTransformer strings() { @@ -288,10 +302,10 @@ public void check(JavaClass item, ConditionEvents events) { } }; - private static final ArchCondition ALWAYS_BE_VALID = - new ArchCondition("always be valid") { + private static final ArchCondition ALWAYS_BE_VALID = + new ArchCondition("always be valid") { @Override - public void check(JavaClass item, ConditionEvents events) { + public void check(Object item, ConditionEvents events) { } }; diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/CompositeArchRuleTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/CompositeArchRuleTest.java index 849c08dad5..68ca4d2d94 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/CompositeArchRuleTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/CompositeArchRuleTest.java @@ -1,16 +1,21 @@ package com.tngtech.archunit.lang; +import java.util.List; + import com.google.common.collect.ImmutableList; +import com.tngtech.archunit.base.DescribedPredicate; 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.testutil.ArchConfigurationRule; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import java.util.List; - import static com.tngtech.archunit.core.domain.TestUtils.importClasses; import static com.tngtech.archunit.lang.Priority.HIGH; import static com.tngtech.archunit.lang.Priority.MEDIUM; @@ -18,12 +23,16 @@ import static com.tngtech.java.junit.dataprovider.DataProviders.$; import static com.tngtech.java.junit.dataprovider.DataProviders.$$; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; @RunWith(DataProviderRunner.class) public class CompositeArchRuleTest { private static final boolean SATISFIED = true; private static final boolean UNSATISFIED = false; + @Rule + public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule(); + @DataProvider public static Object[][] rules_to_AND() { return $$( @@ -83,6 +92,37 @@ public void priority_is_passed() { assertPriority(failureMessage, priority); } + @Test + public void fails_on_empty_should_by_default() { + assertThatThrownBy(new ThrowingCallable() { + @Override + public void call() { + compositeRuleWithPartialEmptyShould().check(new ClassFileImporter().importClasses(Object.class)); + } + }).isInstanceOf(AssertionError.class) + .hasMessageContaining("failed to check any classes"); + } + + @Test + public void should_allow_empty_should_if_configured() { + archConfigurationRule.setFailOnEmptyShould(false); + + compositeRuleWithPartialEmptyShould().check(new ClassFileImporter().importClasses(Object.class)); + } + + @Test + public void allows_empty_should_if_overridden_by_rule() { + archConfigurationRule.setFailOnEmptyShould(true); + + compositeRuleWithPartialEmptyShould().allowEmptyShould(true).check(new ClassFileImporter().importClasses(Object.class)); + } + + private static CompositeArchRule compositeRuleWithPartialEmptyShould() { + return CompositeArchRule + .of(classes().should().bePublic()) + .and(classes().that(DescribedPredicate.alwaysFalse()).should().bePublic()); + } + private void assertPriority(String failureMessage, Priority priority) { assertThat(failureMessage).contains(String.format("[Priority: %s]", priority)); } @@ -108,4 +148,4 @@ public void check(JavaClass item, ConditionEvents events) { } }, Priority.MEDIUM).as(String.format("%s rule", satisfied ? "satisfied" : "failing")); } -} \ No newline at end of file +} diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/ClassesShouldTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/ClassesShouldTest.java index 427e2de7c5..439f564474 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/ClassesShouldTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/ClassesShouldTest.java @@ -19,12 +19,14 @@ import com.tngtech.archunit.core.domain.JavaAccess; import com.tngtech.archunit.core.domain.JavaAnnotation; import com.tngtech.archunit.core.domain.JavaCall; +import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaModifier; import com.tngtech.archunit.core.domain.properties.CanBeAnnotatedTest; import com.tngtech.archunit.core.domain.properties.CanBeAnnotatedTest.ClassRetentionAnnotation; import com.tngtech.archunit.core.domain.properties.CanBeAnnotatedTest.DefaultClassRetentionAnnotation; import com.tngtech.archunit.core.domain.properties.CanBeAnnotatedTest.RuntimeRetentionAnnotation; import com.tngtech.archunit.core.domain.properties.CanBeAnnotatedTest.SourceRetentionAnnotation; +import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.lang.ArchCondition; import com.tngtech.archunit.lang.ArchRule; import com.tngtech.archunit.lang.EvaluationResult; @@ -35,6 +37,7 @@ import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -72,6 +75,7 @@ import static com.tngtech.java.junit.dataprovider.DataProviders.$$; import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach; import static java.util.regex.Pattern.quote; +import static org.assertj.core.api.Assertions.assertThatThrownBy; @RunWith(DataProviderRunner.class) public class ClassesShouldTest { @@ -1767,6 +1771,35 @@ public void onlyCall_should_report_success_if_targets_are_non_resolvable(ArchRul assertThatRule(rule).checking(importClasses(classCallingUnresolvableTarget)).hasNoViolation(); } + @Test + public void should_fail_on_empty_should_by_default() { + assertThatThrownBy(new ThrowingCallable() { + @Override + public void call() { + ruleWithEmptyShould().check(new ClassFileImporter().importClasses(getClass())); + } + }).isInstanceOf(AssertionError.class) + .hasMessageContaining("failed to check any classes"); + } + + @Test + public void should_allow_empty_should_if_configured() { + configurationRule.setFailOnEmptyShould(false); + + ruleWithEmptyShould().check(new ClassFileImporter().importClasses(getClass())); + } + + @Test + public void should_allow_empty_should_if_overridden_by_rule() { + configurationRule.setFailOnEmptyShould(true); + + ruleWithEmptyShould().allowEmptyShould(true).check(new ClassFileImporter().importClasses(getClass())); + } + + private static ArchRule ruleWithEmptyShould() { + return classes().that(DescribedPredicate.alwaysFalse()).should().bePublic(); + } + static String locationPattern(Class clazz) { return String.format("\\(%s.java:\\d+\\)", quote(clazz.getSimpleName())); } diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenClassesThatTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenClassesThatTest.java index 68891ccd5e..93536e8584 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenClassesThatTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenClassesThatTest.java @@ -26,6 +26,7 @@ import com.tngtech.archunit.core.domain.properties.HasType; import com.tngtech.archunit.lang.ArchCondition; import com.tngtech.archunit.lang.ConditionEvents; +import com.tngtech.archunit.testutil.ArchConfigurationRule; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import org.junit.Rule; import org.junit.Test; @@ -56,6 +57,9 @@ public class GivenClassesThatTest { @Rule public final ExpectedException thrown = ExpectedException.none(); + @Rule + public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule(); + @Test public void haveFullyQualifiedName() { List classes = filterResultOf(classes().that().haveFullyQualifiedName(List.class.getName())) @@ -474,6 +478,8 @@ public void implement_type() { assertThatType(getOnlyElement(classes)).matches(ArrayList.class); + archConfigurationRule.setFailOnEmptyShould(false); + classes = filterResultOf(classes().that().implement(Set.class)) .on(ArrayList.class, List.class, Iterable.class); @@ -511,6 +517,8 @@ public void implement_typeName() { assertThatType(getOnlyElement(classes)).matches(ArrayList.class); + archConfigurationRule.setFailOnEmptyShould(false); + classes = filterResultOf(classes().that().implement(AbstractList.class.getName())) .on(ArrayList.class, List.class, Iterable.class); @@ -532,6 +540,8 @@ public void implement_predicate() { assertThatType(getOnlyElement(classes)).matches(ArrayList.class); + archConfigurationRule.setFailOnEmptyShould(false); + classes = filterResultOf(classes().that().implement(classWithNameOf(AbstractList.class))) .on(ArrayList.class, List.class, Iterable.class); @@ -890,6 +900,8 @@ public void or_conjunction() { */ @Test public void conjunctions_aggregate_in_sequence_without_special_precedence() { + archConfigurationRule.setFailOnEmptyShould(false); + List classes = filterResultOf( // (List OR String) AND Collection => empty classes().that().haveSimpleName(List.class.getSimpleName()) diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenCodeUnitsTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenCodeUnitsTest.java index f56c8bee38..73d83e14a7 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenCodeUnitsTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenCodeUnitsTest.java @@ -17,9 +17,11 @@ import com.tngtech.archunit.lang.EvaluationResult; import com.tngtech.archunit.lang.SimpleConditionEvent; import com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.DescribedRuleStart; +import com.tngtech.archunit.testutil.ArchConfigurationRule; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,6 +47,9 @@ @RunWith(DataProviderRunner.class) public class GivenCodeUnitsTest { + @Rule + public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule(); + @Test public void complex_code_unit_syntax() { EvaluationResult result = codeUnits() @@ -209,6 +214,8 @@ public static Object[][] restricted_return_type_rule_starts() { @Test @UseDataProvider("restricted_return_type_rule_starts") public void return_type_predicates(DescribedRuleStart ruleStart, Collection expectedMembers) { + archConfigurationRule.setFailOnEmptyShould(false); + EvaluationResult result = ruleStart.should(everythingViolationPrintMemberName()) .evaluate(importClasses(ClassWithVariousMembers.class)); diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersDeclaredInClassesThatTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersDeclaredInClassesThatTest.java index 4147e7ec2d..1fc413a4e2 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersDeclaredInClassesThatTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersDeclaredInClassesThatTest.java @@ -23,6 +23,7 @@ import com.tngtech.archunit.core.domain.properties.HasName; import com.tngtech.archunit.core.domain.properties.HasType; import com.tngtech.archunit.lang.syntax.elements.GivenClassesThatTest.Evaluator; +import com.tngtech.archunit.testutil.ArchConfigurationRule; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import org.junit.Rule; import org.junit.Test; @@ -51,6 +52,9 @@ public class GivenMembersDeclaredInClassesThatTest { @Rule public final ExpectedException thrown = ExpectedException.none(); + @Rule + public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule(); + @Test public void haveFullyQualifiedName() { List members = filterResultOf(members().that().areDeclaredInClassesThat().haveFullyQualifiedName(List.class.getName())) @@ -420,7 +424,7 @@ public void areMetaAnnotatedWith_type() { List members = filterResultOf(members().that().areDeclaredInClassesThat().areMetaAnnotatedWith(SomeAnnotation.class)) .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class); - assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class); + assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class, MetaAnnotatedAnnotation.class); } @Test @@ -428,7 +432,7 @@ public void areNotMetaAnnotatedWith_type() { List members = filterResultOf(members().that().areDeclaredInClassesThat().areNotMetaAnnotatedWith(SomeAnnotation.class)) .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class); - assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class, MetaAnnotatedAnnotation.class); + assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class); } @Test @@ -436,7 +440,7 @@ public void areMetaAnnotatedWith_typeName() { List members = filterResultOf(members().that().areDeclaredInClassesThat().areMetaAnnotatedWith(SomeAnnotation.class.getName())) .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class); - assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class); + assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class, MetaAnnotatedAnnotation.class); } @Test @@ -444,7 +448,7 @@ public void areNotMetaAnnotatedWith_typeName() { List members = filterResultOf(members().that().areDeclaredInClassesThat().areNotMetaAnnotatedWith(SomeAnnotation.class.getName())) .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class); - assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class, MetaAnnotatedAnnotation.class); + assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class); } @Test @@ -453,7 +457,7 @@ public void areMetaAnnotatedWith_predicate() { List members = filterResultOf(members().that().areDeclaredInClassesThat().areMetaAnnotatedWith(hasNamePredicate)) .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class); - assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class); + assertThatMembers(members).matchInAnyOrderMembersOf(MetaAnnotatedClass.class, AnnotatedClass.class, MetaAnnotatedAnnotation.class); } @Test @@ -462,7 +466,7 @@ public void areNotMetaAnnotatedWith_predicate() { List members = filterResultOf(members().that().areDeclaredInClassesThat().areNotMetaAnnotatedWith(hasNamePredicate)) .on(MetaAnnotatedClass.class, AnnotatedClass.class, SimpleClass.class, MetaAnnotatedAnnotation.class); - assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class, MetaAnnotatedAnnotation.class); + assertThatMembers(members).matchInAnyOrderMembersOf(SimpleClass.class); } @Test @@ -472,6 +476,8 @@ public void implement_type() { assertThatMembers(members).matchInAnyOrderMembersOf(ArrayList.class); + archConfigurationRule.setFailOnEmptyShould(false); + members = filterResultOf(members().that().areDeclaredInClassesThat().implement(Set.class)) .on(ArrayList.class, List.class, Iterable.class); @@ -509,6 +515,8 @@ public void implement_typeName() { assertThatMembers(members).matchInAnyOrderMembersOf(ArrayList.class); + archConfigurationRule.setFailOnEmptyShould(false); + members = filterResultOf(members().that().areDeclaredInClassesThat().implement(AbstractList.class.getName())) .on(ArrayList.class, List.class, Iterable.class); @@ -530,6 +538,8 @@ public void implement_predicate() { assertThatMembers(members).matchInAnyOrderMembersOf(ArrayList.class); + archConfigurationRule.setFailOnEmptyShould(false); + members = filterResultOf(members().that().areDeclaredInClassesThat().implement(classWithNameOf(AbstractList.class))) .on(ArrayList.class, List.class, Iterable.class); @@ -690,15 +700,15 @@ public void areNotEnums_predicate() { @Test public void areAnnotations_predicate() { List members = filterResultOf(members().that().areDeclaredInClassesThat().areAnnotations()) - .on(Deprecated.class, Collection.class, SafeVarargs.class, Integer.class); + .on(SomeAnnotation.class, Collection.class, MetaAnnotatedAnnotation.class, Integer.class); - assertThatMembers(members).matchInAnyOrderMembersOf(Deprecated.class, SafeVarargs.class); + assertThatMembers(members).matchInAnyOrderMembersOf(SomeAnnotation.class, MetaAnnotatedAnnotation.class); } @Test public void areNotAnnotations_predicate() { List members = filterResultOf(members().that().areDeclaredInClassesThat().areNotAnnotations()) - .on(Deprecated.class, Collection.class, SafeVarargs.class, Integer.class); + .on(SomeAnnotation.class, Collection.class, MetaAnnotatedAnnotation.class, Integer.class); assertThatMembers(members).matchInAnyOrderMembersOf(Collection.class, Integer.class); } @@ -899,6 +909,8 @@ public void or_conjunction() { */ @Test public void conjunctions_aggregate_in_sequence_without_special_precedence() { + archConfigurationRule.setFailOnEmptyShould(false); + List members = filterResultOf( // (List OR String) AND Collection => empty members().that().areDeclaredInClassesThat().haveSimpleName(List.class.getSimpleName()) @@ -929,11 +941,13 @@ static Evaluator filterResultOf(GivenMembersConjunction @Retention(RetentionPolicy.RUNTIME) private @interface SomeAnnotation { + String value() default "default"; } @Retention(RetentionPolicy.RUNTIME) @SomeAnnotation private @interface MetaAnnotatedAnnotation { + Class value() default Object.class; } private static class SimpleClass { diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java index d7408a9b81..d1097f8fbc 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java @@ -20,9 +20,11 @@ import com.tngtech.archunit.lang.ConditionEvents; import com.tngtech.archunit.lang.EvaluationResult; import com.tngtech.archunit.lang.SimpleConditionEvent; +import com.tngtech.archunit.testutil.ArchConfigurationRule; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -58,6 +60,9 @@ @RunWith(DataProviderRunner.class) public class GivenMembersTest { + @Rule + public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule(); + @DataProvider public static Object[][] member_syntax_testcases() { return $$( @@ -431,6 +436,8 @@ private static Object[][] annotatedWithDataPoints( @Test @UseDataProvider("restricted_property_rule_starts") public void property_predicates(DescribedRuleStart conjunction, Set expectedMessages) { + archConfigurationRule.setFailOnEmptyShould(false); + EvaluationResult result = conjunction.should(everythingViolationPrintMemberName()) .evaluate(importClasses(ClassWithVariousMembers.class, A.class, B.class, C.class, MetaAnnotation.class)); @@ -676,7 +683,7 @@ static Set allConstructorsExcept(String constructorDescription) { static final Set ALL_MEMBER_DESCRIPTIONS = union(ALL_CODE_UNIT_DESCRIPTIONS, ALL_FIELD_DESCRIPTIONS); - @SuppressWarnings({"unused"}) + @SuppressWarnings({"unused", "FieldCanBeLocal"}) static class ClassWithVariousMembers { @A private String fieldA; diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldTest.java index 14d7bf08f2..52a32fbaac 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/MembersShouldTest.java @@ -10,6 +10,8 @@ import com.google.common.collect.ImmutableSet; import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.base.Function; +import com.tngtech.archunit.core.domain.JavaMember; +import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.lang.ArchRule; import com.tngtech.archunit.lang.EvaluationResult; import com.tngtech.archunit.lang.conditions.ArchConditions; @@ -20,9 +22,12 @@ import com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.MetaAnnotation; import com.tngtech.archunit.lang.syntax.elements.GivenMembersTest.OtherClassWithMembers; import com.tngtech.archunit.lang.syntax.elements.testclasses.SimpleFieldAndMethod; +import com.tngtech.archunit.testutil.ArchConfigurationRule; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.assertj.core.api.ThrowableAssert; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -88,10 +93,14 @@ import static java.util.Collections.emptySet; import static java.util.regex.Pattern.quote; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; @RunWith(DataProviderRunner.class) public class MembersShouldTest { + @Rule + public final ArchConfigurationRule configurationRule = new ArchConfigurationRule(); + @Test public void complex_members_syntax() { EvaluationResult result = members() @@ -672,6 +681,35 @@ public void containNumberOfElements_fails_on_mismatching_predicate() { + "com.tngtech.archunit.lang.syntax.elements.testclasses.SimpleFieldAndMethod.violated()]"); } + @Test + public void should_fail_on_empty_should_by_default() { + assertThatThrownBy(new ThrowableAssert.ThrowingCallable() { + @Override + public void call() { + ruleWithEmptyShould().check(new ClassFileImporter().importClasses(getClass())); + } + }).isInstanceOf(AssertionError.class) + .hasMessageContaining("failed to check any classes"); + } + + @Test + public void should_allow_empty_should_if_configured() { + configurationRule.setFailOnEmptyShould(false); + + ruleWithEmptyShould().check(new ClassFileImporter().importClasses(getClass())); + } + + @Test + public void should_allow_empty_should_if_overridden_by_rule() { + configurationRule.setFailOnEmptyShould(true); + + ruleWithEmptyShould().allowEmptyShould(true).check(new ClassFileImporter().importClasses(getClass())); + } + + private static ArchRule ruleWithEmptyShould() { + return members().that(DescribedPredicate.alwaysFalse()).should().bePublic(); + } + private Set parseMembers(List details) { return parseMembers(ImmutableList.of(ClassWithVariousMembers.class, OtherClassWithMembers.class), details); } diff --git a/archunit/src/test/java/com/tngtech/archunit/library/ArchitecturesTest.java b/archunit/src/test/java/com/tngtech/archunit/library/ArchitecturesTest.java index 23cd7e541a..95328100bc 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/ArchitecturesTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/ArchitecturesTest.java @@ -47,6 +47,7 @@ import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name; import static com.tngtech.archunit.library.Architectures.layeredArchitecture; import static com.tngtech.archunit.library.Architectures.onionArchitecture; +import static com.tngtech.archunit.testutil.Assertions.assertThatRule; import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach; import static java.beans.Introspector.decapitalize; import static java.lang.System.lineSeparator; @@ -158,9 +159,7 @@ public void layered_architecture_allows_empty_layers_if_all_layers_are_optional( JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); - EvaluationResult result = architecture.evaluate(classes); - assertThat(result.hasViolation()).as("result of evaluating empty layers has violation").isFalse(); - assertThat(result.getFailureReport().isEmpty()).as("failure report").isTrue(); + assertThatRule(architecture).checking(classes).hasNoViolation(); } @Test @@ -177,7 +176,8 @@ private LayeredArchitecture aLayeredArchitectureWithEmptyLayers() { return layeredArchitecture() .layer("Some").definedBy(absolute("should.not.be.found..")) .layer("Other").definedBy(absolute("also.not.found")) - .layer("Okay").definedBy("..testclasses.."); + .layer("Okay").definedBy("..testclasses..") + .whereLayer("Other").mayOnlyBeAccessedByLayers("Some"); } private void assertFailureLayeredArchitectureWithEmptyLayers(EvaluationResult result) { diff --git a/archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java b/archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java index b92c0416ac..deafea029a 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java @@ -12,6 +12,8 @@ import com.tngtech.archunit.lang.syntax.elements.GivenConjunction; import com.tngtech.archunit.library.ArchitecturesTest; import com.tngtech.archunit.library.dependencies.syntax.GivenSlices; +import com.tngtech.archunit.testutil.ArchConfigurationRule; +import org.junit.Rule; import org.junit.Test; import static com.tngtech.archunit.lang.conditions.ArchPredicates.have; @@ -21,6 +23,9 @@ public class GivenSlicesTest { static final String TEST_CLASSES_PACKAGE = ArchitecturesTest.class.getPackage().getName() + ".testclasses"; + @Rule + public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule(); + @Test public void chosen_slices() { Set slices = getSlicesMatchedByFilter( @@ -46,6 +51,8 @@ public void chosen_slices() { @Test public void restricting_slices_that_should_not_depend_on_each_other() { + archConfigurationRule.setFailOnEmptyShould(false); + GivenSlices givenSlices = slices().matching("..testclasses.(*).."); JavaClasses classes = new ClassFileImporter().importPackages("com.tngtech.archunit.library.testclasses"); @@ -85,4 +92,4 @@ public void check(Slice item, ConditionEvents events) { }).evaluate(new ClassFileImporter().importPackages(TEST_CLASSES_PACKAGE)); return matched; } -} \ 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 7be30e5521..9debb606b6 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 @@ -16,6 +16,7 @@ import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; import org.assertj.core.api.Condition; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -28,6 +29,7 @@ import static com.tngtech.java.junit.dataprovider.DataProviders.$$; import static java.lang.System.lineSeparator; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; @RunWith(DataProviderRunner.class) public class SliceRuleTest { @@ -147,6 +149,35 @@ public void limits_number_of_reported_dependencies_per_edge_by_default_to_20() { "Dependencies of Slice threedependencies")); } + @Test + public void forbids_empty_should_by_default() { + assertThatThrownBy(new ThrowingCallable() { + @Override + public void call() { + ruleWithEmptyShould().check(new ClassFileImporter().importClasses(getClass())); + } + }).isInstanceOf(AssertionError.class) + .hasMessageContaining("failed to check any classes"); + } + + @Test + public void should_allow_empty_should_if_configured() { + configurationRule.setFailOnEmptyShould(false); + + ruleWithEmptyShould().check(new ClassFileImporter().importClasses(getClass())); + } + + @Test + public void allows_empty_should_if_overridden() { + configurationRule.setFailOnEmptyShould(true); + + ruleWithEmptyShould().allowEmptyShould(true).check(new ClassFileImporter().importClasses(getClass())); + } + + private static SliceRule ruleWithEmptyShould() { + return slices().matching("nothing_because_there_is_no_capture_group").should().beFreeOfCycles(); + } + private List filterLinesMatching(String text, final String regex) { return FluentIterable.from(Splitter.on(lineSeparator()).split(text)) .filter(new Predicate() { diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/ArchConfigurationRule.java b/archunit/src/test/java/com/tngtech/archunit/testutil/ArchConfigurationRule.java index ab349c36b1..48acec827f 100644 --- a/archunit/src/test/java/com/tngtech/archunit/testutil/ArchConfigurationRule.java +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/ArchConfigurationRule.java @@ -1,56 +1,68 @@ package com.tngtech.archunit.testutil; -import java.lang.reflect.AccessibleObject; -import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Callable; import com.tngtech.archunit.ArchConfiguration; import org.junit.rules.ExternalResource; -import static com.tngtech.archunit.testutil.ReflectionTestUtils.field; -import static com.tngtech.archunit.testutil.ReflectionTestUtils.method; - public class ArchConfigurationRule extends ExternalResource { - private boolean resolveMissingDependenciesFromClassPath = ArchConfiguration.get().resolveMissingDependenciesFromClassPath(); + public static final String FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME = "archRule.failOnEmptyShould"; - public static T resetConfigurationAround(Callable callable) { - ArchConfiguration.get().reset(); - try { - return callable.call(); - } catch (Exception e) { - throw new RuntimeException(e); - } finally { - ArchConfiguration.get().reset(); - } - } + private boolean beforeHasBeenExecuted = false; + private final List configurationInitializers = new ArrayList<>(); - public ArchConfigurationRule resolveAdditionalDependenciesFromClassPath(boolean enabled) { - resolveMissingDependenciesFromClassPath = enabled; + public ArchConfigurationRule resolveAdditionalDependenciesFromClassPath(final boolean enabled) { + addConfigurationInitializer(new Runnable() { + @Override + public void run() { + ArchConfiguration.get().setResolveMissingDependenciesFromClassPath(enabled); + } + }); return this; } - public void removeProperty(String propertyName) { - try { - Object properties = accessible(field(ArchConfiguration.class, "properties")).get(ArchConfiguration.get()); - accessible(method(properties.getClass(), "remove", String.class)).invoke(properties, propertyName); - } catch (IllegalAccessException | InvocationTargetException e) { - throw new RuntimeException(e); - } + public ArchConfigurationRule setFailOnEmptyShould(final boolean allowEmptyShould) { + addConfigurationInitializer(new Runnable() { + @Override + public void run() { + ArchConfiguration.get().setProperty(FAIL_ON_EMPTY_SHOULD_PROPERTY_NAME, String.valueOf(allowEmptyShould)); + } + }); + return this; } - private T accessible(T member) { - member.setAccessible(true); - return member; + private void addConfigurationInitializer(Runnable initializer) { + if (beforeHasBeenExecuted) { + initializer.run(); + } else { + configurationInitializers.add(initializer); + } } @Override protected void before() { ArchConfiguration.get().reset(); - ArchConfiguration.get().setResolveMissingDependenciesFromClassPath(resolveMissingDependenciesFromClassPath); + for (Runnable initializer : configurationInitializers) { + initializer.run(); + } + beforeHasBeenExecuted = true; } @Override protected void after() { ArchConfiguration.get().reset(); } + + public static T resetConfigurationAround(Callable callable) { + ArchConfiguration.get().reset(); + try { + return callable.call(); + } catch (Exception e) { + throw new RuntimeException(e); + } finally { + ArchConfiguration.get().reset(); + } + } } diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/syntax/RandomSyntaxTestBase.java b/archunit/src/test/java/com/tngtech/archunit/testutil/syntax/RandomSyntaxTestBase.java index 8cd348bc72..cf1320a050 100644 --- a/archunit/src/test/java/com/tngtech/archunit/testutil/syntax/RandomSyntaxTestBase.java +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/syntax/RandomSyntaxTestBase.java @@ -18,8 +18,10 @@ import com.tngtech.archunit.lang.ArchCondition; import com.tngtech.archunit.lang.ArchRule; import com.tngtech.archunit.lang.ConditionEvents; +import com.tngtech.archunit.testutil.ArchConfigurationRule; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.slf4j.Logger; @@ -39,6 +41,9 @@ public abstract class RandomSyntaxTestBase { protected static final Random random = new Random(); private static final int NUMBER_OF_RULES_TO_BUILD = 1000; + @Rule + public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule().setFailOnEmptyShould(false); + public static List> createRandomRules(RandomSyntaxSeed seed, DescriptionReplacement... replacements) { return createRandomRules(seed, MethodChoiceStrategy.chooseAllArchUnitSyntaxMethods(), replacements); diff --git a/archunit/src/test/resources/archunit.properties b/archunit/src/test/resources/archunit.properties index 19f5f4b93e..8e8235ded4 100644 --- a/archunit/src/test/resources/archunit.properties +++ b/archunit/src/test/resources/archunit.properties @@ -1,4 +1,3 @@ enableMd5InClassSources=true -archRule.failOnEmptyShould=false import.dependencyResolutionProcess.maxIterationsForEnclosingTypes=0 -import.dependencyResolutionProcess.maxIterationsForGenericSignatureTypes=0 \ No newline at end of file +import.dependencyResolutionProcess.maxIterationsForGenericSignatureTypes=0 diff --git a/docs/userguide/010_Advanced_Configuration.adoc b/docs/userguide/010_Advanced_Configuration.adoc index fdeb839fed..b626d7fbf4 100644 --- a/docs/userguide/010_Advanced_Configuration.adoc +++ b/docs/userguide/010_Advanced_Configuration.adoc @@ -158,8 +158,23 @@ However, it actually does not check any classes at all anymore. This is likely not what most users want. Thus, by default ArchUnit will fail checking the rule in this case. If you want to allow evaluating such rules, -i.e. where the actual input to the should-conditionis empty, -you can set the following property: +i.e. where the actual input to the should-clause is empty, +you can use one of the following ways: + +*Allow Empty Should on a Per-Rule Basis* + +On each `ArchRule` you can use the method `ArchRule.allowEmptyShould(..)` to override the behavior +for a single rule, e.g. + +[source,java,options="nowrap"] +---- +// create a rule that allows that no classes are passed to the should-clause +classes().that()...should()...allowEmptyShould(true) +---- + +*Allow Empty Should Globally* + +To allow all rules to be evaluated without checking any classes you can set the following property: [source,options="nowrap"] .archunit.properties