Skip to content
New issue

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

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

Already on GitHub? # to your account

PrivateInFinalClassRule throws false positive when using PHPUnit attribute #[Before] #845

Closed
cosmastech opened this issue Oct 2, 2024 · 0 comments · Fixed by #863
Closed
Assignees
Labels

Comments

@cosmastech
Copy link
Contributor

I'm happy to implement a solution here, just not sure of the best approach. I think the most precise solution is to check if the $node in PrivateInFinalClass has a Before attribute. Maybe this should be configurable at some point 🤷

Steps required to reproduce the problem

I have a test class written like this:

namespace Tests\Feature\Models;

use App\Models\User;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
use PHPUnit\Framework\Attributes\Before;
use PHPUnit\Framework\Attributes\CoversClass;
use Tests\TestCase;

#[CoversClass(User::class)]
final class UserTest extends TestCase
{
    use LazilyRefreshDatabase;
    #[Before(1)]
    protected function unguardModel(): void
    {
        Model::unguard();
    }

    // ... test cases here
}

Expected Result

No warning should be raised from PHPStan.

Actual Result

When running PHPStan with the PrivateInFinalClassRule, it tells me that this method can be private. However, if I convert the error to private, PHPUnit fails because Error: Call to private method Tests\Feature\Models\UserTest::unguardModels() from scope PHPUnit\Framework\TestCase.

Suggested Change

declare(strict_types=1);

/**
 * Copyright (c) 2018-2024 Andreas Möller
 *
 * For the full copyright and license information, please view
 * the LICENSE.md file that was distributed with this source code.
 *
 * @see https://github.com/ergebnis/phpstan-rules
 */

namespace Ergebnis\PHPStan\Rules\Methods;

use PhpParser\Node;
use PHPStan\Analyser;
use PHPStan\Reflection;
use PHPStan\Rules;
use PHPStan\ShouldNotHappenException;


final class PrivateInFinalClassRule implements Rules\Rule
{
    private static $attributesThatIndicateProtectedIsOk = [
        \PHPUnit\Framework\Attributes\Before::class
    ];

    public function getNodeType(): string
    {
        return Node\Stmt\ClassMethod::class;
    }

    public function processNode(
        Node $node,
        Analyser\Scope $scope,
    ): array {
        if (!$node instanceof Node\Stmt\ClassMethod) {
            throw new ShouldNotHappenException(\sprintf(
                'Expected node to be instance of "%s", but got instance of "%s" instead.',
                Node\Stmt\ClassMethod::class,
                $node::class,
            ));
        }

        /** @var Reflection\ClassReflection $containingClass */
        $containingClass = $scope->getClassReflection();

        if (!$containingClass->isFinal()) {
            return [];
        }

        if ($node->isPublic()) {
            return [];
        }

        if ($node->isPrivate()) {
            return [];
        }

        // Check attributes
        foreach($node->attrGroups as $attrGroup) {
            foreach($attrGroup->attrs as $attr) {
                if (in_array($attr->name->toString(), self::$attributesThatIndicateProtectedIsOk)) {
                    return [];
                }
            }
        }

        $methodName = $node->name->toString();

        $parentClass = $containingClass->getNativeReflection()->getParentClass();

        if ($parentClass instanceof \ReflectionClass && $parentClass->hasMethod($methodName)) {
            $parentMethod = $parentClass->getMethod($methodName);

            if ($parentMethod->isProtected()) {
                return [];
            }
        }

        $ruleErrorBuilder = Rules\RuleErrorBuilder::message(\sprintf(
            'Method %s::%s() is protected, but since the containing class is final, it can be private.',
            $containingClass->getName(),
            $methodName,
        ));

        return [
            $ruleErrorBuilder->build(),
        ];
    }
}
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants