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

Tweaks for php generator, based on issues reported by PHP static analysis tools (PHPStan) #7376

Merged
merged 1 commit into from
Sep 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ApiException extends Exception
/**
* The HTTP body of the server response either as Json or string.
*
* @var mixed
* @var \stdClass|string|null
*/
protected $responseBody;

Expand All @@ -48,17 +48,17 @@ class ApiException extends Exception
/**
* The deserialized response object
*
* @var $responseObject;
* @var \stdClass|string|null
*/
protected $responseObject;

/**
* Constructor
*
* @param string $message Error message
* @param int $code HTTP status code
* @param string[]|null $responseHeaders HTTP response header
* @param mixed $responseBody HTTP decoded body of the server response either as \stdClass or string
* @param string $message Error message
* @param int $code HTTP status code
* @param string[]|null $responseHeaders HTTP response header
* @param \stdClass|string|null $responseBody HTTP decoded body of the server response either as \stdClass or string
*/
public function __construct($message = "", $code = 0, $responseHeaders = [], $responseBody = null)
{
Expand All @@ -80,7 +80,7 @@ class ApiException extends Exception
/**
* Gets the HTTP body of the server response either as Json or string
*
* @return mixed HTTP body of the server response either as \stdClass or string
* @return \stdClass|string|null HTTP body of the server response either as \stdClass or string
*/
public function getResponseBody()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ namespace {{invokerPackage}};
*/
class Configuration
{
/**
* @var Configuration
*/
private static $defaultConfiguration;

/**
Expand Down Expand Up @@ -128,7 +131,7 @@ class Configuration
*
* @param string $apiKeyIdentifier API key identifier (authentication scheme)
*
* @return string API key or token
* @return null|string API key or token
*/
public function getApiKey($apiKeyIdentifier)
{
Expand All @@ -154,7 +157,7 @@ class Configuration
*
* @param string $apiKeyIdentifier API key identifier (authentication scheme)
*
* @return string
* @return null|string
*/
public function getApiKeyPrefix($apiKeyIdentifier)
{
Expand Down Expand Up @@ -400,7 +403,7 @@ class Configuration
*
* @param string $apiKeyIdentifier name of apikey
*
* @return string API key with the prefix
* @return null|string API key with the prefix
*/
public function getApiKeyWithPrefix($apiKeyIdentifier)
{
Expand All @@ -423,7 +426,7 @@ class Configuration
/**
* Returns an array of host settings
*
* @return an array of host settings
* @return array an array of host settings
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type completely missing here.

*/
public function getHostSettings()
{
Expand Down Expand Up @@ -461,9 +464,9 @@ class Configuration
/**
* 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
* @param int $index index of the host settings
* @param array|null $variables hash of variable and the corresponding value (optional)
* @return string URL based on host settings
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type completely missing here.

*/
public function getHostFromSettings($index, $variables = null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class HeaderSelector
*
* @param string[] $accept Array of header
*
* @return string Accept (e.g. application/json)
* @return null|string Accept (e.g. application/json)
*/
private function selectAcceptHeader($accept)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,26 @@ class ObjectSerializer
* @param string $type the OpenAPIToolsType of the data
* @param string $format the format of the OpenAPITools type of the data
*
* @return string|object serialized form of $data
* @return scalar|object|array|null serialized form of $data
*/
public static function sanitizeForSerialization($data, $type = null, $format = null)
{
if (is_scalar($data) || null === $data) {
return $data;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code simplified since it's returning, no need for elseif.

} elseif ($data instanceof \DateTime) {
}

if ($data instanceof \DateTime) {
return ($format === 'date') ? $data->format('Y-m-d') : $data->format(self::$dateTimeFormat);
} elseif (is_array($data)) {
}

if (is_array($data)) {
foreach ($data as $property => $value) {
$data[$property] = self::sanitizeForSerialization($value);
}
return $data;
} elseif (is_object($data)) {
}

if (is_object($data)) {
$values = [];
if ($data instanceof ModelInterface) {
$formats = $data::openAPIFormats();
Expand Down Expand Up @@ -250,7 +256,9 @@ class ObjectSerializer
{
if (null === $data) {
return null;
} elseif (strcasecmp(substr($class, -2), '[]') === 0) {
}

if (strcasecmp(substr($class, -2), '[]') === 0) {
$data = is_string($data) ? json_decode($data) : $data;

if (!is_array($data)) {
Expand All @@ -263,7 +271,9 @@ class ObjectSerializer
$values[] = self::deserialize($value, $subClass, null);
}
return $values;
} elseif (substr($class, 0, 4) === 'map[') { // for associative array e.g. map[string,int]
}

if (substr($class, 0, 4) === 'map[') { // for associative array e.g. map[string,int]
$data = is_string($data) ? json_decode($data) : $data;
settype($data, 'array');
$inner = substr($class, 4, -1);
Expand All @@ -276,10 +286,14 @@ class ObjectSerializer
}
}
return $deserialized;
} elseif ($class === 'object') {
}

if ($class === 'object') {
settype($data, 'array');
return $data;
} elseif ($class === '\DateTime') {
}

if ($class === '\DateTime') {
// Some API's return an invalid, empty string as a
// date-time property. DateTime::__construct() will return
// the current time for empty input which is probably not
Expand All @@ -291,10 +305,14 @@ class ObjectSerializer
} else {
return null;
}
} elseif (in_array($class, [{{&primitives}}], true)) {
}

if (in_array($class, [{{&primitives}}], true)) {
settype($data, $class);
return $data;
} elseif ($class === '\SplFileObject') {
}

if ($class === '\SplFileObject') {
/** @var \Psr\Http\Message\StreamInterface $data */

// determine file name
Expand Down
8 changes: 4 additions & 4 deletions modules/openapi-generator/src/main/resources/php/api.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,17 @@ use {{invokerPackage}}\ObjectSerializer;
/**
* Set the host index
*
* @param int Host index (required)
* @param int $hostIndex Host index (required)
*/
public function setHostIndex($host_index)
public function setHostIndex($hostIndex)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS change

{
$this->hostIndex = $host_index;
$this->hostIndex = $hostIndex;
}

/**
* Get the host index
*
* @return Host index
* @return int Host index
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing type.

*/
public function getHostIndex()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* Please update the test case below to test the endpoint.
*/

namespace {{invokerPackage}};
namespace {{invokerPackage}}\Test\Api;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests were placed in totally wrong namespace.


use \{{invokerPackage}}\Configuration;
use \{{invokerPackage}}\ApiException;
Expand Down Expand Up @@ -71,6 +71,8 @@ use PHPUnit\Framework\TestCase;
*/
public function test{{vendorExtensions.x-test-operation-id}}()
{
// TODO: implement
$this->markTestIncomplete('Not implemented');
}
{{/operation}}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@
"psr-4": { "{{escapedInvokerPackage}}\\" : "{{srcBasePath}}/" }
},
"autoload-dev": {
"psr-4": { "{{escapedInvokerPackage}}\\" : "{{testBasePath}}/" }
"psr-4": { "{{escapedInvokerPackage}}\\Test\\" : "{{testBasePath}}/" }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests placed in the wrong namespace.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ use \{{invokerPackage}}\ObjectSerializer;
* @package {{invokerPackage}}
* @author OpenAPI Generator team
* @link https://openapi-generator.tech
{{^isEnum}}
* @implements \ArrayAccess<TKey, TValue>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generics.

* @template TKey int|null
* @template TValue mixed|null
{{/isEnum}}
*/
{{#isEnum}}{{>model_enum}}{{/isEnum}}{{^isEnum}}{{>model_generic}}{{/isEnum}}
{{/model}}{{/models}}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class {{classname}} {{#parentSchema}}extends {{{parent}}} {{/parentSchema}}{{^parentSchema}}implements ModelInterface, ArrayAccess{{/parentSchema}}
{
const DISCRIMINATOR = {{#discriminator}}'{{discriminatorName}}'{{/discriminator}}{{^discriminator}}null{{/discriminator}};
public const DISCRIMINATOR = {{#discriminator}}'{{discriminatorName}}'{{/discriminator}}{{^discriminator}}null{{/discriminator}};

/**
* The original name of the model.
Expand All @@ -23,6 +23,8 @@ class {{classname}} {{#parentSchema}}extends {{{parent}}} {{/parentSchema}}{{^pa
* Array of property to format mappings. Used for (de)serialization
*
* @var string[]
* @phpstan-var array<string, string|null>
* @psalm-var array<string, string|null>
*/
protected static $openAPIFormats = [
{{#vars}}'{{name}}' => {{#dataFormat}}'{{{dataFormat}}}'{{/dataFormat}}{{^dataFormat}}null{{/dataFormat}}{{#hasMore}},
Expand Down Expand Up @@ -161,7 +163,7 @@ class {{classname}} {{#parentSchema}}extends {{{parent}}} {{/parentSchema}}{{^pa

{{/parentSchema}}
{{#vars}}
$this->container['{{name}}'] = isset($data['{{name}}']) ? $data['{{name}}'] : {{#defaultValue}}{{{defaultValue}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}};
$this->container['{{name}}'] = $data['{{name}}'] ?? {{#defaultValue}}{{{defaultValue}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}};
{{/vars}}
{{#discriminator}}

Expand Down Expand Up @@ -278,7 +280,7 @@ class {{classname}} {{#parentSchema}}extends {{{parent}}} {{/parentSchema}}{{^pa
*
* @param {{dataType}}{{^required}}|null{{/required}} ${{name}}{{#description}} {{{description}}}{{/description}}{{^description}} {{{name}}}{{/description}}
*
* @return $this
* @return self
*/
public function {{setter}}(${{name}})
{
Expand Down Expand Up @@ -362,18 +364,18 @@ class {{classname}} {{#parentSchema}}extends {{{parent}}} {{/parentSchema}}{{^pa
*
* @param integer $offset Offset
*
* @return mixed
* @return mixed|null
*/
public function offsetGet($offset)
{
return isset($this->container[$offset]) ? $this->container[$offset] : null;
return isset($this->container[$offset]) ?? null;
}

/**
* Sets value based on offset.
*
* @param integer $offset Offset
* @param mixed $value Value to be set
* @param int|null $offset Offset
* @param mixed $value Value to be set
*
* @return void
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* Please update the test case below to test the model.
*/

namespace {{invokerPackage}};
namespace {{invokerPackage}}\Test\Model;

use PHPUnit\Framework\TestCase;

Expand Down Expand Up @@ -68,6 +68,8 @@ class {{classname}}Test extends TestCase
*/
public function test{{classname}}()
{
// TODO: implement
$this->markTestIncomplete('Not implemented');
}
{{#vars}}

Expand All @@ -76,6 +78,8 @@ class {{classname}}Test extends TestCase
*/
public function testProperty{{nameInCamelCase}}()
{
// TODO: implement
$this->markTestIncomplete('Not implemented');
}
{{/vars}}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@
"psr-4": { "OpenAPI\\Client\\" : "lib/" }
},
"autoload-dev": {
"psr-4": { "OpenAPI\\Client\\" : "test/" }
"psr-4": { "OpenAPI\\Client\\Test\\" : "test/" }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,17 @@ public function __construct(
/**
* Set the host index
*
* @param int Host index (required)
* @param int $hostIndex Host index (required)
*/
public function setHostIndex($host_index)
public function setHostIndex($hostIndex)
{
$this->hostIndex = $host_index;
$this->hostIndex = $hostIndex;
}

/**
* Get the host index
*
* @return Host index
* @return int Host index
*/
public function getHostIndex()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,17 @@ public function __construct(
/**
* Set the host index
*
* @param int Host index (required)
* @param int $hostIndex Host index (required)
*/
public function setHostIndex($host_index)
public function setHostIndex($hostIndex)
{
$this->hostIndex = $host_index;
$this->hostIndex = $hostIndex;
}

/**
* Get the host index
*
* @return Host index
* @return int Host index
*/
public function getHostIndex()
{
Expand Down
Loading