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

Updated AbstractActionController::indexAction return type to reflect real usage #5

Closed
1 task done
weierophinney opened this issue Dec 31, 2019 · 4 comments
Closed
1 task done
Labels
Bug Something isn't working

Comments

@weierophinney
Copy link
Member

  • Is this related to quality assurance?

When running static analysis against a simple userland controller like:

class MyController extends AbstractActionController
{
    public function indexAction()
    {
        return ['posts' => ['foo', 'bar']];
    }
}

That do NOT provide the return type, neither with PHP 7.1 return type nor via DocBlock, tools like PHPstan use inherited docs and report:

Method MyController::indexAction() should return Zend\View\Model\ViewModel but returns array.

Of course users should add return types, but it would result in requiring a massive code update without real benefits, since the default behavior of Zend\Mvc framework is to allow:

  1. null from \Zend\Mvc\View\Http\CreateViewModelListener::createViewModelFromNull listener
  2. array from \Zend\Mvc\View\Http\CreateViewModelListener::createViewModelFromArray listener
  3. ViewModel from \Zend\Mvc\View\Http\DefaultRenderingStrategy::render listener
  4. ResponseInterface from \Zend\Mvc\SendResponseListener::sendResponse listener

This change only applies to indexAction since I guess its usage is widespread: notFoundAction is untouched as users that override it are, IMHO, already advanced ones.

Ping @Ocramius you type lover ❤️

Reference: https://github.com/Slamdunk/phpstan-zend-framework/issues/5


Originally posted by @Slamdunk at zendframework/zend-mvc#312

@weierophinney weierophinney added the Bug Something isn't working label Dec 31, 2019
@weierophinney
Copy link
Member Author

As it seems you've worked on ZF with PHPStan quite a lot, what do you think about this @thomasvargiu?


Originally posted by @Slamdunk at zendframework/zend-mvc#312 (comment)

@weierophinney
Copy link
Member Author

@Slamdunk I have the same problem in my projects and right now I’m ignoring that kind of errors.
But yes, even if I don’t like methods that can return 5 or 6 different types, we should fix the phodoc return type.


Originally posted by @thomasvargiu at zendframework/zend-mvc#312 (comment)

@weierophinney
Copy link
Member Author

But probably it's better to use only ResponseInterface and ModelInterface, starting deprecating array, null, and void.


Originally posted by @thomasvargiu at zendframework/zend-mvc#312 (comment)

@alexmerlin
Copy link
Member

Closing issue due to being inactive for more than 1 year.

@alexmerlin alexmerlin closed this as not planned Won't fix, can't repro, duplicate, stale Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants