Adds ArchUnit tests for naming conventions

src/test/java/org/kohsuke/github/ArchTests.java

@@ -1,20 +1,26 @@
 package org.kohsuke.github;
 
 import com.tngtech.archunit.base.DescribedPredicate;
+import com.tngtech.archunit.base.HasDescription;
 import com.tngtech.archunit.core.domain.*;
 import com.tngtech.archunit.core.domain.properties.HasName;
 import com.tngtech.archunit.core.domain.properties.HasOwner;
+import com.tngtech.archunit.core.domain.properties.HasSourceCodeLocation;
 import com.tngtech.archunit.core.importer.ClassFileImporter;
 import com.tngtech.archunit.core.importer.ImportOption;
 import com.tngtech.archunit.lang.ArchCondition;
 import com.tngtech.archunit.lang.ArchRule;
+import com.tngtech.archunit.lang.conditions.ArchConditions;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringStyle;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.kohsuke.github.GHDiscussion.Creator;
+import org.kohsuke.github.GHPullRequestCommitDetail.Commit;
+import org.kohsuke.github.GHPullRequestCommitDetail.CommitPointer;
 
 import java.io.Closeable;
 import java.io.InputStream;
@@ -26,22 +32,34 @@
 import java.util.stream.Collectors;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.tngtech.archunit.base.DescribedPredicate.not;
+import static com.tngtech.archunit.base.DescribedPredicate.or;
 import static com.tngtech.archunit.core.domain.JavaCall.Predicates.target;
+import static com.tngtech.archunit.core.domain.JavaClass.Predicates.assignableTo;
 import static com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAPackage;
 import static com.tngtech.archunit.core.domain.JavaClass.Predicates.type;
+import static com.tngtech.archunit.core.domain.JavaMember.Predicates.declaredIn;
+import static com.tngtech.archunit.core.domain.JavaModifier.FINAL;
+import static com.tngtech.archunit.core.domain.JavaModifier.STATIC;
+import static com.tngtech.archunit.core.domain.properties.HasModifiers.Predicates.modifier;
 import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name;
 import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.nameContaining;
 import static com.tngtech.archunit.core.domain.properties.HasOwner.Predicates.With.owner;
 import static com.tngtech.archunit.core.domain.properties.HasParameterTypes.Predicates.rawParameterTypes;
 import static com.tngtech.archunit.lang.conditions.ArchConditions.*;
 import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
+import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;
+import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noFields;
+import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noMethods;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.greaterThan;
 
 // TODO: Auto-generated Javadoc
 /**
  * The Class ArchTests.
  */
+@SuppressWarnings({ "LocalVariableNamingConvention", "TestMethodWithoutAssertion", "UnqualifiedStaticUsage",
+        "unchecked", "MethodMayBeStatic", "FieldNamingConvention", "StaticCollection" })
 public class ArchTests {
 
     private static final JavaClasses classFiles = new ClassFileImporter()
@@ -69,6 +87,46 @@ public static void beforeClass() {
         assertThat(classFiles.size(), greaterThan(0));
     }
 
+    /**
+     * Test naming conventions
+     */
+    @Test
+    public void testRequireFollowingNamingConvention() {
+        final String reason = "This project follows standard java naming conventions and does not allow the use of underscores in names.";
+
+        final ArchRule fieldsNotFollowingConvention = noFields().that()
+                .arePublic()
+                .and(not(enumConstants()))
+                .and(not(modifier(STATIC).and(modifier(FINAL)).as("static final")))
+                .should(haveNamesContainingUnless("_"))
+                .because(reason);
+
+        @SuppressWarnings("AccessStaticViaInstance")
+        final ArchRule methodsNotFollowingConvention = noMethods().that()
+                .arePublic()
+                .should(haveNamesContainingUnless("_",
+                        // currently failing method names
+                        // TODO: 2025-03-28 Fix & remove these
+                        declaredIn(assignableTo(PagedIterable.class)).and(name("_iterator")),
+                        declaredIn(GHCompare.class).and(name("getAdded_by")),
+                        declaredIn(GHDeployKey.class).and(name("getAdded_by")),
+                        declaredIn(GHDeployKey.class).and(name("isRead_only")),
+                        declaredIn(assignableTo(GHRepositoryBuilder.class)).and(name("private_")),
+                        declaredIn(Creator.class).and(name("private_")),
+                        declaredIn(GHGistBuilder.class).and(name("public_")),
+                        declaredIn(Commit.class).and(name("getComment_count")),
+                        declaredIn(CommitPointer.class).and(name("getHtml_url")),
+                        declaredIn(GHRelease.class).and(name("getPublished_at"))))
+                .because(reason);
+
+        final ArchRule classesNotFollowingConvention = noClasses().should(haveNamesContainingUnless("_"))
+                .because(reason);
+
+        fieldsNotFollowingConvention.check(classFiles);
+        methodsNotFollowingConvention.check(classFiles);
+        classesNotFollowingConvention.check(classFiles);
+    }
+
     /**
      * Test require use of assert that.
      */
@@ -78,10 +136,10 @@ public void testRequireUseOfAssertThat() {
         final String reason = "This project uses `assertThat(...)` or `assertThrows(...)` instead of other `assert*()` methods.";
 
         final DescribedPredicate<HasName> assertMethodOtherThanAssertThat = nameContaining("assert")
-                .and(DescribedPredicate.not(name("assertThat")).and(DescribedPredicate.not(name("assertThrows"))));
+                .and(not(name("assertThat")).and(not(name("assertThrows"))));
 
         final ArchRule onlyAssertThatRule = classes()
-                .should(not(callMethodWhere(target(assertMethodOtherThanAssertThat))))
+                .should(ArchConditions.not(callMethodWhere(target(assertMethodOtherThanAssertThat))))
                 .because(reason);
 
         onlyAssertThatRule.check(testClassFiles);
@@ -135,6 +193,29 @@ public void testRequireUseOfOnlySpecificApacheCommons() {
         onlyApprovedApacheCommonsMethods.check(classFiles);
     }
 
+    /**
+     * Have names containing unless.
+     *
+     * @param <T>
+     *            the generic type
+     * @param infix
+     *            the infix
+     * @param unlessPredicates
+     *            the unless predicates
+     * @return the arch condition
+     */
+    public static <T extends HasDescription & HasSourceCodeLocation & HasName> ArchCondition<T> haveNamesContainingUnless(
+            final String infix,
+            final DescribedPredicate<? super T>... unlessPredicates) {
+        DescribedPredicate<? super T> restrictedNameContaining = nameContaining(infix);
+
+        if (unlessPredicates.length > 0) {
+            final DescribedPredicate<T> allowed = or(unlessPredicates);
+            restrictedNameContaining = unless(nameContaining(infix), allowed);
+        }
+        return have(restrictedNameContaining);
+    }
+
     /**
      * Not call methods in package unless.
      *
@@ -156,7 +237,7 @@ public static ArchCondition<JavaClass> notCallMethodsInPackageUnless(final Strin
             }
             restrictedPackageCalls = unless(restrictedPackageCalls, allowed);
         }
-        return not(callMethodWhere(restrictedPackageCalls));
+        return ArchConditions.not(callMethodWhere(restrictedPackageCalls));
     }
 
     /**
@@ -200,6 +281,15 @@ public static <T> DescribedPredicate<T> unless(DescribedPredicate<? super T> fir
         return new UnlessPredicate(first, second);
     }
 
+    /**
+     * Enum constants.
+     *
+     * @return the described predicate
+     */
+    private DescribedPredicate<? super JavaField> enumConstants() {
+        return new EnumConstantFieldPredicate();
+    }
+
     private static class UnlessPredicate<T> extends DescribedPredicate<T> {
         private final DescribedPredicate<T> current;
         private final DescribedPredicate<? super T> other;
@@ -215,4 +305,16 @@ public boolean test(T input) {
             return current.test(input) && !other.test(input);
         }
     }
+
+    private static final class EnumConstantFieldPredicate extends DescribedPredicate<JavaField> {
+        private EnumConstantFieldPredicate() {
+            super("are not enum constants");
+        }
+
+        @Override
+        public boolean test(JavaField javaField) {
+            JavaClass owner = javaField.getOwner();
+            return owner.isEnum() && javaField.getRawType().isAssignableTo(owner.reflect());
+        }
+    }
 }