diff --git a/src/test/java/org/kohsuke/github/ArchTests.java b/src/test/java/org/kohsuke/github/ArchTests.java index e21a7ab108..fc4891f868 100644 --- a/src/test/java/org/kohsuke/github/ArchTests.java +++ b/src/test/java/org/kohsuke/github/ArchTests.java @@ -1,13 +1,16 @@ 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; @@ -15,6 +18,9 @@ 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,15 +32,25 @@ 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; @@ -42,6 +58,8 @@ /** * 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 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 + * the generic type + * @param infix + * the infix + * @param unlessPredicates + * the unless predicates + * @return the arch condition + */ + public static ArchCondition haveNamesContainingUnless( + final String infix, + final DescribedPredicate... unlessPredicates) { + DescribedPredicate restrictedNameContaining = nameContaining(infix); + + if (unlessPredicates.length > 0) { + final DescribedPredicate allowed = or(unlessPredicates); + restrictedNameContaining = unless(nameContaining(infix), allowed); + } + return have(restrictedNameContaining); + } + /** * Not call methods in package unless. * @@ -156,7 +237,7 @@ public static ArchCondition notCallMethodsInPackageUnless(final Strin } restrictedPackageCalls = unless(restrictedPackageCalls, allowed); } - return not(callMethodWhere(restrictedPackageCalls)); + return ArchConditions.not(callMethodWhere(restrictedPackageCalls)); } /** @@ -200,6 +281,15 @@ public static DescribedPredicate unless(DescribedPredicate fir return new UnlessPredicate(first, second); } + /** + * Enum constants. + * + * @return the described predicate + */ + private DescribedPredicate enumConstants() { + return new EnumConstantFieldPredicate(); + } + private static class UnlessPredicate extends DescribedPredicate { private final DescribedPredicate current; private final DescribedPredicate other; @@ -215,4 +305,16 @@ public boolean test(T input) { return current.test(input) && !other.test(input); } } + + private static final class EnumConstantFieldPredicate extends DescribedPredicate { + 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()); + } + } }