Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Add a beCalledInClassesThat conjunction #704

Open
cpoissonnier opened this issue Oct 27, 2021 · 2 comments
Open

Feature request: Add a beCalledInClassesThat conjunction #704

cpoissonnier opened this issue Oct 27, 2021 · 2 comments

Comments

@cpoissonnier
Copy link

I wanted to implement a rule like this one :

@ArchTest
    static final ArchRule events_must_be_created_by_aggregates =
            noConstructors().that().areDeclaredInClassesThat().areAssignableTo(Event.class).should().beCalledInClassesThat().areNotAssignableFrom(Aggregate.class)
                    .because("the aggregate should manage its own lifecycle and events");

But it seems that it is now possible without custom predicate.
Is that something that could be added in the ready-to-use conjunctions ?

For reference, the related Stackoverflow question : https://stackoverflow.com/questions/69723542/how-to-check-that-a-constructor-is-called-in-the-right-classes-with-archunit/69728892#69728892

@codecholeric
Copy link
Collaborator

Yes, I think this surely makes sense to be added 🙂
I think it should in fact be added to CodeUnitsShould, so it can be automatically used for methods()...should() and constructors()...should() (and semantically it makes sense for both). And maybe beCalledByClassesThat is a tiny bit nicer as a name 🤔 🤷‍♂️

Of course we could even go a step further and make it

should().beCalledBy().{classesThat()/methodsThat()/constructorsThat()}

But managing expectations: If it is up to me to implement this, it will take quite some time as my ArchUnit task queue is always at the limit 😉 But if anyone wants to jump in and create a PR, I think this would be a nice addition 😄

@JKLedzion
Copy link
Contributor

Hi, I’ve started working on this issue (hope it’s Ok and it’s still open) and I’ve added a PR with initial part which I’ve done. Work is in progress but it would be great to receive some feedback if it goes in right direction.

I’ve started from adding method onlyBeCalledByClassesThat(DescribedPredicate predicate) to CodeUnitsShould.
Now I’m working on possibility to have a rule like:
should().onlyBeCalled().{byClassesThat()/methodsThat()/constructorsThat()}.

Could you also let me know if my understanding of the issue is correct for below points?

  1. I’ve implemented condition “code unit should only be called by”, but few possibilities came to my mind, not sure if I’ve decided on the correct one:
    • code unit should be called only by classes that,
    • code unit should be called by at least one class that,
    • code unit should be called by all classes that (but can be called also by other)
    • code unit should be called by all classes that (and can’t be called by any other).
  2. should().onlyBeCalled().byMethodsThat() - here I’m checking if methods calling selected code unit meet provided criteria but still this code unit can be called by constructors not meeting criteria. Is it correct?

I would appreciate any feedback.
Justyna

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants