8000 WIP [Meta] Add PHPStan to build process by dkarlovi · Pull Request #25536 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

WIP [Meta] Add PHPStan to build process #25536

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.

Sign up for GitHub

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

Closed
wants to merge 14 commits into from
Closed

WIP [Meta] Add PHPStan to build process #25536

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e1be251
[Meta] add PHPStan via jakzal/phpqa
dkarlovi Dec 18, 2017
bfb3048
[Meta] fix issues found via PHPStan in Bridge
dkarlovi Dec 18, 2017
9183062
[Meta] fix issues found via PHPStan in Bundle
dkarlovi Dec 18, 2017
d9e30ae
[Meta] fix issues found via PHPStan in Config component
dkarlovi Dec 18, 2017
83c1208
[Meta] fix issues found via PHPStan in Debug component
dkarlovi Dec 18, 2017
bdeda31
[Meta] fix issues found via PHPStan in DI component
dkarlovi Dec 18, 2017
0a33eb1
[Meta] fix issues found via PHPStan in Filesystem component
dkarlovi Dec 18, 2017
a7dff25
[Meta] fix issues found via PHPStan in Debug component
dkarlovi Dec 18, 2017
03daa41
[Meta] fix issues found via PHPStan in HttpFoundation component
dkarlovi Dec 18, 2017
da9d0e4
[Meta] fix issues found via PHPStan in Workflow component
dkarlovi Dec 18, 2017
75403df
[Meta] PHPStan, switch to a custom Docker image with all required exts
dkarlovi Dec 18, 2017
c5467e0
[Meta] PHPStan, move autoloaded files from Composer to PHPStan
dkarlovi Dec 19, 2017
dbdf9de
[Meta] PHPStan, un 10000 do changes to Debug component
dkarlovi Dec 19, 2017
5fc8d8e
[Meta] PHPStan, undo changes to DependencyInjection component
dkarlovi Dec 19, 2017
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
Prev Previous commit
Next Next commit
[Meta] fix issues found via PHPStan in Bridge
  • Loading branch information
@dkarlovi
dkarlovi committed Dec 18, 2017
commit bfb3048e70e1d28e87742317911186ff76731dc7
3 changes: 2 additions & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@
"egulias/email-validator": "~1.2,>=1.2.8|~2.0",
"symfony/phpunit-bridge": "~3.4|~4.0",
"symfony/security-acl": "~2.8|~3.0",
"phpdocumentor/reflection-docblock": "^3.0|^4.0"
"phpdocumentor/reflection-docblock": "^3.0|^4.0",
"swiftmailer/swiftmailer": "^6.0@dev"
Copy link
Member

Choose a reason for hiding this comment

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

why adding this one? it's not used in the code base so I wouldn't add it to please phpstan :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Monolog/Handler/SwiftMailerHandler.php is using it.

Copy link
Member

Choose a reason for hiding this comment

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

it's not: there is no test case for it, so the code just doens't run - so there is no need to add this in require-dev (it's not required)
unless you want to add a test case :)

},
"conflict": {
"phpdocumentor/reflection-docblock": "<3.0||>=3.2.0,<3.2.2",
Expand Down
5 changes: 2 additions & 3 deletions src/Symfony/Bridge/Doctrine/ManagerRegistry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
namespace Symfony\Bridge\Doctrine;

use ProxyManager\Proxy\LazyLoadingInterface;
use Psr\Container\ContainerInterface;
Copy link
Member

Choose a reason for hiding this comment

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

can you please submit these as separate PRs, on the lowest maintained branch where they apply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it would make sense to introduce this in master only? The way I see it, getting the entire framework to run will be an undertaking and, getting it to run in all branches might be quite the PITA.

Copy link
Member

Choose a reason for hiding this comment

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

I can't tell if we're going to merge this yet (adding phpstan)
But this change (and many others in this PR) are not related to adding phpstan, but are like code cleanups detected by phpstan, isn't it?
So these should be in their own PR to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's exactly my point, don't know if this is even possible to get running yet. Let's see if this can be done, if so, I'll separate bugfixes into separate PRs once we see how it'll even look and what gets caught.

Sounds good?

use Symfony\Component\DependencyInjection\Container;
use Doctrine\Common\Persistence\AbstractManagerRegistry;

Expand All @@ -24,7 +23,7 @@
abstract class ManagerRegistry extends AbstractManagerRegistry
{
/**
* @var ContainerInterface
* @var Container
*/
protected $container;

Expand Down Expand Up @@ -58,7 +57,7 @@ function (&$wrappedInstance, LazyLoadingInterface $manager) use ($name) {
$name = $this->aliases[$name];
}
if (isset($this->fileMap[$name])) {
$wrappedInstance = $this->load($this->fileMap[$name], false);
$wrappedInstance = $this->load($this->fileMap[$name]);
} else {
$method = $this->methodMap[$name] ?? 'get'.strtr($name, $this->underscoreMap).'Service'; // BC with DI v3.4
$wrappedInstance = $this->{$method}(false);
Expand Down
3 changes: 1 addition & 2 deletions ...mfony/Bridge/Doctrine/Tests/DependencyInjection/CompilerPass/RegisterMappingsPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ class RegisterMappingsPassTest extends TestCase
*/
public function testNoDriverParmeterException()
{
$container = $this->createBuilder(array(
));
$container = $this->createBuilder();
$this->process($container, array(
'manager.param.one',
'manager.param.two',
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/DoctrineChoiceLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,18 @@
use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityLoaderInterface;
use Symfony\Bridge\Doctrine\Form\ChoiceList\IdReader;
use Symfony\Component\Form\ChoiceList\ArrayChoiceList;
use Symfony\Component\Form\ChoiceList\Factory\ChoiceListFactoryInterface;

/**
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class DoctrineChoiceLoaderTest extends TestCase
{
/**
* @var ChoiceListFactoryInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $factory;

/**
* @var ObjectManager|\PHPUnit_Framework_MockObject_MockObject
*/
Expand Down
7 changes: 2 additions & 5 deletions src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
F438
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
use Symfony\Component\Form\Forms;
use Symfony\Component\Form\Tests\Extension\Core\Type\BaseTypeTest;
use Symfony\Component\Form\Tests\Extension\Core\Type\FormTypeTest;
use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity;
use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity;

Expand Down Expand Up @@ -1121,8 +1120,7 @@ public function testLoaderCaching()
$repo = $this->em->getRepository(self::SINGLE_IDENT_CLASS);

$entityType = new EntityType(
$this->emRegistry,
PropertyAccess::createPropertyAccessor()
$this->emRegistry
);

$entityTypeGuesser = new DoctrineOrmTypeGuesser($this->emRegistry);
Expand Down Expand Up @@ -1184,8 +1182,7 @@ public function testLoaderCachingWithParameters()
$repo = $this->em->getRepository(self::SINGLE_IDENT_CLASS);

$entityType = new EntityType(
$this->emRegistry,
PropertyAccess::createPropertyAccessor()
$this->emRegistry
);

$entityTypeGuesser = new DoctrineOrmTypeGuesser($this->emRegistry);
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Doctrine/Tests/PropertyInfo/DoctrineExtractorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\PropertyInfo\Tests;
namespace Symfony\Bridge\Doctrine\Tests\PropertyInfo;

use Doctrine\DBAL\Types\Type as DBALType;
use Doctrine\ORM\EntityManager;
Expand Down
0