ResponseInterface should implement \ArrayAccess
ndench opened this issue · 4 comments
ndench commented
Bug Report
Q | A |
---|---|
BC Break | no |
Version | 3.0 |
Summary
According to the documentation, you can process uploaded files using custom logic and pass update the Response using array access to pass extra data through:
// ...
class UploadListener
{
// ...
public function onUpload(PostPersistEvent $event)
{
//...
//if everything went fine
$response = $event->getResponse();
$response['success'] = true;
return $response;
}
}
The issue here is:
$event->getResponse()
returns ResponseInterface- ResponseInterface does not implement
\ArrayAccess
- So at "compile time" (eg. static analysis with phpstan/psalm/your IDE), we receive an error with this approach.
- It works at runtime, but only because all implementations of
ResponseInterface
extend AbstractResponse, which implements\ArrayAccess
How to reproduce
- Create an UploadListener as described in the documentation
- Use some form of static analysis, eg. psalm, phpstan, or you IDE of choice.
- Notice the reported error
PhpStorm with EA Extended plugin:
Suggested fix
Move the implements \ArrayAccess
declaration from AbstractResponse
to ResponseInterface
.
Happy to send through a PR for this, just want to validate that there isn't something I'm missing.
bytehead commented
Hey @ndench
Thanks for the issue! Seems totally legit and reasonable to me.
IMHO is altering an interface a BC break though, isn't it?
ndench commented
@bytehead that's a good point about BC. We should be able to use @implements
in phpdoc to be compatible. Then switch it to a proper implements in the next major version.