kalessil / yii2inspections

MIT License
31 stars 3 forks source link

[fixed, bug] “Missing @property annotations” inspection is ignoring getters #15

Closed brandonkelly closed 7 years ago

brandonkelly commented 7 years ago

Example:

<?php
namespace craft;

use yii\base\Object;

/**
 * Class Foo
 */
class Foo extends Object
{
    private $_bar = 'bar';

    /**
     * @return string
     */
    public function getBar()
    {
        return $this->_bar;
    }
}

The “Missing @property annotations” inspection should be saying this class is missing a $bar property, but it doesn’t. It appears to only be checking for setters.

If you add this method:

    /**
     * @param string $bar
     */
    public function setBar(string $bar)
    {
        $this->_bar = $bar;
    }

the inspection will work as expected.

kalessil commented 7 years ago

Then perhaps we should change the behaviour:

The current implementation requires complimentary getProperty and setProperty existence. The existence of $_property is not checked.

brandonkelly commented 7 years ago

I wouldn't worry about whether a private $_property exists at all.

These are the checks it should have:

For determining the type, it should be a combination of all the unique types from:

For example:

<?php

namespace craft;

use yii\base\Object;

/**
 * Class Foo
 */
class Foo extends Object
{
    private $_bar;

    /**
     * @return string|null
     */
    public function getBar()
    {
        return $this->_bar;
    }

    /**
     * @param string $bar
     */
    public function setBar(string $bar)
    {
        $this->_bar = $bar;
    }
}

should get this @property annotation:

@property string|null $type
kalessil commented 7 years ago

Oh, I see (ONE or BOTH), will fix and extend test cases

brandonkelly commented 7 years ago

Not sure if this warrants a new issue, but in that example above, clicking the “Annotate properties” button adds this:

 * @property \null|\string $bar

Those \s should not be in there.

kalessil commented 7 years ago

@property \null|\string $bar : fixed, working on the ONE or BOTH approach in the inspection

kalessil commented 7 years ago

Now checks ONE or BOTH, with settings for using only BOTH approach.