Verify a method is only called from Constructors or Deprecated methods

[Rocketeer007]

So, this is a bit esoteric, and I can’t quite figure out if it’s even possible.

In the project I’m working on at the moment, I have a legacy mechanism for dependency injection that I’m gradually refactoring out. This mechanism is based on calling a particular static method on a class - let’s say MyInjector.getInstance(SomeClass.class)

I would like to write a rule that says that this method can only be called from Constructors, or from @Deprecated methods. This way I can start a process of moving from ad-hoc injection to Constructor injection - and eventually move entirely away from this legacy system.

I’ve managed to create a simple rule that says “Only classes that are deprecated should access MyInjector.getInstance“, but I can’t figure out any way of saying it should only be called from methods that are @Deprecated...

Is what I’m trying to do even possible?

[codecholeric]

I think you have to extend the API a little with a custom ArchCondition, but maybe something like this?

class DemoTest {
    @Test
    void demo() {
        JavaClasses classes = new ClassFileImporter().importClasses(Okay.class, NotOkay.class, MyInjector.class);

        methods().that().areDeclaredIn(MyInjector.class)
                .and().haveName("getInstance")
                .should(onlyBeCalledByDeprecatedMethodsOrConstructors()).check(classes);
    }

    private ArchCondition<JavaMethod> onlyBeCalledByDeprecatedMethodsOrConstructors() {
        return new ArchCondition<JavaMethod>("only be called by @Deprecated methods or constructors") {
            @Override
            public void check(JavaMethod method, ConditionEvents events) {
                method.getCallsOfSelf().stream()
                        .filter(call -> !call.getOrigin().isConstructor())
                        .filter(call -> !call.getOrigin().isAnnotatedWith(Deprecated.class))
                        .forEach(call -> events.add(SimpleConditionEvent.violated(method, call.getDescription())));
            }
        };
    }

    static class Okay {
        private final Object fromConstructor;
        private Object fromMethod;

        Okay() {
            fromConstructor = MyInjector.getInstance(Object.class);
        }

        @Deprecated
        void alsoOkay() {
            fromMethod = MyInjector.getInstance(Object.class);
        }
    }

    static class NotOkay {
        private Object fromMethodWithoutAnnotation;

        void fromWrongMethod() {
            fromMethodWithoutAnnotation = MyInjector.getInstance(Object.class);
        }
    }

    static class MyInjector {
        static <T> T getInstance(Class<T> type) {
            return null;
        }
    }
}

[codecholeric]

Thinking about it, it would be nice to extend the members()... API for methods like onlyBeAccessedBy()... and codeUnits()...should().onlyBeCalledBy()... 😉

[Rocketeer007]

Thank you, that's really helpful - worked perfectly :)

[codecholeric]

Happy to hear that this solves your problem 😄
Can I close this issue? Or is there any other problem?

[perlun]

@codecholeric - thanks a lot for the example provided in this GitHub issue, it helped me do a similar things just now. 🙏

I wonder if there would be a good place in the ArchUnit docs where we could mention this, making it easier to find for the times when you need to do something like this. Maybe adding it to ArchUnit-Examples would be an idea? 🤔

[codecholeric]

@perlun sure, we could think about it! The original issue here just seemed so super specific to me, that I thought it would be a little weird as part of the examples 🤔 But maybe we can find an example that is a little bit more generic and self-motivated? Would the issue you solved for yourself right now be something that could be generified a little to be put into the examples?

[perlun]

@codecholeric I gave this a try now, please let me know what you think about it: TNG/ArchUnit-Examples#7. It matches our use case pretty closely and also illustrates how certain classes/packages can easily be whitelisted as needed. (both with specific .filter() rules to always allow certain types of calls, but also by providing a parameter which can be overridden from individual tests.)