[REQ][PHP] General improvement ideas

Is your feature request related to a problem? Please describe.

I have a few things that i have to manually correct after generating a new php-client. One of them is specific to the API that i am using - so this is okay. but then there are persistent breaking changes to arrays/objects and a few other points.

this is what i am here to discuss first and fix later

Problem: platform-specific line endings

platform-specific line endings are ignored. generated files are always Linux/unix/macos \n

Proposal

Respect platform-specific line endings when creating files
Windows \r\n
Linux/unix/macos: \n
IMO, it is better to rely on local git installation settings (auto clrf converter)
https://help.github.com/en/github/using-git/configuring-git-to-handle-line-endings#global-settings-for-line-endings
Same problem exists with es-lint airbnb rules (but this is another story)

Problem: code around parameter inclusion with style,explode

if('form' === 'form' && is_array($parameterIds)) {
                foreach($parameterIds as $key => $value) {
                    $queryParams[$key] = $value;
                }
            }
            else {
                $queryParams['parameterIds'] = $parameterIds;
            }

This is correct IF you are passing an "Object" aka associative array. However, if it is a sequential array -> no conversion is needed (the else part).

Proposal:

https://stackoverflow.com/questions/173400/how-to-check-if-php-array-is-associative-or-sequential/4254008#4254008

if(is_array($parameterIds) && (count(array_filter(array_keys($parameterIds), 'is_string')) > 0)) {
                $queryParams = array_merge($queryParams, $parameterIds);
            } else {
                $queryParams['parameterIds'] = $parameterIds;
            }

actually, i'd like to re-work the whole parameter +explode+style methods.. maybe, use this based on types of the values?!

Problem: parameters validation

code if (!empty($parameterIds)) { fixes Empty arrays check, but breaks INTegers and booleans and.. The rest of the things.

Proposal: none

more sophisticated validation. based on types?!

Problem: redundant over-commenting

// query params

block is added for each parameter

Proposal:

remove duplicated comments

Problem: ObjectSerializer does not support boolean values

booleans in php fall into is_scalar type.

At the same time there is NO default boolean conversion inside Guzzle. booleans are converted to empty strings. and this is intended (links are somewhere in Guzzle discussions on GitHub)

Proposal:

...
if (is_bool($data)) {
            return $data ? 'true' : 'false';
        } elseif (is_scalar($data) || null === $data) {
            return $data;
        
...

add boolean to string converter. actually, this needs a bit of research on which strings are parsed correctly as booleans in various programming languages. i know that php accepts everything

ExtendedValidator plugin suggestions

As we have php 7.1 as a minimum version - ExtendedValidator plugin shows a few suggestions

Problem: incorrect type hints / Annotations

AN - is not a type

* @return an array of host settings
     */
    public function getHostSettings()

index, variables, URL - is not a type

/**
     * Returns URL based on the index and variables
     *
     * @param index array index of the host settings
     * @param variables hash of variable and the corresponding value (optional)
     * @return URL based on host settings
     */
    public function getHostFromSettings($index, $variables = null)

Proposal: correct type hints

• array

• integer, string|null, string

Problem: missing spec references in template

README.md doesn't update/generate package name GIT_USER_ID/GIT_REPO_ID

Composer.json doesn't update/generate

• name
• Version ???
• Additional authors

Proposal:

use the appropriate spec parameters

Problem: composer.json improvements

json_decode is used, ext-json is not required

Proposal:

• add ext-json to "require" section. version "*"

Problem: improve packages security

a phpStorm suggestion

Proposal:

• add "roave/security-advisories": "dev-master", to "require-dev"

Problem: .gitignore is not present

[main] ERROR o.o.codegen.DefaultGenerator - can't open 'php\.gitignore' for input; cannot write

Proposal:

Add .gitignore file to the template

Global ones are a also good: https://github.com/github/gitignore/tree/master/Global
Windows.gitignore macOS.gitignore ... etc

i am just tired of removing .DS_Store files from repos :|

other: facelifting

array() is now [] in php - prettier-php does convert them on the fly. not everyone is using prettier-php ;)

Describe alternatives you've considered

• modifying generated code manually

Additional context

OpenAPI v3 Spec in YAML

Win10 x64

openapi-generator 5.0.0-SNAPSHOT from 2020-05-19 + prettier-php

batch file

set PHP_POST_PROCESS_FILE=C:\Users\MyUser\AppData\Roaming\npm\prettier.cmd --write --phpVersion='7.3' --singleQuote=true
npx openapi-generator generate -i swagger.yaml -o . -g php --additional-properties=variableNamingConvention=camelCase,invokerPackage="SomeAPI\Client",packageName="spawn-guy/my-php-sdk" --enable-post-process-file

Start by reviewing the PHP generator templates and the named README.md, composer.json, .gitignore, and ObjectSerializer output; no specific source paths or tests are provided. Separate the line-ending, parameter serialization and validation, annotations, package metadata, security, and formatting proposals before implementation. Done would require agreed scope and generator tests for each accepted change.