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
8000 dbdf9de
[Meta] PHPStan, undo 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 Debug component
  • Loading branch information
@dkarlovi
dkarlovi committed Dec 18, 2017
commit 83c1208fe29432a6e30152e4cfe142a97d864a21
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ QA_DOCKER_IMAGE=jakzal/phpqa:latest
QA_DOCKER_COMMAND=docker run -it --rm -v "$(shell pwd):/project" -w /project ${QA_DOCKER_IMAGE}

phpstan:
sh -c "${QA_DOCKER_COMMAND} phpstan analyse --configuration phpstan.neon --level 0 src/Symfony/Bundle"
sh -c "${QA_DOCKER_COMMAND} phpstan analyse --configuration phpstan.neon --level 0 src/Symfony/Component/Debug"

##
# Special operations
Expand Down
7 changes: 7 additions & 0 deletions phpstan.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ parameters:
- '#__construct\(\) does not call parent constructor from .+#'
- '#Function opcache_[^\s]* not found.#'
- '#Function pcntl_[^\s]* not found.#'
- '#Function xdebug_[^\s]* not found.#'

# not errors, actually expected to fail
- '#Class Symfony\\Bundle\\FrameworkBundle\\Tests\\DependencyInjection\\Compiler\\NotFound not found.#'
Expand Down Expand Up @@ -36,3 +37,9 @@ parameters:

# temporary, loading a NotLoadableClass throws a fatal error
- src/Symfony/Component/VarDumper/Tests/Caster/ReflectionCasterTest.php

# temporary, it's full of actual errors (which are triggers for the handler)
- src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php

# temporary, PHPStan doesn't seem to understand namespaced functions properly
- src/Symfony/Component/Debug/Tests/HeaderMock.php
3 changes: 1 addition & 2 deletions src/Symfony/Component/Debug/FatalErrorHandler/ClassNotFoundFatalErrorHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Symfony\Component\Debug\Exception\FatalErrorException;
use Symfony\Component\Debug\DebugClassLoader;
use Composer\Autoload\ClassLoader as ComposerClassLoader;
use Symfony\Component\ClassLoader\ClassLoader as SymfonyClassLoader;

/**
* ErrorHandler for classes that do not exist.
Expand Down Expand Up @@ -105,7 +104,7 @@ private function getClassCandidates(string $class): array
}
}

if ($function[0] instanceof ComposerClassLoader || $function[0] instanceof SymfonyClassLoader) {
if ($function[0] instanceof ComposerClassLoader) {
Copy link
Member
@nicolas-grekas nicolas-grekas Dec 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

this should be kept: the class exists in cross component versions situations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you give an example?

Copy link
Member

Choose a reason for hiding this comment

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

any project that uses debug v4 + class-loader v3.4

foreach ($function[0]->getPrefixes() as $prefix => $paths) {
foreach ($paths as $path) {
$classes = array_merge($classes, $this->findClassInPath($path, $class, $prefix));
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Debug/FatalErrorHandler/UndefinedFunctionFatalErrorHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function handleError(array $error, FatalErrorException $exception)
}

$candidates = array();
foreach (get_defined_functions() as $type => $definedFunctionNames) {
foreach (get_defined_functions(false) as $type => $definedFunctionNames) {
Copy link
Member

Choose a reason for hiding this comment

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

the function takes no arguments, why this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might be a false positive, it takes an optional param since 7.0.15: http://php.net/manual/en/function.get-defined-functions.php#refsect1-function.get-defined-functions-parameters

Copy link
Member

Choose a reason for hiding this comment

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

ok, should be removed imho as it's the default value, and may break BC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll report it to PHPStan, don't know why it was asked for here.

foreach ($definedFunctionNames as $definedFunctionName) {
if (false !== $namespaceSeparatorIndex = strrpos($definedFunctionName, '\\')) {
$definedFunctionNameBasename = substr($definedFunctionName, $namespaceSeparatorIndex + 1);
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/Debug/Tests/Exception/FlattenExceptionTest.php
View file
Open in desktop
48E2
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ function () {},

public function testRecursionInArguments()
{
if (!isset($a)) {
Copy link
Member

Choose a reason for hiding this comment

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

useless. It will never be set there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used for the variable to not be undeclared in the following line.

Copy link

Choose a reason for hiding this comment

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

The correct fix to help PHPStan understand the code is

$a = null;
$a = array('foo', array(2, &$a));

$a = [];
}
$a = array('foo', array(2, &$a));
$exception = $this->createException($a);

Expand Down
0