How to test for access only by subclass

[abeyecon]

I want to make a rule where classes in a particular package can only be accessed by their subclasses, but I can't figure out how.
Is it possible at the moment?
I thought I could use a DescribedPredicate, but that doesn't provide access to the class that is triggering the check. (i.e. extendThem() only makes sense after the should(), but not before)
Would the following fluent style .byClassesThat().extendThem() be possible to add?

//Not the actual rule
@archtest
public static final ArchRule baseClassesShouldOnlyBeAccessedByTheirSubclasses = classes()
.that().resideInAPackage(BASE)
.should().onlyBeAccessed().byClassesThat().extendThem();

[abeyecon]

I can write a less specific test, but it allows any subclass of any base class to access any base class rather than just the one they extend:
.should().onlyBeAccessed().byClassesThat(IMPLEMENT_ANY_CLASS_IN_BASE_PACKAGE);

[abeyecon]

import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes;
import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.fields;
import static com.tngtech.archunit.library.dependencies.SlicesRuleDefinition.slices;

import org.junit.runner.RunWith;

import com.tngtech.archunit.core.importer.ImportOption;
import com.tngtech.archunit.junit.AnalyzeClasses;
import com.tngtech.archunit.junit.ArchTest;
import com.tngtech.archunit.junit.ArchUnitRunner;
import com.tngtech.archunit.lang.ArchRule;
import com.tngtech.archunit.library.GeneralCodingRules;

@RunWith(ArchUnitRunner.class)
@AnalyzeClasses(packages = "com.eyecon.stellav.integration.wallet.adapter.eyecon", importOptions = {
        ImportOption.DoNotIncludeTests.class })
public class ArchitectureTest {
    private static final String PARENT_PACKAGE = "com.eyecon.stellav.integration.wallet.adapter";
    private static final String BASE_PACKAGE = PARENT_PACKAGE + ".eyecon";
    private static final String SERVICE = BASE_PACKAGE;
    private static final String DAO = BASE_PACKAGE + ".dao..";

    @ArchTest
    public static final ArchRule noCycles = slices().matching(PARENT_PACKAGE + ".(**)").should()
            .beFreeOfCycles();

    @ArchTest
    public static final ArchRule daoOnlyUsedByServiceAndSelf = classes()
            .that().resideInAPackage(DAO)
            .should().onlyBeAccessed().byAnyPackage(SERVICE, DAO);

    @ArchTest
    public static final ArchRule serviceOnlyUsedByJobOrSelf = classes()
            .that().resideInAPackage(SERVICE)
            .should().onlyBeAccessed().byAnyPackage(BASE_PACKAGE, SERVICE);

    @ArchTest
    public static final ArchRule implClassesShouldOnlyBeInjected = classes()
            .that().haveNameMatching(".*Impl$")
            .should().onlyBeAccessed().byClassesThat().haveNameMatching(".*Impl$");

    @ArchTest
    public static final ArchRule noAccessToStandardStreams = GeneralCodingRules.NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS;

    @ArchTest
    public static final ArchRule publicFieldsAreStaticFinal = fields()
            .that().arePublic()
            .should().beFinal()
            .andShould().beStatic();
}```

[abeyecon]

The above class should fail on exactly three lines 53, 62, 69.
Tried to format the code using quote and code tags, but it doesn't work correctly.

[hankem]

I agree that the predicate in

classes().should().onlyBeAccessed().byClassesThat(DescribedPredicate<JavaClass> predicate)

does not allow to capture the context of the access – which is also why

classes().should().onlyBeAccessed().byClassesThat().extendThem()

cannot easily be implemented, as classes().should().onlyBeAccessed().byClassesThat() gives a ClassesThat very similar to classes().that().
(Of course, everything can be done with enough effort... 😉)

So similarly to #254, I'd suggest to implement a custom condition.
With what I thought that I had understood from your first post, I wanted to suggest

    @ArchTest
    public static final ArchRule baseClassesShouldOnlyBeAccessedByTheirSubclasses = classes()
            .that().resideInAPackage(Issue255.class.getPackage().getName())
            .should(onlyBeAccessedBySubclasses());

    private static ArchCondition<JavaClass> onlyBeAccessedBySubclasses() {
        return new ArchCondition<JavaClass>("only be accessed by subclasses") {
            @Override
            public void check(JavaClass clazz, ConditionEvents events) {
                int nAccessesByOtherClasses = 0;
                StringBuilder violations = new StringBuilder();
                for (JavaAccess<?> javaAccess : clazz.getAccessesToSelf()) {
                    boolean originOwnerIsSubclass = javaAccess.getOriginOwner().isAssignableTo(clazz.getName());
                    if (!originOwnerIsSubclass) {
                        nAccessesByOtherClasses++;
                        violations.append(javaAccess.getDescription()).append("\n");
                    }
                }
                boolean satisfied = nAccessesByOtherClasses == 0;
                String message = clazz.getName() + " is accessed by " + nAccessesByOtherClasses +  " classes that are no subclasses";
                if (!satisfied) {
                    message += ":\n" + violations;
                }
                events.add(new SimpleConditionEvent(clazz, satisfied, message));
            }
        };
    }

This catches the illegal access in your line 62

Constructor <A2Impl.<init>()> calls constructor <CImpl.<init>()> 

but not your lines 52 and 69:

        super.op2();//NOT OK because called method name (op2) is not caller method name (op)

        super.op();//NOT OK because A3 name is not A3Impl

(which are IMO valid accesses from subclasses, so I still need to understand your use case).

---

P.S. For the formatting, you can use Markdown with triple backticks for code blocks (cf. Creating and highlighting code blocks):

```java
(code)
```

[abeyecon]

Thanks I've reformatted the post above.

[abeyecon]

You are correct 52 is an unusual requirement. It turns out that it was believed to be a rule in our codebase, but when I looked at our actual codebase it is violated a lot, and in ways that make no sense to change.
69 still looks valid. The intention here is that we have a package of interface - implementation pairs, where some implementation classes may extend other implementations, but we shouldn't have any 'other' classes extending the implementations.

[abeyecon]

Thanks @hankem I think I can achieve this requirement with two separate rules, your one, plus a rule around naming.

[abeyecon]

Modified message:

                final String pluralMessagePart = (nAccessesByOtherClasses == 1) ? " class that is not a subclass"
                        : " classes that are not subclasses";
                String message = clazz.getName() + " is accessed by " + nAccessesByOtherClasses

[abeyecon]

Second Rule:

    public static final ArchRule implClassesShouldOnlyBeSubclassedByImplClasses = classes()
            .that(extendClassesInPackage("archunit255", ".*Impl$"))
            .should().haveNameMatching(".*Impl$");

    private static DescribedPredicate<JavaClass> extendClassesInPackage(final String packageName,
            final String classNameRegex) {
        return new DescribedPredicate<JavaClass>("extend classes in %s named %s", packageName, classNameRegex) {
            @Override
            public boolean apply(final JavaClass input) {
                final Optional<JavaClass> optionalSuperclass = input.getSuperClass();
                if (optionalSuperclass.isPresent()) {
                    final JavaClass superClass = optionalSuperclass.get();
                    return packageName.equals(superClass.getPackageName())
                            && superClass.getName().matches(classNameRegex);
                }
                return false;
            }

        };
    }

[abeyecon]

Thanks for the help. Is there a forum for these sorts of questions? I feel bad creating 'issues' for them.

[codecholeric]

Sorry, we don't have a specific forum. You could use StackOverflow? There are already questions up there. Other than that, don't feel bad, we can just tag these issues as question 😃
It's mainly that I don't have the capacity to maintain a second platform, so far GitHub seemed sufficient for issues of all sorts...