Question about Doctrine/DBAL

[staabm]

Question about Doctrine/DBAL

[staabm]

this error occurs while checking this class: shopware/shopware@5.7/engine/Shopware/Core/sBasket.php (yes, it is a monster 🙈 )

I thinkt this monster should be fixed with #152

[staabm]

• Another one is this with DBAL shopware/shopware@5.7/tests/Functional/Controllers/Backend/OrderTest.php#L390-L410

should be fixed by #153

[staabm]

One question regarding DBAL. having this code:

$test = $myDbalConnection->executeQuery('SELECT name, active FROM s_articles')->fetchAllAssociative();
$id = $test[0]['id'];

Shouldn't the plugin detect, that id cannot be part of the result? 🤔

@mitelg with todays release this should work

[mitelg]

hey @staabm

I tried your current master on our project https://github.com/shopware/shopware

there were some issues found and also some errors during the execution 😁

I guess these things need to be fixed on your side:

• \Doctrine\DBAL\Connection::query returns \Doctrine\DBAL\Driver\Statement and not Doctrine\DBAL\Result. I get errors like this: Call to an undefined method Doctrine\DBAL\Result<array<int|string, int>>::fetchColumn().
• is it intentional that \PDOStatement::fetch does not return false anymore?
• the counting of the query parameter is not correct sometimes
  • it seems to be case while using \PDO::prepare, then \PDOStatement::bindValue and then \PDOStatement::execute without parameter
  • Another one is this with DBAL https://github.com/shopware/shopware/blob/5.7/tests/Functional/Controllers/Backend/OrderTest.php#L390-L410

this error occurs while checking this class: https://github.com/shopware/shopware/blob/5.7/engine/Shopware/Core/sBasket.php (yes, it is a monster 🙈 )

Uncaught staabm\PHPStanDba\DbaException: Unexpected expression type PHPStan\Type\ArrayType in /home/mt/www/5.7dev/vendor/staabm/phpstan-dba/src/QueryReflection/QuerySimulation.php:72
#0 /home/mt/www/5.7dev/vendor/staabm/phpstan-dba/src/QueryReflection/QueryReflection.php(181): staabm\PHPStanDba\QueryReflection\QuerySimulation::simulateParamValueType()
#1 /home/mt/www/5.7dev/vendor/staabm/phpstan-dba/src/Rules/SyntaxErrorInPreparedStatementMethodRule.php(100): staabm\PHPStanDba\QueryReflection\QueryReflection->resolveParameters()
#2 /home/mt/www/5.7dev/vendor/staabm/phpstan-dba/src/Rules/SyntaxErrorInPreparedStatementMethodRule.php(80): staabm\PHPStanDba\Rules\SyntaxErrorInPreparedStatementMethodRule->checkErrors()
#3 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php(94): staabm\PHPStanDba\Rules\SyntaxErrorInPreparedStatementMethodRule->processNode()
#4 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Node/ClassStatementsGatherer.php(95): PHPStan\Analyser\FileAnalyser->PHPStan\Analyser\{closure}()
#5 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(436): PHPStan\Node\ClassStatementsGatherer->__invoke()
#6 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(2316): PHPStan\Analyser\NodeScopeResolver::PHPStan\Analyser\{closure}()
#7 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(1369): PHPStan\Analyser\NodeScopeResolver->callNodeCallbackWithExpression()
#8 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(1385): PHPStan\Analyser\NodeScopeResolver->processExprNode()
#9 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(2563): PHPStan\Analyser\NodeScopeResolver->PHPStan\Analyser\{closure}()
#10 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(1393): PHPStan\Analyser\NodeScopeResolver->processAssignVar()
#11 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(488): PHPStan\Analyser\NodeScopeResolver->processExprNode()
#12 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(289): PHPStan\Analyser\NodeScopeResolver->processStmtNode()
#13 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(451): PHPStan\Analyser\NodeScopeResolver->processStmtNodes()
#14 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(289): PHPStan\Analyser\NodeScopeResolver->processStmtNode()
#15 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(535): PHPStan\Analyser\NodeScopeResolver->processStmtNodes()
#16 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(260): PHPStan\Analyser\NodeScopeResolver->processStmtNode()
#17 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php(181): PHPStan\Analyser\NodeScopeResolver->processNodes()
#18 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/Analyser.php(69): PHPStan\Analyser\FileAnalyser->analyseFile()
#19 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Command/AnalyserRunner.php(62): PHPStan\Analyser\Analyser->analyse()
#20 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Command/AnalyseApplication.php(163): PHPStan\Command\AnalyserRunner->runAnalyser()
#21 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Command/AnalyseApplication.php(95): PHPStan\Command\AnalyseApplication->runAnalyser()
#22 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/src/Command/AnalyseCommand.php(173): PHPStan\Command\AnalyseApplication->analyse()
#23 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Command/Command.php(259): PHPStan\Command\AnalyseCommand->execute()
#24 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(848): _PHPStan_13047ff22\Symfony\Component\Console\Command\Command->run()
#25 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(259): _PHPStan_13047ff22\Symfony\Component\Console\Application->doRunCommand()
#26 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(157): _PHPStan_13047ff22\Symfony\Component\Console\Application->doRun()
#27 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan(94): _PHPStan_13047ff22\Symfony\Component\Console\Application->run()
#28 phar:///home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan(95): _PHPStan_13047ff22\{closure}()
#29 /home/mt/www/5.7dev/vendor/phpstan/phpstan/phpstan(8): require('...')
#30 /home/mt/www/5.7dev/vendor/bin/phpstan(97): include('...')
#31 {main}

I already found 2-3 things that really needs a fix on our side and that is a good thing 👍 nice work 💪

One downside is that the minimum requirement is PHP8, while we are still on PHP7.4 (on which the PHPStan check is running). And of course the the exceptions while checking some classes 😉

So right now, we can not use your extension, but:

[mitelg]

hmm the sources tell me a different story
oh, I forgot to mention that we use version 2.13 🙈 it is like this in version 3.x

✔️ for the result mode

I will have a look for some examples. for the errors I can not promise that, because the debug mode does not really says which code place is causing an exception. but once I have time again, I will have another look on that

[staabm]

because the debug mode does not really says which code place is causing an exception.

with #164 we added new phpstan errors which get reported when a query cannot be analyzed because the involved types are not known.

[staabm]

Hi.

thx for the feedback.

whats already possible today is re-using of SyntaxErrorInPreparedStatementMethodRule to get syntax error checking capabilities for queries which are known at analysis time - see readme

when time allows I am planning to add this by default with #66 (I don't use doctrine/dbal myself, so priorties are on other stuff)

---

what could be done on top of that is making phpstan aware of methods/functions/types involved while building query strings.
for example, if we would have phpstan extensions for the query builder api, queries like

$queryBuilder
    ->select('id', 'name')
    ->from('users')
    ->where('email = ?')
    ->setParameter(0, $userInputEmail)
;

could also be analyzable. to make this happen, phpstan needs to be able to resolve the following example

$query = $queryBuilder
    ->select('id', 'name')
    ->from('users')
    ->where('email = ?')->toString();

PHPStan\Testing\assertType('SELECT id, name FROM users WHERE email = ?', $query);

I am in the process of doing similar things for the REDAXO CMS and their rex_sql API in redaxo/redaxo#4984

---

Or are your interfered types again replaced by the annotations and types of DBAL itself?

as phpstan-dba already works on the schema available in the database, I don't see value atm in reading the doctrine annotations. I think, if we would have the above mentioned steps implemented this might change though.

[mitelg]

that looks promising 👍

just FYI: https://github.com/phpstan/phpstan-doctrine/blob/master/src/Type/Doctrine/DBAL/QueryBuilder/QueryBuilderExecuteMethodExtension.php
in the PHPStan doctrine plugin, there is already a small evaluation of the Querybuilder methods, but it is only about what is execute is giving back.

I will try your extension, once I will find some time 😁

[staabm]

just implemented this idea with zend framework 1 query builder:
https://twitter.com/markusstaab/status/1482999683911987201

[staabm]

thx for the feedback @mitelg

• \Doctrine\DBAL\Connection::query returns \Doctrine\DBAL\Driver\Statement and not Doctrine\DBAL\Result

hmm the sources tell me a different story 🤔

• is it intentional that \PDOStatement::fetch does not return false anymore?

thats a setting on the RuntimeConfiguration.

since php8 started throwing exceptions it no longer returns false.

with #148 you can explicitly opt-in that you want to handle false returns.

• it seems to be case while using \PDO::prepare, then \PDOStatement::bindValue and then \PDOStatement::execute without parameter

hmm could you create a small reproducing example ? bindValue is not yet supported though.. but maybe it false-positives atm

One downside is that the minimum requirement is PHP8, while we are still on PHP7.4 (on which the PHPStan check is running).

maybe I can provide a php 7.4 built sometime in the future. nevertheless I feel static analysis should use the latest php version, but phpstan should be configured for a lower php version

And of course the the exceptions while checking some classes

these should be fixed of course. reproducing examples would be awesome.

thank you so much

[Seldaek]

I tried it out too, no error reported (except for #156), but also no type inference for this code here for example:

https://github.com/composer/packagist/blob/e3f76f42980e6477e453f13526e33ed174318ff2/src/Entity/DownloadRepository.php#L25-L37

I would have expected some type inference to happen for the result, but \PHPStan\dumpType of $stmt and $result outputs:

     Dumped type: Doctrine\DBAL\Result
     Dumped type: array<int, array<string, mixed>>

Maybe I missed something dumb, I am not sure, I had a long day :)

In second thought I suppose

phpstan-dba/config/dba.neon

Line 11 in 06e607a

- 'Doctrine\DBAL\Connection::executeQuery'

is only checking input query/args, and not about the output. So probably this is normal and just not done yet.

[staabm]

thank you for the early feedback

I would have expected some type inference to happen for the result

Doctrine Type inference atm works only for the cases mentioned in https://github.com/staabm/phpstan-dba/blob/main/tests/data/doctrine-dbal.php

is only checking input query/args, and not about the output. So probably this is normal and just not done yet.

correct

[staabm]

I tried it out too, no error reported (except for #156

this should now be possible on the main branch with #191

[staabm]

today I landed a lot of type inferrence stuff for doctrine dbal. its not yet in a release though:

https://twitter.com/markusstaab/status/1485239838630715398

[mitelg]

I just tested version 0.2.6 and it worked well 👌

the bindValue thing still occurs, for example here: https://github.com/shopware/shopware/blob/5.7/engine/Shopware/Components/Session/PdoSessionHandler.php#L326

One question regarding DBAL. having this code:

$test = $myDbalConnection->executeQuery('SELECT name, active FROM s_articles')->fetchAllAssociative();
$id = $test[0]['id'];

Shouldn't the plugin detect, that id cannot be part of the result? 🤔

[staabm]

I just tested version 0.2.6 and it worked well

really cool, thx for the feedback.

the bindValue thing still occurs, for example here: shopware/shopware@5.7/engine/Shopware/Components/Session/PdoSessionHandler.php#L326

just opened #199 so you can subscribe the issue and get notified when its supported (and I will not forget)

Shouldn't the plugin detect, that id cannot be part of the result?

correct. this will work in the future. you can see what works atm in https://github.com/staabm/phpstan-dba/blob/main/tests/data/doctrine-dbal.php

#197 lists all apis which are not yet supported in type inference.

[mitelg]

Nice, thank you 👍

I will close this issue, so we can talk about dedicated problems in own issues 😉 👍