-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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] Use a valid php type for maps #8718
Conversation
Align map array format to that expected by phpdoc and associated tools.
938b0fd
to
e573b75
Compare
travis failure looks to be trouble talking to npm. Skim of the results seem to show all php tests passing. |
@@ -278,7 +278,7 @@ class ObjectSerializer | |||
return $values; | |||
} | |||
|
|||
if (substr($class, 0, 4) === 'map[') { // for associative array e.g. map[string,int] | |||
if (preg_match('/^(array<|map\[)/', $class)) { // for associative array e.g. array<string,int> |
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.
Most of the changes are documentation. There could be no code instances since map was not a valid keyword/type.
This is kinda the glue for what should be the only technical change. When deserializing this handles but the new generated value and the old map value in case it was being references in some code.
I saw some comments that started dealing with this problem by injecting psalm/phpstan comments into the models. This shouldn't be needed anymore and can be removed after this change since it addresses the problem everywhere maps show up. |
Travis CI failure not related to this change. |
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.
LGTM but I've not tested it locally
@neclimdul thanks for the PR, which has been included in the v5.1.0 release: https://twitter.com/oas_generator/status/1373636654024380423 |
This pull request aligns the map types to native array types.
It does this in two steps to show the impact of the change.
map
to the native array type.@jebentier @dkarlovi @mandrean @jfastnacht @ackintosh @ybelenko @renepardon
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
,5.1.x
,6.0.x