phpstan-doctrine
phpstan-doctrine copied to clipboard
Query result type inference too generic for certain function results
- See result of functions here and here. I hope I'm not mistaken, but those functions (COUNT, SUM, AVG, MIN, MAX, LENGTH) should always return int, do they not? Those are now infered as
int<0, max>|numeric-string
. - Also, this CASE should always be int imo.
- And IDENTITY function seems to be infered as mixed even though the type of the FK should be known.
/cc @arnaud-lb
Hi @janedbal
Thank you for your detailed bug reports :)
Doctrine does a good job at consistently returning the right type for entity fields. However, for expressions or function calls it just returns the raw value from the database.
So the type of function calls and arithmetic expressions really depends on the database driver (and the driver version).
For instance, the pdo_mysql and pdo_sqlite drivers return integers and floats as strings in PHP < 8.1, or as integer/float in PHP 8.1 (unless PDO::ATTR_STRINGIFY_FETCHES is set, and unless the integer/float is too large to fit in PHP's integer/float types).
Since Doctrine 2.8, the functions COUNT and LENGTH are casted as int by Doctrine, though, so phpstan-doctrine infers these expressions as just int<0,max>
when using Doctrine >= 2.8.
For other expressions, I chose to infer types that are correct in all environments, for now. It may be possible to refine these in a future version, although it may be challenging in some cases, and also lead to type instability (it depends on too many factors that may change depending on the environment).
In short, depending on many factors including the PHP version, drivers, and driver options, integer expressions in DQL queries may be returned as int
or numeric-string
, so phpstan-doctrine
infers them as int|numeric-string
for now.
- And IDENTITY function seems to be infered as mixed even though the type of the FK should be known.
IDENTITY
is one of the few features that was not supported in the initial PR, but it should be easy to add it now.
Thank you, now I see the huge complexity you were dealing when implementing this. Good work!
Anyway, shouldnt you be able to detect all those settings from EM when provided (parameters.doctrine.objectManagerLoader)? Driver should be known, PHP version too, Doctrine version as well. Problematic might be PDO setup (if you dont want to query the database).
Anyway, shouldnt you be able to detect all those settings from EM when provided
We could definitely, but there is the question of the type stability: It can actually be valuable that inferred types don't change with the environment, as it makes the resulting code more portable. Maybe we could add a config parameter for this.
Maybe we could add a config parameter for this.
I think that would be beneficial.
it makes the resulting code more portable
That might be super important for libraries, but not for company codebase, where you use single driver and single setup. So as said, some config might be handy so that everybody could decide what is better for his codebase.