Skip to content

Commit

Permalink
changes to make psalm and phpstan pass
Browse files Browse the repository at this point in the history
  • Loading branch information
@jaydiablo
jaydiablo committed Jun 4, 2018
1 parent f375bde commit 4e80a5a
Show file tree
Hide file tree
Showing 19 changed files with 590 additions and 148 deletions.
3 changes: 2 additions & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
"zendframework/zend-http": "^2.5",
"zendframework/zend-navigation": "^2.5",
"zendframework/zend-servicemanager": "^2.5,<3.0.0",
"friendsofphp/php-cs-fixer": "^2.10"
"friendsofphp/php-cs-fixer": "^2.10",
"vimeo/psalm": "1.1.4"
},
"suggest": {
"zfr/zfr-cors": "zfr/zfr-cors provides CORS support"
Expand Down
27 changes: 27 additions & 0 deletions phpstan.neon
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
parameters:
ignoreErrors:
# Type-o in ZF2's docblock
- '#Call to method getTypeString\(\) on an unknown class Zend\\Http\\Header\\Accept\\FieldValuePArt\\AcceptFieldValuePart\.#'
# I think in this case an array won't be returned
- '#Array \(array<Zend\\Stdlib\\CallbackHandler>\) does not accept array\|Zend\\Stdlib\\CallbackHandler\.#'
# These can probably be fixed if static type-hints are ever added
- '#Casting to .+ something that.s already .+\.#'
# This is more of a defensive thing, docblocks only allow specific object type and string
- '#Call to function is_object\(\) will always evaluate to false\.#'
# Internal code never sets $identifierName to false, but does have a setter with no restrictions
- '#Strict comparison using !== between false and string will always evaluate to true\.#'
- '#Strict comparison using === between false and string will always evaluate to false\.#'
# This is a version branch, detected earlier
- '#Parameter \#2 \$listener of method Zend\\EventManager\\SharedEventManagerInterface::detach\(\) expects Zend\\Stdlib\\CallbackHandler, string given\.#'
# ZF2 docblocks aren't correct, does accept an array
- '#Parameter \#1 \$nameOrModel of method Zend\\View\\Renderer\\JsonRenderer::render\(\) expects string\|Zend\\View\\Model\\ModelInterface, array given\.#'
# There's a check earlier that checks if there's an ApiProblem (which checks if apiProblem is null), so won't be null here
- '#Parameter \#1 \$problem of method PhlyRestfully\\View\\RestfulJsonStrategy::getStatusCodeFromApiProblem\(\) expects PhlyRestfully\\ApiProblem, PhlyRestfully\\ApiProblem\|null given\.#'
# Other issues
- '#Parameter \#1 \$collection of class PhlyRestfully\\HalCollection constructor expects array\|Traversable, array\|object given\.#'
- '#Call to function count\(\) with argument type array\|Traversable will always result in number 1\.#'
- '#Parameter \#1 \$hydrator of method PhlyRestfully\\Plugin\\HalLinks::setDefaultHydrator\(\) expects Zend\\Hydrator\\HydratorInterface, object given\.#'
- '#Parameter \#1 \$collection of class PhlyRestfully\\HalCollection constructor expects array\|Traversable, object given\.#'
- '#Cannot call method setItemCountPerPage\(\) on array\|Traversable\.#'
- '#Cannot call method setCurrentPageNumber\(\) on array\|Traversable\.#'
- '#Parameter \#1 \$matches of method PhlyRestfully\\ResourceInterface::setRouteMatch\(\) expects Zend\\Mvc\\Router\\RouteMatch, Zend\\Mvc\\Router\\RouteMatch\|null given\.#'
111 changes: 111 additions & 0 deletions psalm.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?xml version="1.0"?>
<psalm
totallyTyped="false"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config ./vendor/vimeo/psalm/config.xsd"
>
<projectFiles>
<directory name="./src" />
</projectFiles>

<issueHandlers>
<PropertyNotSetInConstructor errorLevel="info" />

<InvalidCatch>
<errorLevel type="info">
<!-- All of the Zend\Uri exceptions extend InvalidArgumentException and implement this interface, so
this is technically allowed -->
<file name="src/Link.php" />
</errorLevel>
</InvalidCatch>

<ReservedWord>
<errorLevel type="info">
<!-- "resource" is a "soft" reserved word as of PHP 7.0: https://secure.php.net/manual/en/reserved.other-reserved-words.php -->
<file name="src/Resource.php" />
<file name="src/Factory/ResourceControllerFactory.php" />
</errorLevel>
</ReservedWord>

<InvalidArgument>
<errorLevel type="info">
<!-- There is a version check here to use the opposite parameter order depending on which version of
the SharedEventManager is being used -->
<file name="src/Listener/ResourceParametersListener.php" />
<!-- ZF2's View\Renderer\JsonRenderer has incorrect docblock for this method -->
<file name="src/View/RestfulJsonRenderer.php" />
</errorLevel>
</InvalidArgument>

<InvalidPropertyAssignmentValue>
<errorLevel type="info">
<file name="src/Listener/ResourceParametersListener.php" />
</errorLevel>
</InvalidPropertyAssignmentValue>

<UndefinedClass>
<errorLevel type="info">
<!-- type-o in Zend\Http\Header\AcceptAbstract docblock for match method -->
<file name="src/Listener/ApiProblemListener.php" />
</errorLevel>
</UndefinedClass>

<TypeCoercion>
<errorLevel type="info">
<!-- This one doesn't make any sense to me:
$this->collection expects 'array<mixed, mixed>|Traversable|Zend\Paginator\Paginator', parent type 'array<mixed, mixed>|Traversable|Zend\Paginator\Paginator' provided -->
<file name="src/HalCollection.php" />
<!-- This comes from a config definition, could inline type it, but maybe hides an error -->
<file name="src/Module.php" />
<!-- object most likely gets turned into a HalCollection, but isn't guaranteed in the code -->
<file name="src/Plugin/HalLinks.php" />
</errorLevel>
</TypeCoercion>

<RedundantCondition>
<errorLevel type="info">
<!-- Can't type-hint this (union type), so this check is done to prevent invalid type coming in -->
<file name="src/HalResource.php" />
<file name="src/Plugin/HalLinks.php" />
</errorLevel>
</RedundantCondition>

<PossiblyNullArgument>
<errorLevel type="info">
<!-- this has a check before it that would return false if apiProblem is null -->
<file name="src/View/RestfulJsonStrategy.php" />
</errorLevel>
</PossiblyNullArgument>

<MissingConstructor>
<errorLevel type="info">
<file name="src/View/RestfulJsonRenderer.php" />
</errorLevel>
</MissingConstructor>

<PossiblyInvalidMethodCall>
<errorLevel type="info">
<file name="src/Plugin/HalLinks.php" />
</errorLevel>
</PossiblyInvalidMethodCall>

<PossiblyUndefinedMethod>
<errorLevel type="info">
<file name="src/Plugin/HalLinks.php" />
</errorLevel>
</PossiblyUndefinedMethod>

<PossiblyInvalidArgument>
<errorLevel type="info">
<file name="src/Plugin/HalLinks.php" />
</errorLevel>
</PossiblyInvalidArgument>

<MoreSpecificImplementedParamType errorLevel="info" />
<ImplementedReturnTypeMismatch errorLevel="info" />
<!-- These are all defensive checks since the parameters aren't php type-hinted -->
<RedundantConditionGivenDocblockType errorLevel="info" />
<DocblockTypeContradiction errorLevel="info" />
</issueHandlers>
</psalm>
54 changes: 35 additions & 19 deletions src/ApiProblem.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@

/**
* Object describing an API-Problem payload
*
* Properties provided by __get
* @property string|int $httpStatus
* @property string $describedby
* @property string $described_by
* @property string|int $http_status
* @property string $title
* @property string|\Exception $detail
*/
class ApiProblem
{
Expand All @@ -28,7 +36,7 @@ class ApiProblem

/**
* Description of the specific problem.
* @var string
* @var string|\Exception
*/
protected $detail = '';

Expand Down Expand Up @@ -124,7 +132,7 @@ class ApiProblem
* from $problemStatusTitles as a result.
*
* @param int $httpStatus
* @param string $detail
* @param string|\Exception $detail
* @param string $describedBy
* @param string $title
*/
Expand Down Expand Up @@ -182,7 +190,7 @@ public function __get($name)
/**
* Get passed in exception if available
*
* @return \Exception
* @return \Exception|null
*/
public function getException()
{
Expand Down Expand Up @@ -243,7 +251,7 @@ protected function getDetail()
* If an exception was provided, creates the status code from it;
* otherwise, code as provided is used.
*
* @return string
* @return int|string
*/
protected function getHttpStatus()
{
Expand Down Expand Up @@ -295,31 +303,39 @@ protected function getTitle()
*/
protected function createDetailFromException()
{
$e = $this->detail;
if ($this->detail instanceof \Exception) {
$e = $this->detail;

if (!$this->detailIncludesStackTrace) {
return $e->getMessage();
if (!$this->detailIncludesStackTrace) {
return $e->getMessage();
}
$message = '';
do {
$message .= $e->getMessage() . "\n";
$message .= $e->getTraceAsString() . "\n";
$e = $e->getPrevious();
} while ($e instanceof \Exception);
return trim($message);
}
$message = '';
do {
$message .= $e->getMessage() . "\n";
$message .= $e->getTraceAsString() . "\n";
$e = $e->getPrevious();
} while ($e instanceof \Exception);
return trim($message);

return '';
}

/**
* Create HTTP status from an exception.
*
* @return string
* @return int
*/
protected function createStatusFromException()
{
$e = $this->detail;
$httpStatus = $e->getCode();
if (!empty($httpStatus)) {
return $httpStatus;
if ($this->detail instanceof \Exception) {
$e = $this->detail;
$httpStatus = $e->getCode();
if (!empty($httpStatus)) {
return $httpStatus;
} else {
return 500;
}
} else {
return 500;
}
Expand Down
67 changes: 43 additions & 24 deletions src/Factory/ResourceControllerFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ class ResourceControllerFactory implements AbstractFactoryInterface
/**
* Determine if we can create a service with name
*
* @param ServiceLocatorInterface $controllers
* @param \Zend\ServiceManager\AbstractPluginManager $controllers
* @param string $name
* @param string $requestedName
*
* @return bool
*/
public function canCreateServiceWithName(ServiceLocatorInterface $controllers, $name, $requestedName)
Expand All @@ -39,6 +40,7 @@ public function canCreateServiceWithName(ServiceLocatorInterface $controllers, $
return false;
}

/** @var array $config */
$config = $services->get('config');
if (!isset($config['phlyrestfully'])
|| !isset($config['phlyrestfully']['resources'])
Expand Down Expand Up @@ -73,15 +75,18 @@ public function canCreateServiceWithName(ServiceLocatorInterface $controllers, $
/**
* Create service with name
*
* @param ServiceLocatorInterface $controllers
* @param \Zend\ServiceManager\AbstractPluginManager $controllers
* @param string $name
* @param string $requestedName
*
* @return ResourceController
*
* @throws ServiceNotCreatedException if listener specified is not a ListenerAggregate
*/
public function createServiceWithName(ServiceLocatorInterface $controllers, $name, $requestedName)
{
$services = $controllers->getServiceLocator();
/** @var array $config */
$config = $services->get('config');
$config = $config['phlyrestfully']['resources'][$requestedName];

Expand All @@ -108,6 +113,7 @@ public function createServiceWithName(ServiceLocatorInterface $controllers, $nam
$resourceIdentifiers = array_merge($resourceIdentifiers, $config['resource_identifiers']);
}

/** @var \Zend\EventManager\EventManager $events */
$events = $services->get('EventManager');
$events->attach($listener);
$events->setIdentifiers($resourceIdentifiers);
Expand All @@ -120,6 +126,7 @@ public function createServiceWithName(ServiceLocatorInterface $controllers, $nam
$identifier = $config['identifier'];
}

/** @var \Zend\EventManager\EventManagerInterface $events */
$events = $services->get('EventManager');
$controllerClass = isset($config['controller_class'])
? $config['controller_class']
Expand All @@ -144,8 +151,10 @@ public function createServiceWithName(ServiceLocatorInterface $controllers, $nam
/**
* Loop through configuration to discover and set controller options.
*
* @param array $config
* @param ResourceController $controller
* @param array $config
* @param ResourceController $controller
*
* @return void
*/
protected function setControllerOptions(array $config, ResourceController $controller)
{
Expand Down Expand Up @@ -175,28 +184,38 @@ protected function setControllerOptions(array $config, ResourceController $contr
// the whitelisted query parameters in order to seed the
// collection route options.
$whitelist = $value;
$controller->getEventManager()->attach('getList.post', function ($e) use ($whitelist) {
$request = $e->getTarget()->getRequest();
if (!method_exists($request, 'getQuery')) {
return;
}
$query = $request->getQuery();
$params = [];
foreach ($query as $key => $value) {
if (!in_array($key, $whitelist)) {
continue;
$controller->getEventManager()->attach(
'getList.post',
/**
* @param \Zend\Mvc\MvcEvent $e
* @return void
*/
function ($e) use ($whitelist) {
/** @var \Zend\Mvc\Controller\AbstractController $target */
$target = $e->getTarget();
/** @var \Zend\Http\Request $request */
$request = $target->getRequest();
if (!method_exists($request, 'getQuery')) {
return;
}
$query = $request->getQuery();
$params = [];
foreach ($query as $key => $value) {
if (!in_array($key, $whitelist)) {
continue;
}
$params[$key] = $value;
}
if (empty($params)) {
return;
}
$params[$key] = $value;
}
if (empty($params)) {
return;
}

$collection = $e->getParam('collection');
$collection->setCollectionRouteOptions([
'query' => $params,
]);
});
$collection = $e->getParam('collection');
$collection->setCollectionRouteOptions([
'query' => $params,
]);
}
);
break;

case 'content_types':
Expand Down
Loading

0 comments on commit 4e80a5a

Please sign in to comment.