minor #33236 Add return-types with help from DebugClassLoader in the CI (nicolas-grekas)
This PR was merged into the 4.4 branch.
Discussion
----------
Add return-types with help from DebugClassLoader in the CI
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #33228
| License | MIT
| Doc PR | -
I've spent a great deal of time on this PR, experimenting with adding return types to the codebase.
TL;DR: my conclusion is that we cannot make it for 5.0.
There are two reasons for this:
1. The burden this will put on the community is immense, especially when considering that third party libs must also be updated for any apps to work at all on a return-typed 5.0. Symfony must add them last, not first.
2. We need return type covariance, yet this won't be available before PHP 7.4, while 5.0 supports 7.2.
What's attached?
- ~a draft patching logic in `DebugClassLoader` to add return-type where it discovers this should be done~
- return types added automatically thanks to #33283
- ~manual fixes for situations not handled (yet, if possible at all) by that logic in `DebugClassLoader`~ #33332
What's achieved? Tests are green \o/
At this stage, I think we have to acknowledge we won't add return-types in 5.0 but prepare a serious plan to add them in 6.0.
This plan could be:
- [x] make DebugClassLoader able to automate adding return types.
- [x] in 4.4: add all possible return types that don't break BC, e.g. in `Tests` and in generated code
- [x] spot and fix places where annotations aren't accurate, add more annotations where possible.
- [x] ensure `DebugClassLoader` triggers the best possible deprecations that encourage ppl to add return-types in their libs/apps. This means we could decide to disable the current ones (see #33235) and to re-enable them in 5.1. This will also give us the time to fine-tune the tooling (item 1. on this list)
Ideally, we could reach a point where we could test branch 4.4 *with* return-types: we'd use the tooling to add them automatically in the CI job, then we'd run tests and they should be green. Let's do this?
Help Wanted, here is how:
*With PHP 7.4*, run `php .github/patch-types.php`. This will add return types everywhere possible.
Then run tests, e.g. `./phpunit src/Symfony/Component/HttpFoundation --exclude-group legacy,issue-32995`
Here are the components that fail with return types added, please help me check them all with a PR on [my fork](https://github.com/nicolas-grekas/symfony/tree/eh-return-types):
- [x] src/Symfony/Bridge/Doctrine
- [x] src/Symfony/Bridge/Monolog
- [x] src/Symfony/Bridge/PhpUnit
- [x] src/Symfony/Bridge/ProxyManager
- [x] src/Symfony/Bridge/Twig
- [x] src/Symfony/Bundle/DebugBundle
- [x] src/Symfony/Bundle/FrameworkBundle
- [x] src/Symfony/Bundle/SecurityBundle
- [x] src/Symfony/Bundle/TwigBundle
- [x] src/Symfony/Bundle/WebProfilerBundle
- [x] src/Symfony/Bundle/WebServerBundle
- [x] src/Symfony/Component/Asset
- [x] src/Symfony/Component/BrowserKit
- [x] src/Symfony/Component/Cache
- [x] https://github.com/nicolas-grekas/symfony/pull/28 src/Symfony/Component/Config
- [x] src/Symfony/Component/Console
- [x] src/Symfony/Component/CssSelector
- [x] src/Symfony/Component/Debug
- [x] https://github.com/nicolas-grekas/symfony/pull/28 src/Symfony/Component/DependencyInjection
- [x] src/Symfony/Component/DomCrawler
- [x] src/Symfony/Component/Dotenv
- [x] src/Symfony/Component/ErrorHandler
- [x] src/Symfony/Component/ErrorRenderer
- [x] https://github.com/nicolas-grekas/symfony/pull/24 src/Symfony/Component/EventDispatcher
- [x] src/Symfony/Component/ExpressionLanguage
- [x] src/Symfony/Component/Filesystem
- [x] src/Symfony/Component/Finder
- [x] src/Symfony/Component/Form
- [x] src/Symfony/Component/HttpClient
- [x] src/Symfony/Component/HttpFoundation
- [x] src/Symfony/Component/HttpKernel
- [x] src/Symfony/Component/Inflector
- [x] src/Symfony/Component/Intl
- [x] src/Symfony/Component/Ldap
- [x] src/Symfony/Component/Lock
- [x] src/Symfony/Component/Mailer
- [x] src/Symfony/Component/Messenger
- [x] src/Symfony/Component/Mime
- [x] src/Symfony/Component/OptionsResolver
- [x] src/Symfony/Component/Process
- [x] src/Symfony/Component/PropertyAccess
- [x] src/Symfony/Component/PropertyInfo
- [x] https://github.com/nicolas-grekas/symfony/pull/25 src/Symfony/Component/Routing
- [x] https://github.com/nicolas-grekas/symfony/pull/26 src/Symfony/Component/Security
- [x] src/Symfony/Component/Security/Core
- [x] src/Symfony/Component/Security/Guard
- [x] src/Symfony/Component/Security/Http
- [x] https://github.com/nicolas-grekas/symfony/pull/29 src/Symfony/Component/Serializer
- [x] src/Symfony/Component/Security/Csrf
- [x] src/Symfony/Component/Stopwatch
- [x] src/Symfony/Component/Templating
- [x] https://github.com/nicolas-grekas/symfony/pull/27 src/Symfony/Component/Translation
- [x] src/Symfony/Component/Validator
- [x] src/Symfony/Component/VarDumper
- [x] src/Symfony/Component/VarExporter
- [x] src/Symfony/Component/WebLink
- [x] src/Symfony/Component/Workflow
- [x] src/Symfony/Component/Yaml
- [x] src/Symfony/Contracts
Commits
-------
11149a1fbb
Add return-types with help from DebugClassLoader in the CI
This commit is contained in:
commit
6dd9d24e50
45
.github/patch-types.php
vendored
Normal file
45
.github/patch-types.php
vendored
Normal file
@ -0,0 +1,45 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
if (false === getenv('SYMFONY_PATCH_TYPE_DECLARATIONS')) {
|
||||||
|
putenv('SYMFONY_PATCH_TYPE_DECLARATIONS=force=1&php71-compat=0');
|
||||||
|
}
|
||||||
|
|
||||||
|
require __DIR__.'/../.phpunit/phpunit-8.3-0/vendor/autoload.php';
|
||||||
|
|
||||||
|
file_put_contents(__DIR__.'/../vendor/autoload.php', preg_replace('/^return (Composer.*);/m', <<<'EOTXT'
|
||||||
|
$loader = \1;
|
||||||
|
$loader->addClassMap(['Symfony\Component\Debug\Exception\FlattenException' => \dirname(__DIR__).'/src/Symfony/Component/Debug/Exception/FlattenException.php']);
|
||||||
|
|
||||||
|
return $loader;
|
||||||
|
|
||||||
|
EOTXT
|
||||||
|
, file_get_contents(__DIR__.'/../vendor/autoload.php')));
|
||||||
|
|
||||||
|
$loader = require __DIR__.'/../vendor/autoload.php';
|
||||||
|
|
||||||
|
Symfony\Component\ErrorHandler\DebugClassLoader::enable();
|
||||||
|
|
||||||
|
foreach ($loader->getClassMap() as $class => $file) {
|
||||||
|
switch (true) {
|
||||||
|
case false !== strpos(realpath($file), '/vendor/'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Bridge/PhpUnit/'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Validation/Article.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/Config/Tests/Fixtures/BadParent.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/Debug/Tests/Fixtures/'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Compiler/OptionalServiceClass.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/ParentNotExists.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/BadClasses/MissingParent.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/ErrorHandler/Tests/Fixtures/'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/PropertyInfo/Tests/Fixtures/Dummy.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/PropertyInfo/Tests/Fixtures/ParentDummy.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/Serializer/Tests/Normalizer/Features/ObjectOuter.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/VarDumper/Tests/Fixtures/NotLoadableClass.php'):
|
||||||
|
case false !== strpos($file, '/src/Symfony/Component/VarDumper/Tests/Fixtures/Php74.php') && \PHP_VERSION_ID < 70400:
|
||||||
|
continue 2;
|
||||||
|
}
|
||||||
|
|
||||||
|
class_exists($class);
|
||||||
|
}
|
||||||
|
|
||||||
|
Symfony\Component\ErrorHandler\DebugClassLoader::disable();
|
21
.travis.yml
21
.travis.yml
@ -26,7 +26,7 @@ env:
|
|||||||
matrix:
|
matrix:
|
||||||
include:
|
include:
|
||||||
- php: 7.1
|
- php: 7.1
|
||||||
env: php_extra="7.2"
|
env: php_extra="7.2 7.4snapshot"
|
||||||
- php: 7.3
|
- php: 7.3
|
||||||
env: deps=high
|
env: deps=high
|
||||||
- php: 7.4snapshot
|
- php: 7.4snapshot
|
||||||
@ -79,10 +79,8 @@ before_install:
|
|||||||
export COMPONENTS=$(find src/Symfony -mindepth 2 -type f -name phpunit.xml.dist -printf '%h\n')
|
export COMPONENTS=$(find src/Symfony -mindepth 2 -type f -name phpunit.xml.dist -printf '%h\n')
|
||||||
find ~/.phpenv -name xdebug.ini -delete
|
find ~/.phpenv -name xdebug.ini -delete
|
||||||
|
|
||||||
if [[ $TRAVIS_PHP_VERSION = 7.4* && $deps ]]; then
|
if [[ $TRAVIS_PHP_VERSION = 7.4* ]]; then
|
||||||
export PHPUNIT_X="$PHPUNIT_X,issue-32995"
|
export PHPUNIT_X="$PHPUNIT_X,issue-32995"
|
||||||
elif [[ $TRAVIS_PHP_VERSION = 7.4* ]]; then
|
|
||||||
export PHPUNIT_X="$PHPUNIT --group issue-32995"
|
|
||||||
fi
|
fi
|
||||||
|
|
||||||
nanoseconds () {
|
nanoseconds () {
|
||||||
@ -150,7 +148,7 @@ before_install:
|
|||||||
- |
|
- |
|
||||||
# php.ini configuration
|
# php.ini configuration
|
||||||
for PHP in $TRAVIS_PHP_VERSION $php_extra; do
|
for PHP in $TRAVIS_PHP_VERSION $php_extra; do
|
||||||
phpenv global $PHP 2>/dev/null || (cd / && wget https://s3.amazonaws.com/travis-php-archives/binaries/ubuntu/14.04/x86_64/php-$PHP.tar.bz2 -O - | tar -xj)
|
phpenv global $PHP 2>/dev/null || (cd / && wget https://storage.googleapis.com/travis-ci-language-archives/php/binaries/ubuntu/16.04/x86_64/php-$PHP.tar.bz2 -O - | tar -xj)
|
||||||
INI=~/.phpenv/versions/$PHP/etc/conf.d/travis.ini
|
INI=~/.phpenv/versions/$PHP/etc/conf.d/travis.ini
|
||||||
echo date.timezone = Europe/Paris >> $INI
|
echo date.timezone = Europe/Paris >> $INI
|
||||||
echo memory_limit = -1 >> $INI
|
echo memory_limit = -1 >> $INI
|
||||||
@ -262,7 +260,7 @@ install:
|
|||||||
run_tests () {
|
run_tests () {
|
||||||
set -e
|
set -e
|
||||||
export PHP=$1
|
export PHP=$1
|
||||||
if [[ $PHP != $TRAVIS_PHP_VERSION && $TRAVIS_PULL_REQUEST != false ]]; then
|
if [[ $PHP != 7.4* && $PHP != $TRAVIS_PHP_VERSION && $TRAVIS_PULL_REQUEST != false ]]; then
|
||||||
echo -e "\\n\\e[1;34mIntermediate PHP version $PHP is skipped for pull requests.\\e[0m"
|
echo -e "\\n\\e[1;34mIntermediate PHP version $PHP is skipped for pull requests.\\e[0m"
|
||||||
break
|
break
|
||||||
fi
|
fi
|
||||||
@ -279,6 +277,17 @@ install:
|
|||||||
echo "$COMPONENTS" | parallel --gnu "tfold {} 'cd {} && ([ -e composer.lock ] && ${COMPOSER_UP/update/install} || $COMPOSER_UP --prefer-lowest --prefer-stable) && $PHPUNIT_X'"
|
echo "$COMPONENTS" | parallel --gnu "tfold {} 'cd {} && ([ -e composer.lock ] && ${COMPOSER_UP/update/install} || $COMPOSER_UP --prefer-lowest --prefer-stable) && $PHPUNIT_X'"
|
||||||
echo "$COMPONENTS" | xargs -n1 -I{} tar --append -f ~/php-ext/composer-lowest.lock.tar {}/composer.lock
|
echo "$COMPONENTS" | xargs -n1 -I{} tar --append -f ~/php-ext/composer-lowest.lock.tar {}/composer.lock
|
||||||
else
|
else
|
||||||
|
if [[ $PHP = 7.4* ]]; then
|
||||||
|
# add return types before running the test suite
|
||||||
|
rm vendor/symfony/contracts -Rf
|
||||||
|
ln -sd $(realpath src/Symfony/Contracts) vendor/symfony/contracts
|
||||||
|
sed -i 's/"\*\*\/Tests\/"//' composer.json
|
||||||
|
composer install --optimize-autoloader
|
||||||
|
php .github/patch-types.php
|
||||||
|
php .github/patch-types.php # ensure the script is idempotent
|
||||||
|
export PHPUNIT_X="$PHPUNIT_X,issue-32995,legacy"
|
||||||
|
fi
|
||||||
|
|
||||||
echo "$COMPONENTS" | parallel --gnu "tfold {} $PHPUNIT_X {}"
|
echo "$COMPONENTS" | parallel --gnu "tfold {} $PHPUNIT_X {}"
|
||||||
tfold src/Symfony/Component/Console.tty $PHPUNIT src/Symfony/Component/Console --group tty
|
tfold src/Symfony/Component/Console.tty $PHPUNIT src/Symfony/Component/Console --group tty
|
||||||
if [[ $PHP = ${MIN_PHP%.*} ]]; then
|
if [[ $PHP = ${MIN_PHP%.*} ]]; then
|
||||||
|
@ -100,7 +100,7 @@ class MongoDbSessionHandler extends AbstractSessionHandler
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return int
|
* @return bool
|
||||||
*/
|
*/
|
||||||
public function gc($maxlifetime)
|
public function gc($maxlifetime)
|
||||||
{
|
{
|
||||||
|
Reference in New Issue
Block a user