From d0ee71a0ef7364201b742c4ed7283f852e316fe0 Mon Sep 17 00:00:00 2001
From: Manfred Hanke <Manfred.Hanke@tngtech.com>
Date: Mon, 3 Oct 2022 13:28:31 +0200
Subject: [PATCH] add general coding rule ASSERTIONS_SHOULD_HAVE_DETAIL_MESSAGE

Signed-off-by: Manfred Hanke <Manfred.Hanke@tngtech.com>
---
 .../archunit/library/GeneralCodingRules.java  | 18 ++++++++++
 .../library/GeneralCodingRulesTest.java       | 35 +++++++++++++++++++
 .../assertion/ArchRuleCheckAssertion.java     | 18 ++++------
 3 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/archunit/src/main/java/com/tngtech/archunit/library/GeneralCodingRules.java b/archunit/src/main/java/com/tngtech/archunit/library/GeneralCodingRules.java
index 3296f2f1aa..f0a6725cad 100644
--- a/archunit/src/main/java/com/tngtech/archunit/library/GeneralCodingRules.java
+++ b/archunit/src/main/java/com/tngtech/archunit/library/GeneralCodingRules.java
@@ -480,4 +480,22 @@ public void check(JavaClass implementationClass, ConditionEvents events) {
             }
         };
     }
+
+    /**
+     * A rule that checks that all {@link AssertionError AssertionErrors} (e.g. from the {@code assert} keyword) have a detail message.
+     * <p>
+     * Example:
+     * <pre>{@code
+     * assert x > 0;  // violation
+     * throw new AssertionError();  // violation
+     *
+     * assert x > 0 : "x is not positive";  // no violation
+     * throw new AssertionError("x is not positive");  // no violation
+     * }</pre>
+     </p>
+     */
+    @PublicAPI(usage = ACCESS)
+    public static final ArchRule ASSERTIONS_SHOULD_HAVE_DETAIL_MESSAGE =
+            noClasses().should().callConstructor(AssertionError.class /* without detailMessage */)
+                    .because("assertions should have a detail message");
 }
diff --git a/archunit/src/test/java/com/tngtech/archunit/library/GeneralCodingRulesTest.java b/archunit/src/test/java/com/tngtech/archunit/library/GeneralCodingRulesTest.java
index f274c1d01b..352cffe787 100644
--- a/archunit/src/test/java/com/tngtech/archunit/library/GeneralCodingRulesTest.java
+++ b/archunit/src/test/java/com/tngtech/archunit/library/GeneralCodingRulesTest.java
@@ -12,8 +12,10 @@
 import com.tngtech.archunit.library.testclasses.packages.incorrect.wrongsubdir.defaultsuffix.subdir.ImplementationClassWithWrongTestClassPackageTest;
 import org.junit.Test;
 
+import static com.tngtech.archunit.library.GeneralCodingRules.ASSERTIONS_SHOULD_HAVE_DETAIL_MESSAGE;
 import static com.tngtech.archunit.library.GeneralCodingRules.testClassesShouldResideInTheSamePackageAsImplementation;
 import static com.tngtech.archunit.testutil.Assertions.assertThatRule;
+import static java.util.regex.Pattern.quote;
 
 public class GeneralCodingRulesTest {
 
@@ -80,4 +82,37 @@ public void should_not_pass_when_none_of_multiple_matching_test_classes_resides_
                 );
     }
 
+    @Test
+    public void ASSERTIONS_SHOULD_HAVE_DETAIL_MESSAGE_should_fail_on_assert_without_detail_message() {
+        class InvalidAssertions {
+            void f(int x) {
+                assert x > 0;
+            }
+
+            void f() {
+                throw new AssertionError();
+            }
+        }
+        assertThatRule(ASSERTIONS_SHOULD_HAVE_DETAIL_MESSAGE)
+                .checking(new ClassFileImporter().importClasses(InvalidAssertions.class))
+                .hasViolations(2)
+                .hasViolationMatching(quote("Method <" + InvalidAssertions.class.getName() + ".f(int)> calls constructor <java.lang.AssertionError.<init>()> in ") + ".*")
+                .hasViolationMatching(quote("Method <" + InvalidAssertions.class.getName() + ".f()> calls constructor <java.lang.AssertionError.<init>()> in ") + ".*");
+    }
+
+    @Test
+    public void ASSERTIONS_SHOULD_HAVE_DETAIL_MESSAGE_should_accept_assert_with_detail_message() {
+        class ValidAssertions {
+            void f(int x) {
+                assert x > 0 : "argument should be postive";
+            }
+
+            void f() {
+                throw new AssertionError("f() should not be called");
+            }
+        }
+        assertThatRule(ASSERTIONS_SHOULD_HAVE_DETAIL_MESSAGE)
+                .checking(new ClassFileImporter().importClasses(ValidAssertions.class))
+                .hasNoViolation();
+    }
 }
diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ArchRuleCheckAssertion.java b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ArchRuleCheckAssertion.java
index 7a65183649..e54def8e37 100644
--- a/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ArchRuleCheckAssertion.java
+++ b/archunit/src/test/java/com/tngtech/archunit/testutil/assertion/ArchRuleCheckAssertion.java
@@ -30,20 +30,16 @@ private Optional<AssertionError> checkRule(ArchRule rule, JavaClasses classes) {
     }
 
     public ArchRuleCheckAssertion hasViolationMatching(String regex) {
-        for (String detail : evaluationResult.getFailureReport().getDetails()) {
-            if (detail.matches(regex)) {
-                return this;
-            }
-        }
-        throw new AssertionError(String.format("No violation matches '%s'", regex));
+        assertThat(evaluationResult.getFailureReport().getDetails())
+                .as("violation details (should match '%s')", regex)
+                .anyMatch(detail -> detail.matches(regex));
+        return this;
     }
 
     public ArchRuleCheckAssertion hasNoViolationMatching(String regex) {
-        for (String detail : evaluationResult.getFailureReport().getDetails()) {
-            if (detail.matches(regex)) {
-                throw new AssertionError(String.format("Violation matches '%s'", regex));
-            }
-        }
+        assertThat(evaluationResult.getFailureReport().getDetails())
+                .as("violation details (should not match '%s')", regex)
+                .noneMatch(detail -> detail.matches(regex));
         return this;
     }