-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Fix redundant type casts #44274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix redundant type casts #44274
Conversation
e390092 to
6158331
Compare
| protected function renderForm(FormView $view, array $vars = []) | ||
| { | ||
| return (string) $this->renderer->renderBlock($view, 'form', $vars); | ||
| return $this->renderer->renderBlock($view, 'form', $vars); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FormRendererInterface::renderBlock() retturns a string
| } | ||
|
|
||
| return (string) $this->renderer->searchAndRenderBlock($view, 'label', $vars); | ||
| return $this->renderer->searchAndRenderBlock($view, 'label', $vars); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FormRendererInterface::searchAndRenderBlock() retturns a string (same for the others)
|
|
||
| private function getStorage(): StringStorage | ||
| { | ||
| return $this->getMockBuilder(StringStorage::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced the mock with a real instance.
| { | ||
| if ($methods) { | ||
| $methods = array_map('strtoupper', (array) $methods); | ||
| $methods = array_map('strtoupper', $methods); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
array $methods in the method args
| */ | ||
| private function convertValueDataType($value, int $type) | ||
| { | ||
| $type = (int) $type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int $type in the args
| ['prefix1', false, '->parse() does not parse a number with a string prefix.', 0, false], | ||
| ['1.4suffix', (float) 1.4, '->parse() parses a number with a string suffix.', 3], | ||
| ['1.4suffix', (float) 1.4, '->parse() parses a number with a string suffix.', 3, false], | ||
| ['1.4suffix', 1.4, '->parse() parses a number with a string suffix.', 3], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.4 is already a float
| } | ||
|
|
||
| $score = (int) ($this->getCurrentTimeInMilliseconds() + $delayInMs); | ||
| $score = $this->getCurrentTimeInMilliseconds() + $delayInMs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getCurrentTimeInMilliseconds() returns an int, int $delayInMs = 0 in the args
| public function __toString() | ||
| { | ||
| return (string) $this->template; | ||
| return $this->template; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The $template property is set by the constructor with the string $template arg.
| case '+' === $scalar[0] || '-' === $scalar[0] || '.' === $scalar[0] || is_numeric($scalar[0]): | ||
| if (Parser::preg_match('{^[+-]?[0-9][0-9_]*$}', $scalar)) { | ||
| $scalar = str_replace('_', '', (string) $scalar); | ||
| $scalar = str_replace('_', '', $scalar); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$scalar comes from $scalar = trim($scalar);, trim returns a string.
6158331 to
e724d5a
Compare
|
|
||
| /** | ||
| * @return string | ||
| * @return string|false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, right now, we guarantee a string, so this is a BC break.
|
Thank you @fancyweb. |
This PR was merged into the 5.3 branch. Discussion ---------- Fix redundant type casts | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Continuation of #44274 on 5.3 Commits ------- 4243335 Fix redundant type casts
This PR was merged into the 5.3 branch. Discussion ---------- Fix redundant type casts | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Continuation of symfony/symfony#44274 on 5.3 Commits ------- 4243335012 Fix redundant type casts
This PR was merged into the 5.3 branch. Discussion ---------- Fix redundant type casts | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Continuation of symfony/symfony#44274 on 5.3 Commits ------- 4243335012 Fix redundant type casts
Firstly, let's see if the CI is green as I expect it. I'll explain every change with a review comment.