#26: Add Sniff for Getters not change state

[larsroettig]

see #26

[lenaorobei]

Could you please commit ruleset.xml as well.

[larsroettig]

@lenaorobei done 😸

Need to take property caching into account.

[larsroettig]

Hi @lenaorobei,
if we change this check to the only method should change the class property. We break a very typical Facade design pattern this will not work any more or marked as wrong but it can be valid.

Conditions:

• My SizeService has very complex logic set and get sizes very expensively.

Possbily Solutions

1. We should check for the if block is in there assignment this is okay caching reason.
2. We ignore with set or __construct prefix for property assignments check.

See my Example:

<?php

namespace Foo\Bar;

class SizeService
{
    /**
     * @return int
     */
    public function getSize()
    {
        // some implementation
        return 12;
    }

    /**
     * @param $size
     * @return void
     */
    public function setSize($size)
    {
        // some implementation
    }
}

interface SizeServiceInterface
{
    /**
     * @return int
     */
    public function getSize();

    /**
     * @param $size
     * @return void
     */
    public function setSize($size);
}


class SizeServiceFacade implements SizeServiceInterface
{
    /**
     * @var int
     */
    private $size;

    /**
     * @var SizeService
     */
    private $service;

    /**
     * @param SizeService $service
     */
    public function __construct(SizeServiceInterface $service)
    {
        $this->service = $service;
    }

    /**
     * @return int
     */
    public function getSize()
    {
        if ($this->size === null) {
            $this->size = $this->service->getSize();
        }

        return $this->size;
    }

    /**
     * @return void
     */
    public function setSize($size)
    {
        $this->size = $size;
        $this->service->setSize($size);
    }
}

[larsroettig]

Hi @paliarush and @lenaorobei,
I think we get too many false positives it is very hard to Implementing this with tokens maybe with phpstan it is possible to implement this check.

Testcase:

<?php

namespace Foo\Bar;


abstract class Bar
{
    public static $foobar = 100;
}

class Foo extends Bar
{
    /**
     * @var int
     */
    private static $staticProperty;

    /**
     * @var int
     */
    private $property;

    /**
     * @return int
     */
    public function getStaticProperty()
    {
        self::$staticProperty = 12;
        static::$staticProperty -= 12;
        self::$staticProperty .= 12;
        return self::$staticProperty;
    }

    /**
     * @return int
     */
    public function getProperty()
    {
        if (true) {

        }

        $this->property = 1223;
        return $this->property;
    }

    /**
     * @return int
     */
    public function getPropertyCached()
    {
        if ($this->property === null) {
            $this->property = 1223;
        }

        return $this->property;
    }

    public function getPropertyLocal()
    {
        $local = $this->property;
        $localArray = [
            'payment' => [
                'test' => [
                    'isActive' => $this->config->isActive(),
                    'title' => $this->config->getTitle()
                ]
            ]
        ];
        return $this->property;
    }

    private function getSalesChannelForOrder($order)
    {
        $websiteId = (int)$order->getStore()->getWebsiteId();
        $websiteCode = $this->websiteRepository->getById($websiteId)->getCode();

        return $this->salesChannelFactory->create([
            'data' => [
                'type' => '',
                'code' => $websiteCode
            ]
        ]);
    }

    const MODE_AUTO = 0;

    const MODE_MANUAL = 1;

    public function getOptionsArray()
    {
        return [
            self::MODE_AUTO => __('Automatically'),
            self::MODE_MANUAL => __('Manually')
        ];
    }

    public function testigetFoo()
    {
        $this->property = 1223;
        return $this->property;
    }

    /**
     * @return int
     */
    public function normalMethod()
    {
        $localVariable = 12;
        return $localVariable;
    }

    public function getStorageModel($storage = null, $params = [])
    {
        if ($storage === null) {
            $storage = $this->_coreFileStorage->getCurrentStorageCode();
        }

        switch ($storage) {
            case self::STORAGE_MEDIA_FILE_SYSTEM:
                $model = $this->_fileFactory->create();
                break;
            default:
                return false;
        }

        if (isset($params['init']) && $params['init']) {
            $model->init();
        }

        return $model;
    }
}

$d = function ($test) {
    $test = 123;
};