auraphp / Aura.SqlQuery

Independent query builders for MySQL, PostgreSQL, SQLite, and Microsoft SQL Server.
MIT License
452 stars 86 forks source link

Scrutinizer and data types #125

Closed pavarnos closed 7 years ago

pavarnos commented 7 years ago

So i've been fiddling with Scrutinizer a bit. Got an interesting message I don't know how to resolve.

scrutinizer

where $this->database is an instance of

class Database {
    /**
     * @return QueryFactory
     */
    protected function getQueryFactory()
    {
        if (empty($this->queryFactory)) {
            $this->queryFactory = new QueryFactory($this->isSqlite() ? 'sqlite' : 'mysql');
        }
        return $this->queryFactory;
    }

    /**
     * @return UpdateInterface
     */
    public function update()
    {
        return $this->getQueryFactory()->newUpdate();
    }

Seems like Scrutinizer is looking through the @return type declaration of Auras newUpdate() to the @return type declaration of newInstance('Update').

Is this a scrutiniser problem or something Aura can fix by changing type declarations?

harikt commented 7 years ago

Hey @pavarnos ,

You have already found the problem.

https://github.com/auraphp/Aura.SqlQuery/blob/dd81b57aeb43628180a9c70a4df58d872024d7f2/src/QueryFactory.php#L191

The QueryFactory is having @return AbstractQuery , but it can be an instance of Insert, Update, Delete or Select interfaces. So the return type declared is wrong. I guess if we fix the return type, it will work as expected.

pavarnos commented 7 years ago

OK. What should the return type be?

harikt commented 7 years ago

@pavarnos I am not sure whether we can write something like this.

@return Common\SelectInterface|Common\InsertInterface|Common\UpdateInterface|Common\DeleteInterface

There is multiple return type in this case, ie why. . This will work, I have tested with phpdocumentator and apigen .

harikt commented 7 years ago

@pavarnos if you have time, do send a PR. Else I will look sending one.

Thank you.

pmjones commented 7 years ago

@harikt @pavarnos Any PRs as a result of this issue?

pavarnos commented 7 years ago

Sorry I have no time. As @harikt showed above I think it is a one line fix

harikt commented 7 years ago

PR : https://github.com/auraphp/Aura.SqlQuery/pull/135