Skip to content
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 binary operation `+`, `-` or `*` on string by type casting to integer #32006

Merged
merged 1 commit into from Jun 13, 2019

Conversation

Projects
None yet
4 participants
@steef
Copy link

commented Jun 12, 2019

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 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 of strictness and these errors come up after level 2.

Screenshot example

Old ⛔️

Screenshot 2019-06-12 at 11 49 22

New

Screenshot 2019-06-12 at 11 48 54

How to test

  1. Require PHPStan in Composer:
composer require --dev phpstan/phpstan
  1. Create the following phpstan.neon config 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'
  1. Comment out line 202-204 of the following file:
src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php
  1. 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.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

Is that the only one in the full code base? Can you have a look please?

@steef

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

Good one, will have a look!

@steef steef force-pushed the steef:fix-subtract-on-string branch from a8ad557 to 523ffac Jun 13, 2019

@steef

This comment has been minimized.

Copy link
Author

commented Jun 13, 2019

@nicolas-grekas I've updated the PR! For now I only focussed on the form component as this is my first contribution to an open source project ever 😀

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

Thanks :)
Please check all components, there's no hurry so you can take your time and we'll help review!

@steef

This comment has been minimized.

Copy link
Author

commented Jun 13, 2019

Alright, will do!

@steef steef force-pushed the steef:fix-subtract-on-string branch 3 times, most recently from 1eacc3f to 9973201 Jun 13, 2019

@steef steef changed the title Fix binary operation `-` on string by type casting to integer Fix binary operation `+`, `-` or `*` on string by type casting to integer Jun 13, 2019

@steef

This comment has been minimized.

Copy link
Author

commented Jun 13, 2019

@nicolas-grekas I've checked all components and updated the PR! But I now see that Travis CI for PHP: 7.2 and 7.3 is failing with out of memory errors. Rerunning doesn't fix the problem. Do you have an idea?

@nicolas-grekas
Copy link
Member

left a comment

(for 3.4)

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jun 13, 2019

@fabpot

fabpot approved these changes Jun 13, 2019

Fix binary operation `+`, `-` or `*` on string
By type casting to integer.

@fabpot fabpot changed the base branch from 4.3 to 3.4 Jun 13, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

Thank you @steef.

@fabpot fabpot force-pushed the steef:fix-subtract-on-string branch from 9973201 to d445465 Jun 13, 2019

@fabpot fabpot merged commit d445465 into symfony:3.4 Jun 13, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Jun 13, 2019

minor #32006 Fix binary operation `+`, `-` or `*` on string by type c…
…asting to integer (steef)

This PR was submitted for the 4.3 branch but it was merged into the 3.4 branch instead (closes #32006).

Discussion
----------

Fix binary operation `+`, `-` or `*` on string by type casting to integer

| Q             | A
| ------------- | ---
| Branch?       | 4.3 for bug fixes <!-- see below -->
| 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.