[2.x] Added toUseTrait arch expectation

[ash-jc-allen]

What:

• Bug Fix
• New Feature

Description:

Hey! This PR proposes a new toUseTrait arch expectation. There are times when I want to ensure that classes in a given directory are all using a specific trait.

For example, in my app/Actions folder, I might be using the lorisleiva/laravel-actions package which allows me to add an Lorisleiva\Actions\Concerns\AsAction trait to all my action classes. Example:

namespace App\Actions\Users;

use Lorisleiva\Actions\Concerns\AsAction;

class CreateUser
{
    use AsAction;

    // ...
}

In this situation, I'd like be to able to do something like:

test('all actions use AsAction trait')
    ->expect('App\Actions')
    ->toUseTrait(AsAction::class);

I've also added support for checking multiple at once. For example:

test('all actions use both traits')
    ->expect('App\Actions')
    ->toUseTrait([
        AsAction::class,
        MyAwesomeTrait::class,
    ]);

Running the above would ensure that all the classes in the app/Actions folder will use the AsAction trait AND the MyAwesomeTrait.

There's also the opposite expectation that you can use to make sure a given trait isn't being used:

test('no actions use AsAction trait')
    ->expect('App\Actions')
    ->not->toUseTrait(AsAction::class);

This expectation will bubble up and check all the given class' traits too. So if a class has access to a trait, but via a parent class, this should detect that and pass the expectation.

Caveats

I think the main caveat to what I've done so far is that it's not detecting traits used by other traits. For example, say we have a trait called BaseActionTrait and this trait uses the AsAction trait:

trait BaseActionTrait {
    use AsAction;

    // ...
}

Then let's say you have a class that uses the BaseActionTrait:

class MyAction {
    use BaseActionTrait;

    // ...
}

If you were to run the following test, it would fail:

test('all actions use AsAction trait')
    ->expect('App\Actions')
    ->toUseTrait(AsAction::class);

I don't know if this is an issue or not. But I didn't want to spend too much time working on it just in case it's something that might not be needed or wanted. I'm happy to have a crack at adding it though 🙂

Related:

N/A

If you think this new toUseTrait expectation would be useful, please give me a shout if there's anything you'd like me to update.

I've tried my best to test this out, but I might have missed some edges. Apologies if so! 😄

[devajmeireles]

Hey, @ash-jc-allen ! Thank you for your amazing work here.

Thank you for your amazing work here. My only concern is whether we are not doing the same thing as toUse if we accept this PR. Considering that we have toUse, this already analyzes the use of traits.

The toUse has the same caveats you mentioned: when one trait is being used within another it is not detected.

[ash-jc-allen]

Hey @devajmeireles! I'll be completely honest, I didn't know you could use toUse to check if a trait's being used haha! 😄

[devajmeireles]

I noticed something very interesting when doing this test: when the trait is being used within another trait, the test pass because the trait is only imported, without being used within the class itself.

[devajmeireles]

Hey @devajmeireles! I'll be completely honest, I didn't know you could use toUse to check if a trait's being used haha! 😄

Don't worry, it's normal. PestPhp is too big. But yes, you can use toUse to detect the trait usage too, as explained above.

[ash-jc-allen]

Ahh, so maybe this new toUseTrait will be a bit stricter and give more confidence that the trait is actually being used (and not just being imported)? 🙂

[nunomaduro]

Feel that - if anything were to be done here - would be at toUse level. Meaning that, if you pass a trait into toUse, the code would actually check if the class uses the trait, etc.

[ash-jc-allen]

@nunomaduro Yeah that's a fair point! If you're open to it, I'd like to have a go at making a PR to add that functionality to the toUse method 🙂