Skip to content
This repository has been archived by the owner on Jun 26, 2018. It is now read-only.

Add support for symfony Controllers #12

Merged
merged 6 commits into from
Jan 15, 2018
Merged

Add support for symfony Controllers #12

merged 6 commits into from
Jan 15, 2018

Conversation

mzk
Copy link
Contributor

@mzk mzk commented Dec 29, 2017

getClassName returned for example AccountController, not Controller.

vendor/shipmonk-php-code-checker/vendor/bin/phpstan analyse -l 6 -c phpstan.neon src/Fulfillment/Bundle/AccountApiBundle/Controller/AccountController.php
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------ 
  Line   AccountController.php                                 
 ------ ------------------------------------------------------ 
  44     Service "foobar" is not registered in the container.  
  45     Call to an undefined method object::callNo().         
 ------ ------------------------------------------------------ 

cc https://github.com/ShipMonk/shipmonk/pull/1668

A file should declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it should execute logic with side effects, but should not do both. The first symbol is defined on line 11 and the first side effect is on line 9. (PSR1.Files.SideEffects.FoundWithSymbols)
@lookyman
Copy link
Owner

Looks good, although I am not sure what defining the Controller class in tests does with autoloading.. I'll check it and get back to you.

@mzk
Copy link
Contributor Author

mzk commented Jan 13, 2018

We use it use in our project with a lot of dependencies and there is no problem with the autoloader. But rather, try it for sure.

@mzk
Copy link
Contributor Author

mzk commented Jan 15, 2018

@lookyman pull request was updated, added a fix. Service "" is not registered in the container.

@lookyman lookyman merged commit 1b7b77b into lookyman:master Jan 15, 2018
@lookyman
Copy link
Owner

lookyman commented Jan 15, 2018

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants