-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[PHP] Upgrade php-cs-fixer to 2.12, enables PHP >= 7.2 support #769
Conversation
@@ -58,7 +58,7 @@ class ObjectSerializer | |||
if ($value !== null | |||
&& !in_array($openAPIType, [{{&primitives}}], true) | |||
&& method_exists($openAPIType, 'getAllowableEnumValues') | |||
&& !in_array($value, $openAPIType::getAllowableEnumValues())) { | |||
&& !in_array($value, $openAPIType::getAllowableEnumValues(), true)) { |
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.
Can you explain, please, how this change is related to phpcs
fixer?
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.
php-cs-fixer
raises a strict-param
warning for in_array calls where the $strict
parameter is false (which is the default). Passing true ensures we use strict comparison, i.e. value + type checking.
From my reading of the code I couldn't see any reason why we would need loose comparisons, especially since they can result in weird behaviour as documented here and here.
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.
This PR is a breaking change with fallback, could you please file this PR against 3.3.x
branch instead?
@ackintosh we've set the current master to 3.3.0 (scheduled on coming Friday) after the 3.2.3 (patch) release. |
Oh, sorry 💦 LGTM ✨ I'll merge this PR later today if no further feedback. |
Upgrade Notephp-cs-fixer "require-dev": {
"phpunit/phpunit": "^4.8",
"squizlabs/php_codesniffer": "~2.6",
- "friendsofphp/php-cs-fixer": "~2.12"
+ "friendsofphp/php-cs-fixer": "~1.12"
}, |
@mattmelling thanks again for the PR, which is included in the v3.3.0 minor release: https://twitter.com/oas_generator/status/1046941449609068544 |
…PITools#769) * upgrade php-cs-fixer to 2.12 * ran petstore-security-test for php * updating openapi3 php petstore example
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
.master
,4.0.x
. Default:master
.Description of the PR
The PHP client uses friendsofphp/php-cs-fixer v1.12 for PSR2 analysis. 1,12 doesn't supports PHP up to < 7.2.
PHP >= 7.2 support was added in php-cs-fixer v2 which made some changes to the API and default fixers/rules.
This PR upgrades php-cs-fixer to v2.12. The fixers/rules have been aligned for PSR2 as desired in the original PR on swagger-codegen (#3863). I've changed defaults for some of the rules to prevent lots of whitespace warnings from php-cs-fixer, these typically seem to be down to blank template values and are not raised when using the configuration in master.
Notes
php-cs-fixer added the idea of "risky" fixers in v2. Since the
strict_param
andstrict_comparison
fixers are considered risky, we must alter the command line tovendor/bin/php-cs-fixer fix --dry-run --diff -vvv --allow-risky yes
even when using--dry-run
./cc @jebentier, @dkarlovi, @mandrean, @jfastnacht, @ackintosh, @ybelenko