improve rule to not fail for classes with no tests in their packages

[krzysztof-owczarek]

This will improve the existing rule to test that test classes reside in the same package as their implementation. In particular the rule:

Should not fail if there are multiple test classes with the same simple name and some of the classes have no tests at all

Issue: #1367

[hankem]

Thanks a lot for your contribution! 💚
I'd like to take a closer look whether we can simplify the rule, but I already wanted to leave one comment on the test. 😉

[hankem]

For your test to cover the use case of #1367, this should be

Suggested change

	public class OnlyOneImplementationHaveTestAndItIsMatchingImplPackageTest {
	class MultipleImplementationsWithOnlyOneTestAndInRightPackageTest {

to make your new test actually fail with the old implementation. 🙂

In addition: I'd drop the AndInRightPackage part from this name: It's a relevant aspect to have classes with the same simple name in different packages, so: Is it right or is it wrong??
How about SimpleNameThatOccursInSeveralPackages?

[krzysztof-owczarek]

I was reneming classes at the end and forgot about that one 🤦 Thanks for this suggestion - applied!

[krzysztof-owczarek]

Thanks a lot for your contribution! 💚 I'd like to take a closer look whether we can simplify the rule, but I already wanted to leave one comment on the test. 😉

I have a feeling it might be done in with a simpler check. Sorry for not persuing that before I have created this PR, but I am just before my vacation and had a little time to create the change 🌴

I have another, simpler draft of such a condition, but it requires a bit sloppy name creation function:

Map<String, List<JavaClass>> testClassesBySimpleClassName = new HashMap<>();
Map<String, JavaClass> classesByPackageName = new HashMap<>();

@Override
public void init(Collection<JavaClass> allClasses) {
    testClassesBySimpleClassName = allClasses.stream()
        .filter(clazz -> clazz.getName().endsWith(testClassSuffix))
        .collect(groupingBy(JavaClass::getSimpleName));

    classesByPackageName = allClasses.stream()
         .collect(toMap(JavaClass::getFullName, Function.identity()));
}

and then:

possibleTestClasses.stream()
    .map(testClazz -> classesByPackageName.getOrDefault(createImplClassNameFromTestClassName(testClazz), null))
    .noneMatch(Objects::nonNull);

having:

private String createImplClassNameFromTestClassName(JavaClass testClazz) {
    return testClazz.getPackageName() + "." + testClazz.getSimpleName().replace(testClassSuffix, "");
}

I am not super happy with this draft, but the condition is much simpler that way. Maybe it will be a step towords some better solution.

[krzysztof-owczarek]

Hey @hankem, I will paste the part of annonynous class here, that has a simpler condition, but is based on name resolving as described before. Give me heads up what do you think about it.

private static ArchCondition<JavaClass> resideInTheSamePackageAsTheirTestClasses(String testClassSuffix) {
        return new ArchCondition<JavaClass>("reside in the same package as their test classes") {
            Map<String, List<JavaClass>> testClassesBySimpleClassName = new HashMap<>();
            Map<String, JavaClass> classesByPackageName = new HashMap<>();

            @Override
            public void init(Collection<JavaClass> allClasses) {
                testClassesBySimpleClassName = allClasses.stream()
                        .filter(clazz -> clazz.getName().endsWith(testClassSuffix))
                        .collect(groupingBy(JavaClass::getSimpleName));

                classesByPackageName = allClasses.stream()
                        .collect(toMap(JavaClass::getFullName, Function.identity()));
            }

            @Override
            public void check(JavaClass implementationClass,
                              ConditionEvents events) {
                String implementationClassName = implementationClass.getSimpleName();
                String implementationClassPackageName = implementationClass.getPackageName();
                String possibleTestClassName = implementationClassName + testClassSuffix;
                List<JavaClass> possibleTestClasses = testClassesBySimpleClassName.getOrDefault(possibleTestClassName,
                        emptyList());

                boolean isTestClassInWrongPackage = !possibleTestClasses.isEmpty()
                        && possibleTestClasses.stream().noneMatch(
                        clazz -> clazz.getPackageName().equals(implementationClassPackageName))
                        && implementationNotFoundInTestClassPackages(possibleTestClasses);

                if (isTestClassInWrongPackage) {
                    possibleTestClasses.forEach(wrongTestClass -> {
                        String message = createMessage(wrongTestClass,
                                String.format("does not reside in same package as implementation class <%s>",
                                        implementationClass.getName()));
                        events.add(violated(wrongTestClass, message));
                    });
                }
            }

            private boolean implementationNotFoundInTestClassPackages(List<JavaClass> possibleTestClasses) {
                return possibleTestClasses.stream()
                        .map(testClazz -> classesByPackageName.getOrDefault(
                                resolveImplClassNameFromTestClassName(testClazz), null)
                        )
                        .noneMatch(Objects::nonNull);
            }

            private String resolveImplClassNameFromTestClassName(JavaClass testClazz) {
                return testClazz.getPackageName() + "." + testClazz.getSimpleName().replace(testClassSuffix, "");
            }

        };
    }