Conversation
|
Loads of string casts to be added 😅 |
12fb8e1 to
a14b808
Compare
7b7cffe to
c5e8a7c
Compare
fd275f8 to
c78e0e5
Compare
72cddfb to
d8f1786
Compare
67bd64c to
7e5fb71
Compare
|
One thing I didn't mention yet: this should have an |
b207086 to
e3373c1
Compare
We could only recommend Psalm if our own codebase passes its test. Otherwise, there will be lots of false-positives. What do you think about adopting Psalm/Phan in our CI? At a glance, it looks like a lot more work :-D |
|
I am skeptic towards Psalm, last time I saw it it required a lot of Psalm-specific annotations and stuff. :/ |
|
The end users only need to check the contact surface with DBAL, not DBAL itself, so any of these tools would guarantee decent safety. Psalm is way stricter than PHPStan, so that's up for discussion, but it is capable of much more interesting type additions: https://medium.com/vimeo-engineering-blog/uncovering-php-bugs-with-template-a4ca46eb9aeb |
253b936 to
f62131a
Compare
3850154 to
4fbe91a
Compare
a640b82 to
e7b6c16
Compare
ad01eea to
beb6031
Compare
| @@ -273,7 +275,7 @@ public function testGeneratesForeignKeySqlOnlyWhenSupportingForeignKeys() | |||
| } | |||
| } | |||
|
|
|||
| protected function getBitAndComparisonExpressionSql($value1, $value2) | |||
There was a problem hiding this comment.
I don't understand, I added typehints.
There was a problem hiding this comment.
Sorry. The change is correct. I was saying that this issue wouldn't be possible on PHP 7.1 which doesn't allow loosening argument types in subclasses. I oversaw it recently in #3498.
| $extraParameters = array_slice(func_get_args(), 3); | ||
|
|
||
| if (count($extraParameters) !== 0) { | ||
| $extraParameters[0] = $extraParameters[0] ?? 0; |
There was a problem hiding this comment.
I don't really see how we can improve here. Unlike the userland optional arguments defaulting to some value, the internal arguments are counted, so we have to respect the expected number of arguments being passed and their types.
morozov
left a comment
There was a problem hiding this comment.
Apart from the issue in OCI8Exception, I don't see anything else worth reworking.
No description provided.