skip to Main Content

I’m trying to write a custom phpstan rule to not allow other devs to call Cart::save() ($cart->save()) from everywhere in the codebase, but to call only from CartRepository, and if it’s called outside of repository I want to see an error like Calling Cart::save() method outside of CartRepository class is not allowed.

I tried to do something like this

class CartSaveRule implements Rule
{
    public function getNodeType(): string
    {
        return MethodCall::class;
    }

    /**
     * @param MethodCall $node
     * @param Scope $scope
     * @return array
     */
    public function processNode(Node $node, Scope $scope): array
    {
        // Check if the method call is to the "save" method
        if ($node->name->toString() === 'save') {
            $className = $scope->getClassReflection()->getName();

            // Check if the method call is made from the Product class
            if ($className === 'Cart') {
                // Get the method call location
                $line = $node->getLine();
                $file = $scope->getFile();

                // Check if the method call is not made from inside the ProductRepository class
                if (!$this->isCalledFromProductRepository($scope)) {
                    return [
                        RuleErrorBuilder::message('Calling save() method of Cart class outside of CartRepository class is not allowed.')
                            ->line($line)
                            ->file($file)
                            ->build()
                    ];
                }
            }
        }

        return [];
    }


    private function isCalledFromRepository(Scope $scope): bool
    {
        // Check if the calling class is CartRepository or its subclass
        return  $scope->getClassReflection()->isSubclassOf('CartRepository');
    }
}

but it’s not working as I expect, let’s say in Cart I have a method :

    private function updateVariant()
    {
        $variant =  Variant::
            ->where('id', '=', $this->variantId)
            ->first();
        
        // logic to update variant        

        $variant->save();
    }

and my rule will scream on line $variant->save(); because save is called by Cart, but it’s not Cart related save

2

Answers


  1. Why not just override the save() method of the cart instead? Something like this:

    <?php
    
    class Cart
    {
        public function save(object $repo = null)
        {
            if ($repo === null || !$repo instanceof CartRepository) {
                throw new Exception('Save the cart using the CartRepository' . "n");
            }
            
            $repo->save($this);
        }
    }
    
    class CartRepository
    {
        public function save(Cart $cart)
        {
            return 'saved!';
        }
    }
    
    $repo = new CartRepository();
    $cart = new Cart();
    
    try {
        $cart->save();
    } catch (Exception $e) {
        echo $e->getMessage();
    }
    
    echo $repo->save($cart);
    

    This will output:

    Save the cart using the CartRepository
    
    saved!
    

    You can try it here https://3v4l.org/uHW1e

    Login or Signup to reply.
  2. This is a really good idea for a custom PHPStan rule! It will let static analysis catch something you don’t want in your codebase, and that’s always great!

    The thing you need to care about is the type of $node->var (MethodCall::$var).

    There are several ways how to achieve what you want. One of them is Querying a specific type with Type::isSuperTypeOf():

    $calledOnType = $scope->getType($node->var);
    if ((new ObjectType(Cart::class)->isSuperTypeOf($calledOnType)->yes())) {
      ...
    }
    

    The problem with this is that isSuperTypeOf is not going to work for example on Cart|null.

    There’s a practical shortcut to what you need: Scope::getMethodReflection():

    $calledOnType = $scope->getType($node->var);
    $method = $scope->getMethodReflection($calledOnType, 'save');
    if ($method === null) {
        return []; // method not found
    }
    
    if ($method->getDeclaringClass()->getName() !== Cart::class) {
        return []; // method "save" from different class
    }
    
    // check `$scope->getClassReflection()` here and report an error
    

    Scope::getMethodReflection() will find the method reflection even on a nullable object.

    I wish you luck with your custom rules 🙂

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search