DoctrineKeyValueStyeRule: allow int values in float columns #581
Conversation
do you say a regular |
Given an
But if you say otherwise, I'll take your word for it! :) |
i guess you took that sentence from the phpstan docs somewhere? If so: it describes how native phpstan rules behave according to analysis level. atm the phpstan-dba rules do not at all depend on phpstan rule levels (which does not mean that our RuntimeConfig could take inspiration of the ideas behind phpstan-src rules or implement similar ideas). i don‘t know how phpstan-dba rules work with benevolent unions atm. We would need unit tests to verify it |
hemberger
changed the title
| /** | ||
| * @param array-key $value A benevolent union type (int|string) | ||
| */ | ||
| public function errorWithBenevolentUnionValue(Connection $conn, $value) | ||
| { | ||
| $conn->assembleOneArray('typemix', ['c_int' => $value]); | ||
| } |
There was a problem hiding this comment.
I am not sure about this expectation. I would expect a error for a non-benevolent union, but I think we should be ok with a benevolent union, as long as it contains the type we expect
There was a problem hiding this comment.
I'm happy to change the behavior for benevolent unions as well. Would you be okay with me submitting it as a separate PR?
Using `$databaseType->isSuperTypeOf($valueType)` to check if the input value type matches the database column type was too strict. It was incorrectly raising errors for various legal scenarios, e.g.: * Passing an integer to a float column. * Passing a benevolent (float|int) to an int column. The `accepts` method correctly handles these cases. Add extra logic for `MixedType`, since mixed types are always accepted.
Handle the `FloatType` explicitly instead of testing with `accepts`, since that function does more than what we want. Note that this makes benevolent union types unacceptable again unless all types in the union are acceptable. I've also simplified the logic for casting an `IntegerRangeType` into an `IntegerType`, since a column type can only reasonable have a union with a `NullType`. There is a standard way to using TypeCombinator to mutate the type of a union with null.
As requested in PR review.
staabm
force-pushed
the
doctrine-accepts
branch
from
b4af7aa to
0bee888
Compare
|
thank you |
Using
$databaseType->isSuperTypeOf($valueType)to check if the input value type matches the database column type was too strict. It was incorrectly raising errors for various legal scenarios, e.g.:The
acceptsmethod correctly handles these cases. Add extra logic forMixedType, since mixed types are always accepted.NOTE: The
MixedTypecase may be demoted to a debug-mode-only error, depending on the answer to #564 (comment).