chore: refactor to declare more types#771
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #771 +/- ##
=========================================
Coverage 98.66% 98.66%
Complexity 1757 1757
=========================================
Files 71 71
Lines 5163 5164 +1
=========================================
+ Hits 5094 5095 +1
Misses 69 69 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| */ | ||
| #[\ReturnTypeWillChange] | ||
| public function offsetExists($offset): bool | ||
| public function offsetExists(mixed $offset): bool |
There was a problem hiding this comment.
These don't look so good. The PHPdoc used to say that $offset is an int
But the parent ArrayAccess::offsetExists(mixed $offset): boolallows anything as parameter 1. So we can't writepublic function offsetExists(int $offset): bool`
So we seem to lose something here, the PHPdoc type hint goes, but we can't preserve it in the actual parameter type declaration.
| $param->addValue(1); | ||
| $param->addValue('1'); |
There was a problem hiding this comment.
IMO addValue was supposed to always expect to be passed a string.
In this PR I enforced that. But this test was passing int literals to addValue
| @@ -88,10 +88,8 @@ public static function guessParameterNameByValue(string $value): string | |||
| * Updates the current value. | |||
| * | |||
| * This may be either a single, or multiple strings in an array. | |||
There was a problem hiding this comment.
| * This may be either a single, or multiple strings in an array. | |
| * This may be either a single, or multiple strings in an array. | |
| * @param string|list<string> $value |
I would keep the type just make it more correct
| /** | ||
| * Sets up the object. | ||
| * | ||
| * It's recommended to use the create:: factory method instead. |
There was a problem hiding this comment.
| * It's recommended to use the create:: factory method instead. | |
| * It's recommended to use the create:: factory method instead. | |
| * | |
| * @param string|list<string>|null $value |
| @@ -32,19 +34,15 @@ class Parameter extends Node implements \Stringable | |||
|
|
|||
| /** | |||
| * Parameter value. | |||
There was a problem hiding this comment.
| * Parameter value. | |
| * Parameter value. | |
| * | |
| * @var string|list<string>|null |
2 participants
Trying some changes to type declarations now that PHP 8.2 is the minimum.
I thought that it might be easy to remove lots of
ReturnTypeWillChange.But it turns out to not always be simple. See a few comments below.