Fix isFinal, isAbstract, isReadonly behaviour

[micheleorselli]

This should fix #434

TL;DR super subtle behaviour 😢: when defining a rule we specify an expression to select the classes we'd like to check, like

Rule::allClasses()
        ->that(new IsAbstract())
       ...

What happens when the runner evaluates the ->that(new IsAbstract()) is defined in the class Arkitect\Rules\Spec, in particular

public function allSpecsAreMatchedBy(ClassDescription $classDescription, string $because): bool
{
        /** @var Expression $spec */
        foreach ($this->expressions as $spec) {
            $violations = new Violations();
            $spec->evaluate($classDescription, $violations, $because);

            if ($violations->count() > 0) {
                return false;
            }
        }

        return true;
}

so here we try to evaluate the expression and if no violation is returned, then it means the class we are currently analyzing should be considered for the analysis. With the previous IsAbstract implementation the evaluate method contained this check:

if ($theClass->isAbstract() || $theClass->isInterface() || $theClass->isTrait() || $theClass->isEnum() || $theClass->isFinal()) {
 return;
}

meaning that if a class at least one of the attributes set, then it will be evaluated.

the fix in this PR reverts the check to

if ($theClass->isAbstract()) {
 return;
}

It one tests the behaviour of IsAbstract directly it may be induced to think the first one is the check needed 😢.

To avoid this kind of confusion we should probably separate the condition used to understand if a given class should be analyzed from the actual check, but this implies updating a gazillion of rules 😂

[micheleorselli]

nom nom nom, the fix is less straighforward than expected since when we use isAbstract in the ->that section like

$rule = Rule::allClasses()
            ->that(new IsAbstract())
            ->should(new HaveNameMatching('*Abstract'))
            ->because('we want to prefix abstract classes');

the current code checking only isAbstract is ok, but know if we use it in the ->should section like

$rule = Rule::allClasses()
            ->that(new ResideInOneOfTheseNamespaces('App'))
            ->should(new IsAbstract())
            ->because('we want to prefix abstract classes');

the check fails since it consider readonly and final classes 🤯

ref
CheckClassWithMultipleExpressionsTest::test_is_abstract_in_that and CheckClassWithMultipleExpressionsTest::test_is_abstract_in_should

any ideas @AlessandroMinoccheri?