minor symfony#32006 Fix binary operation +, - or * on string by type casting to integer (steef)

fabpot · fabpot · commit 27316a4206ee · 2019-06-13T17:39:38.000+02:00
This PR was submitted for the 4.3 branch but it was merged into the 3.4 branch instead (closes symfony#32006). Discussion ---------- Fix binary operation `+`, `-` or `*` on string by type casting to integer | Q | A | ------------- | --- | Branch? | 4.3 for bug fixes  | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT # Description After playing around with [PHPStan](https://github.com/phpstan/phpstan) I found potential improvements when we try to add, subtract or multiply on a string by type casting these values to integers. PHPStan has 8 different [levels](https://github.com/phpstan/phpstan#rule-levels) of strictness and these errors come up after level 2. ## Screenshot example ### Old ⛔️ <img width="876" alt="Screenshot 2019-06-12 at 11 49 22" src="https://user-images.githubusercontent.com/34915382/59435551-2bea0280-8dee-11e9-8eb2-954d34973382.png"> ### New ✅ <img width="876" alt="Screenshot 2019-06-12 at 11 48 54" src="https://user-images.githubusercontent.com/34915382/59435562-30aeb680-8dee-11e9-806d-d37985096363.png"> ## How to test 1. Require PHPStan in [Composer](https://getcomposer.org/): ``` composer require --dev phpstan/phpstan ``` 2. Create the following `phpstan.neon` [config](https://github.com/phpstan/phpstan#configuration) file: ``` parameters: excludes_analyse: - 'src/Symfony/Component/Form/Tests' - 'src/Symfony/Component/Cache/Tests' - 'src/Symfony/Component/DomCrawler/Tests' - 'src/Symfony/Component/HttpKernel/Tests' - 'src/Symfony/Component/PropertyAccess/Tests' - 'src/Symfony/Component/Routing/Tests' - 'src/Symfony/Component/Validator/Tests' - 'src/Symfony/Component/HttpKernel/Tests' ``` 3. Comment out line 202-204 of the following file: ``` src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php ``` 4. Analyse the project and search for binary operation errors by running: ``` vendor/bin/phpstan analyse src --level 2 -c phpstan.neon | grep --context=5 "Binary operation" ``` > Keep in mind that four errors will still popup. Three are for `+=` or `+` on arrays and the other one is about `.` between an interface and empty array which in my opinion can't be improved. Commits ------- d445465 Fix binary operation `+`, `-` or `*` on string
diff --git a/src/Symfony/Component/Form/Extension/Core/Type/BirthdayType.php b/src/Symfony/Component/Form/Extension/Core/Type/BirthdayType.php
@@ -21,7 +21,7 @@ class BirthdayType extends AbstractType
      */
     public function configureOptions(OptionsResolver $resolver)
     {
-        $resolver->setDefault('years', range(date('Y') - 120, date('Y')));
+        $resolver->setDefault('years', range((int) date('Y') - 120, date('Y')));
 
         $resolver->setAllowedTypes('years', 'array');
     }
diff --git a/src/Symfony/Component/Form/Extension/Core/Type/DateType.php b/src/Symfony/Component/Form/Extension/Core/Type/DateType.php
@@ -256,7 +256,7 @@ public function configureOptions(OptionsResolver $resolver)
         };
 
         $resolver->setDefaults([
-            'years' => range(date('Y') - 5, date('Y') + 5),
+            'years' => range((int) date('Y') - 5, (int) date('Y') + 5),
             'months' => range(1, 12),
             'days' => range(1, 31),
             'widget' => 'choice',
diff --git a/src/Symfony/Component/HttpFoundation/Response.php b/src/Symfony/Component/HttpFoundation/Response.php
@@ -707,7 +707,7 @@ public function getAge()
             return (int) $age;
         }
 
-        return max(time() - $this->getDate()->format('U'), 0);
+        return max(time() - (int) $this->getDate()->format('U'), 0);
     }
 
     /**
@@ -788,7 +788,7 @@ public function getMaxAge()
         }
 
         if (null !== $this->getExpires()) {
-            return $this->getExpires()->format('U') - $this->getDate()->format('U');
+            return (int) $this->getExpires()->format('U') - (int) $this->getDate()->format('U');
         }
     }
 
diff --git a/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php b/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php
@@ -85,7 +85,7 @@ public function add(Response $response)
         $this->storeRelativeAgeDirective('s-maxage', $response->headers->getCacheControlDirective('s-maxage') ?: $response->headers->getCacheControlDirective('max-age'), $age);
 
         $expires = $response->getExpires();
-        $expires = null !== $expires ? $expires->format('U') - $response->getDate()->format('U') : null;
+        $expires = null !== $expires ? (int) $expires->format('U') - (int) $response->getDate()->format('U') : null;
         $this->storeRelativeAgeDirective('expires', $expires >= 0 ? $expires : null, 0);
     }
 
diff --git a/src/Symfony/Component/Intl/DateFormatter/DateFormat/DayOfYearTransformer.php b/src/Symfony/Component/Intl/DateFormatter/DateFormat/DayOfYearTransformer.php
@@ -25,7 +25,7 @@ class DayOfYearTransformer extends Transformer
      */
     public function format(\DateTime $dateTime, $length)
     {
-        $dayOfYear = $dateTime->format('z') + 1;
+        $dayOfYear = (int) $dateTime->format('z') + 1;
 
         return $this->padLeft($dayOfYear, $length);
     }
diff --git a/src/Symfony/Component/Intl/DateFormatter/DateFormat/FullTransformer.php b/src/Symfony/Component/Intl/DateFormatter/DateFormat/FullTransformer.php
@@ -315,7 +315,7 @@ protected function calculateUnixTimestamp(\DateTime $dateTime, array $options)
         preg_match_all($this->regExp, $this->pattern, $matches);
         if (\in_array('yy', $matches[0])) {
             $dateTime->setTimestamp(time());
-            $year = $year > $dateTime->format('y') + 20 ? 1900 + $year : 2000 + $year;
+            $year = $year > (int) $dateTime->format('y') + 20 ? 1900 + $year : 2000 + $year;
         }
 
         $dateTime->setDate($year, $month, $day);
diff --git a/src/Symfony/Component/Validator/Constraints/IssnValidator.php b/src/Symfony/Component/Validator/Constraints/IssnValidator.php
@@ -120,7 +120,7 @@ public function validate($value, Constraint $constraint)
 
         for ($i = 0; $i < 7; ++$i) {
             // Multiply the first digit by 8, the second by 7, etc.
-            $checkSum += (8 - $i) * $canonical[$i];
+            $checkSum += (8 - $i) * (int) $canonical[$i];
         }
 
         if (0 !== $checkSum % 11) {
diff --git a/src/Symfony/Component/Validator/Constraints/LuhnValidator.php b/src/Symfony/Component/Validator/Constraints/LuhnValidator.php
@@ -83,7 +83,7 @@ public function validate($value, Constraint $constraint)
         //         ^     ^     ^     ^     ^
         //    =    1+8 + 4  +  6  +  1+6 + 2
         for ($i = $length - 2; $i >= 0; $i -= 2) {
-            $checkSum += array_sum(str_split($value[$i] * 2));
+            $checkSum += array_sum(str_split((int) $value[$i] * 2));
         }
 
         if (0 === $checkSum || 0 !== $checkSum % 10) {
diff --git a/src/Symfony/Component/VarDumper/Caster/DateCaster.php b/src/Symfony/Component/VarDumper/Caster/DateCaster.php
@@ -95,7 +95,7 @@ public static function castPeriod(\DatePeriod $p, array $a, Stub $stub, $isNeste
                 if (3 === $i) {
                     $now = new \DateTimeImmutable();
                     $dates[] = sprintf('%s more', ($end = $p->getEndDate())
-                        ? ceil(($end->format('U.u') - $d->format('U.u')) / ($now->add($p->getDateInterval())->format('U.u') - $now->format('U.u')))
+                        ? ceil(($end->format('U.u') - $d->format('U.u')) / ((int) $now->add($p->getDateInterval())->format('U.u') - (int) $now->format('U.u')))
                         : $p->recurrences - $i
                     );
                     break;

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ class BirthdayType extends AbstractType`
`21`	`21`	`*/`
`22`	`22`	`public function configureOptions(OptionsResolver $resolver)`
`23`	`23`	`{`
`24`		`- $resolver->setDefault('years', range(date('Y') - 120, date('Y')));`
	`24`	`+ $resolver->setDefault('years', range((int) date('Y') - 120, date('Y')));`
`25`	`25`
`26`	`26`	`$resolver->setAllowedTypes('years', 'array');`
`27`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -707,7 +707,7 @@ public function getAge()`
`707`	`707`	`return (int) $age;`
`708`	`708`	`}`
`709`	`709`
`710`		`- return max(time() - $this->getDate()->format('U'), 0);`
	`710`	`+ return max(time() - (int) $this->getDate()->format('U'), 0);`
`711`	`711`	`}`
`712`	`712`
`713`	`713`	`/**`
`@@ -788,7 +788,7 @@ public function getMaxAge()`
`788`	`788`	`}`
`789`	`789`
`790`	`790`	`if (null !== $this->getExpires()) {`
`791`		`- return $this->getExpires()->format('U') - $this->getDate()->format('U');`
	`791`	`+ return (int) $this->getExpires()->format('U') - (int) $this->getDate()->format('U');`
`792`	`792`	`}`
`793`	`793`	`}`
`794`	`794`
Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@ public function add(Response $response)`
`85`	`85`	`$this->storeRelativeAgeDirective('s-maxage', $response->headers->getCacheControlDirective('s-maxage') ?: $response->headers->getCacheControlDirective('max-age'), $age);`
`86`	`86`
`87`	`87`	`$expires = $response->getExpires();`
`88`		`- $expires = null !== $expires ? $expires->format('U') - $response->getDate()->format('U') : null;`
	`88`	`+ $expires = null !== $expires ? (int) $expires->format('U') - (int) $response->getDate()->format('U') : null;`
`89`	`89`	`$this->storeRelativeAgeDirective('expires', $expires >= 0 ? $expires : null, 0);`
`90`	`90`	`}`
`91`	`91`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ class DayOfYearTransformer extends Transformer`
`25`	`25`	`*/`
`26`	`26`	`public function format(\DateTime $dateTime, $length)`
`27`	`27`	`{`
`28`		`- $dayOfYear = $dateTime->format('z') + 1;`
	`28`	`+ $dayOfYear = (int) $dateTime->format('z') + 1;`
`29`	`29`
`30`	`30`	`return $this->padLeft($dayOfYear, $length);`
`31`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -315,7 +315,7 @@ protected function calculateUnixTimestamp(\DateTime $dateTime, array $options)`
`315`	`315`	`preg_match_all($this->regExp, $this->pattern, $matches);`
`316`	`316`	`if (\in_array('yy', $matches[0])) {`
`317`	`317`	`$dateTime->setTimestamp(time());`
`318`		`- $year = $year > $dateTime->format('y') + 20 ? 1900 + $year : 2000 + $year;`
	`318`	`+ $year = $year > (int) $dateTime->format('y') + 20 ? 1900 + $year : 2000 + $year;`
`319`	`319`	`}`
`320`	`320`
`321`	`321`	`$dateTime->setDate($year, $month, $day);`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ public function validate($value, Constraint $constraint)`
`120`	`120`
`121`	`121`	`for ($i = 0; $i < 7; ++$i) {`
`122`	`122`	`// Multiply the first digit by 8, the second by 7, etc.`
`123`		`- $checkSum += (8 - $i) * $canonical[$i];`
	`123`	`+ $checkSum += (8 - $i) * (int) $canonical[$i];`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`if (0 !== $checkSum % 11) {`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,7 @@ public function validate($value, Constraint $constraint)`
`83`	`83`	`// ^ ^ ^ ^ ^`
`84`	`84`	`// = 1+8 + 4 + 6 + 1+6 + 2`
`85`	`85`	`for ($i = $length - 2; $i >= 0; $i -= 2) {`
`86`		`- $checkSum += array_sum(str_split($value[$i] * 2));`
	`86`	`+ $checkSum += array_sum(str_split((int) $value[$i] * 2));`
`87`	`87`	`}`
`88`	`88`
`89`	`89`	`if (0 === $checkSum \|\| 0 !== $checkSum % 10) {`